[KeyVault] Onboard KeyVault Round 1 #1037

tjprescott · 2016-10-06T16:04:50Z

This is the first in a series of PRs which will onboard the KeyVault data plane commands. This PR:

Updates management plane commands to enable certificates
Adds majority of commands (and test scenarios) for keys and secrets
Includes first draft of the Python convenience layer (which does NOT include the authentication piece)

Note that the client was generated in autorest and will be removed in favor of direct import once the Python SDK is approved and released.

@yugangw-msft for the changes to profile and @derekbekoe for the changes to the management plane commands. Anyone feel free to drop feedback on the commands themselves. 😄

yugangw-msft · 2016-10-06T16:47:58Z

src/azure-cli-core/azure/cli/core/_profile.py

@@ -74,6 +81,7 @@ def __init__(self, storage=None, auth_ctx_factory=None):
        env = get_env()
        self._management_resource_uri = env[ENDPOINT_URLS.MANAGEMENT]
        self._graph_resource_uri = env[ENDPOINT_URLS.ACTIVE_DIRECTORY_GRAPH_RESOURCE_ID]
+        self._key_vault_resource_uri = env[ENDPOINT_URLS.KEY_VAULT]


i don't think we will need _graph_resource_uri and _key_vault_resource_uri, just remove

The rest change on Profile LGTM

tjprescott · 2016-10-06T16:51:07Z

keyvault group

Group
    az keyvault: Safeguard and maintain control of keys, secrets, and certificates.

Subgroups:
    certificate
    key
    secret

Commands:
    create       : Create a new Key Vault.
    delete       : Delete a Key Vault.
    delete-policy
    list         : List Key Vaults within a subscription or resource group.
    set-policy
    show         : Show details of a Key Vault.
    update       : Update properties of a Key Vault.

keyvault set-policy

Command
    az keyvault set-policy

Arguments
    --name -n      [Required]: Name of the key vault.
    --object-id              : A GUID that identifies the principal that will receive permissions.
    --resource-group -g      : Proceed only if Key Vault belongs to the specified resource group.
    --spn                    : Name of a service principal that will receive permissions.
    --upn                    : Name of a user principal that will receive permissions.

Permission Arguments
    --certificate-permissions: Space separated list. Possible values: all, get, list, delete,
                               create, import, update, managecontacts, getissuers, listissuers,
                               setissuers, deleteissuers, manageissuers.
    --key-permissions        : Space separated list. Possible values: all, encrypt, decrypt,
                               wrapKey, unwrapKey, sign, verify, get, list, create, update, import,
                               delete, backup, restore.
    --secret-permissions     : Space separated list. Possible values: all, get, list, set, delete.

keyvault key group

Group
    az keyvault key

Commands:
    create        : Creates a new, named, key in the specified vault.
    delete        : Deletes the specified key.
    list          : List keys in the specified vault.
    list-versions : List the versions of the specified key.
    set-attributes: Updates the Key Attributes associated with the specified key.
    show          : Retrieves the public portion of a key plus its attributes.

keyvault key create

Command
    az keyvault key create: Creates a new, named, key in the specified vault.

Arguments
    --name -n       [Required]: Name of the key.
    --protection -p [Required]: Specifies the type of key protection.  Allowed values: hsm,
                                software.
    --vault-name    [Required]: Name of the key vault.
    --disabled                : Create key in disabled state.
    --expires                 : Expiration UTC datetime  (Y-m-d'T'H:M'Z').
    --not-before              : Key not usable before the provided UTC datetime  (Y-m-d'T'H:M'Z').
    --ops                     : Space separated list of permitted JSON web key operations. Possible
                                values: encrypt, decrypt, sign, verify, wrapKey, unwrapKey.
    --size                    : The key size in bytes. e.g. 1024 or 2048.
    --tags                    : Space separated tags in 'key[=value]' format. Use "" to clear
                                existing tags.

keyvault key set-attributes

Command
    az keyvault key set-attributes: Updates the Key Attributes associated with the specified key.

Arguments
    --name -n    [Required]: Name of the key.
    --vault-name [Required]: Name of the key vault.
    --enabled              : Enable the key.  Allowed values: false, true.
    --expires              : Expiration UTC datetime  (Y-m-d'T'H:M'Z').
    --not-before           : Key not usable before the provided UTC datetime  (Y-m-d'T'H:M'Z').
    --ops                  : Space separated list of permitted JSON web key operations. Possible
                             values: encrypt, decrypt, sign, verify, wrapKey, unwrapKey.
    --tags                 : Space separated tags in 'key[=value]' format. Use "" to clear existing
                             tags.
    --version -v           : The key version. If omitted, uses the latest version.

keyvault secret group

Group
    az keyvault secret

Commands:
    delete        : Deletes a secret from the specified vault.
    list          : List secrets in the specified vault.
    list-versions : List the versions of the specified secret.
    set           : Sets a secret in the specified vault.
    set-attributes: Updates the attributes associated with the specified secret.
    show          : Gets a secret.

keyvault secret set

Command
    az keyvault secret set: Sets a secret in the specified vault.

Arguments
    --name -n    [Required]: Name of the secret.
    --value      [Required]: The value of the secret.
    --vault-name [Required]: Name of the key vault.
    --content-type         : Type of the secret value such as a password.
    --disabled             : Create secret in disabled state.
    --expires              : Expiration UTC datetime  (Y-m-d'T'H:M'Z').
    --not-before           : Key not usable before the provided UTC datetime  (Y-m-d'T'H:M'Z').
    --tags                 : Space separated tags in 'key[=value]' format. Use "" to clear existing
                             tags.

keyvault secret set-attributes

Command
    az keyvault secret set-attributes: Updates the attributes associated with the specified secret.

Arguments
    --name -n    [Required]: Name of the secret.
    --vault-name [Required]: Name of the key vault.
    --content-type         : Type of the secret value such as a password.
    --enabled              : Enable the secret.  Allowed values: false, true.
    --expires              : Expiration UTC datetime  (Y-m-d'T'H:M'Z').
    --not-before           : Key not usable before the provided UTC datetime  (Y-m-d'T'H:M'Z').
    --tags                 : Space separated tags in 'key[=value]' format. Use "" to clear existing
                             tags.
    --version -v           : The secret version. If omitted, uses the latest version.

tjprescott · 2016-10-06T17:02:56Z

Issue #1029 is preventing this from passing the CI. Please still review. 😅

tjprescott · 2016-10-06T17:37:08Z

cc/ @pomortaz for review and feedback (authentication will be incorporated in a later PR)

derekbekoe

Added a few different comments.

derekbekoe · 2016-10-06T18:28:25Z

mydomain.test.encrypted.pem

@@ -0,0 +1,34 @@
+-----BEGIN CERTIFICATE-----


This file is in the root directory.
Think it should be moved to src/command_modules/azure-cli-keyvault/tests or something like that if it's needed for a keyvault test.

It will eventually be moved there. Right now I'm using it for development of the key import command.

derekbekoe · 2016-10-06T18:29:03Z

mydomain.test.pem

@@ -0,0 +1,31 @@
+-----BEGIN CERTIFICATE-----


Same with this: think it should move from root project directory.

derekbekoe · 2016-10-06T18:29:50Z

policy.json

@@ -0,0 +1,42 @@
+{


Same with this: think it should move from root project directory.

derekbekoe · 2016-10-06T18:30:33Z

scripts/dev_setup.py

@@ -22,10 +22,10 @@ def exec_command(command):



My PR that just merged has this change so we should remove this change (and rebase) to prevent a conflict.

derekbekoe · 2016-10-06T18:32:28Z

src/azure-cli-core/azure/cli/core/_azure_env.py


 _environments = {
    ENV_DEFAULT: {
        ENDPOINT_URLS.MANAGEMENT: 'https://management.core.windows.net/',
        ENDPOINT_URLS.ACTIVE_DIRECTORY_AUTHORITY : 'https://login.microsoftonline.com',
-        ENDPOINT_URLS.ACTIVE_DIRECTORY_GRAPH_RESOURCE_ID: 'https://graph.windows.net/'
+        ENDPOINT_URLS.ACTIVE_DIRECTORY_GRAPH_RESOURCE_ID: 'https://graph.windows.net/',
+        ENDPOINT_URLS.KEY_VAULT: 'https://vault.azure.net'


@yugangw-msft Are you okay with having service endpoints defined in the azure.cli.core package like this?
You're more familiar with the endpoint urls so want to get your input.

This was where they already were defined. I added the endpoint for KeyVault. But this will need to change once the challenge piece is in the convenience layer. Also, I'm definitely not sure about the values I put in for the other hard-coded clouds (which really don't seem like they should be hard coded).

Yes, i am okay to have the service endpoints to defined here (in one single file), the same pattern used by xplat. When our profile design consolidated, we can do any appropriate change we like

derekbekoe · 2016-10-06T19:51:02Z

...vault/azure/cli/command_modules/keyvault/convenience/http_bearer_challenge_cache/__init__.py

+        raise ValueError('URL cannot be None')
+
+    if not challenge:
+        raise ValueError('challenge cannot be None')


really nitpicky but capital C for challenge like everywhere else?

derekbekoe · 2016-10-06T19:52:19Z

...odules/azure-cli-keyvault/azure/cli/command_modules/keyvault/convenience/key_vault_client.py

+
+class KeyVaultClient(object):
+
+    def __init__(self, credentials, api_version='2015-06-01', accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, filepath=None):


Will the client work with multiple API versions? (I'm thinking of the multi-cloud story)

Ultimately, this will be packaged with the data plane SDK, so it will be revved and versioned along with the SDK.

derekbekoe · 2016-10-06T20:14:06Z

...es/azure-cli-keyvault/azure/cli/command_modules/keyvault/convenience/key_vault_credential.py

+from . import httpbearerchallenge
+from . import httpbearerchallengecache as challengecache
+
+class keyvaultcredential(authentication):


KeyVaultCredential? - https://www.python.org/dev/peps/pep-0008/#class-names

It looks like I must have pressed a funky shortcut in VS because it seems to have made this file all lowercase. Currently it's not used by anything (though it may be used in the convenience layer auth later on).

derekbekoe · 2016-10-06T20:17:59Z

...es/azure-cli-keyvault/azure/cli/command_modules/keyvault/convenience/key_vault_credential.py

+
+    def signed_session(self):
+        session = super(keyvaultcredential, self).signed_session()
+        session.headers['authorization'] = 'bearer {}'.format(self._token or '')


Whilst HTTP headers are case-insensitive, I'd recommend 'Authorization' and 'Bearer'.
It could/potentially/maybe cause an obscure issue in the future.
Here they use the capitalized form: https://msdn.microsoft.com/en-us/library/azure/dn903615.aspx

Fixed. 'Twas the lowercase shortcut key.

derekbekoe · 2016-10-06T20:18:52Z

...es/azure-cli-keyvault/azure/cli/command_modules/keyvault/convenience/key_vault_credential.py

+    def signed_session(self):
+        session = super(keyvaultcredential, self).signed_session()
+        session.headers['authorization'] = 'bearer {}'.format(self._token or '')
+        print('signed session w/ auth: {}'.format(session.headers['authorization']))


Shouldn't print to stdout here.
Do you need to print the Authorization header? If so, consider using logging.

No, this was part of debugging. I'll remove it.

tjprescott · 2016-10-07T14:55:07Z

src/azure-cli-core/azure/cli/core/_azure_env.py

-        ENDPOINT_URLS.ACTIVE_DIRECTORY_GRAPH_RESOURCE_ID: 'https://graph.chinacloudapi.cn/'
+        ENDPOINT_URLS.ACTIVE_DIRECTORY_GRAPH_RESOURCE_ID: 'https://graph.chinacloudapi.cn/',
+        # TODO: Verify KeyVault endpoint
+        ENDPOINT_URLS.KEY_VAULT: 'https://vault.chinacloudapi.cn'


@yugangw-msft how do I verify this URL?

Just cross check from xplat environment.js where keyvault team put in the endpoints themselves. If still missing, please ask @karlaugsten

karlaugsten · 2016-10-07T17:21:12Z

@tjprescott the endpoint for KeyVault in china is https://vault.azure.cn

…re-cli into KeyVaultDataPlane

derekbekoe · 2016-10-07T23:49:45Z

@tjprescott: I have worked out why your recent build failed - https://travis-ci.org/Azure/azure-cli/jobs/165952645.

In src/command_modules/azure-cli-keyvault/setup.py, you should list the packages you want to be included in the packages array in the setup(...) function.
You are getting the error ImportError: No module named 'azure.cli.command_modules.keyvault.keyvaultclient' because azure.cli.command_modules.keyvault.keyvaultclient is not in this list of packages.
There may be others if you added other new packages as well.

When I list azure/cli/command_modules/keyvault after the package_verify that the CI runs, I don't see a keyvaultclient directory and that's because of the above.

root@230334cde9c1:/usr/local/lib/python3.5/site-packages/azure/cli/command_modules/keyvault# ls -l
total 56
-rw-r--r-- 1 root staff   632 Oct  7 23:43 __init__.py
drwxr-sr-x 2 root staff  4096 Oct  7 23:45 __pycache__
-rw-r--r-- 1 root staff  3447 Oct  7 23:43 _command_type.py
-rw-r--r-- 1 root staff  1236 Oct  7 23:43 _help.py
-rw-r--r-- 1 root staff  7832 Oct  7 23:43 _params.py
-rw-r--r-- 1 root staff  4431 Oct  7 23:43 _validators.py
-rw-r--r-- 1 root staff 14423 Oct  7 23:43 custom.py
-rw-r--r-- 1 root staff  6038 Oct  7 23:43 generated.py

See this other setup.py file as an example - https://github.com/Azure/azure-cli/blob/master/src/command_modules/azure-cli-network/setup.py#L51.

tjprescott · 2016-10-08T15:14:49Z

@derekbekoe you're a genius! Thank you 😄

…re-cli into KeyVaultDataPlane # Conflicts: # src/command_modules/azure-cli-keyvault/setup.py

tjprescott added New Command(s) KeyVault az keyvault labels Oct 6, 2016

tjprescott added this to the Sprint 4 milestone Oct 6, 2016

tjprescott assigned yugangw-msft and derekbekoe Oct 6, 2016

azurecla added the cla-not-required label Oct 6, 2016

yugangw-msft reviewed Oct 6, 2016

View reviewed changes

derekbekoe reviewed Oct 6, 2016

View reviewed changes

tjprescott added 3 commits October 7, 2016 07:45

Initial KeyVault data plane commands and management plane revisions.

fd358b2

Code review comments.

a87966f

Bump to wake up CI.

b1aa0d2

tjprescott commented Oct 7, 2016

View reviewed changes

Bump to wake up CI.

2a9f408

tjprescott added 2 commits October 7, 2016 15:26

Fix imports.

744f7db

Merge branch 'KeyVaultDataPlane' of https://github.com/tjprescott/azu…

23a7282

…re-cli into KeyVaultDataPlane

tjprescott added 4 commits October 10, 2016 10:44

Updates to setup.py

784a803

Updates to setup.py

03ca133

Merge branch 'KeyVaultDataPlane' of https://github.com/tjprescott/azu…

7422121

…re-cli into KeyVaultDataPlane # Conflicts: # src/command_modules/azure-cli-keyvault/setup.py

Update vault_base_url

1974ebf

tjprescott merged commit d72cfaa into Azure:master Oct 10, 2016

tjprescott deleted the KeyVaultDataPlane branch October 10, 2016 20:20

johanste mentioned this pull request Nov 5, 2016

Argument validation errors (custom validators) do not show the correct error message #1254

Closed

tjprescott mentioned this pull request Nov 8, 2016

Ensure '--ids' validation failure prints usage string #1271

Merged

haroldrandom added the cla-not-required label Oct 25, 2019

haroldrandom added KeyVault az keyvault New Command (PR) labels Oct 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KeyVault] Onboard KeyVault Round 1 #1037

[KeyVault] Onboard KeyVault Round 1 #1037

tjprescott commented Oct 6, 2016

yugangw-msft Oct 6, 2016

tjprescott Oct 7, 2016

tjprescott commented Oct 6, 2016

tjprescott commented Oct 6, 2016 •

edited

Loading

tjprescott commented Oct 6, 2016

derekbekoe left a comment

derekbekoe Oct 6, 2016

tjprescott Oct 6, 2016

derekbekoe Oct 6, 2016

derekbekoe Oct 6, 2016

derekbekoe Oct 6, 2016

tjprescott Oct 6, 2016

derekbekoe Oct 6, 2016

tjprescott Oct 6, 2016 •

edited

Loading

yugangw-msft Oct 7, 2016

derekbekoe Oct 6, 2016

derekbekoe Oct 6, 2016

tjprescott Oct 6, 2016

derekbekoe Oct 6, 2016

tjprescott Oct 6, 2016

derekbekoe Oct 6, 2016

tjprescott Oct 6, 2016

derekbekoe Oct 6, 2016

tjprescott Oct 6, 2016

tjprescott Oct 7, 2016

yugangw-msft Oct 7, 2016

karlaugsten commented Oct 7, 2016

derekbekoe commented Oct 7, 2016 •

edited

Loading

tjprescott commented Oct 8, 2016


		class KeyVaultClient(object):

		def __init__(self, credentials, api_version='2015-06-01', accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, filepath=None):

[KeyVault] Onboard KeyVault Round 1 #1037

[KeyVault] Onboard KeyVault Round 1 #1037

Conversation

tjprescott commented Oct 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjprescott commented Oct 6, 2016

tjprescott commented Oct 6, 2016 • edited Loading

tjprescott commented Oct 6, 2016

derekbekoe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjprescott Oct 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlaugsten commented Oct 7, 2016

derekbekoe commented Oct 7, 2016 • edited Loading

tjprescott commented Oct 8, 2016

tjprescott commented Oct 6, 2016 •

edited

Loading

tjprescott Oct 6, 2016 •

edited

Loading

derekbekoe commented Oct 7, 2016 •

edited

Loading