Merge pull request kubernetes#71674 from grayluck/firewall-event-msg

Change XPN firewall change msg. Should be required by security admin
liufangwai · Oct 15, 2019 · 46a29a0 · 46a29a0
2 parents 63bd1d7 + bd03c3a
commit 46a29a0
Show file tree

Hide file tree

Showing 10 changed files with 36 additions and 16 deletions.
diff --git a/cmd/cloud-controller-manager/app/controllermanager.go b/cmd/cloud-controller-manager/app/controllermanager.go
@@ -93,6 +93,9 @@ the cloud specific control loops shipped with Kubernetes.`,
 		// the gce cloudprovider is removed.
 		globalflag.Register(namedFlagSets.FlagSet("generic"), "cloud-provider-gce-lb-src-cidrs")
 	}
+	if flag.CommandLine.Lookup("cloud-provider-gce-l7lb-src-cidrs") != nil {
+		globalflag.Register(namedFlagSets.FlagSet("generic"), "cloud-provider-gce-l7lb-src-cidrs")
+	}
 	for _, f := range namedFlagSets.FlagSets {
 		fs.AddFlagSet(f)
 	}

diff --git a/cmd/kube-apiserver/app/options/globalflags_providers.go b/cmd/kube-apiserver/app/options/globalflags_providers.go
@@ -26,5 +26,6 @@ import (
 
 func registerLegacyGlobalFlags(fs *pflag.FlagSet) {
 	globalflag.Register(fs, "cloud-provider-gce-lb-src-cidrs")
+	globalflag.Register(fs, "cloud-provider-gce-l7lb-src-cidrs")
 	fs.MarkDeprecated("cloud-provider-gce-lb-src-cidrs", "This flag will be removed once the GCE Cloud Provider is removed from kube-apiserver")
 }
diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer.go
@@ -40,18 +40,25 @@ type cidrs struct {
 }
 
 var (
-	lbSrcRngsFlag cidrs
+	l4LbSrcRngsFlag cidrs
+	l7lbSrcRngsFlag cidrs
 )
 
 func init() {
 	var err error
-	// LB L7 proxies and all L3/4/7 health checkers have client addresses within these known CIDRs.
-	lbSrcRngsFlag.ipn, err = utilnet.ParseIPNets([]string{"130.211.0.0/22", "35.191.0.0/16", "209.85.152.0/22", "209.85.204.0/22"}...)
+	// L3/4 health checkers have client addresses within these known CIDRs.
+	l4LbSrcRngsFlag.ipn, err = utilnet.ParseIPNets([]string{"130.211.0.0/22", "35.191.0.0/16", "209.85.152.0/22", "209.85.204.0/22"}...)
+	if err != nil {
+		panic("Incorrect default GCE L3/4 source ranges")
+	}
+	// L7 health checkers have client addresses within these known CIDRs.
+	l7lbSrcRngsFlag.ipn, err = utilnet.ParseIPNets([]string{"130.211.0.0/22", "35.191.0.0/16"}...)
 	if err != nil {
 		panic("Incorrect default GCE L7 source ranges")
 	}
 
-	flag.Var(&lbSrcRngsFlag, "cloud-provider-gce-lb-src-cidrs", "CIDRs opened in GCE firewall for LB traffic proxy & health checks")
+	flag.Var(&l4LbSrcRngsFlag, "cloud-provider-gce-lb-src-cidrs", "CIDRs opened in GCE firewall for L4 LB traffic proxy & health checks")
+	flag.Var(&l7lbSrcRngsFlag, "cloud-provider-gce-l7lb-src-cidrs", "CIDRs opened in GCE firewall for L7 LB traffic proxy & health checks")
 }
 
 // String is the method to format the flag's value, part of the flag.Value interface.
@@ -82,10 +89,16 @@ func (c *cidrs) Set(value string) error {
 	return nil
 }
 
-// LoadBalancerSrcRanges contains the ranges of ips used by the GCE load balancers (l4 & L7)
+// L4LoadBalancerSrcRanges contains the ranges of ips used by the L3/L4 GCE load balancers
+// for proxying client requests and performing health checks.
+func L4LoadBalancerSrcRanges() []string {
+	return l4LbSrcRngsFlag.ipn.StringSlice()
+}
+
+// L7LoadBalancerSrcRanges contains the ranges of ips used by the GCE load balancers L7
 // for proxying client requests and performing health checks.
-func LoadBalancerSrcRanges() []string {
-	return lbSrcRngsFlag.ipn.StringSlice()
+func L7LoadBalancerSrcRanges() []string {
+	return l7lbSrcRngsFlag.ipn.StringSlice()
 }
 
 // GetLoadBalancer is an implementation of LoadBalancer.GetLoadBalancer

diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external.go
@@ -876,7 +876,7 @@ func (g *Cloud) ensureHTTPHealthCheckFirewall(svc *v1.Service, serviceName, ipAd
 	if !isNodesHealthCheck {
 		desc = makeFirewallDescription(serviceName, ipAddress)
 	}
-	sourceRanges := lbSrcRngsFlag.ipn
+	sourceRanges := l4LbSrcRngsFlag.ipn
 	ports := []v1.ServicePort{{Protocol: "tcp", Port: hcPort}}
 
 	fwName := MakeHealthCheckFirewallName(clusterID, hcName, isNodesHealthCheck)

diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go
@@ -38,6 +38,10 @@ import (
 	utilnet "k8s.io/utils/net"
 )
 
+const (
+	eventMsgFirewallChange = "Firewall change required by security admin"
+)
+
 func TestEnsureStaticIP(t *testing.T) {
 	t.Parallel()
 

diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go
@@ -383,7 +383,7 @@ func (g *Cloud) ensureInternalFirewalls(loadBalancerName, ipAddress, clusterID s
 
 	// Second firewall is for health checking nodes / services
 	fwHCName := makeHealthCheckFirewallName(loadBalancerName, clusterID, sharedHealthCheck)
-	hcSrcRanges := LoadBalancerSrcRanges()
+	hcSrcRanges := L4LoadBalancerSrcRanges()
 	return g.ensureInternalFirewall(svc, fwHCName, "", hcSrcRanges, []string{healthCheckPort}, v1.ProtocolTCP, nodes)
 }
 

diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_utils_test.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_utils_test.go
@@ -43,7 +43,6 @@ import (
 // TODO(yankaiz): Create shared error types for both test/non-test codes.
 const (
 	eventReasonManualChange = "LoadBalancerManualChange"
-	eventMsgFirewallChange  = "Firewall change required by network admin"
 	errPrefixGetTargetPool  = "error getting load balancer's target pool:"
 	wrongTier               = "SupremeLuxury"
 	errStrUnsupportedTier   = "unsupported network tier: \"" + wrongTier + "\""

diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_util.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_util.go
@@ -110,7 +110,7 @@ func getProjectAndZone() (string, string, error) {
 }
 
 func (g *Cloud) raiseFirewallChangeNeededEvent(svc *v1.Service, cmd string) {
-	msg := fmt.Sprintf("Firewall change required by network admin: `%v`", cmd)
+	msg := fmt.Sprintf("Firewall change required by security admin: `%v`", cmd)
 	if g.eventRecorder != nil && svc != nil {
 		g.eventRecorder.Event(svc, v1.EventTypeNormal, "LoadBalancerManualChange", msg)
 	}

diff --git a/test/e2e/framework/providers/gce/firewall.go b/test/e2e/framework/providers/gce/firewall.go
@@ -75,7 +75,7 @@ func ConstructHealthCheckFirewallForLBService(clusterID string, svc *v1.Service,
 	fw := compute.Firewall{}
 	fw.Name = MakeHealthCheckFirewallNameForLBService(clusterID, cloudprovider.DefaultLoadBalancerName(svc), isNodesHealthCheck)
 	fw.TargetTags = []string{nodeTag}
-	fw.SourceRanges = gcecloud.LoadBalancerSrcRanges()
+	fw.SourceRanges = gcecloud.L4LoadBalancerSrcRanges()
 	healthCheckPort := gcecloud.GetNodesHealthCheckPort()
 	if !isNodesHealthCheck {
 		healthCheckPort = svc.Spec.HealthCheckNodePort

diff --git a/test/e2e/framework/providers/gce/gce.go b/test/e2e/framework/providers/gce/gce.go
@@ -310,10 +310,10 @@ func (p *Provider) cleanupGCEResources(c clientset.Interface, loadBalancerName,
 	return
 }
 
-// LoadBalancerSrcRanges contains the ranges of ips used by the GCE load balancers (l4 & L7)
-// for proxying client requests and performing health checks.
-func (p *Provider) LoadBalancerSrcRanges() []string {
-	return gcecloud.LoadBalancerSrcRanges()
+// L4LoadBalancerSrcRanges contains the ranges of ips used by the GCE L4 load
+// balancers for proxying client requests and performing health checks.
+func (p *Provider) L4LoadBalancerSrcRanges() []string {
+	return gcecloud.L4LoadBalancerSrcRanges()
 }
 
 // EnableAndDisableInternalLB returns functions for both enabling and disabling internal Load Balancer