-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
appservice: support assign managed service identity to webapp/functionapp #4837
Conversation
View a preview at https://prompt.ws/r/Azure/azure-cli/4837 |
Note, the service is still in preview. The command layout might change, particularly when we on-board the explicit identity support. az webapp assign-identity -g myrg -n myweb --role reader --scope /subscriptions/0b1f6471-1bf0-4dda-aec3-xxxxxxxxxxxx/resourceGroups/myrg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor issue otherwise looks fine. Merge at your own risk :P
properties = RoleAssignmentProperties(identity_role_id, principal_id) | ||
|
||
logger.info("Creating an assignment with a role '%s' on the scope of '%s'", identity_role_id, identity_scope) | ||
retry_times = 36 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retry 36 times! 1) Magic number? Any guidelines coming from identity team? 2) 36 times !?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it's 3 minutes ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, recommended by the PAS team to workaround a known server replication delay :)
client = get_mgmt_service_client(AuthorizationManagementClient).role_definitions | ||
|
||
role_id = None | ||
if re.match(r'/subscriptions/.+/providers/Microsoft.Authorization/roleDefinitions/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex here can be improved because the wildcard is a GUID which is more restricted than .+
matching.
@@ -44,6 +44,20 @@ | |||
--facebook-oauth-scopes public_profile email | |||
""" | |||
|
|||
helps['webapp assign-identity'] = """ | |||
type: command | |||
short-summary: (PREVIEW)assign managed service identity to the webapp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add space between (PREVIEW)
and assign
vmss: support basic tier of vms (Azure#4847) Azure IoT CA functionality (Azure#4804) * Adds support for X.509 Certificates in IoT Hub. * Modifies release notes and changes help link. * Cleans up .gitignore, and HISTORY.rst. Passes a single factory in commands.py. Moves test utilities. Adds a file completer. * Updates style after rebase based on additional linter restrictions. Reserved Instance cli public PR (Azure#4838) Handle dashes in extension names better (Azure#4839) Fix `az aks get-credentials` on Windows (Azure#4762) * Fix `az aks get-credentials` on Windows * Move k8s file merge logic to helper function appservice: support assign managed service identity to webapp/functionapp (Azure#4837) Extension examples small improvement (Azure#4852) Fixing appservice list-locations (Azure#4846) Add extension name to telemetry and UA header (Azure#4854) Fix Azure#4825. (Azure#4853) Extensions `az extension add` --yes param as flag (Azure#4858) Fix issue Azure#2752. (Azure#4859)
For less code redundancy, I extracted some common role assignments logic and moved to azure-cli-core.
General Guidelines
Command Guidelines
(see Authoring Command Modules)