Merge pull request #377 from erikgb/bundle-data-build

refactor: build bundle data from relevant spec
cert-manager · Jul 7, 2024 · ee6582a · ee6582a
2 parents 92cb2fe + a504039
commit ee6582a
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 94 deletions.
diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go
@@ -147,7 +147,7 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
 	statusPatch = &trustapi.BundleStatus{
 		DefaultCAPackageVersion: bundle.Status.DefaultCAPackageVersion,
 	}
-	resolvedBundle, err := b.buildSourceBundle(ctx, &bundle)
+	resolvedBundle, err := b.buildSourceBundle(ctx, bundle.Spec.Sources, bundle.Spec.Target.AdditionalFormats)
 
 	// If any source is not found, update the Bundle status to an unready state.
 	if errors.As(err, &notFoundError{}) {

diff --git a/pkg/bundle/source.go b/pkg/bundle/source.go
@@ -62,11 +62,11 @@ type bundleData struct {
 // buildSourceBundle retrieves and concatenates all source bundle data for this Bundle object.
 // Each source data is validated and pruned to ensure that all certificates within are valid, and
 // is each bundle is concatenated together with a new line character.
-func (b *bundle) buildSourceBundle(ctx context.Context, bundle *trustapi.Bundle) (bundleData, error) {
+func (b *bundle) buildSourceBundle(ctx context.Context, sources []trustapi.BundleSource, formats *trustapi.AdditionalFormats) (bundleData, error) {
 	var resolvedBundle bundleData
 	var bundles []string
 
-	for _, source := range bundle.Spec.Sources {
+	for _, source := range sources {
 		var (
 			sourceData string
 			err        error
@@ -119,7 +119,7 @@ func (b *bundle) buildSourceBundle(ctx context.Context, bundle *trustapi.Bundle)
 		return bundleData{}, err
 	}
 
-	if err := resolvedBundle.populateData(deduplicatedBundles, bundle.Spec.Target); err != nil {
+	if err := resolvedBundle.populateData(deduplicatedBundles, formats); err != nil {
 		return bundleData{}, err
 	}
 
@@ -318,26 +318,26 @@ func (e pkcs12Encoder) encode(trustBundle string) ([]byte, error) {
 	return encoder.EncodeTrustStoreEntries(entries, e.password)
 }
 
-func (b *bundleData) populateData(bundles []string, target trustapi.BundleTarget) error {
+func (b *bundleData) populateData(bundles []string, formats *trustapi.AdditionalFormats) error {
 	b.data = strings.Join(bundles, "\n") + "\n"
 
-	if target.AdditionalFormats != nil {
+	if formats != nil {
 		b.binaryData = make(map[string][]byte)
 
-		if target.AdditionalFormats.JKS != nil {
-			encoded, err := jksEncoder{password: *target.AdditionalFormats.JKS.Password}.encode(b.data)
+		if formats.JKS != nil {
+			encoded, err := jksEncoder{password: *formats.JKS.Password}.encode(b.data)
 			if err != nil {
 				return fmt.Errorf("failed to encode JKS: %w", err)
 			}
-			b.binaryData[target.AdditionalFormats.JKS.Key] = encoded
+			b.binaryData[formats.JKS.Key] = encoded
 		}
 
-		if target.AdditionalFormats.PKCS12 != nil {
-			encoded, err := pkcs12Encoder{password: *target.AdditionalFormats.PKCS12.Password}.encode(b.data)
+		if formats.PKCS12 != nil {
+			encoded, err := pkcs12Encoder{password: *formats.PKCS12.Password}.encode(b.data)
 			if err != nil {
 				return fmt.Errorf("failed to encode PKCS12: %w", err)
 			}
-			b.binaryData[target.AdditionalFormats.PKCS12.Key] = encoded
+			b.binaryData[formats.PKCS12.Key] = encoded
 		}
 	}
 	return nil

diff --git a/pkg/bundle/source_test.go b/pkg/bundle/source_test.go
@@ -40,7 +40,8 @@ import (
 
 func Test_buildSourceBundle(t *testing.T) {
 	tests := map[string]struct {
-		bundle           *trustapi.Bundle
+		sources          []trustapi.BundleSource
+		formats          *trustapi.AdditionalFormats
 		objects          []runtime.Object
 		expData          string
 		expError         bool
@@ -50,50 +51,49 @@ func Test_buildSourceBundle(t *testing.T) {
 		expPassword      *string
 	}{
 		"if no sources defined, should return an error": {
-			bundle:           &trustapi.Bundle{},
 			objects:          []runtime.Object{},
 			expData:          "",
 			expError:         true,
 			expNotFoundError: false,
 		},
 		"if single InLine source defined with newlines, should trim and return": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{InLine: ptr.To(dummy.TestCertificate1 + "\n" + dummy.TestCertificate2 + "\n\n")},
-			}}},
+			},
 			objects:          []runtime.Object{},
 			expData:          dummy.JoinCerts(dummy.TestCertificate1, dummy.TestCertificate2),
 			expError:         false,
 			expNotFoundError: false,
 		},
 		"if single DefaultPackage source defined, should return": {
-			bundle:           &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{{UseDefaultCAs: ptr.To(true)}}}},
+			sources:          []trustapi.BundleSource{{UseDefaultCAs: ptr.To(true)}},
 			objects:          []runtime.Object{},
 			expData:          dummy.JoinCerts(dummy.TestCertificate5),
 			expError:         false,
 			expNotFoundError: false,
 		},
 		"if single ConfigMap source which doesn't exist, return notFoundError": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
-			}}},
+			},
 			objects:          []runtime.Object{},
 			expData:          "",
 			expError:         true,
 			expNotFoundError: true,
 		},
 		"if single ConfigMap source whose key doesn't exist, return notFoundError": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
-			}}},
+			},
 			objects:          []runtime.Object{&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "configmap"}}},
 			expData:          "",
 			expError:         true,
 			expNotFoundError: true,
 		},
 		"if single ConfigMap source, return data": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
-			}}},
+			},
 			objects: []runtime.Object{&corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
 				Data:       map[string]string{"key": dummy.TestCertificate1 + "\n" + dummy.TestCertificate2},
@@ -103,10 +103,10 @@ func Test_buildSourceBundle(t *testing.T) {
 			expNotFoundError: false,
 		},
 		"if ConfigMap and InLine source, return concatenated data": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
 				{InLine: ptr.To(dummy.TestCertificate2)},
-			}}},
+			},
 			objects: []runtime.Object{&corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
 				Data:       map[string]string{"key": dummy.TestCertificate1},
@@ -116,27 +116,27 @@ func Test_buildSourceBundle(t *testing.T) {
 			expNotFoundError: false,
 		},
 		"if single Secret source exists which doesn't exist, should return not found error": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
-			}}},
+			},
 			objects:          []runtime.Object{},
 			expData:          "",
 			expError:         true,
 			expNotFoundError: true,
 		},
 		"if single Secret source whose key doesn't exist, return notFoundError": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
-			}}},
+			},
 			objects:          []runtime.Object{&corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "secret"}}},
 			expData:          "",
 			expError:         true,
 			expNotFoundError: true,
 		},
 		"if single Secret source, return data": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
-			}}},
+			},
 			objects: []runtime.Object{&corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "secret"},
 				Data:       map[string][]byte{"key": []byte(dummy.TestCertificate1 + "\n" + dummy.TestCertificate2)},
@@ -146,10 +146,10 @@ func Test_buildSourceBundle(t *testing.T) {
 			expNotFoundError: false,
 		},
 		"if Secret and InLine source, return concatenated data": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
 				{InLine: ptr.To(dummy.TestCertificate1)},
-			}}},
+			},
 			objects: []runtime.Object{&corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{Name: "secret"},
 				Data:       map[string][]byte{"key": []byte(dummy.TestCertificate2)},
@@ -159,11 +159,11 @@ func Test_buildSourceBundle(t *testing.T) {
 			expNotFoundError: false,
 		},
 		"if Secret, ConfigmMap and InLine source, return concatenated data": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
 				{InLine: ptr.To(dummy.TestCertificate3)},
 				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
-			}}},
+			},
 			objects: []runtime.Object{
 				&corev1.ConfigMap{
 					ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
@@ -179,10 +179,10 @@ func Test_buildSourceBundle(t *testing.T) {
 			expNotFoundError: false,
 		},
 		"if source Secret exists, but not ConfigMap, return not found error": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
 				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
-			}}},
+			},
 			objects: []runtime.Object{
 				&corev1.ConfigMap{
 					ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
@@ -194,10 +194,10 @@ func Test_buildSourceBundle(t *testing.T) {
 			expNotFoundError: true,
 		},
 		"if source ConfigMap exists, but not Secret, return not found error": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{Sources: []trustapi.BundleSource{
+			sources: []trustapi.BundleSource{
 				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
 				{Secret: &trustapi.SourceObjectKeySelector{Name: "secret", KeySelector: trustapi.KeySelector{Key: "key"}}},
-			}}},
+			},
 			objects: []runtime.Object{
 				&corev1.Secret{
 					ObjectMeta: metav1.ObjectMeta{Name: "secret"},
@@ -209,20 +209,17 @@ func Test_buildSourceBundle(t *testing.T) {
 			expNotFoundError: true,
 		},
 		"if has JKS target, return binaryData with encoded JKS": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{
-				Sources: []trustapi.BundleSource{
-					{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+			sources: []trustapi.BundleSource{
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+			},
+			formats: &trustapi.AdditionalFormats{
+				JKS: &trustapi.JKS{
+					KeySelector: trustapi.KeySelector{
+						Key: jksKey,
+					},
+					Password: ptr.To(DefaultJKSPassword),
 				},
-				Target: trustapi.BundleTarget{
-					AdditionalFormats: &trustapi.AdditionalFormats{
-						JKS: &trustapi.JKS{
-							KeySelector: trustapi.KeySelector{
-								Key: jksKey,
-							},
-							Password: ptr.To(DefaultJKSPassword),
-						},
-					}},
-			}},
+			},
 			objects: []runtime.Object{&corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
 				Data:       map[string]string{"key": dummy.TestCertificate1},
@@ -231,20 +228,17 @@ func Test_buildSourceBundle(t *testing.T) {
 			expJKS:  true,
 		},
 		"if has JKS target with arbitrary password, return binaryData with encoded JKS": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{
-				Sources: []trustapi.BundleSource{
-					{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+			sources: []trustapi.BundleSource{
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+			},
+			formats: &trustapi.AdditionalFormats{
+				JKS: &trustapi.JKS{
+					KeySelector: trustapi.KeySelector{
+						Key: jksKey,
+					},
+					Password: ptr.To("testPasswd123"),
 				},
-				Target: trustapi.BundleTarget{
-					AdditionalFormats: &trustapi.AdditionalFormats{
-						JKS: &trustapi.JKS{
-							KeySelector: trustapi.KeySelector{
-								Key: jksKey,
-							},
-							Password: ptr.To("testPasswd123"),
-						},
-					}},
-			}},
+			},
 			objects: []runtime.Object{&corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
 				Data:       map[string]string{"key": dummy.TestCertificate1},
@@ -254,22 +248,17 @@ func Test_buildSourceBundle(t *testing.T) {
 			expPassword: ptr.To("testPasswd123"),
 		},
 		"if has PKCS12 target, return binaryData with encoded PKCS12": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{
-				Sources: []trustapi.BundleSource{
-					{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
-				},
-				Target: trustapi.BundleTarget{
-					AdditionalFormats: &trustapi.AdditionalFormats{
-						PKCS12: &trustapi.PKCS12{
-							KeySelector: trustapi.KeySelector{
-								Key: pkcs12Key,
-							},
-							Password: ptr.To(DefaultPKCS12Password),
-						},
+			sources: []trustapi.BundleSource{
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+			},
+			formats: &trustapi.AdditionalFormats{
+				PKCS12: &trustapi.PKCS12{
+					KeySelector: trustapi.KeySelector{
+						Key: pkcs12Key,
 					},
+					Password: ptr.To(DefaultPKCS12Password),
 				},
-			}},
-
+			},
 			objects: []runtime.Object{&corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
 				Data:       map[string]string{"key": dummy.TestCertificate1},
@@ -278,22 +267,17 @@ func Test_buildSourceBundle(t *testing.T) {
 			expPKCS12: true,
 		},
 		"if has PKCS12 target with arbitrary password, return binaryData with encoded PKCS12": {
-			bundle: &trustapi.Bundle{Spec: trustapi.BundleSpec{
-				Sources: []trustapi.BundleSource{
-					{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
-				},
-				Target: trustapi.BundleTarget{
-					AdditionalFormats: &trustapi.AdditionalFormats{
-						PKCS12: &trustapi.PKCS12{
-							KeySelector: trustapi.KeySelector{
-								Key: pkcs12Key,
-							},
-							Password: ptr.To("testPasswd123"),
-						},
+			sources: []trustapi.BundleSource{
+				{ConfigMap: &trustapi.SourceObjectKeySelector{Name: "configmap", KeySelector: trustapi.KeySelector{Key: "key"}}},
+			},
+			formats: &trustapi.AdditionalFormats{
+				PKCS12: &trustapi.PKCS12{
+					KeySelector: trustapi.KeySelector{
+						Key: pkcs12Key,
 					},
+					Password: ptr.To("testPasswd123"),
 				},
-			}},
-
+			},
 			objects: []runtime.Object{&corev1.ConfigMap{
 				ObjectMeta: metav1.ObjectMeta{Name: "configmap"},
 				Data:       map[string]string{"key": dummy.TestCertificate1},
@@ -340,7 +324,7 @@ func Test_buildSourceBundle(t *testing.T) {
 				}
 			}
 
-			resolvedBundle, err := b.buildSourceBundle(context.TODO(), test.bundle)
+			resolvedBundle, err := b.buildSourceBundle(context.TODO(), test.sources, test.formats)
 
 			if (err != nil) != test.expError {
 				t.Errorf("unexpected error, exp=%t got=%v", test.expError, err)