Skip to content

Commit

Permalink
Merge pull request #4008 from spectrocloud/support-for-disabling-loca…
Browse files Browse the repository at this point in the history 
…l-accounts

Disable local accounts for aks aad clusters
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 11, 2023
2 parents 1a70b01 + 1841617 commit 7fc4734
Show file tree
Hide file tree
Showing 16 changed files with 1,066 additions and 74 deletions.
3 changes: 3 additions & 0 deletions api/v1beta1/azuremanagedcontrolplane_types.go
View file
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"`
}

// HTTPProxyConfig is the HTTP proxy configuration for the cluster.
Expand Down
90 changes: 65 additions & 25 deletions api/v1beta1/azuremanagedcontrolplane_webhook.go
View file
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 @@ -597,6 +585,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
Expand Down
133 changes: 133 additions & 0 deletions api/v1beta1/azuremanagedcontrolplane_webhook_test.go
View file
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 @@ -1739,6 +1763,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 @@ -1755,6 +1867,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
View file

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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 7fc4734

Please sign in to comment.