Add managed identities support to RG setup scripts #837
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -363,6 +363,13 @@ | |
| "kvName": { | ||
| "type": "string", | ||
| "value": "[variables('kvName')]" | ||
| }, | ||
| "storageAccountNames": { | ||
| "type": "array", | ||
| "copy": { | ||
| "count": "[length(parameters('storageAccountLocations'))]", | ||
| "input": "[concat(variables('storageAccountName'), parameters('storageAccountLocations')[copyIndex()])]" | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,14 +11,17 @@ param( | |
| # Results DB ARM template params | ||
| [Parameter(Mandatory=$False)] | ||
| [string] $resultsDbArmParamsFile, | ||
| [Parameter(Mandatory=$True)] | ||
| [string] $resourceGroupName, | ||
| # Managed Identity params | ||
| [Parameter(Mandatory=$True, ParameterSetName="ByMI")] | ||
|
There was a problem hiding this comment. Are using ServicePrincipals and ManagedIdentity mutually exclusive? |
||
| [string] $managedIdentityName, | ||
| # Service Principal params | ||
| [Parameter(Mandatory=$True, ParameterSetName="BySP")] | ||
| [string] $servicePrincipalName, | ||
| [Parameter(Mandatory=$True, ParameterSetName="BySP")] | ||
| [string] $certName, | ||
| [Parameter(Mandatory=$False, ParameterSetName="BySP")] | ||
| [int] $certExpirationYears = 1 | ||
| ) | ||
|
|
||
|
|
@@ -36,6 +39,9 @@ if (!$?) { | |
| return | ||
| } | ||
|
|
||
| $vmName = $deploymentResults.properties.outputs.vmName.value | ||
| $storageAccountNames = $deploymentResults.properties.outputs.storageAccountNames.value | ||
|
|
||
| # Conditional provisioning of results DB | ||
| if ($resultsDbArmParamsFile) { | ||
| Write-Output "Provisioning results DB..." | ||
|
|
@@ -51,7 +57,6 @@ if ($resultsDbArmParamsFile) { | |
| } | ||
| else { | ||
| $dbName = $dbDeploymentResults.properties.outputs.dbName.value | ||
| $vmIpAddress = $deploymentResults.properties.outputs.vmIpAddress.value | ||
|
|
||
| # VM IP access for results DB | ||
|
|
@@ -83,41 +88,77 @@ $certThumbprint = az keyvault certificate show ` | |
| 2>$null ` | ||
| || "NOCERT" | ||
|
|
||
| switch ($PSCmdlet.ParameterSetName) { | ||
| "BySP" { | ||
| if ($certThumbprint -eq "NOCERT") { | ||
| # The cert does not exist yet. | ||
| # Create the service principal if doesn't exist, storing the cert in the keyvault | ||
| # If it does exist, this also patches the current service principal with the role | ||
| az ad sp create-for-rbac ` | ||
| --name $servicePrincipalName ` | ||
| --role "Contributor" ` | ||
| --scopes $resourceGroupId ` | ||
| --create-cert ` | ||
| --cert $certName ` | ||
| --keyvault $kvName ` | ||
| --years $certExpirationYears | ||
| } else { | ||
| # The cert already exists in the keyvault. | ||
|
|
||
| # Ensure the SP exists with correct roles, without creating a cert. | ||
| az ad sp create-for-rbac ` | ||
| --name $servicePrincipalName ` | ||
| --role "Contributor" ` | ||
| --scopes $resourceGroupId ` | ||
|
|
||
| # SP's certs, which are stored in the registered application instead | ||
| $servicePrincipalAppId = az ad sp list ` | ||
| --display-name $servicePrincipalName ` | ||
| --query "[?servicePrincipalType == 'Application'].appId" ` | ||
| --output tsv | ||
| $spCertThumbprints = az ad app credential list ` | ||
| --id $servicePrincipalAppId ` | ||
| --cert ` | ||
| --query "[].customKeyIdentifier" ` | ||
| --output tsv | ||
|
|
||
| if ($spCertThumbprints.Contains($certThumbprint)) { | ||
| Write-Output "Keyvault contains the certificate '$certName' that is linked to the service principal '$servicePrincipalName' already." | ||
| } else { | ||
| Write-Warning "Keyvault already contains a certificate called '$certName', but is not linked with the service principal '$servicePrincipalName'! Skipping cert handling" | ||
| } | ||
| } | ||
| break | ||
| } | ||
| "ByMI" { | ||
| # Ensure the user managed identity is created | ||
| $miId = az identity create ` | ||
| --name $managedIdentityName ` | ||
| --resource-group $resourceGroupName ` | ||
| --query "principalId" --output tsv | ||
|
|
||
| Write-Output "Using managed identity $managedIdentityName with principalId $miId" | ||
|
|
||
| Write-Output "Assigning the identity access to the Resource Group..." | ||
| az role assignment create ` | ||
| --assignee $miId ` | ||
| --role "Contributor" ` | ||
| --scope $resourceGroupId | ||
|
|
||
| # Assign the identity to the VM | ||
| Write-Output "Assigning the identity to the VM..." | ||
| az vm identity assign --name $vmName --resource-group $resourceGroupName --identities $managedIdentityName | ||
|
|
||
| # Assign the identity access to the storage accounts | ||
| foreach ($storageAccountName in $storageAccountNames) { | ||
| Write-Output "Assigning the identity the role for $storageAccountName..." | ||
| $storageAccountResourceId = az storage account show --name $storageAccountName --resource-group $resourceGroupName --query "id" --output tsv | ||
| az role assignment create ` | ||
| --assignee $miId ` | ||
| --role "Storage File Data Privileged Contributor" ` | ||
| --scope $storageAccountResourceId | ||
| } | ||
| break | ||
| } | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Has any of this been developed with an eye towards rectifying an existing setup?
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.
@bpkroth that is a good point. Unfortunately I have only tested this with setting up from scratch.
However, in theory I think this should work for existing resource groups.
Assuming the existing RG was set up with these scripts, I think all the az cli calls (need to confirm) are idempotent.
For example, using the same ARM template and parameters should give us back the required details of the resources without changing them too much (same naming conventions)
Then the new block of logic for managed identity should then be apply to those existing resources (VMs, storage accounts, RG role)
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.
I think we might need to add end-to-end tests to validate these finally, though maybe in a separate PR.