Azure / terraform-azurerm-avm-res-web-site

MIT License
13 stars 10 forks source link

[AVM Module Issue]: Function app using managed identity - the module keeps recreating the AzureWebJobsDashboard and AzureWebJobsStorage environment variables #96

Closed tb2186 closed 3 months ago

tb2186 commented 4 months ago

Check for previous/existing GitHub issues

Issue Type?

I'm not sure

(Optional) Module Version

0.7.1

(Optional) Correlation Id

No response

Description

Sorry for the long post TL;DR When I use this module it always (re)inserts the AzureWebJobsDashboard and AzureWebJobsStorage environment variables with every apply after I've deleted those to make app managed identity work. It also seems like it can't track the storage_uses_managed_identity and builtin_logging_enabled settings because every plan and apply shows that those variable will be changed when they're not.

~ storage_uses_managed_identity                  = true -> false
...
~ builtin_logging_enabled                        = false -> true

In order for a function app to use its system assigned managed identity to access it's associated storage account, we are supposed to remove the AzureWebJobsDashboard and AzureWebJobsStorage environment variables. Whenever I build a new function app with this module those are created automatically. If I then manually remove them and instead configure the 'AzureWebJobsStorage__accountName' environment variable ( I do this in terraform under app_settings) I can restart the app and it works.

These references talk about how to use managed identity in a function app and once I remove the settings, managed identity works fine and the app can do what it needs in its storage account:

https://learn.microsoft.com/en-us/azure/azure-functions/functions-reference?tabs=blob&pivots=programming-language-csharp#connecting-to-host-storage-with-an-identity

https://techcommunity.microsoft.com/t5/apps-on-azure-blog/use-managed-identity-instead-of-azurewebjobsstorage-to-connect-a/ba-p/3657606

The problem comes when I run terraform apply after I get my app working by removing the settings. Terraform apply will recreates the environment variables without mentioning that it will be doing that during the plan. Once those variables are set again and the app restarts, I have issues in the portal for the app since the app thinks it's using storage keys again instead of managed identity.

This is my env vars before I do terraform apply:

$ az functionapp config appsettings list --name myapp --resource-group rg-my-app | grep -i azurewebjobs
    "name": "AzureWebJobsStorage__accountName",
$

The plan doesn't mention that it will be adding the new variables:

  # module.avm-res-web-site-myapp.azurerm_windows_function_app.this[0] will be updated in-place
  ~ resource "azurerm_windows_function_app" "this" {
      ~ app_settings                                   = {
          + "AzureWebJobsStorage__accountName"            = "stomystorageaccount"
            # (72 unchanged elements hidden)
        }
      ~ builtin_logging_enabled                        = false -> true
        id                                             = "/subscriptions/nnn/resourceGroups/rg-my-app/providers/Microsoft.Web/sites/myapp"
        name                                           = "myapp"
      ~ storage_uses_managed_identity                  = true -> false
        tags                                           = {}
        # (29 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

This is the list of env vars after I run apply with no other changes made in the portal etc.

$ az functionapp config appsettings list --name myapp --resource-group rg-my-app | grep -i azurewebjobs
    "name": "AzureWebJobsStorage",
    "name": "AzureWebJobsDashboard",
    "name": "AzureWebJobsStorage__accountName",
$

These are my configs:

module "avm-res-web-site-myapp" {
  source                                     = "Azure/avm-res-web-site/azurerm"
  version                                    = "0.7.1"
  kind                                       = "functionapp"
  resource_group_name                        = azurerm_resource_group.mobile_resource_group.name
  location                                   = azurerm_resource_group.mobile_resource_group.location
  name                                       = local.utmobilename
  os_type                                    = "Windows"
  service_plan_resource_id                   = data.azurerm_service_plan.myasp.id
  create_service_plan                        = false
  enable_application_insights                = true
  function_app_create_storage_account        = false
  function_app_storage_uses_managed_identity = true
  function_app_storage_account_name          = module.avm-res-storage-storageaccount-mystorageaccount.name
  enable_telemetry                           = false
  https_only                                 = true
  builtin_logging_enabled                    = false

  managed_identities = {
    system_assigned = true
  }

  site_config = {
    scm_basic_auth_enabled = false
    ftp_basic_auth_enabled = false
    http2_enabled          = true
    minimum_tls_version    = "1.2"
    always_on              = true
    application_stack = {
      windows = {
        dotnet_version              = "v8.0"
        use_dotnet_isolated_runtime = true
      }
    }
  }

# this is the setting used to tell Azure that the app will be using managed identity to access this storage account.
  app_settings = {
    "AzureWebJobsStorage__accountName"            = module.avm-res-storage-storageaccount-mystorageaccount.name
donovm4 commented 3 months ago

Hi @tb2186 - apologies for the late response, have been pulled into customer work. Will look into this shortly!

lorenzotan commented 3 months ago

I experienced the same issue and solved it by setting storage_uses_managed_identity to true.

Are you using the correct setting in your module? It looks like function_app_storage_uses_managed_identity needs to be renamed to storage_uses_managed_identity.

tb2186 commented 3 months ago

@lorenzotan I'm not sure how you're able to specify that variable.

This is the code in variables.tf:

variable "function_app_storage_uses_managed_identity" {
  type        = bool
  default     = false
  description = "Should the Storage Account use a Managed Identity?"
}
lorenzotan commented 3 months ago

@tb2186 ,

Looking at the code, storage_uses_managed_identity is defined here.

It is set based on the values of function_app_storage_uses_managed_identity and function_app_storage_account_access_key which appear to be correctly set to true and null respectively.

However, function_app_storage_account is defaulted to an empty map instead of null. It looks like the logic to set storage_uses_managed_identity needs to change or you need to explicitly set function_app_storage_account to null.

donovm4 commented 3 months ago

@lorenzotan + @tb2186:

My first step will likely be to create an example with the next release.

I believe I will have to review the following logic and determine if it is better to change the logic myself or if it is easier to explicitly set function_app_storage_account to null:

storage_uses_managed_identity                  = var.function_app_storage_uses_managed_identity == true && var.function_app_storage_account_access_key == null && var.function_app_storage_account == null ? var.function_app_storage_uses_managed_identity : null

I will keep the issue open for further conversation/feedback until we get this resolved 😄

lorenzotan commented 3 months ago

@donovm4 ,

I just learned about this and wanted to share. It looks like there's a way to validate your variables and raise errors if they're invalid. It feels easier to read than to place the logic inside the resource block. I hope you find this useful.

https://developer.hashicorp.com/terraform/language/expressions/custom-conditions#input-variable-validation

donovm4 commented 3 months ago

@tb2186 + @lorenzotan:

In my investigation, I believe I have solved the issue. Here is my thought process...

Current Logic

storage_uses_managed_identity                  = var.function_app_storage_uses_managed_identity == true && var.function_app_storage_account_access_key == null && var.function_app_storage_account == null ? var.function_app_storage_uses_managed_identity : null

Something important to note is that var.function_app_storage_account is technically an object where its value is:

{
  name = null
  resource_group_name = null
  location = null
  lock = null
}

Problem

Essentially length(keys(function_app_storage_account)) will always be 4 in this case and function_app_storage_account value will never actually be null, unless explicitly set to null...

BUT... function_app_storage_account should not be set to null as it will kick off an error with the azurerm_management_lock feature for the storage account.

Solution

The solution will be to have something like:

storage_uses_managed_identity                  = var.function_app_storage_uses_managed_identity == true && var.function_app_storage_account_access_key == null ? var.function_app_storage_uses_managed_identity : null

This way it should allow for easier implementation of the module's created function app to leverage storage_uses_managed_identity and role_assignments use cases easier with storage_account_access_key existing logic.

I will release a new version along with an example how I have tested it. Would you be able to test with new release to ensure logic works as expected? (@tb2186)

Notes:

@lorenzotan - I will take a look at the documentation you have linked regarding variable conditions.

donovm4 commented 3 months ago

In addition to this logic change, the app settings should automatically change AzureWebJobsStorage to AzureWebJobsStorage__accountname with this fix.

tb2186 commented 3 months ago

@donovm4 I can test. I had found that code you linked to and had tried setting storage account and keys null but none of the permutations seemed to work.

donovm4 commented 3 months ago

Hi @tb2186 - please review the latest release (v0.7.3)

tb2186 commented 3 months ago

Thanks @donovm4 . The new version is working better and no longer reinserts the AzureWebJobsDashboard and AzureWebJobsStorage environment variables