Deployment script modules

[Gordonby]

Description

closes #60

PR is draft, whilst i complete the remainder of the steps in the contribution guide

Adding a new module

• A proposal has been submitted and approved.
• I have included "Closes #{module_proposal_issue_number}" in the PR description.
• I have run brm validate locally to verify the module files.
• I have run deployment tests locally to ensure the module is deployable.
• README.md is complete and ready for review

[alex-frankel]

Thanks for the contribution. Have a few questions, but otherwise looks good.

[alex-frankel]

What if I want to use an existing managed identity? We sometimes see replication delays between creating an MI, assigning a role, then executing the script such that it will fail if you get unlucky. An option to use an existing MI instead seems important.

[Gordonby]

That is a good idea, allowing an existing MI to be used.

Replication delays can be mitigated by adding a sleep to the script. On testing, I hadn't encountered any issues.
In an earlier version, I was setting it to sleep for 30s at the beginning of the script - perhaps i put that back in, and base the sleep interval from a parameter value.

[Gordonby]

Added a configurable script delay to the bicep code. @alex-frankel

[Gordonby]

Also tested with an existing ManagedIdentity. Passing the name of a ManagedIdentity in the same location works great.

[alex-frankel]

Passing the name of a ManagedIdentity in the same location works great.

Can you clarify this? An existing resource should use the existing keyword, so it would require some code like the following:

param existingManagedIdentitySub string = subscription().subscriptionId
param existingManagedIdentityResourceGroup string = resourceGroup().name

param useExistingManagedIdentity bool = false

resource exsitingDepScriptId 'Microsoft.ManagedIdentity/userAssignedIdentities@2018-11-30' existing = if (useExistingManagedIdentity ) {
  name: 'foo'
  scope: resourceGroup(existingManagedIdentitySub, existingManagedIdentityResourceGroup)
}

resource newDepScriptId 'Microsoft.ManagedIdentity/userAssignedIdentities@2018-11-30' = if (!useExistingManagedIdentity) {

}

resource script 'Microsoft.Resources/deploymentScripts@2020-10-01' = {
  name: 'foo'
  kind: 'AzureCLI'
  location: resourceGroup().location
  identity: {
    type: 'UserAssigned'
    userAssignedIdentities: {
      '${useExistingManagedIdentity ? exsitingDepScriptId.id : newDepScriptId.id}': {}
    }
  }
}

Is that what you tried?

[Gordonby]

No, i didn't test that method exactly.

Because of the idempotent nature of the RP, if i ask for the same UserAssignedIdentity to be created multiple times - it will work.

UserAssignedIdentities fortunately have very few properties for this to cause a problem with the bicep.

This was the test i wrote;
https://github.com/Gordonby/bicep-registry-modules-1/blob/f54863e51e5edd0769f0a87f49bdbf52ccbb29dc/modules/deployment-scripts/import-acr/test/main.test.bicep#L51
I ran that main.test.bicep 5 times, and ended up with 5 ACR's. The managed identity was not duplicated and had access to all 5 of the ACR's where RBAC had been granted in that test (as well as some other arbitrary RBAC I'd given the ManagedId).

I'm not suggesting it's as clean as explicitly dealing with resources using the existing keyword, only that because of the resource simplicity and the RP idempotency - it's workable.

[alex-frankel]

only that because of the resource simplicity and the RP idempotency - it's workable.

Yes, but this limits you to an MI that happens to be in the same RG as the deployment script. In practice, this may not always be the case.

[Gordonby]

I've leveraged your code in #65

[alex-frankel]

replication delay issue is relevant with the roleAssignments as well

[Gordonby]

Thanks for the contribution. Have a few questions, but otherwise looks good.

Thanks for the review, I wasn't entirely ready for it (hence the draft status) as I still need to work on the README.md - but your bicep suggestions are good 👍

[Gordonby]

New commits to my branch don't seem to be flowing through to the PR.