Skip to content

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

Merged
Yishi Wang (evelyn-ys) merged 16 commits into
Azure:masterfrom
evelyn-ys:domainservice
Feb 4, 2021
Merged

Yishi Wang (evelyn-ys) merged 16 commits into
Azure:masterfrom
evelyn-ys:domainservice

Conversation

@evelyn-ys

@evelyn-ys Yishi Wang (evelyn-ys) commented Dec 28, 2020

Copy link
Copy Markdown
Contributor

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:

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 Yong Zhang (yonzhan) added this to the S180 milestone Dec 28, 2020
@yonzhan

Copy link
Copy Markdown
Collaborator

AAD

@evelyn-ys Yishi Wang (evelyn-ys) changed the title [AAD] DomainService support [AAD] az ad ds create/update/list/show/delete: Support DomainService Dec 28, 2020
Comment thread src/ad/README.md
Comment on lines +14 to +21
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"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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"
                ...

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.

Auto-generated by CodeGen

Comment thread src/ad/README.md
```

### Included Features ###
#### ad ds ####

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can explain what ds is.

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.

Auto-generated by CodeGen

Comment on lines +15 to +18
helps['ad ds'] = """
type: group
short-summary: Manage domain service with ad
"""

@jiasli Jiashuo Li (jiasli) Jan 5, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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
"""

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.

Auto-generated by CodeGen

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.

Refined in manual/_help.py


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

@jiasli Jiashuo Li (jiasli) Jan 5, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

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.

Auto-generated by CodeGen

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the description is not easy to understand with ad.

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.

Refined in manual/_help.py

FAILED = "failed"


def try_manual(func):

@jiasli Jiashuo Li (jiasli) Jan 5, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

@evelyn-ys Yishi Wang (evelyn-ys) Jan 5, 2021

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.

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

KEY_VNET_NIC = 'nic'


class VirtualNetworkPreparer(NoTrafficRecordingPreparer, SingleValueReplacer):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

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.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

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.

yes Zunli Hu (@Juliehzl) , we are refactoring the test preparers codegen.

Hi Yishi Wang (@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.

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.

@yonzhan Yong Zhang (yonzhan) modified the milestones: S180, S181 Jan 6, 2021

@Juliehzl Zunli Hu (Juliehzl) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not in preview or experimental?

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.

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

Comment thread src/ad/azext_ad/generated/_help.py
Comment thread src/ad/azext_ad/generated/_help.py
Comment on lines +54 to +55
location: Virtual network location
subnet-id: The name of the virtual network that Domain Services will be deployed on. The id of the subnet \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

both of them are required?

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.

Yes

@Juliehzl Zunli Hu (Juliehzl) Feb 3, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread src/ad/azext_ad/generated/_help.py
Comment thread src/ad/azext_ad/generated/_help.py
@yonzhan Yong Zhang (yonzhan) modified the milestones: S181, S182 Jan 26, 2021
@evelyn-ys

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

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

Comment thread src/ad/azext_ad/manual/custom.py Outdated
Comment thread src/ad/azext_ad/tests/latest/test_ad_scenario.py
@evelyn-ys
Yishi Wang (evelyn-ys) merged commit 2415c3b into Azure:master Feb 4, 2021
@jiasli

Copy link
Copy Markdown
Contributor

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:

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:

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.

Takuro Sato (takuro-sato) added a commit to takuro-sato/azure-cli-extensions that referenced this pull request Aug 28, 2026
Bump api_version to 0.12.0 and framework_version to 0.5.0 to match the hcsshim CWCOW policy framework, and add the enforcement points introduced by PR Azure#2842.

The Windows policy template now wires registry_changes, unmount_cims, host_network and load_transparency_trust_list, and emits the allow_host_network, allow_unencrypted_scratch, allow_capability_dropping and allow_registry_changes_dropping switches. The Linux policy template wires host_network and load_transparency_trust_list (newly active at api_version 0.12.0) and emits allow_host_network, which has no framework default. The fragment and sidecar templates are bumped to framework_version 0.5.0.

These enforcement points have no ARM property, so they are exposed only through confcom-owned --input (-i) JSON fields, mirroring the log-provider fields: the top-level allowHostNetwork and allowRegistryChangesDropping bools, the per-container registryChanges object (passed through in the hcsshim add_values/delete_keys shape), and the top-level mappedDirectories list. mapped_directory_mount/unmount are wired only when mappedDirectories is supplied, since wiring them without backing data can only deny; an undeclared hot-add is denied by the framework either way.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Takuro Sato <takurosato@microsoft.com>
Takuro Sato (takuro-sato) added a commit to takuro-sato/azure-cli-extensions that referenced this pull request Aug 28, 2026
Elastic SAN volumes on Confidential ACI are bind-mounted into the workload
container by the atlas mounthost from /tmp/atlas/esanVolume/<deployment>/<volume>,
analogous to Azure File's /tmp/atlas/azureFileVolume/... . Register a matching
"elasticSan" mount type so CCE policies accept the Elastic SAN volume mount.

The type name matches the ARM/CCE volume property (Volume.ElasticSan ->
"elasticSan") and the source mirrors the azureFile entry, both per Compute-ACI
release/rel-20260429.

Compute-ACI also places a Linux pod that mounts an Elastic SAN volume on the
host network (iscsid needs the UVM init netns), so the generated policy for such
a pod must allow the host_network enforcement point (added in Azure#2842). Detect the
elasticSan mount type and force allow_host_network on for Linux policies, leaving
the flag at its default False otherwise.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Takuro Sato <takurosato@microsoft.com>
Takuro Sato (takuro-sato) added a commit to takuro-sato/azure-cli-extensions that referenced this pull request Sep 18, 2026
Bump api_version to 0.12.0 and framework_version to 0.5.0 to match the hcsshim CWCOW policy framework, and add the enforcement points introduced by PR Azure#2842.

The Windows policy template now wires registry_changes, unmount_cims, host_network and load_transparency_trust_list, and emits the allow_host_network, allow_unencrypted_scratch, allow_capability_dropping and allow_registry_changes_dropping switches. The Linux policy template wires host_network and load_transparency_trust_list (newly active at api_version 0.12.0) and emits allow_host_network, which has no framework default. The fragment and sidecar templates are bumped to framework_version 0.5.0.

These enforcement points have no ARM property, so they are exposed only through confcom-owned --input (-i) JSON fields, mirroring the log-provider fields: the top-level allowHostNetwork and allowRegistryChangesDropping bools, the per-container registryChanges object (passed through in the hcsshim add_values/delete_keys shape), and the top-level mappedDirectories list. mapped_directory_mount/unmount are wired only when mappedDirectories is supplied, since wiring them without backing data can only deny; an undeclared hot-add is denied by the framework either way.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Takuro Sato <takurosato@microsoft.com>
Takuro Sato (takuro-sato) added a commit to takuro-sato/azure-cli-extensions that referenced this pull request Sep 18, 2026
Elastic SAN volumes on Confidential ACI are bind-mounted into the workload
container by the atlas mounthost from /tmp/atlas/esanVolume/<deployment>/<volume>,
analogous to Azure File's /tmp/atlas/azureFileVolume/... . Register a matching
"elasticSan" mount type so CCE policies accept the Elastic SAN volume mount.

The type name matches the ARM/CCE volume property (Volume.ElasticSan ->
"elasticSan") and the source mirrors the azureFile entry, both per Compute-ACI
release/rel-20260429.

Compute-ACI also places a Linux pod that mounts an Elastic SAN volume on the
host network (iscsid needs the UVM init netns), so the generated policy for such
a pod must allow the host_network enforcement point (added in Azure#2842). Detect the
elasticSan mount type and force allow_host_network on for Linux policies, leaving
the flag at its default False otherwise.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Takuro Sato <takurosato@microsoft.com>
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.

5 participants