Allow contextual creation of role assignments and related resources #315

christian-calabrese · 2025-02-25T17:18:39Z

Due to the presence of data sources, it is not possible to contextually create the target resource (storage account, redis cache, apim, ...) and the related role assignments in the same terraform plan or apply.

Resolves #CES-781

changeset-bot · 2025-02-25T17:18:43Z

🦋 Changeset detected

Latest commit: 92f1d93

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
azure_role_assignments	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

infra/modules/azure_role_assignments/modules/key_vault/variables.tf

Krusty93 · 2025-02-26T10:56:21Z

Does it make sense to keep name/resource group pairs in variables?

christian-calabrese · 2025-02-26T11:39:16Z

Does it make sense to keep name/resource group pairs in variables?

It's pretty much the same idea as described here.
Do you think we can get rid of the possibility to just specify the name of the service when it's already created?

PS: Actually, I should check if they are still needed when specifying the resource id. Maybe we can make them optional and check that at least one is populated

Krusty93 · 2025-02-26T14:02:09Z

Does it make sense to keep name/resource group pairs in variables?

It's pretty much the same idea as described here. Do you think we can get rid of the possibility to just specify the name of the service when it's already created?

PS: Actually, I should check if they are still needed when specifying the resource id. Maybe we can make them optional and check that at least one is populated

I think we can remove name/rg pairs in both contexts (new/existing) as the id is always needed to the scope property, while the name/rg pair is useful only for data. I suggest to have a look at it before doing changes by the way

christian-calabrese · 2025-02-27T15:15:31Z

Does it make sense to keep name/resource group pairs in variables?

It's pretty much the same idea as described here. Do you think we can get rid of the possibility to just specify the name of the service when it's already created?
PS: Actually, I should check if they are still needed when specifying the resource id. Maybe we can make them optional and check that at least one is populated

I think we can remove name/rg pairs in both contexts (new/existing) as the id is always needed to the scope property, while the name/rg pair is useful only for data. I suggest to have a look at it before doing changes by the way

@Krusty93 currently, the code allows specifying either the target resource's name + resource group or its id.
We can surely remove it, but it needs the azurerm provider function parse_resource_id, hence azurerm 4.x.y.

Since it's a breaking change, I think we can do it anyways with a major.

Moreover, I saw that from version 3.114 it is possible to opt in for the provider functions feature (source)

If you and the team agrees, I proceed removing the name and resource group name variables for simplicity.

… of the id

.changeset/beige-buses-refuse.md

gunzip · 2025-02-28T17:18:29Z

Due to the presence of data sources, it is not possible to contextually create the target resource (storage account, redis cache, apim, ...) and the related role assignments.

when exactly?

gunzip

lgtm, let's see what others think @pagopa/engineering-team-devex

.changeset/beige-buses-refuse.md

infra/modules/azure_role_assignments/modules/apim/locals.tf

Krusty93 · 2025-03-04T10:13:21Z

infra/modules/azure_role_assignments/modules/apim/providers.tf

+  required_providers {
+    azurerm = {
+      source  = "hashicorp/azurerm"
+      version = ">= 3.114, < 5.0"


question: Is 3.114 ok with the provider::azurerm::parse_resource_id function?

I added these lines in the migration guide to explain how to make it work for >= 3.114 azurerm versions

Krusty93 · 2025-03-04T10:15:23Z

infra/modules/azure_role_assignments/modules/apim/locals.tf

-  apims = distinct([for assignment in var.apim : { name = assignment.name, resource_group_name = assignment.resource_group_name }])
-
+  norm_apims = [for apim in var.apim : {
+    name                = provider::azurerm::parse_resource_id(apim.id)["resource_name"]


question: Why do you still need name and resource_group_name?

Mostly to not break old resource indices. Cosmos submodule, instead, will still need them

Do you think we can remove them? Projects already using the module and updating to the new major may need to move some resources state.

Krusty93 · 2025-03-04T10:16:27Z

infra/modules/azure_role_assignments/providers.tf

@@ -2,7 +2,7 @@ terraform {
  required_providers {
    azurerm = {
      source  = "hashicorp/azurerm"
-      version = ">= 3.110, < 5.0"
+      version = ">= 3.114, < 5.0"


question: Isn't this constraint enough for all the sub modules?

Submodules can be instantiated on their own, so the providers definition shall be defined

infra/modules/azure_role_assignments/tests/role.tftest.hcl

Co-authored-by: Marco Comi <[email protected]>

gunzip · 2025-03-04T14:29:52Z

.changeset/beige-buses-refuse.md

+Role assignments can now be created contextually with identities and target resources
+
+**Migration guide**:
+When using the azurerm provider version >= 3.114.0, the beta opt-in feature must be enabled by setting the following environment variable:


question: what's the "the beta opt-in feature" ?

I tried to explain it better in a next commit. The module makes use of features available from azurerm 4.0 version (provider defined functions). The beta opt-in feature allows to use them from 3.114.0 version as well.

It's a way to not break projects that don't support the 4.0 version of the provider yet.

feat: allow contextual role assignments to resources creation

2963e29

christian-calabrese had a problem deploying to dev-ci February 25, 2025 17:19 — with GitHub Actions Failure

gunzip reviewed Feb 25, 2025

View reviewed changes

infra/modules/azure_role_assignments/modules/key_vault/variables.tf Outdated Show resolved Hide resolved

chore: ran pre-commit

8fd9e8a

christian-calabrese temporarily deployed to dev-ci February 26, 2025 11:26 — with GitHub Actions Inactive

fix: optional variables

ffc65c0

christian-calabrese temporarily deployed to dev-ci February 26, 2025 17:03 — with GitHub Actions Inactive

christian-calabrese changed the title ~~Allow contextual creation of role assignments and related resources~~ [CES-871] Allow contextual creation of role assignments and related resources Feb 26, 2025

christian-calabrese changed the title ~~[CES-871] Allow contextual creation of role assignments and related resources~~ [CES-781] Allow contextual creation of role assignments and related resources Feb 26, 2025

docs: added changeset

2bcb333

christian-calabrese temporarily deployed to dev-ci February 26, 2025 17:07 — with GitHub Actions Inactive

christian-calabrese changed the title ~~[CES-781] Allow contextual creation of role assignments and related resources~~ Allow contextual creation of role assignments and related resources Feb 28, 2025

Completely remove the name and resource_group_name variables in favor…

7fe17f2

… of the id

christian-calabrese marked this pull request as ready for review February 28, 2025 17:12

christian-calabrese requested a review from a team as a code owner February 28, 2025 17:12

christian-calabrese temporarily deployed to dev-ci February 28, 2025 17:12 — with GitHub Actions Inactive

gunzip requested changes Feb 28, 2025

View reviewed changes

.changeset/beige-buses-refuse.md Show resolved Hide resolved

Fix docs after removal of name and resource group name variables

51f7686

christian-calabrese temporarily deployed to dev-ci February 28, 2025 17:24 — with GitHub Actions Inactive

gunzip self-requested a review February 28, 2025 17:53

gunzip approved these changes Feb 28, 2025

View reviewed changes

kin0992 reviewed Mar 3, 2025

View reviewed changes

.changeset/beige-buses-refuse.md Outdated Show resolved Hide resolved

.changeset/beige-buses-refuse.md Outdated Show resolved Hide resolved

infra/modules/azure_role_assignments/modules/apim/locals.tf Show resolved Hide resolved

Krusty93 reviewed Mar 4, 2025

View reviewed changes

infra/modules/azure_role_assignments/tests/role.tftest.hcl Outdated Show resolved Hide resolved

christian-calabrese and others added 2 commits March 4, 2025 15:13

Format parameters as code in changeset

effdb55

Co-authored-by: Marco Comi <[email protected]>

Add azurerm 4.0 beta optin in migration guide

f090a00

christian-calabrese temporarily deployed to dev-ci March 4, 2025 14:23 — with GitHub Actions Inactive

Remove unnecessary subscription_id parameter in tests

758c6d0

christian-calabrese temporarily deployed to dev-ci March 4, 2025 14:29 — with GitHub Actions Inactive

gunzip reviewed Mar 4, 2025

View reviewed changes

Explain why beta opt-in feature is needed

92f1d93

christian-calabrese temporarily deployed to dev-ci March 4, 2025 14:40 — with GitHub Actions Inactive

christian-calabrese requested review from Krusty93 and kin0992 March 6, 2025 09:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow contextual creation of role assignments and related resources #315

Allow contextual creation of role assignments and related resources #315

christian-calabrese commented Feb 25, 2025 •

edited

Loading

changeset-bot bot commented Feb 25, 2025 •

edited

Loading

Krusty93 commented Feb 26, 2025

christian-calabrese commented Feb 26, 2025 •

edited

Loading

Krusty93 commented Feb 26, 2025

christian-calabrese commented Feb 27, 2025 •

edited

Loading

gunzip commented Feb 28, 2025

gunzip left a comment

Krusty93 Mar 4, 2025 •

edited

Loading

christian-calabrese Mar 4, 2025

Krusty93 Mar 4, 2025 •

edited

Loading

christian-calabrese Mar 4, 2025

Krusty93 Mar 4, 2025

christian-calabrese Mar 4, 2025

gunzip Mar 4, 2025

christian-calabrese Mar 4, 2025

Allow contextual creation of role assignments and related resources #315

Are you sure you want to change the base?

Allow contextual creation of role assignments and related resources #315

Conversation

christian-calabrese commented Feb 25, 2025 • edited Loading

changeset-bot bot commented Feb 25, 2025 • edited Loading

🦋 Changeset detected

Krusty93 commented Feb 26, 2025

christian-calabrese commented Feb 26, 2025 • edited Loading

Krusty93 commented Feb 26, 2025

christian-calabrese commented Feb 27, 2025 • edited Loading

gunzip commented Feb 28, 2025

gunzip left a comment

Choose a reason for hiding this comment

Krusty93 Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

christian-calabrese Mar 4, 2025

Choose a reason for hiding this comment

Krusty93 Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

christian-calabrese Mar 4, 2025

Choose a reason for hiding this comment

Krusty93 Mar 4, 2025

Choose a reason for hiding this comment

christian-calabrese Mar 4, 2025

Choose a reason for hiding this comment

gunzip Mar 4, 2025

Choose a reason for hiding this comment

christian-calabrese Mar 4, 2025

Choose a reason for hiding this comment

christian-calabrese commented Feb 25, 2025 •

edited

Loading

changeset-bot bot commented Feb 25, 2025 •

edited

Loading

christian-calabrese commented Feb 26, 2025 •

edited

Loading

christian-calabrese commented Feb 27, 2025 •

edited

Loading

Krusty93 Mar 4, 2025 •

edited

Loading

Krusty93 Mar 4, 2025 •

edited

Loading