[AAD] `az ad ds create/update/list/show/delete`: Support DomainService #2842

evelyn-ys · 2020-12-28T09:23:31Z

New command module: az ad ds support management for AAD.DomainService
Commands:

az ad ds create
az ad ds update
az ad ds list
az ad ds show
az ad ds delete

reference: AAD RP Speclet

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

General Guidelines

Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

My extension description/summary conforms to the Extension Summary Guidelines.

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

yonzhan · 2020-12-28T09:36:52Z

AAD

jiasli · 2021-01-05T03:00:32Z

src/ad/README.md

+az ad ds create --domain "TestDomainService.com" --ntlm-v1 "Enabled" --sync-ntlm-passwords "Enabled" \
+    --tls-v1 "Disabled" --filtered-sync "Enabled" --external-access "Enabled" --ldaps "Enabled" \
+    --pfx-certificate "MIIDPDCCAiSgAwIBAgIQQUI9P6tq2p9OFIJa7DLNvTANBgkqhkiG9w0BAQsFADAgMR4w..." \
+    --pfx-certificate-password "<pfxCertificatePassword>" \
+    --additional-recipients "jicha@microsoft.com" "caalmont@microsoft.com" --notify-dc-admins "Enabled" \
+    --notify-global-admins "Enabled" \
+    --replica-sets location="West US" subnet-id="/subscriptions/1639790a-76a2-4ac4-98d9-8562f5dfcb4d/resourceGroups/TestNetworkResourceGroup/providers/Microsoft.Network/virtualNetworks/TestVnetWUS/subnets/TestSubnetWUS" \
+    --name "TestDomainService.com" --resource-group "TestResourceGroup" 


Same as #2817 (comment)

Backward slash \ is only supported by Linux Bash, we usually don't include \ in examples.

However, you can break lines without \ and add a comment to tell the user line breaks are added for legibility only, like

https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-auth-code-flow#request-an-authorization-code

// Line breaks for legibility only https://login.microsoftonline.com/{tenant}/oauth2/v2.0/authorize? client_id=6731de76-14a6-49ae-97bc-6eba6914391e &response_type=code &redirect_uri=http%3A%2F%2Flocalhost%2Fmyapp%2F &response_mode=query &scope=openid%20offline_access%20https%3A%2F%2Fgraph.microsoft.com%2Fmail.read &state=12345 &code_challenge=YTFjNjI1OWYzMzA3MTI4ZDY2Njg5M2RkNmVjNDE5YmEyZGRhOGYyM2IzNjdmZWFhMTQ1ODg3NDcxY2Nl &code_challenge_method=S256

So this command can be written as

az ad ds create --domain "TestDomainService.com" --ntlm-v1 "Enabled" --sync-ntlm-passwords "Enabled" --tls-v1 "Disabled" --filtered-sync "Enabled" --external-access "Enabled" --ldaps "Enabled" ...

Auto-generated by CodeGen

jiasli · 2021-01-05T03:01:40Z

src/ad/README.md

+```
+
+### Included Features ###
+#### ad ds ####


We can explain what ds is.

Auto-generated by CodeGen

jiasli · 2021-01-05T03:04:45Z

src/ad/azext_ad/generated/_help.py

+helps['ad ds'] = """
+    type: group
+    short-summary: Manage domain service with ad
+"""


These are redundant indents, against CLI convention, like

https://github.com/Azure/azure-cli/blob/84e0e45de5f99936ca5b933493fd6234dbf648ae/src/azure-cli/azure/cli/command_modules/vm/_help.py#L65-L73

helps['disk delete'] = """ type: command short-summary: Delete a managed disk. examples: - name: Delete a managed disk. (autogenerated) text: | az disk delete --name MyManagedDisk --resource-group MyResourceGroup crafted: true """

Suggested change

helps['ad ds'] = """

type: group

short-summary: Manage domain service with ad

"""

helps['ad ds'] = """

type: group

short-summary: Manage domain service with ad

"""

Auto-generated by CodeGen

Refined in manual/_help.py

jiasli · 2021-01-05T03:05:16Z

src/ad/azext_ad/generated/_help.py

+
+helps['ad ds'] = """
+    type: group
+    short-summary: Manage domain service with ad


The short-summary should usually ends with a period (.), like

https://github.com/Azure/azure-cli/blob/84e0e45de5f99936ca5b933493fd6234dbf648ae/src/azure-cli/azure/cli/command_modules/vm/_help.py#L67

short-summary: Delete a managed disk.

Auto-generated by CodeGen

the description is not easy to understand with ad.

Refined in manual/_help.py

jiasli · 2021-01-05T03:12:43Z

src/ad/azext_ad/tests/__init__.py

+FAILED = "failed"
+
+
+def try_manual(func):


The meaning of this try_manual is unclear to me as someone who is not familiar with the code. Also, shouldn't it be a general method that appears in the centralized test framework, instead of individual modules/extensions?

Auto-generated by CodeGen aimed to use manual code overwrite generated code

jiasli · 2021-01-05T03:15:44Z

src/ad/azext_ad/tests/latest/preparers.py

+KEY_VNET_NIC = 'nic'
+
+
+class VirtualNetworkPreparer(NoTrafficRecordingPreparer, SingleValueReplacer):


Not used? I can't seem to find any usage of VirtualNetworkPreparer.

Auto-generated by CodeGen. I didn't use it but keep it because it will always be generated. No need to delete every time.

@changlong-liu could we remove these preparer and import from azure cli core module?

yes @Juliehzl , we are refactoring the test preparers codegen.

Hi @evelyn-ys , In latest autorest.az the preparers.py is supposed to be generated only when has some reference for it. Please help to send me the swagger branch to have further check if it generated even have no usage.

@changlong-liu Hope this stable swagger can help

Juliehzl

Current az ad command group has description Manage Azure Active Directory Graph entities needed for Role Based Access Control. It seems that your command group is not for az ad from the description.

Juliehzl · 2021-01-15T03:48:17Z

src/ad/azext_ad/azext_metadata.json

@@ -0,0 +1,3 @@
+{
+    "azext.minCliCoreVersion": "2.15.0"


not in preview or experimental?

az ad is stable. az ad ds is marked as preview in command.py

src/ad/azext_ad/generated/_help.py

Juliehzl · 2021-01-15T03:51:36Z

src/ad/azext_ad/generated/_help.py

+            location: Virtual network location
+            subnet-id: The name of the virtual network that Domain Services will be deployed on. The id of the subnet \


both of them are required?

I see. You could consider add [required] in the description for them. But should the location be the location for subnet?

src/ad/azext_ad/generated/_help.py

evelyn-ys · 2021-01-28T05:15:46Z

Current az ad command group has description Manage Azure Active Directory Graph entities needed for Role Based Access Control. It seems that your command group is not for az ad from the description.

Refined in manual/_help.py

evelyn-ys · 2021-02-03T06:07:07Z

@Juliehzl I have modified according to your comments, could you pls review again? Thanks a lot!

src/ad/azext_ad/manual/custom.py

src/ad/azext_ad/tests/latest/test_ad_scenario.py

jiasli · 2024-01-05T12:51:02Z

Current az ad command group has description Manage Azure Active Directory Graph entities needed for Role Based Access Control. It seems that your command group is not for az ad from the description.

Refined in manual/_help.py

Even though the help message has been updated:

azure-cli-extensions/src/ad/azext_ad/manual/_help.py

Lines 14 to 17 in 2415c3b

    
           helps['ad'] = """ 
        
           type: group 
        
           short-summary: Manage Azure Active Directory related entities and resources. 
        
           """

this design of putting az ad ds under az ds is still against the purpose of az ad which is designed to be a pure data-plane command group for calling AD Graph/Microsoft Graph. Because of this, in main Azure CLI, the global argument _subscription is ignored for az ad:

https://github.com/Azure/azure-cli/blob/e8efb791ea2f0ba5e4f172b26f75725fd8aa8079/src/azure-cli/azure/cli/command_modules/role/_params.py#L25-L26

    with self.argument_context('ad') as c:
        c.ignore('_subscription')  # hide global subscription param

However, apparently, az ad ds is a mgmt-plane command group calling Microsoft.AAD Resource Provider:

azure-cli-extensions/src/ad/azext_ad/vendored_sdks/domainservices/operations/_domain_services_operations.py

Line 118 in 2415c3b

    
           list.metadata = {'url': '/subscriptions/{subscriptionId}/providers/Microsoft.AAD/domainServices'}  # type: ignore

This means the global argument _subscription shouldn't be ignored for az ad ds.

If a specific command such as az ad ds -h is run, the core is smart enough to filter the command loader based on the command name and only load what is needed:

https://github.com/Azure/azure-cli/blob/7a4d03f077151be8fc312c9ce82febd1dda118cb/src/azure-cli-core/azure/cli/core/__init__.py#L499

            command_loaders = self.cmd_to_loader_map.get(command, None)

In such case, only azext_ad.DomainServicesResourceProviderCommandsLoader is loaded and azure.cli.command_modules.role.RoleCommandsLoader will be skipped. _subscription won't be ignored and will still appear in the in-tool help.

However, in the reference doc generation pipeline, both command loaders will be loaded. If azure.cli.command_modules.role.RoleCommandsLoader is load first, when azext_ad.DomainServicesResourceProviderCommandsLoader is loaded, _subscription will be ignored for az ad ds.

See Azure/azure-cli#27296 (comment) for details.

evelyn-ys added 5 commits December 23, 2020 18:03

domain service

8963fc5

fix tests

d16a8c1

linter

9d75f47

test check

2c376df

tests recorded

b5cb147

evelyn-ys self-assigned this Dec 28, 2020

evelyn-ys requested review from Juliehzl, jsntcy and jiasli December 28, 2020 09:24

add codeowner

dedb6c0

yonzhan added this to the S180 milestone Dec 28, 2020

evelyn-ys changed the title ~~[AAD] DomainService support~~ [AAD] az ad ds create/update/list/show/delete: Support DomainService Dec 28, 2020

evelyn-ys added 2 commits January 4, 2021 10:08

Merge branch 'master' into domainservice

29615b6

code owner

84537e4

jiasli reviewed Jan 5, 2021

View reviewed changes

Merge branch 'master' into domainservice

9811af7

yonzhan modified the milestones: S180, S181 Jan 6, 2021

Juliehzl reviewed Jan 15, 2021

View reviewed changes

src/ad/azext_ad/generated/_help.py Show resolved Hide resolved

Juliehzl reviewed Jan 15, 2021

View reviewed changes

src/ad/azext_ad/generated/_help.py Show resolved Hide resolved

Juliehzl reviewed Jan 15, 2021

View reviewed changes

src/ad/azext_ad/generated/_help.py Show resolved Hide resolved

Juliehzl reviewed Jan 15, 2021

View reviewed changes

src/ad/azext_ad/generated/_help.py Show resolved Hide resolved

resolve conflicts

0f1e0d1

yonzhan modified the milestones: S181, S182 Jan 26, 2021

evelyn-ys added 4 commits January 27, 2021 13:44

refine help

2d41ffb

linter

18e7598

change complicated param to json

bab6639

json format

b9e75f2

evelyn-ys requested a review from Juliehzl January 28, 2021 05:19

Juliehzl reviewed Feb 3, 2021

View reviewed changes

src/ad/azext_ad/manual/custom.py Outdated Show resolved Hide resolved

Juliehzl reviewed Feb 3, 2021

View reviewed changes

src/ad/azext_ad/tests/latest/test_ad_scenario.py Show resolved Hide resolved

evelyn-ys added 2 commits February 4, 2021 10:39

add validators

3f4355a

linter

6294971

Juliehzl approved these changes Feb 4, 2021

View reviewed changes

evelyn-ys merged commit 2415c3b into Azure:master Feb 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AAD] `az ad ds create/update/list/show/delete`: Support DomainService #2842

[AAD] `az ad ds create/update/list/show/delete`: Support DomainService #2842

evelyn-ys commented Dec 28, 2020 •

edited

Loading

yonzhan commented Dec 28, 2020

jiasli Jan 5, 2021

evelyn-ys Jan 5, 2021

jiasli Jan 5, 2021

evelyn-ys Jan 5, 2021

jiasli Jan 5, 2021 •

edited

Loading

evelyn-ys Jan 5, 2021

evelyn-ys Jan 28, 2021

jiasli Jan 5, 2021 •

edited

Loading

evelyn-ys Jan 5, 2021

Juliehzl Jan 15, 2021

evelyn-ys Jan 28, 2021

jiasli Jan 5, 2021 •

edited

Loading

evelyn-ys Jan 5, 2021 •

edited

Loading

jiasli Jan 5, 2021

evelyn-ys Jan 5, 2021

Juliehzl Feb 3, 2021

changlong-liu Feb 4, 2021

evelyn-ys Feb 4, 2021

Juliehzl left a comment

Juliehzl Jan 15, 2021

evelyn-ys Jan 19, 2021

Juliehzl Jan 15, 2021

evelyn-ys Jan 28, 2021

Juliehzl Feb 3, 2021 •

edited

Loading

evelyn-ys commented Jan 28, 2021

evelyn-ys commented Feb 3, 2021

jiasli commented Jan 5, 2024

		KEY_VNET_NIC = 'nic'


		class VirtualNetworkPreparer(NoTrafficRecordingPreparer, SingleValueReplacer):

		location: Virtual network location
		subnet-id: The name of the virtual network that Domain Services will be deployed on. The id of the subnet \

		FAILED = "failed"


		def try_manual(func):

[AAD] az ad ds create/update/list/show/delete: Support DomainService #2842

[AAD] az ad ds create/update/list/show/delete: Support DomainService #2842

Conversation

evelyn-ys commented Dec 28, 2020 • edited Loading

General Guidelines

About Extension Publish

yonzhan commented Dec 28, 2020

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

jiasli Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiasli Jan 5, 2021 • 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

jiasli Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

evelyn-ys Jan 5, 2021 • 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

Juliehzl 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

Juliehzl Feb 3, 2021 • edited Loading

Choose a reason for hiding this comment

evelyn-ys commented Jan 28, 2021

evelyn-ys commented Feb 3, 2021

jiasli commented Jan 5, 2024

[AAD] `az ad ds create/update/list/show/delete`: Support DomainService #2842

[AAD] `az ad ds create/update/list/show/delete`: Support DomainService #2842

evelyn-ys commented Dec 28, 2020 •

edited

Loading

jiasli Jan 5, 2021 •

edited

Loading

jiasli Jan 5, 2021 •

edited

Loading

jiasli Jan 5, 2021 •

edited

Loading

evelyn-ys Jan 5, 2021 •

edited

Loading

Juliehzl Feb 3, 2021 •

edited

Loading