Skip to content

Commit 04bd3fb

Browse files
authored
Merge pull request #958 from TortillaZHawaii/deny-flag
Deny firewall for external services
2 parents c59d7c0 + 164cd53 commit 04bd3fb

18 files changed

+2410
-408
lines changed

cmd/cloud-controller-manager/main.go

Lines changed: 46 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ import (
2727
"github.com/spf13/pflag"
2828
"k8s.io/apimachinery/pkg/util/wait"
2929
cloudprovider "k8s.io/cloud-provider"
30-
"k8s.io/cloud-provider-gcp/providers/gce"
31-
_ "k8s.io/cloud-provider-gcp/providers/gce"
3230
"k8s.io/cloud-provider/app"
3331
"k8s.io/cloud-provider/app/config"
3432
"k8s.io/cloud-provider/names"
@@ -39,6 +37,9 @@ import (
3937
_ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration
4038
"k8s.io/klog/v2"
4139
kcmnames "k8s.io/kubernetes/cmd/kube-controller-manager/names"
40+
41+
"k8s.io/cloud-provider-gcp/providers/gce"
42+
_ "k8s.io/cloud-provider-gcp/providers/gce"
4243
)
4344

4445
const (
@@ -47,30 +48,36 @@ const (
4748
gkeServiceAlias = "gke-service"
4849
)
4950

50-
// enableMultiProject is bound to a command-line flag. When true, it enables the
51-
// projectFromNodeProviderID option of the GCE cloud provider, instructing it to
52-
// use the project specified in the Node's providerID for GCE API calls.
53-
//
54-
// This flag should only be enabled when the Node's providerID can be fully
55-
// trusted.
56-
//
57-
// Flag binding occurs in main()
58-
var enableMultiProject bool
59-
60-
// enableDiscretePortForwarding is bound to a command-line flag. It enables
61-
// the same option of the GCE cloud provider to forward individual ports
62-
// instead of port ranges in Forwarding Rules for external load balancers.
63-
var enableDiscretePortForwarding bool
64-
65-
// enableRBSDefaultForGCEL4NetLB is bound to a command-line flag. It enables
66-
// the option to default L4 NetLB to RBS, only controlling NetLB services with
67-
// LoadBalancerClass
68-
var enableRBSDefaultForL4NetLB bool
69-
70-
// enableL4LBAnnotations is bound to a command-line flag. It enables
71-
// the controller to write annotations related to the provisioned resources
72-
// for L4 Load Balancers services
73-
var enableL4LBAnnotations bool
51+
var (
52+
// enableMultiProject is bound to a command-line flag. When true, it enables the
53+
// projectFromNodeProviderID option of the GCE cloud provider, instructing it to
54+
// use the project specified in the Node's providerID for GCE API calls.
55+
//
56+
// This flag should only be enabled when the Node's providerID can be fully
57+
// trusted.
58+
//
59+
// Flag binding occurs in main()
60+
enableMultiProject bool
61+
62+
// enableRBSDefaultForGCEL4NetLB is bound to a command-line flag. It enables
63+
// the option to default L4 NetLB to RBS, only controlling NetLB services with
64+
// LoadBalancerClass
65+
enableRBSDefaultForL4NetLB bool
66+
67+
// enableL4LBAnnotations is bound to a command-line flag. It enables
68+
// the controller to write annotations related to the provisioned resources
69+
// for L4 Load Balancers services
70+
enableL4LBAnnotations bool
71+
72+
// enableL4DenyFirewall creates and manages an additional deny firewall rule
73+
// at priority 1000 and moves the node and healthcheck firewall rule to priority 999.
74+
enableL4DenyFirewall bool
75+
76+
// enableL4DenyFirewallRollbackCleanup enable cleanup codepath of the deny firewalls for rollback.
77+
// The reason for it not being enabled by default is the additional GCE API calls that are made
78+
// for checking if the deny firewalls exist/deletion which will eat up the quota unnecessarily.
79+
enableL4DenyFirewallRollbackCleanup bool
80+
)
7481

7582
func main() {
7683
rand.Seed(time.Now().UnixNano())
@@ -88,9 +95,10 @@ func main() {
8895

8996
cloudProviderFS := fss.FlagSet("GCE Cloud Provider")
9097
cloudProviderFS.BoolVar(&enableMultiProject, "enable-multi-project", false, "Enables project selection from Node providerID for GCE API calls. CAUTION: Only enable if Node providerID is configured by a trusted source.")
91-
cloudProviderFS.BoolVar(&enableDiscretePortForwarding, "enable-discrete-port-forwarding", false, "Enables forwarding of individual ports instead of port ranges for GCE external load balancers.")
9298
cloudProviderFS.BoolVar(&enableRBSDefaultForL4NetLB, "enable-rbs-default-l4-netlb", false, "Enables RBS defaulting for GCE L4 NetLB")
9399
cloudProviderFS.BoolVar(&enableL4LBAnnotations, "enable-l4-lb-annotations", false, "Enables Annotations for GCE L4 LB Services")
100+
cloudProviderFS.BoolVar(&enableL4DenyFirewall, "enable-l4-deny-firewall", false, "Enable creation and updates of Deny VPC Firewall Rules for L4 external load balancers. Requires --enable-pinhole and --enable-l4-deny-firewall-rollback-cleanup to be true.")
101+
cloudProviderFS.BoolVar(&enableL4DenyFirewallRollbackCleanup, "enable-l4-deny-firewall-rollback-cleanup", false, "Enable cleanup codepath of the deny firewalls for rollback. The reason for it not being enabled by default is the additional GCE API calls that are made for checking if the deny firewalls exist/deletion which will eat up the quota unnecessarily.")
94102

95103
// add new controllers and initializers
96104
nodeIpamController := nodeIPAMController{}
@@ -158,16 +166,6 @@ func cloudInitializer(config *config.CompletedConfig) cloudprovider.Interface {
158166
gceCloud.SetProjectFromNodeProviderID(true)
159167
}
160168

161-
if enableDiscretePortForwarding {
162-
gceCloud, ok := (cloud).(*gce.Cloud)
163-
if !ok {
164-
// Fail-fast: If enableDiscretePortForwarding is set, the cloud
165-
// provider MUST be GCE.
166-
klog.Fatalf("enable-discrete-port-forwarding requires GCE cloud provider, but got %T", cloud)
167-
}
168-
gceCloud.SetEnableDiscretePortForwarding(true)
169-
}
170-
171169
if enableRBSDefaultForL4NetLB {
172170
gceCloud, ok := (cloud).(*gce.Cloud)
173171
if !ok {
@@ -188,5 +186,16 @@ func cloudInitializer(config *config.CompletedConfig) cloudprovider.Interface {
188186
gceCloud.SetEnableL4LBAnnotations(true)
189187
}
190188

189+
if enableL4DenyFirewall || enableL4DenyFirewallRollbackCleanup {
190+
gceCloud, ok := (cloud).(*gce.Cloud)
191+
if !ok {
192+
klog.Fatalf("enable-l4-deny-firewall and enable-l4-deny-firewall-rollback-cleanup require GCE cloud provider, but got %T", cloud)
193+
}
194+
if enableL4DenyFirewall && !enableL4DenyFirewallRollbackCleanup {
195+
klog.Fatal("enable-l4-deny-firewall requires enable-l4-deny-firewall-rollback-cleanup to be true")
196+
}
197+
gceCloud.SetEnableL4DenyFirewallRule(enableL4DenyFirewall, enableL4DenyFirewallRollbackCleanup)
198+
}
199+
191200
return cloud
192201
}

providers/gce/BUILD

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,11 @@ go_test(
101101
"gce_annotations_test.go",
102102
"gce_disks_test.go",
103103
"gce_instances_test.go",
104+
"gce_loadbalancer_external_deny_test.go",
104105
"gce_loadbalancer_external_test.go",
105106
"gce_loadbalancer_internal_test.go",
106107
"gce_loadbalancer_metrics_test.go",
108+
"gce_loadbalancer_naming_test.go",
107109
"gce_loadbalancer_test.go",
108110
"gce_loadbalancer_utils_test.go",
109111
"gce_test.go",
@@ -116,6 +118,7 @@ go_test(
116118
"//vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta",
117119
"//vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/mock",
118120
"//vendor/github.com/google/go-cmp/cmp",
121+
"//vendor/github.com/google/go-cmp/cmp/cmpopts",
119122
"//vendor/github.com/stretchr/testify/assert",
120123
"//vendor/github.com/stretchr/testify/require",
121124
"//vendor/golang.org/x/oauth2/google",
@@ -132,6 +135,7 @@ go_test(
132135
"//vendor/k8s.io/client-go/tools/record",
133136
"//vendor/k8s.io/cloud-provider",
134137
"//vendor/k8s.io/cloud-provider/service/helpers",
138+
"//vendor/k8s.io/component-base/metrics/testutil",
135139
"//vendor/k8s.io/utils/net",
136140
],
137141
)

providers/gce/gce.go

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,15 @@ const (
8585
gceComputeAPIEndpointBeta = "https://www.googleapis.com/compute/beta/"
8686
)
8787

88-
var _ cloudprovider.Interface = (*Cloud)(nil)
89-
var _ cloudprovider.Instances = (*Cloud)(nil)
90-
var _ cloudprovider.LoadBalancer = (*Cloud)(nil)
91-
var _ cloudprovider.Routes = (*Cloud)(nil)
92-
var _ cloudprovider.Zones = (*Cloud)(nil)
93-
var _ cloudprovider.PVLabeler = (*Cloud)(nil)
94-
var _ cloudprovider.Clusters = (*Cloud)(nil)
88+
var (
89+
_ cloudprovider.Interface = (*Cloud)(nil)
90+
_ cloudprovider.Instances = (*Cloud)(nil)
91+
_ cloudprovider.LoadBalancer = (*Cloud)(nil)
92+
_ cloudprovider.Routes = (*Cloud)(nil)
93+
_ cloudprovider.Zones = (*Cloud)(nil)
94+
_ cloudprovider.PVLabeler = (*Cloud)(nil)
95+
_ cloudprovider.Clusters = (*Cloud)(nil)
96+
)
9597

9698
type StackType string
9799

@@ -203,15 +205,18 @@ type Cloud struct {
203205
// Enable this ony when the Node's .spec.providerID can be fully trusted.
204206
projectFromNodeProviderID bool
205207

206-
// enableDiscretePortForwarding enables forwarding of individual ports
207-
// instead of port ranges in Forwarding Rules for external load balancers.
208-
enableDiscretePortForwarding bool
209-
210208
// enableRBSDefaultForL4NetLB disable Service controller from picking up services by default
211209
enableRBSDefaultForL4NetLB bool
212210

213211
// enableL4LBAnnotations enable annotations related to provisioned resources in GCE
214212
enableL4LBAnnotations bool
213+
214+
// enableL4DenyFirewallRule creates an additional deny firewall rule at priority 1000
215+
// and moves the allow rule to priority 999 to improve security posture.
216+
enableL4DenyFirewallRule bool
217+
218+
// enableL4DenyFirewallRollbackCleanup
219+
enableL4DenyFirewallRollbackCleanup bool
215220
}
216221

217222
// ConfigGlobal is the in memory representation of the gce.conf config data
@@ -864,11 +869,6 @@ func (g *Cloud) SetProjectFromNodeProviderID(enabled bool) {
864869
g.projectFromNodeProviderID = enabled
865870
}
866871

867-
// SetEnableDiscretePortForwarding configures enableDiscretePortForwarding option.
868-
func (g *Cloud) SetEnableDiscretePortForwarding(enabled bool) {
869-
g.enableDiscretePortForwarding = enabled
870-
}
871-
872872
func (g *Cloud) SetEnableRBSDefaultForL4NetLB(enabled bool) {
873873
g.enableRBSDefaultForL4NetLB = enabled
874874
}
@@ -877,6 +877,11 @@ func (g *Cloud) SetEnableL4LBAnnotations(enabled bool) {
877877
g.enableL4LBAnnotations = enabled
878878
}
879879

880+
func (g *Cloud) SetEnableL4DenyFirewallRule(firewallEnabled, rollbackEnabled bool) {
881+
g.enableL4DenyFirewallRule = firewallEnabled
882+
g.enableL4DenyFirewallRollbackCleanup = rollbackEnabled
883+
}
884+
880885
// getProjectsBasePath returns the compute API endpoint with the `projects/` element.
881886
// The suffix must be added when generating compute resource urls.
882887
func getProjectsBasePath(basePath string) string {
@@ -970,7 +975,7 @@ func getZonesForRegion(svc *compute.Service, projectID, region string) ([]string
970975
// listCall = listCall.Filter("region eq " + region)
971976

972977
var zones []string
973-
var accumulator = func(response *compute.ZoneList) error {
978+
accumulator := func(response *compute.ZoneList) error {
974979
for _, zone := range response.Items {
975980
regionName := lastComponent(zone.Region)
976981
if regionName == region {

0 commit comments

Comments
 (0)