From 7bc2a941b1a92404070577711fb31e4a5fab1661 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Wed, 2 Sep 2026 17:24:44 +0000 Subject: [PATCH] gcp: allow GCD load balancer health-check firewall ranges GCD regions have unique source IP addresses for load-balancer health probes that are distinct from GCP. So for GCD installs we need to populate the source based on the region/soverign data center. These ranges cannot be determined programmatically (e.g. via API), so they must be hard-coded. With this approach, unforunately, any new region will need to be manually updated; although, BYO firewall rules can be used until that time. Additionally, cloud-provider-gcp creates service-type LoadBalancer health-check firewall rules using the public GCP ranges, which GCD rejects, and it cannot be configured with the correct ranges. So this commit pre-creates a rule with the correct ranges, covering both externalTrafficPolicy=Cluster (10256) and =Local (the dynamic healthCheckNodePort range). While this opens a wide range of ports the source is tightly scoped to a well-known IP. Fixes: OCPBUGS-114882, OCPBUGS-112662 --- .../gcp/clusterapi/firewallrules.go | 85 +++++++++++++++++-- .../gcp/clusterapi/firewallrules_test.go | 53 ++++++++++++ 2 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 pkg/infrastructure/gcp/clusterapi/firewallrules_test.go diff --git a/pkg/infrastructure/gcp/clusterapi/firewallrules.go b/pkg/infrastructure/gcp/clusterapi/firewallrules.go index f5b2952ec33..b4495ecc939 100644 --- a/pkg/infrastructure/gcp/clusterapi/firewallrules.go +++ b/pkg/infrastructure/gcp/clusterapi/firewallrules.go @@ -38,6 +38,73 @@ func getHealthChecksPorts() []*compute.FirewallAllowed { } } +// getServiceLoadBalancerHealthCheckPorts returns the ports probed for +// service-type LoadBalancer health checks: 10256 (externalTrafficPolicy=Cluster, +// the shared node health-check port) and the nodeport range +// (externalTrafficPolicy=Local, the dynamically allocated healthCheckNodePort). +func getServiceLoadBalancerHealthCheckPorts() []*compute.FirewallAllowed { + return []*compute.FirewallAllowed{ + { + IPProtocol: "tcp", + Ports: []string{ + "10256", + "30000-32767", + }, + }, + } +} + +// gcpHealthCheckSourceRanges are the health-check prober ranges for public GCP. +var gcpHealthCheckSourceRanges = []string{ + "35.191.0.0/16", + "130.211.0.0/22", +} + +// gcpExternalHealthCheckSourceRanges are additionally required for external +// (public-facing) load balancers on public GCP. +var gcpExternalHealthCheckSourceRanges = []string{ + "209.85.152.0/22", + "209.85.204.0/22", +} + +// gcdHealthCheckSourceRanges maps a GCD region to its health-check prober +// ranges, which differ from public GCP and per region. Sourced from the +// "Probe IP ranges" section of each region's load-balancing/docs/firewall-rules +// (u-france-east1: https://documentation.s3ns.fr). +var gcdHealthCheckSourceRanges = map[string][]string{ + "u-germany-northeast1": { + "34.3.144.0/23", + "34.3.151.0/26", + "34.3.151.64/26", + "136.124.104.0/22", + "136.124.108.0/22", + }, + "u-france-east1": { + "177.222.80.0/23", + "177.222.87.0/26", + "177.222.87.64/26", + "136.124.104.0/22", + "136.124.108.0/22", + }, +} + +// healthCheckSourceRanges returns the health-check prober ranges the ingress +// firewall rule must allow for the given region. GCD regions use their +// region-specific ranges; other regions use the public GCP ranges, appending +// the external ranges when external is true (GCD is private-only). +func healthCheckSourceRanges(region string, external bool) []string { + if ranges, ok := gcdHealthCheckSourceRanges[region]; ok { + // Return a copy so callers cannot mutate the package-level slice. + return append([]string(nil), ranges...) + } + + ranges := append([]string(nil), gcpHealthCheckSourceRanges...) + if external { + ranges = append(ranges, gcpExternalHealthCheckSourceRanges...) + } + return ranges +} + func getControlPlanePorts() []*compute.FirewallAllowed { return []*compute.FirewallAllowed{ { @@ -246,15 +313,23 @@ func createFirewallRules(ctx context.Context, in clusterapi.InfraReadyInput, net firewallName = fmt.Sprintf("%s-health-checks", in.InfraID) srcTags = []string{} targetTags = []string{masterTag} - srcRanges = []string{"35.191.0.0/16", "130.211.0.0/22"} - if in.InstallConfig.Config.Publish == types.ExternalPublishingStrategy { - // public installs require additional google ip addresses for health checks - srcRanges = append(srcRanges, []string{"209.85.152.0/22", "209.85.204.0/22"}...) - } + srcRanges = healthCheckSourceRanges(in.InstallConfig.Config.GCP.Region, in.InstallConfig.Config.PublicAPI()) if err := addFirewallRule(ctx, svc, firewallName, network, projectID, getHealthChecksPorts(), srcTags, targetTags, srcRanges); err != nil { return err } + // On GCD, cloud-provider-gcp creates service-type LoadBalancer health-check firewall rules using the + // public-GCP prober ranges, which GCD does not use, so pre-create an additive rule with the correct + // GCD ranges. + if gcdRanges, ok := gcdHealthCheckSourceRanges[in.InstallConfig.Config.GCP.Region]; ok { + firewallName = fmt.Sprintf("%s-gcd-service-health-checks", in.InfraID) + srcTags = []string{} + targetTags = []string{workerTag, masterTag} + if err := addFirewallRule(ctx, svc, firewallName, network, projectID, getServiceLoadBalancerHealthCheckPorts(), srcTags, targetTags, gcdRanges); err != nil { + return err + } + } + // internal-cluster rules are needed for worker<->master communication for k8s nodes firewallName = fmt.Sprintf("%s-internal-cluster", in.InfraID) srcTags = []string{workerTag, masterTag} diff --git a/pkg/infrastructure/gcp/clusterapi/firewallrules_test.go b/pkg/infrastructure/gcp/clusterapi/firewallrules_test.go new file mode 100644 index 00000000000..818f5a26805 --- /dev/null +++ b/pkg/infrastructure/gcp/clusterapi/firewallrules_test.go @@ -0,0 +1,53 @@ +package clusterapi + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestHealthCheckSourceRanges(t *testing.T) { + cases := []struct { + name string + region string + external bool + expected []string + }{ + { + name: "public region internal", + region: "us-central1", + external: false, + expected: []string{"35.191.0.0/16", "130.211.0.0/22"}, + }, + { + name: "public region external appends google ranges", + region: "us-central1", + external: true, + expected: []string{"35.191.0.0/16", "130.211.0.0/22", "209.85.152.0/22", "209.85.204.0/22"}, + }, + { + name: "gcd berlin uses sovereign prober ranges", + region: "u-germany-northeast1", + external: false, + expected: []string{"34.3.144.0/23", "34.3.151.0/26", "34.3.151.64/26", "136.124.104.0/22", "136.124.108.0/22"}, + }, + { + name: "gcd berlin ignores external (private-only)", + region: "u-germany-northeast1", + external: true, + expected: []string{"34.3.144.0/23", "34.3.151.0/26", "34.3.151.64/26", "136.124.104.0/22", "136.124.108.0/22"}, + }, + { + name: "gcd france uses sovereign prober ranges", + region: "u-france-east1", + external: false, + expected: []string{"177.222.80.0/23", "177.222.87.0/26", "177.222.87.64/26", "136.124.104.0/22", "136.124.108.0/22"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expected, healthCheckSourceRanges(tc.region, tc.external)) + }) + } +}