Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable local accounts for aks aad clusters #4008

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/v1beta1/azuremanagedcontrolplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
}

// HTTPProxyConfig is the HTTP proxy configuration for the cluster.
Expand Down
90 changes: 65 additions & 25 deletions api/v1beta1/azuremanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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...)
}
Expand Down Expand Up @@ -306,6 +285,7 @@ func (m *AzureManagedControlPlane) Validate(cli client.Client) error {
m.validateIdentity,
m.validateNetworkPluginMode,
m.validateDNSPrefix,
m.validateDisableLocalAccounts,
}

var errs []error
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
LochanRn marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
133 changes: 133 additions & 0 deletions api/v1beta1/azuremanagedcontrolplane_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 42 additions & 10 deletions azure/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,11 @@

// 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
Expand Down Expand Up @@ -459,6 +460,24 @@
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{
Expand Down Expand Up @@ -513,6 +532,9 @@
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 {
Expand Down Expand Up @@ -640,14 +662,24 @@
}
}

// 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

Check warning on line 667 in azure/scope/managedcontrolplane.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/managedcontrolplane.go#L666-L667

Added lines #L666 - L667 were not covered by tests
}

// SetAdminKubeconfigData sets admin kubeconfig data.
func (s *ManagedControlPlaneScope) SetAdminKubeconfigData(kubeConfigData []byte) {
s.adminKubeConfigData = kubeConfigData

Check warning on line 672 in azure/scope/managedcontrolplane.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/managedcontrolplane.go#L671-L672

Added lines #L671 - L672 were not covered by tests
}

// GetUserKubeconfigData returns user kubeconfig, required when using AAD with AKS cluster.
func (s *ManagedControlPlaneScope) GetUserKubeconfigData() []byte {
return s.userKubeConfigData

Check warning on line 677 in azure/scope/managedcontrolplane.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/managedcontrolplane.go#L676-L677

Added lines #L676 - L677 were not covered by tests
}

// 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

Check warning on line 682 in azure/scope/managedcontrolplane.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/managedcontrolplane.go#L681-L682

Added lines #L681 - L682 were not covered by tests
}

// SetKubeletIdentity sets the ID of the user-assigned identity for kubelet if not already set.
Expand Down
Loading