Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IAP + CDN #301

Merged
merged 1 commit into from
Jun 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/glbc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/backendconfig/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand Down Expand Up @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions pkg/backendconfig/backendconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
33 changes: 21 additions & 12 deletions pkg/backends/backends.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth to separate this out to another PR if we can't get this in soon..

}
if !resyncWithCloud {
backendPool.snapshotter = storage.NewInMemoryPool()
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pkg/backends/backends_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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
Expand Down
68 changes: 68 additions & 0 deletions pkg/backends/features/cdn.go
Original file line number Diff line number Diff line change
@@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is applying defaults to backendConfig but not backendService, I was confused :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we apply defaults to the BackendConfig so that in the apply[IAP,CDN]Settings functions we don't have to have ugly nil checks.

// 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{}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious is it valid to have be.EnableCDN=false but be.CdnPolicy=non-nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. If the user disabled CDN but provided cache policies, then we will act on it accordingly.

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{}
}
}
Loading