diff --git a/controllers/azurejson_machine_controller.go b/controllers/azurejson_machine_controller.go index 7df5345ca6a..c510364911c 100644 --- a/controllers/azurejson_machine_controller.go +++ b/controllers/azurejson_machine_controller.go @@ -235,7 +235,7 @@ func (r *AzureJSONMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req } } - if azureMachine.Spec.Identity == infrav1.VMIdentityNone { + if azureMachine.Spec.Identity == infrav1.VMIdentityNone && isUsingSPCredentials(ctx, r.Client, azureCluster) { log.Info(fmt.Sprintf("WARNING, %s", spIdentityWarning)) r.Recorder.Eventf(azureMachine, corev1.EventTypeWarning, "VMIdentityNone", spIdentityWarning) } diff --git a/controllers/azurejson_machinepool_controller.go b/controllers/azurejson_machinepool_controller.go index 3cf418680ba..27d3ef24b30 100644 --- a/controllers/azurejson_machinepool_controller.go +++ b/controllers/azurejson_machinepool_controller.go @@ -177,8 +177,10 @@ func (r *AzureJSONMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl } if azureMachinePool.Spec.Identity == infrav1.VMIdentityNone { - log.Info(fmt.Sprintf("WARNING, %s", spIdentityWarning)) - r.Recorder.Eventf(azureMachinePool, corev1.EventTypeWarning, "VMIdentityNone", spIdentityWarning) + if azureCluster := getAzureClusterFromCluster(ctx, r.Client, cluster); azureCluster != nil && isUsingSPCredentials(ctx, r.Client, azureCluster) { + log.Info(fmt.Sprintf("WARNING, %s", spIdentityWarning)) + r.Recorder.Eventf(azureMachinePool, corev1.EventTypeWarning, "VMIdentityNone", spIdentityWarning) + } } newSecret, err := GetCloudProviderSecret( diff --git a/controllers/azurejson_machinetemplate_controller.go b/controllers/azurejson_machinetemplate_controller.go index 2c1e4184bec..c629f4d55a3 100644 --- a/controllers/azurejson_machinetemplate_controller.go +++ b/controllers/azurejson_machinetemplate_controller.go @@ -195,7 +195,7 @@ func (r *AzureJSONTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Re } } - if azureMachineTemplate.Spec.Template.Spec.Identity == infrav1.VMIdentityNone { + if azureMachineTemplate.Spec.Template.Spec.Identity == infrav1.VMIdentityNone && isUsingSPCredentials(ctx, r.Client, azureCluster) { log.Info(fmt.Sprintf("WARNING, %s", spIdentityWarning)) r.Recorder.Eventf(azureMachineTemplate, corev1.EventTypeWarning, "VMIdentityNone", spIdentityWarning) } diff --git a/controllers/helpers.go b/controllers/helpers.go index bfdadf3b0f8..e8001772cd0 100644 --- a/controllers/helpers.go +++ b/controllers/helpers.go @@ -1152,3 +1152,43 @@ func RemoveBlockMoveAnnotation(obj metav1.Object) { delete(azClusterAnnotations, clusterctlv1.BlockMoveAnnotation) obj.SetAnnotations(azClusterAnnotations) } + +// getAzureClusterFromCluster returns the AzureCluster for a Cluster if the infraRef is an AzureCluster. +// Returns nil if the infraRef is not an AzureCluster or if the AzureCluster cannot be fetched. +func getAzureClusterFromCluster(ctx context.Context, c client.Client, cluster *clusterv1.Cluster) *infrav1.AzureCluster { + if cluster.Spec.InfrastructureRef == nil || cluster.Spec.InfrastructureRef.Kind != infrav1.AzureClusterKind { + return nil + } + azureCluster := &infrav1.AzureCluster{} + key := client.ObjectKey{ + Namespace: cluster.Spec.InfrastructureRef.Namespace, + Name: cluster.Spec.InfrastructureRef.Name, + } + if err := c.Get(ctx, key, azureCluster); err != nil { + return nil + } + return azureCluster +} + +// isUsingSPCredentials checks if the cluster is using Service Principal credentials. +func isUsingSPCredentials(ctx context.Context, c client.Client, azureCluster *infrav1.AzureCluster) bool { + if azureCluster.Spec.IdentityRef == nil { + return false + } + identity := &infrav1.AzureClusterIdentity{} + key := client.ObjectKey{ + Name: azureCluster.Spec.IdentityRef.Name, + Namespace: azureCluster.Spec.IdentityRef.Namespace, + } + if key.Namespace == "" { + key.Namespace = azureCluster.Namespace + } + if err := c.Get(ctx, key, identity); err != nil { + return false + } + switch identity.Spec.Type { + case infrav1.ServicePrincipal, infrav1.ManualServicePrincipal, infrav1.ServicePrincipalCertificate: + return true + } + return false +} diff --git a/controllers/helpers_test.go b/controllers/helpers_test.go index 0e37e74f21c..7a7bf60652d 100644 --- a/controllers/helpers_test.go +++ b/controllers/helpers_test.go @@ -1655,3 +1655,251 @@ func TestRemoveBlockMoveAnnotation(t *testing.T) { }) } } + +func TestIsUsingSPCredentials(t *testing.T) { + tests := []struct { + name string + azureCluster *infrav1.AzureCluster + identity *infrav1.AzureClusterIdentity + expected bool + }{ + { + name: "ServicePrincipal returns true", + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: infrav1.AzureClusterSpec{ + AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ + IdentityRef: &corev1.ObjectReference{Name: "identity", Namespace: "default"}, + }, + }, + }, + identity: &infrav1.AzureClusterIdentity{ + ObjectMeta: metav1.ObjectMeta{Name: "identity", Namespace: "default"}, + Spec: infrav1.AzureClusterIdentitySpec{Type: infrav1.ServicePrincipal}, + }, + expected: true, + }, + { + name: "ManualServicePrincipal returns true", + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: infrav1.AzureClusterSpec{ + AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ + IdentityRef: &corev1.ObjectReference{Name: "identity", Namespace: "default"}, + }, + }, + }, + identity: &infrav1.AzureClusterIdentity{ + ObjectMeta: metav1.ObjectMeta{Name: "identity", Namespace: "default"}, + Spec: infrav1.AzureClusterIdentitySpec{Type: infrav1.ManualServicePrincipal}, + }, + expected: true, + }, + { + name: "ServicePrincipalCertificate returns true", + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: infrav1.AzureClusterSpec{ + AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ + IdentityRef: &corev1.ObjectReference{Name: "identity", Namespace: "default"}, + }, + }, + }, + identity: &infrav1.AzureClusterIdentity{ + ObjectMeta: metav1.ObjectMeta{Name: "identity", Namespace: "default"}, + Spec: infrav1.AzureClusterIdentitySpec{Type: infrav1.ServicePrincipalCertificate}, + }, + expected: true, + }, + { + name: "WorkloadIdentity returns false", + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: infrav1.AzureClusterSpec{ + AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ + IdentityRef: &corev1.ObjectReference{Name: "identity", Namespace: "default"}, + }, + }, + }, + identity: &infrav1.AzureClusterIdentity{ + ObjectMeta: metav1.ObjectMeta{Name: "identity", Namespace: "default"}, + Spec: infrav1.AzureClusterIdentitySpec{Type: infrav1.WorkloadIdentity}, + }, + expected: false, + }, + { + name: "UserAssignedMSI returns false", + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: infrav1.AzureClusterSpec{ + AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ + IdentityRef: &corev1.ObjectReference{Name: "identity", Namespace: "default"}, + }, + }, + }, + identity: &infrav1.AzureClusterIdentity{ + ObjectMeta: metav1.ObjectMeta{Name: "identity", Namespace: "default"}, + Spec: infrav1.AzureClusterIdentitySpec{Type: infrav1.UserAssignedMSI}, + }, + expected: false, + }, + { + name: "UserAssignedIdentityCredential returns false", + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: infrav1.AzureClusterSpec{ + AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ + IdentityRef: &corev1.ObjectReference{Name: "identity", Namespace: "default"}, + }, + }, + }, + identity: &infrav1.AzureClusterIdentity{ + ObjectMeta: metav1.ObjectMeta{Name: "identity", Namespace: "default"}, + Spec: infrav1.AzureClusterIdentitySpec{Type: infrav1.UserAssignedIdentityCredential}, + }, + expected: false, + }, + { + name: "nil IdentityRef returns false", + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: infrav1.AzureClusterSpec{}, + }, + identity: nil, + expected: false, + }, + { + name: "empty IdentityRef namespace falls back to cluster namespace", + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "my-namespace"}, + Spec: infrav1.AzureClusterSpec{ + AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ + IdentityRef: &corev1.ObjectReference{Name: "identity", Namespace: ""}, + }, + }, + }, + identity: &infrav1.AzureClusterIdentity{ + ObjectMeta: metav1.ObjectMeta{Name: "identity", Namespace: "my-namespace"}, + Spec: infrav1.AzureClusterIdentitySpec{Type: infrav1.ServicePrincipal}, + }, + expected: true, + }, + { + name: "identity not found returns false", + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: infrav1.AzureClusterSpec{ + AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ + IdentityRef: &corev1.ObjectReference{Name: "missing-identity", Namespace: "default"}, + }, + }, + }, + identity: nil, + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + scheme := setupScheme(g) + + var initObjects []runtime.Object + if tc.identity != nil { + initObjects = append(initObjects, tc.identity) + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(initObjects...).Build() + result := isUsingSPCredentials(t.Context(), fakeClient, tc.azureCluster) + g.Expect(result).To(Equal(tc.expected)) + }) + } +} + +func TestGetAzureClusterFromCluster(t *testing.T) { + tests := []struct { + name string + cluster *clusterv1.Cluster + azureCluster *infrav1.AzureCluster + expectNil bool + }{ + { + name: "returns AzureCluster when infraRef is AzureCluster", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: clusterv1.ClusterSpec{ + InfrastructureRef: &corev1.ObjectReference{ + Kind: infrav1.AzureClusterKind, + Name: "azure-cluster", + Namespace: "default", + }, + }, + }, + azureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "azure-cluster", Namespace: "default"}, + }, + expectNil: false, + }, + { + name: "returns nil when infraRef is nil", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: clusterv1.ClusterSpec{}, + }, + azureCluster: nil, + expectNil: true, + }, + { + name: "returns nil when infraRef is AzureManagedCluster", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: clusterv1.ClusterSpec{ + InfrastructureRef: &corev1.ObjectReference{ + Kind: infrav1.AzureManagedClusterKind, + Name: "azure-managed-cluster", + Namespace: "default", + }, + }, + }, + azureCluster: nil, + expectNil: true, + }, + { + name: "returns nil when AzureCluster not found", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "default"}, + Spec: clusterv1.ClusterSpec{ + InfrastructureRef: &corev1.ObjectReference{ + Kind: infrav1.AzureClusterKind, + Name: "missing-cluster", + Namespace: "default", + }, + }, + }, + azureCluster: nil, + expectNil: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + scheme := setupScheme(g) + + var initObjects []runtime.Object + if tc.azureCluster != nil { + initObjects = append(initObjects, tc.azureCluster) + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(initObjects...).Build() + result := getAzureClusterFromCluster(t.Context(), fakeClient, tc.cluster) + + if tc.expectNil { + g.Expect(result).To(BeNil()) + } else { + g.Expect(result).NotTo(BeNil()) + g.Expect(result.Name).To(Equal(tc.azureCluster.Name)) + } + }) + } +}