Fix gcpfirewalls cr creation to properly set Allow - tcp:all instead of Allow - all

While testing gcpfirewalls cr creation inside ingress-gce we encountered an issue when Allow - TPC:all rule is requested.
The former implementation would create a cr with Allow:all instead, which is a violation of the specification:
https://cloud.google.com/kubernetes-engine/docs/concepts/firewall-rules#ingress-fws

Author: maciejriedl

pkg/firewalls/firewalls_l7_cr.go

@@ -169,6 +169,15 @@ func NewFirewallCR(name string, ports, srcRanges, dstRanges []string, enforced b
 		}
 		protocolPorts = append(protocolPorts, protocolPort)
 	}
+
+	// TCP:all is the default if the ports' list is empty.
+	if len(protocolPorts) == 0 {
+		protocolPort := gcpfirewallv1.ProtocolPort{
+			Protocol: gcpfirewallv1.ProtocolTCP,
+		}
+		protocolPorts = append(protocolPorts, protocolPort)
+	}
+
 	firewallCR.Spec.Ports = protocolPorts
 
 	var src_cidrs, dst_cidrs []gcpfirewallv1.CIDR

pkg/firewalls/firewalls_test.go

@@ -131,13 +131,30 @@ func TestFirewallPoolSyncSrcRanges(t *testing.T) {
 func TestFirewallPoolSyncPorts(t *testing.T) {
 	fwp := NewFakeFirewallsProvider(false, false)
 	fwClient := firewallclient.NewSimpleClientset()
-	fp := NewFirewallPool(fwp, defaultNamer, srcRanges, portRanges(), klog.TODO())
-	fcrp := NewFirewallCRPool(fwClient, fwp, defaultNamer, srcRanges, portRanges(), true, klog.TODO())
 	nodes := []string{"node-a", "node-b", "node-c"}
+	emptyPortRanges := make([]string, 0)
+
+	// Verify empty ports' list
+	fp := NewFirewallPool(fwp, defaultNamer, srcRanges, emptyPortRanges, klog.TODO())
+	fcrp := NewFirewallCRPool(fwClient, fwp, defaultNamer, srcRanges, emptyPortRanges, true, klog.TODO())
 
 	if err := fp.Sync(nodes, nil, nil, true); err != nil {
 		t.Fatal(err)
 	}
+	verifyFirewallRule(fwp, ruleName, nodes, srcRanges, emptyPortRanges, t)
+
+	if err := fcrp.Sync(nodes, nil, nil, true); err != nil {
+		t.Fatal(err)
+	}
+	verifyFirewallCR(fwClient, ruleName, srcRanges, emptyPortRanges, true, t)
+
+	// Verify a preset ports' list
+	fp = NewFirewallPool(fwp, defaultNamer, srcRanges, portRanges(), klog.TODO())
+	fcrp = NewFirewallCRPool(fwClient, fwp, defaultNamer, srcRanges, portRanges(), true, klog.TODO())
+
+	if err := fp.Sync(nodes, nil, nil, true); err != nil {
+		t.Errorf("unexpected err when syncing firewall, err: %v", err)
+	}
 	verifyFirewallRule(fwp, ruleName, nodes, srcRanges, portRanges(), t)
 
 	if err := fcrp.Sync(nodes, nil, nil, true); err != nil {
@@ -399,14 +416,20 @@ func verifyFirewallCR(firewallclient *firewallclient.Clientset, ruleName string,
 	ports := sets.NewString(expectedPorts...)
 	srcranges := sets.NewString(sourceRanges...)
 
+	// Empty ports' list would mean that all protocols are permitted
+	// (not only TCP)
+	if len(actualFW.Spec.Ports) == 0 {
+		t.Errorf("Empty list of allowed protocols is not permited")
+	}
+
 	actualPorts := sets.NewString()
 	for _, protocolports := range actualFW.Spec.Ports {
 		if protocolports.Protocol != "TCP" {
 			t.Errorf("Protocol isn't TCP")
 		}
 		if protocolports.EndPort != nil {
 			actualPorts.Insert(fmt.Sprintf("%d-%d", *protocolports.StartPort, *protocolports.EndPort))
-		} else {
+		} else if protocolports.StartPort != nil {
 			actualPorts.Insert(fmt.Sprintf("%d", *protocolports.StartPort))
 		}