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

azurerm version > 3.89.0 is breaking For/for_each loop parsing #24935

Closed
1 task done
BigFrog-coding opened this issue Feb 19, 2024 · 17 comments
Closed
1 task done

azurerm version > 3.89.0 is breaking For/for_each loop parsing #24935

BigFrog-coding opened this issue Feb 19, 2024 · 17 comments

Comments

@BigFrog-coding
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment and review the contribution guide to help.

Terraform Version

1.6.6

AzureRM Provider Version

3.92

Affected Resource(s)/Data Source(s)

azurerm_pim_eligible_role_assignment

Terraform Configuration Files

resource "azurerm_pim_eligible_role_assignment" "pim_sub_eligible_assignment" {
  for_each = {
    for role in local.roles : "${role.scope}${role.role_definition_id}" => role
  }
  scope              = each.value.scope
  role_definition_id = each.key
  principal_id       = each.value.principal_id

  schedule {
    start_date_time = time_static.static_time.rfc3339
    expiration {
      duration_days = 364
    }
  }

}

example entry of local.roles =
  {
    "principal_id" = "id here"
    "role_definition_id" = "/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c"
    "scope" = "/subscriptions/id here"
  },

Debug Output/Panic Output

The debug shows too much PII and is irrelevant

Expected Behaviour

The role key should have been created using the scope and role_definition_id in the for loop. TF plans

Actual Behaviour

│ Error: Duplicate object key

│ on pim.tf line 24, in resource "azurerm_pim_eligible_role_assignment" "pim_sub_eligible_assignment":
│ 23: for_each = toset({
│ 24: for role in local.roles : "${role.scope}${role.role_definition_id}" => role
│ 25: })
│ ├────────────────
│ │ role.role_definition_id is "/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c"
│ │ role.scope is ""

│ Two different items produced the key "/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c" in this 'for' expression. If duplicates are expected, use the ellipsis (...) after the value expression to enable grouping by key.

scope is empty
This works great in all previous versions of azurerm provider

Steps to Reproduce

Terraform plan

Important Factoids

No response

References

No response

@xuzhang3
Copy link
Contributor

@BigFrog-coding you have duplicate role in the roles which should be unique

@BigFrog-coding
Copy link
Author

no I dont, the role key is created with the for loop. The issue is it is not reading the scope attribute in the object. In version 3.89.0 and previous versions it works fine

@BigFrog-coding
Copy link
Author

@xuzhang3 Can you take another look please? I am surprised no one else is seeing this issue.

@xuzhang3
Copy link
Contributor

xuzhang3 commented Mar 5, 2024

@BigFrog-coding Can you check the roles in the local vals? Where is the roles from, the loop is build the role from it and it have duplicate values.
You can reproduce this error by :

locals {
  roles = {
    roles1 = {
      scope : "scope1"
      role_definition_id : "def1"
      principal_id : "principal_id1"
    },
    roles2 = {
      scope : "scope1"
      role_definition_id : "def1"
      principal_id : "principal_id2"
    }
  }
}

@BigFrog-coding
Copy link
Author

BigFrog-coding commented Mar 5, 2024 via email

@xuzhang3
Copy link
Contributor

xuzhang3 commented Mar 6, 2024

@BigFrog-coding I ran the code, the error message is generated by Terraform and was thrown by the for expression.
image

@BigFrog-coding
Copy link
Author

BigFrog-coding commented Mar 6, 2024 via email

@xuzhang3
Copy link
Contributor

xuzhang3 commented Mar 7, 2024

@BigFrog-coding the key issue here is the role has duplicated values, how do you get the roles?

@BigFrog-coding
Copy link
Author

BigFrog-coding commented Mar 7, 2024 via email

@ABCodeMonkey
Copy link

Greetings

I have peer reviewed @BigFrog-coding results on my end.

3.99

Initializing provider plugins...
- Finding latest version of hashicorp/time...
- Finding latest version of hashicorp/azuread...
- Finding hashicorp/azurerm versions matching "3.99.0"...
- Using previously-installed hashicorp/time v0.11.1
- Using previously-installed hashicorp/azuread v2.48.0
- Installing hashicorp/azurerm v3.99.0...
- Installed hashicorp/azurerm v3.99.0 (signed by HashiCorp)

`Plan: 90 to add, 0 to change, 0 to destroy.`

Plan at 3.89

Initializing provider plugins...
- Finding latest version of hashicorp/azuread...
- Finding hashicorp/azurerm versions matching "3.89.0"...
- Finding latest version of hashicorp/time...
- Using previously-installed hashicorp/time v0.11.1
- Using previously-installed hashicorp/azuread v2.48.0
- Using previously-installed hashicorp/azurerm v3.89.0

Plan: 206 to add, 0 to change, 0 to destroy.

We are creating pim roles for Owner and Contributor
2 group creations
2 pim assignments

We have a data call to gather subscriptions

data "azurerm_subscriptions" "available" {
}

We filter in our locals:

  enabled_subs = { for i, sub in data.azurerm_subscriptions.available.subscriptions : replace(sub.display_name, " ", "") => sub
    if((sub.state == "Enabled")
      && (length(regexall("(?i).*Visual Studio.*", sub.display_name))) == 0
      && (length(regexall("(?i).*AVAILABLE.*", sub.display_name))) == 0
      && (length(regexall("(?i).*Access to.*", sub.display_name))) == 0
      && (length(regexall("(?i).*Azure subscr.*", sub.display_name))) == 0
      && (length(regexall("(?i).*Rebecca.*", sub.display_name))) == 0
      && (length(regexall("(?i).*Azure NCE.*", sub.display_name))) == 0
      && (length(regexall("(?i).*devtest-tf-01*", sub.display_name))) == 0
      && (length(regexall("(?i).*Test-ResourceProviders*", sub.display_name))) == 0
      && (length(regexall("(?i).*Reservations*", sub.display_name))) == 0
      && (length(regexall("(?i).*PROD-Reservations*", sub.display_name))) == 0
      && (length(regexall("(?i).*Archive*", sub.display_name))) == 0
    )
  }

We create groups based on the local keys (2 resources, just showing the one)

resource "azuread_group" "pim_groups_owner" {
  for_each     = local.enabled_subs
  display_name = "a_${each.key}_pim_owner"
  description  = "Used to PIM into the owner role for break glass needs"
  members      = ["fff-fff-fff-fff-fff","fff-fff-fff-fff-fff"] 
  owners       = ["fff-fff-fff-fff-fff", "fff-fff-47c3-b5dd-94ef849905f3"]    
  security_enabled = true         
  prevent_duplicate_names = true
}

Then assign pim for these groups: with a lookup


resource "azurerm_pim_eligible_role_assignment" "pim_sub_eligible_assignment_owner" {
  for_each           = azuread_group.pim_groups_owner
  scope              = local.enabled_subs[each.key].id #key is the sub, poorly named foreach :)
  role_definition_id = data.azurerm_role_definition.owner.id
  principal_id       = each.value.object_id

  schedule {
    start_date_time = time_static.static_time.rfc3339
    expiration {
      duration_days = 364
    }
  }
}

There is a clear discrepancy between the two versions using the same terraform

Plan: 90 to add, 0 to change, 0 to destroy.
Plan: 206 to add, 0 to change, 0 to destroy.

@ABCodeMonkey
Copy link

Just a quick followup,
The local.enabled_subs contains 37 subs in each azurerm version

> length(local.enabled_subs)
37

@tombuildsstuff
Copy link
Contributor

@ABCodeMonkey that's a different issue to the one described above - see #24948


As @xuzhang3 has mentioned, the issue described above by @BigFrog-coding is a configuration issue, since there's duplicate keys in the map:

role.role_definition_id is "/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c"
│ │ role.scope is ""
│
│ Two different items produced the key "/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c" in this 'for' expression. If duplicates are expected, use the ellipsis (...) after the value expression to enable grouping by key.

Since the map keys are being built-up dynamically from an existing set of data, you'd need to check the source data that's being provided to find the duplicate - I'd suggest that terraform console (or outputting this to a list, rather than a map) would be helpful in that endeavour may prove helpful here. However since this appears to be a configuration issue rather than a bug in the Azure Provider I'm going to close this issue for the moment, since we ask that configuration issues and questions are asked instead on the Community Discuss forum.

Thanks!

@BigFrog-coding BigFrog-coding changed the title azurerm version > 3.89.0 is breaking For loop parsing azurerm version > 3.89.0 is breaking For/for_each loop parsing Apr 16, 2024
@BigFrog-coding
Copy link
Author

@tombuildsstuff Why did you close this issue? This is 100% not a configuration issue. Why would it work fine with Azurerm 3.89 and not work with any version greater than that? Please stop saying it is configuration issue when it clearly is not. I must insist you re-open this bug or I will recreate it until everyday until someone actually adresses the issue

@tombuildsstuff
Copy link
Contributor

tombuildsstuff commented Apr 16, 2024

@BigFrog-coding both the error message in your original description:

Actual Behaviour

│ Error: Duplicate object key
│
│ on pim.tf line 24, in resource "azurerm_pim_eligible_role_assignment" "pim_sub_eligible_assignment":
│ 23: for_each = toset({
│ 24: for role in local.roles : "${role.scope}${role.role_definition_id}" => role
│ 25: })
│ ├────────────────
│ │ role.role_definition_id is "/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c"
│ │ role.scope is ""
│
│ Two different items produced the key "/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c" in this 'for' expression. If duplicates are expected, use the ellipsis (...) after the value expression to enable grouping by key.
╵

and the associated repro by @xuzhang3:

image

both show a configuration issue with the value being provided - that a duplicate key exists in the map (where Keys must be unique).

Note that in the error message being output there's only a Role Definition shown, not a Scope - therefore the Scope must be an empty string (see `role.scope is "" in the error message) - which is likely the source of this conflict.

As mentioned above, please double-check the source data being provided here, based on the error messages being returned - this isn't a bug in the Provider but an issue with the value being provided in the Configuration - and as such you'll need to check the source data being provided to diagnose this one.

Whilst it's possible that there could be an issue with a Terraform Resource if that value is interpolated, unfortunately without validating the source data contains no duplicate values - which is what Terraform Core is highlighting is the source of the problem here - it's not possible to know which resource needs further investigation, but since Terraform Core is stating the problem is the source data, that's where I would start digging into this problem.

@BigFrog-coding
Copy link
Author

BigFrog-coding commented Apr 16, 2024

@tombuildsstuff No it does not show a configuration issue, that is our point. You are seeing something that looks like configuration issue when in reality it is an issue with the Azurerm. The source data is a data call to pull all subscriptions and then the for loop creates dynamically a key for the loop based on the subscriptions. There is not any data that we are wrongly supplying. I assure you 100% there is not one duplicate value.

I am happy to get into a call and talk it out if you like. Again, I request you re-open this bug or I will just keep submitting until I get some traction.

@tombuildsstuff
Copy link
Contributor

@BigFrog-coding

Whilst I can understand you’re frustrated that this issue has been closed, please refrain from re-opening duplicates for this issue. Any duplicates will need to be closed in favour of this issue, and this work ultimately distracts maintainers from working on PRs and issues.

As mentioned above: based on the Terraform Configuration provided in the issue description which returns the error “Two different items produced the key” - this is a configuration issue.

Now - as you’ve alluded to in your last comment, there may well be another issue at play here - but the error message being returned here is only returned from Terraform Core when a map contains multiple keys with the same name, since a map must contain unique keys.

In order for us to diagnose any separate issue, we’d need a reproducible Terraform Configuration (and ideally, debug logs) - so if you can provide that then we’re happy to take another look.

However without that additional information we’re unable to ascertain if there’s another issue at play here, and we’re instead left with the error message being returned from Terraform Core:

Two different items produced the key "/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c" in this 'for' expression. If duplicates are expected, use the ellipsis (...) after the value expression to enable grouping by key.

As such whilst I appreciate that you’re frustrated here, without a reproducible Terraform Configuration showing the issue coming from another Data Source/Resource - and/or without any debug logs, there isn’t much more we can do but look at the error message coming back from Terraform Core here.

Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants