Merge pull request #2136 from ecordell/hive-val-bug

Fix CR validation bug on go 1.16

Author: openshift-merge-robot

go.mod

@@ -41,6 +41,7 @@ require (
 	google.golang.org/grpc v1.30.0
 	gopkg.in/yaml.v2 v2.4.0
 	helm.sh/helm/v3 v3.1.0-rc.1.0.20201215141456-e71d38b414eb
+	// can't update to 0.21 until https://github.com/kubernetes/apiserver/issues/65 is resolved
 	k8s.io/api v0.20.6
 	k8s.io/apiextensions-apiserver v0.20.6
 	k8s.io/apimachinery v0.20.6
@@ -50,7 +51,7 @@ require (
 	k8s.io/component-base v0.20.6
 	k8s.io/klog v1.0.0
 	k8s.io/kube-aggregator v0.20.4
-	k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd
+	k8s.io/kube-openapi v0.0.0-20210305001622-591a79e4bda7
 	k8s.io/utils v0.0.0-20210111153108-fddb29f9d009
 	rsc.io/letsencrypt v0.0.3 // indirect
 	sigs.k8s.io/controller-runtime v0.8.3

go.sum

@@ -1439,8 +1439,9 @@ k8s.io/kube-aggregator v0.20.4 h1:j/SUwPy1eO+ud3XOUGmH18gISPyerqhXOoNRZDbv3fs=
 k8s.io/kube-aggregator v0.20.4/go.mod h1:0ixQ9De7KXyHteXizS6nVtrnKqGa4kiuxl9rEBsNccw=
 k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c/go.mod h1:GRQhZsXIAJ1xR0C9bd8UpWHZ5plfAS9fzPjJuQ6JL3E=
 k8s.io/kube-openapi v0.0.0-20200805222855-6aeccd4b50c6/go.mod h1:UuqjUnNftUyPE5H64/qeyjQoUZhGpeFDVdxjTeEVN2o=
-k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd h1:sOHNzJIkytDF6qadMNKhhDRpc6ODik8lVC6nOur7B2c=
 k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd/go.mod h1:WOJ3KddDSol4tAGcJo0Tvi+dK12EcqSLqcWsryKMpfM=
+k8s.io/kube-openapi v0.0.0-20210305001622-591a79e4bda7 h1:vEx13qjvaZ4yfObSSXW7BrMc/KQBBT/Jyee8XtLf4x0=
+k8s.io/kube-openapi v0.0.0-20210305001622-591a79e4bda7/go.mod h1:wXW5VT87nVfh/iLV8FpR2uDvrFyomxbtb1KivDbvPTE=
 k8s.io/kubectl v0.18.0/go.mod h1:LOkWx9Z5DXMEg5KtOjHhRiC1fqJPLyCr3KtQgEolCkU=
 k8s.io/kubectl v0.20.0 h1:q6HH6jILYi2lkzFqBhs63M4bKLxYlM0HpFJ///MgARA=
 k8s.io/kubectl v0.20.0/go.mod h1:8x5GzQkgikz7M2eFGGuu6yOfrenwnw5g4RXOUgbjR1M=

pkg/controller/operators/catalog/operator.go

@@ -1600,7 +1600,7 @@ func validateExistingCRs(dynamicClient dynamic.Interface, gvr schema.GroupVersio
 		}
 		err = validation.ValidateCustomResource(field.NewPath(""), cr.UnstructuredContent(), validator).ToAggregate()
 		if err != nil {
-			return fmt.Errorf("error validating custom resource against new schema %#v: %s", newCRD.Spec.Validation, err)
+			return fmt.Errorf("error validating custom resource against new schema for %s %s/%s: %v", newCRD.Spec.Names.Kind, cr.GetNamespace(), cr.GetName(), err)
 		}
 	}
 	return nil

pkg/controller/operators/catalog/operator_test.go

@@ -13,6 +13,8 @@ import (
 	"testing/quick"
 	"time"
 
+	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/time/rate"
@@ -30,6 +32,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	utilclock "k8s.io/apimachinery/pkg/util/clock"
+	k8syaml "k8s.io/apimachinery/pkg/util/yaml"
 	utilyaml "k8s.io/apimachinery/pkg/util/yaml"
 	"k8s.io/apiserver/pkg/storage/names"
 	fakedynamic "k8s.io/client-go/dynamic/fake"
@@ -1284,6 +1287,70 @@ func TestCompetingCRDOwnersExist(t *testing.T) {
 	}
 }
 
+func TestValidateExistingCRs(t *testing.T) {
+	unstructuredForFile := func(file string) *unstructured.Unstructured {
+		data, err := ioutil.ReadFile(file)
+		require.NoError(t, err)
+		dec := k8syaml.NewYAMLOrJSONDecoder(strings.NewReader(string(data)), 30)
+		k8sFile := &unstructured.Unstructured{}
+		require.NoError(t, dec.Decode(k8sFile))
+		return k8sFile
+	}
+
+	unversionedCRDForV1beta1File := func(file string) *apiextensions.CustomResourceDefinition {
+		data, err := ioutil.ReadFile(file)
+		require.NoError(t, err)
+		dec := k8syaml.NewYAMLOrJSONDecoder(strings.NewReader(string(data)), 30)
+		k8sFile := &apiextensionsv1beta1.CustomResourceDefinition{}
+		require.NoError(t, dec.Decode(k8sFile))
+		convertedCRD := &apiextensions.CustomResourceDefinition{}
+		require.NoError(t, apiextensionsv1beta1.Convert_v1beta1_CustomResourceDefinition_To_apiextensions_CustomResourceDefinition(k8sFile, convertedCRD, nil))
+		return convertedCRD
+	}
+
+	tests := []struct {
+		name            string
+		existingObjects []runtime.Object
+		gvr             schema.GroupVersionResource
+		newCRD          *apiextensions.CustomResourceDefinition
+		want            error
+	}{
+		{
+			name: "label validation",
+			existingObjects: []runtime.Object{
+				unstructuredForFile("testdata/hivebug/cr.yaml"),
+			},
+			gvr: schema.GroupVersionResource{
+				Group:    "hive.openshift.io",
+				Version:  "v1",
+				Resource: "machinepools",
+			},
+			newCRD: unversionedCRDForV1beta1File("testdata/hivebug/crd.yaml"),
+		},
+		{
+			name: "fail validation",
+			existingObjects: []runtime.Object{
+				unstructuredForFile("testdata/hivebug/fail.yaml"),
+			},
+			gvr: schema.GroupVersionResource{
+				Group:    "hive.openshift.io",
+				Version:  "v1",
+				Resource: "machinepools",
+			},
+			newCRD: unversionedCRDForV1beta1File("testdata/hivebug/crd.yaml"),
+			want:   fmt.Errorf("error validating custom resource against new schema for MachinePool /test: [[].spec.clusterDeploymentRef: Invalid value: \"null\": spec.clusterDeploymentRef in body must be of type object: \"null\", [].spec.name: Required value, [].spec.platform: Required value]"),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := fakedynamic.NewSimpleDynamicClientWithCustomListKinds(runtime.NewScheme(), map[schema.GroupVersionResource]string{
+				tt.gvr: "UnstructuredList",
+			}, tt.existingObjects...)
+			require.Equal(t, tt.want, validateExistingCRs(client, tt.gvr, tt.newCRD))
+		})
+	}
+}
+
 func fakeConfigMapData() map[string]string {
 	data := make(map[string]string)
 	yaml, err := yaml.Marshal([]apiextensionsv1beta1.CustomResourceDefinition{crd("fake-crd")})

pkg/controller/operators/catalog/testdata/hivebug/cr.yaml

@@ -0,0 +1,18 @@
+apiVersion: hive.openshift.io/v1
+kind: MachinePool
+metadata:
+  name: test
+spec:
+  name: "test"
+  platform:
+    aws:
+      rootVolume:
+        iops: 1
+        size: 1
+        type: a
+      type: m4-large
+  clusterDeploymentRef:
+    name: osd-abc
+  labels:
+    node-role.kubernetes.io: infra
+    node-role.kubernetes.io/infra: ""