Merge pull request #94 from barbacbd/CORS-4264-openshift-clone

CORS-4264: Update the GCP provider to allow users to manage their own firewall rules

Author: openshift-merge-bot[bot]

cluster/gce/gci/configure-helper.sh

@@ -958,6 +958,14 @@ EOF
     use_cloud_config="true"
     cat <<EOF >>/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 <<EOF >>/etc/gce.conf
+firewall-rules-management = ${FIREWALLRULESMANAGEMENT}
 EOF
   fi
   if [[ -n "${GCE_ALPHA_FEATURES:-}" ]]; then

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

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}

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

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()
 

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

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()