Skip to content

Commit

Permalink
[ACR] Provide a workaround for runtime operations without ARM requests (
Browse files Browse the repository at this point in the history
#7088)

* Provide a workaround for runtime operations without ARM requests
  • Loading branch information
djyou authored and troydai committed Aug 20, 2018
1 parent 4038a5e commit f4ab52e
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 50 deletions.
1 change: 1 addition & 0 deletions src/command_modules/azure-cli-acr/HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@
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
from knack.prompting import prompt, prompt_pass, NoTTYException
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
Expand Down Expand Up @@ -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: ')
Expand All @@ -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:
Expand Down Expand Up @@ -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}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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__)
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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'
Expand Down Expand Up @@ -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
Expand All @@ -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'
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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',
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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',
Expand All @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit f4ab52e

Please sign in to comment.