From e0372aba5d4787d9264376dd32d45b0e43b819df Mon Sep 17 00:00:00 2001 From: Rohit Ramkumar Date: Mon, 14 May 2018 08:19:53 -0700 Subject: [PATCH] IAP and CDN plumbing --- cmd/glbc/main.go | 2 +- pkg/apis/backendconfig/v1beta1/types.go | 6 +- .../v1beta1/zz_generated.openapi.go | 9 +- pkg/backendconfig/backendconfig_test.go | 5 +- pkg/backends/backends.go | 33 ++- pkg/backends/backends_test.go | 6 +- pkg/backends/features/cdn.go | 68 ++++++ pkg/backends/features/cdn_test.go | 206 ++++++++++++++++++ pkg/backends/features/iap.go | 67 ++++++ pkg/backends/features/iap_test.go | 188 ++++++++++++++++ pkg/composite/composite.go | 21 ++ pkg/controller/cluster_manager.go | 13 +- pkg/controller/controller.go | 2 +- pkg/controller/fakes.go | 2 +- pkg/controller/translator/translator.go | 9 +- pkg/utils/utils.go | 6 +- 16 files changed, 604 insertions(+), 39 deletions(-) create mode 100644 pkg/backends/features/cdn.go create mode 100644 pkg/backends/features/cdn_test.go create mode 100644 pkg/backends/features/iap.go create mode 100644 pkg/backends/features/iap_test.go diff --git a/cmd/glbc/main.go b/cmd/glbc/main.go index 0f4e0e4824..4f1c2fa621 100644 --- a/cmd/glbc/main.go +++ b/cmd/glbc/main.go @@ -165,7 +165,7 @@ func runControllers(ctx *context.ControllerContext) { } defaultBackendServicePortID := app.DefaultBackendServicePortID(ctx.KubeClient) - clusterManager, err := controller.NewClusterManager(ctx.Cloud, namer, defaultBackendServicePortID, flags.F.HealthCheckPath, flags.F.DefaultSvcHealthCheckPath) + clusterManager, err := controller.NewClusterManager(ctx, namer, defaultBackendServicePortID, flags.F.HealthCheckPath, flags.F.DefaultSvcHealthCheckPath) if err != nil { glog.Fatalf("controller.NewClusterManager(cloud, namer, %+v, %q, %q) = %v", defaultBackendServicePortID, flags.F.HealthCheckPath, flags.F.DefaultSvcHealthCheckPath, err) } diff --git a/pkg/apis/backendconfig/v1beta1/types.go b/pkg/apis/backendconfig/v1beta1/types.go index 896247e657..08ad2b8a61 100644 --- a/pkg/apis/backendconfig/v1beta1/types.go +++ b/pkg/apis/backendconfig/v1beta1/types.go @@ -30,7 +30,7 @@ type BackendConfig struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec BackendConfigSpec `json:"spec,omitempty"` + Spec BackendConfigSpec `json:"spec"` Status BackendConfigStatus `json:"status,omitempty"` } @@ -60,14 +60,14 @@ type BackendConfigList struct { // +k8s:openapi-gen=true type IAPConfig struct { Enabled bool `json:"enabled"` - OAuthClientCredentials *OAuthClientCredentials `json:"clientCredentials"` + OAuthClientCredentials *OAuthClientCredentials `json:"oauthclientCredentials"` } // OAuthClientCredentials contains credentials for a single IAP-enabled backend. // +k8s:openapi-gen=true type OAuthClientCredentials struct { // The name of a k8s secret which stores the OAuth client id & secret. - SecretName string `json:"secret"` + SecretName string `json:"secretName"` // Direct reference to OAuth client id. ClientID string `json:"clientID,omitempty"` // Direct reference to OAuth client secret. diff --git a/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go b/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go index 00baec1423..ddb7f17e3b 100644 --- a/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go +++ b/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go @@ -64,6 +64,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA }, }, }, + Required: []string{"spec"}, }, }, Dependencies: []string{ @@ -188,13 +189,13 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA Format: "", }, }, - "clientCredentials": { + "oauthclientCredentials": { SchemaProps: spec.SchemaProps{ Ref: ref("k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1.OAuthClientCredentials"), }, }, }, - Required: []string{"enabled", "clientCredentials"}, + Required: []string{"enabled", "oauthclientCredentials"}, }, }, Dependencies: []string{ @@ -205,7 +206,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA SchemaProps: spec.SchemaProps{ Description: "OAuthClientCredentials contains credentials for a single IAP-enabled backend.", Properties: map[string]spec.Schema{ - "secret": { + "secretName": { SchemaProps: spec.SchemaProps{ Description: "The name of a k8s secret which stores the OAuth client id & secret.", Type: []string{"string"}, @@ -227,7 +228,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA }, }, }, - Required: []string{"secret"}, + Required: []string{"secretName"}, }, }, Dependencies: []string{}, diff --git a/pkg/backendconfig/backendconfig_test.go b/pkg/backendconfig/backendconfig_test.go index ad9431a3c0..363cff1e5f 100644 --- a/pkg/backendconfig/backendconfig_test.go +++ b/pkg/backendconfig/backendconfig_test.go @@ -297,8 +297,9 @@ func TestGetBackendConfigForServicePort(t *testing.T) { expectedConfig: testBackendConfig, }, { - desc: "service with no backend config", - svc: svcWithoutConfig, + desc: "service with no backend config", + svc: svcWithoutConfig, + expectedConfig: nil, }, { desc: "service with backend config but port doesn't match", diff --git a/pkg/backends/backends.go b/pkg/backends/backends.go index 28405a7d3b..4d9ce1709a 100644 --- a/pkg/backends/backends.go +++ b/pkg/backends/backends.go @@ -32,6 +32,7 @@ import ( "k8s.io/kubernetes/pkg/cloudprovider/providers/gce" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/meta" + "k8s.io/ingress-gce/pkg/backends/features" "k8s.io/ingress-gce/pkg/composite" "k8s.io/ingress-gce/pkg/healthchecks" "k8s.io/ingress-gce/pkg/instances" @@ -76,13 +77,14 @@ const maxRPS = 1 // Backends implements BackendPool. type Backends struct { - cloud *gce.GCECloud - negGetter NEGGetter - nodePool instances.NodePool - healthChecker healthchecks.HealthChecker - snapshotter storage.Snapshotter - prober ProbeProvider - namer *utils.Namer + cloud *gce.GCECloud + negGetter NEGGetter + nodePool instances.NodePool + healthChecker healthchecks.HealthChecker + snapshotter storage.Snapshotter + prober ProbeProvider + namer *utils.Namer + backendConfigEnabled bool } // Backends is a BackendPool. @@ -101,14 +103,16 @@ func NewBackendPool( healthChecker healthchecks.HealthChecker, nodePool instances.NodePool, namer *utils.Namer, + backendConfigEnabled, resyncWithCloud bool) *Backends { backendPool := &Backends{ - cloud: cloud, - negGetter: negGetter, - nodePool: nodePool, - healthChecker: healthChecker, - namer: namer, + cloud: cloud, + negGetter: negGetter, + nodePool: nodePool, + healthChecker: healthChecker, + namer: namer, + backendConfigEnabled: backendConfigEnabled, } if !resyncWithCloud { backendPool.snapshotter = storage.NewInMemoryPool() @@ -289,6 +293,11 @@ func (b *Backends) ensureBackendService(sp utils.ServicePort, igLinks []string) needUpdate := ensureProtocol(be, sp) needUpdate = ensureHealthCheckLink(be, hcLink) || needUpdate needUpdate = ensureDescription(be, sp.Description()) || needUpdate + if b.backendConfigEnabled && sp.BackendConfig != nil { + needUpdate = features.EnsureCDN(sp, be) || needUpdate + needUpdate = features.EnsureIAP(sp, be) || needUpdate + } + if needUpdate { if err = composite.UpdateBackendService(be, b.cloud); err != nil { return err diff --git a/pkg/backends/backends_test.go b/pkg/backends/backends_test.go index 98cc3fd3e0..452147d436 100644 --- a/pkg/backends/backends_test.go +++ b/pkg/backends/backends_test.go @@ -69,7 +69,7 @@ func newTestJig(gce *gce.GCECloud, fakeIGs instances.InstanceGroups, syncWithClo nodePool.Init(&instances.FakeZoneLister{Zones: []string{defaultZone}}) healthCheckProvider := healthchecks.NewFakeHealthCheckProvider() healthChecks := healthchecks.NewHealthChecker(healthCheckProvider, "/", "/healthz", defaultNamer, defaultBackendSvc) - bp := NewBackendPool(gce, negGetter, healthChecks, nodePool, defaultNamer, syncWithCloud) + bp := NewBackendPool(gce, negGetter, healthChecks, nodePool, defaultNamer, false, syncWithCloud) probes := map[utils.ServicePort]*api_v1.Probe{{NodePort: 443, Protocol: annotations.ProtocolHTTPS}: existingProbe} bp.Init(NewFakeProbeProvider(probes)) @@ -642,7 +642,7 @@ func TestBackendPoolDeleteLegacyHealthChecks(t *testing.T) { nodePool.Init(&instances.FakeZoneLister{Zones: []string{defaultZone}}) hcp := healthchecks.NewFakeHealthCheckProvider() healthChecks := healthchecks.NewHealthChecker(hcp, "/", "/healthz", defaultNamer, defaultBackendSvc) - bp := NewBackendPool(fakeGCE, negGetter, healthChecks, nodePool, defaultNamer, false) + bp := NewBackendPool(fakeGCE, negGetter, healthChecks, nodePool, defaultNamer, false, false) probes := map[utils.ServicePort]*api_v1.Probe{} bp.Init(NewFakeProbeProvider(probes)) @@ -832,7 +832,7 @@ func TestLinkBackendServiceToNEG(t *testing.T) { nodePool.Init(&instances.FakeZoneLister{Zones: []string{defaultZone}}) hcp := healthchecks.NewFakeHealthCheckProvider() healthChecks := healthchecks.NewHealthChecker(hcp, "/", "/healthz", defaultNamer, defaultBackendSvc) - bp := NewBackendPool(fakeGCE, fakeNEG, healthChecks, nodePool, defaultNamer, false) + bp := NewBackendPool(fakeGCE, fakeNEG, healthChecks, nodePool, defaultNamer, false, false) // Add standard hooks for mocking update calls. Each test can set a update different hook if it chooses to. (fakeGCE.Compute().(*cloud.MockGCE)).MockAlphaBackendServices.UpdateHook = mock.UpdateAlphaBackendServiceHook diff --git a/pkg/backends/features/cdn.go b/pkg/backends/features/cdn.go new file mode 100644 index 0000000000..0915a968ae --- /dev/null +++ b/pkg/backends/features/cdn.go @@ -0,0 +1,68 @@ +/* +Copyright 2018 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 features + +import ( + "reflect" + + "github.com/golang/glog" + backendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1" + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/utils" +) + +// EnsureCDN reads the CDN configuration specified in the ServicePort.BackendConfig +// and applies it to the BackendService. It returns true if there were existing +// settings on the BackendService that were overwritten. +func EnsureCDN(sp utils.ServicePort, be *composite.BackendService) bool { + beTemp := &composite.BackendService{} + applyCDNSettings(sp, beTemp) + if !reflect.DeepEqual(beTemp.CdnPolicy, be.CdnPolicy) || beTemp.EnableCDN != be.EnableCDN { + applyCDNSettings(sp, be) + glog.V(2).Infof("Updated CDN settings for service %v/%v.", sp.ID.Service.Namespace, sp.ID.Service.Name) + return true + } + return false +} + +// applyCDNSettings applies the CDN settings specified in the BackendConfig +// to the passed in compute.BackendService. A GCE API call still needs to be +// made to actually persist the changes. +func applyCDNSettings(sp utils.ServicePort, be *composite.BackendService) { + beConfig := sp.BackendConfig + setCDNDefaults(beConfig) + // Apply the boolean switch + be.EnableCDN = beConfig.Spec.Cdn.Enabled + // Apply the cache key policies + cacheKeyPolicy := beConfig.Spec.Cdn.CachePolicy + be.CdnPolicy = &composite.BackendServiceCdnPolicy{CacheKeyPolicy: &composite.CacheKeyPolicy{}} + be.CdnPolicy.CacheKeyPolicy.IncludeHost = cacheKeyPolicy.IncludeHost + be.CdnPolicy.CacheKeyPolicy.IncludeProtocol = cacheKeyPolicy.IncludeProtocol + be.CdnPolicy.CacheKeyPolicy.IncludeQueryString = cacheKeyPolicy.IncludeQueryString + be.CdnPolicy.CacheKeyPolicy.QueryStringBlacklist = cacheKeyPolicy.QueryStringBlacklist + be.CdnPolicy.CacheKeyPolicy.QueryStringWhitelist = cacheKeyPolicy.QueryStringWhitelist +} + +// setCDNDefaults initializes any nil pointers in CDN configuration which ensures that there are defaults for missing sub-types. +func setCDNDefaults(beConfig *backendconfigv1beta1.BackendConfig) { + if beConfig.Spec.Cdn == nil { + beConfig.Spec.Cdn = &backendconfigv1beta1.CDNConfig{} + } + if beConfig.Spec.Cdn.CachePolicy == nil { + beConfig.Spec.Cdn.CachePolicy = &backendconfigv1beta1.CacheKeyPolicy{} + } +} diff --git a/pkg/backends/features/cdn_test.go b/pkg/backends/features/cdn_test.go new file mode 100644 index 0000000000..851e6fb899 --- /dev/null +++ b/pkg/backends/features/cdn_test.go @@ -0,0 +1,206 @@ +/* +Copyright 2015 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 features + +import ( + "reflect" + "testing" + + backendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1" + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/utils" +) + +func TestEnsureCDN(t *testing.T) { + testCases := []struct { + desc string + sp utils.ServicePort + be *composite.BackendService + expected bool + }{ + { + "settings are identical, no update needed", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Cdn: &backendconfigv1beta1.CDNConfig{ + Enabled: true, + CachePolicy: &backendconfigv1beta1.CacheKeyPolicy{ + IncludeHost: true, + }, + }, + }, + }, + }, + &composite.BackendService{ + EnableCDN: true, + CdnPolicy: &composite.BackendServiceCdnPolicy{ + CacheKeyPolicy: &composite.CacheKeyPolicy{ + IncludeHost: true, + }, + }, + }, + false, + }, + { + "cache settings are different, update needed", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Cdn: &backendconfigv1beta1.CDNConfig{ + Enabled: true, + CachePolicy: &backendconfigv1beta1.CacheKeyPolicy{ + QueryStringWhitelist: []string{"foo"}, + }, + }, + }, + }, + }, + &composite.BackendService{ + EnableCDN: true, + }, + true, + }, + { + "enabled setting is different, update needed", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Cdn: &backendconfigv1beta1.CDNConfig{ + Enabled: true, + }, + }, + }, + }, + &composite.BackendService{ + EnableCDN: false, + }, + true, + }, + } + + for _, testCase := range testCases { + result := EnsureCDN(testCase.sp, testCase.be) + if result != testCase.expected { + t.Errorf("%v: expected %v but got %v", testCase.desc, testCase.expected, result) + } + } +} + +func TestApplyCDNSettings(t *testing.T) { + testCases := []struct { + desc string + sp utils.ServicePort + be *composite.BackendService + expected composite.BackendService + }{ + { + "apply settings on empty BackendService", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Cdn: &backendconfigv1beta1.CDNConfig{ + Enabled: true, + CachePolicy: &backendconfigv1beta1.CacheKeyPolicy{ + IncludeHost: true, + }, + }, + }, + }, + }, + &composite.BackendService{}, + composite.BackendService{ + EnableCDN: true, + CdnPolicy: &composite.BackendServiceCdnPolicy{ + CacheKeyPolicy: &composite.CacheKeyPolicy{ + IncludeHost: true, + }, + }, + }, + }, + { + "overwrite some fields on existing settings", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Cdn: &backendconfigv1beta1.CDNConfig{ + Enabled: true, + CachePolicy: &backendconfigv1beta1.CacheKeyPolicy{ + IncludeHost: true, + IncludeProtocol: false, + IncludeQueryString: true, + }, + }, + }, + }, + }, + &composite.BackendService{ + EnableCDN: false, + CdnPolicy: &composite.BackendServiceCdnPolicy{ + CacheKeyPolicy: &composite.CacheKeyPolicy{ + IncludeHost: false, + IncludeProtocol: true, + IncludeQueryString: true, + }, + }, + }, + composite.BackendService{ + EnableCDN: true, + CdnPolicy: &composite.BackendServiceCdnPolicy{ + CacheKeyPolicy: &composite.CacheKeyPolicy{ + IncludeHost: true, + IncludeProtocol: false, + IncludeQueryString: true, + }, + }, + }, + }, + { + "no feature settings in spec", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Cdn: nil, + }, + }, + }, + &composite.BackendService{ + EnableCDN: true, + CdnPolicy: &composite.BackendServiceCdnPolicy{ + CacheKeyPolicy: &composite.CacheKeyPolicy{ + IncludeHost: true, + }, + }, + }, + composite.BackendService{ + EnableCDN: false, + CdnPolicy: &composite.BackendServiceCdnPolicy{ + CacheKeyPolicy: &composite.CacheKeyPolicy{ + IncludeHost: false, + }, + }, + }, + }, + } + + for _, testCase := range testCases { + applyCDNSettings(testCase.sp, testCase.be) + if !reflect.DeepEqual(testCase.expected, *testCase.be) { + t.Errorf("%v: expected %+v but got %+v", testCase.desc, testCase.expected, *testCase.be) + } + } +} diff --git a/pkg/backends/features/iap.go b/pkg/backends/features/iap.go new file mode 100644 index 0000000000..dace585152 --- /dev/null +++ b/pkg/backends/features/iap.go @@ -0,0 +1,67 @@ +/* +Copyright 2015 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 features + +import ( + "crypto/sha256" + "fmt" + + "github.com/golang/glog" + backendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1" + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/utils" +) + +// EnsureIAP reads the IAP configuration specified in the BackendConfig +// and applies it to the BackendService if it is stale. It returns true +// if there were existing settings on the BackendService that were overwritten. +func EnsureIAP(sp utils.ServicePort, be *composite.BackendService) bool { + beTemp := &composite.BackendService{} + applyIAPSettings(sp, beTemp) + // We need to compare the SHA256 of the client secret instead of the client secret itself + // since that field is redacted when getting a BackendService. + beTemp.Iap.Oauth2ClientSecretSha256 = fmt.Sprintf("%x", sha256.Sum256([]byte(beTemp.Iap.Oauth2ClientSecret))) + if be.Iap == nil || beTemp.Iap.Enabled != be.Iap.Enabled || beTemp.Iap.Oauth2ClientId != be.Iap.Oauth2ClientId || beTemp.Iap.Oauth2ClientSecretSha256 != be.Iap.Oauth2ClientSecretSha256 { + applyIAPSettings(sp, be) + glog.V(2).Infof("Updated IAP settings for service %v/%v.", sp.ID.Service.Namespace, sp.ID.Service.Name) + return true + } + return false +} + +// applyIAPSettings applies the IAP settings specified in the BackendConfig +// to the passed in compute.BackendService. A GCE API call still needs to be +// made to actually persist the changes. +func applyIAPSettings(sp utils.ServicePort, be *composite.BackendService) { + beConfig := sp.BackendConfig + setIAPDefaults(beConfig) + // Apply the boolean switch + be.Iap = &composite.BackendServiceIAP{Enabled: beConfig.Spec.Iap.Enabled} + // Apply the OAuth credentials + be.Iap.Oauth2ClientId = beConfig.Spec.Iap.OAuthClientCredentials.ClientID + be.Iap.Oauth2ClientSecret = beConfig.Spec.Iap.OAuthClientCredentials.ClientSecret +} + +// setIAPDefaults initializes any nil pointers in IAP configuration which ensures that there are defaults for missing sub-types. +func setIAPDefaults(beConfig *backendconfigv1beta1.BackendConfig) { + if beConfig.Spec.Iap == nil { + beConfig.Spec.Iap = &backendconfigv1beta1.IAPConfig{} + } + if beConfig.Spec.Iap.OAuthClientCredentials == nil { + beConfig.Spec.Iap.OAuthClientCredentials = &backendconfigv1beta1.OAuthClientCredentials{} + } +} diff --git a/pkg/backends/features/iap_test.go b/pkg/backends/features/iap_test.go new file mode 100644 index 0000000000..aa0a61ccec --- /dev/null +++ b/pkg/backends/features/iap_test.go @@ -0,0 +1,188 @@ +/* +Copyright 2015 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 features + +import ( + "crypto/sha256" + "fmt" + "reflect" + "testing" + + backendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1" + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/utils" +) + +func TestApplyIAPSettings(t *testing.T) { + testCases := []struct { + desc string + sp utils.ServicePort + be *composite.BackendService + expected composite.BackendService + }{ + { + "apply settings on empty BackendService", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Iap: &backendconfigv1beta1.IAPConfig{ + Enabled: true, + OAuthClientCredentials: &backendconfigv1beta1.OAuthClientCredentials{ + ClientID: "foo", + ClientSecret: "bar", + }, + }, + }, + }, + }, + &composite.BackendService{}, + composite.BackendService{ + Iap: &composite.BackendServiceIAP{ + Enabled: true, + Oauth2ClientId: "foo", + Oauth2ClientSecret: "bar", + }, + }, + }, + { + "overwrite some fields on existing settings", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Iap: &backendconfigv1beta1.IAPConfig{ + Enabled: true, + OAuthClientCredentials: &backendconfigv1beta1.OAuthClientCredentials{ + ClientID: "foo", + ClientSecret: "baz", + }, + }, + }, + }, + }, + &composite.BackendService{ + Iap: &composite.BackendServiceIAP{ + Enabled: false, + Oauth2ClientId: "foo", + Oauth2ClientSecret: "bar", + }, + }, + composite.BackendService{ + Iap: &composite.BackendServiceIAP{ + Enabled: true, + Oauth2ClientId: "foo", + Oauth2ClientSecret: "baz", + }, + }, + }, + { + "no feature settings in spec", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Iap: nil, + }, + }, + }, + &composite.BackendService{ + Iap: &composite.BackendServiceIAP{ + Enabled: false, + Oauth2ClientId: "foo", + Oauth2ClientSecret: "bar", + }, + }, + composite.BackendService{ + Iap: &composite.BackendServiceIAP{ + Enabled: false, + Oauth2ClientId: "", + Oauth2ClientSecret: "", + }, + }, + }, + } + + for _, testCase := range testCases { + applyIAPSettings(testCase.sp, testCase.be) + if !reflect.DeepEqual(testCase.expected, *testCase.be) { + t.Errorf("%v: expected %+v but got %+v", testCase.desc, testCase.expected, *testCase.be) + } + } +} + +func TestEnsureIAP(t *testing.T) { + testCases := []struct { + desc string + sp utils.ServicePort + be *composite.BackendService + expected bool + }{ + { + "settings are identical, no update needed", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Iap: &backendconfigv1beta1.IAPConfig{ + Enabled: true, + OAuthClientCredentials: &backendconfigv1beta1.OAuthClientCredentials{ + ClientID: "foo", + ClientSecret: "bar", + }, + }, + }, + }, + }, + &composite.BackendService{ + Iap: &composite.BackendServiceIAP{ + Enabled: true, + Oauth2ClientId: "foo", + Oauth2ClientSecretSha256: fmt.Sprintf("%x", sha256.Sum256([]byte("bar"))), + }, + }, + false, + }, + { + "credential settings are different, update needed", + utils.ServicePort{ + BackendConfig: &backendconfigv1beta1.BackendConfig{ + Spec: backendconfigv1beta1.BackendConfigSpec{ + Iap: &backendconfigv1beta1.IAPConfig{ + Enabled: true, + OAuthClientCredentials: &backendconfigv1beta1.OAuthClientCredentials{ + ClientID: "foo", + ClientSecret: "baz", + }, + }, + }, + }, + }, + &composite.BackendService{ + Iap: &composite.BackendServiceIAP{ + Enabled: true, + Oauth2ClientId: "foo", + Oauth2ClientSecret: "bar", + }, + }, + true, + }, + } + + for _, testCase := range testCases { + result := EnsureIAP(testCase.sp, testCase.be) + if result != testCase.expected { + t.Errorf("%v: expected %v but got %v", testCase.desc, testCase.expected, result) + } + } +} diff --git a/pkg/composite/composite.go b/pkg/composite/composite.go index 32480f685b..cdf9599a13 100644 --- a/pkg/composite/composite.go +++ b/pkg/composite/composite.go @@ -240,6 +240,13 @@ func (be *BackendService) toAlpha() (*computealpha.BackendService, error) { if err != nil { return nil, fmt.Errorf("error unmarshalling BackendService JSON to compute alpha type: %v", err) } + // Set force send fields. This is a temporary hack. + if alpha.CdnPolicy != nil && alpha.CdnPolicy.CacheKeyPolicy != nil { + alpha.CdnPolicy.CacheKeyPolicy.ForceSendFields = []string{"IncludeHost", "IncludeProtocol", "IncludeQueryString", "QueryStringBlacklist", "QueryStringWhitelist"} + } + if alpha.Iap != nil { + alpha.Iap.ForceSendFields = []string{"Enabled", "Oauth2ClientId", "Oauth2ClientSecret"} + } return alpha, nil } @@ -255,6 +262,13 @@ func (be *BackendService) toBeta() (*computebeta.BackendService, error) { if err != nil { return nil, fmt.Errorf("error unmarshalling BackendService JSON to compute beta type: %v", err) } + // Set force send fields. This is a temporary hack. + if beta.CdnPolicy != nil && beta.CdnPolicy.CacheKeyPolicy != nil { + beta.CdnPolicy.CacheKeyPolicy.ForceSendFields = []string{"IncludeHost", "IncludeProtocol", "IncludeQueryString", "QueryStringBlacklist", "QueryStringWhitelist"} + } + if beta.Iap != nil { + beta.Iap.ForceSendFields = []string{"Enabled", "Oauth2ClientId", "Oauth2ClientSecret"} + } return beta, nil } @@ -270,5 +284,12 @@ func (be *BackendService) toGA() (*compute.BackendService, error) { if err != nil { return nil, fmt.Errorf("error unmarshalling BackendService JSON to compute GA type: %v", err) } + // Set force send fields. This is a temporary hack. + if ga.CdnPolicy != nil && ga.CdnPolicy.CacheKeyPolicy != nil { + ga.CdnPolicy.CacheKeyPolicy.ForceSendFields = []string{"IncludeHost", "IncludeProtocol", "IncludeQueryString", "QueryStringBlacklist", "QueryStringWhitelist"} + } + if ga.Iap != nil { + ga.Iap.ForceSendFields = []string{"Enabled", "Oauth2ClientId", "Oauth2ClientSecret"} + } return ga, nil } diff --git a/pkg/controller/cluster_manager.go b/pkg/controller/cluster_manager.go index 1b27efd438..8c4f49b9a7 100644 --- a/pkg/controller/cluster_manager.go +++ b/pkg/controller/cluster_manager.go @@ -25,6 +25,7 @@ import ( gce "k8s.io/kubernetes/pkg/cloudprovider/providers/gce" "k8s.io/ingress-gce/pkg/backends" + "k8s.io/ingress-gce/pkg/context" "k8s.io/ingress-gce/pkg/firewalls" "k8s.io/ingress-gce/pkg/flags" "k8s.io/ingress-gce/pkg/healthchecks" @@ -175,7 +176,7 @@ func (c *ClusterManager) GC(lbNames []string, nodePorts []utils.ServicePort) err // - healthCheckPath: is the default path used for L7 health checks, eg: "/healthz". // - defaultBackendHealthCheckPath: is the default path used for the default backend health checks. func NewClusterManager( - cloud *gce.GCECloud, + ctx *context.ControllerContext, namer *utils.Namer, defaultBackendSvcPortID utils.ServicePortID, healthCheckPath string, @@ -185,14 +186,14 @@ func NewClusterManager( cluster := ClusterManager{ClusterNamer: namer, defaultBackendSvcPortID: defaultBackendSvcPortID} // NodePool stores GCE vms that are in this Kubernetes cluster. - cluster.instancePool = instances.NewNodePool(cloud, namer) + cluster.instancePool = instances.NewNodePool(ctx.Cloud, namer) // BackendPool creates GCE BackendServices and associated health checks. - cluster.healthChecker = healthchecks.NewHealthChecker(cloud, healthCheckPath, defaultBackendHealthCheckPath, cluster.ClusterNamer, defaultBackendSvcPortID.Service) - cluster.backendPool = backends.NewBackendPool(cloud, cloud, cluster.healthChecker, cluster.instancePool, cluster.ClusterNamer, true) + cluster.healthChecker = healthchecks.NewHealthChecker(ctx.Cloud, healthCheckPath, defaultBackendHealthCheckPath, cluster.ClusterNamer, defaultBackendSvcPortID.Service) + cluster.backendPool = backends.NewBackendPool(ctx.Cloud, ctx.Cloud, cluster.healthChecker, cluster.instancePool, cluster.ClusterNamer, ctx.BackendConfigEnabled, true) // L7 pool creates targetHTTPProxy, ForwardingRules, UrlMaps, StaticIPs. - cluster.l7Pool = loadbalancers.NewLoadBalancerPool(cloud, cluster.ClusterNamer) - cluster.firewallPool = firewalls.NewFirewallPool(cloud, cluster.ClusterNamer, gce.LoadBalancerSrcRanges(), flags.F.NodePortRanges.Values()) + cluster.l7Pool = loadbalancers.NewLoadBalancerPool(ctx.Cloud, cluster.ClusterNamer) + cluster.firewallPool = firewalls.NewFirewallPool(ctx.Cloud, cluster.ClusterNamer, gce.LoadBalancerSrcRanges(), flags.F.NodePortRanges.Values()) return &cluster, nil } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 91f27a2056..e9751afcf9 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -178,7 +178,7 @@ func NewLoadBalancerController( }) } - lbc.Translator = translator.NewTranslator(lbc.CloudClusterManager.ClusterNamer, ctx) + lbc.Translator = translator.NewTranslator(lbc.CloudClusterManager.ClusterNamer, lbc.ctx) lbc.tlsLoader = &tls.TLSCertsFromSecretsLoader{Client: lbc.client} // Register health check on controller context. diff --git a/pkg/controller/fakes.go b/pkg/controller/fakes.go index 7b6ca7bdb8..d268215cac 100644 --- a/pkg/controller/fakes.go +++ b/pkg/controller/fakes.go @@ -69,7 +69,7 @@ func NewFakeClusterManager(clusterName, firewallName string) *fakeClusterManager backendPool := backends.NewBackendPool( fakeBackends, fakeNEG, - healthChecker, nodePool, namer, false) + healthChecker, nodePool, namer, false, false) l7Pool := loadbalancers.NewLoadBalancerPool(fakeLbs, namer) frPool := firewalls.NewFirewallPool(firewalls.NewFakeFirewallsProvider(false, false), namer, testSrcRanges, testNodePortRanges) cm := &ClusterManager{ diff --git a/pkg/controller/translator/translator.go b/pkg/controller/translator/translator.go index ecd9c8617c..8fc70c98d0 100644 --- a/pkg/controller/translator/translator.go +++ b/pkg/controller/translator/translator.go @@ -107,7 +107,11 @@ func (t *Translator) getServicePort(id utils.ServicePortID) (*utils.ServicePort, if t.ctx.BackendConfigEnabled { beConfig, err = backendconfig.GetBackendConfigForServicePort(t.ctx.BackendConfigInformer.GetIndexer(), svc, port) if err != nil { - return svcPort, errors.ErrSvcBackendConfig{ServicePortID: id, Err: err} + if err == backendconfig.ErrNoBackendConfigForPort { + beConfig = &backendconfigv1beta1.BackendConfig{} + } else { + return svcPort, errors.ErrSvcBackendConfig{ServicePortID: id, Err: err} + } } // Object in cache could be changed in-flight. Deepcopy to // reduce race conditions. @@ -115,9 +119,8 @@ func (t *Translator) getServicePort(id utils.ServicePortID) (*utils.ServicePort, if err = backendconfig.Validate(t.ctx.KubeClient, beConfig); err != nil { return svcPort, errors.ErrBackendConfigValidation{BackendConfig: *beConfig, Err: err} } + svcPort.BackendConfig = beConfig } - svcPort.BackendConfig = beConfig - return svcPort, nil } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index d7afb4d1e0..cd46056fae 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -176,11 +176,11 @@ func BackendServiceRelativeResourcePath(name string) string { // for a global BackendService. // global/backendServices/[BACKEND_SERVICE_NAME] func BackendServiceComparablePath(url string) string { - path_parts := strings.Split(url, "global/") - if len(path_parts) != 2 { + pathParts := strings.Split(url, "global/") + if len(pathParts) != 2 { return "" } - return fmt.Sprintf("global/%s", path_parts[1]) + return fmt.Sprintf("global/%s", pathParts[1]) } // IGLinks returns a list of links extracted from the passed in list of