Skip to content

gce: use set type where order does not matter #17377

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

gce: use set type where order does not matter #17377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d264354
gce: use set type where order does not matter
justinsb Apr 27, 2025
ee0b15b
autogen: update expected output for Set[string] changes
justinsb Apr 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions pkg/model/gcemodel/api_loadbalancer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand All @@ -112,19 +113,19 @@ 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)),
})
}

if b.Cluster.UsesNoneDNS() {
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)),
})
}
}
Expand Down
35 changes: 18 additions & 17 deletions pkg/model/gcemodel/external_access.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
})
Expand All @@ -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,
})
}
Expand All @@ -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,
})
}
Expand All @@ -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,
})

Expand All @@ -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)),
})
}
}
Expand Down
55 changes: 28 additions & 27 deletions pkg/model/gcemodel/firewall.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"),
})
}

Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
Expand All @@ -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...),
})
}
}
Expand All @@ -189,19 +190,19 @@ 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)
}

// 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)
}
}

Expand All @@ -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)
Expand All @@ -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)
Expand Down
36 changes: 18 additions & 18 deletions tests/integration/update_cluster/ha_gce/kubernetes.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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
Expand All @@ -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"
Expand Down
Loading
Loading