Disable local accounts for aks aad clusters

kubernetes-sigs · Oct 11, 2023 · 1841617 · 1841617
1 parent b9430aa
commit 1841617
Show file tree

Hide file tree

Showing 16 changed files with 1,066 additions and 74 deletions.
diff --git a/api/v1beta1/azuremanagedcontrolplane_types.go b/api/v1beta1/azuremanagedcontrolplane_types.go
@@ -218,6 +218,9 @@ type AzureManagedControlPlaneSpec struct {
 	// Immutable.
 	// +optional
 	DNSPrefix *string `json:"dnsPrefix,omitempty"`
+	// DisableLocalAccounts disables getting static credentials for this cluster when set. Expected to only be used for AAD clusters.
+	// +optional
+	DisableLocalAccounts *bool `json:"disableLocalAccounts,omitempty"`
 }
 
 // HTTPProxyConfig is the HTTP proxy configuration for the cluster.

diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook.go b/api/v1beta1/azuremanagedcontrolplane_webhook.go
@@ -222,31 +222,6 @@ func (mw *azureManagedControlPlaneWebhook) ValidateUpdate(ctx context.Context, o
 		allErrs = append(allErrs, err)
 	}
 
-	if old.Spec.AADProfile != nil {
-		if m.Spec.AADProfile == nil {
-			allErrs = append(allErrs,
-				field.Invalid(
-					field.NewPath("Spec", "AADProfile"),
-					m.Spec.AADProfile,
-					"field cannot be nil, cannot disable AADProfile"))
-		} else {
-			if !m.Spec.AADProfile.Managed && old.Spec.AADProfile.Managed {
-				allErrs = append(allErrs,
-					field.Invalid(
-						field.NewPath("Spec", "AADProfile.Managed"),
-						m.Spec.AADProfile.Managed,
-						"cannot set AADProfile.Managed to false"))
-			}
-			if len(m.Spec.AADProfile.AdminGroupObjectIDs) == 0 {
-				allErrs = append(allErrs,
-					field.Invalid(
-						field.NewPath("Spec", "AADProfile.AdminGroupObjectIDs"),
-						m.Spec.AADProfile.AdminGroupObjectIDs,
-						"length of AADProfile.AdminGroupObjectIDs cannot be zero"))
-			}
-		}
-	}
-
 	if err := webhookutils.ValidateImmutable(
 		field.NewPath("Spec", "DNSPrefix"),
 		m.Spec.DNSPrefix,
@@ -277,6 +252,10 @@ func (mw *azureManagedControlPlaneWebhook) ValidateUpdate(ctx context.Context, o
 		allErrs = append(allErrs, errs...)
 	}
 
+	if errs := m.validateAADProfileUpdateAndLocalAccounts(old); len(errs) > 0 {
+		allErrs = append(allErrs, errs...)
+	}
+
 	if errs := m.validateOIDCIssuerProfileUpdate(old); len(errs) > 0 {
 		allErrs = append(allErrs, errs...)
 	}
@@ -306,6 +285,7 @@ func (m *AzureManagedControlPlane) Validate(cli client.Client) error {
 		m.validateIdentity,
 		m.validateNetworkPluginMode,
 		m.validateDNSPrefix,
+		m.validateDisableLocalAccounts,
 	}
 
 	var errs []error
@@ -338,6 +318,14 @@ func (m *AzureManagedControlPlane) validateDNSPrefix(_ client.Client) error {
 	return kerrors.NewAggregate(allErrs.ToAggregate().Errors())
 }
 
+// validateVersion disabling local accounts for AAD based clusters.
+func (m *AzureManagedControlPlane) validateDisableLocalAccounts(_ client.Client) error {
+	if m.Spec.DisableLocalAccounts != nil && m.Spec.AADProfile == nil {
+		return errors.New("DisableLocalAccounts should be set only for AAD enabled clusters")
+	}
+	return nil
+}
+
 // validateVersion validates the Kubernetes version.
 func (m *AzureManagedControlPlane) validateVersion(_ client.Client) error {
 	if !kubeSemver.MatchString(m.Spec.Version) {
@@ -595,6 +583,58 @@ func (m *AzureManagedControlPlane) validateNetworkPluginModeUpdate(old *AzureMan
 	return allErrs
 }
 
+// validateAADProfileUpdateAndLocalAccounts validates updates for AADProfile.
+func (m *AzureManagedControlPlane) validateAADProfileUpdateAndLocalAccounts(old *AzureManagedControlPlane) field.ErrorList {
+	var allErrs field.ErrorList
+	if old.Spec.AADProfile != nil {
+		if m.Spec.AADProfile == nil {
+			allErrs = append(allErrs,
+				field.Invalid(
+					field.NewPath("Spec", "AADProfile"),
+					m.Spec.AADProfile,
+					"field cannot be nil, cannot disable AADProfile"))
+		} else {
+			if !m.Spec.AADProfile.Managed && old.Spec.AADProfile.Managed {
+				allErrs = append(allErrs,
+					field.Invalid(
+						field.NewPath("Spec", "AADProfile.Managed"),
+						m.Spec.AADProfile.Managed,
+						"cannot set AADProfile.Managed to false"))
+			}
+			if len(m.Spec.AADProfile.AdminGroupObjectIDs) == 0 {
+				allErrs = append(allErrs,
+					field.Invalid(
+						field.NewPath("Spec", "AADProfile.AdminGroupObjectIDs"),
+						m.Spec.AADProfile.AdminGroupObjectIDs,
+						"length of AADProfile.AdminGroupObjectIDs cannot be zero"))
+			}
+		}
+	}
+
+	if old.Spec.DisableLocalAccounts == nil &&
+		m.Spec.DisableLocalAccounts != nil &&
+		m.Spec.AADProfile == nil {
+		allErrs = append(allErrs,
+			field.Invalid(
+				field.NewPath("Spec", "DisableLocalAccounts"),
+				m.Spec.DisableLocalAccounts,
+				"DisableLocalAccounts can be set only for AAD enabled clusters"))
+	}
+
+	if old.Spec.DisableLocalAccounts != nil {
+		// Prevent DisableLocalAccounts modification if it was already set to some value
+		if err := webhookutils.ValidateImmutable(
+			field.NewPath("Spec", "DisableLocalAccounts"),
+			m.Spec.DisableLocalAccounts,
+			old.Spec.DisableLocalAccounts,
+		); err != nil {
+			allErrs = append(allErrs, err)
+		}
+	}
+
+	return allErrs
+}
+
 // validateOIDCIssuerProfile validates an OIDCIssuerProfile.
 func (m *AzureManagedControlPlane) validateOIDCIssuerProfileUpdate(old *AzureManagedControlPlane) field.ErrorList {
 	var allErrs field.ErrorList

diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go
@@ -922,6 +922,30 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "DisableLocalAccounts cannot be set for non AAD clusters",
+			amcp: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					Version:              "v1.21.2",
+					DisableLocalAccounts: ptr.To[bool](true),
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "DisableLocalAccounts can be set for AAD clusters",
+			amcp: &AzureManagedControlPlane{
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.21.2",
+					AADProfile: &AADProfile{
+						Managed:             true,
+						AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"},
+					},
+					DisableLocalAccounts: ptr.To[bool](true),
+				},
+			},
+			wantErr: false,
+		},
 	}
 	client := mockClient{ReturnError: false}
 	for _, tc := range tests {
@@ -1715,6 +1739,94 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "DisableLocalAccounts can be set only for AAD enabled clusters",
+			oldAMCP: &AzureManagedControlPlane{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.18.0",
+					AADProfile: &AADProfile{
+						Managed:             true,
+						AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"},
+					},
+				},
+			},
+			amcp: &AzureManagedControlPlane{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+				Spec: AzureManagedControlPlaneSpec{
+					Version:              "v1.18.0",
+					DisableLocalAccounts: ptr.To[bool](true),
+					AADProfile: &AADProfile{
+						Managed:             true,
+						AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "DisableLocalAccounts cannot be disabled",
+			oldAMCP: &AzureManagedControlPlane{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.18.0",
+					AADProfile: &AADProfile{
+						Managed:             true,
+						AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"},
+					},
+					DisableLocalAccounts: ptr.To[bool](true),
+				},
+			},
+			amcp: &AzureManagedControlPlane{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.18.0",
+					AADProfile: &AADProfile{
+						Managed:             true,
+						AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "DisableLocalAccounts cannot be disabled",
+			oldAMCP: &AzureManagedControlPlane{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.18.0",
+					AADProfile: &AADProfile{
+						Managed:             true,
+						AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"},
+					},
+					DisableLocalAccounts: ptr.To[bool](true),
+				},
+			},
+			amcp: &AzureManagedControlPlane{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+				Spec: AzureManagedControlPlaneSpec{
+					Version:              "v1.18.0",
+					DisableLocalAccounts: ptr.To[bool](false),
+					AADProfile: &AADProfile{
+						Managed:             true,
+						AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"},
+					},
+				},
+			},
+			wantErr: true,
+		},
 		{
 			name: "AzureManagedControlPlane DNSPrefix is immutable error nil -> capz-aks",
 			oldAMCP: &AzureManagedControlPlane{
@@ -1731,6 +1843,27 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "DisableLocalAccounts cannot be set for non AAD clusters",
+			oldAMCP: &AzureManagedControlPlane{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+				Spec: AzureManagedControlPlaneSpec{
+					Version: "v1.18.0",
+				},
+			},
+			amcp: &AzureManagedControlPlane{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+				Spec: AzureManagedControlPlaneSpec{
+					Version:              "v1.18.0",
+					DisableLocalAccounts: ptr.To[bool](true),
+				},
+			},
+			wantErr: true,
+		},
 		{
 			name: "AzureManagedControlPlane DNSPrefix is immutable error nil -> empty",
 			oldAMCP: &AzureManagedControlPlane{

diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go
@@ -116,10 +116,11 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
 
 // ManagedControlPlaneScope defines the basic context for an actuator to operate upon.
 type ManagedControlPlaneScope struct {
-	Client         client.Client
-	patchHelper    *patch.Helper
-	kubeConfigData []byte
-	cache          *ManagedControlPlaneCache
+	Client              client.Client
+	patchHelper         *patch.Helper
+	adminKubeConfigData []byte
+	userKubeConfigData  []byte
+	cache               *ManagedControlPlaneCache
 
 	AzureClients
 	Cluster             *clusterv1.Cluster
@@ -459,6 +460,24 @@ func (s *ManagedControlPlaneScope) ManagedClusterAnnotations() map[string]string
 	return s.ControlPlane.Annotations
 }
 
+// AreLocalAccountsDisabled checks if local accounts are disabled for aad enabled managed clusters.
+func (s *ManagedControlPlaneScope) AreLocalAccountsDisabled() bool {
+	if s.IsAADEnabled() &&
+		s.ControlPlane.Spec.DisableLocalAccounts != nil &&
+		*s.ControlPlane.Spec.DisableLocalAccounts {
+		return true
+	}
+	return false
+}
+
+// IsAADEnabled checks if azure active directory is enabled for managed clusters.
+func (s *ManagedControlPlaneScope) IsAADEnabled() bool {
+	if s.ControlPlane.Spec.AADProfile != nil && s.ControlPlane.Spec.AADProfile.Managed {
+		return true
+	}
+	return false
+}
+
 // ManagedClusterSpec returns the managed cluster spec.
 func (s *ManagedControlPlaneScope) ManagedClusterSpec() azure.ResourceSpecGetter {
 	managedClusterSpec := managedclusters.ManagedClusterSpec{
@@ -513,6 +532,9 @@ func (s *ManagedControlPlaneScope) ManagedClusterSpec() azure.ResourceSpecGetter
 			EnableAzureRBAC:     s.ControlPlane.Spec.AADProfile.Managed,
 			AdminGroupObjectIDs: s.ControlPlane.Spec.AADProfile.AdminGroupObjectIDs,
 		}
+		if s.ControlPlane.Spec.DisableLocalAccounts != nil {
+			managedClusterSpec.DisableLocalAccounts = s.ControlPlane.Spec.DisableLocalAccounts
+		}
 	}
 
 	if s.ControlPlane.Spec.AddonProfiles != nil {
@@ -640,14 +662,24 @@ func (s *ManagedControlPlaneScope) MakeEmptyKubeConfigSecret() corev1.Secret {
 	}
 }
 
-// GetKubeConfigData returns a []byte that contains kubeconfig.
-func (s *ManagedControlPlaneScope) GetKubeConfigData() []byte {
-	return s.kubeConfigData
+// GetAdminKubeconfigData returns admin kubeconfig.
+func (s *ManagedControlPlaneScope) GetAdminKubeconfigData() []byte {
+	return s.adminKubeConfigData
+}
+
+// SetAdminKubeconfigData sets admin kubeconfig data.
+func (s *ManagedControlPlaneScope) SetAdminKubeconfigData(kubeConfigData []byte) {
+	s.adminKubeConfigData = kubeConfigData
+}
+
+// GetUserKubeconfigData returns user kubeconfig, required when using AAD with AKS cluster.
+func (s *ManagedControlPlaneScope) GetUserKubeconfigData() []byte {
+	return s.userKubeConfigData
 }
 
-// SetKubeConfigData sets kubeconfig data.
-func (s *ManagedControlPlaneScope) SetKubeConfigData(kubeConfigData []byte) {
-	s.kubeConfigData = kubeConfigData
+// SetUserKubeconfigData sets userKubeconfig data.
+func (s *ManagedControlPlaneScope) SetUserKubeconfigData(kubeConfigData []byte) {
+	s.userKubeConfigData = kubeConfigData
 }
 
 // SetKubeletIdentity sets the ID of the user-assigned identity for kubelet if not already set.