search

hashicorp / terraform-provider-azurerm

Terraform provider for Azure Resource Manager
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs
Mozilla Public License 2.0
4.6k stars 4.64k forks source link

two runs needed to add new app service to AD group #21632

Closed fgarcia-cnb closed 1 year ago

fgarcia-cnb commented 1 year ago

Is there an existing issue for this?

Community Note

Terraform Version

1.0.7

AzureRM Provider Version

3.50.0

Affected Resource(s)/Data Source(s)

azurerm_linux_web_app, azurerm_windows_web_app

Terraform Configuration Files

We have a module that builds app services for linux and windows. the relevant outputs look like this:

output "webapp_ids" {
  value = var.is_linux ? azurerm_linux_web_app.webapp_linux[*].id : azurerm_windows_web_app.webapp_windows[*].id
}

output "webapp_managed_identities" {
  value = var.is_linux ? azurerm_linux_web_app.webapp_linux[*].identity[0].principal_id : azurerm_windows_web_app.webapp_windows[*].identity[0].principal_id
}

These outputs are fed into another module with azuread_group resources (using var.object_ids):

resource "azuread_group" "group" {
  count            = var.resource_count > 0 ? 1 : 0
  display_name     = "g_${var.location}-${var.environment}${var.env_suffix}-${var.appname}-${var.name_suffix}"
  members          = var.object_ids
  security_enabled = true
}

Debug Output/Panic Output

n/a

Expected Behaviour

when an additional app service is created with count, terraform should know the output of the module will change, and thus the group members of the AD group will also change.

Actual Behaviour

the additional app service creation and adding it to the ad_group member should all happen within 1 run, but it takes 2 runs... the first run creates the app service, and the AD group wont see the generated principal id until the second run

am i misunderstanding something?

Steps to Reproduce

No response

Important Factoids

No response

References

No response

fgarcia-cnb commented 1 year ago

is this related to https://github.com/hashicorp/terraform-provider-azurerm/issues/13490 ??

xiaxyi commented 1 year ago

Thanks @fgarcia-cnb for raising this issue, may I know if you are able to output the app service identity information using below example:

output "appservice-msi" {
    value = azurerm_linux_web_app.noauth.identity[0].principal_id
}
xiaxyi commented 1 year ago

is this related to #13490 ??

The issue is about managed identity not being recognized by kv access policy if the msi is not enabled during the resource creation.

As I can tell, you enabled the managed identity during creation. am I correct?

fgarcia-cnb commented 1 year ago

well its a bit similar. you are correct, the managed identitiy is being created at web app creation. the principal id is an output of the module. that output is fed into the azuread_group members parameter

when the count value increases, the plan shows the app service being built, but it doesnt show the azuread_group members parameter changing. but it should detect that the webapp_managed_identities output will change, and should show as "known after apply". you have to do a 2nd plan/apply for the AD group to be updated. 

### module "webapp"
resource "azurerm_windows_web_app" "webapp_windows" {
  count               = var.is_linux == false ? var.webapp_count : 0
  name                = "${var.group_prefix}webapp${count.index}"
  resource_group_name = var.resource_group_name

  location = local.region[var.location]
  tags     = local.tags

  client_certificate_mode = var.webapp_client_cert_mode
  https_only              = var.webapp_https_only
  service_plan_id         = var.app_service_plan_id

  app_settings = merge({
    "WEBSITE_VNET_ROUTE_ALL" = "1"
  }, var.additional_webapp_settings)

  identity {
    type = "SystemAssigned"
  }

  site_config {
    always_on              = var.webapp_always_on
    vnet_route_all_enabled = true
    ftps_state             = "FtpsOnly"
    http2_enabled          = true
  }

  lifecycle {
    ignore_changes = [
      app_settings,
      auth_settings,
      backup,
      client_affinity_enabled,
      client_certificate_enabled,
      connection_string,
      site_config,
      sticky_settings,
      virtual_network_subnet_id,
    ]
  }
}

output "webapp_managed_identities" {
  value = azurerm_windows_web_app.webapp_windows[*].identity[0].principal_id
}

###module aadgroup
resource "azuread_group" "group" {
  count            = var.resource_count > 0 ? 1 : 0
  display_name     = "g_${var.location}-${var.environment}${var.env_suffix}-${var.appname}-${var.name_suffix}"
  members          = var.object_ids
  security_enabled = true
}

###

module "webapp" {
  source              = "localterraform.com/bats/app-services/azurerm"
  version             = "~> 1.0.0"
  resource_group_name = module.resource_group.name

  appname     = var.appname
  azure_tags  = var.azure_tags
  cost_center = var.cost_center
  environment = var.environment
  location    = var.location
  owner       = var.owner
  workspace   = var.workspace

  app_service_plan_id     = try(module.webapp_sp[0].app_service_plan_ids, "")
  create_webapp_storage   = false
  group_prefix            = "web-${module.naming.hostname_prefix}${random_string.hostname.result}"
  is_linux                = false
  kudu_group_id           = data.azuread_group.devops.id
  webapp_always_on        = true
  webapp_client_cert_mode = null
  webapp_count            = var.webapp_count
  webapp_subnet_id        = module.webapp_subnet.subnet_ids[0]
  webapp_vi_subnet_id     = module.webapp-vi_subnet.subnet_ids[0]

  providers = {
    azurerm     = azurerm
    azurerm.prd = azurerm.prd
  }

module "webapp_msigroup" {
  source         = "localterraform.com/bats/aadgroup/azurerm"
  version        = "~> 4.0.0"
  resource_count = var.webapp_count > 0 ? 1 : 0
  location       = var.location
  environment    = var.environment
  env_suffix     = var.env_suffix
  appname        = var.appname
  name_suffix    = "webapp-msi"
  object_ids     = module.webapp.webapp_managed_identities

When i increase the web app count from 5->6 i see the following resources being added, but no changes to the azure AD group until i apply and do a 2nd plan

image

fgarcia-cnb commented 1 year ago

maybe its related to this? https://github.com/hashicorp/terraform-provider-azuread/issues/431

similar behavior with datafactory

xiaxyi commented 1 year ago

@fgarcia-cnb I'm not sure about the current behavior of the inconsistence of the azure ad graph api given the issue was opened 2 years ago...

I tried to reproduce the issue from my side, unfortunately, I can't get the same result. Everything works fine from my end, the only difference is I didn't use module but just the simplest RP block.

resource "azurerm_linux_web_app" "noauth" {
  name = "tftestlwa-msi-733"
  location            = azurerm_resource_group.test.location
  resource_group_name = azurerm_resource_group.test.name
  service_plan_id = azurerm_service_plan.linux.id
  https_only = true

  site_config {}

  identity {
    type = "SystemAssigned"
  }
}

resource "azuread_group" "group" {
  display_name             = "${azurerm_linux_web_app.noauth.name}-group"
  security_enabled = true

  members = [
    azurerm_linux_web_app.noauth.identity[0].principal_id,
  ]
}

The member is added successfully to the azure ad group.

One thing I do noticed is that you are using name instead of display_name in azuread_group. https://registry.terraform.io/providers/hashicorp/azuread/latest/docs/resources/group#display_name

Can you get the config ran successfully by using an undefined property as I was getting this error: image

fgarcia-cnb commented 1 year ago

yea sorry i accidentally used outdated code... display_name is the correct parameter. i fixed the code in my previous comments. could you try a couple more things for me?

-add a count to the app service. then go from 1->2. does the AD group update correctly? -if that works, would you mind putthing your code in modules? and then rebuild, and go from 1->2 again

xiaxyi commented 1 year ago

How did you pass the object_id to azuread_group?

resource "azuread_group" "group" {
  count            = var.resource_count > 0 ? 1 : 0
  display_name     = "g_${var.location}-${var.environment}${var.env_suffix}-${var.appname}-${var.name_suffix}"
  members          = var.object_ids
  security_enabled = true
}
fgarcia-cnb commented 1 year ago

its in my code above... adding here again (the object_ids parameter):


module "webapp_msigroup" {
  source         = "localterraform.com/bats/aadgroup/azurerm"
  version        = "~> 4.0.0"
  resource_count = var.webapp_count > 0 ? 1 : 0
  location       = var.location
  environment    = var.environment
  env_suffix     = var.env_suffix
  appname        = var.appname
  name_suffix    = "webapp-msi"
  object_ids     = module.webapp.webapp_managed_identities

###module content
resource "azuread_group" "group" {
  count            = var.resource_count > 0 ? 1 : 0
  display_name     = "g_${var.location}-${var.environment}${var.env_suffix}-${var.appname}-${var.name_suffix}"
  members          = var.object_ids
  security_enabled = true
}
xiaxyi commented 1 year ago

the app services are added to the member list as expected, even i set the count to 5... image

I noticed that you mentioned "When i increase the web app count from 5->6 i see the following resources being added, but no changes to the azure AD group until i apply and do a 2nd plan"

are you suggesting that it worked if the web app count is 2？

BTW, you got the expected output, right? image

fgarcia-cnb commented 1 year ago

after a lot of testing, i found the issue. i previously cleaned up my posted code for simplicity, but i ended up accidentally removing the cause of the issue. i was actually passing the following to the ad group module:

object_ids = compact(concat(module.webapp.webapp_managed_identities, data.azuread_group.dr_webapp_msigroup[*].id))

for some reason, the compact function prevents changes. so in your example, try this:

members = compact(concat(azurerm_linux_web_app.noauth[*].identity[0].principal_id, []))

youll see the behavior im describing when increasing the web app count. if you remove compact and use

members = concat(azurerm_linux_web_app.noauth[*].identity[0].principal_id, [])

everything works again. can you confirm this bevavior?

fgarcia-cnb commented 1 year ago

the bottom half of this post describes the behavior of compact on "known after apply" values.

xiaxyi commented 1 year ago

Ah, if I use compact function, I can reproduce the issue....

xiaxyi commented 1 year ago

maybe we can use foreach, for example:

locals {
    webapp_managed_identities = azurerm_linux_web_app.noauth[*].identity[0].principal_id
}

resource "azuread_group" "group" {
  display_name             = "lwa-group"
  security_enabled = true

  members = [for i in local.webapp_managed_identities : i] 
}
fgarcia-cnb commented 1 year ago

i just removed compact... its not really necessary, it was just precautionary. thanks for the help. this ticket can be closed

xiaxyi commented 1 year ago

No problems @fgarcia-cnb , glad to see the issue is resolved. Would you mind closing this issue from your side? Thanks!

github-actions[bot] commented 5 months ago

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.