azurerm_kubernetes_cluster - support for the plugin certificate_authority for istio addon

[hqhqhqhqhqhqhqhqhqhqhq]

Community Note

• Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
• Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

Adds new property certificate_authority for azurerm_kubernetes_cluster istio addon. This configuration allows users to bring their own root certificate and keys for Istio CA in the Istio-based service mesh add-on for Azure Kubernetes Service.

PR Checklist

• I have followed the guidelines in our Contributing Documentation.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
• I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
• I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
  For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

• I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
• I have written new tests for my resource or datasource changes & updated any relevent documentation.
• I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
• [] (For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

• My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

• azurerm_kubernetes_cluster - support for the certificate_authority property under service_mesh_profile

This is a (please select all that apply):

• Bug Fix
• New Feature (ie adding a service, resource, or data source)
• Enhancement
• Breaking Change

Related Issue(s)

Closes #26311

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

[stephybun]

I see that this would reduce the size of the test config, but we tend to not use dynamic configurations in the provider tests and prefer the test configs to be purely declarative for clarity purposes

[hqhqhqhqhqhqhqhqhqhqhq]

Updated in latest commit

[stephybun]

Since this is touching add-on functionality would you mind moving this test into the kubernetes_cluster_addons_resource_test.go and follow the test naming convention in that file?

[hqhqhqhqhqhqhqhqhqhqhq]

Updated in latest commit

[stephybun]

It looks like there's an unnecessary level of nesting here, since plugin can also contain at most 1 element could the schema be flattened and simplified to

Suggested change

	certificate_authority {
	plugin {
	key_vault_id = azurerm_key_vault.test.id
	root_cert_object_name = azurerm_key_vault_certificate.test_cert1.name
	cert_chain_object_name = azurerm_key_vault_certificate.test_cert2.name
	cert_object_name = azurerm_key_vault_certificate.test_cert3.name
	key_object_name = azurerm_key_vault_key.test.name
	}
	}
	certificate_authority {
	key_vault_id = azurerm_key_vault.test.id
	root_cert_object_name = azurerm_key_vault_certificate.test_cert1.name
	cert_chain_object_name = azurerm_key_vault_certificate.test_cert2.name
	cert_object_name = azurerm_key_vault_certificate.test_cert3.name
	key_object_name = azurerm_key_vault_key.test.name
	}

[hqhqhqhqhqhqhqhqhqhqhq]

I added this mainly because:

1. It reflects the request structure passed to the AKS API.
2. If there are new properties to add under certificate_authority that do not correlate with these plugin settings, it will be easier to manage.

In this case, I believe the benefits of adding an extra layer outweigh the downsides

[stephybun]

It reflects the request structure passed to the AKS API.

We don't always map API property names or object structures 1:1 in the provider since the provider has it's own patterns of exposing Azure functionality, and often what's exposed in the API needs simplification for a better user experience in Terraform. In the case of AKS which is a complicated and huge resource, proactively flattening objects also simplifies the code and reduces the cognitive load when working with the resource. It might seem minor for something like this but it adds up over time.

If there are new properties to add under certificate_authority that do not correlate with these plugin settings, it will be easier to manage.

This can be managed by introducing an additional block under certificate_authority for properties that do not correlate to plugin settings, or by introducing them as root layer properties at that point in time, when and if that even happens. In this particular case the properties key_vault_id, root_cert_object_name etc. relate much more to certificate_authority than to plugin, so further nesting under plugin is actually obfuscating than helpful for the user in this case.

[hqhqhqhqhqhqhqhqhqhqhq]

Thanks, noted.

I have changed the code & doc, ran the tests again and it is passing.