[KeyVault] az keyvault create: support `--network-acls`, `--network-acls-ips` and `--network-acls-vnets` for specifying network rules while creating vault #12716

bim-msft · 2020-03-23T12:58:41Z

Support for AB#1150

Usage:

Use JSON string directly, for the vnet field, {vnet_name}/{subnet_name} and {subnet_id} are both supported:

az keyvault create --network-acls "{
     \"ip\":  [\"1.2.3.4\", \"2.3.4.0/24\"], 
     \"vnet\": [
           \"vnet_name_1/subnet_name1\", 
           \"/subscriptions/000000-0000-0000/resourceGroups/MyResourceGroup/providers/Microsoft.Network/virtualNetworks/vnet2/subnets/subnet2\"
     ]
}"

Use JSON file:

az keyvault create --network-acls network-acls-example.json

Use flattened parameters (IP rule):

az keyvault create --network-acls-ips 1.2.3.4 2.3.4.0/24

Virtual network rule:

az keyvault create --network-acls-vnets vnet_name_1/subnet_name1 /subscriptions/000000-0000-0000/resourceGroups/MyResourceGroup/providers/Microsoft.Network/virtualNetworks/vnet2/subnets/subnet2

Also, the three parameters --network-acls, --network-acls-ips and --network-acls-vnets are all optional and they can be used together:

az keyvault create --network-acls ... --network-acls-ips ... --network-acls-vnets ...

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

The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.

yungezz · 2020-03-23T13:02:56Z

src/azure-cli/azure/cli/command_modules/keyvault/custom.py

+            network_acls_json = json.load(f)
+
+    try:
+        network_acls_json = json.loads(json.dumps(network_acls_json))


why load again?

why load again?

@yungezz Good point, actually I added this sentence in order to make sure the input is a valid JSON. But later I modified the code that makes this part useless... I will remove this part.

src/azure-cli/azure/cli/command_modules/keyvault/custom.py

yungezz · 2020-03-23T13:05:22Z

src/azure-cli/azure/cli/command_modules/keyvault/custom.py

+    for vnet_rule in network_acls_json.get('vnet', []):
+        items = vnet_rule.split('/')
+        if len(items) != 2:
+            raise CLIError('Invalid VNet rule: {}. Format: {{vnet}}/{{subnet}}'.format(vnet_rule))


can user input resource id directly?

can user input resource id directly?

@yungezz It should be supported, I will update the code.

src/azure-cli/azure/cli/command_modules/keyvault/_params.py

yonzhan · 2020-03-23T23:13:10Z

keyvault

yungezz

LGTM, one comment left

yungezz · 2020-03-24T09:16:21Z

src/azure-cli/azure/cli/command_modules/keyvault/_help.py

+        az keyvault create --location westus2 --name MyKeyVault --resource-group MyResourceGroup --network-acls network-acls-example.json
+  - name: Create a key vault with network ACLs specified (use --network-acls, --network-acls-ips and --network-acls-vnets at the same time).
+    text: |
+        az keyvault create --location westus2 --name MyKeyVault --resource-group MyResourceGroup --network-acls network-acls-example.json --network-acls-ips 3.4.5.0/24 4.5.6.0/24 --network-acls-vnets vnet1/subnet1 vnet2/subnet2 /subscriptions/000000-0000-0000/resourceGroups/MyResourceGroup/providers/Microsoft.Network/virtualNetworks/vnet3/subnets/subnet3


thanks @bim-msft for quick change. Should we fail validation when json and flatten params from user at same time?

thanks @bim-msft for quick change. Should we fail validation when json and flatten params from user at same time?

@yungezz I think it's safe to pass validation for this case which all three params are specified at the same time. As far as I can see, there is one scenario may benefits from this: user has one basic network rules json file, and this file is always applied to a new created vault, also there are different additional rules for each new vault.

:) I would think user would like to put all configure rules in one place either one json file or params value. anyway, the validation could be added later after get some feedback

fengzhou-msft · 2020-03-25T05:37:31Z

src/azure-cli/azure/cli/command_modules/keyvault/_help.py

+
+  - name: Create a key vault with network ACLs specified (use --network-acls to specify IP and VNet rules by using a JSON file).
+    text: |
+        az keyvault create --location westus2 --name MyKeyVault --resource-group MyResourceGroup --network-acls network-acls-example.json


No @ prefix for file?

No @ prefix for file?

@fengzhou-msft Is @ a convention? I think in this case users don't need to specify the marker explicitly, the program can detect whether it's a JSON string or a JSON filename.

It seems json str or @file.json is a convention, and type=validate_file_or_dict can be used. @Juliehzl @myronfanqiu any guideline for this?

Tested with validate_file_or_dict, it can support json string and file path with or without @ prefix.

@fengzhou-msft That's good, I will use type=validate_file_or_dict to refine the code.

@fengzhou-msft For the help messages and examples, I decided not to add @ as a prefix since this is not mandatory and may cause confusion.

bim-msft requested a review from fengzhou-msft as a code owner March 23, 2020 12:58

bim-msft requested review from yungezz, qwordy and arrownj March 23, 2020 12:58

yungezz reviewed Mar 23, 2020

View reviewed changes

yonzhan assigned bim-msft Mar 23, 2020

bim-msft added 4 commits March 24, 2020 14:09

code finished

3133adf

Updated

dcc3278

Remove useless code

7e0ee02

Support flatten params

f00c115

bim-msft force-pushed the bim_kv_networkrule branch from 2ecfd43 to f00c115 Compare March 24, 2020 08:44

Fix linter errors

15d9ccb

yungezz reviewed Mar 24, 2020

View reviewed changes

bim-msft added 2 commits March 25, 2020 12:18

Refine examples

c4f18ca

Fix linter error

63d7514

yungezz approved these changes Mar 25, 2020

View reviewed changes

fengzhou-msft reviewed Mar 25, 2020

View reviewed changes

bim-msft added 3 commits March 25, 2020 13:57

Fix linter error

2dba5c3

Fix linter error

609f256

Use 'validate_file_or_dict'

90098ad

bim-msft requested a review from fengzhou-msft March 25, 2020 09:45

fengzhou-msft approved these changes Mar 25, 2020

View reviewed changes

bim-msft merged commit bfb3067 into Azure:dev Mar 25, 2020

bim-msft deleted the bim_kv_networkrule branch March 25, 2020 10:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KeyVault] az keyvault create: support `--network-acls`, `--network-acls-ips` and `--network-acls-vnets` for specifying network rules while creating vault #12716

[KeyVault] az keyvault create: support `--network-acls`, `--network-acls-ips` and `--network-acls-vnets` for specifying network rules while creating vault #12716

bim-msft commented Mar 23, 2020 •

edited

Loading

yungezz Mar 23, 2020

bim-msft Mar 24, 2020

yungezz Mar 23, 2020

bim-msft Mar 24, 2020

yonzhan commented Mar 23, 2020

yungezz left a comment

yungezz Mar 24, 2020

bim-msft Mar 24, 2020

yungezz Mar 25, 2020 •

edited

Loading

fengzhou-msft Mar 25, 2020

bim-msft Mar 25, 2020

fengzhou-msft Mar 25, 2020 •

edited

Loading

fengzhou-msft Mar 25, 2020

bim-msft Mar 25, 2020

bim-msft Mar 25, 2020

[KeyVault] az keyvault create: support --network-acls, --network-acls-ips and --network-acls-vnets for specifying network rules while creating vault #12716

[KeyVault] az keyvault create: support --network-acls, --network-acls-ips and --network-acls-vnets for specifying network rules while creating vault #12716

Conversation

bim-msft commented Mar 23, 2020 • 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

yonzhan commented Mar 23, 2020

yungezz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yungezz Mar 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fengzhou-msft Mar 25, 2020 • 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

[KeyVault] az keyvault create: support `--network-acls`, `--network-acls-ips` and `--network-acls-vnets` for specifying network rules while creating vault #12716

[KeyVault] az keyvault create: support `--network-acls`, `--network-acls-ips` and `--network-acls-vnets` for specifying network rules while creating vault #12716

bim-msft commented Mar 23, 2020 •

edited

Loading

yungezz Mar 25, 2020 •

edited

Loading

fengzhou-msft Mar 25, 2020 •

edited

Loading