Skip to content

Commit

Permalink
refactor: dedicated struct for building source data
Browse files Browse the repository at this point in the history
Signed-off-by: Erik Godding Boye <[email protected]>
  • Loading branch information
erikgb committed Dec 8, 2024
1 parent c8d2a25 commit f485127
Show file tree
Hide file tree
Showing 11 changed files with 149 additions and 123 deletions.
25 changes: 22 additions & 3 deletions cmd/trust-manager/app/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -60,7 +58,7 @@ type Options struct {
Webhook

// Bundle are options specific to the Bundle controller.
Bundle bundle.Options
Bundle Bundle

// log are options controlling logging
log logOptions
Expand Down Expand Up @@ -248,3 +246,24 @@ func (o *Options) addWebhookFlags(fs *pflag.FlagSet) {
"Certificate and private key must be named 'tls.crt' and 'tls.key' "+
"respectively.")
}

// Bundle hold options for the Bundle controller.
type Bundle 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
}
40 changes: 8 additions & 32 deletions pkg/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"
"strings"

"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -35,51 +34,28 @@ 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"
"github.com/cert-manager/trust-manager/pkg/fspkg"
)

// 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 {
// a cache-backed Kubernetes client
client client.Client

// defaultPackage holds the loaded 'default' certificate package, if one was specified
// at startup.
defaultPackage *fspkg.Package

// recorder is used for create Kubernetes Events for reconciled Bundles.
recorder record.EventRecorder

// clock returns time which can be overwritten for testing.
clock clock.Clock

// Options holds options for the Bundle controller.
Options
Options options.Bundle

sources *target.BundleBuilder

targetReconciler *target.Reconciler
}
Expand All @@ -106,7 +82,7 @@ func (b *bundle) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
}

func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result ctrl.Result, statusPatch *trustapi.BundleStatus, returnedErr error) {
log := b.Log.WithValues("bundle", req.NamespacedName.Name)
log := b.Options.Log.WithValues("bundle", req.NamespacedName.Name)
log.V(2).Info("syncing bundle")

var bundle trustapi.Bundle
Expand Down Expand Up @@ -135,10 +111,10 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
statusPatch = &trustapi.BundleStatus{
DefaultCAPackageVersion: bundle.Status.DefaultCAPackageVersion,
}
resolvedBundle, err := b.buildSourceBundle(ctx, bundle.Spec.Sources, bundle.Spec.Target.AdditionalFormats)
resolvedBundle, err := b.sources.BuildBundle(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,
Expand Down Expand Up @@ -309,7 +285,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
}

Expand Down
18 changes: 12 additions & 6 deletions pkg/bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,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"
Expand Down Expand Up @@ -1443,15 +1444,20 @@ func Test_Reconcile(t *testing.T) {
)

log, ctx := ktesting.NewTestContext(t)
opts := options.Bundle{
Log: log,
Namespace: trustNamespace,
SecretTargetsEnabled: !test.disableSecretTargets,
FilterExpiredCerts: true,
}
b := &bundle{
client: fakeClient,
recorder: fakeRecorder,
clock: fixedclock,
Options: Options{
Log: log,
Namespace: trustNamespace,
SecretTargetsEnabled: !test.disableSecretTargets,
FilterExpiredCerts: true,
Options: opts,
sources: &target.BundleBuilder{
Client: fakeClient,
Options: opts,
},
targetReconciler: &target.Reconciler{
Client: fakeClient,
Expand All @@ -1467,7 +1473,7 @@ func Test_Reconcile(t *testing.T) {
}

if test.configureDefaultPackage {
b.defaultPackage = testDefaultPackage.Clone()
b.sources.DefaultPackage = testDefaultPackage.Clone()
}
resp, result, err := b.reconcileBundle(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: bundleName}})
if (err != nil) != test.expError {
Expand Down
26 changes: 12 additions & 14 deletions pkg/bundle/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ 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"
)

// AddBundleController will register the Bundle controller with the
Expand All @@ -49,31 +49,29 @@ import (
func AddBundleController(
ctx context.Context,
mgr manager.Manager,
opts Options,
opts options.Bundle,
targetCache cache.Cache,
) error {
sourceBuilder := &target.BundleBuilder{
Client: mgr.GetClient(),
Options: opts,
}
if err := sourceBuilder.Init(); err != nil {
return err
}

b := &bundle{
client: mgr.GetClient(),
recorder: mgr.GetEventRecorderFor("bundles"),
clock: clock.RealClock{},
Options: opts,
sources: sourceBuilder,
targetReconciler: &target.Reconciler{
Client: mgr.GetClient(),
Cache: targetCache,
},
}

if b.Options.DefaultPackageLocation != "" {
pkg, err := fspkg.LoadPackageFromFile(b.Options.DefaultPackageLocation)
if err != nil {
return fmt.Errorf("must load default package successfully when default package location is set: %w", err)
}

b.defaultPackage = &pkg

b.Options.Log.Info("successfully loaded default package from filesystem", "path", b.Options.DefaultPackageLocation)
}

// Only reconcile config maps that match the well known name
controller := ctrl.NewControllerManagedBy(mgr).
Named("bundles").
Expand Down Expand Up @@ -191,7 +189,7 @@ func (b *bundle) enqueueRequestsFromBundleFunc(fn func(obj client.Object, bundle
func (b *bundle) mustBundleList(ctx context.Context) *trustapi.BundleList {
var bundleList trustapi.BundleList
if err := b.client.List(ctx, &bundleList); err != nil {
b.Log.Error(err, "failed to list all Bundles, exiting error")
b.Options.Log.Error(err, "failed to list all Bundles, exiting error")
os.Exit(-1)
}

Expand Down
Loading

0 comments on commit f485127

Please sign in to comment.