From 34740e08853257ab6edcce2ca3ab9b1a425cb3d0 Mon Sep 17 00:00:00 2001 From: Julio Camarero Date: Tue, 22 Oct 2024 14:26:34 +0200 Subject: [PATCH 1/6] Introduce API change Signed-off-by: Julio Camarero --- pkg/apis/trust/v1alpha1/types_bundle.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/apis/trust/v1alpha1/types_bundle.go b/pkg/apis/trust/v1alpha1/types_bundle.go index 7e26a0d9..eafbbf27 100644 --- a/pkg/apis/trust/v1alpha1/types_bundle.go +++ b/pkg/apis/trust/v1alpha1/types_bundle.go @@ -68,13 +68,13 @@ type BundleSpec struct { // BundleSource is the set of sources whose data will be appended and synced to // the BundleTarget in all Namespaces. type BundleSource struct { - // ConfigMap is a reference (by name) to a ConfigMap's `data` key, or to a - // list of ConfigMap's `data` key using label selector, in the trust Namespace. + // ConfigMap is a reference (by name) to a ConfigMap's `data` key(s), or to a + // list of ConfigMap's `data` key(s) using label selector, in the trust Namespace. // +optional ConfigMap *SourceObjectKeySelector `json:"configMap,omitempty"` - // Secret is a reference (by name) to a Secret's `data` key, or to a - // list of Secret's `data` key using label selector, in the trust Namespace. + // Secret is a reference (by name) to a Secret's `data` key(s), or to a + // list of Secret's `data` key(s) using label selector, in the trust Namespace. // +optional Secret *SourceObjectKeySelector `json:"secret,omitempty"` @@ -156,7 +156,7 @@ type NamespaceSelector struct { MatchLabels map[string]string `json:"matchLabels,omitempty"` } -// SourceObjectKeySelector is a reference to a source object and its `data` key +// SourceObjectKeySelector is a reference to a source object and its `data` key(s) // in the trust Namespace. type SourceObjectKeySelector struct { // Name is the name of the source object in the trust Namespace. @@ -170,7 +170,13 @@ type SourceObjectKeySelector struct { Selector *metav1.LabelSelector `json:"selector,omitempty"` // KeySelector is the key of the entry in the objects' `data` field to be referenced. - KeySelector `json:",inline"` + //+optional + KeySelector `json:",inline,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. + //+optional + IncludeAllKeys bool `json:"includeAllKeys,omitempty"` } // KeySelector is a reference to a key for some map data object. From 054f6c37757deb65d4cbf4025ca9655bb77aebf1 Mon Sep 17 00:00:00 2001 From: Julio Camarero Date: Tue, 22 Oct 2024 14:28:26 +0200 Subject: [PATCH 2/6] autogenerated changes Signed-off-by: Julio Camarero --- .../crd-trust.cert-manager.io_bundles.yaml | 18 ++++++++++--- docs/api/api.md | 26 ++++++++++++------- 2 files changed, 30 insertions(+), 14 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 index 799827d1..9e01c938 100644 --- 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 @@ -72,9 +72,14 @@ spec: properties: configMap: description: |- - ConfigMap is a reference (by name) to a ConfigMap's `data` key, or to a - list of ConfigMap's `data` key using label selector, in the trust Namespace. + ConfigMap is a reference (by name) to a ConfigMap's `data` key(s), or to a + list of ConfigMap's `data` key(s) using label selector, in the trust Namespace. properties: + includeAllKeys: + description: |- + 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. + type: boolean key: description: Key is the key of the entry in the object's `data` field to be used. type: string @@ -137,9 +142,14 @@ spec: type: string secret: description: |- - Secret is a reference (by name) to a Secret's `data` key, or to a - list of Secret's `data` key using label selector, in the trust Namespace. + Secret is a reference (by name) to a Secret's `data` key(s), or to a + list of Secret's `data` key(s) using label selector, in the trust Namespace. properties: + includeAllKeys: + description: |- + 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. + type: boolean key: description: Key is the key of the entry in the object's `data` field to be used. type: string diff --git a/docs/api/api.md b/docs/api/api.md index de4bcc05..e6abce41 100644 --- a/docs/api/api.md +++ b/docs/api/api.md @@ -206,7 +206,7 @@ func (in *Bundle) DeepCopyObject() runtime.Object DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. -## type [BundleCondition]() +## type [BundleCondition]() BundleCondition contains condition information for a Bundle. @@ -319,13 +319,13 @@ BundleSource is the set of sources whose data will be appended and synced to the ```go type BundleSource struct { - // ConfigMap is a reference (by name) to a ConfigMap's `data` key, or to a - // list of ConfigMap's `data` key using label selector, in the trust Namespace. + // ConfigMap is a reference (by name) to a ConfigMap's `data` key(s), or to a + // list of ConfigMap's `data` key(s) using label selector, in the trust Namespace. // +optional ConfigMap *SourceObjectKeySelector `json:"configMap,omitempty"` - // Secret is a reference (by name) to a Secret's `data` key, or to a - // list of Secret's `data` key using label selector, in the trust Namespace. + // Secret is a reference (by name) to a Secret's `data` key(s), or to a + // list of Secret's `data` key(s) using label selector, in the trust Namespace. // +optional Secret *SourceObjectKeySelector `json:"secret,omitempty"` @@ -398,7 +398,7 @@ func (in *BundleSpec) DeepCopyInto(out *BundleSpec) DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil. -## type [BundleStatus]() +## type [BundleStatus]() BundleStatus defines the observed state of the Bundle. @@ -520,7 +520,7 @@ func (in *JKS) DeepCopyInto(out *JKS) DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil. -## type [KeySelector]() +## type [KeySelector]() KeySelector is a reference to a key for some map data object. @@ -617,9 +617,9 @@ func (in *PKCS12) DeepCopyInto(out *PKCS12) DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non\-nil. -## type [SourceObjectKeySelector]() +## type [SourceObjectKeySelector]() -SourceObjectKeySelector is a reference to a source object and its \`data\` key in the trust Namespace. +SourceObjectKeySelector is a reference to a source object and its \`data\` key\(s\) in the trust Namespace. ```go type SourceObjectKeySelector struct { @@ -634,7 +634,13 @@ type SourceObjectKeySelector struct { Selector *metav1.LabelSelector `json:"selector,omitempty"` // KeySelector is the key of the entry in the objects' `data` field to be referenced. - KeySelector `json:",inline"` + //+optional + KeySelector `json:",inline,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. + //+optional + IncludeAllKeys bool `json:"includeAllKeys,omitempty"` } ``` From 5362ff721a73eef23cc7fb1337d08e779a04d58f Mon Sep 17 00:00:00 2001 From: Julio Camarero Date: Tue, 22 Oct 2024 14:58:08 +0200 Subject: [PATCH 3/6] add validation for new field Signed-off-by: Julio Camarero --- pkg/webhook/validation.go | 14 +++++++++---- pkg/webhook/validation_test.go | 38 ++++++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/pkg/webhook/validation.go b/pkg/webhook/validation.go index eb837116..c5f21059 100644 --- a/pkg/webhook/validation.go +++ b/pkg/webhook/validation.go @@ -105,8 +105,11 @@ func (v *validator) validate(obj runtime.Object) (admission.Warnings, error) { if len(configMap.Name) > 0 && configMap.Selector != nil { el = append(el, field.Invalid(path, fmt.Sprintf("name: %s, selector: {}", configMap.Name), "must validate one and only one schema (oneOf): [name, selector]. Found both set")) } - if len(configMap.Key) == 0 { - el = append(el, field.Invalid(path.Child("key"), configMap.Key, "source configMap key must be defined")) + if len(configMap.Key) == 0 && !configMap.IncludeAllKeys { + el = append(el, field.Invalid(path, fmt.Sprintf("key: ' ', includeAllKeys: %t", configMap.IncludeAllKeys), "source configMap key must be defined when includeAllKeys is false")) + } + if len(configMap.Key) > 0 && configMap.IncludeAllKeys { + el = append(el, field.Invalid(path, fmt.Sprintf("key: %s, includeAllKeys: %t", configMap.Key, configMap.IncludeAllKeys), "source configMap key cannot be defined when includeAllKeys is true")) } } @@ -121,8 +124,11 @@ func (v *validator) validate(obj runtime.Object) (admission.Warnings, error) { if len(secret.Name) > 0 && secret.Selector != nil { el = append(el, field.Invalid(path, fmt.Sprintf("name: %s, selector: {}", secret.Name), "must validate one and only one schema (oneOf): [name, selector]. Found both set")) } - if len(secret.Key) == 0 { - el = append(el, field.Invalid(path.Child("key"), secret.Key, "source secret key must be defined")) + if len(secret.Key) == 0 && !secret.IncludeAllKeys { + el = append(el, field.Invalid(path, fmt.Sprintf("key: ' ', includeAllKeys: %t", secret.IncludeAllKeys), "source secret key must be defined when includeAllKeys is false")) + } + if len(secret.Key) > 0 && secret.IncludeAllKeys { + el = append(el, field.Invalid(path, fmt.Sprintf("key: %s, includeAllKeys: %t", secret.Key, secret.IncludeAllKeys), "source secret key cannot be defined when includeAllKeys is true")) } } diff --git a/pkg/webhook/validation_test.go b/pkg/webhook/validation_test.go index 1aab0a7c..a7b6f285 100644 --- a/pkg/webhook/validation_test.go +++ b/pkg/webhook/validation_test.go @@ -153,9 +153,9 @@ func Test_validate(t *testing.T) { }, expErr: ptr.To(field.ErrorList{ field.Invalid(field.NewPath("spec", "sources", "[0]", "configMap"), "name: ' ', selector: nil", "must validate one and only one schema (oneOf): [name, selector]. Found none valid"), - field.Invalid(field.NewPath("spec", "sources", "[0]", "configMap", "key"), "", "source configMap key must be defined"), + field.Invalid(field.NewPath("spec", "sources", "[0]", "configMap"), "key: ' ', includeAllKeys: false", "source configMap key must be defined when includeAllKeys is false"), field.Invalid(field.NewPath("spec", "sources", "[2]", "secret"), "name: ' ', selector: nil", "must validate one and only one schema (oneOf): [name, selector]. Found none valid"), - field.Invalid(field.NewPath("spec", "sources", "[2]", "secret", "key"), "", "source secret key must be defined"), + field.Invalid(field.NewPath("spec", "sources", "[2]", "secret"), "key: ' ', includeAllKeys: false", "source secret key must be defined when includeAllKeys is false"), }.ToAggregate().Error()), }, "sources names and selectors are both set": { @@ -174,6 +174,22 @@ func Test_validate(t *testing.T) { field.Invalid(field.NewPath("spec", "sources", "[2]", "secret"), "name: some-secret, selector: {}", "must validate one and only one schema (oneOf): [name, selector]. Found both set"), }.ToAggregate().Error()), }, + "sources key is set and includeAllKeys is true": { + bundle: &trustapi.Bundle{ + Spec: trustapi.BundleSpec{ + Sources: []trustapi.BundleSource{ + {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "some-config-map", KeySelector: trustapi.KeySelector{Key: "test"}, IncludeAllKeys: true}}, + {InLine: ptr.To("test")}, + {Secret: &trustapi.SourceObjectKeySelector{Name: "some-secret", KeySelector: trustapi.KeySelector{Key: "test"}, IncludeAllKeys: true}}, + }, + Target: trustapi.BundleTarget{ConfigMap: &trustapi.KeySelector{Key: "test"}}, + }, + }, + expErr: ptr.To(field.ErrorList{ + field.Invalid(field.NewPath("spec", "sources", "[0]", "configMap"), "key: test, includeAllKeys: true", "source configMap key cannot be defined when includeAllKeys is true"), + field.Invalid(field.NewPath("spec", "sources", "[2]", "secret"), "key: test, includeAllKeys: true", "source secret key cannot be defined when includeAllKeys is true"), + }.ToAggregate().Error()), + }, "sources defines the same configMap target": { bundle: &trustapi.Bundle{ ObjectMeta: metav1.ObjectMeta{Name: "test-bundle"}, @@ -390,6 +406,24 @@ func Test_validate(t *testing.T) { }, expErr: nil, }, + "valid Bundle including all keys": { + bundle: &trustapi.Bundle{ + ObjectMeta: metav1.ObjectMeta{Name: "test-bundle-1"}, + Spec: trustapi.BundleSpec{ + Sources: []trustapi.BundleSource{ + {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "some-config-map", IncludeAllKeys: true}}, + {Secret: &trustapi.SourceObjectKeySelector{Name: "some-secret", IncludeAllKeys: true}}, + }, + Target: trustapi.BundleTarget{ + ConfigMap: &trustapi.KeySelector{Key: "test-1"}, + NamespaceSelector: &trustapi.NamespaceSelector{ + MatchLabels: map[string]string{"foo": "bar"}, + }, + }, + }, + }, + expErr: nil, + }, } for name, test := range tests { From 2cf654ad84cb13689b89e4f7ebb3c1bcad10aa8e Mon Sep 17 00:00:00 2001 From: Julio Camarero Date: Tue, 22 Oct 2024 15:33:43 +0200 Subject: [PATCH 4/6] implement including all keys Signed-off-by: Julio Camarero --- pkg/bundle/source.go | 34 ++++++++++++------ pkg/bundle/source_test.go | 74 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 95 insertions(+), 13 deletions(-) diff --git a/pkg/bundle/source.go b/pkg/bundle/source.go index d3bcea6d..df678851 100644 --- a/pkg/bundle/source.go +++ b/pkg/bundle/source.go @@ -145,12 +145,19 @@ func (b *bundle) configMapBundle(ctx context.Context, ref *trustapi.SourceObject var results strings.Builder for _, cm := range configMaps { - data, ok := cm.Data[ref.Key] - if !ok { - return "", notFoundError{fmt.Errorf("no data found in ConfigMap %s/%s at key %q", cm.Namespace, cm.Name, ref.Key)} + if len(ref.Key) > 0 { + data, ok := cm.Data[ref.Key] + if !ok { + return "", notFoundError{fmt.Errorf("no data found in ConfigMap %s/%s at key %q", cm.Namespace, cm.Name, ref.Key)} + } + results.WriteString(data) + results.WriteByte('\n') + } else if ref.IncludeAllKeys { + for _, data := range cm.Data { + results.WriteString(data) + results.WriteByte('\n') + } } - results.WriteString(data) - results.WriteByte('\n') } return results.String(), nil } @@ -192,12 +199,19 @@ func (b *bundle) secretBundle(ctx context.Context, ref *trustapi.SourceObjectKey var results strings.Builder for _, secret := range secrets { - data, ok := secret.Data[ref.Key] - if !ok { - return "", notFoundError{fmt.Errorf("no data found in Secret %s/%s at key %q", secret.Namespace, secret.Name, ref.Key)} + if len(ref.Key) > 0 { + data, ok := secret.Data[ref.Key] + if !ok { + return "", notFoundError{fmt.Errorf("no data found in Secret %s/%s at key %q", secret.Namespace, secret.Name, ref.Key)} + } + results.Write(data) + results.WriteByte('\n') + } else if ref.IncludeAllKeys { + for _, data := range secret.Data { + results.Write(data) + results.WriteByte('\n') + } } - results.Write(data) - results.WriteByte('\n') } return results.String(), nil } diff --git a/pkg/bundle/source_test.go b/pkg/bundle/source_test.go index dd5031b8..651a6c8d 100644 --- a/pkg/bundle/source_test.go +++ b/pkg/bundle/source_test.go @@ -97,7 +97,7 @@ func Test_buildSourceBundle(t *testing.T) { expError: true, expNotFoundError: true, }, - "if single ConfigMap source, return data": { + "if single ConfigMap source referencing single key, return data": { sources: []trustapi.BundleSource{ {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}}, }, @@ -109,6 +109,18 @@ func Test_buildSourceBundle(t *testing.T) { expError: false, expNotFoundError: false, }, + "if single ConfigMap source including all keys, return data": { + sources: []trustapi.BundleSource{ + {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", IncludeAllKeys: true}}, + }, + objects: []runtime.Object{&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "configmap"}, + Data: map[string]string{"cert-1": dummy.TestCertificate1 + "\n" + dummy.TestCertificate2, "cert-2": dummy.TestCertificate3}, + }}, + expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3), + expError: false, + expNotFoundError: false, + }, "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{ @@ -144,6 +156,28 @@ func Test_buildSourceBundle(t *testing.T) { expError: false, expNotFoundError: false, }, + "if selects at least one ConfigMap source including all keys, return data": { + sources: []trustapi.BundleSource{ + {ConfigMap: &trustapi.SourceObjectKeySelector{IncludeAllKeys: true, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"trust-bundle.certs": "includes"}}}}, + }, + objects: []runtime.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "configmap", Labels: map[string]string{"trust-bundle.certs": "includes"}}, + Data: map[string]string{ + "cert-1": dummy.TestCertificate1 + "\n" + dummy.TestCertificate2, + "cert-3": dummy.TestCertificate3, + }, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "configmap2", Labels: map[string]string{"trust-bundle.certs": "includes"}}, + Data: map[string]string{ + "cert-4": dummy.TestCertificate4, + }, + }}, + expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3), + expError: false, + expNotFoundError: false, + }, "if ConfigMap and InLine source, return concatenated data": { sources: []trustapi.BundleSource{ {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}}, @@ -175,7 +209,7 @@ func Test_buildSourceBundle(t *testing.T) { expError: true, expNotFoundError: true, }, - "if single Secret source, return data": { + "if single Secret source referencing single key, return data": { sources: []trustapi.BundleSource{ {Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}}, }, @@ -187,6 +221,18 @@ func Test_buildSourceBundle(t *testing.T) { expError: false, expNotFoundError: false, }, + "if single Secret source including all keys, return data": { + sources: []trustapi.BundleSource{ + {Secret: &trustapi.SourceObjectKeySelector{Name: "secret", IncludeAllKeys: true}}, + }, + objects: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "secret"}, + Data: map[string][]byte{"cert-1": []byte(dummy.TestCertificate1 + "\n" + dummy.TestCertificate2), "cert-9": []byte(dummy.TestCertificate4)}, + }}, + expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4), + expError: false, + expNotFoundError: false, + }, "if Secret and InLine source, return concatenated data": { sources: []trustapi.BundleSource{ {Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}}, @@ -200,7 +246,7 @@ func Test_buildSourceBundle(t *testing.T) { expError: false, expNotFoundError: false, }, - "if Secret, ConfigmMap and InLine source, return concatenated data": { + "if Secret, ConfigMap and InLine source, return concatenated data": { sources: []trustapi.BundleSource{ {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}}, {InLine: ptr.To(dummy.TestCertificate3)}, @@ -250,6 +296,28 @@ func Test_buildSourceBundle(t *testing.T) { expError: true, expNotFoundError: true, }, + "if selects at least one Secret source including all keys, return data": { + sources: []trustapi.BundleSource{ + {Secret: &trustapi.SourceObjectKeySelector{IncludeAllKeys: true, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"trust-bundle.certs": "includes"}}}}, + }, + objects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "secret1", Labels: map[string]string{"trust-bundle.certs": "includes"}}, + Data: map[string][]byte{ + "cert-1": []byte(dummy.TestCertificate1 + "\n" + dummy.TestCertificate2), + "cert-3": []byte(dummy.TestCertificate3), + }, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "secret2", Labels: map[string]string{"trust-bundle.certs": "includes"}}, + Data: map[string][]byte{ + "cert-4": []byte(dummy.TestCertificate4), + }, + }}, + expData: dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3), + expError: false, + expNotFoundError: false, + }, "if has JKS target, return binaryData with encoded JKS": { sources: []trustapi.BundleSource{ {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}}, From 9769df6282c44fdc7def8529f8c1d9ace80f942d Mon Sep 17 00:00:00 2001 From: Julio Camarero Date: Tue, 22 Oct 2024 15:58:18 +0200 Subject: [PATCH 5/6] return an error when trying to include all keys of TLS secrets Signed-off-by: Julio Camarero --- pkg/bundle/source.go | 7 +++++ pkg/bundle/source_test.go | 59 +++++++++++++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/pkg/bundle/source.go b/pkg/bundle/source.go index df678851..7c86af68 100644 --- a/pkg/bundle/source.go +++ b/pkg/bundle/source.go @@ -36,6 +36,8 @@ type notFoundError struct{ error } type selectsNothingError struct{ error } +type invalidSecretSourceError struct{ error } + // bundleData holds the result of a call to buildSourceBundle. It contains the resulting PEM-encoded // certificate data from concatenating all the sources together, binary data for any additional formats and // any metadata from the sources which needs to be exposed on the Bundle resource's status field. @@ -207,6 +209,11 @@ func (b *bundle) secretBundle(ctx context.Context, ref *trustapi.SourceObjectKey results.Write(data) results.WriteByte('\n') } else if ref.IncludeAllKeys { + // This is done to prevent mistakes. All keys should never be included for a TLS secret, since that would include the private key. + if secret.Type == corev1.SecretTypeTLS { + return "", invalidSecretSourceError{fmt.Errorf("includeAllKeys is not supported for TLS Secrets such as %s/%s", secret.Namespace, secret.Name)} + } + for _, data := range secret.Data { results.Write(data) results.WriteByte('\n') diff --git a/pkg/bundle/source_test.go b/pkg/bundle/source_test.go index 651a6c8d..930d1a71 100644 --- a/pkg/bundle/source_test.go +++ b/pkg/bundle/source_test.go @@ -47,15 +47,17 @@ const ( func Test_buildSourceBundle(t *testing.T) { tests := map[string]struct { - sources []trustapi.BundleSource - formats *trustapi.AdditionalFormats - objects []runtime.Object - expData string - expError bool - expNotFoundError bool - expJKS bool - expPKCS12 bool - expPassword *string + sources []trustapi.BundleSource + formats *trustapi.AdditionalFormats + objects []runtime.Object + expData string + expError bool + expNotFoundError bool + expInvalidSecretSourceError bool + bool + expJKS bool + expPKCS12 bool + expPassword *string }{ "if no sources defined, should return an error": { objects: []runtime.Object{}, @@ -209,6 +211,19 @@ func Test_buildSourceBundle(t *testing.T) { expError: true, expNotFoundError: true, }, + "if single Secret source of type TLS including all keys, return invalidSecretSourceError": { + sources: []trustapi.BundleSource{ + {Secret: &trustapi.SourceObjectKeySelector{Name: "secret", IncludeAllKeys: true}}, + }, + objects: []runtime.Object{&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "secret"}, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{"cert-1": []byte(dummy.TestCertificate1), "cert-2": []byte(dummy.TestCertificate2)}, + }}, + expData: "", + expError: true, + expInvalidSecretSourceError: true, + }, "if single Secret source referencing single key, return data": { sources: []trustapi.BundleSource{ {Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}}, @@ -318,6 +333,29 @@ func Test_buildSourceBundle(t *testing.T) { expError: false, expNotFoundError: false, }, + "if selects at least one Secret source of type TLS including all keys, return invalidSecretSourceError": { + sources: []trustapi.BundleSource{ + {Secret: &trustapi.SourceObjectKeySelector{IncludeAllKeys: true, Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"trust-bundle.certs": "includes"}}}}, + }, + objects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "secret1", Labels: map[string]string{"trust-bundle.certs": "includes"}}, + Data: map[string][]byte{ + "cert-1": []byte(dummy.TestCertificate1 + "\n" + dummy.TestCertificate2), + "cert-3": []byte(dummy.TestCertificate3), + }, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "secret2", Labels: map[string]string{"trust-bundle.certs": "includes"}}, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + "cert-4": []byte(dummy.TestCertificate4), + }, + }}, + expData: "", + expError: true, + expInvalidSecretSourceError: true, + }, "if has JKS target, return binaryData with encoded JKS": { sources: []trustapi.BundleSource{ {ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}}, @@ -441,6 +479,9 @@ func Test_buildSourceBundle(t *testing.T) { if errors.As(err, ¬FoundError{}) != test.expNotFoundError { t.Errorf("unexpected notFoundError, exp=%t got=%v", test.expNotFoundError, err) } + if errors.As(err, &invalidSecretSourceError{}) != test.expInvalidSecretSourceError { + t.Errorf("unexpected invalidSecretSourceError, exp=%t got=%v", test.expInvalidSecretSourceError, err) + } if resolvedBundle.Data.Data != test.expData { t.Errorf("unexpected data, exp=%q got=%q", test.expData, resolvedBundle.Data.Data) From 223b7af65b6032fcbff006002e5dbd3fe5b83208 Mon Sep 17 00:00:00 2001 From: Julio Camarero Date: Wed, 23 Oct 2024 14:15:58 +0200 Subject: [PATCH 6/6] integration tests Signed-off-by: Julio Camarero --- test/integration/bundle/suite.go | 216 +++++++++++++++++++++++++++++++ 1 file changed, 216 insertions(+) diff --git a/test/integration/bundle/suite.go b/test/integration/bundle/suite.go index 7fbfa8b4..a8e26987 100644 --- a/test/integration/bundle/suite.go +++ b/test/integration/bundle/suite.go @@ -183,6 +183,29 @@ var _ = Describe("Integration", func() { testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) }) + It("should update all targets when a ConfigMap source including all keys is added", func() { + Expect(cl.Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-bundle-source", + Namespace: opts.Namespace, + }, + Data: map[string]string{ + "new-source-key-1": dummy.TestCertificate4, + "new-source-key-2": dummy.TestCertificate5, + }, + })).NotTo(HaveOccurred()) + + Expect(komega.Update(testBundle, func() { + testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{ + ConfigMap: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", IncludeAllKeys: true}, + }) + })()).To(Succeed()) + + expectedData := dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3, dummy.TestCertificate5) + + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + }) + It("should update all targets when a Secret source is added", func() { Expect(cl.Create(ctx, &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -205,6 +228,29 @@ var _ = Describe("Integration", func() { testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) }) + It("should update all targets when a Secret source including all keys is added", func() { + Expect(cl.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-bundle-source", + Namespace: opts.Namespace, + }, + Data: map[string][]byte{ + "new-source-key-1": []byte(dummy.TestCertificate4), + "new-source-key-2": []byte(dummy.TestCertificate5), + }, + })).NotTo(HaveOccurred()) + + Expect(komega.Update(testBundle, func() { + testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{ + Secret: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", IncludeAllKeys: true}, + }) + })()).To(Succeed()) + + expectedData := dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3, dummy.TestCertificate5) + + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + }) + It("should update all targets when an inLine source is added", func() { newInLine := dummy.TestCertificate4 @@ -276,6 +322,91 @@ var _ = Describe("Integration", func() { testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) }) + It("should update all targets when a ConfigMap source including all keys has a new key", func() { + secret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-bundle-source", + Namespace: opts.Namespace, + }, + Data: map[string][]byte{ + "new-source-key-1": []byte(dummy.TestCertificate4), + }, + } + + Expect(cl.Create(ctx, &secret)).NotTo(HaveOccurred()) + Expect(komega.Update(testBundle, func() { + testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{ + Secret: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", IncludeAllKeys: true}, + }) + })()).To(Succeed()) + + expectedData := dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + + secret.Data["new-source-key-2"] = []byte(dummy.TestCertificate5) + Expect(cl.Update(ctx, &secret)).NotTo(HaveOccurred()) + + expectedData = dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3, dummy.TestCertificate5) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + }) + + It("should update all targets when a ConfigMap source including all keys has a key removed", func() { + secret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-bundle-source", + Namespace: opts.Namespace, + }, + Data: map[string][]byte{ + "new-source-key-1": []byte(dummy.TestCertificate4), + "new-source-key-2": []byte(dummy.TestCertificate5), + }, + } + + Expect(cl.Create(ctx, &secret)).NotTo(HaveOccurred()) + Expect(komega.Update(testBundle, func() { + testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{ + Secret: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", IncludeAllKeys: true}, + }) + })()).To(Succeed()) + + expectedData := dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3, dummy.TestCertificate5) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + + delete(secret.Data, "new-source-key-2") + Expect(cl.Update(ctx, &secret)).NotTo(HaveOccurred()) + + expectedData = dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + }) + + It("should update all targets when a ConfigMap source including all keys has a key updated", func() { + secret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-bundle-source", + Namespace: opts.Namespace, + }, + Data: map[string][]byte{ + "new-source-key-1": []byte(dummy.TestCertificate4), + }, + } + + Expect(cl.Create(ctx, &secret)).NotTo(HaveOccurred()) + Expect(komega.Update(testBundle, func() { + testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{ + Secret: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", IncludeAllKeys: true}, + }) + })()).To(Succeed()) + + expectedData := dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + + secret.Data["new-source-key-1"] = []byte(dummy.TestCertificate5) + Expect(cl.Update(ctx, &secret)).NotTo(HaveOccurred()) + + expectedData = dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3, dummy.TestCertificate5) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + }) + It("should update all targets when a Secret source has been modified", func() { var secret corev1.Secret @@ -290,6 +421,91 @@ var _ = Describe("Integration", func() { testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) }) + It("should update all targets when a Secret source including all keys has a new key", func() { + configMap := corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-bundle-source", + Namespace: opts.Namespace, + }, + Data: map[string]string{ + "new-source-key-1": dummy.TestCertificate4, + }, + } + + Expect(cl.Create(ctx, &configMap)).NotTo(HaveOccurred()) + Expect(komega.Update(testBundle, func() { + testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{ + ConfigMap: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", IncludeAllKeys: true}, + }) + })()).To(Succeed()) + + expectedData := dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + + configMap.Data["new-source-key-2"] = dummy.TestCertificate5 + Expect(cl.Update(ctx, &configMap)).NotTo(HaveOccurred()) + + expectedData = dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3, dummy.TestCertificate5) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + }) + + It("should update all targets when a Secret source including all keys has a key removed", func() { + configMap := corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-bundle-source", + Namespace: opts.Namespace, + }, + Data: map[string]string{ + "new-source-key-1": dummy.TestCertificate4, + "new-source-key-2": dummy.TestCertificate5, + }, + } + + Expect(cl.Create(ctx, &configMap)).NotTo(HaveOccurred()) + Expect(komega.Update(testBundle, func() { + testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{ + ConfigMap: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", IncludeAllKeys: true}, + }) + })()).To(Succeed()) + + expectedData := dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3, dummy.TestCertificate5) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + + delete(configMap.Data, "new-source-key-2") + Expect(cl.Update(ctx, &configMap)).NotTo(HaveOccurred()) + + expectedData = dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + }) + + It("should update all targets when a Secret source including all keys has a key updated", func() { + configMap := corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-bundle-source", + Namespace: opts.Namespace, + }, + Data: map[string]string{ + "new-source-key-1": dummy.TestCertificate4, + }, + } + + Expect(cl.Create(ctx, &configMap)).NotTo(HaveOccurred()) + Expect(komega.Update(testBundle, func() { + testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{ + ConfigMap: &trustapi.SourceObjectKeySelector{Name: "new-bundle-source", IncludeAllKeys: true}, + }) + })()).To(Succeed()) + + expectedData := dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate4, dummy.TestCertificate3) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + + configMap.Data["new-source-key-1"] = dummy.TestCertificate5 + Expect(cl.Update(ctx, &configMap)).NotTo(HaveOccurred()) + + expectedData = dummy.JoinCerts(dummy.TestCertificate2, dummy.TestCertificate1, dummy.TestCertificate3, dummy.TestCertificate5) + testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, expectedData) + }) + It("should update all targets when an InLine source has been modified", func() { newInLine := dummy.TestCertificate4