Merge pull request #2668 from k8s-infra-cherrypick-robot/cherry-pick-2661-to-release-1.5

[release-1.5] Add finalizer to AzureClusterIdentity

Author: k8s-ci-robot

controllers/azurecluster_controller.go

@@ -159,14 +159,10 @@ func (acr *AzureClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	}
 
 	if azureCluster.Spec.IdentityRef != nil {
-		identity, err := GetClusterIdentityFromRef(ctx, acr.Client, azureCluster.Namespace, azureCluster.Spec.IdentityRef)
+		err := EnsureClusterIdentity(ctx, acr.Client, azureCluster, azureCluster.Spec.IdentityRef, infrav1.ClusterFinalizer)
 		if err != nil {
 			return reconcile.Result{}, err
 		}
-		if !scope.IsClusterNamespaceAllowed(ctx, acr.Client, identity.Spec.AllowedNamespaces, azureCluster.Namespace) {
-			conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, infrav1.NamespaceNotAllowedByIdentity, clusterv1.ConditionSeverityError, "")
-			return reconcile.Result{}, errors.New("AzureClusterIdentity list of allowed namespaces doesn't include current cluster namespace")
-		}
 	} else {
 		log.Info(fmt.Sprintf("WARNING, %s", deprecatedManagerCredsWarning))
 		acr.Recorder.Eventf(azureCluster, corev1.EventTypeWarning, "AzureClusterIdentity", deprecatedManagerCredsWarning)
@@ -298,7 +294,15 @@ func (acr *AzureClusterReconciler) reconcileDelete(ctx context.Context, clusterS
 	}
 
 	// Cluster is deleted so remove the finalizer.
-	controllerutil.RemoveFinalizer(clusterScope.AzureCluster, infrav1.ClusterFinalizer)
+	controllerutil.RemoveFinalizer(azureCluster, infrav1.ClusterFinalizer)
+
+	if azureCluster.Spec.IdentityRef != nil {
+		// Cluster is deleted so remove the identity finalizer.
+		err := RemoveClusterIdentityFinalizer(ctx, acr.Client, azureCluster, azureCluster.Spec.IdentityRef, infrav1.ClusterFinalizer)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
+	}
 
 	return reconcile.Result{}, nil
 }

controllers/helpers.go

@@ -42,10 +42,13 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
+	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/cluster-api/util/patch"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 )
 
@@ -597,3 +600,49 @@ func GetClusterIdentityFromRef(ctx context.Context, c client.Client, azureCluste
 	}
 	return nil, nil
 }
+
+func clusterIdentityFinalizer(prefix, clusterNamespace, clusterName string) string {
+	return fmt.Sprintf("%s/%s-%s", prefix, clusterNamespace, clusterName)
+}
+
+// EnsureClusterIdentity ensures that the identity ref is allowed in the namespace and sets a finalizer.
+func EnsureClusterIdentity(ctx context.Context, c client.Client, object conditions.Setter, identityRef *corev1.ObjectReference, finalizerPrefix string) error {
+	name := object.GetName()
+	namespace := object.GetNamespace()
+	identity, err := GetClusterIdentityFromRef(ctx, c, namespace, identityRef)
+	if err != nil {
+		return err
+	}
+	if !scope.IsClusterNamespaceAllowed(ctx, c, identity.Spec.AllowedNamespaces, namespace) {
+		conditions.MarkFalse(object, infrav1.NetworkInfrastructureReadyCondition, infrav1.NamespaceNotAllowedByIdentity, clusterv1.ConditionSeverityError, "")
+		return errors.New("AzureClusterIdentity list of allowed namespaces doesn't include current cluster namespace")
+	}
+	identityHelper, err := patch.NewHelper(identity, c)
+	if err != nil {
+		return errors.Wrap(err, "failed to init patch helper")
+	}
+	// If the AzureClusterIdentity doesn't have our finalizer, add it.
+	controllerutil.AddFinalizer(identity, clusterIdentityFinalizer(finalizerPrefix, namespace, name))
+	// Register the finalizer immediately to avoid orphaning Azure resources on delete.
+	return identityHelper.Patch(ctx, identity)
+}
+
+// RemoveClusterIdentityFinalizer removes the finalizer on an AzureClusterIdentity.
+func RemoveClusterIdentityFinalizer(ctx context.Context, c client.Client, object client.Object, identityRef *corev1.ObjectReference, finalizerPrefix string) error {
+	name := object.GetName()
+	namespace := object.GetNamespace()
+	identity, err := GetClusterIdentityFromRef(ctx, c, namespace, identityRef)
+	if err != nil {
+		return err
+	}
+	identityHelper, err := patch.NewHelper(identity, c)
+	if err != nil {
+		return errors.Wrap(err, "failed to init patch helper")
+	}
+	controllerutil.RemoveFinalizer(identity, clusterIdentityFinalizer(finalizerPrefix, namespace, name))
+	err = identityHelper.Patch(ctx, identity)
+	if err != nil {
+		return errors.Wrap(err, "failed to patch AzureClusterIdentity")
+	}
+	return nil
+}

exp/api/v1beta1/azuremanagedcontrolplane_types.go

@@ -24,6 +24,10 @@ import (
 )
 
 const (
+	// ManagedClusterFinalizer allows Reconcile to clean up Azure resources associated with the AzureManagedControlPlane before
+	// removing it from the apiserver.
+	ManagedClusterFinalizer = "azuremanagedcontrolplane.infrastructure.cluster.x-k8s.io"
+
 	// PrivateDNSZoneModeSystem represents mode System for azuremanagedcontrolplane.
 	PrivateDNSZoneModeSystem string = "System"
 

exp/controllers/azuremanagedcontrolplane_controller.go

@@ -181,13 +181,10 @@ func (amcpr *AzureManagedControlPlaneReconciler) Reconcile(ctx context.Context,
 
 	// check if the control plane's namespace is allowed for this identity and update owner references for the identity.
 	if azureControlPlane.Spec.IdentityRef != nil {
-		identity, err := infracontroller.GetClusterIdentityFromRef(ctx, amcpr.Client, azureControlPlane.Namespace, azureControlPlane.Spec.IdentityRef)
+		err := infracontroller.EnsureClusterIdentity(ctx, amcpr.Client, azureControlPlane, azureControlPlane.Spec.IdentityRef, infrav1exp.ManagedClusterFinalizer)
 		if err != nil {
 			return reconcile.Result{}, err
 		}
-		if !scope.IsClusterNamespaceAllowed(ctx, amcpr.Client, identity.Spec.AllowedNamespaces, azureControlPlane.Namespace) {
-			return reconcile.Result{}, errors.New("AzureClusterIdentity list of allowed namespaces doesn't include current azure managed control plane namespace")
-		}
 	} else {
 		warningMessage := ("You're using deprecated functionality: ")
 		warningMessage += ("Using Azure credentials from the manager environment is deprecated and will be removed in future releases. ")
@@ -228,8 +225,10 @@ func (amcpr *AzureManagedControlPlaneReconciler) reconcileNormal(ctx context.Con
 
 	log.Info("Reconciling AzureManagedControlPlane")
 
+	// Remove deprecated Cluster finalizer if it exists.
+	controllerutil.RemoveFinalizer(scope.ControlPlane, infrav1.ClusterFinalizer)
 	// If the AzureManagedControlPlane doesn't have our finalizer, add it.
-	controllerutil.AddFinalizer(scope.ControlPlane, infrav1.ClusterFinalizer)
+	controllerutil.AddFinalizer(scope.ControlPlane, infrav1exp.ManagedClusterFinalizer)
 	// Register the finalizer immediately to avoid orphaning Azure resources on delete
 	if err := scope.PatchObject(ctx); err != nil {
 		amcpr.Recorder.Eventf(scope.ControlPlane, corev1.EventTypeWarning, "AzureManagedControlPlane unavailable", "failed to patch resource: %s", err)
@@ -275,7 +274,14 @@ func (amcpr *AzureManagedControlPlaneReconciler) reconcileDelete(ctx context.Con
 	}
 
 	// Cluster is deleted so remove the finalizer.
-	controllerutil.RemoveFinalizer(scope.ControlPlane, infrav1.ClusterFinalizer)
+	controllerutil.RemoveFinalizer(scope.ControlPlane, infrav1exp.ManagedClusterFinalizer)
+
+	if scope.ControlPlane.Spec.IdentityRef != nil {
+		err := infracontroller.RemoveClusterIdentityFinalizer(ctx, amcpr.Client, scope.ControlPlane, scope.ControlPlane.Spec.IdentityRef, infrav1exp.ManagedClusterFinalizer)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
+	}
 
 	return reconcile.Result{}, nil
 }