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

azurerm_monitor_diagnostic_setting re applies every time the log_analytics_destination_type property on plan/apply #6676

Closed
andymac4182 opened this issue Apr 28, 2020 · 7 comments
Assignees
Labels
bug upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR waiting-response

Comments

@andymac4182
Copy link

Terraform (and AzureRM Provider) Version

Terraform v0.12.24

  • provider.azuread v0.8.0
  • provider.azurerm v2.7.0

Affected Resource(s)

  • resource azurerm_monitor_diagnostic_setting

Terraform Configuration Files

data "azurerm_monitor_diagnostic_categories" "diagnostic_categories" {
  resource_id = var.target_resource_id
}

resource "azurerm_monitor_diagnostic_setting" "diagnostic_settings" {
  name                           = "Send all to log analytics"
  target_resource_id             = var.target_resource_id
  log_analytics_workspace_id     = var.log_analytics_workspace_id
  log_analytics_destination_type = "Dedicated"

  dynamic "log" {
    for_each = data.azurerm_monitor_diagnostic_categories.diagnostic_categories.logs

    content {
      category = log.value
      enabled  = true
      retention_policy {
        enabled = false
      }
    }
  }

  dynamic "metric" {
    for_each = data.azurerm_monitor_diagnostic_categories.diagnostic_categories.metrics

    content {
      category = metric.value
      enabled  = true
      retention_policy {
        enabled = false
      }
    }
  }
}

Debug Output

https://gist.github.com/andymac4182/9e5382bbc0450d1a89d6b5022ad56f1a

Expected Behavior

log_analytics_destination_type is persisted

Actual Behavior

log_analytics_destination_type keeps saying its being applied every plan/apply

Steps to Reproduce

  1. terraform apply
  2. terraform plan
  3. terraform apply
@magodo magodo self-assigned this Apr 29, 2020
@magodo magodo added the bug label Apr 29, 2020
@magodo
Copy link
Collaborator

magodo commented Apr 30, 2020

@andymac4182 Thank you for letting us know this issue 👍
Could you please tell which kind of resource is the one of target_resource_id (also can't tell in your debug output)?

I have made some local test, for some resources (e.g. azurerm_data_factory), the log_analytics_destination_type is persisted. While for some other resources (e.g. azurerm_service_app), the field is not persisted. So it is more like a case-by-case azure issue here.
The reason why the field doesn't persist for some resource is that the Read (GET) will return a nil for that field (where nil means default destination).
For the particular case, azurerm_service_app, if you open the Portal you would see there is a "preview" sign beside the "Diagnostic settings" button. Seems there could be some limitation for that resource to be able to export dedicated type to Log Analytics.

So what we can do is:

  • Either check if there is some API to tell if a certain kind of resource is able to export dedicated type to Log Analytics, then we will depend on that information to error out from terraform if the current resource doesn't support the specified type.
  • Or ask the service team to not ignore the user input in API, error out in API level.

@andymac4182
Copy link
Author

We set it to dedicated because it was still reporting before we added dedicated. We added dedicated thinking that would of helped.

Here is the resources I have seen affected.

  • Microsoft.KeyVault (Key Vault)
  • Microsoft.Web/sites (Web App & Slots)
  • Microsoft.Sql/servers/{}/databases (Azure SQL Database)
  • Microsoft.Network/frontdoors (Front Door)

@ghost ghost removed the waiting-response label Apr 30, 2020
@magodo
Copy link
Collaborator

magodo commented Apr 30, 2020

@andymac4182 Thank you for providing the background!
I just tested on azurerm_app_service with log_analytics_destination_type absent in azurerm_monitor_diagnostic_setting. There is no plan diff after an apply.
Would you please check again on your side? Or is there any discrepancy between us?

@andymac4182
Copy link
Author

image
Confirmed once removed it affects API Management as setting to null every time

@ghost ghost removed the waiting-response label Apr 30, 2020
@magodo
Copy link
Collaborator

magodo commented May 5, 2020

@andymac4182 That feels like an Azure skew. Currently, the validation in the provider doesn't allow values other than "Dedicated". I submitted a PR to relax the constraints a bit to work around those skew.

@magodo
Copy link
Collaborator

magodo commented May 9, 2020

@andymac4182 I am going to close this issue as the PR has been merged, you shall address this issue in the next release of the provider.
If there are other issues, please feel free to open another issue :)

@magodo magodo closed this as completed May 9, 2020
@ghost
Copy link

ghost commented Jun 8, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Jun 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR waiting-response
Projects
None yet
Development

No branches or pull requests

2 participants