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

Add AAD Support for AKS #1407

Closed
LochanRn opened this issue Jun 2, 2021 · 11 comments · Fixed by #1560
Closed

Add AAD Support for AKS #1407

LochanRn opened this issue Jun 2, 2021 · 11 comments · Fixed by #1560
Assignees
Labels
area/managedclusters Issues related to managed AKS clusters created through the CAPZ ManagedCluster Type kind/feature Categorizes issue or PR as related to a new feature.

Comments

@LochanRn
Copy link
Member

LochanRn commented Jun 2, 2021

/kind feature
/area managedclusters

Describe the solution you'd like
[A clear and concise description of what you want to happen.]
You can integrate Azure Active Directory with an AKS cluster and use that to authorise users who access the cluster. We should add support for this in CAPZ

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

  • cluster-api-provider-azure version:
  • Kubernetes version: (use kubectl version):
  • OS (e.g. from /etc/os-release):
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. area/managedclusters Issues related to managed AKS clusters created through the CAPZ ManagedCluster Type labels Jun 2, 2021
@LochanRn
Copy link
Member Author

LochanRn commented Jun 2, 2021

Hey, we have a requirement for this feature, I would like to take this up

We propose to add this field to AzureManagedControlPlaneSpec

type AzureManagedControlPlaneSpec struct {

…..
	// AadProfile - Profile of Azure Active Directory configuration to integrate with AKS for aad authentication.
	// +optional
         AADProfile *ManagedClusterAADProfile `json:"aadProfile,omitempty"`

}

type ManagedClusterAADProfile struct {
	// Managed - Whether to enable managed AAD.
	// +optional
	Managed *bool `json:"managed,omitempty"`

	// EnableAzureRBAC - Whether to enable Azure RBAC for Kubernetes authorization.
	// +optional
	EnableAzureRBAC *bool `json:"enableAzureRBAC,omitempty"`

	// AdminGroupObjectIDs - AAD group object IDs that will have admin role of the cluster.
	// +optional
	AdminGroupObjectIDs *[]string `json:"adminGroupObjectIDs,omitempty"`

	// ClientAppID - The client AAD application ID.
	// +optional
	ClientAppID *string `json:"clientAppID,omitempty"`

	// ServerAppID - The server AAD application ID.
	// +optional
	ServerAppID *string `json:"serverAppID,omitempty"`

	// ServerAppSecret - The server AAD application secret.
	// +optional
	ServerAppSecret *string `json:"serverAppSecret,omitempty"`

	// TenantID - The AAD tenant ID to use for authentication. If not specified, will use the tenant of the deployment subscription.
	// +optional
	TenantID *string `json:"tenantID,omitempty"`
}

Behaviour

For the latest support
On setting Managed, EnableAzureRBAC and providing objectId for AdminGroupObjectIDs it must create or update the aks managedcluster to use aks aad support.

For legacy support
On providing ClientAppID, ServerAppID, ServerAppSecret, TenantID it must create or update the aks managedcluster to use aks aad support.

Limitations:

  • AKS-managed Azure AD integration can't be disabled
  • Changing a AKS-managed Azure AD integrated cluster to legacy AAD is not supported
  • non-Kubernetes RBAC enabled clusters aren't supported for AKS-managed Azure AD integration
  • Changing the Azure AD tenant associated with AKS-managed Azure AD integration isn't supported

Other Required Changes:

Currently Capz is using the 2020-02-01 SDK which has only legacy support for AADProfile. We need to update it to 2021-03-01 SDK for the latest support.

@LochanRn LochanRn changed the title Add AAD Support Add AAD Support for AKS Jun 2, 2021
@devigned
Copy link
Contributor

devigned commented Jun 2, 2021

  1. Would there be any functionality difference moving to v2021-03-01 that would affect exiting clusters which use legacy support?
  2. How long should legacy support be supported? Should those fields be marked as deprecated?

@LochanRn
Copy link
Member Author

LochanRn commented Jun 2, 2021

  1. Would there be any functionality difference moving to v2021-03-01 that would affect exiting clusters which use legacy support?
  2. How long should legacy support be supported? Should those fields be marked as deprecated?

1.) We don't have to change the entire SDK we only need to change the containerservice package to 2021-03-01. So after looking into the differences in both the models the changes would be very minimal and so will be the functional difference.

2.) Yup marking the legacy support as deprecated does makes sense.

@CecileRobertMichon
Copy link
Contributor

According to AKS docs, it's possible to migrate an existing legacy AAD cluster to the new managed AAD, I would expect the same to apply here.

@LochanRn
Copy link
Member Author

LochanRn commented Jun 2, 2021

According to AKS docs, it's possible to migrate an existing legacy AAD cluster to the new managed AAD, I would expect the same to apply here.

Ok 👍 .

@LochanRn
Copy link
Member Author

LochanRn commented Jun 2, 2021

According to AKS docs, it's possible to migrate an existing legacy AAD cluster to the new managed AAD, I would expect the same to apply here.

The Limitations section I added it from the AKS docs
and it says migrating an AKS-managed Azure AD integrated cluster to legacy AAD is not supported.

@CecileRobertMichon
Copy link
Contributor

migrating an AKS-managed Azure AD integrated cluster to legacy AAD is not supported

correct, but the reverse is (from legacy to managed)

@LochanRn
Copy link
Member Author

LochanRn commented Jun 2, 2021

migrating an AKS-managed Azure AD integrated cluster to legacy AAD is not supported

correct, but the reverse is (from legacy to managed)

yup, will ensure that feature is supported. :)

@alexeldeib
Copy link
Contributor

fwiw, this all sounds good to me :) I was planning to update the SDK for #1376, but if you want to do it here go ahead! I'm on vacation for the next week anyway so probably won't get to it until after that.

let's make sure we capture as many of the validations as possible on the webhook side, since it should be fairly easy to do.

@LochanRn
Copy link
Member Author

LochanRn commented Jun 3, 2021

fwiw, this all sounds good to me :) I was planning to update the SDK for #1376, but if you want to do it here go ahead! I'm on vacation for the next week anyway so probably won't get to it until after that.

let's make sure we capture as many of the validations as possible on the webhook side, since it should be fairly easy to do.

ok

@LochanRn
Copy link
Member Author

/assign @LochanRn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/managedclusters Issues related to managed AKS clusters created through the CAPZ ManagedCluster Type kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
5 participants