Create role assignment for MSI when enable_vnet_integration is true #9153

MuhammadAliFleet · 2025-09-11T02:59:59Z

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

Related command

This PR:

removes the is_preview=True flag from enable_vnet_integration and apiserver_subnet_id
creates role assignment for msi on api and agent subnet when enable_vnet_integration is true

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? (pip install wheel==0.30.0 required)
My extension version conforms to the Extension version schema

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 pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

azure-client-tools-bot-prd · 2025-09-11T03:00:02Z

️✔️Azure CLI Extensions Breaking Change Test

️✔️Non Breaking Changes

azure-client-tools-bot-prd · 2025-09-11T03:00:05Z

Hi @MuhammadAliFleet,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

yonzhan · 2025-09-11T03:00:09Z

Thank you for your contribution! We will review the pull request and get back to you soon.

github-actions · 2025-09-11T03:00:10Z

The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR.

Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions).
After that please run the following commands to enable git hooks:

pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>

Copilot

Pull Request Overview

This PR removes the preview flag from VNet integration functionality and adds proper role assignment management for managed service identities when VNet integration is enabled. The changes enable production use of VNet integration features and ensure proper network permissions are granted.

Remove is_preview=True from VNet integration parameters to promote them to general availability
Add role assignments for managed service identity on API server and agent subnets when VNet integration is enabled
Refactor subnet role assignment logic to support different identity types

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/fleet/azext_fleet/custom.py	Updates fleet creation logic to assign network contributor roles for MSI when VNet integration is enabled
src/fleet/azext_fleet/_params.py	Removes preview flags from enable_vnet_integration and apiserver_subnet_id parameters
src/fleet/azext_fleet/_helpers.py	Refactors role assignment function and adds MSI object ID retrieval functionality
src/fleet/azext_fleet/_client_factory.py	Adds MSI client factory function for managed identity operations

Copilot · 2025-09-11T03:00:41Z

src/fleet/azext_fleet/custom.py

-        assign_network_contributor_role_to_subnet(cmd, resource_group_name, agent_subnet_id)
+        assign_network_contributor_role_to_subnet(cmd, FLEET_1P_APP_ID, agent_subnet_id)
+
+    if enable_vnet_integration:


The code calls get_msi_object_id(cmd, assign_identity) without checking if assign_identity is None. This will cause an error when enable_vnet_integration is True but no user-assigned identity is provided.

Suggested change

if enable_vnet_integration:

if enable_vnet_integration:

if assign_identity is None:

raise CLIError("User-assigned identity must be provided for VNet integration.")

This is good feedback.
1- we should validate that MSI is not nil if enable_vnet_integration is set
2- we should call assign_network_contributor_role_to_subnet here only if we know it's a user MSI

now validating that MSI is not nil if enable_vnet_integration is set

call assign_network_contributor_role_to_subnet here only if we know it's a user MSI. I tested creating private fleet v2 with system assigned msi and it fails due to missing perm action. We would need the role on the system MSI as well.

src/fleet/azext_fleet/_helpers.py

github-actions · 2025-09-11T03:03:27Z

Hi @MuhammadAliFleet

Release Suggestions

Module: fleet

Please log updates into to src/fleet/HISTORY.rst
Update VERSION to 1.7.0 in src/fleet/setup.py

Notes

For more info about extension versioning, please refer to Extension version schema

src/fleet/azext_fleet/_helpers.py

jim-minter · 2025-09-11T23:27:36Z

src/fleet/azext_fleet/custom.py

-        assign_network_contributor_role_to_subnet(cmd, resource_group_name, agent_subnet_id)
+        assign_network_contributor_role_to_subnet(cmd, FLEET_1P_APP_ID, agent_subnet_id)
+
+    if enable_vnet_integration:


This is good feedback.
1- we should validate that MSI is not nil if enable_vnet_integration is set
2- we should call assign_network_contributor_role_to_subnet here only if we know it's a user MSI

jim-minter · 2025-09-11T23:28:01Z

src/fleet/azext_fleet/custom.py

+        assign_network_contributor_role_to_subnet(cmd, FLEET_1P_APP_ID, agent_subnet_id)
+
+    if enable_vnet_integration:
+        assign_network_contributor_role_to_subnet(cmd, get_msi_object_id(cmd, assign_identity), apiserver_subnet_id)


calling get_msi_object_id twice is 2 API calls. Call once and save the result for the second add_role call

jim-minter · 2025-09-11T23:28:28Z

src/fleet/azext_fleet/_helpers.py

+
+def get_msi_object_id(cmd, msi_resource_id):
+    try:
+        if not is_valid_resource_id(msi_resource_id):


this should be in a validator

src/fleet/azext_fleet/_helpers.py

jim-minter · 2025-09-17T18:28:45Z

src/fleet/azext_fleet/custom.py

        raise CLIError("Cannot assign identity without enabling managed identity.")

+    if enable_vnet_integration:
+        if not enable_managed_identity and assign_identity is None:


shouldn't this just be if not enable_managed_identity:?

can this test be moved to _validators.py (i.e. add a new validate_enable_vnet_integration function and update _params.py with c.argument('enable_vnet_integration', validator=validate_enable_vnet_integration, ...)?

moved to _validators.py

jim-minter · 2025-09-17T18:34:48Z

src/fleet/azext_fleet/_helpers.py

-def assign_network_contributor_role_to_subnet(cmd, subnet_id):
+def assign_network_contributor_role_to_subnet(cmd, object_id, subnet_id):
+    if not add_role_assignment(cmd, 'Network Contributor', object_id, scope=subnet_id):
+        logger.warning("Failed to create Network Contributor role assignment on the subnet.\n"


Could you provide a more detailed error? Maybe even ideally to the point at which someone could copy/paste an az role assignment create command and ask their admin to run that for them?

Added detailed error with command

src/fleet/azext_fleet/_helpers.py

jim-minter · 2025-09-19T00:04:58Z

src/fleet/azext_fleet/_validators.py


+def validate_enable_vnet_integration(namespace):
+    if namespace.enable_vnet_integration and not namespace.enable_managed_identity:
+        raise CLIError("--enable-vnet-integration requires managed identity to be enabled. "


although it's theoretically possible to do vnet integration with system MSI, it's a real pain. I'm worried this message will send people down that painful path. I'm tempted to require they use user MSI in the CLI, i.e. validate both enable_managed_identity and assign_identity, and update the error message to include adding --assign-identity as well?

now validating both enable_managed_identity and assign_identity

jim-minter

LGTM

jim-minter · 2025-09-19T15:29:23Z

@kairu-ms please merge?

Copilot AI review requested due to automatic review settings September 11, 2025 02:59

azure-client-tools-bot-prd bot added this to the October 2025 (2025-10-07) milestone Sep 11, 2025

Copilot AI reviewed Sep 11, 2025

View reviewed changes

yonzhan requested review from kairu-ms and jsntcy September 11, 2025 07:12

yonzhan assigned kairu-ms Sep 11, 2025

jim-minter suggested changes Sep 11, 2025

View reviewed changes

MuhammadAliFleet added 4 commits September 12, 2025 13:17

update error message

72e5c3a

grant vnet perm

8eb2b2c

address comments

08a7867

setup.py

5d9f635

MuhammadAliFleet force-pushed the alimuhammad/fleet/privfleet-err-msg branch from 6b3cb6f to 5d9f635 Compare September 12, 2025 17:23

github-actions bot added the release-version-block Updates do not qualify release version rules. NOTE: please do not edit it manually. label Sep 12, 2025

jim-minter reviewed Sep 17, 2025

View reviewed changes

MuhammadAliFleet added 3 commits September 17, 2025 15:57

update

283ee3e

update style

19456f6

release version block

735393d

github-actions bot removed the release-version-block Updates do not qualify release version rules. NOTE: please do not edit it manually. label Sep 17, 2025

jim-minter reviewed Sep 19, 2025

View reviewed changes

src/fleet/azext_fleet/_helpers.py Outdated Show resolved Hide resolved

jim-minter reviewed Sep 19, 2025

View reviewed changes

update

54307d2

jim-minter approved these changes Sep 19, 2025

View reviewed changes

kairu-ms approved these changes Sep 22, 2025

View reviewed changes

kairu-ms merged commit 5618fed into Azure:main Sep 22, 2025
25 checks passed

Create role assignment for MSI when enable_vnet_integration is true #9153

Create role assignment for MSI when enable_vnet_integration is true #9153

Uh oh!

Conversation

MuhammadAliFleet commented Sep 11, 2025

Related command

General Guidelines

About Extension Publish

Uh oh!

azure-client-tools-bot-prd bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-client-tools-bot-prd bot commented Sep 11, 2025

Uh oh!

yonzhan commented Sep 11, 2025

Uh oh!

github-actions bot commented Sep 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Suggestions

Module: fleet

Notes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jim-minter left a comment

Choose a reason for hiding this comment

Uh oh!

jim-minter commented Sep 19, 2025

Uh oh!

Uh oh!

Uh oh!

azure-client-tools-bot-prd bot commented Sep 11, 2025 •

edited

Loading

github-actions bot commented Sep 11, 2025 •

edited

Loading