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

Make network policy configurable #166

Merged
merged 2 commits into from
Feb 6, 2025
Merged

Make network policy configurable #166

merged 2 commits into from
Feb 6, 2025

Conversation

zioproto
Copy link
Collaborator

@zioproto zioproto commented Feb 5, 2025

Description

Make network policy configurable, using either cilium or calico.

Fixes #165
Closes #165

@@ -121,9 +121,9 @@ resource "azurerm_kubernetes_cluster" "this" {
network_profile {
network_plugin = "azure"
load_balancer_sku = "standard"
network_data_plane = "cilium"
network_data_plane = var.network_policy == "cilium" ? "cilium" : null
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nellyk is there a more elegant way to write this condition ? I am not sure I have done this in the best way

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how we have done it on our private module

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same way as I did in #161 - except I also included the azure CNI.

I can understand not including this to guide people towards calico or cilium, makes sense without affecting support for recommended practices.

Just noting I'm waiting feedback from either @nellyk or @zioproto on whether the general direction of #161 is of interest & it is worth splitting it out?

@zioproto zioproto requested a review from nellyk February 5, 2025 16:26
@nellyk nellyk merged commit 908f377 into main Feb 6, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Triage 🔍 Maintainers need to triage still
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AVM Module Issue]: "network_data_plane" and "network_policy" hard coded to Cilium
4 participants