From f4ab52e6d125e5eb649fec643efdaa45f1b99685 Mon Sep 17 00:00:00 2001 From: Dongjiang You Date: Mon, 20 Aug 2018 10:56:15 -0700 Subject: [PATCH] [ACR] Provide a workaround for runtime operations without ARM requests (#7088) * Provide a workaround for runtime operations without ARM requests --- src/command_modules/azure-cli-acr/HISTORY.rst | 1 + .../cli/command_modules/acr/_docker_utils.py | 47 +++++++++++++++++-- .../azure/cli/command_modules/acr/custom.py | 6 ++- .../cli/command_modules/acr/repository.py | 27 ++--------- .../tests/latest/test_acr_commands_mock.py | 40 ++++++++-------- 5 files changed, 71 insertions(+), 50 deletions(-) diff --git a/src/command_modules/azure-cli-acr/HISTORY.rst b/src/command_modules/azure-cli-acr/HISTORY.rst index f2afc9fc92b..1497ad38e50 100644 --- a/src/command_modules/azure-cli-acr/HISTORY.rst +++ b/src/command_modules/azure-cli-acr/HISTORY.rst @@ -5,6 +5,7 @@ Release History 2.1.4 +++++ +* Provide a workaround for runtime operations without ARM requests. * Exclude version control files (eg, .git, .gitignore) from uploaded tar by default in build command. 2.1.3 diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_docker_utils.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_docker_utils.py index 0ca93e115a4..85f1b711375 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_docker_utils.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/_docker_utils.py @@ -10,7 +10,9 @@ from urlparse import urlparse, urlunparse from json import loads +from base64 import b64encode import requests +from requests.utils import to_native_string from msrest.http_logger import log_request, log_response from knack.util import CLIError @@ -18,6 +20,7 @@ from knack.log import get_logger from azure.cli.core.util import should_disable_connection_verify +from azure.cli.core.cloud import CloudSuffixNotSetException from ._client_factory import cf_acr_registries from ._constants import MANAGED_REGISTRY_SKU @@ -118,11 +121,18 @@ def _get_credentials(cli_ctx, :param str repository: Repository for which the access token is requested :param str permission: The requested permission on the repository, '*' or 'pull' """ - registry, resource_group_name = get_registry_by_name(cli_ctx, registry_name, resource_group_name) - login_server = registry.login_server - # 1. if username was specified, verify that password was also specified if username: + # Try to use the pre-defined login server suffix to construct login server from registry name. + # This is to avoid a management server request if username/password are already provided. + # In all other cases, including the suffix not defined, login server will be obtained from server. + login_server_suffix = get_login_server_suffix(cli_ctx) + if login_server_suffix: + login_server = '{}{}'.format(registry_name, login_server_suffix) + else: + registry, _ = get_registry_by_name(cli_ctx, registry_name, resource_group_name) + login_server = registry.login_server + if not password: try: password = prompt_pass(msg='Password: ') @@ -131,6 +141,9 @@ def _get_credentials(cli_ctx, return login_server, username, password + registry, resource_group_name = get_registry_by_name(cli_ctx, registry_name, resource_group_name) + login_server = registry.login_server + # 2. if we don't yet have credentials, attempt to get a refresh token if not password and registry.sku.name in MANAGED_REGISTRY_SKU: try: @@ -222,4 +235,30 @@ def log_registry_response(response): def get_login_server_suffix(cli_ctx): """Get the Azure Container Registry login server suffix in the current cloud.""" - return cli_ctx.cloud.suffixes.acr_login_server_endpoint + try: + return cli_ctx.cloud.suffixes.acr_login_server_endpoint + except CloudSuffixNotSetException: + # Ignore the error if the suffix is not set, the caller should then try to get login server from server. + return None + + +def _get_basic_auth_str(username, password): + return 'Basic ' + to_native_string( + b64encode(('%s:%s' % (username, password)).encode('latin1')).strip() + ) + + +def _get_bearer_auth_str(token): + return 'Bearer ' + token + + +def get_authorization_header(username, password): + """Get the authorization header as Basic auth if username is provided, or Bearer auth otherwise + :param str username: The username used to log into the container registry + :param str password: The password used to log into the container registry + """ + if username: + auth = _get_basic_auth_str(username, password) + else: + auth = _get_bearer_auth_str(password) + return {'Authorization': auth} diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py index 1999bd8e123..ab7f6705256 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/custom.py @@ -183,9 +183,11 @@ def acr_login(cmd, registry_name, resource_group_name=None, username=None, passw "--password", password, login_server]) p.wait() - elif b'--password-stdin' in stderr: - pass else: + if b'--password-stdin' in stderr: + errors = [err for err in stderr.decode().split('\n') if '--password-stdin' not in err] + stderr = '\n'.join(errors).encode() + import sys output = getattr(sys.stderr, 'buffer', sys.stderr) output.write(stderr) diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/repository.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/repository.py index 02f22f440f9..0e1d4482311 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/repository.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/repository.py @@ -4,9 +4,7 @@ # -------------------------------------------------------------------------------------------- import time -from base64 import b64encode import requests -from requests.utils import to_native_string try: from urllib.parse import unquote except ImportError: @@ -19,7 +17,7 @@ from azure.cli.core.util import should_disable_connection_verify from ._utils import validate_managed_registry -from ._docker_utils import get_access_credentials, log_registry_response +from ._docker_utils import get_access_credentials, get_authorization_header, log_registry_response logger = get_logger(__name__) @@ -43,25 +41,6 @@ DEFAULT_PAGINATION = 20 -def _get_basic_auth_str(username, password): - return 'Basic ' + to_native_string( - b64encode(('%s:%s' % (username, password)).encode('latin1')).strip() - ) - - -def _get_bearer_auth_str(token): - return 'Bearer ' + token - - -def _get_authorization_header(username, password): - if username is None: - auth = _get_bearer_auth_str(password) - else: - auth = _get_basic_auth_str(username, password) - - return {'Authorization': auth} - - def _parse_error_message(error_message, response): import json try: @@ -100,7 +79,7 @@ def _request_data_from_registry(http_method, raise ValueError("Non-empty json payload is required for http method: {}".format(http_method)) url = 'https://{}{}'.format(login_server, path) - headers = _get_authorization_header(username, password) + headers = get_authorization_header(username, password) for i in range(0, retry_times): errorMessage = None @@ -146,7 +125,7 @@ def _request_data_from_registry(http_method, def _get_manifest_digest(login_server, repository, tag, username, password, retry_times=3, retry_interval=5): url = 'https://{}/v2/{}/manifests/{}'.format(login_server, repository, tag) - headers = _get_authorization_header(username, password) + headers = get_authorization_header(username, password) headers.update(MANIFEST_V2_HEADER) for i in range(0, retry_times): diff --git a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py index e6382273d33..cd8b8779505 100644 --- a/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py +++ b/src/command_modules/azure-cli-acr/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py @@ -20,12 +20,12 @@ acr_repository_show, acr_repository_delete, acr_repository_untag, - _get_authorization_header, MANIFEST_V2_HEADER ) from azure.cli.command_modules.acr._docker_utils import ( get_login_credentials, - get_access_credentials + get_access_credentials, + get_authorization_header ) from azure.cli.core.mock import DummyCli @@ -53,7 +53,7 @@ def test_repository_list(self, mock_requests_get, mock_get_access_credentials, m mock_requests_get.assert_called_with( method='get', url='https://testregistry.azurecr.io/v2/_catalog', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params={ 'n': 20, 'orderby': None @@ -68,7 +68,7 @@ def test_repository_list(self, mock_requests_get, mock_get_access_credentials, m mock_requests_get.assert_called_with( method='get', url='https://testregistry.azurecr.io/acr/v1/_catalog', - headers=_get_authorization_header(None, 'password'), + headers=get_authorization_header(None, 'password'), params={ 'n': 10, 'orderby': None @@ -107,7 +107,7 @@ def test_repository_show_tags(self, mock_requests_get, mock_get_access_credentia mock_requests_get.assert_called_with( method='get', url='https://testregistry.azurecr.io/v2/testrepository/tags/list', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params={ 'n': 20, 'orderby': None @@ -125,7 +125,7 @@ def test_repository_show_tags(self, mock_requests_get, mock_get_access_credentia mock_requests_get.assert_called_with( method='get', url='https://testregistry.azurecr.io/acr/v1/testrepository/_tags', - headers=_get_authorization_header(None, 'password'), + headers=get_authorization_header(None, 'password'), params={ 'n': 10, 'orderby': 'timedesc' @@ -163,7 +163,7 @@ def test_repository_show_manifests(self, mock_requests_get, mock_get_access_cred mock_requests_get.assert_called_with( method='get', url='https://testregistry.azurecr.io/v2/_acr/testrepository/manifests/list', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params={ 'n': 20, 'orderby': None @@ -179,7 +179,7 @@ def test_repository_show_manifests(self, mock_requests_get, mock_get_access_cred mock_requests_get.assert_called_with( method='get', url='https://testregistry.azurecr.io/acr/v1/testrepository/_manifests', - headers=_get_authorization_header(None, 'password'), + headers=get_authorization_header(None, 'password'), params={ 'n': 10, 'orderby': 'timedesc' @@ -214,7 +214,7 @@ def test_repository_show(self, mock_requests_get, mock_get_access_credentials, m mock_requests_get.assert_called_with( method='get', url='https://testregistry.azurecr.io/acr/v1/testrepository', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY) @@ -226,7 +226,7 @@ def test_repository_show(self, mock_requests_get, mock_get_access_credentials, m mock_requests_get.assert_called_with( method='get', url='https://testregistry.azurecr.io/acr/v1/testrepository/_tags/testtag', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY) @@ -238,7 +238,7 @@ def test_repository_show(self, mock_requests_get, mock_get_access_credentials, m mock_requests_get.assert_called_with( method='get', url='https://testregistry.azurecr.io/acr/v1/testrepository/_manifests/sha256:c5515758d4c5e1e838e9cd307f6c6a0d620b5e07e6f927b07d05f6d12a1ac8d7', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY) @@ -274,7 +274,7 @@ def test_repository_delete(self, mock_requests_get, mock_requests_delete, mock_g mock_requests_delete.assert_called_with( method='delete', url='https://testregistry.azurecr.io/v2/_acr/testrepository/repository', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY) @@ -284,7 +284,7 @@ def test_repository_delete(self, mock_requests_get, mock_requests_delete, mock_g registry_name='testregistry', image='testrepository:testtag', yes=True) - expected_get_headers = _get_authorization_header('username', 'password') + expected_get_headers = get_authorization_header('username', 'password') expected_get_headers.update(MANIFEST_V2_HEADER) mock_requests_get.assert_called_with( url='https://testregistry.azurecr.io/v2/testrepository/manifests/testtag', @@ -293,7 +293,7 @@ def test_repository_delete(self, mock_requests_get, mock_requests_delete, mock_g mock_requests_delete.assert_called_with( method='delete', url='https://testregistry.azurecr.io/v2/testrepository/manifests/sha256:c5515758d4c5e1e838e9cd307f6c6a0d620b5e07e6f927b07d05f6d12a1ac8d7', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY) @@ -306,7 +306,7 @@ def test_repository_delete(self, mock_requests_get, mock_requests_delete, mock_g mock_requests_delete.assert_called_with( method='delete', url='https://testregistry.azurecr.io/v2/testrepository/manifests/sha256:c5515758d4c5e1e838e9cd307f6c6a0d620b5e07e6f927b07d05f6d12a1ac8d7', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY) @@ -318,7 +318,7 @@ def test_repository_delete(self, mock_requests_get, mock_requests_delete, mock_g mock_requests_delete.assert_called_with( method='delete', url='https://testregistry.azurecr.io/v2/_acr/testrepository/tags/testtag', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY) @@ -328,14 +328,14 @@ def test_repository_delete(self, mock_requests_get, mock_requests_delete, mock_g mock_requests_delete.assert_called_with( method='delete', url='https://testregistry.azurecr.io/v2/_acr/testrepository/tags/testtag', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY) # Delete manifest with tag (deprecating) acr_repository_delete(cmd, 'testregistry', 'testrepository', tag='testtag', manifest='', yes=True) - expected_get_headers = _get_authorization_header('username', 'password') + expected_get_headers = get_authorization_header('username', 'password') expected_get_headers.update(MANIFEST_V2_HEADER) mock_requests_get.assert_called_with( url='https://testregistry.azurecr.io/v2/testrepository/manifests/testtag', @@ -344,7 +344,7 @@ def test_repository_delete(self, mock_requests_get, mock_requests_delete, mock_g mock_requests_delete.assert_called_with( method='delete', url='https://testregistry.azurecr.io/v2/testrepository/manifests/sha256:c5515758d4c5e1e838e9cd307f6c6a0d620b5e07e6f927b07d05f6d12a1ac8d7', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY) @@ -354,7 +354,7 @@ def test_repository_delete(self, mock_requests_get, mock_requests_delete, mock_g mock_requests_delete.assert_called_with( method='delete', url='https://testregistry.azurecr.io/v2/testrepository/manifests/sha256:c5515758d4c5e1e838e9cd307f6c6a0d620b5e07e6f927b07d05f6d12a1ac8d7', - headers=_get_authorization_header('username', 'password'), + headers=get_authorization_header('username', 'password'), params=None, json=None, verify=mock.ANY)