Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: dedicated struct for building source data #442

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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