Skip to content

[Container] Add secrets for container instances#5578

Merged
tjprescott merged 7 commits intoAzure:devfrom
paulmey:aci-secrets
Feb 20, 2018
Merged

[Container] Add secrets for container instances#5578
tjprescott merged 7 commits intoAzure:devfrom
paulmey:aci-secrets

Conversation

@paulmey
Copy link
Copy Markdown
Member

@paulmey paulmey commented Feb 13, 2018

Add secrets for container instances on create/update.


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

Command Guidelines

  • Each command and parameter has a meaningful description.
  • Each new command has a test.

(see Authoring Command Modules)

Adds --secrets and --secrets-mount-path to add secrets for an Azure
Container Instance.
@promptws
Copy link
Copy Markdown

View a preview at https://prompt.ws/r/Azure/azure-cli/5578
This is an experimental preview for @microsoft users.

Copy link
Copy Markdown
Member

@derekbekoe derekbekoe left a comment

Choose a reason for hiding this comment

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

Fix typo in release notes but LGTM otherwise.


0.1.19
++++++
* Add '--secrets' and '--secrets-mount-path' options to 'az container create' for usgin secrets in ACI
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typo: usgin

@paulmey
Copy link
Copy Markdown
Member Author

paulmey commented Feb 14, 2018

Thanks @derekbekoe, I fixed the typo

@paulmey
Copy link
Copy Markdown
Member Author

paulmey commented Feb 14, 2018

Now fixing some tests...

@paulmey
Copy link
Copy Markdown
Member Author

paulmey commented Feb 15, 2018

Fixed a type, a pylint issue and re-recorded the test. Please review.

Copy link
Copy Markdown
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

A question but otherwise LGTM

secrets_type = CLIArgumentType(
validator=validate_secrets,
help="space-separated secrets in 'key=value' format.",
nargs='*'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nargs='*' means it takes 0 or more arguments. Is this what you intended? 1 or more is nargs='+'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch, I think that + makes more sense. I was 'inspired' by the --tags pattern, but that has a valid use case for 'empty' tags array.
Pending...

@tjprescott tjprescott merged commit 226ad74 into Azure:dev Feb 20, 2018
LukaszStem pushed a commit to LukaszStem/azure-cli that referenced this pull request Feb 21, 2018
* Add secrets for container instances

Adds --secrets and --secrets-mount-path to add secrets for an Azure
Container Instance.

* Version 0.1.19

* fixup! Add secrets for container instances

* fixup! Version 0.1.19

* fixup! Add secrets for container instances

* fixup! Add secrets for container instances

* Do not allow empty array of secrets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants