terraform-provider-azurerm

terraform-provider-azurerm copied to clipboard

Fixes https://github.com/hashicorp/terraform-provider-azurerm/issues/17843

@ms-henglu unfortunately setting these all of the time would breaks the use of ignore_changes for the addons - so we'd instead need to retrieve the current list of addons from the API and conditionally update these instead.

However, #17843 is ultimately an Azure Policy issue rather than a Terraform bug, the Azure Policy API should be passing through the (validly/previously configured) AzurePolicy addon to the Azure Policy being tested - so can you reach out to the Azure Policy team about that?

Thanks!

@tombuildsstuff @ms-henglu If a subsequent update on AKS will be missing the azure policy addon, isn't it going to be removed from the cluster?

Check this line: https://github.com/hashicorp/terraform-provider-azurerm/blob/6fafede2c501208d64e4b0007442a0cabf3ac910/internal/services/containers/kubernetes_cluster_resource.go#L1344

If any single of the addons change then the whole section of addonProfiles is overwritten:

	if d.HasChange("aci_connector_linux") || d.HasChange("azure_policy_enabled") || d.HasChange("http_application_routing_enabled") || d.HasChange("oms_agent") || d.HasChange("ingress_application_gateway") || d.HasChange("open_service_mesh_enabled") || d.HasChange("key_vault_secrets_provider") {
		updateCluster = true
		addOns := collectKubernetesAddons(d)
		addonProfiles, err := expandKubernetesAddOns(d, addOns, env)
		if err != nil {
			return err
		}
		existing.ManagedClusterProperties.AddonProfiles = *addonProfiles
	}

I think the maps should be merged or only the changed addon sections (map keys) should be updated.

@piotrgwiazda ~the Update is done via a PATCH API, if the addon isn't specified then it won't be updated (or removed)~ apologies - digging into that, it appears not anymore?

When parsing these out of the config this should be patching the properties returned from the API here: https://github.com/hashicorp/terraform-provider-azurerm/blob/6fafede2c501208d64e4b0007442a0cabf3ac910/internal/services/containers/kubernetes_cluster_resource.go#L1339-L1340 which'd solve this whilst allowing ignore_changes to work as expected here

@tombuildsstuff the collectKubernetesAddons returns Terraform configuration to the addOns map. Then expandKubernetesAddOns creates JSON API representation based on the addOns map. Then the whole section of JSON is overriden existing.ManagedClusterProperties.AddonProfiles = *addonProfiles The existing AddonProfiles is not taken into account.

Closing since the current implementation is deliberate and working as expected, see this comment over on #18068 for more details.

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.