diff --git a/pkg/model/gcemodel/api_loadbalancer.go b/pkg/model/gcemodel/api_loadbalancer.go index 7d97d10801c19..a8f1fed825e30 100644 --- a/pkg/model/gcemodel/api_loadbalancer.go +++ b/pkg/model/gcemodel/api_loadbalancer.go @@ -21,6 +21,7 @@ import ( "strconv" "golang.org/x/exp/slices" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/wellknownports" "k8s.io/kops/pkg/wellknownservices" @@ -101,9 +102,9 @@ func (b *APILoadBalancerBuilder) addFirewallRules(c *fi.CloudupModelBuilderConte b.AddFirewallRulesTasks(c, "https-api", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, Network: network, - SourceRanges: b.Cluster.Spec.API.Access, + SourceRanges: sets.New(b.Cluster.Spec.API.Access...), TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane)}, - Allowed: []string{"tcp:" + strconv.Itoa(wellknownports.KubeAPIServer)}, + Allowed: sets.New("tcp:" + strconv.Itoa(wellknownports.KubeAPIServer)), }) if b.NetworkingIsIPAlias() { @@ -112,9 +113,9 @@ func (b *APILoadBalancerBuilder) addFirewallRules(c *fi.CloudupModelBuilderConte Lifecycle: b.Lifecycle, Network: network, Family: gcetasks.AddressFamilyIPv4, // ip alias is always ipv4 - SourceRanges: []string{b.Cluster.Spec.Networking.PodCIDR}, + SourceRanges: sets.New(b.Cluster.Spec.Networking.PodCIDR), TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane)}, - Allowed: []string{"tcp:" + strconv.Itoa(wellknownports.KubeAPIServer)}, + Allowed: sets.New("tcp:" + strconv.Itoa(wellknownports.KubeAPIServer)), }) } @@ -122,9 +123,9 @@ func (b *APILoadBalancerBuilder) addFirewallRules(c *fi.CloudupModelBuilderConte b.AddFirewallRulesTasks(c, "kops-controller", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, Network: network, - SourceRanges: b.Cluster.Spec.API.Access, + SourceRanges: sets.New(b.Cluster.Spec.API.Access...), TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane)}, - Allowed: []string{"tcp:" + strconv.Itoa(wellknownports.KopsControllerPort)}, + Allowed: sets.New("tcp:" + strconv.Itoa(wellknownports.KopsControllerPort)), }) } } diff --git a/pkg/model/gcemodel/external_access.go b/pkg/model/gcemodel/external_access.go index 792d4401e8f28..fe8ed3971eb4b 100644 --- a/pkg/model/gcemodel/external_access.go +++ b/pkg/model/gcemodel/external_access.go @@ -19,6 +19,7 @@ package gcemodel import ( "strconv" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/wellknownports" @@ -58,21 +59,21 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.CloudupModelBuilderContext) err b.AddFirewallRulesTasks(c, "ssh-external-to-bastion", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleBastion)}, - Allowed: []string{"tcp:22"}, - SourceRanges: b.Cluster.Spec.SSHAccess, + Allowed: sets.New("tcp:22"), + SourceRanges: sets.New(b.Cluster.Spec.SSHAccess...), Network: network, }) b.AddFirewallRulesTasks(c, "bastion-to-master-ssh", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane), b.GCETagForRole("Master")}, - Allowed: []string{"tcp:22"}, + Allowed: sets.New("tcp:22"), SourceTags: []string{b.GCETagForRole(kops.InstanceGroupRoleBastion)}, Network: network, }) b.AddFirewallRulesTasks(c, "bastion-to-node-ssh", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleNode)}, - Allowed: []string{"tcp:22"}, + Allowed: sets.New("tcp:22"), SourceTags: []string{b.GCETagForRole(kops.InstanceGroupRoleBastion)}, Network: network, }) @@ -84,16 +85,16 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.CloudupModelBuilderContext) err b.AddFirewallRulesTasks(c, "ssh-external-to-master", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane), b.GCETagForRole("Master")}, - Allowed: []string{"tcp:22"}, - SourceRanges: b.Cluster.Spec.SSHAccess, + Allowed: sets.New("tcp:22"), + SourceRanges: sets.New(b.Cluster.Spec.SSHAccess...), Network: network, }) b.AddFirewallRulesTasks(c, "ssh-external-to-node", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleNode)}, - Allowed: []string{"tcp:22"}, - SourceRanges: b.Cluster.Spec.SSHAccess, + Allowed: sets.New("tcp:22"), + SourceRanges: sets.New(b.Cluster.Spec.SSHAccess...), Network: network, }) } @@ -113,11 +114,11 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.CloudupModelBuilderContext) err b.AddFirewallRulesTasks(c, "nodeport-external-to-node", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleNode)}, - Allowed: []string{ - "tcp:" + nodePortRangeString, - "udp:" + nodePortRangeString, - }, - SourceRanges: b.Cluster.Spec.NodePortAccess, + Allowed: sets.New( + "tcp:"+nodePortRangeString, + "udp:"+nodePortRangeString, + ), + SourceRanges: sets.New(b.Cluster.Spec.NodePortAccess...), Network: network, }) } @@ -136,8 +137,8 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.CloudupModelBuilderContext) err b.AddFirewallRulesTasks(c, "kubernetes-master-https", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane), b.GCETagForRole("Master")}, - Allowed: []string{"tcp:443"}, - SourceRanges: b.Cluster.Spec.API.Access, + Allowed: sets.New("tcp:443"), + SourceRanges: sets.New(b.Cluster.Spec.API.Access...), Network: network, }) @@ -147,9 +148,9 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.CloudupModelBuilderContext) err Lifecycle: b.Lifecycle, Network: network, Family: gcetasks.AddressFamilyIPv4, // ip alias is always ipv4 - SourceRanges: []string{b.Cluster.Spec.Networking.PodCIDR}, + SourceRanges: sets.New(b.Cluster.Spec.Networking.PodCIDR), TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane)}, - Allowed: []string{"tcp:" + strconv.Itoa(wellknownports.KubeAPIServer)}, + Allowed: sets.New("tcp:" + strconv.Itoa(wellknownports.KubeAPIServer)), }) } } diff --git a/pkg/model/gcemodel/firewall.go b/pkg/model/gcemodel/firewall.go index 29e0be8770169..4ea20f43b99cd 100644 --- a/pkg/model/gcemodel/firewall.go +++ b/pkg/model/gcemodel/firewall.go @@ -21,6 +21,7 @@ import ( "net" "strings" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/apis/kops/model" @@ -57,16 +58,16 @@ func (b *FirewallModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { Lifecycle: b.Lifecycle, Network: network, Family: gcetasks.AddressFamilyIPv4, - SourceRanges: []string{ + SourceRanges: sets.New( // IP ranges for load balancer health checks // https://cloud.google.com/load-balancing/docs/health-checks "35.191.0.0/16", "130.211.0.0/22", "209.85.204.0/22", "209.85.152.0/22", - }, + ), TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane)}, - Allowed: []string{"tcp"}, + Allowed: sets.New("tcp"), }) } @@ -82,7 +83,7 @@ func (b *FirewallModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { Network: network, SourceTags: []string{b.GCETagForRole(kops.InstanceGroupRoleNode)}, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleNode)}, - Allowed: allProtocols, + Allowed: sets.New(allProtocols...), } c.AddTask(t) } @@ -99,7 +100,7 @@ func (b *FirewallModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { Network: network, SourceTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane), b.GCETagForRole("Master")}, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane), b.GCETagForRole("Master")}, - Allowed: allProtocols, + Allowed: sets.New(allProtocols...), } c.AddTask(t) } @@ -116,7 +117,7 @@ func (b *FirewallModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { Network: network, SourceTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane), b.GCETagForRole("Master")}, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleNode)}, - Allowed: allProtocols, + Allowed: sets.New(allProtocols...), } c.AddTask(t) } @@ -133,25 +134,25 @@ func (b *FirewallModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { Network: network, SourceTags: []string{b.GCETagForRole(kops.InstanceGroupRoleNode)}, TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleControlPlane), b.GCETagForRole("Master")}, - Allowed: []string{ + Allowed: sets.New( fmt.Sprintf("tcp:%d", wellknownports.KubeAPIServer), fmt.Sprintf("tcp:%d", wellknownports.KubeletAPI), fmt.Sprintf("tcp:%d", wellknownports.KopsControllerPort), - }, + ), } if b.Cluster.UsesLegacyGossip() { - t.Allowed = append(t.Allowed, fmt.Sprintf("udp:%d", wellknownports.DNSControllerGossipMemberlist)) - t.Allowed = append(t.Allowed, fmt.Sprintf("tcp:%d", wellknownports.DNSControllerGossipMemberlist)) - t.Allowed = append(t.Allowed, fmt.Sprintf("udp:%d", wellknownports.ProtokubeGossipMemberlist)) - t.Allowed = append(t.Allowed, fmt.Sprintf("tcp:%d", wellknownports.ProtokubeGossipMemberlist)) + t.Allowed.Insert(fmt.Sprintf("udp:%d", wellknownports.DNSControllerGossipMemberlist)) + t.Allowed.Insert(fmt.Sprintf("tcp:%d", wellknownports.DNSControllerGossipMemberlist)) + t.Allowed.Insert(fmt.Sprintf("udp:%d", wellknownports.ProtokubeGossipMemberlist)) + t.Allowed.Insert(fmt.Sprintf("tcp:%d", wellknownports.ProtokubeGossipMemberlist)) } if b.NetworkingIsCalico() { - t.Allowed = append(t.Allowed, "ipip") + t.Allowed.Insert("ipip") } if b.NetworkingIsCilium() { - t.Allowed = append(t.Allowed, fmt.Sprintf("udp:%d", wellknownports.VxlanUDP)) + t.Allowed.Insert(fmt.Sprintf("udp:%d", wellknownports.VxlanUDP)) if model.UseCiliumEtcd(b.Cluster) { - t.Allowed = append(t.Allowed, fmt.Sprintf("tcp:%d", wellknownports.EtcdCiliumClientPort)) + t.Allowed.Insert(fmt.Sprintf("tcp:%d", wellknownports.EtcdCiliumClientPort)) } } c.AddTask(t) @@ -174,9 +175,9 @@ func (b *FirewallModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { b.AddFirewallRulesTasks(c, "pod-cidrs-to-node", &gcetasks.FirewallRule{ Lifecycle: b.Lifecycle, Network: network, - SourceRanges: []string{b.Cluster.Spec.Networking.PodCIDR}, + SourceRanges: sets.New(b.Cluster.Spec.Networking.PodCIDR), TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleNode)}, - Allowed: allProtocols, + Allowed: sets.New(allProtocols...), }) } } @@ -189,9 +190,9 @@ func (b *FirewallModelBuilder) Build(c *fi.CloudupModelBuilderContext) error { // Furthermore, an empty SourceRange with empty SourceTags is interpreted as allow-everything, // but we intend for it to block everything; so we can Disabled to achieve the desired blocking. func (b *GCEModelContext) AddFirewallRulesTasks(c *fi.CloudupModelBuilderContext, name string, rule *gcetasks.FirewallRule) { - var ipv4SourceRanges []string - var ipv6SourceRanges []string - for _, sourceRange := range rule.SourceRanges { + ipv4SourceRanges := sets.New[string]() + ipv6SourceRanges := sets.New[string]() + for sourceRange := range rule.SourceRanges { _, cidr, err := net.ParseCIDR(sourceRange) if err != nil { klog.Fatalf("failed to parse invalid sourceRange %q", sourceRange) @@ -199,9 +200,9 @@ func (b *GCEModelContext) AddFirewallRulesTasks(c *fi.CloudupModelBuilderContext // Split into ipv4s and ipv6s, but treat IPv4-mapped IPv6 addresses as IPv6 if cidr.IP.To4() != nil && !strings.Contains(sourceRange, ":") { - ipv4SourceRanges = append(ipv4SourceRanges, sourceRange) + ipv4SourceRanges.Insert(sourceRange) } else { - ipv6SourceRanges = append(ipv6SourceRanges, sourceRange) + ipv6SourceRanges.Insert(sourceRange) } } @@ -214,7 +215,7 @@ func (b *GCEModelContext) AddFirewallRulesTasks(c *fi.CloudupModelBuilderContext // 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"} + ipv4.SourceRanges = sets.New("0.0.0.0/0") } } c.AddTask(&ipv4) @@ -227,16 +228,16 @@ func (b *GCEModelContext) AddFirewallRulesTasks(c *fi.CloudupModelBuilderContext if len(ipv6.SourceRanges) == 0 { // We specify explicitly so the rule is in IPv6 mode ipv6.Disabled = true - ipv6.SourceRanges = []string{"::/0"} + ipv6.SourceRanges = sets.New("::/0") } } - var ipv6Allowed []string - for _, allowed := range ipv6.Allowed { + ipv6Allowed := sets.New[string]() + for allowed := range ipv6.Allowed { // Map icmp to icmpv6; easier than maintaining separate lists if allowed == "icmp" { allowed = "58" // 58 == the IANA protocol number for ICMPv6 } - ipv6Allowed = append(ipv6Allowed, allowed) + ipv6Allowed.Insert(allowed) } ipv6.Allowed = ipv6Allowed c.AddTask(&ipv6) diff --git a/tests/integration/update_cluster/ha_gce/kubernetes.tf b/tests/integration/update_cluster/ha_gce/kubernetes.tf index 93acd239a8671..0012be5160f5b 100644 --- a/tests/integration/update_cluster/ha_gce/kubernetes.tf +++ b/tests/integration/update_cluster/ha_gce/kubernetes.tf @@ -316,22 +316,22 @@ resource "google_compute_firewall" "kubernetes-master-https-ipv6-ha-gce-example- resource "google_compute_firewall" "master-to-master-ha-gce-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-master-ha-gce-example-com" @@ -342,22 +342,22 @@ resource "google_compute_firewall" "master-to-master-ha-gce-example-com" { resource "google_compute_firewall" "master-to-node-ha-gce-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-node-ha-gce-example-com" @@ -368,15 +368,15 @@ resource "google_compute_firewall" "master-to-node-ha-gce-example-com" { resource "google_compute_firewall" "node-to-master-ha-gce-example-com" { allow { - ports = ["443"] + ports = ["10250"] protocol = "tcp" } allow { - ports = ["10250"] + ports = ["3988"] protocol = "tcp" } allow { - ports = ["3988"] + ports = ["443"] protocol = "tcp" } disabled = false @@ -388,22 +388,22 @@ resource "google_compute_firewall" "node-to-master-ha-gce-example-com" { resource "google_compute_firewall" "node-to-node-ha-gce-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "node-to-node-ha-gce-example-com" diff --git a/tests/integration/update_cluster/many-addons-gce/kubernetes.tf b/tests/integration/update_cluster/many-addons-gce/kubernetes.tf index 8385bff582bc7..bdacac9d2e4ee 100644 --- a/tests/integration/update_cluster/many-addons-gce/kubernetes.tf +++ b/tests/integration/update_cluster/many-addons-gce/kubernetes.tf @@ -244,22 +244,22 @@ resource "google_compute_firewall" "kubernetes-master-https-minimal-example-com" resource "google_compute_firewall" "master-to-master-minimal-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-master-minimal-example-com" @@ -270,22 +270,22 @@ resource "google_compute_firewall" "master-to-master-minimal-example-com" { resource "google_compute_firewall" "master-to-node-minimal-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-node-minimal-example-com" @@ -296,15 +296,15 @@ resource "google_compute_firewall" "master-to-node-minimal-example-com" { resource "google_compute_firewall" "node-to-master-minimal-example-com" { allow { - ports = ["443"] + ports = ["10250"] protocol = "tcp" } allow { - ports = ["10250"] + ports = ["3988"] protocol = "tcp" } allow { - ports = ["3988"] + ports = ["443"] protocol = "tcp" } disabled = false @@ -316,22 +316,22 @@ resource "google_compute_firewall" "node-to-master-minimal-example-com" { resource "google_compute_firewall" "node-to-node-minimal-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "node-to-node-minimal-example-com" diff --git a/tests/integration/update_cluster/minimal_gce/kubernetes.tf b/tests/integration/update_cluster/minimal_gce/kubernetes.tf index 379bb5ec5b02b..f2ee8380214ea 100644 --- a/tests/integration/update_cluster/minimal_gce/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce/kubernetes.tf @@ -220,22 +220,22 @@ resource "google_compute_firewall" "kubernetes-master-https-minimal-gce-example- resource "google_compute_firewall" "master-to-master-minimal-gce-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-master-minimal-gce-example-com" @@ -246,22 +246,22 @@ resource "google_compute_firewall" "master-to-master-minimal-gce-example-com" { resource "google_compute_firewall" "master-to-node-minimal-gce-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-node-minimal-gce-example-com" @@ -272,15 +272,15 @@ resource "google_compute_firewall" "master-to-node-minimal-gce-example-com" { resource "google_compute_firewall" "node-to-master-minimal-gce-example-com" { allow { - ports = ["443"] + ports = ["10250"] protocol = "tcp" } allow { - ports = ["10250"] + ports = ["3988"] protocol = "tcp" } allow { - ports = ["3988"] + ports = ["443"] protocol = "tcp" } disabled = false @@ -292,22 +292,22 @@ resource "google_compute_firewall" "node-to-master-minimal-gce-example-com" { resource "google_compute_firewall" "node-to-node-minimal-gce-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "node-to-node-minimal-gce-example-com" diff --git a/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf index fb59dbdcb7868..db1cbe0212d3a 100644 --- a/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_dns-none/kubernetes.tf @@ -248,28 +248,28 @@ resource "google_compute_firewall" "lb-health-checks-minimal-gce-example-com" { disabled = false name = "lb-health-checks-minimal-gce-example-com" network = google_compute_network.minimal-gce-example-com.name - source_ranges = ["35.191.0.0/16", "130.211.0.0/22", "209.85.204.0/22", "209.85.152.0/22"] + source_ranges = ["130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22", "35.191.0.0/16"] target_tags = ["minimal-gce-example-com-k8s-io-role-control-plane"] } resource "google_compute_firewall" "master-to-master-minimal-gce-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-master-minimal-gce-example-com" @@ -280,22 +280,22 @@ resource "google_compute_firewall" "master-to-master-minimal-gce-example-com" { resource "google_compute_firewall" "master-to-node-minimal-gce-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-node-minimal-gce-example-com" @@ -306,15 +306,15 @@ resource "google_compute_firewall" "master-to-node-minimal-gce-example-com" { resource "google_compute_firewall" "node-to-master-minimal-gce-example-com" { allow { - ports = ["443"] + ports = ["10250"] protocol = "tcp" } allow { - ports = ["10250"] + ports = ["3988"] protocol = "tcp" } allow { - ports = ["3988"] + ports = ["443"] protocol = "tcp" } disabled = false @@ -326,22 +326,22 @@ resource "google_compute_firewall" "node-to-master-minimal-gce-example-com" { resource "google_compute_firewall" "node-to-node-minimal-gce-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "node-to-node-minimal-gce-example-com" diff --git a/tests/integration/update_cluster/minimal_gce_ilb/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_ilb/kubernetes.tf index e052c7b202718..e34ec3034beee 100644 --- a/tests/integration/update_cluster/minimal_gce_ilb/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_ilb/kubernetes.tf @@ -232,28 +232,28 @@ resource "google_compute_firewall" "lb-health-checks-minimal-gce-ilb-example-com disabled = false name = "lb-health-checks-minimal-gce-ilb-example-com" network = google_compute_network.minimal-gce-ilb-example-com.name - source_ranges = ["35.191.0.0/16", "130.211.0.0/22", "209.85.204.0/22", "209.85.152.0/22"] + source_ranges = ["130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22", "35.191.0.0/16"] target_tags = ["minimal-gce-ilb-example-com-k8s-io-role-control-plane"] } resource "google_compute_firewall" "master-to-master-minimal-gce-ilb-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-master-minimal-gce-ilb-example-com" @@ -264,22 +264,22 @@ resource "google_compute_firewall" "master-to-master-minimal-gce-ilb-example-com resource "google_compute_firewall" "master-to-node-minimal-gce-ilb-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-node-minimal-gce-ilb-example-com" @@ -290,15 +290,15 @@ resource "google_compute_firewall" "master-to-node-minimal-gce-ilb-example-com" resource "google_compute_firewall" "node-to-master-minimal-gce-ilb-example-com" { allow { - ports = ["443"] + ports = ["10250"] protocol = "tcp" } allow { - ports = ["10250"] + ports = ["3988"] protocol = "tcp" } allow { - ports = ["3988"] + ports = ["443"] protocol = "tcp" } disabled = false @@ -310,22 +310,22 @@ resource "google_compute_firewall" "node-to-master-minimal-gce-ilb-example-com" resource "google_compute_firewall" "node-to-node-minimal-gce-ilb-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "node-to-node-minimal-gce-ilb-example-com" diff --git a/tests/integration/update_cluster/minimal_gce_ilb_longclustername/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_ilb_longclustername/kubernetes.tf index 53a5aace2a709..5294484a4a06b 100644 --- a/tests/integration/update_cluster/minimal_gce_ilb_longclustername/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_ilb_longclustername/kubernetes.tf @@ -232,28 +232,28 @@ resource "google_compute_firewall" "lb-health-checks-minimal-gce-with-a-very-ver disabled = false name = "lb-health-checks-minimal-gce-with-a-very-very-very-very--96dqvi" network = google_compute_network.minimal-gce-with-a-very-very-very-very-very-long-name-ex-96dqvi.name - source_ranges = ["35.191.0.0/16", "130.211.0.0/22", "209.85.204.0/22", "209.85.152.0/22"] + source_ranges = ["130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22", "35.191.0.0/16"] target_tags = ["minimal-gce-with-a-very-very-v-96dqvi-k8s-io-role-control-plane"] } resource "google_compute_firewall" "master-to-master-minimal-gce-with-a-very-very-very-very--96dqvi" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-master-minimal-gce-with-a-very-very-very-very--96dqvi" @@ -264,22 +264,22 @@ resource "google_compute_firewall" "master-to-master-minimal-gce-with-a-very-ver resource "google_compute_firewall" "master-to-node-minimal-gce-with-a-very-very-very-very-ve-96dqvi" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-node-minimal-gce-with-a-very-very-very-very-ve-96dqvi" @@ -290,15 +290,15 @@ resource "google_compute_firewall" "master-to-node-minimal-gce-with-a-very-very- resource "google_compute_firewall" "node-to-master-minimal-gce-with-a-very-very-very-very-ve-96dqvi" { allow { - ports = ["443"] + ports = ["10250"] protocol = "tcp" } allow { - ports = ["10250"] + ports = ["3988"] protocol = "tcp" } allow { - ports = ["3988"] + ports = ["443"] protocol = "tcp" } disabled = false @@ -310,22 +310,22 @@ resource "google_compute_firewall" "node-to-master-minimal-gce-with-a-very-very- resource "google_compute_firewall" "node-to-node-minimal-gce-with-a-very-very-very-very-very-96dqvi" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "node-to-node-minimal-gce-with-a-very-very-very-very-very-96dqvi" diff --git a/tests/integration/update_cluster/minimal_gce_longclustername/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_longclustername/kubernetes.tf index 9db8fb701d367..aff636912c8b6 100644 --- a/tests/integration/update_cluster/minimal_gce_longclustername/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_longclustername/kubernetes.tf @@ -220,22 +220,22 @@ resource "google_compute_firewall" "kubernetes-master-https-minimal-gce-with-a-v resource "google_compute_firewall" "master-to-master-minimal-gce-with-a-very-very-very-very--96dqvi" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-master-minimal-gce-with-a-very-very-very-very--96dqvi" @@ -246,22 +246,22 @@ resource "google_compute_firewall" "master-to-master-minimal-gce-with-a-very-ver resource "google_compute_firewall" "master-to-node-minimal-gce-with-a-very-very-very-very-ve-96dqvi" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-node-minimal-gce-with-a-very-very-very-very-ve-96dqvi" @@ -272,15 +272,15 @@ resource "google_compute_firewall" "master-to-node-minimal-gce-with-a-very-very- resource "google_compute_firewall" "node-to-master-minimal-gce-with-a-very-very-very-very-ve-96dqvi" { allow { - ports = ["443"] + ports = ["10250"] protocol = "tcp" } allow { - ports = ["10250"] + ports = ["3988"] protocol = "tcp" } allow { - ports = ["3988"] + ports = ["443"] protocol = "tcp" } disabled = false @@ -292,22 +292,22 @@ resource "google_compute_firewall" "node-to-master-minimal-gce-with-a-very-very- resource "google_compute_firewall" "node-to-node-minimal-gce-with-a-very-very-very-very-very-96dqvi" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "node-to-node-minimal-gce-with-a-very-very-very-very-very-96dqvi" diff --git a/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf b/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf index ff1863206620e..b79b6acd77cd9 100644 --- a/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_plb/kubernetes.tf @@ -236,28 +236,28 @@ resource "google_compute_firewall" "lb-health-checks-minimal-gce-plb-example-com disabled = false name = "lb-health-checks-minimal-gce-plb-example-com" network = google_compute_network.minimal-gce-plb-example-com.name - source_ranges = ["35.191.0.0/16", "130.211.0.0/22", "209.85.204.0/22", "209.85.152.0/22"] + source_ranges = ["130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22", "35.191.0.0/16"] target_tags = ["minimal-gce-plb-example-com-k8s-io-role-control-plane"] } resource "google_compute_firewall" "master-to-master-minimal-gce-plb-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-master-minimal-gce-plb-example-com" @@ -268,22 +268,22 @@ resource "google_compute_firewall" "master-to-master-minimal-gce-plb-example-com resource "google_compute_firewall" "master-to-node-minimal-gce-plb-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-node-minimal-gce-plb-example-com" @@ -294,15 +294,15 @@ resource "google_compute_firewall" "master-to-node-minimal-gce-plb-example-com" resource "google_compute_firewall" "node-to-master-minimal-gce-plb-example-com" { allow { - ports = ["443"] + ports = ["10250"] protocol = "tcp" } allow { - ports = ["10250"] + ports = ["3988"] protocol = "tcp" } allow { - ports = ["3988"] + ports = ["443"] protocol = "tcp" } disabled = false @@ -314,22 +314,22 @@ resource "google_compute_firewall" "node-to-master-minimal-gce-plb-example-com" resource "google_compute_firewall" "node-to-node-minimal-gce-plb-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "node-to-node-minimal-gce-plb-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 595e87d7dfd44..c686edc448d57 100644 --- a/tests/integration/update_cluster/minimal_gce_private/kubernetes.tf +++ b/tests/integration/update_cluster/minimal_gce_private/kubernetes.tf @@ -220,22 +220,22 @@ resource "google_compute_firewall" "kubernetes-master-https-minimal-gce-private- resource "google_compute_firewall" "master-to-master-minimal-gce-private-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-master-minimal-gce-private-example-com" @@ -246,22 +246,22 @@ resource "google_compute_firewall" "master-to-master-minimal-gce-private-example resource "google_compute_firewall" "master-to-node-minimal-gce-private-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "master-to-node-minimal-gce-private-example-com" @@ -272,15 +272,15 @@ resource "google_compute_firewall" "master-to-node-minimal-gce-private-example-c resource "google_compute_firewall" "node-to-master-minimal-gce-private-example-com" { allow { - ports = ["443"] + ports = ["10250"] protocol = "tcp" } allow { - ports = ["10250"] + ports = ["3988"] protocol = "tcp" } allow { - ports = ["3988"] + ports = ["443"] protocol = "tcp" } disabled = false @@ -292,22 +292,22 @@ resource "google_compute_firewall" "node-to-master-minimal-gce-private-example-c resource "google_compute_firewall" "node-to-node-minimal-gce-private-example-com" { allow { - protocol = "tcp" + protocol = "ah" } allow { - protocol = "udp" + protocol = "esp" } allow { protocol = "icmp" } allow { - protocol = "esp" + protocol = "sctp" } allow { - protocol = "ah" + protocol = "tcp" } allow { - protocol = "sctp" + protocol = "udp" } disabled = false name = "node-to-node-minimal-gce-private-example-com" diff --git a/upup/pkg/fi/changes.go b/upup/pkg/fi/changes.go index e876a16cc4dc7..1abd78cbbd3f8 100644 --- a/upup/pkg/fi/changes.go +++ b/upup/pkg/fi/changes.go @@ -138,12 +138,11 @@ func equalFieldValues(a, e reflect.Value) bool { // equalMapValues performs a deep-equality check on a map, but using our custom comparison logic (equalFieldValues) func equalMapValues(a, e reflect.Value) bool { - if a.IsNil() != e.IsNil() { + aIsEmpty := a.IsNil() || a.Len() == 0 + eIsEmpty := e.IsNil() || e.Len() == 0 + if aIsEmpty != eIsEmpty { return false } - if a.IsNil() && e.IsNil() { - return true - } if a.Len() != e.Len() { return false } diff --git a/upup/pkg/fi/cloudup/gcetasks/firewallrule.go b/upup/pkg/fi/cloudup/gcetasks/firewallrule.go index 29ceb66297493..c47422aaf51de 100644 --- a/upup/pkg/fi/cloudup/gcetasks/firewallrule.go +++ b/upup/pkg/fi/cloudup/gcetasks/firewallrule.go @@ -22,6 +22,7 @@ import ( "strings" compute "google.golang.org/api/compute/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/gce" "k8s.io/kops/upup/pkg/fi/cloudup/terraform" @@ -42,9 +43,9 @@ type FirewallRule struct { Network *Network SourceTags []string - SourceRanges []string + SourceRanges sets.Set[string] TargetTags []string - Allowed []string + Allowed sets.Set[string] // Disabled: Denotes whether the firewall rule is disabled. When set to // true, the firewall rule is not enforced and the network behaves as if @@ -75,11 +76,14 @@ func (e *FirewallRule) Find(c *fi.CloudupContext) (*FirewallRule, error) { actual.Name = &r.Name actual.Network = &Network{Name: fi.PtrTo(lastComponent(r.Network))} actual.TargetTags = r.TargetTags - actual.SourceRanges = r.SourceRanges + actual.SourceRanges = sets.New(r.SourceRanges...) actual.SourceTags = r.SourceTags actual.Disabled = r.Disabled for _, a := range r.Allowed { - actual.Allowed = append(actual.Allowed, serializeFirewallAllowed(a)) + if actual.Allowed == nil { + actual.Allowed = sets.New[string]() + } + actual.Allowed.Insert(serializeFirewallAllowed(a)) } // Ignore "system" fields @@ -114,7 +118,7 @@ func (e *FirewallRule) Normalize(c *fi.CloudupContext) error { // Make sure we've split the ipv4 / ipv6 addresses. // A single firewall rule can't mix ipv4 and ipv6 addresses, so we split them into two rules. - for _, sourceRange := range e.SourceRanges { + for sourceRange := range e.SourceRanges { _, cidr, err := net.ParseCIDR(sourceRange) if err != nil { return fmt.Errorf("sourceRange %q is not valid: %w", sourceRange, err) @@ -182,7 +186,7 @@ func serializeFirewallAllowed(r *compute.FirewallAllowed) string { func (e *FirewallRule) mapToGCE(project string) (*compute.Firewall, error) { var allowed []*compute.FirewallAllowed if e.Allowed != nil { - for _, a := range e.Allowed { + for _, a := range sets.List(e.Allowed) { p, err := parseFirewallAllowed(a) if err != nil { return nil, err @@ -194,7 +198,7 @@ func (e *FirewallRule) mapToGCE(project string) (*compute.Firewall, error) { Name: *e.Name, Network: e.Network.URL(project), SourceTags: e.SourceTags, - SourceRanges: e.SourceRanges, + SourceRanges: sets.List(e.SourceRanges), TargetTags: e.TargetTags, Allowed: allowed, Disabled: e.Disabled, diff --git a/upup/pkg/fi/topological_sort.go b/upup/pkg/fi/topological_sort.go index 9f1382e5f5fa8..b1477760b4036 100644 --- a/upup/pkg/fi/topological_sort.go +++ b/upup/pkg/fi/topological_sort.go @@ -125,6 +125,18 @@ func getDependencies[T SubContext](tasks map[string]Task[T], v reflect.Value) [] return nil } + // Ignore empty struct (struct{}) and other non-addressable types + if !v.CanAddr() { + typeName := v.Type().PkgPath() + "/" + v.Type().Name() + switch typeName { + case "k8s.io/apimachinery/pkg/util/sets/Empty": + // known + default: + klog.Warningf("skipping non-addressable type %v name=%q", v.Type(), typeName) + } + return nil + } + // TODO: Can we / should we use a type-switch statement intf := v.Addr().Interface() if hd, ok := intf.(HasDependencies[T]); ok {