Skip to content
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

Consider moving templates to bicep deployer() #4620

Open
jongio opened this issue Dec 10, 2024 · 2 comments
Open

Consider moving templates to bicep deployer() #4620

jongio opened this issue Dec 10, 2024 · 2 comments

Comments

@jongio
Copy link
Member

jongio commented Dec 10, 2024

Bicep just pushed deployer concept, which they didn't have when we shipped azd. So we implemented that as AZURE_PRINCIPAL_ID. We should consider using "deployer" instead

Azure/bicep#15340

resource roleAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
  // can be used to help make GUID unique
  name: guid(deployer().objectId, readerRoleDefinitionId, resourceGroup().id)
  properties: {
    principalId: deployer().objectId // easily retrieve objectId
    roleDefinitionId: readerRoleDefinitionId
  }
}
@ellismg
Copy link
Member

ellismg commented Dec 11, 2024

When we do this, if we want to be really cute - we should see if we can eliminate computing the value of AZURE_PRINCIPAL_ID in some cases (like when it is not listed in in the main.parameters.json and so we know we won't need it. That would save some end to end time for us since computing the principal id is a little expensive (we have to do some graph calls).

@Menghua1
Copy link
Member

Menghua1 commented Dec 19, 2024

We plan to modify the AZD CLI code to support the deployer() method first, and then update the reference to principalId in the todo templates.

The specific AZD CLI modification approach is as follows:

First, Update the minimum Bicep version to 0.32.4 in the bicep.go, as this is the version that supports the deployer(). Currently, the Bicep version that is automatically downloaded in AZD is 0.29.47, which does not supports the deployer().

Second, regarding @ellismg's comment, it is agreed that avoiding unnecessary AZURE_PRINCIPAL_ID calculations is beneficial. To achieve this, it is planned to modify the loadParameters(). Specifically, before computing the principalId, check if main.parameters.json refers to AZURE_PRINCIPAL_ID. If not, computing principalId will be skipped to save time.

CC: @rajeshkamal5050

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants