Skip to content

Commit

Permalink
Merge pull request #1674 from sugangli/pinhole-fw
Browse files Browse the repository at this point in the history
Fix firewall pinhole
  • Loading branch information
k8s-ci-robot authored Jun 28, 2022
2 parents ed766c5 + 6fd2dbe commit d5436d7
Show file tree
Hide file tree
Showing 12 changed files with 299 additions and 68 deletions.
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ module k8s.io/ingress-gce
go 1.16

require (
github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.0
github.com/go-openapi/jsonpointer v0.19.5 // indirect
github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.1-0.20220218231025-f11817397a1b
github.com/go-openapi/spec v0.19.3
github.com/google/go-cmp v0.5.6
github.com/imdario/mergo v0.3.7 // indirect
github.com/kr/pretty v0.2.0
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym
github.com/GoogleCloudPlatform/k8s-cloud-provider v0.0.0-20200415212048-7901bc822317/go.mod h1:DF8FZRxMHMGv/vP2lQP6h+dYzzjpuRn24VeRiYn3qjQ=
github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.0 h1:bmLVE4VV8Zcx5iRDkCVsWhP3DOQHfnkUs8jGp3kOJKc=
github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.0/go.mod h1:FNj4KYEAAHfYu68kRYolGoxkaJn+6mdEsaM12VTwuI0=
github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.1-0.20220218231025-f11817397a1b h1:Heo1J/ttaQFgGJSVnCZquy3e5eH5j1nqxBuomztB3P0=
github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.1-0.20220218231025-f11817397a1b/go.mod h1:FNj4KYEAAHfYu68kRYolGoxkaJn+6mdEsaM12VTwuI0=
github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ=
github.com/OneOfOne/xxhash v1.2.2 h1:KMrpdQIwFcEqXDklaen+P1axHaj9BSKzvpUUfnHldSE=
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
Expand Down Expand Up @@ -979,6 +981,8 @@ k8s.io/client-go v0.22.2 h1:DaSQgs02aCC1QcwUdkKZWOeaVsQjYvWv8ZazcZ6JcHc=
k8s.io/client-go v0.22.2/go.mod h1:sAlhrkVDf50ZHx6z4K0S40wISNTarf1r800F+RlCF6U=
k8s.io/cloud-provider v0.20.0 h1:CVPQ66iyfNgeGomUq2jE/TWrfzE77bdCpemhFS8955U=
k8s.io/cloud-provider v0.20.0/go.mod h1:Lz/luSVD5BrHDDhtVdjFh0C2qQCRYdf0b9BHQ9L+bXc=
k8s.io/cloud-provider-gcp v0.21.0 h1:dZUWxMltT/PLYmvmA00O5M4ug+8bDFmA054aVa9tna0=
k8s.io/cloud-provider-gcp/providers v0.21.1 h1:eUgGWakdxS7+yNrldf5zcY+3SfFRiVltnV/mBof/fsw=
k8s.io/code-generator v0.20.0/go.mod h1:UsqdF+VX4PU2g46NC2JRs4gc+IfrctnwHb76RNbWHJg=
k8s.io/component-base v0.20.0 h1:BXGL8iitIQD+0NgW49UsM7MraNUUGDU3FBmrfUAtmVQ=
k8s.io/component-base v0.20.0/go.mod h1:wKPj+RHnAr8LW2EIBIK7AxOHPde4gme2lzXwVSoRXeA=
Expand Down
69 changes: 69 additions & 0 deletions pkg/firewalls/firewalladapter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
Copyright 2022 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 firewalls

import (
compute "google.golang.org/api/compute/v1"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"k8s.io/ingress-gce/pkg/composite/metrics"
"k8s.io/legacy-cloud-providers/gce"
)

// firewallAdapter is a temporary shim to consolidate accesses to
// Cloud and push them outside of this package.
type firewallAdapter struct {
gc *gce.Cloud
}

// NewFirewallAdapter takes a Cloud and construct a firewallAdapter
func NewFirewallAdapter(g *gce.Cloud) *firewallAdapter {
return &firewallAdapter{
gc: g,
}
}

// GetFirewall returns the Firewall by name.
func (fa *firewallAdapter) GetFirewall(name string) (*compute.Firewall, error) {
ctx, cancel := cloud.ContextWithCallTimeout()
defer cancel()
mc := metrics.NewMetricContext("firewall", "get", "<n/a>", "<n/a>", "<n/a>")
v, err := fa.gc.Compute().Firewalls().Get(ctx, meta.GlobalKey(name))
return v, mc.Observe(err)
}

// CreateFirewall creates the passed firewall
func (fa *firewallAdapter) CreateFirewall(f *compute.Firewall) error {
ctx, cancel := cloud.ContextWithCallTimeout()
defer cancel()
mc := metrics.NewMetricContext("firewall", "create", "<n/a>", "<n/a>", "<n/a>")
return mc.Observe(fa.gc.Compute().Firewalls().Insert(ctx, meta.GlobalKey(f.Name), f))
}

// DeleteFirewall deletes the given firewall rule.
func (fa *firewallAdapter) DeleteFirewall(name string) error {
ctx, cancel := cloud.ContextWithCallTimeout()
defer cancel()
mc := metrics.NewMetricContext("firewall", "delete", "<n/a>", "<n/a>", "<n/a>")
return mc.Observe(fa.gc.Compute().Firewalls().Delete(ctx, meta.GlobalKey(name)))
}

// PatchFirewall applies the given firewall as a patch to an existing service.
func (fa *firewallAdapter) PatchFirewall(f *compute.Firewall) error {
ctx, cancel := cloud.ContextWithCallTimeout()
defer cancel()
mc := metrics.NewMetricContext("firewall", "patch", "<n/a>", "<n/a>", "<n/a>")
return mc.Observe(fa.gc.Compute().Firewalls().Patch(ctx, meta.GlobalKey(f.Name), f))
}
34 changes: 21 additions & 13 deletions pkg/firewalls/firewalls_l4.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,27 @@ import (
"google.golang.org/api/compute/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/client-go/tools/record"
"k8s.io/ingress-gce/pkg/flags"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/klog"
"k8s.io/legacy-cloud-providers/gce"
)

// FirewallParams holds all data needed to create firewall for L4 LB
type FirewallParams struct {
Name string
IP string
SourceRanges []string
PortRanges []string
NodeNames []string
Protocol string
L4Type utils.L4LBType
Name string
IP string
SourceRanges []string
DestinationRanges []string
PortRanges []string
NodeNames []string
Protocol string
L4Type utils.L4LBType
}

func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParams, sharedRule bool) error {
existingFw, err := cloud.GetFirewall(params.Name)
fa := NewFirewallAdapter(cloud)
existingFw, err := fa.GetFirewall(params.Name)
if err != nil && !utils.IsNotFoundError(err) {
return err
}
Expand All @@ -66,9 +69,12 @@ func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParam
},
},
}
if flags.F.EnablePinhole {
expectedFw.DestinationRanges = params.DestinationRanges
}
if existingFw == nil {
klog.V(2).Infof("EnsureL4FirewallRule(%v): creating L4 %s firewall rule", params.Name, params.L4Type.ToString())
err = cloud.CreateFirewall(expectedFw)
err = fa.CreateFirewall(expectedFw)
if utils.IsForbiddenError(err) && cloud.OnXPN() {
gcloudCmd := gce.FirewallToGCloudCreateCmd(expectedFw, cloud.NetworkProjectID())

Expand All @@ -82,18 +88,20 @@ func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParam
if firewallRuleEqual(expectedFw, existingFw, sharedRule) {
return nil
}
klog.V(2).Infof("EnsureL4FirewallRule(%v): updating L4 %s firewall", params.Name, params.L4Type.ToString())
err = cloud.UpdateFirewall(expectedFw)

klog.V(2).Infof("EnsureL4FirewallRule(%v): patching L4 %s firewall", params.Name, params.L4Type.ToString())
err = fa.PatchFirewall(expectedFw)
if utils.IsForbiddenError(err) && cloud.OnXPN() {
gcloudCmd := gce.FirewallToGCloudUpdateCmd(expectedFw, cloud.NetworkProjectID())
klog.V(3).Infof("EnsureL4FirewallRule(%v): Could not update L4 %s firewall on XPN cluster: %v. Raising event for cmd: %q", params.Name, params.L4Type.ToString(), err, gcloudCmd)
klog.V(3).Infof("EnsureL4FirewallRule(%v): Could not patch L4 %s firewall on XPN cluster: %v. Raising event for cmd: %q", params.Name, params.L4Type.ToString(), err, gcloudCmd)
return newFirewallXPNError(err, gcloudCmd)
}
return err
}

func EnsureL4FirewallRuleDeleted(cloud *gce.Cloud, fwName string) error {
if err := utils.IgnoreHTTPNotFound(cloud.DeleteFirewall(fwName)); err != nil {
fa := NewFirewallAdapter(cloud)
if err := utils.IgnoreHTTPNotFound(fa.DeleteFirewall(fwName)); err != nil {
if utils.IsForbiddenError(err) && cloud.OnXPN() {
gcloudCmd := gce.FirewallToGCloudDeleteCmd(fwName, cloud.NetworkProjectID())
klog.V(3).Infof("EnsureL4FirewallRuleDeleted(%v): could not delete traffic firewall on XPN cluster. Raising event.", fwName)
Expand Down
2 changes: 2 additions & 0 deletions pkg/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ var (
EnableTrafficScaling bool
EnableEndpointSlices bool
EnableMultipleIgs bool
EnablePinhole bool
MaxIgSize int
}{
GCERateLimitScale: 1.0,
Expand Down Expand Up @@ -247,6 +248,7 @@ L7 load balancing. CSV values accepted. Example: -node-port-ranges=80,8080,400-5
flag.StringVar(&F.GKEClusterType, "gke-cluster-type", "ZONAL", "The cluster type of the GKE cluster this Ingress Controller will be interacting with")
flag.BoolVar(&F.EnableTrafficScaling, "enable-traffic-scaling", false, "Enable support for Service {max-rate-per-endpoint, capacity-scaler}")
flag.BoolVar(&F.EnableEndpointSlices, "enable-endpoint-slices", false, "Enable using Endpoint Slices API instead of Endpoints API")
flag.BoolVar(&F.EnablePinhole, "enable-pinhole", false, "Enable Pinhole firewall feature")
flag.BoolVar(&F.EnableMultipleIgs, "enable-multiple-igs", false, "Enable using unmanaged instance group management")
flag.IntVar(&F.MaxIgSize, "max-ig-size", 1000, "Max number of instances in Instance Group")
flag.DurationVar(&F.MetricsExportInterval, "metrics-export-interval", 10*time.Minute, `Period for calculating and exporting metrics related to state of managed objects.`)
Expand Down
2 changes: 1 addition & 1 deletion pkg/l4lb/l4netlbcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ func TestProcessServiceUpdate(t *testing.T) {
},
} {
svc, l4netController := createAndSyncNetLBSvc(t)
(l4netController.ctx.Cloud.Compute().(*cloud.MockGCE)).MockFirewalls.UpdateHook = mock.UpdateFirewallHook
(l4netController.ctx.Cloud.Compute().(*cloud.MockGCE)).MockFirewalls.PatchHook = mock.UpdateFirewallHook
(l4netController.ctx.Cloud.Compute().(*cloud.MockGCE)).MockRegionBackendServices.UpdateHook = mock.UpdateRegionBackendServiceHook
newSvc, err := l4netController.ctx.KubeClient.CoreV1().Services(svc.Namespace).Get(context.TODO(), svc.Name, metav1.GetOptions{})
if err != nil {
Expand Down
48 changes: 25 additions & 23 deletions pkg/loadbalancers/l4.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,29 +215,6 @@ func (l *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service)

_, portRanges, _, protocol := utils.GetPortsAndProtocol(l.Service.Spec.Ports)

// ensure firewalls
sourceRanges, err := helpers.GetLoadBalancerSourceRanges(l.Service)
if err != nil {
result.Error = err
return result
}
// Add firewall rule for ILB traffic to nodes
nodesFWRParams := firewalls.FirewallParams{
PortRanges: portRanges,
SourceRanges: sourceRanges.StringSlice(),
Protocol: string(protocol),
Name: name,
NodeNames: nodeNames,
}

if err := firewalls.EnsureL4LBFirewallForNodes(l.Service, &nodesFWRParams, l.cloud, l.recorder); err != nil {
result.GCEResourceInError = annotations.FirewallRuleResource
result.Error = err
return result
}
result.Annotations[annotations.FirewallRuleKey] = name
result.Annotations[annotations.FirewallRuleForHealthcheckKey] = hcResult.HCFirewallRuleName

// Check if protocol has changed for this service. In this case, forwarding rule should be deleted before
// the backend service can be updated.
existingBS, err := l.backendPool.Get(name, meta.VersionGA, l.scope)
Expand Down Expand Up @@ -277,6 +254,31 @@ func (l *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service)
result.Annotations[annotations.UDPForwardingRuleKey] = frName
}

// ensure firewalls
sourceRanges, err := helpers.GetLoadBalancerSourceRanges(l.Service)
if err != nil {
result.Error = err
return result
}
// Add firewall rule for ILB traffic to nodes
nodesFWRParams := firewalls.FirewallParams{
PortRanges: portRanges,
SourceRanges: sourceRanges.StringSlice(),
DestinationRanges: []string{fr.IPAddress},
Protocol: string(protocol),
Name: name,
NodeNames: nodeNames,
L4Type: utils.ILB,
}

if err := firewalls.EnsureL4LBFirewallForNodes(l.Service, &nodesFWRParams, l.cloud, l.recorder); err != nil {
result.GCEResourceInError = annotations.FirewallRuleResource
result.Error = err
return result
}
result.Annotations[annotations.FirewallRuleKey] = name
result.Annotations[annotations.FirewallRuleForHealthcheckKey] = hcResult.HCFirewallRuleName

result.MetricsState.InSuccess = true
if options.AllowGlobalAccess {
result.MetricsState.EnabledGlobalAccess = true
Expand Down
22 changes: 12 additions & 10 deletions pkg/loadbalancers/l4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ limitations under the License.
import (
"context"
"fmt"
"k8s.io/ingress-gce/pkg/healthchecks"
"reflect"
"strings"
"testing"

"k8s.io/ingress-gce/pkg/healthchecks"

"google.golang.org/api/compute/v1"
"k8s.io/ingress-gce/pkg/backends"
"k8s.io/ingress-gce/pkg/firewalls"
Expand Down Expand Up @@ -57,7 +58,7 @@ func getFakeGCECloud(vals gce.TestClusterValues) *gce.Cloud {

(fakeGCE.Compute().(*cloud.MockGCE)).MockRegionBackendServices.UpdateHook = mock.UpdateRegionBackendServiceHook
(fakeGCE.Compute().(*cloud.MockGCE)).MockHealthChecks.UpdateHook = mock.UpdateHealthCheckHook
(fakeGCE.Compute().(*cloud.MockGCE)).MockFirewalls.UpdateHook = mock.UpdateFirewallHook
(fakeGCE.Compute().(*cloud.MockGCE)).MockFirewalls.PatchHook = mock.UpdateFirewallHook
return fakeGCE
}

Expand Down Expand Up @@ -1049,23 +1050,24 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) {
}
c := fakeGCE.Compute().(*cloud.MockGCE)
c.MockFirewalls.InsertHook = nil
c.MockFirewalls.UpdateHook = nil
c.MockFirewalls.PatchHook = nil

nodeNames := []string{"test-node-1"}
_, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName)
if err != nil {
t.Errorf("Unexpected error when adding nodes %v", err)
}
c.MockFirewalls.InsertHook = nil
c.MockFirewalls.UpdateHook = nil
c.MockFirewalls.PatchHook = nil

fwrParams := firewalls.FirewallParams{
Name: fwName,
SourceRanges: []string{"10.0.0.0/20"},
PortRanges: utils.GetPortRanges(tc.Input),
NodeNames: nodeNames,
Protocol: string(v1.ProtocolTCP),
IP: "1.2.3.4",
Name: fwName,
SourceRanges: []string{"10.0.0.0/20"},
DestinationRanges: []string{"20.0.0.0/20"},
PortRanges: utils.GetPortRanges(tc.Input),
NodeNames: nodeNames,
Protocol: string(v1.ProtocolTCP),
IP: "1.2.3.4",
}
firewalls.EnsureL4FirewallRule(l.cloud, utils.ServiceKeyFunc(svc.Namespace, svc.Name), &fwrParams /*sharedRule = */, false)
if err != nil {
Expand Down
45 changes: 27 additions & 18 deletions pkg/loadbalancers/l4netlb.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,9 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service)
result.Annotations[annotations.HealthcheckKey] = hcResult.HCName

name := l4netlb.ServicePort.BackendName()
protocol, res := l4netlb.createFirewalls(name, nodeNames)
if res.Error != nil {
return res
}
result.Annotations[annotations.FirewallRuleKey] = name
result.Annotations[annotations.FirewallRuleForHealthcheckKey] = hcResult.HCFirewallRuleName
_, portRanges, _, protocol := utils.GetPortsAndProtocol(l4netlb.Service.Spec.Ports)

bs, err := l4netlb.backendPool.EnsureL4BackendService(name, hcResult.HCLink, protocol, string(l4netlb.Service.Spec.SessionAffinity), string(cloud.SchemeExternal), l4netlb.NamespacedName, meta.VersionGA)
bs, err := l4netlb.backendPool.EnsureL4BackendService(name, hcResult.HCLink, string(protocol), string(l4netlb.Service.Spec.SessionAffinity), string(cloud.SchemeExternal), l4netlb.NamespacedName, meta.VersionGA)
if err != nil {
result.GCEResourceInError = annotations.BackendServiceResource
result.Error = fmt.Errorf("Failed to ensure backend service %s - %w", name, err)
Expand All @@ -161,6 +156,20 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service)
result.Status = &corev1.LoadBalancerStatus{Ingress: []corev1.LoadBalancerIngress{{IP: fr.IPAddress}}}
result.MetricsState.IsPremiumTier = fr.NetworkTier == cloud.NetworkTierPremium.ToGCEValue()
result.MetricsState.IsManagedIP = ipAddrType == IPAddrManaged
if fr.NetworkTier == cloud.NetworkTierPremium.ToGCEValue() {
result.MetricsState.IsPremiumTier = true
}
if ipAddrType == IPAddrManaged {
result.MetricsState.IsManagedIP = true
}

res := l4netlb.createFirewalls(name, nodeNames, fr.IPAddress, portRanges, string(protocol))
if res.Error != nil {
return res
}
result.Annotations[annotations.FirewallRuleKey] = name
result.Annotations[annotations.FirewallRuleForHealthcheckKey] = hcResult.HCFirewallRuleName

return result
}

Expand Down Expand Up @@ -239,29 +248,29 @@ func (l4netlb *L4NetLB) GetFRName() string {
return utils.LegacyForwardingRuleName(l4netlb.Service)
}

func (l4netlb *L4NetLB) createFirewalls(name string, nodeNames []string) (string, *L4NetLBSyncResult) {
_, portRanges, _, protocol := utils.GetPortsAndProtocol(l4netlb.Service.Spec.Ports)
func (l4netlb *L4NetLB) createFirewalls(name string, nodeNames []string, ipAddress string, portRanges []string, protocol string) *L4NetLBSyncResult {
result := &L4NetLBSyncResult{}
sourceRanges, err := helpers.GetLoadBalancerSourceRanges(l4netlb.Service)
if err != nil {
result.Error = err
return "", result
return result
}

// Add firewall rule for L4 External LoadBalancer traffic to nodes
nodesFWRParams := firewalls.FirewallParams{
PortRanges: portRanges,
SourceRanges: sourceRanges.StringSlice(),
Protocol: string(protocol),
Name: name,
IP: l4netlb.Service.Spec.LoadBalancerIP,
NodeNames: nodeNames,
PortRanges: portRanges,
SourceRanges: sourceRanges.StringSlice(),
DestinationRanges: []string{ipAddress},
Protocol: string(protocol),
Name: name,
IP: l4netlb.Service.Spec.LoadBalancerIP,
NodeNames: nodeNames,
}
result.Error = firewalls.EnsureL4LBFirewallForNodes(l4netlb.Service, &nodesFWRParams, l4netlb.cloud, l4netlb.recorder)
if result.Error != nil {
result.GCEResourceInError = annotations.FirewallRuleResource
result.Error = err
return "", result
return result
}
return string(protocol), result
return result
}
Loading

0 comments on commit d5436d7

Please sign in to comment.