From de8848cb1a343ab2a4a427c5263cd9b9463cdbf5 Mon Sep 17 00:00:00 2001 From: Julien Stroheker Date: Fri, 12 Apr 2019 02:16:24 +0200 Subject: [PATCH 1/6] Remove FQDN parameters and update ensure_osa_aad flow --- .../azure/cli/command_modules/acs/_format.py | 2 +- .../azure/cli/command_modules/acs/_help.py | 9 +- .../azure/cli/command_modules/acs/custom.py | 101 ++++++++++-------- .../acs/tests/latest/test_osa_commands.py | 18 +--- 4 files changed, 66 insertions(+), 64 deletions(-) diff --git a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/_format.py b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/_format.py index d2291c84f36..b2f031f814f 100644 --- a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/_format.py +++ b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/_format.py @@ -45,7 +45,7 @@ def _osa_table_format(result): resourceGroup: resourceGroup, openShiftVersion: openShiftVersion, provisioningState: provisioningState, - fqdn: fqdn + publicHostname: publicHostname }""") # use ordered dicts so headers are predictable return parsed.search(result, Options(dict_cls=OrderedDict)) diff --git a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/_help.py b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/_help.py index 6d28635f898..957bcb586ea 100644 --- a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/_help.py +++ b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/_help.py @@ -641,9 +641,6 @@ - name: --compute-count -c type: int short-summary: Number of nodes in the OpenShift node pool. - - name: --fqdn - type: string - short-summary: FQDN for OpenShift API server loadbalancer internal hostname. For example myopenshiftcluster.eastus.cloudapp.azure.com - name: --aad-client-app-id type: string short-summary: The ID of an Azure Active Directory client application. If not specified, a new Azure Active Directory client is created. @@ -666,11 +663,11 @@ examples: - name: Create an OpenShift cluster and auto create an AAD Client - text: az openshift create -g MyResourceGroup -n MyManagedCluster --fqdn {FQDN} + text: az openshift create -g MyResourceGroup -n MyManagedCluster - 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 + text: az openshift create -g MyResourceGroup -n MyManagedCluster --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 - text: az openshift create -g MyResourceGroup -n MyManagedCluster --fqdn {FQDN} --vnet-peer "/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/openshift-vnet/providers/Microsoft.Network/virtualNetworks/test" + text: az openshift create -g MyResourceGroup -n MyManagedCluster --vnet-peer "/subscriptions/0000000-0000-0000-0000-000000000000/resourceGroups/openshift-vnet/providers/Microsoft.Network/virtualNetworks/test" """ helps['openshift delete'] = """ diff --git a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/custom.py b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/custom.py index 8032ebc3fb1..5b3669d3841 100644 --- a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/custom.py +++ b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/custom.py @@ -41,7 +41,9 @@ from azure.cli.core.commands.client_factory import get_mgmt_service_client from azure.cli.core.keys import is_valid_ssh_rsa_public_key from azure.cli.core.util import in_cloud_console, shell_safe_json_parse, truncate_text, sdk_no_wait +from azure.cli.core.commands import LongRunningOperation from azure.graphrbac.models import (ApplicationCreateParameters, + ApplicationUpdateParameters, PasswordCredential, KeyCredential, ServicePrincipalCreateParameters, @@ -1207,6 +1209,8 @@ def update_application(client, object_id, display_name, homepage, identifier_uri client.update_key_credentials(object_id, key_creds) if password_creds: client.update_password_credentials(object_id, password_creds) + if reply_urls: + client.patch(object_id, ApplicationUpdateParameters(reply_urls=reply_urls)) return except GraphErrorException as ex: if 'insufficient privileges' in str(ex).lower(): @@ -2237,18 +2241,23 @@ def _ensure_aks_service_principal(cli_ctx, store_acs_service_principal(subscription_id, client_secret, service_principal, file_name=file_name_aks) return load_acs_service_principal(subscription_id, file_name=file_name_aks) - def _ensure_osa_aad(cli_ctx, aad_client_app_id=None, aad_client_app_secret=None, aad_tenant_id=None, identifier=None, - name=None, update=False): + name=None, create=False): + """Create or Update the OSA AAD. + If identifer is None, reply_url is not set + If identifier passed reply_url is set + This function will be called twice, before and after OSA cluster creation + """ rbac_client = get_graph_rbac_management_client(cli_ctx) - if not aad_client_app_id: - if not aad_client_app_secret and update: + if create: + # This reply_url is temporary set since Azure need one to create the AAD. + app_id_name = 'https://{}'.format(name) + if not aad_client_app_secret: aad_client_app_secret = _create_client_secret() - reply_url = 'https://{}/oauth2callback/Azure%20AD'.format(identifier) # Delegate Sign In and Read User Profile permissions on Windows Azure Active Directory API resource_access = ResourceAccess(id="311a71cc-e848-46a1-bdf8-97ff7156d8e6", @@ -2257,36 +2266,33 @@ def _ensure_osa_aad(cli_ctx, 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: - if list_aad_filtered: - update_application(client=rbac_client.applications, - object_id=list_aad_filtered[0].object_id, - display_name=identifier, - identifier_uris=[reply_url], - reply_urls=[reply_url], - homepage=reply_url, - password=aad_client_app_secret, - required_resource_accesses=[required_osa_aad_access]) - aad_client_app_id = list_aad_filtered[0].app_id - logger.info('Updated AAD: %s', aad_client_app_id) - else: - result = create_application(client=rbac_client.applications, - display_name=identifier, - identifier_uris=[reply_url], - reply_urls=[reply_url], - homepage=reply_url, - password=aad_client_app_secret, - required_resource_accesses=[required_osa_aad_access]) - aad_client_app_id = result.app_id - logger.info('Created an AAD: %s', aad_client_app_id) - else: + .format(app_id_name))) + if list_aad_filtered: aad_client_app_id = list_aad_filtered[0].app_id - aad_client_app_secret = 'whatever' - # Get the TenantID - if aad_tenant_id is None: - profile = Profile(cli_ctx=cli_ctx) - _, _, aad_tenant_id = profile.get_login_credentials() + # Updating reply_url with the correct FQDN information returned by the RP + reply_url = 'https://{}/oauth2callback/Azure%20AD'.format(identifier) + update_application(client=rbac_client.applications, + object_id=list_aad_filtered[0].object_id, + display_name=name, + identifier_uris=[app_id_name], + reply_urls=[reply_url], + homepage=app_id_name, + password=aad_client_app_secret, + required_resource_accesses=[required_osa_aad_access]) + logger.info('Updated AAD: %s', aad_client_app_id) + else: + result = create_application(client=rbac_client.applications, + display_name=name, + identifier_uris=[app_id_name], + homepage=app_id_name, + password=aad_client_app_secret, + required_resource_accesses=[required_osa_aad_access]) + aad_client_app_id = result.app_id + logger.info('Created an AAD: %s', aad_client_app_id) + # Get the TenantID + if aad_tenant_id is None: + profile = Profile(cli_ctx=cli_ctx) + _, _, aad_tenant_id = profile.get_login_credentials() return OpenShiftManagedClusterAADIdentityProvider( client_id=aad_client_app_id, secret=aad_client_app_secret, @@ -2416,7 +2422,7 @@ def _remove_osa_nulls(managed_clusters): This works around a quirk of the SDK for python behavior. These fields are not sent by the server, but get recreated by the CLI's own "to_dict" serialization. """ - attrs = ['tags', 'public_hostname', 'plan', 'type', 'id'] + attrs = ['tags', 'plan', 'type', 'id'] ap_master_attrs = ['name', 'os_type'] net_attrs = ['peer_vnet_id'] for managed_cluster in managed_clusters: @@ -2472,7 +2478,6 @@ def osa_list(cmd, client, resource_group_name=None): def openshift_create(cmd, client, resource_group_name, name, # pylint: disable=too-many-locals - fqdn, location=None, compute_vm_size="Standard_D4s_v3", compute_count=3, @@ -2518,17 +2523,21 @@ def openshift_create(cmd, client, resource_group_name, name, # pylint: disable= ) identity_providers = [] + # Validating if aad_client_app_id aad_client_app_secret aad_tenant_id are set + create_aad = False + if aad_client_app_id is None and aad_client_app_secret is None and aad_tenant_id is None: + create_aad = True + # Validating if the cluster is not existing since we are not supporting the AAD rotation on OSA for now - update_aad_secret = False try: client.get(resource_group_name, name) except CloudError: - update_aad_secret = True + create_aad = True osa_aad_identity = _ensure_osa_aad(cmd.cli_ctx, 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) + aad_tenant_id=aad_tenant_id, identifier=None, + name=name, create=create_aad) identity_providers.append( OpenShiftManagedClusterIdentityProvider( name='Azure AD', @@ -2555,7 +2564,6 @@ def openshift_create(cmd, client, resource_group_name, name, # pylint: disable= osamc = OpenShiftManagedCluster( location=location, tags=tags, open_shift_version="v3.11", - fqdn=fqdn, network_profile=network_profile, auth_profile=auth_profile, agent_pool_profiles=agent_pool_profiles, @@ -2564,11 +2572,18 @@ def openshift_create(cmd, client, resource_group_name, name, # pylint: disable= try: # long_running_operation_timeout=300 - return sdk_no_wait(no_wait, client.create_or_update, + result = sdk_no_wait(no_wait, client.create_or_update, resource_group_name=resource_group_name, resource_name=name, parameters=osamc) + result = LongRunningOperation(cmd.cli_ctx)(result) + instance = client.get(resource_group_name, name) + _ensure_osa_aad(cmd.cli_ctx, + aad_client_app_id=osa_aad_identity.client_id, + aad_client_app_secret=osa_aad_identity.secret, + aad_tenant_id=osa_aad_identity.tenant_id, identifier=instance.public_hostname, + name=name, create=True) except CloudError as ex: raise ex - + def openshift_show(cmd, client, resource_group_name, name): mc = client.get(resource_group_name, name) diff --git a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/tests/latest/test_osa_commands.py b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/tests/latest/test_osa_commands.py index 7d61b96b2c7..2104766e434 100644 --- a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/tests/latest/test_osa_commands.py +++ b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/tests/latest/test_osa_commands.py @@ -29,7 +29,6 @@ def test_openshift_create_default_service(self, resource_group, resource_group_l self.kwargs.update({ 'resource_group': resource_group, 'name': osa_name, - 'fqdn': self.generate_random_fqdn(osa_name, resource_group_location), 'location': resource_group_location, 'aad_client_app_id': aad_client_app_id, 'aad_client_app_secret': aad_client_app_secret, @@ -38,7 +37,7 @@ def test_openshift_create_default_service(self, resource_group, resource_group_l # create create_cmd = 'openshift create --resource-group={resource_group} --name={name} --location={location} ' \ - '--fqdn={fqdn} --compute-count=1 ' \ + '--compute-count=1 ' \ '--aad-client-app-id {aad_client_app_id} --aad-client-app-secret {aad_client_app_secret}' self.cmd(create_cmd, checks=[ self.exists('fqdn'), @@ -53,7 +52,6 @@ def test_openshift_create_default_service(self, resource_group, resource_group_l self.check('agentPoolProfiles[0].count', 1), self.check('agentPoolProfiles[0].osType', 'Linux'), self.check('agentPoolProfiles[0].vmSize', 'Standard_D4s_v3'), - self.check('fqdn', '{fqdn}'), self.exists('openShiftVersion') ]) @@ -80,14 +78,13 @@ def test_openshift_create_service_no_wait(self, resource_group, resource_group_l self.kwargs.update({ 'resource_group': resource_group, 'name': osa_name, - 'fqdn': self.generate_random_fqdn(osa_name, resource_group_location), 'location': resource_group_location, 'aad_client_app_id': aad_client_app_id, 'aad_client_app_secret': aad_client_app_secret }) # create --no-wait - create_cmd = 'openshift create -g {resource_group} -n {name} --fqdn {fqdn} ' \ + create_cmd = 'openshift create -g {resource_group} -n {name} ' \ '-l {location} -c 1 --aad-client-app-id {aad_client_app_id} ' \ '--aad-client-app-secret {aad_client_app_secret} ' \ '--tags scenario_test --no-wait' @@ -102,7 +99,6 @@ def test_openshift_create_service_no_wait(self, resource_group, resource_group_l self.check('resourceGroup', '{resource_group}'), self.check('agentPoolProfiles[0].count', 1), self.check('agentPoolProfiles[0].vmSize', 'Standard_D4s_v3'), - self.check('fqdn', '{fqdn}'), self.check('provisioningState', 'Succeeded'), self.exists('openShiftVersion') ]) @@ -122,14 +118,13 @@ def test_openshift_create_default_service_no_aad(self, resource_group, resource_ self.kwargs.update({ 'resource_group': resource_group, 'name': osa_name, - 'fqdn': self.generate_random_fqdn(osa_name, resource_group_location), 'location': resource_group_location, 'resource_type': 'Microsoft.ContainerService/OpenShiftManagedClusters' }) # create create_cmd = 'openshift create --resource-group={resource_group} --name={name} --location={location} ' \ - '--fqdn={fqdn} --compute-count=1 ' + '--compute-count=1 ' self.cmd(create_cmd, checks=[ self.exists('fqdn'), self.check('provisioningState', 'Succeeded') @@ -143,7 +138,6 @@ def test_openshift_create_default_service_no_aad(self, resource_group, resource_ self.check('agentPoolProfiles[0].count', 1), self.check('agentPoolProfiles[0].osType', 'Linux'), self.check('agentPoolProfiles[0].vmSize', 'Standard_D4s_v3'), - self.check('fqdn', '{fqdn}'), self.exists('openShiftVersion') ]) @@ -158,8 +152,4 @@ def test_openshift_create_default_service_no_aad(self, resource_group, resource_ ]) # delete - self.cmd('openshift delete -g {resource_group} -n {name} --yes --no-wait', checks=[self.is_empty()]) - - @classmethod - def generate_random_fqdn(self, name, location): - return "{}.{}.cloudapp.azure.com".format(name, location) + self.cmd('openshift delete -g {resource_group} -n {name} --yes --no-wait', checks=[self.is_empty()] From f765d5c2fcadb7be9b7ac2a2f9fb1e2f13983d83 Mon Sep 17 00:00:00 2001 From: Julien Stroheker Date: Fri, 12 Apr 2019 02:58:26 +0200 Subject: [PATCH 2/6] pep8 --- .../azure/cli/command_modules/acs/custom.py | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/custom.py b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/custom.py index 5b3669d3841..e97cff0f714 100644 --- a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/custom.py +++ b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/custom.py @@ -2241,17 +2241,13 @@ def _ensure_aks_service_principal(cli_ctx, store_acs_service_principal(subscription_id, client_secret, service_principal, file_name=file_name_aks) return load_acs_service_principal(subscription_id, file_name=file_name_aks) + def _ensure_osa_aad(cli_ctx, aad_client_app_id=None, aad_client_app_secret=None, aad_tenant_id=None, identifier=None, name=None, create=False): - """Create or Update the OSA AAD. - If identifer is None, reply_url is not set - If identifier passed reply_url is set - This function will be called twice, before and after OSA cluster creation - """ rbac_client = get_graph_rbac_management_client(cli_ctx) if create: # This reply_url is temporary set since Azure need one to create the AAD. @@ -2272,13 +2268,13 @@ def _ensure_osa_aad(cli_ctx, # Updating reply_url with the correct FQDN information returned by the RP reply_url = 'https://{}/oauth2callback/Azure%20AD'.format(identifier) update_application(client=rbac_client.applications, - object_id=list_aad_filtered[0].object_id, - display_name=name, - identifier_uris=[app_id_name], - reply_urls=[reply_url], - homepage=app_id_name, - password=aad_client_app_secret, - required_resource_accesses=[required_osa_aad_access]) + object_id=list_aad_filtered[0].object_id, + display_name=name, + identifier_uris=[app_id_name], + reply_urls=[reply_url], + homepage=app_id_name, + password=aad_client_app_secret, + required_resource_accesses=[required_osa_aad_access]) logger.info('Updated AAD: %s', aad_client_app_id) else: result = create_application(client=rbac_client.applications, @@ -2573,7 +2569,7 @@ def openshift_create(cmd, client, resource_group_name, name, # pylint: disable= try: # long_running_operation_timeout=300 result = sdk_no_wait(no_wait, client.create_or_update, - resource_group_name=resource_group_name, resource_name=name, parameters=osamc) + resource_group_name=resource_group_name, resource_name=name, parameters=osamc) result = LongRunningOperation(cmd.cli_ctx)(result) instance = client.get(resource_group_name, name) _ensure_osa_aad(cmd.cli_ctx, @@ -2583,7 +2579,7 @@ def openshift_create(cmd, client, resource_group_name, name, # pylint: disable= name=name, create=True) except CloudError as ex: raise ex - + def openshift_show(cmd, client, resource_group_name, name): mc = client.get(resource_group_name, name) From 969a6c0e148a2ab072462aceff49ff119d609d73 Mon Sep 17 00:00:00 2001 From: Julien Stroheker Date: Fri, 12 Apr 2019 03:00:35 +0200 Subject: [PATCH 3/6] Bump version --- src/command_modules/azure-cli-acs/HISTORY.rst | 4 ++++ src/command_modules/azure-cli-acs/setup.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/command_modules/azure-cli-acs/HISTORY.rst b/src/command_modules/azure-cli-acs/HISTORY.rst index b2dfb24aabc..aa055e6c30f 100644 --- a/src/command_modules/azure-cli-acs/HISTORY.rst +++ b/src/command_modules/azure-cli-acs/HISTORY.rst @@ -2,6 +2,10 @@ Release History =============== +2.3.21 +++++++ +* removing `--fqdn` flag on az openshift commands + 2.3.20 ++++++ * Minor fixes. diff --git a/src/command_modules/azure-cli-acs/setup.py b/src/command_modules/azure-cli-acs/setup.py index 99db7888e92..4327452b3e1 100644 --- a/src/command_modules/azure-cli-acs/setup.py +++ b/src/command_modules/azure-cli-acs/setup.py @@ -14,7 +14,7 @@ logger.warn("Wheel is not available, disabling bdist_wheel hook") cmdclass = {} -VERSION = "2.3.20" +VERSION = "2.3.21" CLASSIFIERS = [ 'Development Status :: 5 - Production/Stable', 'Intended Audience :: Developers', From 024e273fe73d392239ce30dbb298c48d7d36da8d Mon Sep 17 00:00:00 2001 From: Julien Stroheker Date: Fri, 12 Apr 2019 03:06:44 +0200 Subject: [PATCH 4/6] bump setup.py version --- src/command_modules/azure-cli-acs/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command_modules/azure-cli-acs/setup.py b/src/command_modules/azure-cli-acs/setup.py index 4327452b3e1..ef2bbc5b38c 100644 --- a/src/command_modules/azure-cli-acs/setup.py +++ b/src/command_modules/azure-cli-acs/setup.py @@ -14,7 +14,7 @@ logger.warn("Wheel is not available, disabling bdist_wheel hook") cmdclass = {} -VERSION = "2.3.21" +VERSION = "2.3.22" CLASSIFIERS = [ 'Development Status :: 5 - Production/Stable', 'Intended Audience :: Developers', From 997f7f7ad194967164302b0d6f59b786e5702bb8 Mon Sep 17 00:00:00 2001 From: Julien Stroheker Date: Fri, 12 Apr 2019 03:28:38 +0200 Subject: [PATCH 5/6] fix automation tests --- .../cli/command_modules/acs/tests/latest/test_osa_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/tests/latest/test_osa_commands.py b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/tests/latest/test_osa_commands.py index 2104766e434..e7e95a724b7 100644 --- a/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/tests/latest/test_osa_commands.py +++ b/src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/tests/latest/test_osa_commands.py @@ -152,4 +152,4 @@ def test_openshift_create_default_service_no_aad(self, resource_group, resource_ ]) # delete - self.cmd('openshift delete -g {resource_group} -n {name} --yes --no-wait', checks=[self.is_empty()] + self.cmd('openshift delete -g {resource_group} -n {name} --yes --no-wait', checks=[self.is_empty()]) From 7f2a0f932e22abe9821d3da1d3aac7714c7f2fd5 Mon Sep 17 00:00:00 2001 From: Julien Stroheker Date: Fri, 26 Apr 2019 10:10:35 -0400 Subject: [PATCH 6/6] bump 2.4.0 --- src/command_modules/azure-cli-acs/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command_modules/azure-cli-acs/setup.py b/src/command_modules/azure-cli-acs/setup.py index a3efb393bc0..7633041606d 100644 --- a/src/command_modules/azure-cli-acs/setup.py +++ b/src/command_modules/azure-cli-acs/setup.py @@ -14,7 +14,7 @@ logger.warn("Wheel is not available, disabling bdist_wheel hook") cmdclass = {} -VERSION = "2.3.22" +VERSION = "2.4.0" CLASSIFIERS = [ 'Development Status :: 5 - Production/Stable', 'Intended Audience :: Developers',