Skip to content

[Storage] Support user assigned identity for storage account#16613

Merged
Juliehzl merged 10 commits intoAzure:devfrom
Juliehzl:user-assigned-identity
Apr 26, 2021
Merged

[Storage] Support user assigned identity for storage account#16613
Juliehzl merged 10 commits intoAzure:devfrom
Juliehzl:user-assigned-identity

Conversation

@Juliehzl
Copy link
Copy Markdown
Contributor

@Juliehzl Juliehzl commented Jan 21, 2021

Description

See https://microsoft-my.sharepoint.com/:w:/p/ozgun/EYc_gSQnco5LkPJI4MYSidsBPSBDRLXpQfFyQtjtXPwsyQ?e=LvWsGk&CID=BFBDB6A6-36D8-4992-81C3-864729996585&wdLOR=cD2686608-0966-4F97-8754-B02974231DAE

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


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

@Juliehzl Juliehzl added the Need SDK released Pending for sdk release label Jan 21, 2021
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Jan 21, 2021

Storage

@yonzhan yonzhan added this to the S182 milestone Jan 21, 2021
@yonzhan yonzhan modified the milestones: S182, S183 Feb 6, 2021
@yonzhan yonzhan modified the milestones: S183 - For Ignite, S184 Feb 26, 2021
@Juliehzl Juliehzl removed the Need SDK released Pending for sdk release label Mar 11, 2021
@yonzhan yonzhan modified the milestones: S184, S185 Mar 20, 2021
@Juliehzl Juliehzl force-pushed the user-assigned-identity branch from 74ecab9 to bad3b3f Compare March 29, 2021 07:15
@yonzhan yonzhan modified the milestones: S185, S186 Apr 10, 2021
@Juliehzl Juliehzl force-pushed the user-assigned-identity branch from 65de846 to f21fc0f Compare April 23, 2021 07:09
@Juliehzl Juliehzl marked this pull request as ready for review April 23, 2021 07:11
@Juliehzl Juliehzl force-pushed the user-assigned-identity branch from 10d8851 to 77a0871 Compare April 23, 2021 07:42
Comment on lines +84 to +89
if params.encryption.key_source and params.encryption.key_source == "Microsoft.Keyvault":
if params.encryption.key_vault_properties is None:
KeyVaultProperties = cmd.get_models('KeyVaultProperties')
params.encryption.key_vault_properties = KeyVaultProperties(key_name=encryption_key_name,
key_vault_uri=encryption_key_vault,
key_version=encryption_key_version)
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.

Shall we add

    else:
        if any([encryption_key_name, encryption_key_vault, encryption_key_version]):
            raise ValueError(
                'Specify `--encryption-key-source=Microsoft.Keyvault` to configure key vault properties.')

like what update_storage_account does

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually it is by design.
In create scenario, we already have validator for input value as you can see below

def validate_encryption_source(namespace):
if namespace.encryption_key_source == 'Microsoft.Keyvault' and \
not (namespace.encryption_key_name and namespace.encryption_key_vault):
raise ValueError('--encryption-key-name and --encryption-key-vault are required '
'when --encryption-key-source=Microsoft.Keyvault is specified.')
if namespace.encryption_key_name or namespace.encryption_key_version is not None or namespace.encryption_key_vault:
if namespace.encryption_key_source and namespace.encryption_key_source != 'Microsoft.Keyvault':
raise ValueError('--encryption-key-name, --encryption-key-vault, and --encryption-key-version are not '
'applicable without Microsoft.Keyvault key-source.')

But in update scenario, we need to consider existing parameter values, so we add an addition check here.

@Juliehzl Juliehzl merged commit e448cb7 into Azure:dev Apr 26, 2021
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.

3 participants