Merge pull request #474 from juliocamarero/fix_crd_validation

Fix crd validation: Key should not be required anymore in bundle source resources
cert-manager · Nov 15, 2024 · 41dc93d · 41dc93d
2 parents 68fec39 + 18ba82c
commit 41dc93d
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 53 deletions.
diff --git a/deploy/charts/trust-manager/templates/crd-trust.cert-manager.io_bundles.yaml b/deploy/charts/trust-manager/templates/crd-trust.cert-manager.io_bundles.yaml
@@ -81,7 +81,7 @@ spec:
                               This field must not be true when `Key` is set.
                             type: boolean
                           key:
-                            description: Key is the key of the entry in the object's `data` field to be used.
+                            description: Key of the entry in the object's `data` field to be used.
                             type: string
                           name:
                             description: |-
@@ -134,8 +134,6 @@ spec:
                                 type: object
                             type: object
                             x-kubernetes-map-type: atomic
-                        required:
-                          - key
                         type: object
                       inLine:
                         description: InLine is a simple string to append as the source data.
@@ -151,7 +149,7 @@ spec:
                               This field must not be true when `Key` is set.
                             type: boolean
                           key:
-                            description: Key is the key of the entry in the object's `data` field to be used.
+                            description: Key of the entry in the object's `data` field to be used.
                             type: string
                           name:
                             description: |-
@@ -204,8 +202,6 @@ spec:
                                 type: object
                             type: object
                             x-kubernetes-map-type: atomic
-                        required:
-                          - key
                         type: object
                       useDefaultCAs:
                         description: |-

diff --git a/docs/api/api.md b/docs/api/api.md
@@ -633,9 +633,9 @@ type SourceObjectKeySelector struct {
     //+optional
     Selector *metav1.LabelSelector `json:"selector,omitempty"`
 
-    // KeySelector is the key of the entry in the objects' `data` field to be referenced.
+    // Key of the entry in the object's `data` field to be used.
     //+optional
-    KeySelector `json:",inline,omitempty"`
+    Key string `json:"key,omitempty"`
 
     // IncludeAllKeys is a flag to include all keys in the object's `data` field to be used. False by default.
     // This field must not be true when `Key` is set.
@@ -645,7 +645,7 @@ type SourceObjectKeySelector struct {
 ```
 
 <a name="SourceObjectKeySelector.DeepCopy"></a>
-### func \(\*SourceObjectKeySelector\) [DeepCopy](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L339>)
+### func \(\*SourceObjectKeySelector\) [DeepCopy](<https://github.com/cert-manager/trust-manager/blob/main/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go#L338>)
 
 ```go
 func (in *SourceObjectKeySelector) DeepCopy() *SourceObjectKeySelector

diff --git a/pkg/apis/trust/v1alpha1/types_bundle.go b/pkg/apis/trust/v1alpha1/types_bundle.go
@@ -169,9 +169,9 @@ type SourceObjectKeySelector struct {
 	//+optional
 	Selector *metav1.LabelSelector `json:"selector,omitempty"`
 
-	// KeySelector is the key of the entry in the objects' `data` field to be referenced.
+	// Key of the entry in the object's `data` field to be used.
 	//+optional
-	KeySelector `json:",inline,omitempty"`
+	Key string `json:"key,omitempty"`
 
 	// IncludeAllKeys is a flag to include all keys in the object's `data` field to be used. False by default.
 	// This field must not be true when `Key` is set.

diff --git a/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go
@@ -113,8 +113,8 @@ func Test_Reconcile(t *testing.T) {
 			},
 			Spec: trustapi.BundleSpec{
 				Sources: []trustapi.BundleSource{
-					{ConfigMap: &trustapi.SourceObjectKeySelector{Name: sourceConfigMapName, KeySelector: trustapi.KeySelector{Key: sourceConfigMapKey}}},
-					{Secret: &trustapi.SourceObjectKeySelector{Name: sourceSecretName, KeySelector: trustapi.KeySelector{Key: sourceSecretKey}}},
+					{ConfigMap: &trustapi.SourceObjectKeySelector{Name: sourceConfigMapName, Key: sourceConfigMapKey}},
+					{Secret: &trustapi.SourceObjectKeySelector{Name: sourceSecretName, Key: sourceSecretKey}},
 					{InLine: ptr.To(dummy.TestCertificate3)},
 				},
 				Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: targetKey}},

diff --git a/pkg/bundle/source_test.go b/pkg/bundle/source_test.go
@@ -83,7 +83,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if single ConfigMap source which doesn't exist, return notFoundError": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 			},
 			objects:          []runtime.Object{},
 			expData:          "",
@@ -92,7 +92,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if single ConfigMap source whose key doesn't exist, return notFoundError": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 			},
 			objects:          []runtime.Object{&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "configmap"}}},
 			expData:          "",
@@ -101,7 +101,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if single ConfigMap source referencing single key, return data": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 			},
 			objects: []runtime.Object{&corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
@@ -126,7 +126,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		"if single ConfigMap source, return data even when order changes": {
 			// Test uses the same data as the previous one but with different order
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 			},
 			objects: []runtime.Object{&corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
@@ -138,7 +138,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if selects no ConfigMap sources, should return an error": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{KeySelector: trustapi.KeySelector{Key: "key"}, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"selects-nothing": "true"}}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Key: "key", Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"selects-nothing": "true"}}}},
 			},
 			objects:          []runtime.Object{},
 			expData:          "",
@@ -147,8 +147,8 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if selects at least one ConfigMap source, return data": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{KeySelector: trustapi.KeySelector{Key: "key"}, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"trust-bundle.certs": "includes"}}}},
-				{ConfigMap: &trustapi.SourceObjectKeySelector{KeySelector: trustapi.KeySelector{Key: "key"}, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"selects-nothing": "true"}}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Key: "key", Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"trust-bundle.certs": "includes"}}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Key: "key", Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"selects-nothing": "true"}}}},
 			},
 			objects: []runtime.Object{&corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{Name: "configmap", Labels: map[string]string{"trust-bundle.certs": "includes"}},
@@ -182,7 +182,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if ConfigMap and InLine source, return concatenated data": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 				{InLine: ptr.To(dummy.TestCertificate2)},
 			},
 			objects: []runtime.Object{&corev1.ConfigMap{
@@ -195,7 +195,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if single Secret source exists which doesn't exist, should return not found error": {
 			sources: []trustapi.BundleSource{
-				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", Key: "key"}},
 			},
 			objects:          []runtime.Object{},
 			expData:          "",
@@ -204,7 +204,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if single Secret source whose key doesn't exist, return notFoundError": {
 			sources: []trustapi.BundleSource{
-				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", Key: "key"}},
 			},
 			objects:          []runtime.Object{&corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "secret"}}},
 			expData:          "",
@@ -226,7 +226,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if single Secret source referencing single key, return data": {
 			sources: []trustapi.BundleSource{
-				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", Key: "key"}},
 			},
 			objects: []runtime.Object{&corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "secret"},
@@ -250,7 +250,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if Secret and InLine source, return concatenated data": {
 			sources: []trustapi.BundleSource{
-				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", Key: "key"}},
 				{InLine: ptr.To(dummy.TestCertificate1)},
 			},
 			objects: []runtime.Object{&corev1.Secret{
@@ -263,9 +263,9 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if Secret, ConfigMap and InLine source, return concatenated data": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 				{InLine: ptr.To(dummy.TestCertificate3)},
-				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", Key: "key"}},
 			},
 			objects: []runtime.Object{
 				&corev1.ConfigMap{
@@ -283,8 +283,8 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if source Secret exists, but not ConfigMap, return not found error": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
-				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
+				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", Key: "key"}},
 			},
 			objects: []runtime.Object{
 				&corev1.ConfigMap{
@@ -298,8 +298,8 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if source ConfigMap exists, but not Secret, return not found error": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
-				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
+				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", Key: "key"}},
 			},
 			objects: []runtime.Object{
 				&corev1.Secret{
@@ -358,7 +358,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if has JKS target, return binaryData with encoded JKS": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 			},
 			formats: &trustapi.AdditionalFormats{
 				JKS: &trustapi.JKS{
@@ -377,7 +377,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if has JKS target with arbitrary password, return binaryData with encoded JKS": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 			},
 			formats: &trustapi.AdditionalFormats{
 				JKS: &trustapi.JKS{
@@ -397,7 +397,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if has PKCS12 target, return binaryData with encoded PKCS12": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 			},
 			formats: &trustapi.AdditionalFormats{
 				PKCS12: &trustapi.PKCS12{
@@ -416,7 +416,7 @@ func Test_buildSourceBundle(t *testing.T) {
 		},
 		"if has PKCS12 target with arbitrary password, return binaryData with encoded PKCS12": {
 			sources: []trustapi.BundleSource{
-				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", Key: "key"}},
 			},
 			formats: &trustapi.AdditionalFormats{
 				PKCS12: &trustapi.PKCS12{

diff --git a/pkg/webhook/validation_test.go b/pkg/webhook/validation_test.go
@@ -55,13 +55,13 @@ func Test_validate(t *testing.T) {
 				Spec: trustapi.BundleSpec{
 					Sources: []trustapi.BundleSource{
 						{
-							ConfigMap: &trustapi.SourceObjectKeySelector{Name: "test", KeySelector: trustapi.KeySelector{Key: "test"}},
+							ConfigMap: &trustapi.SourceObjectKeySelector{Name: "test", Key: "test"},
 							InLine:    ptr.To("test"),
 						},
 						{InLine: ptr.To("test")},
 						{
-							ConfigMap: &trustapi.SourceObjectKeySelector{Name: "test", KeySelector: trustapi.KeySelector{Key: "test"}},
-							Secret:    &trustapi.SourceObjectKeySelector{Name: "test", KeySelector: trustapi.KeySelector{Key: "test"}},
+							ConfigMap: &trustapi.SourceObjectKeySelector{Name: "test", Key: "test"},
+							Secret:    &trustapi.SourceObjectKeySelector{Name: "test", Key: "test"},
 						},
 					},
 					Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}},
@@ -144,9 +144,9 @@ func Test_validate(t *testing.T) {
 			bundle: &trustapi.Bundle{
 				Spec: trustapi.BundleSpec{
 					Sources: []trustapi.BundleSource{
-						{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "", KeySelector: trustapi.KeySelector{Key: ""}}},
+						{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "", Key: ""}},
 						{InLine: ptr.To("test")},
-						{Secret: &trustapi.SourceObjectKeySelector{Name: "", KeySelector: trustapi.KeySelector{Key: ""}}},
+						{Secret: &trustapi.SourceObjectKeySelector{Name: "", Key: ""}},
 					},
 					Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}},
 				},
@@ -162,9 +162,9 @@ func Test_validate(t *testing.T) {
 			bundle: &trustapi.Bundle{
 				Spec: trustapi.BundleSpec{
 					Sources: []trustapi.BundleSource{
-						{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "some-config-map", Selector: &metav1.LabelSelector{}, KeySelector: trustapi.KeySelector{Key: "test"}}},
+						{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "some-config-map", Selector: &metav1.LabelSelector{}, Key: "test"}},
 						{InLine: ptr.To("test")},
-						{Secret: &trustapi.SourceObjectKeySelector{Name: "some-secret", Selector: &metav1.LabelSelector{}, KeySelector: trustapi.KeySelector{Key: "test"}}},
+						{Secret: &trustapi.SourceObjectKeySelector{Name: "some-secret", Selector: &metav1.LabelSelector{}, Key: "test"}},
 					},
 					Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}},
 				},
@@ -178,9 +178,9 @@ func Test_validate(t *testing.T) {
 			bundle: &trustapi.Bundle{
 				Spec: trustapi.BundleSpec{
 					Sources: []trustapi.BundleSource{
-						{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "some-config-map", KeySelector: trustapi.KeySelector{Key: "test"}, IncludeAllKeys: true}},
+						{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "some-config-map", Key: "test", IncludeAllKeys: true}},
 						{InLine: ptr.To("test")},
-						{Secret: &trustapi.SourceObjectKeySelector{Name: "some-secret", KeySelector: trustapi.KeySelector{Key: "test"}, IncludeAllKeys: true}},
+						{Secret: &trustapi.SourceObjectKeySelector{Name: "some-secret", Key: "test", IncludeAllKeys: true}},
 					},
 					Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}},
 				},
@@ -196,7 +196,7 @@ func Test_validate(t *testing.T) {
 				Spec: trustapi.BundleSpec{
 					Sources: []trustapi.BundleSource{
 						{InLine: ptr.To("test")},
-						{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "test-bundle", KeySelector: trustapi.KeySelector{Key: "test"}}},
+						{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "test-bundle", Key: "test"}},
 					},
 					Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}},
 				},
@@ -211,7 +211,7 @@ func Test_validate(t *testing.T) {
 				Spec: trustapi.BundleSpec{
 					Sources: []trustapi.BundleSource{
 						{InLine: ptr.To("test")},
-						{Secret: &trustapi.SourceObjectKeySelector{Name: "test-bundle", KeySelector: trustapi.KeySelector{Key: "test"}}},
+						{Secret: &trustapi.SourceObjectKeySelector{Name: "test-bundle", Key: "test"}},
 					},
 					Target: trustapi.BundleTarget{Secret: &trustapi.KeySelector{Key: "test"}},
 				},

diff --git a/test/env/data.go b/test/env/data.go
@@ -111,15 +111,15 @@ func newTestBundle(ctx context.Context, cl client.Client, opts bundlectrl.Option
 			Sources: []trustapi.BundleSource{
 				{
 					ConfigMap: &trustapi.SourceObjectKeySelector{
-						Name:        configMap.Name,
-						KeySelector: trustapi.KeySelector{Key: td.Sources.ConfigMap.Key},
+						Name: configMap.Name,
+						Key:  td.Sources.ConfigMap.Key,
 					},
 				},
 
 				{
 					Secret: &trustapi.SourceObjectKeySelector{
-						Name:        secret.Name,
-						KeySelector: trustapi.KeySelector{Key: td.Sources.Secret.Key},
+						Name: secret.Name,
+						Key:  td.Sources.Secret.Key,
 					},
 				},
 

diff --git a/test/integration/bundle/suite.go b/test/integration/bundle/suite.go
@@ -174,7 +174,7 @@ var _ = Describe("Integration", func() {
 
 		Expect(komega.Update(testBundle, func() {
 			testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{
-				ConfigMap: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", KeySelector: trustapi.KeySelector{Key: "new-source-key"}},
+				ConfigMap: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", Key: "new-source-key"},
 			})
 		})()).To(Succeed())
 
@@ -219,7 +219,7 @@ var _ = Describe("Integration", func() {
 
 		Expect(komega.Update(testBundle, func() {
 			testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{
-				Secret: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", KeySelector: trustapi.KeySelector{Key: "new-source-key"}},
+				Secret: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", Key: "new-source-key"},
 			})
 		})()).To(Succeed())