diff --git a/pkg/model/gcemodel/firewall.go b/pkg/model/gcemodel/firewall.go index 01163a5f74..8e37b9689e 100644 --- a/pkg/model/gcemodel/firewall.go +++ b/pkg/model/gcemodel/firewall.go @@ -154,12 +154,21 @@ func (b *GCEModelContext) AddFirewallRulesTasks(c *fi.ModelBuilderContext, name ipv4 := *rule ipv4.Name = s(b.NameForFirewallRule(name)) ipv4.SourceRanges = ipv4SourceRanges - ipv4.DisableIfEmptySourceRanges() + if len(ipv4.SourceRanges) == 0 { + // This is helpful because empty SourceRanges and SourceTags are interpreted as allow everything, + // but the intent is usually to block everything, which can be achieved with Disabled=true. + ipv4.Disabled = true + ipv4.SourceRanges = []string{"0.0.0.0/0"} + } c.AddTask(&ipv4) ipv6 := *rule ipv6.Name = s(b.NameForFirewallRule(name + "-ipv6")) ipv6.SourceRanges = ipv6SourceRanges - ipv6.DisableIfEmptySourceRanges() + if len(ipv6.SourceRanges) == 0 { + // We specify explicitly so the rule is in IPv6 mode + ipv6.Disabled = true + ipv6.SourceRanges = []string{"::/0"} + } c.AddTask(&ipv6) } diff --git a/tests/integration/update_cluster/ha_gce/kubernetes.tf b/tests/integration/update_cluster/ha_gce/kubernetes.tf index ac376973ca..c6bc941e9d 100644 --- a/tests/integration/update_cluster/ha_gce/kubernetes.tf +++ b/tests/integration/update_cluster/ha_gce/kubernetes.tf @@ -291,10 +291,11 @@ resource "google_compute_firewall" "cidr-to-master-ipv6-ha-gce-example-com" { ports = ["4194"] protocol = "tcp" } - disabled = true - name = "cidr-to-master-ipv6-ha-gce-example-com" - network = google_compute_network.default.name - target_tags = ["ha-gce-example-com-k8s-io-role-master"] + disabled = true + name = "cidr-to-master-ipv6-ha-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["ha-gce-example-com-k8s-io-role-master"] } resource "google_compute_firewall" "cidr-to-node-ha-gce-example-com" { @@ -342,10 +343,11 @@ resource "google_compute_firewall" "cidr-to-node-ipv6-ha-gce-example-com" { allow { protocol = "sctp" } - disabled = true - name = "cidr-to-node-ipv6-ha-gce-example-com" - network = google_compute_network.default.name - target_tags = ["ha-gce-example-com-k8s-io-role-node"] + disabled = true + name = "cidr-to-node-ipv6-ha-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["ha-gce-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "kubernetes-master-https-ha-gce-example-com" { @@ -475,10 +477,11 @@ resource "google_compute_firewall" "nodeport-external-to-node-ha-gce-example-com ports = ["30000-32767"] protocol = "udp" } - disabled = true - name = "nodeport-external-to-node-ha-gce-example-com" - network = google_compute_network.default.name - target_tags = ["ha-gce-example-com-k8s-io-role-node"] + disabled = true + name = "nodeport-external-to-node-ha-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["0.0.0.0/0"] + target_tags = ["ha-gce-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "nodeport-external-to-node-ipv6-ha-gce-example-com" { @@ -490,10 +493,11 @@ resource "google_compute_firewall" "nodeport-external-to-node-ipv6-ha-gce-exampl ports = ["30000-32767"] protocol = "udp" } - disabled = true - name = "nodeport-external-to-node-ipv6-ha-gce-example-com" - network = google_compute_network.default.name - target_tags = ["ha-gce-example-com-k8s-io-role-node"] + disabled = true + name = "nodeport-external-to-node-ipv6-ha-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["ha-gce-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "ssh-external-to-master-ha-gce-example-com" { diff --git a/tests/integration/update_cluster/minimal_gce/kubernetes.tf b/tests/integration/update_cluster/minimal_gce/kubernetes.tf index c31088fe4d..d3a6a9f532 100644 --- a/tests/integration/update_cluster/minimal_gce/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce/kubernetes.tf @@ -211,10 +211,11 @@ resource "google_compute_firewall" "cidr-to-master-ipv6-minimal-gce-example-com" ports = ["4194"] protocol = "tcp" } - disabled = true - name = "cidr-to-master-ipv6-minimal-gce-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-example-com-k8s-io-role-master"] + disabled = true + name = "cidr-to-master-ipv6-minimal-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-example-com-k8s-io-role-master"] } resource "google_compute_firewall" "cidr-to-master-minimal-gce-example-com" { @@ -252,10 +253,11 @@ resource "google_compute_firewall" "cidr-to-node-ipv6-minimal-gce-example-com" { allow { protocol = "sctp" } - disabled = true - name = "cidr-to-node-ipv6-minimal-gce-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-example-com-k8s-io-role-node"] + disabled = true + name = "cidr-to-node-ipv6-minimal-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "cidr-to-node-minimal-gce-example-com" { @@ -289,10 +291,11 @@ resource "google_compute_firewall" "kubernetes-master-https-ipv6-minimal-gce-exa ports = ["443"] protocol = "tcp" } - disabled = true - name = "kubernetes-master-https-ipv6-minimal-gce-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-example-com-k8s-io-role-master"] + disabled = true + name = "kubernetes-master-https-ipv6-minimal-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-example-com-k8s-io-role-master"] } resource "google_compute_firewall" "kubernetes-master-https-minimal-gce-example-com" { @@ -410,10 +413,11 @@ resource "google_compute_firewall" "nodeport-external-to-node-ipv6-minimal-gce-e ports = ["30000-32767"] protocol = "udp" } - disabled = true - name = "nodeport-external-to-node-ipv6-minimal-gce-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-example-com-k8s-io-role-node"] + disabled = true + name = "nodeport-external-to-node-ipv6-minimal-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "nodeport-external-to-node-minimal-gce-example-com" { @@ -425,10 +429,11 @@ resource "google_compute_firewall" "nodeport-external-to-node-minimal-gce-exampl ports = ["30000-32767"] protocol = "udp" } - disabled = true - name = "nodeport-external-to-node-minimal-gce-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-example-com-k8s-io-role-node"] + disabled = true + name = "nodeport-external-to-node-minimal-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["0.0.0.0/0"] + target_tags = ["minimal-gce-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "ssh-external-to-master-ipv6-minimal-gce-example-com" { @@ -436,10 +441,11 @@ resource "google_compute_firewall" "ssh-external-to-master-ipv6-minimal-gce-exam ports = ["22"] protocol = "tcp" } - disabled = true - name = "ssh-external-to-master-ipv6-minimal-gce-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-example-com-k8s-io-role-master"] + disabled = true + name = "ssh-external-to-master-ipv6-minimal-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-example-com-k8s-io-role-master"] } resource "google_compute_firewall" "ssh-external-to-master-minimal-gce-example-com" { @@ -459,10 +465,11 @@ resource "google_compute_firewall" "ssh-external-to-node-ipv6-minimal-gce-exampl ports = ["22"] protocol = "tcp" } - disabled = true - name = "ssh-external-to-node-ipv6-minimal-gce-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-example-com-k8s-io-role-node"] + disabled = true + name = "ssh-external-to-node-ipv6-minimal-gce-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "ssh-external-to-node-minimal-gce-example-com" { diff --git a/tests/integration/update_cluster/minimal_gce_private/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_private/kubernetes.tf index 4cffa875f4..f0fd7b2d06 100644 --- a/tests/integration/update_cluster/minimal_gce_private/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_private/kubernetes.tf @@ -211,10 +211,11 @@ resource "google_compute_firewall" "cidr-to-master-ipv6-minimal-gce-private-exam ports = ["4194"] protocol = "tcp" } - disabled = true - name = "cidr-to-master-ipv6-minimal-gce-private-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-private-example-com-k8s-io-role-master"] + disabled = true + name = "cidr-to-master-ipv6-minimal-gce-private-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-private-example-com-k8s-io-role-master"] } resource "google_compute_firewall" "cidr-to-master-minimal-gce-private-example-com" { @@ -252,10 +253,11 @@ resource "google_compute_firewall" "cidr-to-node-ipv6-minimal-gce-private-exampl allow { protocol = "sctp" } - disabled = true - name = "cidr-to-node-ipv6-minimal-gce-private-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-private-example-com-k8s-io-role-node"] + disabled = true + name = "cidr-to-node-ipv6-minimal-gce-private-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-private-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "cidr-to-node-minimal-gce-private-example-com" { @@ -289,10 +291,11 @@ resource "google_compute_firewall" "kubernetes-master-https-ipv6-minimal-gce-pri ports = ["443"] protocol = "tcp" } - disabled = true - name = "kubernetes-master-https-ipv6-minimal-gce-private-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-private-example-com-k8s-io-role-master"] + disabled = true + name = "kubernetes-master-https-ipv6-minimal-gce-private-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-private-example-com-k8s-io-role-master"] } resource "google_compute_firewall" "kubernetes-master-https-minimal-gce-private-example-com" { @@ -410,10 +413,11 @@ resource "google_compute_firewall" "nodeport-external-to-node-ipv6-minimal-gce-p ports = ["30000-32767"] protocol = "udp" } - disabled = true - name = "nodeport-external-to-node-ipv6-minimal-gce-private-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-private-example-com-k8s-io-role-node"] + disabled = true + name = "nodeport-external-to-node-ipv6-minimal-gce-private-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-private-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "nodeport-external-to-node-minimal-gce-private-example-com" { @@ -425,10 +429,11 @@ resource "google_compute_firewall" "nodeport-external-to-node-minimal-gce-privat ports = ["30000-32767"] protocol = "udp" } - disabled = true - name = "nodeport-external-to-node-minimal-gce-private-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-private-example-com-k8s-io-role-node"] + disabled = true + name = "nodeport-external-to-node-minimal-gce-private-example-com" + network = google_compute_network.default.name + source_ranges = ["0.0.0.0/0"] + target_tags = ["minimal-gce-private-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "ssh-external-to-master-ipv6-minimal-gce-private-example-com" { @@ -436,10 +441,11 @@ resource "google_compute_firewall" "ssh-external-to-master-ipv6-minimal-gce-priv ports = ["22"] protocol = "tcp" } - disabled = true - name = "ssh-external-to-master-ipv6-minimal-gce-private-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-private-example-com-k8s-io-role-master"] + disabled = true + name = "ssh-external-to-master-ipv6-minimal-gce-private-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-private-example-com-k8s-io-role-master"] } resource "google_compute_firewall" "ssh-external-to-master-minimal-gce-private-example-com" { @@ -459,10 +465,11 @@ resource "google_compute_firewall" "ssh-external-to-node-ipv6-minimal-gce-privat ports = ["22"] protocol = "tcp" } - disabled = true - name = "ssh-external-to-node-ipv6-minimal-gce-private-example-com" - network = google_compute_network.default.name - target_tags = ["minimal-gce-private-example-com-k8s-io-role-node"] + disabled = true + name = "ssh-external-to-node-ipv6-minimal-gce-private-example-com" + network = google_compute_network.default.name + source_ranges = ["::/0"] + target_tags = ["minimal-gce-private-example-com-k8s-io-role-node"] } resource "google_compute_firewall" "ssh-external-to-node-minimal-gce-private-example-com" { diff --git a/upup/pkg/fi/cloudup/gcetasks/firewallrule.go b/upup/pkg/fi/cloudup/gcetasks/firewallrule.go index 938fe63c86..0108fd7c06 100644 --- a/upup/pkg/fi/cloudup/gcetasks/firewallrule.go +++ b/upup/pkg/fi/cloudup/gcetasks/firewallrule.go @@ -93,16 +93,10 @@ func (e *FirewallRule) Run(c *fi.Context) error { func (e *FirewallRule) sanityCheck() error { if !e.Disabled { // Treat it as an error if SourceRanges _and_ SourceTags empty with Disabled=false - // this is interpreted as SourceRanges="0.0.0.0/0", which is likely what was intended. + // this is interpreted as SourceRanges="0.0.0.0/0", which is likely not what was intended. if len(e.SourceRanges) == 0 && len(e.SourceTags) == 0 { return fmt.Errorf("either SourceRanges or SourceTags should be specified when Disabled is false") } - } else { - // Treat it as an error if SourceRanges/SourceTags non-empty with Disabled - // this is allowed but is likely not what was intended. - if len(e.SourceRanges) != 0 || len(e.SourceTags) != 0 { - return fmt.Errorf("setting Disabled=true overrules SourceRanges or SourceTags") - } } // Treat it as an error if SourceRanges _and_ SourceTags both set; @@ -268,13 +262,3 @@ func (_ *FirewallRule) RenderTerraform(t *terraform.TerraformTarget, a, e, chang return t.RenderResource("google_compute_firewall", *e.Name, tf) } - -// DisableIfEmptySourceRanges sets Disabled if SourceRanges is empty. -// This is helpful because empty SourceRanges and SourceTags are interpreted as allow everything, -// but the intent is usually to block everything, which can be achieved with Disabled=true. -func (e *FirewallRule) DisableIfEmptySourceRanges() *FirewallRule { - if len(e.SourceRanges) == 0 { - e.Disabled = true - } - return e -}