Skip to content
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

{ACR}: expose --expiration from token commands #13451

Merged
merged 2 commits into from
May 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 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 @@ -35,7 +35,8 @@
validate_set,
validate_set_secret,
validate_retention_days,
validate_registry_name
validate_registry_name,
validate_expiration_time
)
from .scope_map import ScopeMapActions

Expand Down Expand Up @@ -332,6 +333,11 @@ def load_arguments(self, _): # pylint: disable=too-many-statements
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)

for scope in ['acr token create', 'acr token credential generate']:
with self.argument_context(scope) as c:
c.argument('expiration', validator=validate_expiration_time,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems there're different expiration unit/measure: expire in days, expire at exact date etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider expire_at as option name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yungezz, Yes expiration is for exact date, the expiration-in-days is for days to make the input easier.

@fengzhou-msft, I name it as expiration for wording consistency with --expiration-in-days; or I can do --expire-in-days and --expire-at, let me know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think --expire-in-days and --expire-at is more clear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yugangw-msft let me know whether you would make the change. Either is fine as users can always know the meaning of the options from help messages.

help='UTC time for which the credentials will be valid. In the format of %Y-%m-%dT%H:%M:%SZ, e.g. 2025-12-31T12:59:59Z')

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)
c.argument('password2', options_list=['--password2'], help='Flag indicating if second password should be deleted.', action='store_true', required=False)
Expand Down
11 changes: 11 additions & 0 deletions src/azure-cli/azure/cli/command_modules/acr/_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,14 @@ def validate_registry_name(cmd, namespace):
if pos > 0:
logger.warning("The login server endpoint suffix '%s' is automatically omitted.", acr_suffix)
namespace.registry_name = registry[:pos]


def validate_expiration_time(namespace):
import datetime
DATE_TIME_FORMAT = '%Y-%m-%dT%H:%M:%SZ'
if namespace.expiration:
try:
namespace.expiration = datetime.datetime.strptime(namespace.expiration, DATE_TIME_FORMAT)
except ValueError:
raise CLIError("Input '{}' is not valid datetime. Valid example: 2025-12-31T12:59:59Z".format(
namespace.expiration))
2 changes: 1 addition & 1 deletion src/azure-cli/azure/cli/command_modules/acr/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def load_command_table(self, _): # pylint: disable=too-many-statements
g.command('delete', 'acr_delete')
g.show_command('show', 'acr_show')
g.command('show-usage', 'acr_show_usage', table_transformer=usage_output_format)
g.command('show-endpoints', 'acr_show_endpoints', table_transformer=endpoints_output_format, is_preview=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do apologize for the back and forth, but we decide to make it GA

g.command('show-endpoints', 'acr_show_endpoints', table_transformer=endpoints_output_format)
g.generic_update_command('update',
getter_name='acr_update_get',
setter_name='acr_update_set',
Expand Down
3 changes: 3 additions & 0 deletions src/azure-cli/azure/cli/command_modules/acr/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ def acr_show_endpoints(cmd,
'endpoint': host,
})
else:
logger.warning('To configure client firewall w/o using wildcard storage blob urls, '
'use "az acr update --name %s --data-endpoint-enabled" to enable dedicated '
'data endpoints.', registry_name)
from ._client_factory import cf_acr_replications
replicate_client = cf_acr_replications(cmd.cli_ctx)
replicates = list(replicate_client.list(resource_group_name, registry_name))
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ def test_repository_token_create(self):
'scope_map': 'scope-map',
'token': 'acr-token',
'token2': 'token-2',
'token_short_lived': 'token-3'
'token_short_lived': 'token-3',
'token_long_lived': 'token-4'
})
self.cmd('acr create -g {rg} -n {registry} --sku premium')

Expand Down Expand Up @@ -53,3 +54,6 @@ def test_repository_token_create(self):
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))

self.cmd('acr token create -r {registry} -n {token_long_lived} --repository foo content/read --expiration 2100-12-31T12:59:59Z',
checks=self.check('credentials.passwords[0].expiry', '2100-12-31T12:59:59+00:00'))
20 changes: 14 additions & 6 deletions src/azure-cli/azure/cli/command_modules/acr/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ def acr_token_create(cmd,
status=None,
resource_group_name=None,
no_passwords=None,
expiration=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.")
if no_passwords and (expiration_in_days is not None or expiration is not None):
raise CLIError("usage error: --no-passwords and expiration arguments are mutually exclusive.")
if expiration_in_days is not None and expiration is not None:
raise CLIError("usage error: --expiration 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 @@ -57,7 +60,8 @@ def acr_token_create(cmd,
return poller

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


Expand All @@ -83,12 +87,13 @@ 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, expiration_in_days):
def _create_default_passwords(cmd, resource_group_name, registry_name, token, logger,
expiration_in_days, expiration):
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, expiration_in_days=expiration_in_days,
resource_group_name=resource_group_name)
password1=True, password2=True, resource_group_name=resource_group_name,
expiration_in_days=expiration_in_days, expiration=expiration)
credentials = LongRunningOperation(cmd.cli_ctx)(poller)
setattr(token.credentials, 'username', credentials.username)
setattr(token.credentials, 'passwords', credentials.passwords)
Expand Down Expand Up @@ -184,6 +189,7 @@ def acr_token_credential_generate(cmd,
password1=False,
password2=False,
expiration_in_days=None,
expiration=None,
resource_group_name=None):

from ._utils import get_resource_id_by_registry_name
Expand All @@ -198,6 +204,8 @@ def acr_token_credential_generate(cmd,
if expiration_in_days:
from ._utils import add_days_to_now
expiry = add_days_to_now(expiration_in_days)
elif expiration:
expiry = expiration

GenerateCredentialsParameters = cmd.get_models('GenerateCredentialsParameters')

Expand Down