chore: remove support for ownership and managed-by legacy labels (#976)

CHANGELOG.md

@@ -34,6 +34,9 @@
   [#972](https://github.com/Kong/gateway-operator/pull/972)
 - Allow more than 1 replica for `ControlPlane`'s `Deployment` to support HA deployments of KIC.
   [#978](https://github.com/Kong/gateway-operator/pull/978)
+- Removed support for the migration of legacy labels so upgrading the operator from 1.3 (or older) to 1.5.0,
+  should be done through 1.4.1
+  [#976](https://github.com/Kong/gateway-operator/pull/976)
 
 ### Fixes
 
@@ -167,7 +170,7 @@
 ### Fixed
 
 - Fixed `ControlPlane` cluster wide resources not migrating to new ownership labels
-  (introduced in 1.3.0) when upgrading the operator form 1.2 (or older) to 1.3.0.
+  (introduced in 1.3.0) when upgrading the operator from 1.2 (or older) to 1.3.0.
   [#369](https://github.com/Kong/gateway-operator/pull/369)
 - Requeue instead of reporting an error when a finalizer removal yields a conflict.
   [#454](https://github.com/Kong/gateway-operator/pull/454)

controller/controlplane/controller_reconciler_utils.go

@@ -292,22 +292,6 @@ func (r *Reconciler) ensureClusterRole(
 	ctx context.Context,
 	cp *operatorv1beta1.ControlPlane,
 ) (createdOrUpdated bool, cr *rbacv1.ClusterRole, err error) {
-	// NOTE: Code below performs a migration from the old managedBy label to the new one.
-	// It lists both resources labeled with the old and the new label set and merges them,
-	// then it reduces the number of resources to 1 and eventually updates the resource.
-	// After several versions of soak time we can remove handling the legacy label set.
-	// PR that introduced the new set of labels: https://github.com/Kong/gateway-operator/pull/259
-	// PR that introduced the migration: https://github.com/Kong/gateway-operator/pull/369
-	// TODO: https://github.com/Kong/gateway-operator/issues/401.
-	clusterRolesLegacy, err := k8sutils.ListClusterRoles(
-		ctx,
-		r.Client,
-		client.MatchingLabels(k8sutils.GetLegacyManagedByLabelSet(cp)),
-	)
-	if err != nil {
-		return false, nil, err
-	}
-
 	clusterRoles, err := k8sutils.ListClusterRoles(
 		ctx,
 		r.Client,
@@ -317,19 +301,6 @@ func (r *Reconciler) ensureClusterRole(
 		return false, nil, err
 	}
 
-	// NOTE: This is a temporary workaround to handle the migration from the old managedBy label to the new one.
-	// The reason for this is because those label sets overlap.
-	for i, cr := range clusterRolesLegacy {
-		nn := client.ObjectKeyFromObject(&cr)
-		if lo.ContainsBy(clusterRoles, func(iterator rbacv1.ClusterRole) bool {
-			return client.ObjectKeyFromObject(&iterator) == nn
-		}) {
-			continue
-		}
-
-		clusterRoles = append(clusterRoles, clusterRolesLegacy[i])
-	}
-
 	count := len(clusterRoles)
 	if count > 1 {
 		if err := k8sreduce.ReduceClusterRoles(ctx, r.Client, clusterRoles); err != nil {
@@ -377,22 +348,6 @@ func (r *Reconciler) ensureClusterRoleBinding(
 ) (createdOrUpdate bool, crb *rbacv1.ClusterRoleBinding, err error) {
 	logger := log.GetLogger(ctx, "controlplane.ensureClusterRoleBinding", r.DevelopmentMode)
 
-	// NOTE: Code below performs a migration from the old managedBy label to the new one.
-	// It lists both resources labeled with the old and the new label set and merges them,
-	// then it reduces the number of resources to 1 and eventually updates the resource.
-	// After several versions of soak time we can remove handling the legacy label set.
-	// PR that introduced the new set of labels: https://github.com/Kong/gateway-operator/pull/259
-	// PR that introduced the migration: https://github.com/Kong/gateway-operator/pull/369
-	// TODO: https://github.com/Kong/gateway-operator/issues/401.
-	clusterRoleBindingsLegacy, err := k8sutils.ListClusterRoleBindings(
-		ctx,
-		r.Client,
-		client.MatchingLabels(k8sutils.GetLegacyManagedByLabelSet(cp)),
-	)
-	if err != nil {
-		return false, nil, err
-	}
-
 	clusterRoleBindings, err := k8sutils.ListClusterRoleBindings(
 		ctx,
 		r.Client,
@@ -402,19 +357,6 @@ func (r *Reconciler) ensureClusterRoleBinding(
 		return false, nil, err
 	}
 
-	// NOTE: This is a temporary workaround to handle the migration from the old managedBy label to the new one.
-	// The reason for this is because those label sets overlap.
-	for i, crb := range clusterRoleBindingsLegacy {
-		nn := client.ObjectKeyFromObject(&crb)
-		if lo.ContainsBy(clusterRoleBindings, func(iterator rbacv1.ClusterRoleBinding) bool {
-			return client.ObjectKeyFromObject(&iterator) == nn
-		}) {
-			continue
-		}
-
-		clusterRoleBindings = append(clusterRoleBindings, clusterRoleBindingsLegacy[i])
-	}
-
 	count := len(clusterRoleBindings)
 	if count > 1 {
 		if err := k8sreduce.ReduceClusterRoleBindings(ctx, r.Client, clusterRoleBindings); err != nil {
@@ -556,15 +498,6 @@ func (r *Reconciler) ensureOwnedClusterRolesDeleted(
 	ctx context.Context,
 	cp *operatorv1beta1.ControlPlane,
 ) (deletions bool, err error) {
-	// TODO: Remove listing with old labels https://github.com/Kong/gateway-operator/issues/401.
-	clusterRolesLegacy, err := k8sutils.ListClusterRoles(
-		ctx,
-		r.Client,
-		client.MatchingLabels(k8sutils.GetLegacyManagedByLabelSet(cp)),
-	)
-	if err != nil {
-		return false, err
-	}
 
 	clusterRoles, err := k8sutils.ListClusterRoles(
 		ctx,
@@ -575,15 +508,12 @@ func (r *Reconciler) ensureOwnedClusterRolesDeleted(
 		return false, err
 	}
 
-	clusterRoles = append(clusterRoles, clusterRolesLegacy...)
-
 	var (
 		deleted bool
 		errs    []error
 	)
 	for i := range clusterRoles {
-		err = r.Client.Delete(ctx, &clusterRoles[i])
-		if client.IgnoreNotFound(err) != nil {
+		if err = r.Client.Delete(ctx, &clusterRoles[i]); client.IgnoreNotFound(err) != nil {
 			errs = append(errs, err)
 			continue
 		}
@@ -600,16 +530,6 @@ func (r *Reconciler) ensureOwnedClusterRoleBindingsDeleted(
 	ctx context.Context,
 	cp *operatorv1beta1.ControlPlane,
 ) (deletions bool, err error) {
-	// TODO: Remove listing with old labels https://github.com/Kong/gateway-operator/issues/401.
-	clusterRoleBindingsLegacy, err := k8sutils.ListClusterRoleBindings(
-		ctx,
-		r.Client,
-		client.MatchingLabels(k8sutils.GetLegacyManagedByLabelSet(cp)),
-	)
-	if err != nil {
-		return false, err
-	}
-
 	clusterRoleBindings, err := k8sutils.ListClusterRoleBindings(
 		ctx,
 		r.Client,
@@ -619,15 +539,12 @@ func (r *Reconciler) ensureOwnedClusterRoleBindingsDeleted(
 		return false, err
 	}
 
-	clusterRoleBindings = append(clusterRoleBindings, clusterRoleBindingsLegacy...)
-
 	var (
 		deleted bool
 		errs    []error
 	)
 	for i := range clusterRoleBindings {
-		err = r.Client.Delete(ctx, &clusterRoleBindings[i])
-		if client.IgnoreNotFound(err) != nil {
+		if err = r.Client.Delete(ctx, &clusterRoleBindings[i]); client.IgnoreNotFound(err) != nil {
 			errs = append(errs, err)
 			continue
 		}
@@ -640,18 +557,6 @@ func (r *Reconciler) ensureOwnedClusterRoleBindingsDeleted(
 func (r *Reconciler) ensureOwnedValidatingWebhookConfigurationDeleted(ctx context.Context,
 	cp *operatorv1beta1.ControlPlane,
 ) (deletions bool, err error) {
-	// TODO: Remove listing with old labels and owner ref https://github.com/Kong/gateway-operator/issues/401.
-	validatingWebhookConfigurationsLegacy, err := k8sutils.ListValidatingWebhookConfigurationsForOwner(
-		ctx,
-		r.Client,
-		cp.GetUID(),
-		// NOTE: this uses only the 1 label to find the legacy webhook configurations not the label set
-		// because app:<name> is not set on ValidatingWebhookConfiguration.
-		client.MatchingLabels(k8sutils.GetLegacyManagedByLabel(cp)),
-	)
-	if err != nil {
-		return false, fmt.Errorf("failed listing webhook configurations for owner: %w", err)
-	}
 
 	validatingWebhookConfigurations, err := k8sutils.ListValidatingWebhookConfigurations(
 		ctx,
@@ -662,15 +567,12 @@ func (r *Reconciler) ensureOwnedValidatingWebhookConfigurationDeleted(ctx contex
 		return false, fmt.Errorf("failed listing webhook configurations for owner: %w", err)
 	}
 
-	validatingWebhookConfigurations = append(validatingWebhookConfigurations, validatingWebhookConfigurationsLegacy...)
-
 	var (
 		deleted bool
 		errs    []error
 	)
 	for i := range validatingWebhookConfigurations {
-		err = r.Client.Delete(ctx, &validatingWebhookConfigurations[i])
-		if client.IgnoreNotFound(err) != nil {
+		if err = r.Client.Delete(ctx, &validatingWebhookConfigurations[i]); client.IgnoreNotFound(err) != nil {
 			errs = append(errs, err)
 			continue
 		}
@@ -762,25 +664,6 @@ func (r *Reconciler) ensureValidatingWebhookConfiguration(
 ) (op.Result, error) {
 	logger := log.GetLogger(ctx, "controlplane.ensureValidatingWebhookConfiguration", r.DevelopmentMode)
 
-	// NOTE: Code below performs a migration from the old managedBy label to the new one.
-	// It lists both resources labeled with the old and the new label set and merges them,
-	// then it reduces the number of resources to 1 and eventually updates the resource.
-	// After several versions of soak time we can remove handling the legacy label set.
-	// PR that introduced the new set of labels: https://github.com/Kong/gateway-operator/pull/259
-	// PR that introduced the migration: https://github.com/Kong/gateway-operator/pull/369
-	// TODO: https://github.com/Kong/gateway-operator/issues/401.
-	validatingWebhookConfigurationsLegacy, err := k8sutils.ListValidatingWebhookConfigurationsForOwner(
-		ctx,
-		r.Client,
-		cp.GetUID(),
-		// NOTE: this uses only the 1 label to find the legacy webhook configurations not the label set
-		// because app:<name> is not set on ValidatingWebhookConfiguration.
-		client.MatchingLabels(k8sutils.GetLegacyManagedByLabel(cp)),
-	)
-	if err != nil {
-		return op.Noop, fmt.Errorf("failed listing webhook configurations for owner: %w", err)
-	}
-
 	validatingWebhookConfigurations, err := k8sutils.ListValidatingWebhookConfigurations(
 		ctx,
 		r.Client,
@@ -790,19 +673,6 @@ func (r *Reconciler) ensureValidatingWebhookConfiguration(
 		return op.Noop, fmt.Errorf("failed listing webhook configurations for owner: %w", err)
 	}
 
-	// NOTE: This is a temporary workaround to handle the migration from the old managedBy label to the new one.
-	// The reason for this is because those label sets overlap.
-	for i, vwc := range validatingWebhookConfigurationsLegacy {
-		nn := client.ObjectKeyFromObject(&vwc)
-		if lo.ContainsBy(validatingWebhookConfigurations, func(iterator admregv1.ValidatingWebhookConfiguration) bool {
-			return client.ObjectKeyFromObject(&iterator) == nn
-		}) {
-			continue
-		}
-
-		validatingWebhookConfigurations = append(validatingWebhookConfigurations, validatingWebhookConfigurationsLegacy[i])
-	}
-
 	count := len(validatingWebhookConfigurations)
 	if count > 1 {
 		if err := k8sreduce.ReduceValidatingWebhookConfigurations(ctx, r.Client, validatingWebhookConfigurations); err != nil {

controller/controlplane/controller_watch.go

@@ -17,7 +17,6 @@ import (
 	operatorerrors "github.com/kong/gateway-operator/internal/errors"
 	"github.com/kong/gateway-operator/internal/utils/index"
 	"github.com/kong/gateway-operator/pkg/consts"
-	k8sutils "github.com/kong/gateway-operator/pkg/utils/kubernetes"
 )
 
 // -----------------------------------------------------------------------------
@@ -165,18 +164,10 @@ func (r *Reconciler) getControlPlaneRequestFromManagedByNameLabel(ctx context.Co
 }
 
 // objectIsOwnedByControlPlane checks if the object is owned by the control plane.
-//
-// NOTE: We are using the managed-by-name label to identify the owner of the resource.
-// To keep backward compatibility, we also check the owner reference which
-// is not used anymore for cluster-scoped resources since that's considered
-// an error.
+// It relies on managed-by-name label to identify the owner of the resource.
+// It doesn't check owner reference which is considered as an error for cluster-scoped resources.
 func objectIsOwnedByControlPlane(obj client.Object, cp *operatorv1beta1.ControlPlane) bool {
-	if k8sutils.IsOwnedByRefUID(obj, cp.GetUID()) {
-		return true
-	}
-
-	labels := obj.GetLabels()
-	if labels[consts.GatewayOperatorManagedByNameLabel] == cp.Name {
+	if obj.GetLabels()[consts.GatewayOperatorManagedByNameLabel] == cp.Name {
 		if obj.GetNamespace() != "" {
 			return cp.GetNamespace() == obj.GetNamespace()
 		} else {

controller/dataplane/bluegreen_controller_test.go

@@ -122,10 +122,9 @@ func TestDataPlaneBlueGreenReconciler_Reconcile(t *testing.T) {
 						Name:      "test-dataplane-deployment",
 						Namespace: "default",
 						Labels: map[string]string{
-							"app":                                      "test-dataplane",
-							consts.DataPlaneDeploymentStateLabel:       string(consts.DataPlaneStateLabelValueLive),
-							consts.GatewayOperatorManagedByLabel:       string(consts.DataPlaneManagedLabelValue),
-							consts.GatewayOperatorManagedByLabelLegacy: string(consts.DataPlaneManagedLabelValue),
+							"app":                                "test-dataplane",
+							consts.DataPlaneDeploymentStateLabel: string(consts.DataPlaneStateLabelValueLive),
+							consts.GatewayOperatorManagedByLabel: string(consts.DataPlaneManagedLabelValue),
 						},
 					},
 					Status: appsv1.DeploymentStatus{
@@ -139,11 +138,10 @@ func TestDataPlaneBlueGreenReconciler_Reconcile(t *testing.T) {
 						Name:      "test-admin-service",
 						Namespace: "default",
 						Labels: map[string]string{
-							"app":                                      "test-dataplane",
-							consts.DataPlaneServiceTypeLabel:           string(consts.DataPlaneAdminServiceLabelValue),
-							consts.DataPlaneServiceStateLabel:          string(consts.DataPlaneStateLabelValueLive),
-							consts.GatewayOperatorManagedByLabel:       string(consts.DataPlaneManagedLabelValue),
-							consts.GatewayOperatorManagedByLabelLegacy: string(consts.DataPlaneManagedLabelValue),
+							"app":                                "test-dataplane",
+							consts.DataPlaneServiceTypeLabel:     string(consts.DataPlaneAdminServiceLabelValue),
+							consts.DataPlaneServiceStateLabel:    string(consts.DataPlaneStateLabelValueLive),
+							consts.GatewayOperatorManagedByLabel: string(consts.DataPlaneManagedLabelValue),
 						},
 					},
 					Spec: corev1.ServiceSpec{
@@ -155,11 +153,10 @@ func TestDataPlaneBlueGreenReconciler_Reconcile(t *testing.T) {
 						Name:      "test-proxy-service",
 						Namespace: "default",
 						Labels: map[string]string{
-							"app":                                      "test-dataplane",
-							consts.DataPlaneServiceStateLabel:          consts.DataPlaneStateLabelValueLive,
-							consts.DataPlaneServiceTypeLabel:           string(consts.DataPlaneIngressServiceLabelValue),
-							consts.GatewayOperatorManagedByLabel:       string(consts.DataPlaneManagedLabelValue),
-							consts.GatewayOperatorManagedByLabelLegacy: string(consts.DataPlaneManagedLabelValue),
+							"app":                                "test-dataplane",
+							consts.DataPlaneServiceStateLabel:    consts.DataPlaneStateLabelValueLive,
+							consts.DataPlaneServiceTypeLabel:     string(consts.DataPlaneIngressServiceLabelValue),
+							consts.GatewayOperatorManagedByLabel: string(consts.DataPlaneManagedLabelValue),
 						},
 					},
 					Spec: corev1.ServiceSpec{
@@ -465,9 +462,9 @@ func TestEnsurePreviewIngressService(t *testing.T) {
 					Labels: map[string]string{
 						"app":                                "dp-0",
 						consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue,
-						consts.GatewayOperatorManagedByLabelLegacy: consts.DataPlaneManagedLabelValue,
-						consts.DataPlaneServiceTypeLabel:           string(consts.DataPlaneIngressServiceLabelValue),
-						consts.DataPlaneServiceStateLabel:          consts.DataPlaneStateLabelValuePreview,
+
+						consts.DataPlaneServiceTypeLabel:  string(consts.DataPlaneIngressServiceLabelValue),
+						consts.DataPlaneServiceStateLabel: consts.DataPlaneStateLabelValuePreview,
 					},
 				},
 				Spec: corev1.ServiceSpec{
@@ -496,9 +493,9 @@ func TestEnsurePreviewIngressService(t *testing.T) {
 					Labels: map[string]string{
 						"app":                                "dp-1",
 						consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue,
-						consts.GatewayOperatorManagedByLabelLegacy: consts.DataPlaneManagedLabelValue,
-						consts.DataPlaneServiceTypeLabel:           string(consts.DataPlaneIngressServiceLabelValue),
-						consts.DataPlaneServiceStateLabel:          consts.DataPlaneStateLabelValuePreview,
+
+						consts.DataPlaneServiceTypeLabel:  string(consts.DataPlaneIngressServiceLabelValue),
+						consts.DataPlaneServiceStateLabel: consts.DataPlaneStateLabelValuePreview,
 					},
 				},
 				Spec: corev1.ServiceSpec{
@@ -542,9 +539,9 @@ func TestEnsurePreviewIngressService(t *testing.T) {
 					Labels: map[string]string{
 						"app":                                "dp-1",
 						consts.GatewayOperatorManagedByLabel: consts.DataPlaneManagedLabelValue,
-						consts.GatewayOperatorManagedByLabelLegacy: consts.DataPlaneManagedLabelValue,
-						consts.DataPlaneServiceTypeLabel:           string(consts.DataPlaneIngressServiceLabelValue),
-						consts.DataPlaneServiceStateLabel:          consts.DataPlaneStateLabelValuePreview,
+
+						consts.DataPlaneServiceTypeLabel:  string(consts.DataPlaneIngressServiceLabelValue),
+						consts.DataPlaneServiceStateLabel: consts.DataPlaneStateLabelValuePreview,
 					},
 				},
 				Spec: corev1.ServiceSpec{