hashicorp / terraform-provider-azuread

Terraform provider for Azure Active Directory
https://registry.terraform.io/providers/hashicorp/azuread/latest/docs
Mozilla Public License 2.0
419 stars 288 forks source link

Inconsistencies between terraform configuration and states regarding group owners #757

Open piiertho opened 2 years ago

piiertho commented 2 years ago

Community Note

Terraform (and AzureAD Provider) Version

Terraform version: 1.0.6 Azure ad provider version: 2.18.0

Affected Resource(s)

Expected Behavior

When I add some owners to an azuread_group resource, it automatically adds the current user applying terraform to group owners, whithout the need to specify it.
This user is added is terraform state as one of the group owners. I expect on next apply that I should not have to modify group owners to add this user (we should not have to change terraform configuration to match state).

Actual Behavior

When I add some owners to an azuread_group resource, it automatically adds the current user applying terraform to group owners, whithout the need to specify it.
This user is added is terraform state as one of the group owners. When I apply a second time (without adding the user applying terraform to owners), it detects changes in configuration. It removes the user applying terraform from owners.

Steps to Reproduce

  1. Create a group with owners, without adding user applying terraform to those owners.
  2. Make a first terraform apply.
  3. When done, make a second apply and observe it finds differences with state and will remove current user applying terraform (without having made any configuration change).

References

I think this is linked to https://github.com/hashicorp/terraform-provider-azuread/issues/624 But I also think this is making group owner assignments just not usable. We have inconsistencies between configuration and terraform state, which is terraform anti pattern.

manicminer commented 2 years ago

Hi @piiertho, thanks for reporting this. Unfortunately group ownerships are quite complex and depend on a number of factors including the type of principal being used to execute Terraform, the privileges of that principal, and whether you are creating a group or updating.

In order to diagnose this further, please could you provide a complete debug log whilst reproducing this, which will show the API requests being made, along with your Terraform configurations at each apply step and a copy of the resulting unwanted diff. Thanks!

piiertho commented 2 years ago

Hi @manicminer I don't think I can post such logs for confidentiality reasons.
I'm using a classic user for this. Please give me the privileges you need to know if present to debug this. So that I can tell if account have them or not.

manicminer commented 2 years ago

Hi @piiertho, thanks for the quick reply. If you're using a user principal, can you advise the directory roles assigned for that user? If using any custom roles, we'd need to know the requisite permissions that make up the custom role.

Are you able to isolate this to a reproducible test case? I wouldn't expect you to post your internal configuration and logs but if you can trigger the same issue with a smaller example that would be ideal. Thanks!

piiertho commented 2 years ago

@manicminer It seems I don't have rights to see directory privilege, or account have none maybe. I made an other test: If I try to create my group for the first time, and specify account used when applying in owners, by setting data.azuread_client_config.current.object_id in it, I got:

Error: Creating group "my_group"
  on modules/azure_ad/group/main.tf line 1, in resource "azuread_group" "group":
   1: resource "azuread_group" "group" {
GroupsClient.BaseClient.Post(): unexpected status 400 with OData error:
Request_BadRequest: Request contains a property with duplicate values.

When I look to provider code, it seems to happen here: https://github.com/hashicorp/terraform-provider-azuread/blob/fef2a3399fc11a06af3da5a63a89e469530804f8/internal/services/groups/group_resource.go#L575 What I don't understand is why this user get added another time ? I see one place this is done, but can't understand why this condition would pass: https://github.com/hashicorp/terraform-provider-azuread/blob/fef2a3399fc11a06af3da5a63a89e469530804f8/internal/services/groups/group_resource.go#L560, as we should have more than 0 owners in it: https://github.com/hashicorp/terraform-provider-azuread/blob/fef2a3399fc11a06af3da5a63a89e469530804f8/internal/services/groups/group_resource.go#L533

manicminer commented 2 years ago

@piiertho The way we manage owners in groups is complicated and the API has recently changed its behavior (again) in a breaking way for some users/scenarios. The short explanation is that we specify the calling principal as the first owner intentionally to avoid orphaning the group, however the API seems to perform some opaque translation of the request and appends the caller again in some conditions, resulting in the "duplicate values" error.

I am planning to look at this soon to see what we can do to improve the user experience here; it is however tricky as since I mentioned the API behavior changes depending on what seems to be a couple of different factors (e.g. the type of principal that's authenticated, and potentially what permissions they have).

manicminer commented 2 years ago

In pursuit of your original reported problem of an unwanted diff, if you could provide some logs whilst reproducing, this would be of great help in narrowing down the cause of the issue. Thanks!

Teuvz commented 2 years ago

Hi, I've noticed that AzureAD now automatically adds me as owner of a group when creating it manually, I don't think it used to do it a few weeks back. Maybe Azure changed something on that side and it's what's causing the provider problem ?

piiertho commented 2 years ago

In pursuit of your original reported problem of an unwanted diff, if you could provide some logs whilst reproducing, this would be of great help in narrowing down the cause of the issue. Thanks!

I can't post logs from my company.
I think original problem is same as second one I mentionned.
What @Teuvz remarked might explain why this is happening.

rolandjohann commented 2 years ago

running into this as well. The problem is that azuread_group.owners automatically takes the current user id as owner. Specifying it explicitly causes the API to bail out with GroupsClient.BaseClient.Post(): unexpected status 400 with OData error: Request_BadRequest: Request contains a property with duplicate values.

This is somehow bad since normally the CI/CD system executes those terraform templates and depending on who executes them the list of owner object ids changes, which is a crazy, not controllable side effect and from terraform perspective not deterministic.

My observation is that a group with the current client id only will be created, but as soon as there are other ids in the list it fails.

Terraform v1.1.9
on darwin_arm64
+ provider registry.terraform.io/hashicorp/azuread v2.21.0
+ provider registry.terraform.io/hashicorp/azurerm v3.9.0

works

data "azuread_client_config" "current" {}

resource "azuread_group" "some_group" {
  display_name     = "some-group-name"
  security_enabled = true
  owners           = [data.azuread_client_config.current.object_id]
}

this fails

data "azuread_client_config" "current" {}

data "azuread_group" "owner_group" {
  object_id = "<some uuid>"
}

resource "azuread_group" "some_group" {
  display_name     = "some-group-name"
  security_enabled = true
  owners           = toset(concat([data.azuread_client_config.current.object_id], data.azuread_group.owner_group.members))
}

Is the issue at the Azure API, where the current client will be added automatically as owner?

EDIT: Since I'm facing this issue currently: azuread_service_principal needs to get all owners explicitly. If you create this resource, you are not the owner and can't change it. This is the behavior that should be expected. IMHO azuread_group ownership behavior is inconsistent and wrong.

manicminer commented 2 years ago

@rolandjohann You are correct, this is largely an API issue. We may be able to work around it but it will require lots of testing to make sure we don't break other users at the same time.

clbx commented 1 year ago

@rolandjohann what permissions does your user running terraform have when you run the module that works;

data "azuread_client_config" "current" {}

resource "azuread_group" "some_group" {
  display_name     = "some-group-name"
  security_enabled = true
  owners           = [data.azuread_client_config.current.object_id]
}

I am running this, and it gives me the same error of unexpected status 400 with OData error: Request_BadRequest: Request contains a property with duplicate values.