role_defintion is now including the scope as part of the assignable_scopes causing constant removals

[tonedefdev]

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

Terraform (and AzureRM Provider) Version

Terraform v0.12.24

• provider.azuread v1.0.0
• provider.azuredevops v0.0.1
• provider.azurerm v2.28.0
• provider.null v2.1.2
• provider.random v2.3.0
• provider.vault v2.14.0

Affected Resource(s)

• azurerm_role_definition

Terraform Configuration Files

resource "azurerm_role_definition" "example" {
  count              = var.resource_count > 0 ? 1 : 0
  name               = "example-iam"
  role_definition_id = random_uuid.example.result
  scope              = data.azurerm_subscription.primary.id

  description = "This is a sample custom rule set"

  permissions {
    actions          = module.permissions.allow
    not_actions      = module.permissions.deny
    data_actions     = []
    not_data_actions = []
  }

  assignable_scopes = [var.resource_group_id] 
}

Expected Behavior

Assignable scopes should only include the var.resource_group_id or the assignable_scopes that are defined.

Actual Behavior

After viewing the state file both the var.resource_group_id and the scope data.azurerm_subscription.primary.id are shown in the assignable_scopes. This causes each subsequent run to try and remove the data.azurerm_subscription.primary.id or whatever is in the scope reference.

Steps to Reproduce

1. terraform apply

Important Factoids

I was digging through the provider code and I believe the issue is here. Prior to 2.27 there was never a check for assignedScope as part of the assignableScopes. Here's the code from 2.26:

func expandRoleDefinitionAssignableScopes(d *schema.ResourceData) []string {
	scopes := make([]string, 0)

	assignableScopes := d.Get("assignable_scopes").([]interface{})
	for _, scope := range assignableScopes {
		scopes = append(scopes, scope.(string))
	}

	return scopes
}

Now it's making a check to see if the assignableScopes contains the assignedScope which appears to actually be adding the scope now:

func expandRoleDefinitionAssignableScopes(d *schema.ResourceData) []string {
	scopes := make([]string, 0)

	// The first scope in the list must be the target scope as it it not returned in any API call
	assignedScope := d.Get("scope").(string)
        // This is now adding the assignedScope to the list scopes
	scopes = append(scopes, assignedScope) 
	assignableScopes := d.Get("assignable_scopes").([]interface{})
	for _, scope := range assignableScopes {
		// Ensure the assigned scope is not duplicated in the list if also specified in `assignable_scopes`
		if scope != assignedScope {
			scopes = append(scopes, scope.(string))
		}
	}

	return scopes
}

[magodo]

@tonedefdev Thank you for submitting this!

It is odd to introduce this change in this PR. Maybe @jackofallops could provide some insight on this?

On the other hand, I'll try to submit a PR to roll this back...

[tonedefdev]

@tonedefdev Thank you for submitting this!

It is odd to introduce this change in this PR. Maybe @jackofallops could provide some insight on this?

On the other hand, I'll try to submit a PR to roll this back...

Awesome! Thank you @magodo!

[jackofallops]

It is a bug, the test config that should have covered it actually hid it as the same value was used for scope as assignable_scopes so I missed it, my bad, sorry. I'll check the PR, I think we can resolve it without the breaking change of going from Optional to Required.

[tonedefdev]

@magodo or @jackofallops - is there any update on when this will be added? I was hoping the next release of the provider would include this fix but that doesn't appear to be the case. This is a very annoying bug that clutters output of runs when you have a lot of role assignments, but we need to also use the ->2.28 provider version because it fixes a lot of other bugs with AKS.

[fgarcia-cnb]

@magodo and @jackofallops any updates? its been over a month since this was reported. this is very annoying, you guys seem to have a fix already, and there have been several provider releases

[magodo]

Ping @jackofallops, anything else I need to do to get the PR merged?

[fgarcia-cnb]

still waiting for this fix. any updates?

[fgarcia-cnb]

@jackofallops @magodo please, can we get this fix merged already?

[magodo]

I've resolved the conflicts now, but I guess this PR is blocked because it introduces breaking changes, which will not be ideal to merge until a major version bump.

[fgarcia-cnb]

is it possible to fix without introducing breaking changes?

[ghost]

This has been released in version 2.45.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.45.0"
}
# ... other configuration ...

[ghost]

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!