[DevTestLab] Explicitly enable usage of saved secrets while lab vm creation #2686
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #2686 +/- ##
==========================================
- Coverage 61.13% 61.11% -0.02%
==========================================
Files 480 480
Lines 25679 25686 +7
Branches 3886 3888 +2
==========================================
+ Hits 15698 15699 +1
- Misses 9055 9061 +6
Partials 926 926
Continue to review full report at Codecov.
|
tjprescott
suggested changes
Mar 31, 2017
| @@ -307,6 +307,10 @@ def validate_authentication_type(namespace, formula=None): | |||
| "incorrect usage for authentication-type 'password': " | |||
| "[--admin-username USERNAME] --admin-password PASSWORD") | |||
|
|
|||
| # Respect user's provided saved secret name for password authentication | |||
| if namespace.saved_secret: | |||
| namespace.admin_password = "[[{}]]".format(namespace.saved_secret) | |||
There was a problem hiding this comment.
Is this double bracket syntax a service-side construct?
Either way, there should be validation that errors out if the user provides --admin-password AND --saved-secret. The usage string error message should be updated as well. Same applies for SSH
There was a problem hiding this comment.
Yup [[VALUE]] is the construct expected from the REST endpoint so that it understands that the value needs to be searched in lab's secrets store.
| if namespace.saved_secret: | ||
| namespace.ssh_key = "[[{}]]".format(namespace.saved_secret) | ||
| else: | ||
| validate_ssh_key(namespace) |
There was a problem hiding this comment.
We need to check and issue an error if --ssh-key and --saved-secret are used together. This is a general CLI principle that we don't override or ignore conflicting parameters. It leads to customer confusion.
tjprescott
approved these changes
Apr 5, 2017
johanste
pushed a commit
to johanste/azure-cli
that referenced
this pull request
Apr 7, 2017
…eation (Azure#2686) * Explicitly enable usage of saved secrets for vm creation * Better error message with not overriding competing paramters
tjprescott
pushed a commit
that referenced
this pull request
Apr 11, 2017
* Enable delay-load of descriptions for commands (speed up az) * Update find indexing commands to accept callables for description. * Command load time in progress * - Moved previously dead command filter from parser to application configuration. - Removed unused configuration object/argv on application create. * Remove unused argument (pylint) * Remove dummy parameter * Fix for python 2.7 * Fix yet incorrect passage of parameters * Fix up additional pylint complaints * Update tests * Update tests * Fix up more tests * Fix up more core tests * Enable delay-load of descriptions for commands (speed up az) * Update find indexing commands to accept callables for description. * [Network] Remove nulls from VPN connection show/list output (#2748) * Fix #1615. * Code review feedback. * Update test docs for running individual test and all tests in mod (#2763) * Update test docs for running individual test and all tests in mod * Made feedback changes * Make argument parameters match up. (#2717) Make lock command parameter aliases match up with resource commands. * [DevTestLabs] Adding scenario test to create simple Linux + Windows VM in lab (#2767) * WIP create linux + Windows vm in lab * Adding recording * Add some more error checking/handling. (#2768) Add more validation to resolve "lock level" for lock commands. * Fix doc references to azure.cli.commands (#2740) * Fix doc references to azure.cli.commands This module has moved to azure.cli.core.commands * Fix PyLint * Add clearer guidelines on modifying changelog (#2739) * Add clearer guidelines on modifying changelog * A few smaller changes * another small format change * Code review changes * [DevTestLabs] Exposing commands to manage secrets in the lab (#2691) * ACS Update: nulling out the windows profile so that there isn't a validation fail… (#2764) * nulling out the windows profile so that there isn't a valdiation failure for missing password ACS doesn't return a password on GET. az acs scale command does a GET then PUT, but since ACS doesn't return the password the verification is failing before the PUT is sent to ACS. There is a bug in ACS this exposes. So this shouldn't be merged until after the ACS rollout finishes. Should be about start of next week. * updating history * updating version in history * removing white space added by editor * [Compute] Fix issues with VMSS and VM availability set update. (#2773) * Fix issues with VMSS and VM availability set update. * Update help. Fix #2762. * Error out if you try to list resources for a group that doesn't exist. (#2769) * Minor text fixes (#2776) * Add docs for az lock update. (#2702) * [DevTestLab] Explicitly enable usage of saved secrets while lab vm creation (#2686) * Explicitly enable usage of saved secrets for vm creation * Better error message with not overriding competing paramters * Adding export-artifacts commands on formula (#2707) * core: apply configured defaults on optional arg (#2770) * Core:apply configured defaults on optional argument * add a test * add tests * update history doc * address review feedback * [Network] Support active-active VNet gateways (#2751) * Start active-active test scenario. * Add active-active parameter. * Active-active scenario test 1 (cross premise) * Add second active-active scenario (vnet-to-vnet) * Refine active-active gateway configuration. * Pylint... * Code review feedback * Packaged release notes and changes for 0.2.4 (#2735) * Modify HISTORY.md * Update Dockerfile * Update debian also * Add pip dependencies also * Command load time in progress * - Moved previously dead command filter from parser to application configuration. - Removed unused configuration object/argv on application create. * Remove unused argument (pylint) * Remove dummy parameter * Fix for python 2.7 * Fix yet incorrect passage of parameters * Fix up additional pylint complaints * Update tests * Update tests * Fix up more tests * Fix up more core tests * Improve load time of custom.py for profile, find and configure (speeds up raw az command) * Pylint + flake8 fixes * Fix new vm tests that failed due to perf refactoring * Update redis tests that was broken due to perf refactoring * Delay-load msrest for command executions that don't need it * Fix flake8 issues * Fixing/improving detection of pageable class * flake8 fixes * Fix broken merge from upstream/master * Fix broken merge (again) * flake8 fixes * Fix up even more merge errors from last upstream merge * Flake8 fixes (wrong number of newlines) * Fix delay load of storage assembly for az * Update history to reference improved performance
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2687
passwordbased authsshbased authUntil now user were allowed to use them but not explicitly. This enables supplying the name of the secrets to be used from the lab's secret store itself.
Reference: https://azure.microsoft.com/en-us/updates/azure-devtest-labs-keep-your-secrets-safe-and-easy-to-use-with-the-new-personal-secret-store/
NOTE: I'll be sending new PRs on DTL work but it's not necessary for us to ship them with this release so feel free to label it as team prefers