-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
update shadow firewall support #741
update shadow firewall support #741
Conversation
Thanks for the PR! 🚀 |
@ericyz Please rebase on master. |
217bf8d
to
685a2db
Compare
984b9db
to
afe3d90
Compare
Hey team, any feedback on this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ericyz Can you explain more about the purpose of "shadow" firewall rules? Maybe we could have an explanation somewhere in the README or description?
…-google-kubernetes-engine into feature/shadow-firewall
Hi @morgante , I have added more descriptions to the TF variables. The purpose of shadow firewall is to gain more network insight of the packages flowing between GKE worker and master nodes. |
autogen/main/variables.tf.tmpl
Outdated
default = false | ||
} | ||
|
||
variable "firewall_priority" { | ||
type = number | ||
description = "Priority rule for firewall rules" | ||
description = "The firewall priority of GKE shadow firewall rules. The priority should be less than 1000, which is the priority of Google-managed GKE firewall." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = "The firewall priority of GKE shadow firewall rules. The priority should be less than 1000, which is the priority of Google-managed GKE firewall." | |
description = "The firewall priority of custom cluster firewall rules. |
autogen/main/variables.tf.tmpl
Outdated
@@ -545,13 +545,13 @@ variable "enable_binary_authorization" { | |||
|
|||
variable "add_cluster_firewall_rules" { | |||
type = bool | |||
description = "Create additional firewall rules" | |||
description = "Create GKE shadow firewall rules by creating the same firewall rules as Google-managed ones with higher priority and firewall logs enabled." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = "Create GKE shadow firewall rules by creating the same firewall rules as Google-managed ones with higher priority and firewall logs enabled." | |
description = "Create additional firewall rules for common GKE tasks." |
firewall.tf
Outdated
count = var.add_shadow_firewall_rules ? 1 : 0 | ||
|
||
name = "gke-shadow-${substr(var.name, 0, min(25, length(var.name)))}-all" | ||
description = "Managed by terraform gke module: A shadow firewall rule to match the fireall allow pod communication." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = "Managed by terraform gke module: A shadow firewall rule to match the fireall allow pod communication." | |
description = "Managed by terraform gke module: A shadow firewall rule to match the default rule allowing pod communication." |
8c4840e
to
8f0a90d
Compare
844287a
to
4f5f6f6
Compare
Thanks @morgante for updating my typo. Please review and let me know anything to update for the merge. |
The goal of this PR is to add visibility of the traffic between the Google-managed default GKE firewall. The context of the request is that there are some compliance-driven requirements to capture all the inter-node traffics, which doesn't provide by the default GKE firewalls out of box.
The solution including in this PR can be described as: