diff --git a/changelogs/unreleased/2322-nrb b/changelogs/unreleased/2322-nrb new file mode 100644 index 000000000..4faaa4721 --- /dev/null +++ b/changelogs/unreleased/2322-nrb @@ -0,0 +1 @@ +Fix CRD backup failures when fields contained a whole number. diff --git a/pkg/backup/remap_crd_version_action.go b/pkg/backup/remap_crd_version_action.go index 06c81cefd..63680b392 100644 --- a/pkg/backup/remap_crd_version_action.go +++ b/pkg/backup/remap_crd_version_action.go @@ -17,6 +17,8 @@ limitations under the License. package backup import ( + "encoding/json" + "github.com/pkg/errors" "github.com/sirupsen/logrus" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -62,8 +64,18 @@ func (a *RemapCRDVersionAction) Execute(item runtime.Unstructured, backup *v1.Ba // We've got a v1 CRD, so proceed. var crd apiextv1.CustomResourceDefinition - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(item.UnstructuredContent(), &crd); err != nil { - return nil, nil, errors.Wrap(err, "unable to convert unstructured item to a v1 CRD") + + // 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. + js, err := json.Marshal(item.UnstructuredContent()) + if err != nil { + return nil, nil, errors.Wrap(err, "unable to convert unstructured item to JSON") + } + + if err = json.Unmarshal(js, &crd); err != nil { + return nil, nil, errors.Wrap(err, "unable to convert JSON to CRD Go type") } log := a.logger.WithField("plugin", "RemapCRDVersionAction").WithField("CRD", crd.Name) diff --git a/pkg/backup/remap_crd_version_action_test.go b/pkg/backup/remap_crd_version_action_test.go index 99513d7de..e40409fe5 100644 --- a/pkg/backup/remap_crd_version_action_test.go +++ b/pkg/backup/remap_crd_version_action_test.go @@ -17,6 +17,7 @@ limitations under the License. package backup import ( + "encoding/json" "testing" "github.com/stretchr/testify/assert" @@ -58,4 +59,24 @@ func TestRemapCRDVersionAction(t *testing.T) { require.NoError(t, err) assert.Equal(t, "apiextensions.k8s.io/v1beta1", item.UnstructuredContent()["apiVersion"]) }) + + t.Run("Having an integer on a float64 field should work (issue 2319)", func(t *testing.T) { + 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) + + _, _, err = a.Execute(&u, backup) + require.NoError(t, err) + }) } diff --git a/pkg/builder/json_schema_props_builder.go b/pkg/builder/json_schema_props_builder.go new file mode 100644 index 000000000..2eafb8605 --- /dev/null +++ b/pkg/builder/json_schema_props_builder.go @@ -0,0 +1,43 @@ +/* +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 builder + +import ( + apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +) + +// JSONSchemaPropsBuilder builds JSONSchemaProps objects. +type JSONSchemaPropsBuilder struct { + object *apiextv1.JSONSchemaProps +} + +// ForJSONSchemaPropsBuilder is the constructor for a JSONSchemaPropsBuilder. +func ForJSONSchemaPropsBuilder() *JSONSchemaPropsBuilder { + return &JSONSchemaPropsBuilder{ + object: &apiextv1.JSONSchemaProps{}, + } +} + +// Maximum sets the Maximum field on a JSONSchemaPropsBuilder object. +func (b *JSONSchemaPropsBuilder) Maximum(f float64) *JSONSchemaPropsBuilder { + b.object.Maximum = &f + return b +} + +// Result returns the built object. +func (b *JSONSchemaPropsBuilder) Result() *apiextv1.JSONSchemaProps { + return b.object +} diff --git a/pkg/builder/v1_customresourcedefinition_builder.go b/pkg/builder/v1_customresourcedefinition_builder.go index 7249de7e6..55969a0d3 100644 --- a/pkg/builder/v1_customresourcedefinition_builder.go +++ b/pkg/builder/v1_customresourcedefinition_builder.go @@ -126,6 +126,14 @@ func (b *V1CustomResourceDefinitionVersionBuilder) Storage(s bool) *V1CustomReso return b } +func (b *V1CustomResourceDefinitionVersionBuilder) Schema(s *apiextv1.JSONSchemaProps) *V1CustomResourceDefinitionVersionBuilder { + if b.object.Schema == nil { + b.object.Schema = new(apiextv1.CustomResourceValidation) + } + b.object.Schema.OpenAPIV3Schema = s + return b +} + // Result returns the built CustomResourceDefinitionVersion. func (b *V1CustomResourceDefinitionVersionBuilder) Result() apiextv1.CustomResourceDefinitionVersion { return b.object diff --git a/pkg/util/kube/utils.go b/pkg/util/kube/utils.go index a78502b80..8e27974c8 100644 --- a/pkg/util/kube/utils.go +++ b/pkg/util/kube/utils.go @@ -157,6 +157,9 @@ func IsCRDReady(crd *apiextv1beta1.CustomResourceDefinition) bool { // TODO: Delete this function and use IsCRDReady when the upstream runtime.FromUnstructured function properly handles int64 field conversions. // Duplicated function because the velero install package uses IsCRDReady with the beta types. // See https://github.com/kubernetes/kubernetes/issues/87675 +// This is different from the fix for https://github.com/vmware-tanzu/velero/issues/2319 because here, +// we need to account for *both* v1beta1 and v1 CRDs, so doing marshalling into JSON to convert to a Go type may not be as useful here, unless we do +// type switching. func IsUnstructuredCRDReady(crd *unstructured.Unstructured) (bool, error) { var isEstablished, namesAccepted bool