Skip to content

Commit

Permalink
[ACR]: bug fixes on token create and image import commands (Azure#13392)
Browse files Browse the repository at this point in the history
  • Loading branch information
yugangw-msft authored May 9, 2020
1 parent 14ecf9e commit 05d6f12
Show file tree
Hide file tree
Showing 5 changed files with 597 additions and 4,455 deletions.
4 changes: 3 additions & 1 deletion src/azure-cli/azure/cli/command_modules/acr/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,14 +321,16 @@ def load_arguments(self, _): # pylint: disable=too-many-statements
c.argument('repository_actions_list', options_list=['--repository'], nargs='+', action='append',
help='repository permissions. Use the format "--repository REPO [ACTION1 ACTION2 ...]" per flag. ' + valid_actions)
c.argument('no_passwords', arg_type=get_three_state_flag(), help='Do not generate passwords, instead use "az acr token credential generate"')
c.argument('expiration_in_days', help='Number of days for which the credentials will be valid. If not specified, the expiration will default to the max value "9999-12-31T23:59:59.999999+00:00"', type=int, required=False)

with self.argument_context('acr token update') as c:
c.argument('scope_map_name', options_list=['--scope-map'], help='The name of the scope map associated with the token. If not specified, running this command will disassociate the current scope map related to the token.', required=False)

with self.argument_context('acr token credential generate') as c:
c.argument('password1', options_list=['--password1'], help='Flag indicating if password1 should be generated.', action='store_true', required=False)
c.argument('password2', options_list=['--password2'], help='Flag indicating if password2 should be generated.', action='store_true', required=False)
c.argument('days', options_list=['--days'], help='Number of days for which the credentials will be valid. If not specified, the expiration will default to the max value "9999-12-31T23:59:59.999999+00:00"', type=int, required=False)
c.argument('expiration_in_days', options_list=['--expiration-in-days', c.deprecate(target='--days', redirect='--expiration-in-days', hide=True)],
help='Number of days for which the credentials will be valid. If not specified, the expiration will default to the max value "9999-12-31T23:59:59.999999+00:00"', type=int, required=False)

with self.argument_context('acr token credential delete') as c:
c.argument('password1', options_list=['--password1'], help='Flag indicating if first password should be deleted', action='store_true', required=False)
Expand Down
8 changes: 7 additions & 1 deletion src/azure-cli/azure/cli/command_modules/acr/import.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,16 @@ def acr_import(cmd,
if source_registry:
if is_valid_resource_id(source_registry):
source = ImportSource(resource_id=source_registry, source_image=source_image)

else:
registry = get_registry_from_name_or_login_server(cmd.cli_ctx, source_registry, source_registry)
if registry:
# trim away redundant login server name, a common error
prefix = registry.login_server + '/'
if source_image.lower().startswith(prefix.lower()):
warning = ('The login server name of "%s" in the "--source" argument will be ignored as '
'"--registry" already supplies the same information')
logger.warning(warning, prefix[:-1])
source_image = source_image[len(prefix):]
# For Azure container registry
source = ImportSource(resource_id=registry.id, source_image=source_image)
else:
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

import datetime
import unittest
from azure_devtools.scenario_tests import AllowLargeResponse
from azure.cli.testsdk import ScenarioTest, ResourceGroupPreparer
Expand All @@ -12,24 +13,27 @@ class AcrTokenCommandsTests(ScenarioTest):

@AllowLargeResponse()
@ResourceGroupPreparer()
@unittest.skip('blocked till the feature gets deployed to production')
def test_repository_token_create(self):
self.kwargs.update({
'registry': self.create_random_name('clireg', 20),
'scope_map': 'scope-map',
'token': 'acr-token',
'token2': 'token-2'
'token2': 'token-2',
'token_short_lived': 'token-3'
})
self.cmd('acr create -g {rg} -n {registry} --sku premium')

# quick create
# ACR ever verifies the existence of the repository, hence we will feed a fake
self.cmd('acr token create -r {registry} -n {token} --repository foo content/read', checks=[
output = self.cmd('acr token create -r {registry} -n {token} --repository foo content/read', checks=[
self.check('status', 'enabled'),
self.check('credentials.passwords[0].name', 'password1'),
self.check('credentials.passwords[1].name', 'password2'),
self.check('credentials.username', self.kwargs['token'])
])
self.check('credentials.username', self.kwargs['token']),
self.check('credentials.passwords[0].expiry', None)
]).get_output_in_json()
today = datetime.datetime.strptime(output['credentials']['passwords'][0]['creationTime'].split('T')[0], "%Y-%m-%d")

self.cmd('acr scope-map show -r {registry} -n {token}-scope-map', checks=[
self.check('actions', ['repositories/foo/content/read'])
])
Expand All @@ -45,3 +49,7 @@ def test_repository_token_create(self):
self.check('passwords[0].name', 'password1'),
self.check('passwords[1].name', 'password2')
])

output = self.cmd('acr token create -r {registry} -n {token_short_lived} --repository foo content/read --expiration-in-days 1').get_output_in_json()
tomorrow = datetime.datetime.strptime(output['credentials']['passwords'][0]['expiry'].split('T')[0], "%Y-%m-%d")
self.assertEqual(tomorrow - today, datetime.timedelta(1))
17 changes: 10 additions & 7 deletions src/azure-cli/azure/cli/command_modules/acr/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ def acr_token_create(cmd,
repository_actions_list=None,
status=None,
resource_group_name=None,
no_passwords=None):
no_passwords=None,
expiration_in_days=None):
from knack.log import get_logger
from ._utils import get_resource_id_by_registry_name

if bool(repository_actions_list) == bool(scope_map_name):
raise CLIError("usage error: --repository | --scope-map-name")
if no_passwords and expiration_in_days is not None:
raise CLIError("usage error: --no-passwords and --expiration-in-days are mutually exclusive.")

resource_group_name = get_resource_group_name_by_registry_name(cmd.cli_ctx, registry_name, resource_group_name)

Expand Down Expand Up @@ -54,7 +57,7 @@ def acr_token_create(cmd,
return poller

token = LongRunningOperation(cmd.cli_ctx)(poller)
_create_default_passwords(cmd, resource_group_name, registry_name, token, logger)
_create_default_passwords(cmd, resource_group_name, registry_name, token, logger, expiration_in_days)
return token


Expand All @@ -80,11 +83,11 @@ def _create_default_scope_map(cmd, resource_group_name, registry_name, token_nam
return scope_map.id


def _create_default_passwords(cmd, resource_group_name, registry_name, token, logger):
def _create_default_passwords(cmd, resource_group_name, registry_name, token, logger, expiration_in_days):
from ._client_factory import cf_acr_token_credentials, cf_acr_registries
cred_client = cf_acr_token_credentials(cmd.cli_ctx)
poller = acr_token_credential_generate(cmd, cred_client, registry_name, token.name,
password1=True, password2=True, days=None,
password1=True, password2=True, expiration_in_days=expiration_in_days,
resource_group_name=resource_group_name)
credentials = LongRunningOperation(cmd.cli_ctx)(poller)
setattr(token.credentials, 'username', credentials.username)
Expand Down Expand Up @@ -180,7 +183,7 @@ def acr_token_credential_generate(cmd,
token_name,
password1=False,
password2=False,
days=None,
expiration_in_days=None,
resource_group_name=None):

from ._utils import get_resource_id_by_registry_name
Expand All @@ -192,9 +195,9 @@ def acr_token_credential_generate(cmd,
# We only want to specify a password if only one was passed.
name = ("password1" if password1 else "password2") if password1 ^ password2 else None
expiry = None
if days:
if expiration_in_days:
from ._utils import add_days_to_now
expiry = add_days_to_now(days)
expiry = add_days_to_now(expiration_in_days)

GenerateCredentialsParameters = cmd.get_models('GenerateCredentialsParameters')

Expand Down

0 comments on commit 05d6f12

Please sign in to comment.