refactor: move source into target package

Signed-off-by: Erik Godding Boye <[email protected]>
cert-manager · Sep 21, 2024 · 9e0af14 · 9e0af14
1 parent b1ee0f0
commit 9e0af14
Show file tree

Hide file tree

Showing 9 changed files with 107 additions and 111 deletions.
diff --git a/cmd/trust-manager/app/options/options.go b/cmd/trust-manager/app/options/options.go
@@ -31,8 +31,6 @@ import (
 	cliflag "k8s.io/component-base/cli/flag"
 	"k8s.io/klog/v2"
 
-	"github.com/cert-manager/trust-manager/pkg/bundle"
-
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 )
 
@@ -60,7 +58,7 @@ type Options struct {
 	Webhook
 
 	// Bundle are options specific to the Bundle controller.
-	Bundle bundle.Options
+	Bundle BundleOptions
 
 	// log are options controlling logging
 	log logOptions
@@ -248,3 +246,24 @@ func (o *Options) addWebhookFlags(fs *pflag.FlagSet) {
 			"Certificate and private key must be named 'tls.crt' and 'tls.key' "+
 			"respectively.")
 }
+
+// BundleOptions hold options for the Bundle controller.
+type BundleOptions struct {
+	// Log is the Bundle controller logger.
+	Log logr.Logger
+
+	// Namespace is the trust Namespace that source data can be referenced.
+	Namespace string
+
+	// DefaultPackageLocation is the location on the filesystem from which the 'default'
+	// certificate package should be loaded. If set, a valid package must be successfully
+	// loaded in order for the controller to start. If unset, referring to the default
+	// certificate package in a `Bundle` resource will cause that Bundle to error.
+	DefaultPackageLocation string
+
+	// SecretTargetsEnabled controls if secret targets are enabled in the Bundle API.
+	SecretTargetsEnabled bool
+
+	// FilterExpiredCerts controls if expired certificates are filtered from the bundle.
+	FilterExpiredCerts bool
+}
diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go
@@ -35,32 +35,12 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/cert-manager/trust-manager/cmd/trust-manager/app/options"
 	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 	"github.com/cert-manager/trust-manager/pkg/bundle/internal/ssa_client"
 	"github.com/cert-manager/trust-manager/pkg/bundle/internal/target"
 )
 
-// Options hold options for the Bundle controller.
-type Options struct {
-	// Log is the Bundle controller logger.
-	Log logr.Logger
-
-	// Namespace is the trust Namespace that source data can be referenced.
-	Namespace string
-
-	// DefaultPackageLocation is the location on the filesystem from which the 'default'
-	// certificate package should be loaded. If set, a valid package must be successfully
-	// loaded in order for the controller to start. If unset, referring to the default
-	// certificate package in a `Bundle` resource will cause that Bundle to error.
-	DefaultPackageLocation string
-
-	// SecretTargetsEnabled controls if secret targets are enabled in the Bundle API.
-	SecretTargetsEnabled bool
-
-	// FilterExpiredCerts controls if expired certificates are filtered from the bundle.
-	FilterExpiredCerts bool
-}
-
 // bundle is a controller-runtime controller. Implements the actual controller
 // logic by reconciling over Bundles.
 type bundle struct {
@@ -73,10 +53,10 @@ type bundle struct {
 	// clock returns time which can be overwritten for testing.
 	clock clock.Clock
 
-	// Options holds options for the Bundle controller.
-	Options
+	// BundleOptions holds options for the Bundle controller.
+	options.BundleOptions
 
-	sourceDataBuilder *bundleDataBuilder
+	sourceDataBuilder *target.BundleDataBuilder
 
 	targetReconciler *target.Reconciler
 }
@@ -132,10 +112,10 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
 	statusPatch = &trustapi.BundleStatus{
 		DefaultCAPackageVersion: bundle.Status.DefaultCAPackageVersion,
 	}
-	resolvedBundle, err := b.sourceDataBuilder.buildSourceBundle(ctx, bundle.Spec.Sources, bundle.Spec.Target.AdditionalFormats)
+	resolvedBundle, err := b.sourceDataBuilder.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{}) {
+	if errors.As(err, &target.SourceNotFoundError{}) {
 		log.Error(err, "bundle source was not found")
 		b.setBundleCondition(
 			bundle.Status.Conditions,
@@ -161,7 +141,7 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
 	}
 
 	// Detect if we have a bundle with Secret targets but the feature is disabled.
-	if !b.Options.SecretTargetsEnabled && bundle.Spec.Target.Secret != nil {
+	if !b.SecretTargetsEnabled && bundle.Spec.Target.Secret != nil {
 
 		log.Error(err, "bundle has Secret targets but the feature is disabled")
 		b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SecretTargetsDisabled", "Bundle has Secret targets but the feature is disabled")
@@ -235,7 +215,7 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
 
 	// Find all old existing target resources.
 	targetKinds := []targetKind{configMapTarget}
-	if b.Options.SecretTargetsEnabled {
+	if b.SecretTargetsEnabled {
 		targetKinds = append(targetKinds, secretTarget)
 	}
 	for _, kind := range targetKinds {
@@ -330,7 +310,7 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
 		}
 	}
 
-	if b.setBundleStatusDefaultCAVersion(statusPatch, resolvedBundle.defaultCAPackageStringID) {
+	if b.setBundleStatusDefaultCAVersion(statusPatch, resolvedBundle.DefaultCAPackageStringID) {
 		needsUpdate = true
 	}
 

diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go
@@ -39,6 +39,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+	"github.com/cert-manager/trust-manager/cmd/trust-manager/app/options"
 	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 	"github.com/cert-manager/trust-manager/pkg/bundle/internal/ssa_client"
 	"github.com/cert-manager/trust-manager/pkg/bundle/internal/target"
@@ -1312,20 +1313,20 @@ func Test_Reconcile(t *testing.T) {
 			)
 
 			log, ctx := ktesting.NewTestContext(t)
-			opts := Options{
+			opts := options.BundleOptions{
 				Log:                  log,
 				Namespace:            trustNamespace,
 				SecretTargetsEnabled: !test.disableSecretTargets,
 				FilterExpiredCerts:   true,
 			}
 			b := &bundle{
-				client:   fakeClient,
-				recorder: fakeRecorder,
-				clock:    fixedclock,
-				Options:  opts,
-				sourceDataBuilder: &bundleDataBuilder{
-					client:  fakeClient,
-					Options: opts,
+				client:        fakeClient,
+				recorder:      fakeRecorder,
+				clock:         fixedclock,
+				BundleOptions: opts,
+				sourceDataBuilder: &target.BundleDataBuilder{
+					Client:        fakeClient,
+					BundleOptions: opts,
 				},
 				targetReconciler: &target.Reconciler{
 					Client: fakeClient,
@@ -1341,7 +1342,7 @@ func Test_Reconcile(t *testing.T) {
 			}
 
 			if test.configureDefaultPackage {
-				b.sourceDataBuilder.defaultPackage = testDefaultPackage.Clone()
+				b.sourceDataBuilder.DefaultPackage = testDefaultPackage.Clone()
 			}
 			resp, result, err := b.reconcileBundle(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: bundleName}})
 			if (err != nil) != test.expError {

diff --git a/pkg/bundle/controller.go b/pkg/bundle/controller.go
@@ -36,6 +36,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
+	"github.com/cert-manager/trust-manager/cmd/trust-manager/app/options"
 	trustapi "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"
 	"github.com/cert-manager/trust-manager/pkg/bundle/internal/target"
 	"github.com/cert-manager/trust-manager/pkg/fspkg"
@@ -49,33 +50,33 @@ import (
 func AddBundleController(
 	ctx context.Context,
 	mgr manager.Manager,
-	opts Options,
+	opts options.BundleOptions,
 	targetCache cache.Cache,
 ) error {
 	b := &bundle{
-		client:   mgr.GetClient(),
-		recorder: mgr.GetEventRecorderFor("bundles"),
-		clock:    clock.RealClock{},
-		Options:  opts,
-		sourceDataBuilder: &bundleDataBuilder{
-			client:  mgr.GetClient(),
-			Options: opts,
+		client:        mgr.GetClient(),
+		recorder:      mgr.GetEventRecorderFor("bundles"),
+		clock:         clock.RealClock{},
+		BundleOptions: opts,
+		sourceDataBuilder: &target.BundleDataBuilder{
+			Client:        mgr.GetClient(),
+			BundleOptions: opts,
 		},
 		targetReconciler: &target.Reconciler{
 			Client: mgr.GetClient(),
 			Cache:  targetCache,
 		},
 	}
 
-	if b.Options.DefaultPackageLocation != "" {
-		pkg, err := fspkg.LoadPackageFromFile(b.Options.DefaultPackageLocation)
+	if b.DefaultPackageLocation != "" {
+		pkg, err := fspkg.LoadPackageFromFile(b.DefaultPackageLocation)
 		if err != nil {
 			return fmt.Errorf("must load default package successfully when default package location is set: %w", err)
 		}
 
-		b.sourceDataBuilder.defaultPackage = &pkg
+		b.sourceDataBuilder.DefaultPackage = &pkg
 
-		b.Options.Log.Info("successfully loaded default package from filesystem", "path", b.Options.DefaultPackageLocation)
+		b.Log.Info("successfully loaded default package from filesystem", "path", b.BundleOptions.DefaultPackageLocation)
 	}
 
 	// Only reconcile config maps that match the well known name
@@ -144,7 +145,7 @@ func AddBundleController(
 					}
 				}
 				return false
-			}), builder.WithPredicates(inNamespacePredicate(b.Options.Namespace))).
+			}), builder.WithPredicates(inNamespacePredicate(b.Namespace))).
 
 		// Watch Secrets in trust Namespace.
 		// Reconcile Bundles who reference a modified source Secret.
@@ -156,7 +157,7 @@ func AddBundleController(
 					}
 				}
 				return false
-			}), builder.WithPredicates(inNamespacePredicate(b.Options.Namespace)))
+			}), builder.WithPredicates(inNamespacePredicate(b.Namespace)))
 
 	// Complete controller.
 	if err := controller.Complete(b); err != nil {