Add custom validation for BackendConfig + hook validation into Transl…

…ator

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.

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
+}

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)
+		}
+	}
+}

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

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)
+}

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
 }