Skip to content

Commit

Permalink
Fix firewalls for L4 ILBs nad NetLBs.
Browse files Browse the repository at this point in the history
There was an omission in the code for multinet and some firewall rules were created in the incorrect VPC.
  • Loading branch information
mmamczur committed Jun 9, 2023
1 parent 6b2e60c commit 06386ad
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/loadbalancers/l4.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ func (l4 *L4) ensureIPv4NodesFirewall(nodeNames []string, ipAddress string, resu
Name: firewallName,
NodeNames: nodeNames,
L4Type: utils.ILB,
Network: l4.network,
}

err = firewalls.EnsureL4LBFirewallForNodes(l4.Service, &nodesFWRParams, l4.cloud, l4.recorder)
Expand Down
119 changes: 119 additions & 0 deletions pkg/loadbalancers/l4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,110 @@ func TestEnsureInternalDualStackLoadBalancer(t *testing.T) {
}
}

func TestEnsureIPv4Firewall4Nodes(t *testing.T) {
t.Parallel()
fakeGCE := getFakeGCECloud(gce.DefaultTestClusterValues())
nodeNames := []string{"test-node-1"}
// create a test VM so that target tags can be found
createVMInstanceWithTag(t, fakeGCE, "test-node-1", "test-node-1")

svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)

networkURL := "testNet"
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
Network: network.NetworkInfo{NetworkURL: networkURL, SubnetworkURL: "testSubnet"},
}
l4 := NewL4Handler(l4ilbParams)
syncResult := &L4ILBSyncResult{
Annotations: make(map[string]string),
}

l4.ensureIPv4NodesFirewall(nodeNames, "10.0.0.7", syncResult)
if syncResult.Error != nil {
t.Fatalf("ensureIPv4NodesFirewall() error %+v", syncResult)
}

firewallName := l4.namer.L4Firewall(l4.Service.Namespace, l4.Service.Name)
firewall, err := fakeGCE.GetFirewall(firewallName)
if err != nil {
t.Fatalf("GetFirewall error %v", err)
}
if firewall.Name != firewallName {
t.Errorf("Firewall.Name invalid, want=%s, got=%s", firewallName, firewall.Name)
}
if firewall.Network != networkURL {
t.Errorf("Firewall.Network invalid, want=%s, got=%s", networkURL, firewall.Network)
}
if len(firewall.Allowed) != 1 {
t.Errorf("Firewall.Allowed len unexpected, want=1, got=%d", len(firewall.Allowed))
}
expectedAllowed := &compute.FirewallAllowed{
Ports: []string{"8080"},
IPProtocol: "tcp",
}
allowed := firewall.Allowed[0]
if diff := cmp.Diff(expectedAllowed, allowed); diff != "" {
t.Errorf("Firewall.Allowed invalid, (-want +got):\n%s", diff)
}
}

func TestEnsureIPv6Firewall4Nodes(t *testing.T) {
t.Parallel()

fakeGCE := getFakeGCECloud(gce.DefaultTestClusterValues())
// create a test VM so that target tags can be found
createVMInstanceWithTag(t, fakeGCE, "test-node-1", "test-node-1")
nodeNames := []string{"test-node-1"}

svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)

networkURL := "testNet"
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
Network: network.NetworkInfo{NetworkURL: networkURL, SubnetworkURL: "testSubnet"},
}
l4 := NewL4Handler(l4ilbParams)
syncResult := &L4ILBSyncResult{
Annotations: make(map[string]string),
}

l4.ensureIPv6NodesFirewall("2001:db8::ff00:42:8329", nodeNames, syncResult)
if syncResult.Error != nil {
t.Fatalf("ensureIPv6NodesFirewall() error %+v", syncResult)
}
firewallName := l4.namer.L4IPv6Firewall(l4.Service.Namespace, l4.Service.Name)
firewall, err := fakeGCE.GetFirewall(firewallName)
if err != nil {
t.Fatalf("GetFirewall error %v", err)
}
if firewall.Name != firewallName {
t.Errorf("Firewall.Name invalid, want=%s, got=%s", firewallName, firewall.Name)
}
if firewall.Network != networkURL {
t.Errorf("Firewall.Network invalid, want=%s, got=%s", networkURL, firewall.Network)
}
if len(firewall.Allowed) != 1 {
t.Errorf("Firewall.Allowed len unexpected, want=1, got=%d", len(firewall.Allowed))
}
expectedAllowed := &compute.FirewallAllowed{
Ports: []string{"8080"},
IPProtocol: "tcp",
}
allowed := firewall.Allowed[0]
if diff := cmp.Diff(expectedAllowed, allowed); diff != "" {
t.Errorf("Firewall.Allowed invalid, (-want +got):\n%s", diff)
}
}

// This is exhaustive test that checks for all possible transitions of
// - ServiceExternalTrafficPolicy
// - Protocol
Expand Down Expand Up @@ -2380,3 +2484,18 @@ func verifyILBIPv6ResourcesDeletedOnSync(l4 *L4) error {

return nil
}

func createVMInstanceWithTag(t *testing.T, fakeGCE *gce.Cloud, name, tag string) {
err := fakeGCE.Compute().Instances().Insert(context.Background(),
meta.ZonalKey(tag, "us-central1-b"),
&compute.Instance{
Name: name,
Zone: "us-central1-b",
Tags: &compute.Tags{
Items: []string{tag},
},
})
if err != nil {
t.Errorf("failed to create instance err=%v", err)
}
}
1 change: 1 addition & 0 deletions pkg/loadbalancers/l4ipv6.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ func (l4 *L4) ensureIPv6NodesFirewall(ipAddress string, nodeNames []string, resu
Name: firewallName,
NodeNames: nodeNames,
L4Type: utils.ILB,
Network: l4.network,
}

err = firewalls.EnsureL4LBFirewallForNodes(l4.Service, &ipv6nodesFWRParams, l4.cloud, l4.recorder)
Expand Down
2 changes: 2 additions & 0 deletions pkg/loadbalancers/l4netlb.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ func (l4netlb *L4NetLB) ensureIPv4NodesFirewall(nodeNames []string, ipAddress st
}

// Add firewall rule for L4 External LoadBalancer traffic to nodes
defaultNetwork := network.DefaultNetwork(l4netlb.cloud)
nodesFWRParams := firewalls.FirewallParams{
PortRanges: portRanges,
SourceRanges: sourceRanges,
Expand All @@ -274,6 +275,7 @@ func (l4netlb *L4NetLB) ensureIPv4NodesFirewall(nodeNames []string, ipAddress st
Name: firewallName,
IP: l4netlb.Service.Spec.LoadBalancerIP,
NodeNames: nodeNames,
Network: *defaultNetwork,
}
result.Error = firewalls.EnsureL4LBFirewallForNodes(l4netlb.Service, &nodesFWRParams, l4netlb.cloud, l4netlb.recorder)
if result.Error != nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/loadbalancers/l4netlbipv6.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package loadbalancers

import (
"k8s.io/ingress-gce/pkg/network"
"strings"
"time"

Expand Down Expand Up @@ -122,6 +123,7 @@ func (l4netlb *L4NetLB) ensureIPv6NodesFirewall(ipAddress string, nodeNames []st
return
}

defaultNetwork := network.DefaultNetwork(l4netlb.cloud)
ipv6nodesFWRParams := firewalls.FirewallParams{
PortRanges: portRanges,
SourceRanges: ipv6SourceRanges,
Expand All @@ -130,6 +132,7 @@ func (l4netlb *L4NetLB) ensureIPv6NodesFirewall(ipAddress string, nodeNames []st
Name: firewallName,
NodeNames: nodeNames,
L4Type: utils.XLB,
Network: *defaultNetwork,
}

err = firewalls.EnsureL4LBFirewallForNodes(l4netlb.Service, &ipv6nodesFWRParams, l4netlb.cloud, l4netlb.recorder)
Expand Down

0 comments on commit 06386ad

Please sign in to comment.