From 690a2a05feb61375a213dcb8436a91d78017a450 Mon Sep 17 00:00:00 2001 From: Sugang Li Date: Fri, 11 Feb 2022 22:35:45 +0000 Subject: [PATCH 1/5] Fix firewall pinhole in ILB --- go.mod | 3 ++- pkg/firewalls/firewalls_l4.go | 23 ++++++++++------- pkg/loadbalancers/l4.go | 48 ++++++++++++++++++----------------- pkg/loadbalancers/l4_test.go | 16 +++++++----- vendor/modules.txt | 2 ++ 5 files changed, 52 insertions(+), 40 deletions(-) diff --git a/go.mod b/go.mod index a9f81ff9e5..c4418fd1f3 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,9 @@ module k8s.io/ingress-gce go 1.16 require ( - github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.0 + github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.1-0.20220218231025-f11817397a1b github.com/go-openapi/jsonpointer v0.19.5 // indirect + github.com/go-openapi/spec v0.19.3 // indirect github.com/google/go-cmp v0.5.6 github.com/imdario/mergo v0.3.7 // indirect github.com/kr/pretty v0.2.0 diff --git a/pkg/firewalls/firewalls_l4.go b/pkg/firewalls/firewalls_l4.go index b336aefc36..c49889a00c 100644 --- a/pkg/firewalls/firewalls_l4.go +++ b/pkg/firewalls/firewalls_l4.go @@ -23,6 +23,7 @@ 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" @@ -30,13 +31,14 @@ import ( // 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 { @@ -66,6 +68,9 @@ 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) @@ -82,11 +87,11 @@ 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()) + klog.V(2).Infof("EnsureL4FirewallRule(%v): patching L4 %s firewall", params.Name, params.L4Type.ToString()) err = cloud.UpdateFirewall(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 diff --git a/pkg/loadbalancers/l4.go b/pkg/loadbalancers/l4.go index 8b346bcef4..bcce040449 100644 --- a/pkg/loadbalancers/l4.go +++ b/pkg/loadbalancers/l4.go @@ -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) @@ -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 diff --git a/pkg/loadbalancers/l4_test.go b/pkg/loadbalancers/l4_test.go index efb19b6c1e..3c75776646 100644 --- a/pkg/loadbalancers/l4_test.go +++ b/pkg/loadbalancers/l4_test.go @@ -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" @@ -1060,12 +1061,13 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) { c.MockFirewalls.UpdateHook = 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 { diff --git a/vendor/modules.txt b/vendor/modules.txt index cc5fb349b6..353b045175 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -87,6 +87,8 @@ github.com/go-logr/logr github.com/go-openapi/jsonpointer # github.com/go-openapi/jsonreference v0.19.3 github.com/go-openapi/jsonreference +# github.com/go-openapi/spec v0.19.3 +## explicit # github.com/go-openapi/swag v0.19.5 github.com/go-openapi/swag # github.com/gogo/protobuf v1.3.2 From fa7128e83dcf2aa8d287f4b299baef5da6c0d75d Mon Sep 17 00:00:00 2001 From: Sugang Li Date: Wed, 2 Mar 2022 23:52:45 +0000 Subject: [PATCH 2/5] update k8s-cloud-provider to include the new patch operation --- go.mod | 4 +- go.sum | 4 + .../k8s-cloud-provider/pkg/cloud/gen.go | 131 +++++++++++++++++- .../k8s-cloud-provider/pkg/cloud/meta/meta.go | 3 + vendor/modules.txt | 2 +- 5 files changed, 140 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index c4418fd1f3..f86c287a95 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,9 @@ module k8s.io/ingress-gce go 1.16 require ( - github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.1-0.20220218231025-f11817397a1b github.com/go-openapi/jsonpointer v0.19.5 // indirect - github.com/go-openapi/spec v0.19.3 // 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 diff --git a/go.sum b/go.sum index bfbaf4d37c..c42f309f17 100644 --- a/go.sum +++ b/go.sum @@ -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= @@ -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= diff --git a/vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/gen.go b/vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/gen.go index 89cea72660..edf4a1ae52 100644 --- a/vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/gen.go +++ b/vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/gen.go @@ -1,5 +1,5 @@ /* -Copyright 2021 Google LLC +Copyright 2022 Google LLC Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -8985,6 +8985,7 @@ type AlphaFirewalls interface { List(ctx context.Context, fl *filter.F) ([]*alpha.Firewall, error) Insert(ctx context.Context, key *meta.Key, obj *alpha.Firewall) error Delete(ctx context.Context, key *meta.Key) error + Patch(context.Context, *meta.Key, *alpha.Firewall) error Update(context.Context, *meta.Key, *alpha.Firewall) error } @@ -9025,6 +9026,7 @@ type MockAlphaFirewalls struct { ListHook func(ctx context.Context, fl *filter.F, m *MockAlphaFirewalls) (bool, []*alpha.Firewall, error) InsertHook func(ctx context.Context, key *meta.Key, obj *alpha.Firewall, m *MockAlphaFirewalls) (bool, error) DeleteHook func(ctx context.Context, key *meta.Key, m *MockAlphaFirewalls) (bool, error) + PatchHook func(context.Context, *meta.Key, *alpha.Firewall, *MockAlphaFirewalls) error UpdateHook func(context.Context, *meta.Key, *alpha.Firewall, *MockAlphaFirewalls) error // X is extra state that can be used as part of the mock. Generated code @@ -9171,6 +9173,14 @@ func (m *MockAlphaFirewalls) Obj(o *alpha.Firewall) *MockFirewallsObj { return &MockFirewallsObj{o} } +// Patch is a mock for the corresponding method. +func (m *MockAlphaFirewalls) Patch(ctx context.Context, key *meta.Key, arg0 *alpha.Firewall) error { + if m.PatchHook != nil { + return m.PatchHook(ctx, key, arg0, m) + } + return nil +} + // Update is a mock for the corresponding method. func (m *MockAlphaFirewalls) Update(ctx context.Context, key *meta.Key, arg0 *alpha.Firewall) error { if m.UpdateHook != nil { @@ -9321,6 +9331,39 @@ func (g *GCEAlphaFirewalls) Delete(ctx context.Context, key *meta.Key) error { return err } +// Patch is a method on GCEAlphaFirewalls. +func (g *GCEAlphaFirewalls) Patch(ctx context.Context, key *meta.Key, arg0 *alpha.Firewall) error { + klog.V(5).Infof("GCEAlphaFirewalls.Patch(%v, %v, ...): called", ctx, key) + + if !key.Valid() { + klog.V(2).Infof("GCEAlphaFirewalls.Patch(%v, %v, ...): key is invalid (%#v)", ctx, key, key) + return fmt.Errorf("invalid GCE key (%+v)", key) + } + projectID := g.s.ProjectRouter.ProjectID(ctx, "alpha", "Firewalls") + rk := &RateLimitKey{ + ProjectID: projectID, + Operation: "Patch", + Version: meta.Version("alpha"), + Service: "Firewalls", + } + klog.V(5).Infof("GCEAlphaFirewalls.Patch(%v, %v, ...): projectID = %v, rk = %+v", ctx, key, projectID, rk) + + if err := g.s.RateLimiter.Accept(ctx, rk); err != nil { + klog.V(4).Infof("GCEAlphaFirewalls.Patch(%v, %v, ...): RateLimiter error: %v", ctx, key, err) + return err + } + call := g.s.Alpha.Firewalls.Patch(projectID, key.Name, arg0) + call.Context(ctx) + op, err := call.Do() + if err != nil { + klog.V(4).Infof("GCEAlphaFirewalls.Patch(%v, %v, ...) = %+v", ctx, key, err) + return err + } + err = g.s.WaitForCompletion(ctx, op) + klog.V(4).Infof("GCEAlphaFirewalls.Patch(%v, %v, ...) = %+v", ctx, key, err) + return err +} + // Update is a method on GCEAlphaFirewalls. func (g *GCEAlphaFirewalls) Update(ctx context.Context, key *meta.Key, arg0 *alpha.Firewall) error { klog.V(5).Infof("GCEAlphaFirewalls.Update(%v, %v, ...): called", ctx, key) @@ -9360,6 +9403,7 @@ type BetaFirewalls interface { List(ctx context.Context, fl *filter.F) ([]*beta.Firewall, error) Insert(ctx context.Context, key *meta.Key, obj *beta.Firewall) error Delete(ctx context.Context, key *meta.Key) error + Patch(context.Context, *meta.Key, *beta.Firewall) error Update(context.Context, *meta.Key, *beta.Firewall) error } @@ -9400,6 +9444,7 @@ type MockBetaFirewalls struct { ListHook func(ctx context.Context, fl *filter.F, m *MockBetaFirewalls) (bool, []*beta.Firewall, error) InsertHook func(ctx context.Context, key *meta.Key, obj *beta.Firewall, m *MockBetaFirewalls) (bool, error) DeleteHook func(ctx context.Context, key *meta.Key, m *MockBetaFirewalls) (bool, error) + PatchHook func(context.Context, *meta.Key, *beta.Firewall, *MockBetaFirewalls) error UpdateHook func(context.Context, *meta.Key, *beta.Firewall, *MockBetaFirewalls) error // X is extra state that can be used as part of the mock. Generated code @@ -9546,6 +9591,14 @@ func (m *MockBetaFirewalls) Obj(o *beta.Firewall) *MockFirewallsObj { return &MockFirewallsObj{o} } +// Patch is a mock for the corresponding method. +func (m *MockBetaFirewalls) Patch(ctx context.Context, key *meta.Key, arg0 *beta.Firewall) error { + if m.PatchHook != nil { + return m.PatchHook(ctx, key, arg0, m) + } + return nil +} + // Update is a mock for the corresponding method. func (m *MockBetaFirewalls) Update(ctx context.Context, key *meta.Key, arg0 *beta.Firewall) error { if m.UpdateHook != nil { @@ -9696,6 +9749,39 @@ func (g *GCEBetaFirewalls) Delete(ctx context.Context, key *meta.Key) error { return err } +// Patch is a method on GCEBetaFirewalls. +func (g *GCEBetaFirewalls) Patch(ctx context.Context, key *meta.Key, arg0 *beta.Firewall) error { + klog.V(5).Infof("GCEBetaFirewalls.Patch(%v, %v, ...): called", ctx, key) + + if !key.Valid() { + klog.V(2).Infof("GCEBetaFirewalls.Patch(%v, %v, ...): key is invalid (%#v)", ctx, key, key) + return fmt.Errorf("invalid GCE key (%+v)", key) + } + projectID := g.s.ProjectRouter.ProjectID(ctx, "beta", "Firewalls") + rk := &RateLimitKey{ + ProjectID: projectID, + Operation: "Patch", + Version: meta.Version("beta"), + Service: "Firewalls", + } + klog.V(5).Infof("GCEBetaFirewalls.Patch(%v, %v, ...): projectID = %v, rk = %+v", ctx, key, projectID, rk) + + if err := g.s.RateLimiter.Accept(ctx, rk); err != nil { + klog.V(4).Infof("GCEBetaFirewalls.Patch(%v, %v, ...): RateLimiter error: %v", ctx, key, err) + return err + } + call := g.s.Beta.Firewalls.Patch(projectID, key.Name, arg0) + call.Context(ctx) + op, err := call.Do() + if err != nil { + klog.V(4).Infof("GCEBetaFirewalls.Patch(%v, %v, ...) = %+v", ctx, key, err) + return err + } + err = g.s.WaitForCompletion(ctx, op) + klog.V(4).Infof("GCEBetaFirewalls.Patch(%v, %v, ...) = %+v", ctx, key, err) + return err +} + // Update is a method on GCEBetaFirewalls. func (g *GCEBetaFirewalls) Update(ctx context.Context, key *meta.Key, arg0 *beta.Firewall) error { klog.V(5).Infof("GCEBetaFirewalls.Update(%v, %v, ...): called", ctx, key) @@ -9735,6 +9821,7 @@ type Firewalls interface { List(ctx context.Context, fl *filter.F) ([]*ga.Firewall, error) Insert(ctx context.Context, key *meta.Key, obj *ga.Firewall) error Delete(ctx context.Context, key *meta.Key) error + Patch(context.Context, *meta.Key, *ga.Firewall) error Update(context.Context, *meta.Key, *ga.Firewall) error } @@ -9775,6 +9862,7 @@ type MockFirewalls struct { ListHook func(ctx context.Context, fl *filter.F, m *MockFirewalls) (bool, []*ga.Firewall, error) InsertHook func(ctx context.Context, key *meta.Key, obj *ga.Firewall, m *MockFirewalls) (bool, error) DeleteHook func(ctx context.Context, key *meta.Key, m *MockFirewalls) (bool, error) + PatchHook func(context.Context, *meta.Key, *ga.Firewall, *MockFirewalls) error UpdateHook func(context.Context, *meta.Key, *ga.Firewall, *MockFirewalls) error // X is extra state that can be used as part of the mock. Generated code @@ -9921,6 +10009,14 @@ func (m *MockFirewalls) Obj(o *ga.Firewall) *MockFirewallsObj { return &MockFirewallsObj{o} } +// Patch is a mock for the corresponding method. +func (m *MockFirewalls) Patch(ctx context.Context, key *meta.Key, arg0 *ga.Firewall) error { + if m.PatchHook != nil { + return m.PatchHook(ctx, key, arg0, m) + } + return nil +} + // Update is a mock for the corresponding method. func (m *MockFirewalls) Update(ctx context.Context, key *meta.Key, arg0 *ga.Firewall) error { if m.UpdateHook != nil { @@ -10071,6 +10167,39 @@ func (g *GCEFirewalls) Delete(ctx context.Context, key *meta.Key) error { return err } +// Patch is a method on GCEFirewalls. +func (g *GCEFirewalls) Patch(ctx context.Context, key *meta.Key, arg0 *ga.Firewall) error { + klog.V(5).Infof("GCEFirewalls.Patch(%v, %v, ...): called", ctx, key) + + if !key.Valid() { + klog.V(2).Infof("GCEFirewalls.Patch(%v, %v, ...): key is invalid (%#v)", ctx, key, key) + return fmt.Errorf("invalid GCE key (%+v)", key) + } + projectID := g.s.ProjectRouter.ProjectID(ctx, "ga", "Firewalls") + rk := &RateLimitKey{ + ProjectID: projectID, + Operation: "Patch", + Version: meta.Version("ga"), + Service: "Firewalls", + } + klog.V(5).Infof("GCEFirewalls.Patch(%v, %v, ...): projectID = %v, rk = %+v", ctx, key, projectID, rk) + + if err := g.s.RateLimiter.Accept(ctx, rk); err != nil { + klog.V(4).Infof("GCEFirewalls.Patch(%v, %v, ...): RateLimiter error: %v", ctx, key, err) + return err + } + call := g.s.GA.Firewalls.Patch(projectID, key.Name, arg0) + call.Context(ctx) + op, err := call.Do() + if err != nil { + klog.V(4).Infof("GCEFirewalls.Patch(%v, %v, ...) = %+v", ctx, key, err) + return err + } + err = g.s.WaitForCompletion(ctx, op) + klog.V(4).Infof("GCEFirewalls.Patch(%v, %v, ...) = %+v", ctx, key, err) + return err +} + // Update is a method on GCEFirewalls. func (g *GCEFirewalls) Update(ctx context.Context, key *meta.Key, arg0 *ga.Firewall) error { klog.V(5).Infof("GCEFirewalls.Update(%v, %v, ...): called", ctx, key) diff --git a/vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta/meta.go b/vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta/meta.go index 5b6ef4ed21..2f816843f9 100644 --- a/vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta/meta.go +++ b/vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta/meta.go @@ -228,6 +228,7 @@ var AllServices = []*ServiceInfo{ serviceType: reflect.TypeOf(&alpha.FirewallsService{}), additionalMethods: []string{ "Update", + "Patch", }, }, { @@ -239,6 +240,7 @@ var AllServices = []*ServiceInfo{ serviceType: reflect.TypeOf(&beta.FirewallsService{}), additionalMethods: []string{ "Update", + "Patch", }, }, { @@ -249,6 +251,7 @@ var AllServices = []*ServiceInfo{ serviceType: reflect.TypeOf(&ga.FirewallsService{}), additionalMethods: []string{ "Update", + "Patch", }, }, { diff --git a/vendor/modules.txt b/vendor/modules.txt index 353b045175..71dabf2039 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1,6 +1,6 @@ # cloud.google.com/go v0.97.0 cloud.google.com/go/compute/metadata -# github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.0 +# github.com/GoogleCloudPlatform/k8s-cloud-provider v1.18.1-0.20220218231025-f11817397a1b ## explicit github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/filter From 4df3bcb6886fc22fc333ab0780b48634589757db Mon Sep 17 00:00:00 2001 From: Sugang Li Date: Wed, 2 Mar 2022 23:54:04 +0000 Subject: [PATCH 3/5] Add an adapter to push out the dependency of legacy-cloud-provider --- pkg/firewalls/firewalladapter.go | 69 ++++++++++++++++++++++++++++++ pkg/firewalls/firewalls_l4.go | 11 +++-- pkg/l4lb/l4netlbcontroller_test.go | 2 +- pkg/loadbalancers/l4_test.go | 6 +-- 4 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 pkg/firewalls/firewalladapter.go diff --git a/pkg/firewalls/firewalladapter.go b/pkg/firewalls/firewalladapter.go new file mode 100644 index 0000000000..e9d770697f --- /dev/null +++ b/pkg/firewalls/firewalladapter.go @@ -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", "", "", "") + 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", "", "", "") + 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", "", "", "") + 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", "", "", "") + return mc.Observe(fa.gc.Compute().Firewalls().Patch(ctx, meta.GlobalKey(f.Name), f)) +} diff --git a/pkg/firewalls/firewalls_l4.go b/pkg/firewalls/firewalls_l4.go index c49889a00c..d79bfcb574 100644 --- a/pkg/firewalls/firewalls_l4.go +++ b/pkg/firewalls/firewalls_l4.go @@ -42,7 +42,8 @@ type FirewallParams struct { } 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 } @@ -73,7 +74,7 @@ func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParam } 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()) @@ -87,8 +88,9 @@ func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParam if firewallRuleEqual(expectedFw, existingFw, sharedRule) { return nil } + klog.V(2).Infof("EnsureL4FirewallRule(%v): patching L4 %s firewall", params.Name, params.L4Type.ToString()) - err = cloud.UpdateFirewall(expectedFw) + err = fa.PatchFirewall(expectedFw) if utils.IsForbiddenError(err) && cloud.OnXPN() { gcloudCmd := gce.FirewallToGCloudUpdateCmd(expectedFw, cloud.NetworkProjectID()) 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) @@ -98,7 +100,8 @@ func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParam } 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) diff --git a/pkg/l4lb/l4netlbcontroller_test.go b/pkg/l4lb/l4netlbcontroller_test.go index 0758f9720f..b84c9d2dbb 100644 --- a/pkg/l4lb/l4netlbcontroller_test.go +++ b/pkg/l4lb/l4netlbcontroller_test.go @@ -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 { diff --git a/pkg/loadbalancers/l4_test.go b/pkg/loadbalancers/l4_test.go index 3c75776646..eb4abc3732 100644 --- a/pkg/loadbalancers/l4_test.go +++ b/pkg/loadbalancers/l4_test.go @@ -58,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 } @@ -1050,7 +1050,7 @@ 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) @@ -1058,7 +1058,7 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) { 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, From 88dd0b8c7b441b235c919c31d42cd90d6f4adcf2 Mon Sep 17 00:00:00 2001 From: Sugang Li Date: Thu, 31 Mar 2022 18:48:14 +0000 Subject: [PATCH 4/5] add netlb changes --- pkg/firewalls/firewalls_l4.go | 4 ++++ pkg/loadbalancers/l4netlb.go | 41 +++++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/pkg/firewalls/firewalls_l4.go b/pkg/firewalls/firewalls_l4.go index d79bfcb574..8c70a1a0ad 100644 --- a/pkg/firewalls/firewalls_l4.go +++ b/pkg/firewalls/firewalls_l4.go @@ -52,7 +52,11 @@ func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParam if err != nil { return err } +<<<<<<< HEAD fwDesc, err := utils.MakeL4LBFirewallDescription(nsName, params.IP, meta.VersionGA, sharedRule) +======= + fwDesc, err := utils.MakeL4LBServiceDescription(nsName, params.IP, meta.VersionGA, sharedRule, params.L4Type) +>>>>>>> 2a315c6d (add netlb changes) if err != nil { klog.Warningf("EnsureL4FirewallRule(%v): failed to generate description for L4 %s rule, err: %v", params.Name, params.L4Type.ToString(), err) } diff --git a/pkg/loadbalancers/l4netlb.go b/pkg/loadbalancers/l4netlb.go index f6e2ed6e28..9781e21f5f 100644 --- a/pkg/loadbalancers/l4netlb.go +++ b/pkg/loadbalancers/l4netlb.go @@ -131,12 +131,7 @@ 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) if err != nil { @@ -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 } @@ -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, 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 } From 6fd2dbe2ea6908f9c2c279b9a1a59331dcfe72e0 Mon Sep 17 00:00:00 2001 From: Sugang Li Date: Mon, 27 Jun 2022 18:14:37 +0000 Subject: [PATCH 5/5] add flag to protect pinhole feature --- pkg/firewalls/firewalls_l4.go | 4 ---- pkg/flags/flags.go | 2 ++ pkg/loadbalancers/l4netlb.go | 6 +++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/firewalls/firewalls_l4.go b/pkg/firewalls/firewalls_l4.go index 8c70a1a0ad..d79bfcb574 100644 --- a/pkg/firewalls/firewalls_l4.go +++ b/pkg/firewalls/firewalls_l4.go @@ -52,11 +52,7 @@ func EnsureL4FirewallRule(cloud *gce.Cloud, nsName string, params *FirewallParam if err != nil { return err } -<<<<<<< HEAD fwDesc, err := utils.MakeL4LBFirewallDescription(nsName, params.IP, meta.VersionGA, sharedRule) -======= - fwDesc, err := utils.MakeL4LBServiceDescription(nsName, params.IP, meta.VersionGA, sharedRule, params.L4Type) ->>>>>>> 2a315c6d (add netlb changes) if err != nil { klog.Warningf("EnsureL4FirewallRule(%v): failed to generate description for L4 %s rule, err: %v", params.Name, params.L4Type.ToString(), err) } diff --git a/pkg/flags/flags.go b/pkg/flags/flags.go index 8f1dd48faa..777ce88c38 100644 --- a/pkg/flags/flags.go +++ b/pkg/flags/flags.go @@ -108,6 +108,7 @@ var ( EnableTrafficScaling bool EnableEndpointSlices bool EnableMultipleIgs bool + EnablePinhole bool MaxIgSize int }{ GCERateLimitScale: 1.0, @@ -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.`) diff --git a/pkg/loadbalancers/l4netlb.go b/pkg/loadbalancers/l4netlb.go index 9781e21f5f..05822645da 100644 --- a/pkg/loadbalancers/l4netlb.go +++ b/pkg/loadbalancers/l4netlb.go @@ -133,7 +133,7 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service) name := l4netlb.ServicePort.BackendName() _, 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) @@ -248,7 +248,7 @@ func (l4netlb *L4NetLB) GetFRName() string { return utils.LegacyForwardingRuleName(l4netlb.Service) } -func (l4netlb *L4NetLB) createFirewalls(name, nodeNames []string, ipAddress string, portRanges []string, protocol string) *L4NetLBSyncResult { +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 { @@ -272,5 +272,5 @@ func (l4netlb *L4NetLB) createFirewalls(name, nodeNames []string, ipAddress stri result.Error = err return result } - return string(protocol), result + return result }