From 18416174195f4ebf538d94990c1b2514a216f46c Mon Sep 17 00:00:00 2001 From: LochanRn Date: Tue, 19 Sep 2023 02:00:31 +0530 Subject: [PATCH] Disable local accounts for aks aad clusters --- api/v1beta1/azuremanagedcontrolplane_types.go | 3 + .../azuremanagedcontrolplane_webhook.go | 90 +++- .../azuremanagedcontrolplane_webhook_test.go | 133 ++++++ api/v1beta1/zz_generated.deepcopy.go | 5 + azure/scope/managedcontrolplane.go | 52 ++- azure/scope/managedcontrolplane_test.go | 428 ++++++++++++++++++ azure/services/managedclusters/client.go | 18 + .../managedclusters/managedclusters.go | 99 +++- .../managedclusters/managedclusters_test.go | 56 ++- .../mock_managedclusters/client_mock.go | 15 + .../managedclusters_mock.go | 90 +++- azure/services/managedclusters/spec.go | 13 + azure/services/token/client.go | 74 +++ ...er.x-k8s.io_azuremanagedcontrolplanes.yaml | 4 + .../azuremanagedcontrolplane_reconciler.go | 30 +- docs/book/src/topics/managedcluster.md | 30 ++ 16 files changed, 1066 insertions(+), 74 deletions(-) create mode 100644 azure/services/token/client.go diff --git a/api/v1beta1/azuremanagedcontrolplane_types.go b/api/v1beta1/azuremanagedcontrolplane_types.go index 40004006440..545deffdb8a 100644 --- 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 index 2dbdbf904d0..d9131b104ba 100644 --- 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 index 9aee6064d1c..719b50002f5 100644 --- 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 index 6e1797cc41d..6e83abe36b1 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1252,6 +1252,11 @@ func (in *AzureManagedControlPlaneSpec) DeepCopyInto(out *AzureManagedControlPla *out = new(string) **out = **in } + if in.DisableLocalAccounts != nil { + in, out := &in.DisableLocalAccounts, &out.DisableLocalAccounts + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureManagedControlPlaneSpec. diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 41b4104acb0..c52a4c5c9e9 100644 --- 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. diff --git a/azure/scope/managedcontrolplane_test.go b/azure/scope/managedcontrolplane_test.go index 9b477d48fa5..48f5668a3b6 100644 --- a/azure/scope/managedcontrolplane_test.go +++ b/azure/scope/managedcontrolplane_test.go @@ -610,3 +610,431 @@ func TestManagedControlPlaneScope_IsVnetManagedCache(t *testing.T) { }) } } + +func TestManagedControlPlaneScope_AADProfile(t *testing.T) { + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + + cases := []struct { + Name string + Input ManagedControlPlaneScopeParams + Expected *managedclusters.AADProfile + }{ + { + Name: "Without AADProfile", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: nil, + }, + { + Name: "With AADProfile", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + AADProfile: &infrav1.AADProfile{ + Managed: true, + AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"}, + }, + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: &managedclusters.AADProfile{ + Managed: true, + EnableAzureRBAC: true, + AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"}, + }, + }, + } + for _, c := range cases { + c := c + t.Run(c.Name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(c.Input.ControlPlane).Build() + c.Input.Client = fakeClient + s, err := NewManagedControlPlaneScope(context.TODO(), c.Input) + g.Expect(err).To(Succeed()) + managedClusterGetter := s.ManagedClusterSpec() + managedCluster, ok := managedClusterGetter.(*managedclusters.ManagedClusterSpec) + g.Expect(ok).To(BeTrue()) + g.Expect(managedCluster.AADProfile).To(Equal(c.Expected)) + }) + } +} + +func TestManagedControlPlaneScope_DisableLocalAccounts(t *testing.T) { + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + + cases := []struct { + Name string + Input ManagedControlPlaneScopeParams + Expected *bool + }{ + { + Name: "Without DisableLocalAccounts", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: nil, + }, + { + Name: "Without AAdProfile and With DisableLocalAccounts", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + DisableLocalAccounts: ptr.To[bool](true), + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: nil, + }, + { + Name: "With AAdProfile and With DisableLocalAccounts", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + AADProfile: &infrav1.AADProfile{ + Managed: true, + AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"}, + }, + DisableLocalAccounts: ptr.To[bool](true), + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: ptr.To[bool](true), + }, + } + for _, c := range cases { + c := c + t.Run(c.Name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(c.Input.ControlPlane).Build() + c.Input.Client = fakeClient + s, err := NewManagedControlPlaneScope(context.TODO(), c.Input) + g.Expect(err).To(Succeed()) + managedClusterGetter := s.ManagedClusterSpec() + managedCluster, ok := managedClusterGetter.(*managedclusters.ManagedClusterSpec) + g.Expect(ok).To(BeTrue()) + g.Expect(managedCluster.DisableLocalAccounts).To(Equal(c.Expected)) + }) + } +} + +func TestIsAADEnabled(t *testing.T) { + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + + cases := []struct { + Name string + Input ManagedControlPlaneScopeParams + Expected bool + }{ + { + Name: "AAD is not enabled", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: false, + }, + { + Name: "AAdProfile and With DisableLocalAccounts", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + AADProfile: &infrav1.AADProfile{ + Managed: true, + AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"}, + }, + DisableLocalAccounts: ptr.To[bool](true), + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: true, + }, + } + for _, c := range cases { + c := c + t.Run(c.Name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(c.Input.ControlPlane).Build() + c.Input.Client = fakeClient + s, err := NewManagedControlPlaneScope(context.TODO(), c.Input) + g.Expect(err).To(Succeed()) + aadEnabled := s.IsAADEnabled() + g.Expect(aadEnabled).To(Equal(c.Expected)) + }) + } +} + +func TestAreLocalAccountsDisabled(t *testing.T) { + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + + cases := []struct { + Name string + Input ManagedControlPlaneScopeParams + Expected bool + }{ + { + Name: "DisbaleLocalAccount is not enabled", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: false, + }, + { + Name: "With AAdProfile and Without DisableLocalAccounts", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + AADProfile: &infrav1.AADProfile{ + Managed: true, + AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"}, + }, + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: false, + }, + { + Name: "With AAdProfile and With DisableLocalAccounts", + Input: ManagedControlPlaneScopeParams{ + AzureClients: AzureClients{ + Authorizer: autorest.NullAuthorizer{}, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + AADProfile: &infrav1.AADProfile{ + Managed: true, + AdminGroupObjectIDs: []string{"00000000-0000-0000-0000-000000000000"}, + }, + DisableLocalAccounts: ptr.To[bool](true), + }, + }, + ManagedMachinePools: []ManagedMachinePool{ + { + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + }, + Expected: true, + }, + } + for _, c := range cases { + c := c + t.Run(c.Name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(c.Input.ControlPlane).Build() + c.Input.Client = fakeClient + s, err := NewManagedControlPlaneScope(context.TODO(), c.Input) + g.Expect(err).To(Succeed()) + localAccountsDisabled := s.AreLocalAccountsDisabled() + g.Expect(localAccountsDisabled).To(Equal(c.Expected)) + }) + } +} diff --git a/azure/services/managedclusters/client.go b/azure/services/managedclusters/client.go index e64975c63cd..9f7757331c4 100644 --- a/azure/services/managedclusters/client.go +++ b/azure/services/managedclusters/client.go @@ -31,6 +31,7 @@ import ( // CredentialGetter is a helper interface for getting managed cluster credentials. type CredentialGetter interface { GetCredentials(context.Context, string, string) ([]byte, error) + GetUserCredentials(context.Context, string, string) ([]byte, error) } // azureClient contains the Azure go-sdk Client. @@ -84,6 +85,23 @@ func (ac *azureClient) GetCredentials(ctx context.Context, resourceGroupName, na return credentialList.Kubeconfigs[0].Value, nil } +// GetUserCredentials fetches the user kubeconfig for a managed cluster. +func (ac *azureClient) GetUserCredentials(ctx context.Context, resourceGroupName, name string) ([]byte, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "managedclusters.azureClient.GetCredentials") + defer done() + + credentialList, err := ac.managedclusters.ListClusterUserCredentials(ctx, resourceGroupName, name, nil) + if err != nil { + return nil, err + } + + if len(credentialList.Kubeconfigs) == 0 { + return nil, errors.New("no user kubeconfigs available for the managed cluster") + } + + return credentialList.Kubeconfigs[0].Value, nil +} + // CreateOrUpdateAsync creates or updates a managed cluster. // It sends a PUT request to Azure and if accepted without error, the func will return a Poller which can be used to track the ongoing // progress of the operation. diff --git a/azure/services/managedclusters/managedclusters.go b/azure/services/managedclusters/managedclusters.go index 3b3fd23cd36..c66c8989a4e 100644 --- a/azure/services/managedclusters/managedclusters.go +++ b/azure/services/managedclusters/managedclusters.go @@ -22,18 +22,25 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/clientcmd" "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/token" "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) -const serviceName = "managedcluster" +const ( + serviceName = "managedcluster" + kubeletIdentityKey = "kubeletidentity" -const kubeletIdentityKey = "kubeletidentity" + // The aadResourceID is the application-id used by the server side. The access token accessing AKS clusters need to be issued for this app. + // Refer: https://azure.github.io/kubelogin/concepts/aks.html?highlight=6dae42f8-4368-4678-94ff-3960e28e3630#azure-kubernetes-service-aad-server + aadResourceID = "6dae42f8-4368-4678-94ff-3960e28e3630" +) // ManagedClusterScope defines the scope interface for a managed cluster. type ManagedClusterScope interface { @@ -43,8 +50,12 @@ type ManagedClusterScope interface { SetControlPlaneEndpoint(clusterv1.APIEndpoint) SetKubeletIdentity(string) MakeEmptyKubeConfigSecret() corev1.Secret - GetKubeConfigData() []byte - SetKubeConfigData([]byte) + GetAdminKubeconfigData() []byte + SetAdminKubeconfigData([]byte) + GetUserKubeconfigData() []byte + SetUserKubeconfigData([]byte) + IsAADEnabled() bool + AreLocalAccountsDisabled() bool SetOIDCIssuerProfileStatus(*infrav1.OIDCIssuerProfileStatus) } @@ -102,11 +113,13 @@ func (s *Service) Reconcile(ctx context.Context) error { // Update kubeconfig data // Always fetch credentials in case of rotation - kubeConfigData, err := s.GetCredentials(ctx, managedClusterSpec.ResourceGroupName(), managedClusterSpec.ResourceName()) + adminKubeConfigData, userKubeConfigData, err := s.ReconcileKubeconfig(ctx, managedClusterSpec) if err != nil { - return errors.Wrap(err, "failed to get credentials for managed cluster") + return errors.Wrap(err, "error while reconciling adminKubeConfigData") } - s.Scope.SetKubeConfigData(kubeConfigData) + + s.Scope.SetAdminKubeconfigData(adminKubeConfigData) + s.Scope.SetUserKubeconfigData(userKubeConfigData) // This field gets populated by AKS when not set by the user. Persist AKS's value so for future diffs, // the "before" reflects the correct value. @@ -147,3 +160,75 @@ func (s *Service) Delete(ctx context.Context) error { func (s *Service) IsManaged(ctx context.Context) (bool, error) { return true, nil } + +// ReconcileKubeconfig will reconcile admin kubeconfig and user kubeconfig. +/* + Returns the admin kubeconfig and user kubeconfig + If aad is enabled a user kubeconfig will also get generated and stored in the secret -kubeconfig-user + If we disable local accounts for aad clusters we do not have access to admin kubeconfig, hence we need to create + the admin kubeconfig by authenticating with the user credentials and retrieving the token for kubeconfig. + The token is used to create the admin kubeconfig. + The user needs to ensure to provide service principle with admin aad privileges. +*/ +func (s *Service) ReconcileKubeconfig(ctx context.Context, managedClusterSpec azure.ResourceSpecGetter) (userKubeConfigData []byte, adminKubeConfigData []byte, err error) { + if s.Scope.IsAADEnabled() { + if userKubeConfigData, err = s.GetUserKubeconfigData(ctx, managedClusterSpec); err != nil { + return nil, nil, errors.Wrap(err, "error while trying to get user kubeconfig") + } + } + + if s.Scope.AreLocalAccountsDisabled() { + userKubeconfigWithToken, err := s.GetUserKubeConfigWithToken(userKubeConfigData, ctx, managedClusterSpec) + if err != nil { + return nil, nil, errors.Wrap(err, "error while trying to get user kubeconfig with token") + } + return userKubeconfigWithToken, userKubeConfigData, nil + } + + adminKubeConfigData, err = s.GetCredentials(ctx, managedClusterSpec.ResourceGroupName(), managedClusterSpec.ResourceName()) + if err != nil { + return nil, nil, errors.Wrap(err, "failed to get credentials for managed cluster") + } + return adminKubeConfigData, userKubeConfigData, nil +} + +// GetUserKubeconfigData gets user kubeconfig when aad is enabled for the aad clusters. +func (s *Service) GetUserKubeconfigData(ctx context.Context, managedClusterSpec azure.ResourceSpecGetter) ([]byte, error) { + kubeConfigData, err := s.GetUserCredentials(ctx, managedClusterSpec.ResourceGroupName(), managedClusterSpec.ResourceName()) + if err != nil { + return nil, errors.Wrap(err, "failed to get credentials for managed cluster") + } + return kubeConfigData, nil +} + +// GetUserKubeConfigWithToken returns the kubeconfig with user token, for capz to create the target cluster. +func (s *Service) GetUserKubeConfigWithToken(userKubeConfigData []byte, ctx context.Context, managedClusterSpec azure.ResourceSpecGetter) ([]byte, error) { + tokenClient, err := token.NewClient(s.Scope) + if err != nil { + return nil, errors.Wrap(err, "error while getting aad token client") + } + + token, err := tokenClient.GetAzureActiveDirectoryToken(ctx, aadResourceID) + if err != nil { + return nil, errors.Wrap(err, "error while getting aad token for user kubeconfig") + } + + return s.CreateUserKubeconfigWithToken(token, userKubeConfigData) +} + +// CreateUserKubeconfigWithToken gets the kubeconfigdata for authenticating with target cluster. +func (s *Service) CreateUserKubeconfigWithToken(token string, userKubeConfigData []byte) ([]byte, error) { + config, err := clientcmd.Load(userKubeConfigData) + if err != nil { + return nil, errors.Wrap(err, "error while trying to unmarshal new user kubeconfig with token") + } + for _, auth := range config.AuthInfos { + auth.Token = token + auth.Exec = nil + } + kubeconfig, err := clientcmd.Write(*config) + if err != nil { + return nil, errors.Wrap(err, "error while trying to marshal new user kubeconfig with token") + } + return kubeconfig, nil +} diff --git a/azure/services/managedclusters/managedclusters_test.go b/azure/services/managedclusters/managedclusters_test.go index ef0a592b6e4..25eb36badfb 100644 --- a/azure/services/managedclusters/managedclusters_test.go +++ b/azure/services/managedclusters/managedclusters_test.go @@ -33,6 +33,14 @@ import ( ) var fakeManagedClusterSpec = &ManagedClusterSpec{Name: "my-managedcluster", ResourceGroup: "my-rg"} +var fakeManagedClusterSpecWithAAD = &ManagedClusterSpec{ + Name: "my-managedcluster", + ResourceGroup: "my-rg", + AADProfile: &AADProfile{ + Managed: true, + AdminGroupObjectIDs: []string{"000000-000000-000000-000000"}, + }, +} func TestReconcile(t *testing.T) { testcases := []struct { @@ -60,6 +68,7 @@ func TestReconcile(t *testing.T) { name: "create managed cluster succeeds", expectedError: "", expect: func(m *mock_managedclusters.MockCredentialGetterMockRecorder, s *mock_managedclusters.MockManagedClusterScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + var userKubeConfigData []byte s.ManagedClusterSpec().Return(fakeManagedClusterSpec) r.CreateOrUpdateResource(gomockinternal.AContext(), fakeManagedClusterSpec, serviceName).Return(armcontainerservice.ManagedCluster{ Properties: &armcontainerservice.ManagedClusterProperties{ @@ -80,8 +89,49 @@ func TestReconcile(t *testing.T) { Host: "my-managedcluster-fqdn", Port: 443, }) + s.IsAADEnabled().Return(false) + s.AreLocalAccountsDisabled().Return(false) + m.GetCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return([]byte("credentials"), nil) + s.SetAdminKubeconfigData([]byte("credentials")) + s.SetUserKubeconfigData(userKubeConfigData) + s.SetKubeletIdentity("kubelet-id") + s.SetOIDCIssuerProfileStatus(nil) + s.SetOIDCIssuerProfileStatus(&infrav1.OIDCIssuerProfileStatus{ + IssuerURL: ptr.To("oidc issuer url"), + }) + s.UpdatePutStatus(infrav1.ManagedClusterRunningCondition, serviceName, nil) + }, + }, + { + name: "create managed cluster succeeds with user kubeconfig", + expectedError: "", + expect: func(m *mock_managedclusters.MockCredentialGetterMockRecorder, s *mock_managedclusters.MockManagedClusterScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.ManagedClusterSpec().Return(fakeManagedClusterSpecWithAAD) + r.CreateOrUpdateResource(gomockinternal.AContext(), fakeManagedClusterSpecWithAAD, serviceName).Return(armcontainerservice.ManagedCluster{ + Properties: &armcontainerservice.ManagedClusterProperties{ + Fqdn: ptr.To("my-managedcluster-fqdn"), + ProvisioningState: ptr.To("Succeeded"), + IdentityProfile: map[string]*armcontainerservice.UserAssignedIdentity{ + kubeletIdentityKey: { + ResourceID: ptr.To("kubelet-id"), + }, + }, + OidcIssuerProfile: &armcontainerservice.ManagedClusterOIDCIssuerProfile{ + Enabled: ptr.To(true), + IssuerURL: ptr.To("oidc issuer url"), + }, + }, + }, nil) + s.SetControlPlaneEndpoint(clusterv1.APIEndpoint{ + Host: "my-managedcluster-fqdn", + Port: 443, + }) + s.IsAADEnabled().Return(true) + s.AreLocalAccountsDisabled().Return(false) m.GetCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return([]byte("credentials"), nil) - s.SetKubeConfigData([]byte("credentials")) + m.GetUserCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return([]byte("credentials-user"), nil) + s.SetAdminKubeconfigData([]byte("credentials")) + s.SetUserKubeconfigData([]byte("credentials-user")) s.SetKubeletIdentity("kubelet-id") s.SetOIDCIssuerProfileStatus(nil) s.SetOIDCIssuerProfileStatus(&infrav1.OIDCIssuerProfileStatus{ @@ -92,7 +142,7 @@ func TestReconcile(t *testing.T) { }, { name: "fail to get managed cluster credentials", - expectedError: "failed to get credentials for managed cluster: internal server error", + expectedError: "error while reconciling adminKubeConfigData: failed to get credentials for managed cluster: internal server error", expect: func(m *mock_managedclusters.MockCredentialGetterMockRecorder, s *mock_managedclusters.MockManagedClusterScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.ManagedClusterSpec().Return(fakeManagedClusterSpec) r.CreateOrUpdateResource(gomockinternal.AContext(), fakeManagedClusterSpec, serviceName).Return(armcontainerservice.ManagedCluster{ @@ -105,6 +155,8 @@ func TestReconcile(t *testing.T) { Host: "my-managedcluster-fqdn", Port: 443, }) + s.IsAADEnabled().Return(false) + s.AreLocalAccountsDisabled().Return(false) m.GetCredentials(gomockinternal.AContext(), "my-rg", "my-managedcluster").Return([]byte(""), errors.New("internal server error")) }, }, diff --git a/azure/services/managedclusters/mock_managedclusters/client_mock.go b/azure/services/managedclusters/mock_managedclusters/client_mock.go index 2f6c2a4b99c..1a12aa326dc 100644 --- a/azure/services/managedclusters/mock_managedclusters/client_mock.go +++ b/azure/services/managedclusters/mock_managedclusters/client_mock.go @@ -68,3 +68,18 @@ func (mr *MockCredentialGetterMockRecorder) GetCredentials(arg0, arg1, arg2 any) mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCredentials", reflect.TypeOf((*MockCredentialGetter)(nil).GetCredentials), arg0, arg1, arg2) } + +// GetUserCredentials mocks base method. +func (m *MockCredentialGetter) GetUserCredentials(arg0 context.Context, arg1, arg2 string) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserCredentials", arg0, arg1, arg2) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserCredentials indicates an expected call of GetUserCredentials. +func (mr *MockCredentialGetterMockRecorder) GetUserCredentials(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserCredentials", reflect.TypeOf((*MockCredentialGetter)(nil).GetUserCredentials), arg0, arg1, arg2) +} diff --git a/azure/services/managedclusters/mock_managedclusters/managedclusters_mock.go b/azure/services/managedclusters/mock_managedclusters/managedclusters_mock.go index 22629be8706..671e2b18aad 100644 --- a/azure/services/managedclusters/mock_managedclusters/managedclusters_mock.go +++ b/azure/services/managedclusters/mock_managedclusters/managedclusters_mock.go @@ -58,6 +58,20 @@ func (m *MockManagedClusterScope) EXPECT() *MockManagedClusterScopeMockRecorder return m.recorder } +// AreLocalAccountsDisabled mocks base method. +func (m *MockManagedClusterScope) AreLocalAccountsDisabled() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AreLocalAccountsDisabled") + ret0, _ := ret[0].(bool) + return ret0 +} + +// AreLocalAccountsDisabled indicates an expected call of AreLocalAccountsDisabled. +func (mr *MockManagedClusterScopeMockRecorder) AreLocalAccountsDisabled() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AreLocalAccountsDisabled", reflect.TypeOf((*MockManagedClusterScope)(nil).AreLocalAccountsDisabled)) +} + // BaseURI mocks base method. func (m *MockManagedClusterScope) BaseURI() string { m.ctrl.T.Helper() @@ -126,18 +140,18 @@ func (mr *MockManagedClusterScopeMockRecorder) DeleteLongRunningOperationState(a return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockManagedClusterScope)(nil).DeleteLongRunningOperationState), arg0, arg1, arg2) } -// GetKubeConfigData mocks base method. -func (m *MockManagedClusterScope) GetKubeConfigData() []byte { +// GetAdminKubeconfigData mocks base method. +func (m *MockManagedClusterScope) GetAdminKubeconfigData() []byte { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetKubeConfigData") + ret := m.ctrl.Call(m, "GetAdminKubeconfigData") ret0, _ := ret[0].([]byte) return ret0 } -// GetKubeConfigData indicates an expected call of GetKubeConfigData. -func (mr *MockManagedClusterScopeMockRecorder) GetKubeConfigData() *gomock.Call { +// GetAdminKubeconfigData indicates an expected call of GetAdminKubeconfigData. +func (mr *MockManagedClusterScopeMockRecorder) GetAdminKubeconfigData() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKubeConfigData", reflect.TypeOf((*MockManagedClusterScope)(nil).GetKubeConfigData)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAdminKubeconfigData", reflect.TypeOf((*MockManagedClusterScope)(nil).GetAdminKubeconfigData)) } // GetLongRunningOperationState mocks base method. @@ -154,6 +168,20 @@ func (mr *MockManagedClusterScopeMockRecorder) GetLongRunningOperationState(arg0 return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockManagedClusterScope)(nil).GetLongRunningOperationState), arg0, arg1, arg2) } +// GetUserKubeconfigData mocks base method. +func (m *MockManagedClusterScope) GetUserKubeconfigData() []byte { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserKubeconfigData") + ret0, _ := ret[0].([]byte) + return ret0 +} + +// GetUserKubeconfigData indicates an expected call of GetUserKubeconfigData. +func (mr *MockManagedClusterScopeMockRecorder) GetUserKubeconfigData() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserKubeconfigData", reflect.TypeOf((*MockManagedClusterScope)(nil).GetUserKubeconfigData)) +} + // HashKey mocks base method. func (m *MockManagedClusterScope) HashKey() string { m.ctrl.T.Helper() @@ -168,6 +196,20 @@ func (mr *MockManagedClusterScopeMockRecorder) HashKey() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HashKey", reflect.TypeOf((*MockManagedClusterScope)(nil).HashKey)) } +// IsAADEnabled mocks base method. +func (m *MockManagedClusterScope) IsAADEnabled() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsAADEnabled") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsAADEnabled indicates an expected call of IsAADEnabled. +func (mr *MockManagedClusterScopeMockRecorder) IsAADEnabled() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsAADEnabled", reflect.TypeOf((*MockManagedClusterScope)(nil).IsAADEnabled)) +} + // MakeEmptyKubeConfigSecret mocks base method. func (m *MockManagedClusterScope) MakeEmptyKubeConfigSecret() v1.Secret { m.ctrl.T.Helper() @@ -196,28 +238,28 @@ func (mr *MockManagedClusterScopeMockRecorder) ManagedClusterSpec() *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ManagedClusterSpec", reflect.TypeOf((*MockManagedClusterScope)(nil).ManagedClusterSpec)) } -// SetControlPlaneEndpoint mocks base method. -func (m *MockManagedClusterScope) SetControlPlaneEndpoint(arg0 v1beta10.APIEndpoint) { +// SetAdminKubeconfigData mocks base method. +func (m *MockManagedClusterScope) SetAdminKubeconfigData(arg0 []byte) { m.ctrl.T.Helper() - m.ctrl.Call(m, "SetControlPlaneEndpoint", arg0) + m.ctrl.Call(m, "SetAdminKubeconfigData", arg0) } -// SetControlPlaneEndpoint indicates an expected call of SetControlPlaneEndpoint. -func (mr *MockManagedClusterScopeMockRecorder) SetControlPlaneEndpoint(arg0 any) *gomock.Call { +// SetAdminKubeconfigData indicates an expected call of SetAdminKubeconfigData. +func (mr *MockManagedClusterScopeMockRecorder) SetAdminKubeconfigData(arg0 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetControlPlaneEndpoint", reflect.TypeOf((*MockManagedClusterScope)(nil).SetControlPlaneEndpoint), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAdminKubeconfigData", reflect.TypeOf((*MockManagedClusterScope)(nil).SetAdminKubeconfigData), arg0) } -// SetKubeConfigData mocks base method. -func (m *MockManagedClusterScope) SetKubeConfigData(arg0 []byte) { +// SetControlPlaneEndpoint mocks base method. +func (m *MockManagedClusterScope) SetControlPlaneEndpoint(arg0 v1beta10.APIEndpoint) { m.ctrl.T.Helper() - m.ctrl.Call(m, "SetKubeConfigData", arg0) + m.ctrl.Call(m, "SetControlPlaneEndpoint", arg0) } -// SetKubeConfigData indicates an expected call of SetKubeConfigData. -func (mr *MockManagedClusterScopeMockRecorder) SetKubeConfigData(arg0 any) *gomock.Call { +// SetControlPlaneEndpoint indicates an expected call of SetControlPlaneEndpoint. +func (mr *MockManagedClusterScopeMockRecorder) SetControlPlaneEndpoint(arg0 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetKubeConfigData", reflect.TypeOf((*MockManagedClusterScope)(nil).SetKubeConfigData), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetControlPlaneEndpoint", reflect.TypeOf((*MockManagedClusterScope)(nil).SetControlPlaneEndpoint), arg0) } // SetKubeletIdentity mocks base method. @@ -256,6 +298,18 @@ func (mr *MockManagedClusterScopeMockRecorder) SetOIDCIssuerProfileStatus(arg0 a return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetOIDCIssuerProfileStatus", reflect.TypeOf((*MockManagedClusterScope)(nil).SetOIDCIssuerProfileStatus), arg0) } +// SetUserKubeconfigData mocks base method. +func (m *MockManagedClusterScope) SetUserKubeconfigData(arg0 []byte) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetUserKubeconfigData", arg0) +} + +// SetUserKubeconfigData indicates an expected call of SetUserKubeconfigData. +func (mr *MockManagedClusterScopeMockRecorder) SetUserKubeconfigData(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetUserKubeconfigData", reflect.TypeOf((*MockManagedClusterScope)(nil).SetUserKubeconfigData), arg0) +} + // SubscriptionID mocks base method. func (m *MockManagedClusterScope) SubscriptionID() string { m.ctrl.T.Helper() diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index 39514f2d7ca..ac723ee0e72 100644 --- a/azure/services/managedclusters/spec.go +++ b/azure/services/managedclusters/spec.go @@ -125,6 +125,8 @@ type ManagedClusterSpec struct { // DNSPrefix allows the user to customize dns prefix. DNSPrefix *string + // DisableLocalAccounts disables getting static credentials for this cluster when set. Expected to only be used for AAD clusters. + DisableLocalAccounts *bool } // HTTPProxyConfig is the HTTP proxy configuration for the cluster. @@ -390,6 +392,9 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{ EnableAzureRBAC: &s.AADProfile.EnableAzureRBAC, AdminGroupObjectIDs: azure.PtrSlice[string](&s.AADProfile.AdminGroupObjectIDs), } + if s.DisableLocalAccounts != nil { + managedCluster.Properties.DisableLocalAccounts = s.DisableLocalAccounts + } } for i := range s.AddonProfiles { @@ -740,6 +745,14 @@ func computeDiffOfNormalizedClusters(managedCluster armcontainerservice.ManagedC } } + if managedCluster.Properties.DisableLocalAccounts != nil { + clusterNormalized.Properties.DisableLocalAccounts = managedCluster.Properties.DisableLocalAccounts + } + + if existingMC.Properties.DisableLocalAccounts != nil { + existingMCClusterNormalized.Properties.DisableLocalAccounts = existingMC.Properties.DisableLocalAccounts + } + diff := cmp.Diff(clusterNormalized, existingMCClusterNormalized) return diff } diff --git a/azure/services/token/client.go b/azure/services/token/client.go new file mode 100644 index 00000000000..1dfcff401a3 --- /dev/null +++ b/azure/services/token/client.go @@ -0,0 +1,74 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package token + +import ( + "context" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/pkg/errors" + "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/util/tele" +) + +// AzureClient to get azure active directory token. +type AzureClient struct { + aadToken *azidentity.ClientSecretCredential +} + +// NewClient creates a new azure active directory token client from an authorizer. +func NewClient(auth azure.Authorizer) (*AzureClient, error) { + aadToken, err := newAzureActiveDirectoryTokenClient(auth.TenantID(), + auth.ClientID(), + auth.ClientSecret(), + auth.CloudEnvironment()) + if err != nil { + return nil, err + } + return &AzureClient{ + aadToken: aadToken, + }, nil +} + +// newAzureActiveDirectoryTokenClient creates a new aad token client from an authorizer. +func newAzureActiveDirectoryTokenClient(tenantID, clientID, clientSecret, envName string) (*azidentity.ClientSecretCredential, error) { + cliOpts, err := azure.ARMClientOptions(envName) + if err != nil { + return nil, errors.Wrap(err, "error while getting client options") + } + clientOptions := &azidentity.ClientSecretCredentialOptions{ + ClientOptions: cliOpts.ClientOptions, + } + cred, err := azidentity.NewClientSecretCredential(tenantID, clientID, clientSecret, clientOptions) + if err != nil { + return nil, errors.Wrap(err, "error while getting az client secret credentials") + } + return cred, nil +} + +// GetAzureActiveDirectoryToken gets the token for authentication with azure active directory. +func (ac *AzureClient) GetAzureActiveDirectoryToken(ctx context.Context, resourceID string) (string, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "aadToken.GetToken") + defer done() + + spnAccessToken, err := ac.aadToken.GetToken(ctx, policy.TokenRequestOptions{Scopes: []string{resourceID + "/.default"}}) + if err != nil { + return "", errors.Wrap(err, "failed to get token") + } + return spnAccessToken.Token, nil +} diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml index bf6f822d866..f0e7e06b802 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml @@ -239,6 +239,10 @@ spec: - host - port type: object + disableLocalAccounts: + description: DisableLocalAccounts disables getting static credentials + for this cluster when set. Expected to only be used for AAD clusters. + type: boolean dnsPrefix: description: DNSPrefix allows the user to customize dns prefix. Immutable. type: string diff --git a/controllers/azuremanagedcontrolplane_reconciler.go b/controllers/azuremanagedcontrolplane_reconciler.go index 0314a8fbedb..0a0a8bd90de 100644 --- a/controllers/azuremanagedcontrolplane_reconciler.go +++ b/controllers/azuremanagedcontrolplane_reconciler.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "fmt" "github.com/pkg/errors" "sigs.k8s.io/cluster-api-provider-azure/azure" @@ -138,20 +139,25 @@ func (r *azureManagedControlPlaneService) reconcileKubeconfig(ctx context.Contex ctx, _, done := tele.StartSpanWithLogger(ctx, "controllers.azureManagedControlPlaneService.reconcileKubeconfig") defer done() - kubeConfigData := r.scope.GetKubeConfigData() - if kubeConfigData == nil { - return nil - } - kubeConfigSecret := r.scope.MakeEmptyKubeConfigSecret() + kubeConfigs := [][]byte{r.scope.GetAdminKubeconfigData(), r.scope.GetUserKubeconfigData()} - // Always update credentials in case of rotation - if _, err := controllerutil.CreateOrUpdate(ctx, r.kubeclient, &kubeConfigSecret, func() error { - kubeConfigSecret.Data = map[string][]byte{ - secret.KubeconfigDataName: kubeConfigData, + for i, kubeConfigData := range kubeConfigs { + if len(kubeConfigData) == 0 { + continue + } + kubeConfigSecret := r.scope.MakeEmptyKubeConfigSecret() + if i == 1 { + // 2nd kubeconfig is the user kubeconfig + kubeConfigSecret.Name = fmt.Sprintf("%s-user", kubeConfigSecret.Name) + } + if _, err := controllerutil.CreateOrUpdate(ctx, r.kubeclient, &kubeConfigSecret, func() error { + kubeConfigSecret.Data = map[string][]byte{ + secret.KubeconfigDataName: kubeConfigData, + } + return nil + }); err != nil { + return errors.Wrap(err, "failed to kubeconfig secret for cluster") } - return nil - }); err != nil { - return errors.Wrap(err, "failed to kubeconfig secret for cluster") } return nil diff --git a/docs/book/src/topics/managedcluster.md b/docs/book/src/topics/managedcluster.md index 11ffa5b1435..c8ca75a5f1e 100644 --- a/docs/book/src/topics/managedcluster.md +++ b/docs/book/src/topics/managedcluster.md @@ -265,6 +265,36 @@ spec: ... ``` +### Disable Local Accounts in AKS when using Azure Active Directory + +When deploying an AKS cluster, local accounts are enabled by default. +Even when you enable RBAC or Azure AD integration, +--admin access still exists as a non-auditable backdoor option. +Disabling local accounts closes the backdoor access to the cluster +Example to disable local accounts for AAD enabled cluster. + +```yaml +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +kind: AzureManagedMachinePool +metadata: + ... +spec: + aadProfile: + managed: true + adminGroupObjectIDs: + - 00000000-0000-0000-0000-000000000000 # group object id created in azure. + disableLocalAccounts: true + ... +``` + +Note: CAPZ and CAPI requires access to the target cluster to maintain and manage the cluster. +Disabling local accounts will cut off direct access to the target cluster. +CAPZ and CAPI can access target cluster only via the Service Principal, +hence the user has to provide appropriate access to the Service Principal to access the target cluster. +User can do that by adding the Service Principal to the appropriate group defined in Azure and +add the corresponding group ID in `spec.aadProfile.adminGroupObjectIDs`. +CAPI and CAPZ will be able to authenticate via AAD while accessing the target cluster. + ## Features AKS clusters deployed from CAPZ currently only support a limited,