-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Osa cluster admin #9058
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
Osa cluster admin #9058
Changes from 6 commits
494a9c8
b244fa7
1356978
0fcfc2e
8fdd587
8084cc6
3e87b7c
a96742c
15133e9
b8405a4
1f75865
5af166f
8643736
1459498
f58917a
889e482
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -648,11 +648,16 @@ | |
| - name: --subnet-prefix | ||
| type: string | ||
| short-summary: The CIDR used on the Subnet into which to deploy the cluster. | ||
| - name: --customer-admin-group-id | ||
| type: string | ||
| short-summary: The ID of an Azure Active Directory Group that memberships will get synced into the OpenShift group "osa-customer-admins". If not specified, no cluster admin access will be granted. | ||
|
||
|
|
||
|
|
||
| examples: | ||
| - name: Create an OpenShift cluster and auto create an AAD Client | ||
| text: az openshift create -g MyResourceGroup -n MyManagedCluster --fqdn {FQDN} | ||
| - name: Create an OpenShift cluster and auto create an AAD Client and setup cluster admin group | ||
| text: az openshift create -g MyResourceGroup -n MyManagedCluster --fqdn {FQDN} --customer-admin-group-id {GROUP_ID} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is going to be incompatible with PR #9083 which removes FQDN. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tjprescott FQDN is not being removed. Its being made optional. PR #9083 will be done on top of this PR. |
||
| - name: Create an OpenShift cluster with 5 compute nodes and a custom AAD Client. | ||
| text: az openshift create -g MyResourceGroup -n MyManagedCluster --fqdn {FQDN} --aad-client-app-id {APP_ID} --aad-client-app-secret {APP_SECRET} --aad-tenant-id {TENANT_ID} --compute-count 5 | ||
| - name: Create an Openshift cluster using a custom vnet | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2243,7 +2243,8 @@ def _ensure_osa_aad(cli_ctx, | |
| aad_client_app_secret=None, | ||
| aad_tenant_id=None, | ||
| identifier=None, | ||
| name=None, update=False): | ||
| name=None, update=False, | ||
| customer_admin_group_id=None): | ||
| rbac_client = get_graph_rbac_management_client(cli_ctx) | ||
| if not aad_client_app_id: | ||
| if not aad_client_app_secret and update: | ||
|
|
@@ -2253,9 +2254,14 @@ def _ensure_osa_aad(cli_ctx, | |
| # Delegate Sign In and Read User Profile permissions on Windows Azure Active Directory API | ||
| resource_access = ResourceAccess(id="311a71cc-e848-46a1-bdf8-97ff7156d8e6", | ||
| additional_properties=None, type="Scope") | ||
| required_osa_aad_access = RequiredResourceAccess(resource_access=[resource_access], | ||
| # Read directory permissions on Windows Azure Active Directory API | ||
| directory_access = ResourceAccess(id="5778995a-e1bf-45b8-affa-663a9f3f4d04", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this hard-coded ID? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@tjprescott these are scope GUIDs for programmatic use and represent various permissions: https://docs.microsoft.com/en-us/graph/permissions-reference. Currently there isn't a straightforward documentation from AAD for this and this is obtained by looking at currently configured apps in AAD. |
||
| additional_properties=None, type="Role") | ||
|
|
||
| required_osa_aad_access = RequiredResourceAccess(resource_access=[resource_access, directory_access], | ||
| additional_properties=None, | ||
| resource_app_id="00000002-0000-0000-c000-000000000000") | ||
|
|
||
| list_aad_filtered = list(rbac_client.applications.list(filter="identifierUris/any(s:s eq '{}')" | ||
| .format(reply_url))) | ||
| if update: | ||
|
|
@@ -2291,7 +2297,8 @@ def _ensure_osa_aad(cli_ctx, | |
| client_id=aad_client_app_id, | ||
| secret=aad_client_app_secret, | ||
| tenant_id=aad_tenant_id, | ||
| kind='AADIdentityProvider') | ||
| kind='AADIdentityProvider', | ||
| customer_admin_group_id=customer_admin_group_id) | ||
|
|
||
|
|
||
| def _ensure_service_principal(cli_ctx, | ||
|
|
@@ -2483,7 +2490,8 @@ def openshift_create(cmd, client, resource_group_name, name, # pylint: disable= | |
| subnet_prefix="10.0.0.0/24", | ||
| vnet_peer=None, | ||
| tags=None, | ||
| no_wait=False): | ||
| no_wait=False, | ||
| customer_admin_group_id=None): | ||
|
|
||
| if location is None: | ||
| location = _get_rg_location(cmd.cli_ctx, resource_group_name) | ||
|
|
@@ -2499,7 +2507,7 @@ def openshift_create(cmd, client, resource_group_name, name, # pylint: disable= | |
|
|
||
| agent_infra_pool_profile = OpenShiftManagedClusterAgentPoolProfile( | ||
| name='infra', # Must be 12 chars or less before ACS RP adds to it | ||
| count=int(2), | ||
| count=int(3), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this change the default node pool count from 2 to 3? That seems like a significant change (not related to this PR). Maybe it should be mentioned separately in the release notes.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we agreed on this to switch from 2 to 3 |
||
| vm_size="Standard_D4s_v3", | ||
| os_type="Linux", | ||
| role=OpenShiftAgentPoolProfileRole.infra, | ||
|
|
@@ -2528,7 +2536,8 @@ def openshift_create(cmd, client, resource_group_name, name, # pylint: disable= | |
| aad_client_app_id=aad_client_app_id, | ||
| aad_client_app_secret=aad_client_app_secret, | ||
| aad_tenant_id=aad_tenant_id, identifier=fqdn, | ||
| name=name, update=update_aad_secret) | ||
| name=name, update=update_aad_secret, | ||
| customer_admin_group_id=customer_admin_group_id) | ||
| identity_providers.append( | ||
| OpenShiftManagedClusterIdentityProvider( | ||
| name='Azure AD', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move to the top
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, moved to top as 2.3.22 and bumped version