From 7f2fae417310c414e096dc1c476b7f61240108db Mon Sep 17 00:00:00 2001 From: barbacbd Date: Tue, 21 Oct 2025 09:58:36 -0400 Subject: [PATCH] UPSTREAM: 911: allow users to manage their own firewall rules cluster: Update the scripts to include the new variables providers/gce: Update the config to include the new `FirewallRulesManagement` string that can be set to Enabled or Disabled. This variable will allow users to skip the creation, deletion, and updates to firewall rules when set to Disabled. Users may not want or have the ability to add the permissions to perform these actions on their service account. When this is the case the firewall rules should be pre created and managed by someone with permissions to achieve the same goal. ** This is a cherry-pick from the upstream project --- cluster/gce/gci/configure-helper.sh | 8 ++ cluster/gce/util.sh | 1 + providers/gce/gce.go | 20 ++++ providers/gce/gce_fake.go | 67 +++++++------ providers/gce/gce_firewall.go | 20 ++++ providers/gce/gce_loadbalancer_external.go | 21 ++++ .../gce/gce_loadbalancer_external_test.go | 98 +++++++++++++++++++ providers/gce/gce_loadbalancer_internal.go | 14 +++ 8 files changed, 217 insertions(+), 32 deletions(-) diff --git a/cluster/gce/gci/configure-helper.sh b/cluster/gce/gci/configure-helper.sh index 74788b2949..4cf048379b 100755 --- a/cluster/gce/gci/configure-helper.sh +++ b/cluster/gce/gci/configure-helper.sh @@ -958,6 +958,14 @@ EOF use_cloud_config="true" cat <>/etc/gce.conf regional = ${MULTIMASTER} +EOF + fi +# FirewallRulesManagement indicates whethere the firewall rules are +# managed (enabled) or not (disabled) for the provider. + if [[ -n "${FIREWALLRULESMANAGEMENT:-}" ]]; then + use_cloud_config="true" + cat <>/etc/gce.conf +firewall-rules-management = ${FIREWALLRULESMANAGEMENT} EOF fi if [[ -n "${GCE_ALPHA_FEATURES:-}" ]]; then diff --git a/cluster/gce/util.sh b/cluster/gce/util.sh index 494b4f0766..0cc8e65c24 100755 --- a/cluster/gce/util.sh +++ b/cluster/gce/util.sh @@ -1190,6 +1190,7 @@ KUBE_DOCKER_REGISTRY: $(yaml-quote "${KUBE_DOCKER_REGISTRY:-}") KUBE_ADDON_REGISTRY: $(yaml-quote "${KUBE_ADDON_REGISTRY:-}") MULTIZONE: $(yaml-quote "${MULTIZONE:-}") MULTIMASTER: $(yaml-quote "${MULTIMASTER:-}") +FIREWALLRULESMANAGEMENT: $(yaml-quote "${FIREWALLRULESMANAGEMENT:-}") NON_MASQUERADE_CIDR: $(yaml-quote "${NON_MASQUERADE_CIDR:-}") ENABLE_DEFAULT_STORAGE_CLASS: $(yaml-quote "${ENABLE_DEFAULT_STORAGE_CLASS:-}") # (TODO/cloud-provider-gcp): Need to figure out how to inject this diff --git a/providers/gce/gce.go b/providers/gce/gce.go index b17c9f8142..6c0ac72d37 100644 --- a/providers/gce/gce.go +++ b/providers/gce/gce.go @@ -116,6 +116,17 @@ const clusterStackIPV4 StackType = "IPV4" // The underlying VPC's stack type could be either IPV6 or dual stack IPV4_IPV6. const clusterStackIPV6 StackType = "IPV6" +// FirewallRulesManagement indicates how firewall rules are managed by the provider. +type FirewallRulesManagement string + +// firewallRulesManagementEnabled indicates that the firewall rules should be managed by the provider. +// This includes firewall rule creation, deletion, and updates. +const firewallRulesManagementEnabled FirewallRulesManagement = "Enabled" + +// firewallRulesManagementDisabled indicates that the firewall rules should not be managed by the provider. +// This includes firewall rule creation, deletion, and updates. +const firewallRulesManagementDisabled FirewallRulesManagement = "Disabled" + // Cloud is an implementation of Interface, LoadBalancer and Instances for Google Compute Engine. type Cloud struct { // ClusterID contains functionality for getting (and initializing) the ingress-uid. Call Cloud.Initialize() @@ -213,6 +224,8 @@ type Cloud struct { // enableRBSDefaultForL4NetLB disable Service controller from picking up services by default enableRBSDefaultForL4NetLB bool + + firewallRulesManagement FirewallRulesManagement } // ConfigGlobal is the in memory representation of the gce.conf config data @@ -254,6 +267,10 @@ type ConfigGlobal struct { // ExternalInstanceGroupsPrefix, when not-empty, is used to filter instance groups (from an external GCP Project) // and include them in the backend for ILB. ExternalInstanceGroupsPrefix string `gcfg:"external-instance-groups-prefix"` + + // FirewallRulesManagement indicates whether the provider should handle all firewall + // operations, such as creation, deletion, and updates. + FirewallRulesManagement string `gcfg:"firewall-rules-management"` } // ConfigFile is the struct used to parse the /etc/gce.conf configuration file. @@ -289,6 +306,7 @@ type CloudConfig struct { AlphaFeatureGate *AlphaFeatureGate StackType string ExternalInstanceGroupsPrefix string + FirewallRulesManagement string } func init() { @@ -380,6 +398,7 @@ func generateCloudConfig(configFile *ConfigFile) (cloudConfig *CloudConfig, err cloudConfig.NodeInstancePrefix = configFile.Global.NodeInstancePrefix cloudConfig.AlphaFeatureGate = NewAlphaFeatureGate(configFile.Global.AlphaFeatures) cloudConfig.ExternalInstanceGroupsPrefix = configFile.Global.ExternalInstanceGroupsPrefix + cloudConfig.FirewallRulesManagement = configFile.Global.FirewallRulesManagement } // retrieve projectID and zone @@ -584,6 +603,7 @@ func CreateGCECloud(config *CloudConfig) (*Cloud, error) { projectsBasePath: getProjectsBasePath(service.BasePath), stackType: StackType(config.StackType), externalInstanceGroupsPrefix: config.ExternalInstanceGroupsPrefix, + firewallRulesManagement: FirewallRulesManagement(config.FirewallRulesManagement), } gce.manager = &gceServiceManager{gce} diff --git a/providers/gce/gce_fake.go b/providers/gce/gce_fake.go index 1c27eec0f0..0f99cfea43 100644 --- a/providers/gce/gce_fake.go +++ b/providers/gce/gce_fake.go @@ -30,30 +30,32 @@ import ( // TestClusterValues holds the config values for the fake/test gce cloud object. type TestClusterValues struct { - ProjectID string - Region string - ZoneName string - SecondaryZoneName string - ClusterID string - ClusterName string - OnXPN bool - Regional bool - NetworkURL string - SubnetworkURL string - StackType StackType + ProjectID string + Region string + ZoneName string + SecondaryZoneName string + ClusterID string + ClusterName string + OnXPN bool + Regional bool + NetworkURL string + SubnetworkURL string + StackType StackType + FirewallRulesManagement FirewallRulesManagement } // DefaultTestClusterValues Creates a reasonable set of default cluster values // for generating a new test fake GCE cloud instance. func DefaultTestClusterValues() TestClusterValues { return TestClusterValues{ - ProjectID: "test-project", - Region: "us-central1", - ZoneName: "us-central1-b", - SecondaryZoneName: "us-central1-c", - ClusterID: "test-cluster-id", - ClusterName: "Test-Cluster-Name", - StackType: clusterStackIPV4, + ProjectID: "test-project", + Region: "us-central1", + ZoneName: "us-central1-b", + SecondaryZoneName: "us-central1-c", + ClusterID: "test-cluster-id", + ClusterName: "Test-Cluster-Name", + StackType: clusterStackIPV4, + FirewallRulesManagement: firewallRulesManagementEnabled, } } @@ -75,20 +77,21 @@ func NewFakeGCECloud(vals TestClusterValues) *Cloud { panic(err) } gce := &Cloud{ - region: vals.Region, - service: service, - managedZones: []string{vals.ZoneName, vals.SecondaryZoneName}, - localZone: vals.ZoneName, - projectID: vals.ProjectID, - networkProjectID: vals.ProjectID, - ClusterID: fakeClusterID(vals.ClusterID), - onXPN: vals.OnXPN, - metricsCollector: newLoadBalancerMetrics(), - projectsBasePath: getProjectsBasePath(service.BasePath), - regional: vals.Regional, - networkURL: vals.NetworkURL, - unsafeSubnetworkURL: vals.SubnetworkURL, - stackType: vals.StackType, + region: vals.Region, + service: service, + managedZones: []string{vals.ZoneName, vals.SecondaryZoneName}, + localZone: vals.ZoneName, + projectID: vals.ProjectID, + networkProjectID: vals.ProjectID, + ClusterID: fakeClusterID(vals.ClusterID), + onXPN: vals.OnXPN, + metricsCollector: newLoadBalancerMetrics(), + projectsBasePath: getProjectsBasePath(service.BasePath), + regional: vals.Regional, + networkURL: vals.NetworkURL, + unsafeSubnetworkURL: vals.SubnetworkURL, + stackType: vals.StackType, + firewallRulesManagement: vals.FirewallRulesManagement, } c := cloud.NewMockGCE(&gceProjectRouter{gce}) gce.c = c diff --git a/providers/gce/gce_firewall.go b/providers/gce/gce_firewall.go index afb59fe6c1..ac2d167efb 100644 --- a/providers/gce/gce_firewall.go +++ b/providers/gce/gce_firewall.go @@ -32,6 +32,10 @@ func newFirewallMetricContext(request string) *metricContext { // GetFirewall returns the Firewall by name. func (g *Cloud) GetFirewall(name string) (*compute.Firewall, error) { + if g.firewallRulesManagement == firewallRulesManagementDisabled { + return nil, nil + } + ctx, cancel := cloud.ContextWithCallTimeout() defer cancel() @@ -42,6 +46,10 @@ func (g *Cloud) GetFirewall(name string) (*compute.Firewall, error) { // CreateFirewall creates the passed firewall func (g *Cloud) CreateFirewall(f *compute.Firewall) error { + if g.firewallRulesManagement == firewallRulesManagementDisabled { + return nil + } + ctx, cancel := cloud.ContextWithCallTimeout() defer cancel() @@ -51,6 +59,10 @@ func (g *Cloud) CreateFirewall(f *compute.Firewall) error { // DeleteFirewall deletes the given firewall rule. func (g *Cloud) DeleteFirewall(name string) error { + if g.firewallRulesManagement == firewallRulesManagementDisabled { + return nil + } + ctx, cancel := cloud.ContextWithCallTimeout() defer cancel() @@ -60,6 +72,10 @@ func (g *Cloud) DeleteFirewall(name string) error { // UpdateFirewall applies the given firewall as an update to an existing service. func (g *Cloud) UpdateFirewall(f *compute.Firewall) error { + if g.firewallRulesManagement == firewallRulesManagementDisabled { + return nil + } + ctx, cancel := cloud.ContextWithCallTimeout() defer cancel() @@ -69,6 +85,10 @@ func (g *Cloud) UpdateFirewall(f *compute.Firewall) error { // PatchFirewall applies the given firewall as an update to an existing service. func (g *Cloud) PatchFirewall(f *compute.Firewall) error { + if g.firewallRulesManagement == firewallRulesManagementDisabled { + return nil + } + ctx, cancel := cloud.ContextWithCallTimeout() defer cancel() diff --git a/providers/gce/gce_loadbalancer_external.go b/providers/gce/gce_loadbalancer_external.go index e0855a7c8c..cb6015603b 100644 --- a/providers/gce/gce_loadbalancer_external.go +++ b/providers/gce/gce_loadbalancer_external.go @@ -964,6 +964,11 @@ func translateAffinityType(affinityType v1.ServiceAffinity) string { } func (g *Cloud) firewallNeedsUpdate(name, serviceName, ipAddress string, ports []v1.ServicePort, sourceRanges utilnet.IPNetSet) (exists bool, needsUpdate bool, err error) { + if g.firewallRulesManagement == firewallRulesManagementDisabled { + klog.V(2).Infof("firewallNeedsUpdate(%v): firewall rules are unmanaged", name) + return false, false, nil + } + fw, err := g.GetFirewall(MakeFirewallName(name)) if err != nil { if isHTTPErrorCode(err, http.StatusNotFound) { @@ -1009,6 +1014,11 @@ func (g *Cloud) firewallNeedsUpdate(name, serviceName, ipAddress string, ports [ } func (g *Cloud) ensureHTTPHealthCheckFirewall(svc *v1.Service, serviceName, ipAddress, region, clusterID string, hosts []*gceInstance, hcName string, hcPort int32, isNodesHealthCheck bool) error { + if g.firewallRulesManagement == firewallRulesManagementDisabled { + klog.V(2).Infof("ensureHTTPHealthCheckFirewall(%v): firewall rules are unmanaged", hcName) + return nil + } + // Prepare the firewall params for creating / checking. desc := fmt.Sprintf(`{"kubernetes.io/cluster-id":"%s"}`, clusterID) if !isNodesHealthCheck { @@ -1087,6 +1097,17 @@ func (g *Cloud) createFirewall(svc *v1.Service, name, desc, destinationIP string if err != nil { return err } + + if g.firewallRulesManagement == firewallRulesManagementDisabled { + klog.V(2).Infof("createFirewall(%v): firewall rules are unmanaged", name) + project := g.NetworkProjectID() + if project == "" { + project = g.ProjectID() + } + g.raiseFirewallChangeNeededEvent(svc, FirewallToGCloudCreateCmd(firewall, project)) + return nil + } + if err = g.CreateFirewall(firewall); err != nil { if isHTTPErrorCode(err, http.StatusConflict) { return nil diff --git a/providers/gce/gce_loadbalancer_external_test.go b/providers/gce/gce_loadbalancer_external_test.go index 8c6742772f..5dcd423ce5 100644 --- a/providers/gce/gce_loadbalancer_external_test.go +++ b/providers/gce/gce_loadbalancer_external_test.go @@ -1902,6 +1902,104 @@ func TestFirewallNeedsUpdate(t *testing.T) { } } +func TestDisabledFirewallOperations(t *testing.T) { + vals := DefaultTestClusterValues() + vals.FirewallRulesManagement = firewallRulesManagementDisabled + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + + fw, err := gce.GetFirewall(MakeFirewallName("test")) + assert.NoError(t, err) + assert.Nil(t, fw) + + ipnet, err := utilnet.ParseIPNets("0.0.0.0/0") + require.NoError(t, err) + + ports := []v1.ServicePort{ + {Name: "port1", Protocol: v1.ProtocolTCP, Port: int32(80), TargetPort: intstr.FromInt(80)}, + {Name: "port2", Protocol: v1.ProtocolTCP, Port: int32(81), TargetPort: intstr.FromInt(81)}, + {Name: "port3", Protocol: v1.ProtocolTCP, Port: int32(82), TargetPort: intstr.FromInt(82)}, + {Name: "port4", Protocol: v1.ProtocolTCP, Port: int32(84), TargetPort: intstr.FromInt(84)}, + {Name: "port5", Protocol: v1.ProtocolTCP, Port: int32(85), TargetPort: intstr.FromInt(85)}, + {Name: "port6", Protocol: v1.ProtocolTCP, Port: int32(86), TargetPort: intstr.FromInt(86)}, + {Name: "port7", Protocol: v1.ProtocolTCP, Port: int32(88), TargetPort: intstr.FromInt(87)}, + } + + firewall, err := gce.firewallObject(MakeFirewallName("test"), "Test Description", "0.0.0.0/0", ipnet, ports, nil) + + err = gce.CreateFirewall(firewall) + assert.NoError(t, err) + + err = gce.UpdateFirewall(firewall) + assert.NoError(t, err) + + err = gce.PatchFirewall(firewall) + assert.NoError(t, err) + + err = gce.DeleteFirewall(MakeFirewallName("test")) + assert.NoError(t, err) +} + +func TestDisabledFirewallNeedsUpdate(t *testing.T) { + t.Parallel() + + vals := DefaultTestClusterValues() + vals.FirewallRulesManagement = firewallRulesManagementDisabled + gce, err := fakeGCECloud(vals) + require.NoError(t, err) + svc := fakeLoadbalancerService("") + + svc, err = gce.client.CoreV1().Services(svc.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{}) + require.NoError(t, err) + + svc.Spec.Ports = []v1.ServicePort{ + {Name: "port1", Protocol: v1.ProtocolTCP, Port: int32(80), TargetPort: intstr.FromInt(80)}, + {Name: "port2", Protocol: v1.ProtocolTCP, Port: int32(81), TargetPort: intstr.FromInt(81)}, + {Name: "port3", Protocol: v1.ProtocolTCP, Port: int32(82), TargetPort: intstr.FromInt(82)}, + {Name: "port4", Protocol: v1.ProtocolTCP, Port: int32(84), TargetPort: intstr.FromInt(84)}, + {Name: "port5", Protocol: v1.ProtocolTCP, Port: int32(85), TargetPort: intstr.FromInt(85)}, + {Name: "port6", Protocol: v1.ProtocolTCP, Port: int32(86), TargetPort: intstr.FromInt(86)}, + {Name: "port7", Protocol: v1.ProtocolTCP, Port: int32(88), TargetPort: intstr.FromInt(87)}, + } + + status, err := createExternalLoadBalancer(gce, svc, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName) + require.NotNil(t, status) + require.NoError(t, err) + svcName := "/" + svc.ObjectMeta.Name + + ipAddr := status.Ingress[0].IP + lbName := gce.GetLoadBalancerName(context.TODO(), "", svc) + + ipnet, err := utilnet.ParseIPNets("0.0.0.0/0") + require.NoError(t, err) + + fw, err := gce.GetFirewall(MakeFirewallName(lbName)) + require.NoError(t, err) + + for desc := range map[string]struct { + hasErr bool + }{ + "need to update port-ranges ": {}, + } { + t.Run(desc, func(t *testing.T) { + fw, err = gce.GetFirewall(MakeFirewallName(lbName)) + assert.NoError(t, err) + assert.Nil(t, fw) + + exists, needsUpdate, err := gce.firewallNeedsUpdate( + lbName, + svcName, + ipAddr, + svc.Spec.Ports, + ipnet) + + assert.Equal(t, false, exists, "firewall should not exist") + assert.Equal(t, false, needsUpdate, "firewall should not exist, no update needed") + assert.NoError(t, err) + }) + } +} + func TestDeleteWrongNetworkTieredResourcesSucceedsWhenNotFound(t *testing.T) { t.Parallel() diff --git a/providers/gce/gce_loadbalancer_internal.go b/providers/gce/gce_loadbalancer_internal.go index 800871d688..0022bddbf3 100644 --- a/providers/gce/gce_loadbalancer_internal.go +++ b/providers/gce/gce_loadbalancer_internal.go @@ -406,6 +406,10 @@ func (g *Cloud) ensureInternalLoadBalancerDeleted(clusterName, clusterID string, } deleteFunc := func(fwName string) error { + if g.firewallRulesManagement == firewallRulesManagementDisabled { + klog.V(2).Infof("ensureInternalLoadBalancerDeleted(%v): firewall rules are unmanaged", fwName) + return nil + } if err := ignoreNotFound(g.DeleteFirewall(fwName)); err != nil { if isForbidden(err) && g.OnXPN() { klog.V(2).Infof("ensureInternalLoadBalancerDeleted(%v): could not delete traffic firewall on XPN cluster. Raising event.", loadBalancerName) @@ -481,6 +485,11 @@ func (g *Cloud) teardownInternalHealthCheckAndFirewall(svc *v1.Service, hcName s } klog.V(2).Infof("teardownInternalHealthCheckAndFirewall(%v): health check deleted", hcName) + if g.firewallRulesManagement == firewallRulesManagementDisabled { + klog.V(2).Infof("teardownInternalHealthCheckAndFirewall(%v): unmanaged firewall rules", hcName) + return nil + } + hcFirewallName := makeHealthCheckFirewallNameFromHC(hcName) if err := ignoreNotFound(g.DeleteFirewall(hcFirewallName)); err != nil { if isForbidden(err) && g.OnXPN() { @@ -497,6 +506,11 @@ func (g *Cloud) teardownInternalHealthCheckAndFirewall(svc *v1.Service, hcName s func (g *Cloud) ensureInternalFirewall(svc *v1.Service, fwName, fwDesc, destinationIP string, sourceRanges []string, portRanges []string, protocol v1.Protocol, nodes []*v1.Node, legacyFwName string) error { klog.V(2).Infof("ensureInternalFirewall(%v): checking existing firewall", fwName) + if g.firewallRulesManagement == firewallRulesManagementDisabled { + klog.V(2).Infof("ensureInternalFirewall(%v): firewall rules are unmanaged", fwName) + return nil + } + targetTags, err := g.GetNodeTags(nodeNames(nodes)) if err != nil { return err