azurerm_kubernetes_cluster - add policy property to network_profile.advanced_networking block#31506
azurerm_kubernetes_cluster - add policy property to network_profile.advanced_networking block#31506nddq wants to merge 1 commit into
azurerm_kubernetes_cluster - add policy property to network_profile.advanced_networking block#31506Conversation
|
@rcskosir hello, when can I expect to get a review for this PR? Thanks! |
|
Hi @nddq , The PR is merged, you can rebase to main branch. I'm happy to take over this task if you're occupied, let me know if you need any help. |
6dfbc10 to
6f3dcab
Compare
|
@ms-henglu Thanks! I have rebased this branch onto latest main |
|
@magodo @WodansSon Any chance y'all can take a pass? |
wuxu92
left a comment
There was a problem hiding this comment.
Thanks for the contribution! I left some comments otherwise look good!
azurerm_kubernetes_cluster - add advanced_network_policies property to network_profile.advanced_networking blockazurerm_kubernetes_cluster - add policy property to network_profile.advanced_networking block
|
@sreallymatt any chance of queuing for reviews from hashicorp? |
|
hi @nddq please take a looks at the failed CI actions |
|
Any updates for this? |
catriona-m
left a comment
There was a problem hiding this comment.
Thanks for working on this @nddq - I left a couple of comments inline but otherwise this is looking good. Thanks!
|
@catriona-m one more nudge for a review |
|
@catriona-m Ping again to avoid PR going stale |
catriona-m
left a comment
There was a problem hiding this comment.
Hi @nddq I had another look at this and left a couple of comments on adding more test steps around removing/re-adding policy and also removing the advanced_networking entirely. Once that is addressed and the tests are passing, I think this should be good to go! Thanks!
| check.That(data.ResourceName).ExistsInAzure(r), | ||
| ), | ||
| }, | ||
| data.ImportStep(), |
There was a problem hiding this comment.
could we add more steps to this test where we omit the policy field entirely and then add it again?
| } | ||
| security := &managedclusters.AdvancedNetworkingSecurity{ | ||
| Enabled: pointer.To(securityEnabled), | ||
| AdvancedNetworkPolicies: pointer.To(advancedNetworkPolicies), |
There was a problem hiding this comment.
policy currently gets set to None here if the policy property is omitted when the block is present in the config, but when the advanced_networking block is removed entirely, the other properties in the block are set to false explicitly and policy is nil. We should update the tests to cover this case and investigate if leaving it nil or set to None is correct here
…ile.advanced_networking` block This change adds support for the `policy` property within the `network_profile.advanced_networking` block for Azure Kubernetes Service clusters. The property maps to the Azure SDK's `AdvancedNetworkingSecurity.AdvancedNetworkPolicies` field. Changes: - Add `policy` field with valid values `FQDN`, `L7` and `None` - The field is Optional and Computed: the API defaults an unset policy to `FQDN` when `security_enabled` is `true`, so the server-applied value is reflected in state, and `None` is exposed as the explicit value to disable advanced network policy enforcement - Add plan-time validation: `FQDN` and `L7` can only be configured when `security_enabled` is `true`, and `L7` cannot be combined with a service mesh profile in `Istio` mode - Send an explicit `None` when security is disabled or the `advanced_networking` block is removed, so a previously configured policy is reset rather than left for the API to retain - Add acceptance tests covering the API-side `FQDN` default, updating the policy, omitting it (value is retained), removing the `advanced_networking` block, re-adding it, and explicitly disabling with `None` - Update documentation Signed-off-by: Quang Nguyen <nguyenquang@microsoft.com>
The
advanced_networkingblock currently exposesobservability_enabledandsecurity_enabled, but not the advanced network policy mode that AdvancedContainer Networking Services (ACNS) security applies — enabling security
always yields the service default, with no way to select L7 policy enforcement
or disable policy enforcement from Terraform.
This PR adds the
policyproperty to thenetwork_profile.advanced_networkingblock, mapping to the
AdvancedNetworkingSecurity.AdvancedNetworkPoliciesfield on the
managedclustersAPI.The API treats
advancedNetworkPoliciesas a tri-state field with aservice-side default: when
security_enabledistrueand the field isunset, the service defaults it to
FQDN; when security is disabled, theservice normalizes it to
None. The schema models this directly:policyisOptional+Computedwith possible valuesFQDN,L7andNone. When omitted, the service-applied value flows into state,so clusters that already have
security_enabled = truekeep their currentpolicy with no diff on provider upgrade.
Noneis exposed as an explicitvalue because omission cannot express it: an unset
policyon asecurity-enabled cluster is defaulted by the API to
FQDN, so disablingenforcement requires
policy = "None".FQDNandL7can only be configured whensecurity_enabledistrue. The check applies only whenpolicyisexplicitly set in the configuration (via
GetRawConfig), so aComputedvalue carried over from state does not block disabling security — the
provider sends
Nonein that case, matching the API's own normalization.L7cannot be combined with aservice_mesh_profileinIstiomode;FQDNwith Istio is allowed, matching the API.security_enabledisfalseor theadvanced_networkingblock is removed entirely, an explicitNoneis sentso a previously configured policy is reset rather than left for the API to
retain.
FQDNdefault, updatingFQDN→L7, omittingpolicy(current value is retained), removing theadvanced_networkingblock entirely, re-adding it, explicitly disablingwith
None, and the negative cases (L7withoutsecurity_enabled,L7with Istio).
policyargument and its preconditions added to theazurerm_kubernetes_clusterdocumentation.Community Note
Description
See above.
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Validated two ways against a dev-override build of the provider:
Plan-time matrix (10 cases):
FQDN/L7/Noneplan cleanly with securityenabled; explicit
Noneis accepted with security disabled; explicitL7without
security_enabledis rejected;L7with Istio is rejected whileFQDNwith Istio plans cleanly;security_enabled = truewith a non-ciliumdata plane is still rejected; invalid values are rejected by schema validation
listing
["FQDN" "L7" "None"].Apply-time lifecycle against the live API (
2025-10-01), mirroring theTestAccKubernetesCluster_advancedNetworkingPoliciessteps, withTF_LOG=DEBUGconfirming the request and response bodies at each step:security_enabled = true,policyomittedadvancedNetworkPolicies; API defaults it toFQDN; state =FQDNpolicy = "L7"L7; state =L7policyomittedsecurity_enabled = falsewith computedL7in stateNone; state =Nonesecurity_enabled = trueagainNoneretained — the state value is sent explicitly, so the API default does not re-fireadvanced_networkingblock removedadvancedNetworkPolicies: None; block removed from statepolicy = "FQDN"FQDNChange Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_kubernetes_cluster- support for thepolicyproperty in thenetwork_profile.advanced_networkingblock [azurerm_kubernetes_cluster- addpolicyproperty tonetwork_profile.advanced_networkingblock #31506]This is a (please select all that apply):
Related Issue(s)
N/A
AI Assistance Disclosure
AI was used for code generation and test authoring; all changes were reviewed before submission.
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.