-
Notifications
You must be signed in to change notification settings - Fork 67
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
azure: Add wizard steps to list/create UserAssignedIdentities
and execute role definitions
#1757
Conversation
I should probably revisit exposing the scope since it contains the subscription id of the resource as well. Probably can just split by |
|
||
export async function createAuthorizationManagementClient(context: InternalAzExtClientContext): Promise<AuthorizationManagementClient> { | ||
if (parseClientContext(context).isCustomCloud) { | ||
return <AuthorizationManagementClient><unknown>createAzureClient(context, (await import('@azure/arm-authorization-profile-2020-09-01-hybrid')).AuthorizationManagementClient); |
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.
Oh nice, so the hybrid profile version was made to support custom clouds?
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.
Is there a downside to just always using the one that supports custom clouds?
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.
Discussed offline, but the api-versions for custom clouds aren't kept up to date as quickly as standard Azure SDKs.
Time is running out |
|
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.
Initial pass
|
||
export async function createAuthorizationManagementClient(context: InternalAzExtClientContext): Promise<AuthorizationManagementClient> { | ||
if (parseClientContext(context).isCustomCloud) { | ||
return <AuthorizationManagementClient><unknown>createAzureClient(context, (await import('@azure/arm-authorization-profile-2020-09-01-hybrid')).AuthorizationManagementClient); |
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.
Is there a downside to just always using the one that supports custom clouds?
See an example of this being used here: https://github.com/microsoft/vscode-azurefunctions/pull/4213/files
This adds
managedIdentity
to theResourceWizardContext
since managed identity will be included in a lot of our resources.The
UserAssignedIdentity
steps are for listing and creating new identities. Right now, it will always default to creating it in the resource group. That reminds me, I need to test what happens if I pick an existing resource group that already has an MI in it. I'm not even sure what behavior should be-- create a new one with a suffix or use the existing one?The
RoleAssignmentStep
should work for both system and user assigned identities. Because the scope will be created during the wizard, you need to pass a function that returns the property that you want since the context will be updated by the time the function is called. Execute priority is also 900 because in most cases, it should always be the last thing to execute.