diff --git a/changelogs/unreleased/2484-skriss b/changelogs/unreleased/2484-skriss new file mode 100644 index 000000000..afa5fa3de --- /dev/null +++ b/changelogs/unreleased/2484-skriss @@ -0,0 +1 @@ +bug fix: in CRD restore plugin, don't use runtime.DefaultUnstructuredConverter.FromUnstructured(...) to avoid conversion issues when float64 fields contain int values diff --git a/pkg/restore/crd_v1_preserve_unknown_fields_action.go b/pkg/restore/crd_v1_preserve_unknown_fields_action.go index ebe12c787..f67d47910 100644 --- a/pkg/restore/crd_v1_preserve_unknown_fields_action.go +++ b/pkg/restore/crd_v1_preserve_unknown_fields_action.go @@ -17,6 +17,8 @@ limitations under the License. package restore import ( + "encoding/json" + "github.com/pkg/errors" "github.com/sirupsen/logrus" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -65,10 +67,13 @@ func (c *CRDV1PreserveUnknownFieldsAction) Execute(input *velero.RestoreItemActi }, nil } - var crd apiextv1.CustomResourceDefinition - - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(input.Item.UnstructuredContent(), &crd); err != nil { - return nil, errors.Wrap(err, "unable to convert unstructured item to custom resource definition") + // Do not use runtime.DefaultUnstructuredConverter.FromUnstructured here because it has a bug when converting integers/whole + // numbers in float fields (https://github.com/kubernetes/kubernetes/issues/87675). + // Using JSON as a go-between avoids this issue, without adding a bunch of type conversion by using unstructured helper functions + // to inspect the fields we want to look at. + crd, err := fromUnstructured(input.Item.UnstructuredContent()) + if err != nil { + return nil, errors.Wrap(err, "unable to convert CRD from unstructured to structured") } // The v1 API doesn't allow the PreserveUnknownFields value to be true, so make sure the schema flag is set instead @@ -101,3 +106,18 @@ func (c *CRDV1PreserveUnknownFieldsAction) Execute(input *velero.RestoreItemActi UpdatedItem: &unstructured.Unstructured{Object: res}, }, nil } + +func fromUnstructured(unstructured map[string]interface{}) (*apiextv1.CustomResourceDefinition, error) { + var crd apiextv1.CustomResourceDefinition + + js, err := json.Marshal(unstructured) + if err != nil { + return nil, errors.Wrap(err, "unable to convert unstructured item to JSON") + } + + if err = json.Unmarshal(js, &crd); err != nil { + return nil, errors.Wrap(err, "unable to convert JSON to CRD Go type") + } + + return &crd, nil +} diff --git a/pkg/restore/crd_v1_preserve_unknown_fields_action_test.go b/pkg/restore/crd_v1_preserve_unknown_fields_action_test.go new file mode 100644 index 000000000..77045b596 --- /dev/null +++ b/pkg/restore/crd_v1_preserve_unknown_fields_action_test.go @@ -0,0 +1,53 @@ +/* +Copyright 2020 the Velero contributors. + + Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restore + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/vmware-tanzu/velero/pkg/builder" + "github.com/vmware-tanzu/velero/pkg/plugin/velero" + "github.com/vmware-tanzu/velero/pkg/test" +) + +func TestExecuteForACRDWithAnIntOnAFloat64FieldShouldWork(t *testing.T) { + // ref. reopen of https://github.com/vmware-tanzu/velero/issues/2319 + + b := builder.ForV1CustomResourceDefinition("test.velero.io") + // 5 here is just an int value, it could be any other whole number. + schema := builder.ForJSONSchemaPropsBuilder().Maximum(5).Result() + b.Version(builder.ForV1CustomResourceDefinitionVersion("v1").Served(true).Storage(true).Schema(schema).Result()) + c := b.Result() + + // Marshall in and out of JSON because the problem doesn't manifest when we use ToUnstructured directly + // This should simulate the JSON passing over the wire in an HTTP request/response with a dynamic client + js, err := json.Marshal(c) + require.NoError(t, err) + + var u unstructured.Unstructured + err = json.Unmarshal(js, &u) + require.NoError(t, err) + + a := NewCRDV1PreserveUnknownFieldsAction(test.NewLogger()) + + _, err = a.Execute(&velero.RestoreItemActionExecuteInput{Item: &u}) + require.NoError(t, err) +}