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

azuread_group recreates groups when they are (manually) turned into dynamic groups #576

Closed
rafabu opened this issue Sep 16, 2021 · 3 comments

Comments

@rafabu
Copy link

rafabu commented Sep 16, 2021

Community Note

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

Terraform (and AzureAD Provider) Version

Terraform v1.0.7
on windows_amd64
+ provider registry.terraform.io/hashicorp/azuread v2.2.1

Affected Resource(s)

  • azuread_group

Terraform Configuration Files

terraform {
  backend "local" {}
  required_version = "~> 1.0"
  required_providers {
    azuread = {
      source  = "hashicorp/azuread"
      version = "~> 2.2"
    }
  }
}

provider "azuread" {
  alias     = "dev"
}

resource "azuread_group" "group-to-be-made-dynamic" {
  provider = azuread.dev

  display_name            = "~~testing~~ make this dynamic later"
  description             = "test group for terraform azuread 2.x"
  assignable_to_role      = false
  prevent_duplicate_names = true
  security_enabled        = true
  visibility              = "Private"
}

output "group_resource" {
  value = azuread_group.group-to-be-made-dynamic
}

Output

terraform plan
azuread_group.group-to-be-made-dynamic: Refreshing state... [id=2b219e17-1405-414b-82ec-96b86461469d]
╷
│ Error: An existing "azuread_group" with name "~~testing~~ make this dynamic later" (ID: "2b219e17-1405-414b-82ec-96b86461469d") was found and `prevent_duplicate_names` was specified. To be managed via Terraform, this resource needs to be imported into the State. Please see the resource documentation for "azuread_group" for more information.
│

Expected Behavior

Running terraform plan or terraform apply should still find the azuread_group it created (or was imported) earlier , regardless of the object's membershipRule setting. This was the behaviour of provider <= 1.6.

Actual Behavior

Terraform does not detect the group any longer and aims at creating a fresh one. Depending on the setting of prevent_duplicate_names it will either create an additional copy or refuse the creation (see output above).

Steps to Reproduce

  1. terraform apply
  2. manually change the created AzureAD group to have a dynamic membership rule (any will do)
  3. terraform plan

Important Factoids

I am porting terraform workflows to provider 2.x. In some cases groups created initially via terraform, are turned into dynamic membership groups by a Graph API call after terraform apply.
Nonetheless those groups are kept in terraform state in order to support referencing them (e.g. for azurerm RBAC rules) as well as to support future display_name, description and owner changes.
This process works perfectly on azuread provider <= 1.6
I do realize that actual support for membershipRule might be on the roadmap already but as long as this is not implemented, the provider should also consider and check if groups are already in its state regardless of their membershipRule attribute.

@manicminer
Copy link
Contributor

manicminer commented Sep 16, 2021

Hi @rafabu, thanks for opening this issue. As you have posited, this incompatibility stems from introduction of support for unified groups (M365 groups) and the way in which the API overloads these two concerns in a single property.

Whilst we could add a workaround to avoid recreating groups with dynamic membership, such a workaround would have to be removed when we soon add proper support for dynamic memberships, and that removal would be a breaking change. Unfortunately that means we won't be able to work around this in the immediate term.

I would suggest considering the azuread_group data source as a workaround in the meantime, or use a lifecycle block to force the provider to ignore the type property, for example:

resource "azuread_group" "test" {
  display_name = "dynamic-group-test"
  security_enabled = true

  lifecycle {
    ignore_changes = ["types"]
  }
}

I'd also recommend subscribing to #132 as that issue will be updated when we add support for dynamic memberships. For now, I'm going to close this one as there's unfortunately no stable workaround that we can implement in the meantime.

@rafabu
Copy link
Author

rafabu commented Sep 16, 2021

Thank you very much @manicminer
Excluding the types attribute on the security groups does serve as a perfect workaround for now.

@github-actions
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 Oct 17, 2021
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

2 participants