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: move FieldManager to more available package #397

Merged
merged 1 commit into from
Aug 14, 2024
Merged
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
21 changes: 2 additions & 19 deletions pkg/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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})
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
50 changes: 50 additions & 0 deletions pkg/bundle/internal/ssa_client/migrate.go
Original file line number Diff line number Diff line change
@@ -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
}
4 changes: 4 additions & 0 deletions pkg/bundle/internal/ssa_client/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

const (
FieldManager = client.FieldOwner("trust-manager")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constant seems a bit "misplaced", but I didn't want to create a new Go-file for just this...

)

type applyPatch struct {
patch []byte
}
Expand Down
39 changes: 6 additions & 33 deletions pkg/bundle/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand Down Expand Up @@ -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)
}
Expand All @@ -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")
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}