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

Add Network Contributor role assignments scoped to AKS nodepools subnets #327

Merged

Conversation

zioproto
Copy link
Collaborator

This PR creates a role assignment for the AKS Service Principal to be a Network Contributor on the subnets used for the AKS Cluster.

The AKS cluster identity has the Contributor role on the AKS second resource group (MC_myResourceGroup_myAKSCluster_eastus)
In this case no additional change is necessary.

However when using a custom VNET, the AKS cluster identity needs the Network Contributor role on the VNET subnets
used by the system node pool and by any additional node pools.
Docs:

This PR detects if a custom VNET is used looking at the values of the variable var.vnet_subnet_ids and at the value of vnet_subnet_ids in the map variable of the node pools.

Issue number

Fixes #178

Checklist before requesting a review

  • The pr title can be used to describe what this pr did in CHANGELOG.md file
  • I have executed pre-commit on my machine
  • I have passed pr-check on my machine

@zioproto zioproto force-pushed the aks-network-contributor-role-assignment branch from a49e8ac to 9fc283d Compare March 16, 2023 17:15
Copy link
Member

@lonegunmanb lonegunmanb left a comment

Choose a reason for hiding this comment

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

Thanks for opening this pr @zioproto!. Almost LGTM but only a few issues. It would be nice if we can assign this new variable in multiple_node_pools example.

variables.tf Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
locals.tf Outdated Show resolved Hide resolved
@zioproto zioproto force-pushed the aks-network-contributor-role-assignment branch from 9fc283d to b9f3c30 Compare March 17, 2023 15:00
@lonegunmanb
Copy link
Member

Hi @zioproto this pr almost LGTM but it's base version contains a failed example that was fixed by #328, would you please rebase this pr to the latest main branch and try again? Thanks!

…k Contributor on the subnets used for the AKS Cluster
@zioproto zioproto force-pushed the aks-network-contributor-role-assignment branch from b9f3c30 to 3f848ac Compare March 21, 2023 10:18
@zioproto
Copy link
Collaborator Author

Rebased on current origin/main. Thanks

@zioproto zioproto temporarily deployed to acctests March 22, 2023 04:50 — with GitHub Actions Inactive
Copy link
Member

@lonegunmanb lonegunmanb left a comment

Choose a reason for hiding this comment

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

Thanks @zioproto, LGTM! 🚀

@lonegunmanb lonegunmanb merged commit ac1d3f7 into Azure:main Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure CNI requires cluster identity to have Network Contributor permissions
2 participants