diff --git a/pkg/apis/backendconfig/v1beta1/types.go b/pkg/apis/backendconfig/v1beta1/types.go index cb1f6d1f57..896247e657 100644 --- a/pkg/apis/backendconfig/v1beta1/types.go +++ b/pkg/apis/backendconfig/v1beta1/types.go @@ -59,15 +59,19 @@ type BackendConfigList struct { // IAPConfig contains configuration for IAP-enabled backends. // +k8s:openapi-gen=true type IAPConfig struct { - Enabled bool `json:"enabled"` - ClientCredentials *OAuthClientCredentials `json:"clientCredentials,omitempty"` + Enabled bool `json:"enabled"` + OAuthClientCredentials *OAuthClientCredentials `json:"clientCredentials"` } // 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. - Secret string `json:"secret"` + SecretName string `json:"secret"` + // Direct reference to OAuth client id. + ClientID string `json:"clientID,omitempty"` + // Direct reference to OAuth client secret. + ClientSecret string `json:"clientSecret,omitempty"` } // CDNConfig contains configuration for CDN-enabled backends. diff --git a/pkg/apis/backendconfig/v1beta1/zz_generated.deepcopy.go b/pkg/apis/backendconfig/v1beta1/zz_generated.deepcopy.go index 9efd5ff4bf..a441f963cb 100644 --- a/pkg/apis/backendconfig/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/backendconfig/v1beta1/zz_generated.deepcopy.go @@ -198,8 +198,8 @@ func (in *CacheKeyPolicy) DeepCopy() *CacheKeyPolicy { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IAPConfig) DeepCopyInto(out *IAPConfig) { *out = *in - if in.ClientCredentials != nil { - in, out := &in.ClientCredentials, &out.ClientCredentials + if in.OAuthClientCredentials != nil { + in, out := &in.OAuthClientCredentials, &out.OAuthClientCredentials if *in == nil { *out = nil } else { diff --git a/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go b/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go index b1182e9318..00baec1423 100644 --- a/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go +++ b/pkg/apis/backendconfig/v1beta1/zz_generated.openapi.go @@ -194,7 +194,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA }, }, }, - Required: []string{"enabled"}, + Required: []string{"enabled", "clientCredentials"}, }, }, Dependencies: []string{ @@ -212,6 +212,20 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA Format: "", }, }, + "clientID": { + SchemaProps: spec.SchemaProps{ + Description: "Direct reference to OAuth client id.", + Type: []string{"string"}, + Format: "", + }, + }, + "clientSecret": { + SchemaProps: spec.SchemaProps{ + Description: "Direct reference to OAuth client secret.", + Type: []string{"string"}, + Format: "", + }, + }, }, Required: []string{"secret"}, }, diff --git a/pkg/backendconfig/validation.go b/pkg/backendconfig/validation.go new file mode 100644 index 0000000000..4c2da79391 --- /dev/null +++ b/pkg/backendconfig/validation.go @@ -0,0 +1,69 @@ +/* +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 backendconfig + +import ( + "fmt" + + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + backendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1" +) + +const ( + OAuthClientIDKey = "client_id" + OAuthClientSecretKey = "client_secret" +) + +func Validate(kubeClient kubernetes.Interface, beConfig *backendconfigv1beta1.BackendConfig) error { + if beConfig == nil { + return nil + } + return validateIAP(kubeClient, beConfig) +} + +// TODO(rramkumar): Return errors as constants so that the unit tests can distinguish +// between which error is returned. +func validateIAP(kubeClient kubernetes.Interface, beConfig *backendconfigv1beta1.BackendConfig) error { + // If IAP settings are not found or IAP is not enabled then don't bother continuing. + if beConfig.Spec.Iap == nil || beConfig.Spec.Iap.Enabled == false { + return nil + } + // If necessary, get the OAuth credentials stored in the K8s secret. + if beConfig.Spec.Iap.OAuthClientCredentials != nil && beConfig.Spec.Iap.OAuthClientCredentials.SecretName != "" { + secretName := beConfig.Spec.Iap.OAuthClientCredentials.SecretName + secret, err := kubeClient.Core().Secrets(beConfig.Namespace).Get(secretName, meta_v1.GetOptions{}) + if err != nil { + return fmt.Errorf("error retrieving secret %v: %v", secretName, err) + } + clientID, ok := secret.Data[OAuthClientIDKey] + if !ok { + return fmt.Errorf("secret %v missing %v data", secretName, OAuthClientIDKey) + } + clientSecret, ok := secret.Data[OAuthClientSecretKey] + if !ok { + return fmt.Errorf("secret %v missing %v data'", secretName, OAuthClientSecretKey) + } + beConfig.Spec.Iap.OAuthClientCredentials.ClientID = string(clientID) + beConfig.Spec.Iap.OAuthClientCredentials.ClientSecret = string(clientSecret) + } + + if beConfig.Spec.Cdn != nil && beConfig.Spec.Cdn.Enabled { + return fmt.Errorf("iap and cdn cannot be enabled at the same time") + } + return nil +} diff --git a/pkg/backendconfig/validation_test.go b/pkg/backendconfig/validation_test.go new file mode 100644 index 0000000000..70cdac227d --- /dev/null +++ b/pkg/backendconfig/validation_test.go @@ -0,0 +1,149 @@ +/* +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 backendconfig + +import ( + "testing" + + v1 "k8s.io/api/core/v1" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" + backendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1" +) + +var ( + beConfig = &backendconfigv1beta1.BackendConfig{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + }, + Spec: backendconfigv1beta1.BackendConfigSpec{ + Iap: &backendconfigv1beta1.IAPConfig{ + Enabled: true, + OAuthClientCredentials: &backendconfigv1beta1.OAuthClientCredentials{ + SecretName: "foo", + }, + }, + }, + } +) + +func TestValidateIAP(t *testing.T) { + testCases := []struct { + desc string + init func(kubeClient kubernetes.Interface) + expectError bool + }{ + { + desc: "secret does not exist", + init: func(kubeClient kubernetes.Interface) { + secret := &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "wrong-namespace", + Name: "foo", + }, + } + kubeClient.Core().Secrets("wrong-namespace").Create(secret) + }, + expectError: true, + }, + { + desc: "secret does not contain client_id", + init: func(kubeClient kubernetes.Interface) { + secret := &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + Name: "foo", + }, + Data: map[string][]byte{ + "client_secret": []byte("my-secret"), + }, + } + kubeClient.Core().Secrets("default").Create(secret) + }, + expectError: true, + }, + { + desc: "secret does not contain client_secret", + init: func(kubeClient kubernetes.Interface) { + secret := &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + Name: "foo", + }, + Data: map[string][]byte{ + "client_id": []byte("my-id"), + }, + } + kubeClient.Core().Secrets("default").Create(secret) + }, + expectError: true, + }, + { + desc: "validation passes", + init: func(kubeClient kubernetes.Interface) { + secret := &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + Name: "foo", + }, + Data: map[string][]byte{ + "client_id": []byte("my-id"), + "client_secret": []byte("my-secret"), + }, + } + kubeClient.Core().Secrets("default").Create(secret) + }, + expectError: false, + }, + { + desc: "iap and cdn enabled at the same time", + init: func(kubeClient kubernetes.Interface) { + // TODO(rramkumar): Don't modify in-flight. + // This works now since this is the last test in the + // list of cases. + beConfig.Spec.Cdn = &backendconfigv1beta1.CDNConfig{ + Enabled: true, + } + secret := &v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "default", + Name: "foo", + }, + Data: map[string][]byte{ + "client_id": []byte("my-id"), + "client_secret": []byte("my-secret"), + }, + } + kubeClient.Core().Secrets("default").Create(secret) + }, + expectError: true, + }, + } + + for _, testCase := range testCases { + kubeClient := fake.NewSimpleClientset() + testCase.init(kubeClient) + err := Validate(kubeClient, beConfig) + if testCase.expectError && err == nil { + t.Errorf("%v: Expected error but got nil", testCase.desc) + } + if !testCase.expectError && err != nil { + t.Errorf("%v: Did not expect error but got: %v", testCase.desc, err) + } + } +} diff --git a/pkg/context/context.go b/pkg/context/context.go index 701119c57b..1d4d1fc2ba 100644 --- a/pkg/context/context.go +++ b/pkg/context/context.go @@ -35,7 +35,7 @@ import ( // ControllerContext holds type ControllerContext struct { - kubeClient kubernetes.Interface + KubeClient kubernetes.Interface IngressInformer cache.SharedIndexInformer ServiceInformer cache.SharedIndexInformer @@ -60,7 +60,7 @@ func NewControllerContext( return cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc} } context := &ControllerContext{ - kubeClient: kubeClient, + KubeClient: kubeClient, IngressInformer: informerv1beta1.NewIngressInformer(kubeClient, namespace, resyncPeriod, newIndexer()), ServiceInformer: informerv1.NewServiceInformer(kubeClient, namespace, resyncPeriod, newIndexer()), PodInformer: informerv1.NewPodInformer(kubeClient, namespace, resyncPeriod, newIndexer()), @@ -107,7 +107,7 @@ func (ctx *ControllerContext) Recorder(ns string) record.EventRecorder { broadcaster := record.NewBroadcaster() broadcaster.StartLogging(glog.Infof) broadcaster.StartRecordingToSink(&corev1.EventSinkImpl{ - Interface: ctx.kubeClient.Core().Events(ns), + Interface: ctx.KubeClient.Core().Events(ns), }) rec := broadcaster.NewRecorder(scheme.Scheme, apiv1.EventSource{Component: "loadbalancer-controller"}) ctx.recorders[ns] = rec diff --git a/pkg/controller/errors/errors.go b/pkg/controller/errors/errors.go index d050872210..35589d595b 100644 --- a/pkg/controller/errors/errors.go +++ b/pkg/controller/errors/errors.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/ingress-gce/pkg/annotations" + backendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1beta1" "k8s.io/ingress-gce/pkg/utils" ) @@ -62,3 +63,24 @@ type ErrSvcAppProtosParsing struct { func (e ErrSvcAppProtosParsing) Error() string { return fmt.Sprintf("could not parse %v annotation on Service %v/%v, err: %v", annotations.ServiceApplicationProtocolKey, e.Svc.Namespace, e.Svc.Name, e.Err) } + +// ErrSvcBackendConfig is returned when there was an error getting the +// BackendConfig for a service port. +type ErrSvcBackendConfig struct { + utils.ServicePortID + Err error +} + +func (e ErrSvcBackendConfig) Error() string { + return fmt.Sprintf("error getting BackendConfig for port %v on service %v/%v, err: %v", e.ServicePortID.Port, e.ServicePortID.Service.Namespace, e.ServicePortID.Service.Name, e.Err) +} + +// ErrBackendConfigValidation is returned when there was an error validating a BackendConfig. +type ErrBackendConfigValidation struct { + backendconfigv1beta1.BackendConfig + Err error +} + +func (e ErrBackendConfigValidation) Error() string { + return fmt.Sprintf("BackendConfig %v/%v is not valid: %v", e.BackendConfig.Namespace, e.BackendConfig.Name, e.Err) +} diff --git a/pkg/controller/translator/translator.go b/pkg/controller/translator/translator.go index 022f6fb9fd..cf3c38c0a9 100644 --- a/pkg/controller/translator/translator.go +++ b/pkg/controller/translator/translator.go @@ -112,16 +112,17 @@ PortLoop: return nil, errors.ErrSvcNotNodePort{Service: id.Service} } - var backendConfig *backendconfigv1beta1.BackendConfig + var beConfig *backendconfigv1beta1.BackendConfig if t.ctx.BackendConfigInformer != nil { - backendConfigInStore, err := backendconfig.GetBackendConfigForServicePort(t.ctx.BackendConfigInformer.GetIndexer(), svc, port) + beConfig, err := backendconfig.GetBackendConfigForServicePort(t.ctx.BackendConfigInformer.GetIndexer(), svc, port) if err != nil { - return nil, err + return nil, errors.ErrSvcBackendConfig{ServicePortID: id, Err: err} } - if backendConfigInStore != nil { - // Object in cache could be changed in-flight. Deepcopy to - // reduce race conditions. - backendConfig = backendConfigInStore.DeepCopy() + // Object in cache could be changed in-flight. Deepcopy to + // reduce race conditions. + beConfig = beConfig.DeepCopy() + if err = backendconfig.Validate(t.ctx.KubeClient, beConfig); err != nil { + return nil, errors.ErrBackendConfigValidation{BackendConfig: *beConfig, Err: err} } } @@ -131,7 +132,7 @@ PortLoop: Protocol: proto, SvcTargetPort: port.TargetPort.String(), NEGEnabled: t.negEnabled && negEnabled, - BackendConfig: backendConfig, + BackendConfig: beConfig, }, nil }