Merge pull request #59373 from nicksardo/ingress-gce-firewall

Automatic merge from submit-queue (batch tested with PRs 59373, 59379, 59252, 58295, 57786). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

GCE: Check nodeports are covered by firewall rule with port ranges

**What this PR does / why we need it**:
When testing firewalls for GCE ingresses, we should assert that particular nodeports are covered by individual ports or port ranges. Currently, port ranges are not acceptable input.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Will fix the currently erroring `ingress-gce-e2e` tests

**Special notes for your reviewer**:
/cc @MrHohn 
/assign @bowei

**Release note**:
```release-note
NONE
```

test/e2e/framework/BUILD

@@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
+    "go_test",
 )
 
 go_library(
@@ -169,3 +170,10 @@ filegroup(
     ],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["firewall_util_test.go"],
+    embed = [":go_default_library"],
+    importpath = "k8s.io/kubernetes/test/e2e/framework",
+)

test/e2e/framework/firewall_util.go

@@ -302,6 +302,83 @@ func PackProtocolsPortsFromFirewall(alloweds []*compute.FirewallAllowed) []strin
 	return protocolPorts
 }
 
+type portRange struct {
+	protocol string
+	min, max int
+}
+
+func toPortRange(s string) (pr portRange, err error) {
+	protoPorts := strings.Split(s, "/")
+	// Set protocol
+	pr.protocol = strings.ToUpper(protoPorts[0])
+
+	if len(protoPorts) != 2 {
+		return pr, fmt.Errorf("expected a single '/' in %q", s)
+	}
+
+	ports := strings.Split(protoPorts[1], "-")
+	switch len(ports) {
+	case 1:
+		v, err := strconv.Atoi(ports[0])
+		if err != nil {
+			return pr, err
+		}
+		pr.min, pr.max = v, v
+	case 2:
+		start, err := strconv.Atoi(ports[0])
+		if err != nil {
+			return pr, err
+		}
+		end, err := strconv.Atoi(ports[1])
+		if err != nil {
+			return pr, err
+		}
+		pr.min, pr.max = start, end
+	default:
+		return pr, fmt.Errorf("unexpected range value %q", protoPorts[1])
+	}
+
+	return pr, nil
+}
+
+// isPortsSubset asserts that the "requiredPorts" are covered by the "coverage" ports.
+// requiredPorts - must be single-port, examples: 'tcp/50', 'udp/80'.
+// coverage - single or port-range values, example: 'tcp/50', 'udp/80-1000'.
+// Returns true if every requiredPort exists in the list of coverage rules.
+func isPortsSubset(requiredPorts, coverage []string) error {
+	for _, reqPort := range requiredPorts {
+		rRange, err := toPortRange(reqPort)
+		if err != nil {
+			return err
+		}
+		if rRange.min != rRange.max {
+			return fmt.Errorf("requiring a range is not supported: %q", reqPort)
+		}
+
+		var covered bool
+		for _, c := range coverage {
+			cRange, err := toPortRange(c)
+			if err != nil {
+				return err
+			}
+
+			if rRange.protocol != cRange.protocol {
+				continue
+			}
+
+			if rRange.min >= cRange.min && rRange.min <= cRange.max {
+				covered = true
+				break
+			}
+		}
+
+		if !covered {
+			return fmt.Errorf("%q is not covered by %v", reqPort, coverage)
+		}
+	}
+	return nil
+}
+
 // SameStringArray verifies whether two string arrays have the same strings, return error if not.
 // Order does not matter.
 // When `include` is set to true, verifies whether result includes all elements from expected.
@@ -334,10 +411,19 @@ func VerifyFirewallRule(res, exp *compute.Firewall, network string, portsSubset
 	if !strings.HasSuffix(res.Network, "/"+network) {
 		return fmt.Errorf("incorrect network: %v, expected ends with: %v", res.Network, "/"+network)
 	}
-	if err := SameStringArray(PackProtocolsPortsFromFirewall(res.Allowed),
-		PackProtocolsPortsFromFirewall(exp.Allowed), portsSubset); err != nil {
+
+	actualPorts := PackProtocolsPortsFromFirewall(res.Allowed)
+	expPorts := PackProtocolsPortsFromFirewall(exp.Allowed)
+	if portsSubset {
+		if err := isPortsSubset(expPorts, actualPorts); err != nil {
+			return fmt.Errorf("incorrect allowed protocol ports: %v", err)
+		}
+	} else {
+		if err := SameStringArray(actualPorts, expPorts, false); err != nil {
 			return fmt.Errorf("incorrect allowed protocols ports: %v", err)
 		}
+	}
+
 	if err := SameStringArray(res.SourceRanges, exp.SourceRanges, false); err != nil {
 		return fmt.Errorf("incorrect source ranges %v, expected %v: %v", res.SourceRanges, exp.SourceRanges, err)
 	}

test/e2e/framework/firewall_util_test.go

@@ -0,0 +1,54 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package framework
+
+import "testing"
+
+func TestIsPortsSubset(t *testing.T) {
+	tc := map[string]struct {
+		required  []string
+		coverage  []string
+		expectErr bool
+	}{
+		"Single port coverage": {
+			required: []string{"tcp/50"},
+			coverage: []string{"tcp/50", "tcp/60", "tcp/70"},
+		},
+		"Port range coverage": {
+			required: []string{"tcp/50"},
+			coverage: []string{"tcp/20-30", "tcp/45-60"},
+		},
+		"Multiple Port range coverage": {
+			required: []string{"tcp/50", "tcp/29", "tcp/46"},
+			coverage: []string{"tcp/20-30", "tcp/45-60"},
+		},
+		"Not covered": {
+			required:  []string{"tcp/50"},
+			coverage:  []string{"udp/50", "tcp/49", "tcp/51-60"},
+			expectErr: true,
+		},
+	}
+
+	for name, c := range tc {
+		t.Run(name, func(t *testing.T) {
+			gotErr := isPortsSubset(c.required, c.coverage)
+			if c.expectErr != (gotErr != nil) {
+				t.Errorf("isPortsSubset(%v, %v) = %v, wanted err? %v", c.required, c.coverage, gotErr, c.expectErr)
+			}
+		})
+	}
+}