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: 2.37.0 > 2.37.1 causes group replacement #1072

Closed
Official-James opened this issue Apr 18, 2023 · 7 comments · Fixed by #1076
Closed

azuread_group: 2.37.0 > 2.37.1 causes group replacement #1072

Official-James opened this issue Apr 18, 2023 · 7 comments · Fixed by #1076

Comments

@Official-James
Copy link

Official-James commented Apr 18, 2023

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

AzureAD: 2.37.0 > 2.37.1

Affected Resource(s)

Seems to be all azuread resources

Terraform Configuration Files

terraform {
  backend "azurerm" {}
  required_providers {
    azuread = {
      version = "2.37.1" #Originally 2.27.0
    }
  }
}

Expected Behavior

No destroy/create of resources

Actual Behavior

All resources will destroy and create (force replaced) due to moving from version 2.37.0 to 2.37.1. It seems to be the following causing this:

- onpremises_group_type          = "UniversalSecurityGroup" -> null # forces replacement

Steps to Reproduce

  1. Already run terraform apply using provider version 2.37.0
  2. Amend provider version to 2.37.1
  3. Run terraform apply

Important Factoids

Yesterday's release of 2.37.0 has a bug that was introduced. During this time, resources were deployed, which amended all azuread resources. Now moving to anything other than 2.37.0 results in all resources being destroyed and recreated.

References

  • 1067 - was found to identify the problem and fix was implemented.
@manicminer
Copy link
Contributor

@Official-James Thanks for reporting this issue. As you have accurately summarised, this breaking change was reluctantly introduced in order to fix a bug introduced with v2.37.0. Ordinarily this would not have as much of an issue, but in this case, the API has a bug which makes it impossible to unset this field.

Unfortunately, working around this any other way would have created other side effects and simply postponed the problem to a future release where it's likely that more practitioners would be affected. However, there are a couple of options available to you if you are now presented with this unexpected diff.

The first and easiest option would be to allow Terraform to delete and recreate the groups. Assuming that you are using Terraform to manage manual group memberships and other dependencies such as role assignments, this will momentarily break your setup but should restore everything in a desired state after the apply is completed.

The second option, which you may wish to use if recreating the group is going to cause you additional problems (such as hardcoding the uuid elsewhere, or managing the group out of band by other unrecoverable means), is to use the ignore_changes lifecycle option to have Terraform ignore this property. For example:

terraform {
  required_providers {
    azuread = {
      source  = "hashicorp/azuread"
      version = "2.37.1"
    }
  }
}

resource "azuread_group" "test" {
  display_name     = "aaatest"
  security_enabled = true

  lifecycle {
    ignore_changes = [onpremises_group_type]
  }
}

After which, running Terraform apply should suppress the unwanted diff, going from this:

~/tmp/azuread-issue-1072 12s
❯ terraform apply
azuread_group.test: Refreshing state... [id=b25f6ae7-1087-40ce-b27f-231a5e2f788b]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # azuread_group.test must be replaced
-/+ resource "azuread_group" "test" {
      - administrative_unit_ids        = [] -> null
      - assignable_to_role             = false -> null
      ~ auto_subscribe_new_members     = false -> (known after apply)
      - behaviors                      = [] -> null
      ~ external_senders_allowed       = false -> (known after apply)
      ~ hide_from_address_lists        = false -> (known after apply)
      ~ hide_from_outlook_clients      = false -> (known after apply)
      ~ id                             = "b25f6ae7-1087-40ce-b27f-231a5e2f788b" -> (known after apply)
      + mail                           = (known after apply)
      - mail_enabled                   = false -> null
      ~ mail_nickname                  = "108b5dfd-7" -> (known after apply)
      ~ members                        = [] -> (known after apply)
      ~ object_id                      = "b25f6ae7-1087-40ce-b27f-231a5e2f788b" -> (known after apply)
      + onpremises_domain_name         = (known after apply)
      - onpremises_group_type          = "UniversalSecurityGroup" -> null # forces replacement
      + onpremises_netbios_name        = (known after apply)
      + onpremises_sam_account_name    = (known after apply)
      + onpremises_security_identifier = (known after apply)
      ~ onpremises_sync_enabled        = false -> (known after apply)
      ~ owners                         = [
          - "3aa04c8c-5a75-4e5e-9117-1b7cf6f33e21",
        ] -> (known after apply)
      + preferred_language             = (known after apply)
      - provisioning_options           = [] -> null
      ~ proxy_addresses                = [] -> (known after apply)
      - types                          = [] -> null
      + visibility                     = (known after apply)
        # (4 unchanged attributes hidden)

      - timeouts {}
    }

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

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value:

to this:

~/tmp/azuread-issue-1072 12s
❯ terraform apply
azuread_group.test: Refreshing state... [id=b25f6ae7-1087-40ce-b27f-231a5e2f788b]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

~/tmp/azuread-issue-1072
❯ 

Apologies for the situation this has clearly presented you with, as mentioned alas this was the least worst option available to us, after the original bug was unintentionally introduced in last week's release.

@manicminer manicminer changed the title 2.37.0 > 2.37.1 results in all resources being destroyed azuread_group: 2.37.0 > 2.37.1 causes group replacement Apr 18, 2023
manicminer added a commit that referenced this issue Apr 18, 2023
@AtzeDeVries
Copy link

Although this was a bug fix, but it ends up creating braking changes unless you change your local code. I wonder if a bugfix semver bump was the right thing here.

@lra
Copy link

lra commented Apr 19, 2023

@manicminer Thanks, for the detailed explanations on both tickets (other is #1067). I'm switching here as this better represents the problem we now have.

From what you are saying, for those like us who can't realistically recreate those AD groups, it looks like fixing the Azure API would also fix the this behavior.

Where can we push for this API bug to be fixed? If this is a known issue, do you know where we can put our weigh to help get this bug fix? We are a rather large Azure customer, so this might help getting this prioritized.

@HorizonNet
Copy link
Contributor

@manicminer Thanks for the insights. Looking a little bit down the road, if we now apply the second option, is there a chance that we can remove the ignore configuration at a later point in time again if the API bug has been fixed and with that it would be possible to unset this value?

A little bit of background: We manage several hundred groups via the provider, which are used in group provisioning in enterprise applications, as well as manual member assignment and device management. For us recreating all groups would mean that everything is essentially broken with an unreasonable amount of resources needed to get everything into the correct place again.

@manicminer
Copy link
Contributor

@Official-James, @AtzeDeVries, @lra, @HorizonNet After further extensive testing, it looks like we can work around this in a nondestructive manner and remove the conditional ForceNew. Appreciate your feedback and understanding with this issue. This additional fix is included in #1076 and we're planning to issue a second patch release v2.37.2 to incorporate this and restore continuity for those with affected groups.

I've opened an upstream issue microsoftgraph/msgraph-metadata#332 to report the API bug, although it's not as critical right now since we have a workaround.

As part of remediation to try and avoid this kind of situation arising in future, I have made further test fixes, and we will be improving our testing regime and environment, particularly with the azuread_group resource.

@manicminer manicminer added this to the v2.37.2 milestone Apr 19, 2023
@github-actions
Copy link

This functionality has been released in v2.37.2 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@lra
Copy link

lra commented Apr 20, 2023

Confirmed to work with 2.37.2, thanks a lot!

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

Successfully merging a pull request may close this issue.

5 participants