fix: fix ControlPlane managed resource labels during migration from 1.2 and older

CHANGELOG.md

@@ -30,6 +30,12 @@
 - Proper `User-Agent` header is now set on outgoing HTTP requests.
   [#387](https://github.com/Kong/gateway-operator/pull/387)
 
+### 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.
+  [#369](https://github.com/Kong/gateway-operator/pull/369)
+
 ## [v1.3.0]
 
 > Release date: 2024-06-24

controller/controlplane/controller.go

@@ -63,9 +63,6 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	validatinWebhookConfigurationOwnerPredicate.UpdateFunc = func(e event.UpdateEvent) bool {
 		return r.validatingWebhookConfigurationHasControlPlaneOwner(e.ObjectOld)
 	}
-	validatinWebhookConfigurationOwnerPredicate.DeleteFunc = func(e event.DeleteEvent) bool {
-		return r.validatingWebhookConfigurationHasControlPlaneOwner(e.Object)
-	}
 
 	return ctrl.NewControllerManagedBy(mgr).
 		// watch ControlPlane objects

controller/controlplane/controller_reconciler_utils.go

@@ -293,16 +293,44 @@ func (r *Reconciler) ensureClusterRole(
 	ctx context.Context,
 	cp *operatorv1beta1.ControlPlane,
 ) (createdOrUpdated bool, cr *rbacv1.ClusterRole, err error) {
-	managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
+	// 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,
-		client.MatchingLabels(managedByLabelSet),
+		client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
 	)
 	if err != nil {
 		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 {
@@ -350,17 +378,44 @@ func (r *Reconciler) ensureClusterRoleBinding(
 ) (createdOrUpdate bool, crb *rbacv1.ClusterRoleBinding, err error) {
 	logger := log.GetLogger(ctx, "controlplane.ensureClusterRoleBinding", r.DevelopmentMode)
 
-	managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
+	// 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,
-		client.MatchingLabels(managedByLabelSet),
+		client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
 	)
 	if err != nil {
 		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 {
@@ -503,24 +558,39 @@ func (r *Reconciler) ensureOwnedClusterRolesDeleted(
 	ctx context.Context,
 	cp *operatorv1beta1.ControlPlane,
 ) (deletions bool, err error) {
-	managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
+	// 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, r.Client,
-		client.MatchingLabels(managedByLabelSet),
+		ctx,
+		r.Client,
+		client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
 	)
 	if err != nil {
 		return false, err
 	}
 
+	clusterRoles = append(clusterRoles, clusterRolesLegacy...)
+
 	var (
 		deleted bool
 		errs    []error
 	)
 	for i := range clusterRoles {
 		err = r.Client.Delete(ctx, &clusterRoles[i])
-		if err != nil && !k8serrors.IsNotFound(err) {
+		if err != nil {
+			if k8serrors.IsNotFound(err) {
+				continue
+			}
 			errs = append(errs, err)
+			continue
 		}
 		deleted = true
 	}
@@ -535,24 +605,39 @@ func (r *Reconciler) ensureOwnedClusterRoleBindingsDeleted(
 	ctx context.Context,
 	cp *operatorv1beta1.ControlPlane,
 ) (deletions bool, err error) {
-	managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
+	// 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,
-		client.MatchingLabels(managedByLabelSet),
+		ctx,
+		r.Client,
+		client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
 	)
 	if err != nil {
 		return false, err
 	}
 
+	clusterRoleBindings = append(clusterRoleBindings, clusterRoleBindingsLegacy...)
+
 	var (
 		deleted bool
 		errs    []error
 	)
 	for i := range clusterRoleBindings {
 		err = r.Client.Delete(ctx, &clusterRoleBindings[i])
-		if err != nil && !k8serrors.IsNotFound(err) {
+		if err != nil {
+			if k8serrors.IsNotFound(err) {
+				continue
+			}
 			errs = append(errs, err)
+			continue
 		}
 		deleted = true
 	}
@@ -561,26 +646,44 @@ func (r *Reconciler) ensureOwnedClusterRoleBindingsDeleted(
 }
 
 func (r *Reconciler) ensureOwnedValidatingWebhookConfigurationDeleted(ctx context.Context,
-	cp *operatorv1beta1.ControlPlane) (deletions bool, err error) {
-	managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
+	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,
 		r.Client,
-		client.MatchingLabels(managedByLabelSet),
+		client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
 	)
 	if err != nil {
 		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 err != nil && !k8serrors.IsNotFound(err) {
+		if err != nil {
+			if k8serrors.IsNotFound(err) {
+				continue
+			}
 			errs = append(errs, err)
+			continue
 		}
 		deleted = true
 	}
@@ -671,15 +774,45 @@ func (r *Reconciler) ensureValidatingWebhookConfiguration(
 ) (op.Result, error) {
 	logger := log.GetLogger(ctx, "controlplane.ensureValidatingWebhookConfiguration", r.DevelopmentMode)
 
-	managedByLabelSet := k8sutils.GetManagedByLabelSet(cp)
+	// 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,
-		client.MatchingLabels(managedByLabelSet),
+		client.MatchingLabels(k8sutils.GetManagedByLabelSet(cp)),
 	)
 	if err != nil {
-		return op.Noop, err
+		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)
@@ -736,7 +869,8 @@ func (r *Reconciler) ensureValidatingWebhookConfiguration(
 
 		updated, webhookConfiguration.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(webhookConfiguration.ObjectMeta, generatedWebhookConfiguration.ObjectMeta)
 
-		if !cmp.Equal(webhookConfiguration.Webhooks, generatedWebhookConfiguration.Webhooks) {
+		if !cmp.Equal(webhookConfiguration.Webhooks, generatedWebhookConfiguration.Webhooks) ||
+			!cmp.Equal(webhookConfiguration.Labels, generatedWebhookConfiguration.Labels) {
 			webhookConfiguration.Webhooks = generatedWebhookConfiguration.Webhooks
 			updated = true
 		}