Skip to content

Commit

Permalink
refactor: establish target.Reconciler
Browse files Browse the repository at this point in the history
Signed-off-by: Erik Godding Boye <[email protected]>
  • Loading branch information
erikgb committed Jul 18, 2024
1 parent fb6516a commit 98b51c9
Show file tree
Hide file tree
Showing 13 changed files with 540 additions and 518 deletions.
11 changes: 10 additions & 1 deletion docs/api/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,19 @@ import "github.com/cert-manager/trust-manager/pkg/apis/trust/v1alpha1"

## Constants

<a name="BundleConditionSynced"></a>
<a name="DefaultJKSPassword"></a>

```go
const (
// DefaultJKSPassword is the default password that Java uses; it's a Java convention to use this exact password.
// Since we're not storing anything secret in the JKS files we generate, this password is not a meaningful security measure
// but seems often to be expected by applications consuming JKS files
DefaultJKSPassword = "changeit"
// DefaultPKCS12Password is the empty string, that will create a password-less PKCS12 truststore.
// Password-less PKCS is the new default Java truststore from Java 18.
// By password-less, it means the certificates are not encrypted, and it contains no MacData for integrity check.
DefaultPKCS12Password = ""

// BundleConditionSynced indicates that the Bundle has successfully synced
// all source bundle data to the Bundle target in all Namespaces.
BundleConditionSynced string = "Synced"
Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/trust/v1alpha1/types_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,15 @@ type BundleCondition struct {
}

const (
// DefaultJKSPassword is the default password that Java uses; it's a Java convention to use this exact password.
// Since we're not storing anything secret in the JKS files we generate, this password is not a meaningful security measure
// but seems often to be expected by applications consuming JKS files
DefaultJKSPassword = "changeit"
// DefaultPKCS12Password is the empty string, that will create a password-less PKCS12 truststore.
// Password-less PKCS is the new default Java truststore from Java 18.
// By password-less, it means the certificates are not encrypted, and it contains no MacData for integrity check.
DefaultPKCS12Password = ""

// BundleConditionSynced indicates that the Bundle has successfully synced
// all source bundle data to the Bundle target in all Namespaces.
BundleConditionSynced string = "Synced"
Expand Down
26 changes: 8 additions & 18 deletions pkg/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ import (
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/csaupgrade"
"k8s.io/utils/clock"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

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"
)

Expand Down Expand Up @@ -69,10 +69,6 @@ type bundle struct {
// a cache-backed Kubernetes client
client client.Client

// targetCache is a cache.Cache that holds cached ConfigMap and Secret
// resources that are used as targets for Bundles.
targetCache client.Reader

// defaultPackage holds the loaded 'default' certificate package, if one was specified
// at startup.
defaultPackage *fspkg.Package
Expand All @@ -86,9 +82,7 @@ type bundle struct {
// Options holds options for the Bundle controller.
Options

// patchResourceOverwrite allows use to override the patchResource function
// it is used for testing purposes
patchResourceOverwrite func(ctx context.Context, obj interface{}) error
targetReconciler *target.Reconciler
}

// Reconcile is the top level function for reconciling over synced Bundles.
Expand All @@ -103,12 +97,7 @@ func (b *bundle) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
return ctrl.Result{}, utilerrors.NewAggregate([]error{resultErr, err})
}

if err := b.client.Status().Patch(ctx, con, patch, &client.SubResourcePatchOptions{
PatchOptions: client.PatchOptions{
FieldManager: fieldManager,
Force: ptr.To(true),
},
}); err != nil {
if err := b.client.Status().Patch(ctx, con, patch, ssa_client.FieldManager, client.ForceOwnership); err != nil {
err = fmt.Errorf("failed to apply bundle status patch: %w", err)
return ctrl.Result{}, utilerrors.NewAggregate([]error{resultErr, err})
}
Expand Down Expand Up @@ -260,7 +249,7 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
Kind: string(kind),
},
}
err := b.targetCache.List(ctx, targetList, &client.ListOptions{
err := b.targetReconciler.Cache.List(ctx, targetList, &client.ListOptions{
LabelSelector: labels.SelectorFromSet(map[string]string{
trustapi.BundleLabelKey: bundle.Name,
}),
Expand Down Expand Up @@ -310,12 +299,12 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result

if target.Kind == configMapTarget {
syncFunc = func(targetLog logr.Logger, target targetResource, shouldExist bool) (bool, error) {
return b.syncConfigMapTarget(ctx, targetLog, &bundle, target.Name, target.Namespace, resolvedBundle, shouldExist)
return b.targetReconciler.SyncConfigMap(ctx, targetLog, &bundle, target.NamespacedName, resolvedBundle.Data, shouldExist)
}
}
if target.Kind == secretTarget {
syncFunc = func(targetLog logr.Logger, target targetResource, shouldExist bool) (bool, error) {
return b.syncSecretTarget(ctx, targetLog, &bundle, target.Name, target.Namespace, resolvedBundle, shouldExist)
return b.targetReconciler.SyncSecret(ctx, targetLog, &bundle, target.NamespacedName, resolvedBundle.Data, shouldExist)
}
}

Expand Down Expand Up @@ -394,7 +383,8 @@ func (b *bundle) bundleTargetNamespaceSelector(bundleObj *trustapi.Bundle) (labe
// to ensure that the apply operations will also remove fields that were
// created by the Update operation.
func (b *bundle) migrateBundleStatusToApply(ctx context.Context, obj client.Object) (bool, error) {
patch, err := csaupgrade.UpgradeManagedFieldsPatch(obj, sets.New(fieldManager, crRegressionFieldManager), fieldManager, csaupgrade.Subresource("status"))
fieldManager := string(ssa_client.FieldManager)
patch, err := csaupgrade.UpgradeManagedFieldsPatch(obj, sets.New(fieldManager, ssa_client.CRRegressionFieldManager), fieldManager, csaupgrade.Subresource("status"))
if err != nil {
return false, err
}
Expand Down
33 changes: 19 additions & 14 deletions pkg/bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import (
fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"

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"
"github.com/cert-manager/trust-manager/test/dummy"
"github.com/cert-manager/trust-manager/test/gen"
Expand All @@ -48,7 +50,7 @@ import (
func testEncodeJKS(t *testing.T, data string) []byte {
t.Helper()

encoded, err := jksEncoder{password: DefaultJKSPassword}.encode(data)
encoded, err := target.JKSEncoder{Password: trustapi.DefaultJKSPassword}.Encode(data)
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -210,7 +212,7 @@ func Test_Reconcile(t *testing.T) {
Labels: baseBundleLabels,
Annotations: annotations,
OwnerReferences: baseBundleOwnerRef,
ManagedFields: managedFieldEntries(dataEntries, binDataEntries),
ManagedFields: ssa_client.ManagedFieldEntries(dataEntries, binDataEntries),
},
Data: data,
BinaryData: binData,
Expand Down Expand Up @@ -247,7 +249,7 @@ func Test_Reconcile(t *testing.T) {
Labels: baseBundleLabels,
Annotations: annotations,
OwnerReferences: baseBundleOwnerRef,
ManagedFields: managedFieldEntries(dataEntries, nil),
ManagedFields: ssa_client.ManagedFieldEntries(dataEntries, nil),
},
Data: binaryData,
}
Expand Down Expand Up @@ -493,7 +495,7 @@ func Test_Reconcile(t *testing.T) {
KeySelector: trustapi.KeySelector{
Key: "target.jks",
},
Password: ptr.To(DefaultJKSPassword),
Password: ptr.To(trustapi.DefaultJKSPassword),
},
}),
)},
Expand Down Expand Up @@ -566,7 +568,7 @@ func Test_Reconcile(t *testing.T) {
KeySelector: trustapi.KeySelector{
Key: "target.jks",
},
Password: ptr.To(DefaultJKSPassword),
Password: ptr.To(trustapi.DefaultJKSPassword),
},
}),
),
Expand Down Expand Up @@ -1304,22 +1306,25 @@ func Test_Reconcile(t *testing.T) {

log, ctx := ktesting.NewTestContext(t)
b := &bundle{
client: fakeclient,
targetCache: fakeclient,
recorder: fakerecorder,
clock: fixedclock,
client: fakeclient,
recorder: fakerecorder,
clock: fixedclock,
Options: Options{
Log: log,
Namespace: trustNamespace,
SecretTargetsEnabled: !test.disableSecretTargets,
FilterExpiredCerts: true,
},
patchResourceOverwrite: func(ctx context.Context, obj interface{}) error {
logMutex.Lock()
defer logMutex.Unlock()
targetReconciler: &target.Reconciler{
Client: fakeclient,
Cache: fakeclient,
PatchResourceOverwrite: func(ctx context.Context, obj interface{}) error {
logMutex.Lock()
defer logMutex.Unlock()

resourcePatches = append(resourcePatches, obj)
return nil
resourcePatches = append(resourcePatches, obj)
return nil
},
},
}

Expand Down
14 changes: 9 additions & 5 deletions pkg/bundle/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

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"
)

Expand All @@ -52,11 +53,14 @@ func AddBundleController(
targetCache cache.Cache,
) error {
b := &bundle{
client: mgr.GetClient(),
targetCache: targetCache,
recorder: mgr.GetEventRecorderFor("bundles"),
clock: clock.RealClock{},
Options: opts,
client: mgr.GetClient(),
recorder: mgr.GetEventRecorderFor("bundles"),
clock: clock.RealClock{},
Options: opts,
targetReconciler: &target.Reconciler{
Client: mgr.GetClient(),
Cache: targetCache,
},
}

if b.Options.DefaultPackageLocation != "" {
Expand Down
40 changes: 40 additions & 0 deletions pkg/bundle/internal/ssa_client/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,19 @@ limitations under the License.
package ssa_client

import (
"k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/structured-merge-diff/fieldpath"
)

const (
FieldManager = client.FieldOwner("trust-manager")
// CRRegressionFieldManager is the field manager that was introduced by a regression in controller-runtime
// version 0.15.0; fixed in 15.1 and 0.16.0: https://github.com/kubernetes-sigs/controller-runtime/pull/2435
// trust-manager 0.6.0 was released with this regression in controller-runtime, which means that we have to
// take extra care when migrating from CSA to SSA.
CRRegressionFieldManager = "Go-http-client"
)

type applyPatch struct {
Expand All @@ -34,3 +45,32 @@ func (p applyPatch) Data(_ client.Object) ([]byte, error) {
func (p applyPatch) Type() types.PatchType {
return types.ApplyPatchType
}

func ManagedFieldEntries(fields []string, dataFields []string) []v1.ManagedFieldsEntry {
fieldset := fieldpath.NewSet()
for _, property := range fields {
fieldset.Insert(
fieldpath.MakePathOrDie("data", property),
)
}
for _, property := range dataFields {
fieldset.Insert(
fieldpath.MakePathOrDie("binaryData", property),
)
}

jsonFieldSet, err := fieldset.ToJSON()
if err != nil {
panic(err)
}

return []v1.ManagedFieldsEntry{
{
Manager: "trust-manager",
Operation: v1.ManagedFieldsOperationApply,
FieldsV1: &v1.FieldsV1{
Raw: jsonFieldSet,
},
},
}
}
Loading

0 comments on commit 98b51c9

Please sign in to comment.