diff --git a/pkg/firewalls/firewalls_l7_cr.go b/pkg/firewalls/firewalls_l7_cr.go index e986064989..85d0ef3352 100644 --- a/pkg/firewalls/firewalls_l7_cr.go +++ b/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 diff --git a/pkg/firewalls/firewalls_test.go b/pkg/firewalls/firewalls_test.go index 156840545e..f63b070fd3 100644 --- a/pkg/firewalls/firewalls_test.go +++ b/pkg/firewalls/firewalls_test.go @@ -130,13 +130,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 { @@ -398,6 +415,12 @@ 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" { @@ -405,7 +428,7 @@ func verifyFirewallCR(firewallclient *firewallclient.Clientset, ruleName string, } 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)) }