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.

Sign up for GitHub

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

Jump to bottom

feat: add all pod_ranges to cluster firewall rules and add missing shadow rules #1480

Merged
merged 9 commits into from
Dec 30, 2022
Merged

feat: add all pod_ranges to cluster firewall rules and add missing shadow rules #1480

merged 9 commits into from
Dec 30, 2022

Conversation

splichy
Copy link
Contributor

@splichy splichy commented Nov 28, 2022

Firewall rules now include also discontinuous POD IP ranges, previously only cluster-wide pod_range was used for -intra-cluster-egress & -all firewall rules.
Also adding missing(& undocumented) shadow firewall rules (inkubelet, exkubelet)- GKE itself creates incomplete rules, so shadow rules can be used as a quick workaround when someone using discontinuous pod ranges.

@splichy splichy force-pushed the feat/firewall_disco_pod_ranges branch from 3db1497 to 37bfd15 Compare November 28, 2022 18:36
bharathkkb
bharathkkb reviewed Dec 6, 2022
View reviewed changes
Copy link
Member

@bharathkkb bharathkkb 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 the PR @splichy, these look reasonable to me!
@ericyz I saw you added this, mind taking another look?

@ericyz ericyz self-requested a review December 6, 2022 23:16
ericyz
ericyz approved these changes Dec 6, 2022
View reviewed changes
Copy link
Collaborator

@ericyz ericyz left a comment

Choose a reason for hiding this comment

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

LGTM overall. Minor comment on suggesting to validate the shadow_firewall_priority variable.

modules/beta-private-cluster/firewall.tf Outdated Show resolved Hide resolved
@splichy splichy changed the title add all pod_ranges to cluster firewall rules and add missing shadow rules feat: add all pod_ranges to cluster firewall rules and add missing shadow rules Dec 14, 2022
@splichy splichy force-pushed the feat/firewall_disco_pod_ranges branch from 938db8a to 28fab3d Compare December 14, 2022 22:11
@splichy
Copy link
Contributor Author

splichy commented Dec 14, 2022

Hi, I have added shadow_firewall_rules_priority validation, I have also introduced shadow_firewall_rules_log_config which can be used to tune/disable logging for shadow rules.
I have also written a short paragraph about discontiguous multi-Pod CIDR in private_clusters.md and changed pod_range ID to name in readme, as there is nothing like a secondary IP range ID, there is just a name.

@comment-bot-dev
Copy link

@splichy
Thanks for the PR! 🚀
✅ Lint checks have passed.

@bharathkkb bharathkkb merged commit bcd5e03 into terraform-google-modules:master Dec 30, 2022
@release-please release-please bot mentioned this pull request Dec 30, 2022
Merged
@michaellzc michaellzc mentioned this pull request Nov 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericyz ericyz ericyz approved these changes

@bharathkkb bharathkkb bharathkkb approved these changes

@Jberlinsky Jberlinsky Awaiting requested review from Jberlinsky

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@splichy @comment-bot-dev @ericyz @bharathkkb