Merge pull request #397 from erikgb/refactor-fieldmanager

refactor: move FieldManager to more available package

pkg/bundle/bundle.go

@@ -29,7 +29,6 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/csaupgrade"
 	"k8s.io/utils/clock"
@@ -102,7 +101,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.FieldOwner(fieldManager), client.ForceOwnership); 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})
 		}
@@ -129,7 +128,7 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result
 
 	// MIGRATION: If we are upgrading from a version of trust-manager that did use Update to set
 	// the Bundle status, we need to ensure that we do remove the old status fields in case we apply.
-	if didMigrate, err := b.migrateBundleStatusToApply(ctx, &bundle); err != nil {
+	if didMigrate, err := ssa_client.MigrateToApply(ctx, b.client, &bundle, csaupgrade.Subresource("status")); err != nil {
 		log.Error(err, "failed to migrate bundle status")
 		return ctrl.Result{}, nil, fmt.Errorf("failed to migrate bundle status: %w", err)
 	} else if didMigrate {
@@ -382,19 +381,3 @@ func (b *bundle) bundleTargetNamespaceSelector(bundleObj *trustapi.Bundle) (labe
 
 	return metav1.LabelSelectorAsSelector(&metav1.LabelSelector{MatchLabels: nsSelector.MatchLabels})
 }
-
-// MIGRATION: This is a migration function that migrates the ownership of
-// fields from the Update operation to the Apply operation. This is required
-// 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"))
-	if err != nil {
-		return false, err
-	}
-	if patch != nil {
-		return true, b.client.Patch(ctx, obj, client.RawPatch(types.JSONPatchType, patch))
-	}
-	// No work to be done - already upgraded
-	return false, nil
-}

pkg/bundle/internal/ssa_client/migrate.go

@@ -0,0 +1,50 @@
+/*
+Copyright 2021 The cert-manager Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package ssa_client
+
+import (
+	"context"
+
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/client-go/util/csaupgrade"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+const (
+	// 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"
+)
+
+// / MIGRATION: This is a migration function that migrates the ownership of
+// fields from the Update operation to the Apply operation. This is required
+// to ensure that the apply operations will also remove fields that were
+// created by the Update operation.
+func MigrateToApply(ctx context.Context, c client.Client, obj client.Object, opts ...csaupgrade.Option) (bool, error) {
+	patch, err := csaupgrade.UpgradeManagedFieldsPatch(obj, sets.New(string(FieldManager), crRegressionFieldManager), string(FieldManager), opts...)
+	if err != nil {
+		return false, err
+	}
+	if patch != nil {
+		return true, c.Patch(ctx, obj, client.RawPatch(types.JSONPatchType, patch))
+	}
+	// No work to be done - already upgraded
+	return false, nil
+}

pkg/bundle/internal/ssa_client/patch.go

@@ -21,6 +21,10 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+const (
+	FieldManager = client.FieldOwner("trust-manager")
+)
+
 type applyPatch struct {
 	patch []byte
 }

pkg/bundle/target.go

@@ -27,11 +27,9 @@ import (
 	"github.com/go-logr/logr"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	coreapplyconfig "k8s.io/client-go/applyconfigurations/core/v1"
 	metav1applyconfig "k8s.io/client-go/applyconfigurations/meta/v1"
-	"k8s.io/client-go/util/csaupgrade"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/structured-merge-diff/fieldpath"
@@ -40,15 +38,6 @@ import (
 	"github.com/cert-manager/trust-manager/pkg/bundle/internal/ssa_client"
 )
 
-const (
-	// 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"
-	fieldManager             = "trust-manager"
-)
-
 // syncConfigMapTarget syncs the given data to the target ConfigMap in the given namespace.
 // The name of the ConfigMap is the same as the Bundle.
 // Ensures the ConfigMap is owned by the given Bundle, and the data is up to date.
@@ -298,7 +287,7 @@ func (b *bundle) needsUpdate(ctx context.Context, kind targetKind, log logr.Logg
 			return false, fmt.Errorf("unknown targetType: %s", kind)
 		}
 
-		properties, err := listManagedProperties(obj, fieldManager, targetFieldNames...)
+		properties, err := listManagedProperties(obj, ssa_client.FieldManager, targetFieldNames...)
 		if err != nil {
 			return false, fmt.Errorf("failed to list managed properties: %w", err)
 		}
@@ -316,7 +305,7 @@ func (b *bundle) needsUpdate(ctx context.Context, kind targetKind, log logr.Logg
 		if kind == targetKindConfigMap {
 			if bundle.Spec.Target.ConfigMap != nil {
 				// Check if we need to migrate the ConfigMap managed fields to the Apply field operation
-				if didMigrate, err := b.migrateConfigMapToApply(ctx, obj); err != nil {
+				if didMigrate, err := ssa_client.MigrateToApply(ctx, b.client, obj); err != nil {
 					return false, fmt.Errorf("failed to migrate ConfigMap %s/%s to Apply: %w", obj.Namespace, obj.Name, err)
 				} else if didMigrate {
 					log.V(2).Info("migrated configmap from CSA to SSA")
@@ -328,12 +317,12 @@ func (b *bundle) needsUpdate(ctx context.Context, kind targetKind, log logr.Logg
 	return needsUpdate, nil
 }
 
-func listManagedProperties(configmap *metav1.PartialObjectMetadata, fieldManager string, fieldNames ...string) (sets.Set[string], error) {
+func listManagedProperties(configmap *metav1.PartialObjectMetadata, fieldManager client.FieldOwner, fieldNames ...string) (sets.Set[string], error) {
 	properties := sets.New[string]()
 
 	for _, managedField := range configmap.ManagedFields {
 		// If the managed field isn't owned by the cert-manager controller, ignore.
-		if managedField.Manager != fieldManager || managedField.FieldsV1 == nil {
+		if managedField.Manager != string(fieldManager) || managedField.FieldsV1 == nil {
 			continue
 		}
 
@@ -370,7 +359,7 @@ func (b *bundle) patchConfigMapResource(ctx context.Context, applyConfig *coreap
 		return fmt.Errorf("failed to generate patch: %w", err)
 	}
 
-	err = b.client.Patch(ctx, configMap, patch, client.FieldOwner(fieldManager), client.ForceOwnership)
+	err = b.client.Patch(ctx, configMap, patch, ssa_client.FieldManager, client.ForceOwnership)
 	if err != nil {
 		return err
 	}
@@ -393,7 +382,7 @@ func (b *bundle) patchSecretResource(ctx context.Context, applyConfig *coreapply
 		return fmt.Errorf("failed to generate patch: %w", err)
 	}
 
-	err = b.client.Patch(ctx, secret, patch, client.FieldOwner(fieldManager), client.ForceOwnership)
+	err = b.client.Patch(ctx, secret, patch, ssa_client.FieldManager, client.ForceOwnership)
 	if err != nil {
 		return err
 	}
@@ -405,19 +394,3 @@ func (b *bundle) patchSecretResource(ctx context.Context, applyConfig *coreapply
 
 	return nil
 }
-
-// MIGRATION: This is a migration function that migrates the ownership of
-// fields from the Update operation to the Apply operation. This is required
-// to ensure that the apply operations will also remove fields that were
-// created by the Update operation.
-func (b *bundle) migrateConfigMapToApply(ctx context.Context, obj client.Object) (bool, error) {
-	patch, err := csaupgrade.UpgradeManagedFieldsPatch(obj, sets.New(fieldManager, crRegressionFieldManager), fieldManager)
-	if err != nil {
-		return false, err
-	}
-	if patch != nil {
-		return true, b.client.Patch(ctx, obj, client.RawPatch(types.JSONPatchType, patch))
-	}
-	// No work to be done - already upgraded
-	return false, nil
-}