refactor: remove finalizer removal code (#3085)

Signed-off-by: Alex Pana <[email protected]> Signed-off-by: Sertaç Özercan <[email protected]> Signed-off-by: alex <[email protected]> Co-authored-by: Sertaç Özercan <[email protected]>
open-policy-agent · Oct 25, 2023 · b18dde9 · b18dde9
1 parent 8544368
commit b18dde9
Show file tree

Hide file tree

Showing 5 changed files with 6 additions and 124 deletions.
diff --git a/pkg/controller/config/config_controller.go b/pkg/controller/config/config_controller.go
@@ -39,8 +39,7 @@ import (
 )
 
 const (
-	ctrlName      = "config-controller"
-	finalizerName = "finalizers.gatekeeper.sh/config"
+	ctrlName = "config-controller"
 )
 
 var log = logf.Log.WithName("controller").WithValues("kind", "Config")
@@ -131,8 +130,7 @@ type ReconcileConfig struct {
 
 // Reconcile reads that state of the cluster for a Config object and makes changes based on the state read
 // and what is in the Config.Spec
-// Automatically generate RBAC rules to allow the Controller to read all things (for sync)
-// update is needed for finalizers.
+// Automatically generate RBAC rules to allow the Controller to read all things (for sync).
 func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
 	// Short-circuit if shutting down.
 	if r.cs != nil {
@@ -161,16 +159,6 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque
 		}
 	}
 
-	// Actively remove config finalizer. This should automatically remove
-	// the finalizer over time even if state teardown didn't work correctly
-	// after a deprecation period, all finalizer code can be removed.
-	if exists && hasFinalizer(instance) {
-		removeFinalizer(instance)
-		if err := r.writer.Update(ctx, instance); err != nil {
-			return reconcile.Result{}, err
-		}
-	}
-
 	newExcluder := process.New()
 	var statsEnabled bool
 	// If the config is being deleted the user is saying they don't want to
@@ -203,30 +191,3 @@ func (r *ReconcileConfig) Reconcile(ctx context.Context, request reconcile.Reque
 
 	return reconcile.Result{}, nil
 }
-
-func containsString(s string, items []string) bool {
-	for _, item := range items {
-		if item == s {
-			return true
-		}
-	}
-	return false
-}
-
-func removeString(s string, items []string) []string {
-	var rval []string
-	for _, item := range items {
-		if item != s {
-			rval = append(rval, item)
-		}
-	}
-	return rval
-}
-
-func hasFinalizer(instance *configv1alpha1.Config) bool {
-	return containsString(finalizerName, instance.GetFinalizers())
-}
-
-func removeFinalizer(instance *configv1alpha1.Config) {
-	instance.SetFinalizers(removeString(finalizerName, instance.GetFinalizers()))
-}
diff --git a/pkg/controller/config/config_controller_test.go b/pkg/controller/config/config_controller_test.go
@@ -301,9 +301,8 @@ func TestConfig_DeleteSyncResources(t *testing.T) {
 	// create the Config object and expect the Reconcile to be created when controller starts
 	instance := &configv1alpha1.Config{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:       "config",
-			Namespace:  "gatekeeper-system",
-			Finalizers: []string{finalizerName},
+			Name:      "config",
+			Namespace: "gatekeeper-system",
 		},
 		Spec: configv1alpha1.ConfigSpec{
 			Sync: configv1alpha1.Sync{
@@ -418,8 +417,6 @@ func setupController(ctx context.Context, mgr manager.Manager, wm *watch.Manager
 		}
 	}
 
-	// ControllerSwitch will be used to disable controllers during our teardown process,
-	// avoiding conflicts in finalizer cleanup.
 	cs := watch.NewSwitch()
 	processExcluder := process.Get()
 	syncMetricsCache := syncutil.NewMetricsCache()

diff --git a/pkg/controller/constraint/constraint_controller.go b/pkg/controller/constraint/constraint_controller.go
@@ -52,10 +52,6 @@ import (
 
 var log = logf.Log.WithName("controller").WithValues(logging.Process, "constraint_controller")
 
-const (
-	finalizerName = "finalizers.gatekeeper.sh/constraint"
-)
-
 type Adder struct {
 	CFClient         *constraintclient.Client
 	ConstraintsCache *ConstraintsCache
@@ -268,13 +264,6 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R
 		}
 	}()
 
-	if HasFinalizer(instance) {
-		RemoveFinalizer(instance)
-		if err := r.writer.Update(ctx, instance); err != nil {
-			return reconcile.Result{Requeue: true}, nil
-		}
-	}
-
 	if !deleted {
 		r.log.Info("handling constraint update", "instance", instance)
 		status, err := r.getOrCreatePodStatus(ctx, instance)
@@ -422,33 +411,6 @@ func (r *ReconcileConstraint) cacheConstraint(ctx context.Context, instance *uns
 	return nil
 }
 
-func RemoveFinalizer(instance *unstructured.Unstructured) {
-	instance.SetFinalizers(removeString(finalizerName, instance.GetFinalizers()))
-}
-
-func HasFinalizer(instance *unstructured.Unstructured) bool {
-	return containsString(finalizerName, instance.GetFinalizers())
-}
-
-func containsString(s string, items []string) bool {
-	for _, item := range items {
-		if item == s {
-			return true
-		}
-	}
-	return false
-}
-
-func removeString(s string, items []string) []string {
-	var rval []string
-	for _, item := range items {
-		if item != s {
-			rval = append(rval, item)
-		}
-	}
-	return rval
-}
-
 func NewConstraintsCache() *ConstraintsCache {
 	return &ConstraintsCache{
 		cache: make(map[string]tags),

diff --git a/pkg/controller/constrainttemplate/constrainttemplate_controller.go b/pkg/controller/constrainttemplate/constrainttemplate_controller.go
@@ -53,8 +53,7 @@ import (
 )
 
 const (
-	finalizerName = "constrainttemplate.finalizers.gatekeeper.sh"
-	ctrlName      = "constrainttemplate-controller"
+	ctrlName = "constrainttemplate-controller"
 )
 
 var logger = log.Log.WithName("controller").WithValues("kind", "ConstraintTemplate", logging.Process, "constraint_template_controller")
@@ -242,9 +241,9 @@ type ReconcileConstraintTemplate struct {
 
 // +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=templates.gatekeeper.sh,resources=constrainttemplates,verbs=get;list;watch;create;update;patch;delete
+// TODO(acpana): remove in 3.16 as per https://github.com/open-policy-agent/gatekeeper/issues/3084
 // +kubebuilder:rbac:groups=templates.gatekeeper.sh,resources=constrainttemplates/finalizers,verbs=get;update;patch;delete
 // +kubebuilder:rbac:groups=templates.gatekeeper.sh,resources=constrainttemplates/status,verbs=get;update;patch
-
 // +kubebuilder:rbac:groups=externaldata.gatekeeper.sh,resources=providers,verbs=get;list;watch;create;update;patch;delete
 
 // Reconcile reads that state of the cluster for a ConstraintTemplate object and makes changes based on the state read
@@ -271,21 +270,9 @@ func (r *ReconcileConstraintTemplate) Reconcile(ctx context.Context, request rec
 			return reconcile.Result{}, err
 		}
 		deleted = true
-		// be sure we are using a blank constraint template so that
-		// we know finalizer removal code won't break (can be removed once that
-		// code is removed)
-		ct = &v1beta1.ConstraintTemplate{}
 	}
 	deleted = deleted || !ct.GetDeletionTimestamp().IsZero()
 
-	if containsString(finalizerName, ct.GetFinalizers()) {
-		RemoveFinalizer(ct)
-		if err := r.Update(ctx, ct); err != nil && !errors.IsNotFound(err) {
-			logger.Error(err, "update error")
-			return reconcile.Result{Requeue: true}, nil
-		}
-	}
-
 	if deleted {
 		ctRef := &templates.ConstraintTemplate{}
 		ctRef.SetNamespace(request.Namespace)
@@ -601,33 +588,10 @@ func logError(name string) {
 	)
 }
 
-func RemoveFinalizer(instance *v1beta1.ConstraintTemplate) {
-	instance.SetFinalizers(removeString(finalizerName, instance.GetFinalizers()))
-}
-
 func makeGvk(kind string) schema.GroupVersionKind {
 	return schema.GroupVersionKind{
 		Group:   "constraints.gatekeeper.sh",
 		Version: "v1beta1",
 		Kind:    kind,
 	}
 }
-
-func containsString(s string, items []string) bool {
-	for _, item := range items {
-		if item == s {
-			return true
-		}
-	}
-	return false
-}
-
-func removeString(s string, items []string) []string {
-	var rval []string
-	for _, item := range items {
-		if item != s {
-			rval = append(rval, item)
-		}
-	}
-	return rval
-}
diff --git a/pkg/readiness/ready_tracker_test.go b/pkg/readiness/ready_tracker_test.go
@@ -116,8 +116,6 @@ func setupController(
 		return fmt.Errorf("setting up tracker: %w", err)
 	}
 
-	// ControllerSwitch will be used to disable controllers during our teardown process,
-	// avoiding conflicts in finalizer cleanup.
 	sw := watch.NewSwitch()
 
 	pod := fakes.Pod(