terraform-provider-azuread icon indicating copy to clipboard operation
terraform-provider-azuread copied to clipboard

`azuread_access_package_assignment_policy`: change `requestors` type from List to Set

Open bigwheel opened this issue 2 years ago • 13 comments

Resolve #1116

What I tested

  • [x] make test
  • [x] Fixed actual behavior and become expected hehavior (no diffs)

Test steps

  1. Clone provider code
  2. Change the code
  3. make build
  4. terraform init and terraform plan several times. Check plans show requestor diffs.
  5. Override azuread provider
provider_installation {
  dev_overrides {
    "hashicorp/azuread" = "/home/kbigwheel/.asdf/installs/golang/1.19.4/packages/bin"
  }
  direct {}
}
  1. terraform init and terraform plan several times. Check plans don't show requestor diffs.

bigwheel avatar Jun 11 '23 04:06 bigwheel

Thanks for suggesting this change @bigwheel. In my testing I have not been able to reproduce this as yet, can you confirm if this happens repeatably or intermittently? If you update your configuration to match the order of requestors, does the diff go away (and not return after a few plans)?

In principle I'd be happy to make this change, however changing a property from a TypeList to a TypeSet is usually considered a breaking change, since a property might be referenced elsewhere in users' configurations. This means we normally only make such a change with a new major version of a provider, e.g. v2.x -> v3.0.

manicminer avatar Jul 12 '23 22:07 manicminer

@manicminer Thank you for reply!

In my testing I have not been able to reproduce this as yet, can you confirm if this happens repeatably or intermittently? If you update your configuration to match the order of requestors, does the diff go away (and not return after a few plans)?

No, it doesn't. I applied several times but diff still remained yet.

This means we normally only make such a change with a new major version of a provider, e.g. v2.x -> v3.0.

Agree. we were added requestor to ignore_changes list. Therefore, it is not ciritical problem at least now.

bigwheel avatar Jul 13 '23 02:07 bigwheel

Thanks for the feedback. Agreed this should be a TypeSet, as mentioned we'll fix this in v3.0.

manicminer avatar Jul 13 '23 11:07 manicminer

Thanks @manicminer. I don't suppose you have a rough ETA on v3.0 yet? Are we talking days/weeks/months?

tjrobinson avatar Aug 14 '23 10:08 tjrobinson

Unfortunately we don't have a timeframe as yet. There is a lot of ground work going on right now in preparation for it, but I'm afraid can't offer an estimate at this point in time.

manicminer avatar Aug 14 '23 20:08 manicminer

Hey, any update on this? We are currently scaling up the use of terraformed access packages, and this is really messing up the plans :)

sklakegg avatar Sep 20 '23 11:09 sklakegg

This would be nice to get merged. I'd prefer not to have to use the ignore_changes workaround, especially with security related infra. Additionally, https://github.com/hashicorp/terraform/issues/5666 means I have to ignore the entire requestor_settings block and not just the requestor field.

seanhoughton avatar Jun 13 '24 15:06 seanhoughton