Actually Use Authentication and Authorization (#615)

* Add flag to disable auth for testing purposes

Some of the tests run the service catalog API server without a
corresponding Kubernetes API server.  In this case, no delegating
authentication or authorization is possible.  The `--disable-auth`
flag causes the API server setup to skip setting up authentication
and authorization in these cases.

* Update Helm Charts for Auth and Aggregation

This commit updates the helm charts, inserting the requisite options
for running the service catalog behind the master's integrated
API server aggregator.

The new options are:

- `useAggregator`: causes the API server to be registered with the k8s
  API server and points the controller manager at the there instead
  of at the service catalog API server.

- `apiserver.tls.requestHeaderCA`: the CA used to authenticate requests
  from the k8s API server proxy, so that we can trust the authentication
  information that it passes us (because it handles authentication
  itself, and then passes us the extracted information).

- `apiserver.tls.ca`: the CA used to sign the serving certificates for
  the API server.  This is needed by the API registration information,
  so that the k8s API server proxy can verify that it's talking to the
  correct server.

In order to use aggregation, you must also pass
`apiserver.tls.{cert,key}`, instead of letting them be autogenerated
as self-signed certificates.

* Fix up Kubeconfig handling

This commit uncomments and fixes up the Kubeconfig handling, making
it possible to actually pass in Kubeconfigs to use to connect to
the k8s API server and the service catalog API server.

Furthermore, we explicitly try and use in-cluster config for service
catalog API server connections, avoiding a generic error message about
it "possibly" not working in favor of our own warning, which advises
viewers to make sure that the service catalog APIs are registered with
the aggregator.

* Actually die if APIs aren't available

Previously, we'd start (and otherwise appear healthy), but not
actually do anything if the service catalog APIs weren't available.

Now, the controller manager fails if the APIs aren't available, so
that we eventually get restarted.

* Renable authentication and authorization

This re-enables actually applying the effects of the authentication and authorization
options.  The API server will now attempt to delegate authentication and
authorization options to a main Kube API server.

These should work fine now that we're using the 1.6 generic API server
code.

* Ensure walkthrough requestheader certs

This PR makes sure that when running the walkthrough test,
the extensions-apiserver-authentication configmap, which is
autogenerated by the main Kubernetes API server, contains requestheader
certs.  If it doesn't, it augments the configmap with requestheader
certs matching the client certs from the cluster, since GKE does set
these (this might not necessarily be what you normally want, but works
fine for testing).

* Force jenkins to use k8s 1.6.1

This commit tells Jenkins to use a 1.6.1 cluster.  This ensures that we
always know what cluster version we're running tests against, and also
opts us in to using a 1.6 cluster (which is not the default at the time
of authorship of this PR).

Author: DirectXMan12

Jenkinsfile

@@ -108,6 +108,7 @@ node {
             --registry gcr.io/${test_project}/catalog/ \
             --version ${version} \
             --cleanup \
+            --fix-auth \
             --create-artifacts
       """
 
@@ -116,6 +117,7 @@ node {
             --version ${version} \
             --with-tpr \
             --cleanup \
+            --fix-auth \
             --create-artifacts
       """
     } catch (Exception e) {

charts/catalog/README.md

@@ -44,6 +44,8 @@ chart and their default values.
 | `apiserver.insecure`  | Whether to expose an insecure endpoint; keep this enabled because there are some outstanding problems with the TLS-secured endpoint | `true` |
 | `apiserver.tls.cert` | Base64-encoded x509 certificate | A self-signed certificate |
 | `apiserver.tls.key` | Base64-encoded private key | The private key for the certificate above |
+| `apiserver.tls.ca` | Base64-encoded CA certificate used to sign the above certificate | |
+| `apiserver.tls.requestHeaderCA` | Base64-encoded CA used to validate request-header authentication, when receiving delegated authentication from an aggregator | *none (will disable requestheader authentication)* |
 | `apiserver.service.type` | Type of service; valid values are `LoadBalancer` and `NodePort` | `NodePort` |
 | `apiserver.service.nodePort.securePort` | If service type is `NodePort`, specifies a port in allowable range (e.g. 30000 - 32767 on minikube); The TLS-enabled endpoint will be exposed here | `30443` |
 | `apiserver.service.nodePort.insecurePort` | If service type is `NodePort`, specifies a port in allowable range (e.g. 30000 - 32767 on minikube); The insecure endpoint, if enabled, will be exposed here | `30080` |
@@ -57,6 +59,7 @@ chart and their default values.
 | `controllerManager.verbosity` | Log level; valid values are in the range 0 - 10 | `10` |
 | `controllerManager.resyncInterval` | How often the controller should resync informers; duration format (`20m`, `1h`, etc) | `5m` |
 | `controllerManager.brokerRelistInterval` | How often the controller should relist the catalogs of ready brokers; duration format (`20m`, `1h`, etc) | `24h` |
+| `useAggregator` | whether or not to set up the controller-manager to go through the main Kubernetes API server's API aggregator (requires setting `apiserver.tls.ca` to work) | `false` |
 
 Specify each parameter using the `--set key=value[,key=value]` argument to
 `helm install`.

charts/catalog/templates/apiregistration.yaml

@@ -0,0 +1,14 @@
+{{- if .Values.useAggregator }}
+apiVersion: apiregistration.k8s.io/v1alpha1
+kind: APIService
+metadata:
+  name: v1alpha1.servicecatalog.k8s.io
+spec:
+  group: servicecatalog.k8s.io
+  version: v1alpha1
+  service:
+    namespace: {{ .Release.Namespace }}
+    name: {{ template "fullname" . }}-apiserver
+  caBundle: {{ .Values.apiserver.tls.ca }}
+  priority: 100
+{{ end }}

charts/catalog/templates/apiserver-cert-secret.yaml

@@ -11,3 +11,6 @@ type: Opaque
 data:
   tls.crt: {{ .Values.apiserver.tls.cert }}
   tls.key: {{ .Values.apiserver.tls.key }}
+{{- if .Values.apiserver.tls.requestHeaderCA }}
+  requestheader-ca.crt: {{ .Values.apiserver.tls.requestHeaderCA }}
+{{- end }}

charts/catalog/templates/apiserver-deployment.yaml

@@ -51,6 +51,9 @@ spec:
         {{- end }}
         - -v
         - "{{ .Values.apiserver.verbosity }}"
+        {{- if .Values.apiserver.tls.requestHeaderCA }}
+        - --requestheader-client-ca-file=/var/run/kubernetes-service-catalog/requestheader-ca.crt
+        {{- end }}
         ports:
         {{- if .Values.apiserver.insecure }}
         - containerPort: 8080
@@ -133,6 +136,10 @@ spec:
             path: apiserver.crt
           - key: tls.key
             path: apiserver.key
+          {{- if .Values.apiserver.tls.requestHeaderCA }}
+          - key: requestheader-ca.crt
+            path: requestheader-ca.crt
+          {{- end }}
       {{- if eq .Values.apiserver.storage.type "etcd" }}
       - name: etcd-data-dir
         emptyDir: {}

charts/catalog/templates/controller-manager-deployment.yaml

@@ -34,12 +34,14 @@ spec:
         args:
         - --port
         - "8080"
+        {{- if not .Values.useAggregator }}
         - --service-catalog-api-server-url
         {{- if .Values.apiserver.insecure }}
         - http://{{ template "fullname" . }}-apiserver
         {{- else }}
         - https://{{ template "fullname" . }}-apiserver
         {{- end }}
+        {{- end }}
         - -v
         - "{{ .Values.controllerManager.verbosity }}"
         - --resync-interval

charts/catalog/values.yaml

@@ -56,3 +56,4 @@ controllerManager:
   resyncInterval: 5m
   # Broker relist interval; format is a duration (`20m`, `1h`, etc)
   brokerRelistInterval: 24h
+useAggregator: false

cmd/apiserver/app/server/options.go

@@ -41,7 +41,9 @@ type ServiceCatalogServerOptions struct {
 	EtcdOptions *EtcdOptions
 	// TPROptions are options for serving with TPR as the backing store
 	TPROptions *TPROptions
-	StopCh     <-chan struct{}
+	// DisableAuth disables delegating authentication and authorization for testing scenarios
+	DisableAuth bool
+	StopCh      <-chan struct{}
 }
 
 func (s *ServiceCatalogServerOptions) addFlags(flags *pflag.FlagSet) {
@@ -52,6 +54,13 @@ func (s *ServiceCatalogServerOptions) addFlags(flags *pflag.FlagSet) {
 		"The type of backing storage this API server should use",
 	)
 
+	flags.BoolVar(
+		&s.DisableAuth,
+		"disable-auth",
+		false,
+		"Disable authentication and authorization for testing purposes",
+	)
+
 	s.GenericServerRunOptions.AddUniversalFlags(flags)
 	s.SecureServingOptions.AddFlags(flags)
 	s.AuthenticationOptions.AddFlags(flags)

cmd/apiserver/app/server/util.go

@@ -17,6 +17,8 @@ limitations under the License.
 package server
 
 import (
+	"github.com/golang/glog"
+
 	genericapiserver "k8s.io/apiserver/pkg/server"
 )
 
@@ -43,22 +45,18 @@ func setupBasicServer(s *ServiceCatalogServerOptions) (*genericapiserver.Config,
 		return nil, err
 	}
 
-	// glog.V(4).Info("Setting up authn (disabled)")
-	// need to figure out what's throwing the `missing clientCA file` err
-	/*
-		if _, err := genericConfig.ApplyDelegatingAuthenticationOptions(serverOptions.AuthenticationOptions); err != nil {
-			glog.Infoln(err)
-			return err
+	if !s.DisableAuth {
+		if err := s.AuthenticationOptions.ApplyTo(genericConfig); err != nil {
+			return nil, err
 		}
-	*/
 
-	// glog.V(4).Infoln("Setting up authz (disabled)")
-	// having this enabled causes the server to crash for any call
-	/*
-		if _, err := genericConfig.ApplyDelegatingAuthorizationOptions(serverOptions.AuthorizationOptions); err != nil {
-			glog.Infoln(err)
-			return err
+		if err := s.AuthorizationOptions.ApplyTo(genericConfig); err != nil {
+			return nil, err
 		}
-	*/
+	} else {
+		// always warn when auth is disabled, since this should only be used for testing
+		glog.Infof("Authentication and authorization disabled for testing purposes")
+	}
+
 	return genericConfig, nil
 }

cmd/controller-manager/app/controller_manager.go

@@ -90,19 +90,20 @@ func Run(controllerManagerOptions *options.ControllerManagerServer) error {
 	// Build the K8s kubeconfig / client / clientBuilder
 	glog.V(4).Info("Building k8s kubeconfig")
 
-	k8sKubeconfig, err := rest.InClusterConfig()
+	var err error
+	var k8sKubeconfig *rest.Config
+	if controllerManagerOptions.K8sAPIServerURL == "" && controllerManagerOptions.K8sKubeconfigPath == "" {
+		k8sKubeconfig, err = rest.InClusterConfig()
+	} else {
+		k8sKubeconfig, err = clientcmd.BuildConfigFromFlags(
+			controllerManagerOptions.K8sAPIServerURL,
+			controllerManagerOptions.K8sKubeconfigPath)
+	}
 	if err != nil {
-		glog.Fatalf("Failed to get kube client config (%s)", err)
+		return fmt.Errorf("failed to get Kubernetes client config: %v", err)
 	}
 	k8sKubeconfig.GroupVersion = &schema.GroupVersion{}
 
-	// k8sKubeconfig, err := clientcmd.BuildConfigFromFlags(
-	// 	controllerManagerOptions.K8sAPIServerURL,
-	// 	controllerManagerOptions.K8sKubeconfigPath)
-	// if err != nil {
-	// 	// TODO: disambiguate API errors
-	// 	return err
-	// }
 	k8sKubeconfig.ContentConfig.ContentType = controllerManagerOptions.ContentType
 	// Override kubeconfig qps/burst settings from flags
 	k8sKubeconfig.QPS = controllerManagerOptions.KubeAPIQPS
@@ -111,21 +112,30 @@ func Run(controllerManagerOptions *options.ControllerManagerServer) error {
 		rest.AddUserAgent(k8sKubeconfig, controllerManagerAgentName),
 	)
 	if err != nil {
-		glog.Fatalf("Invalid k8s API configuration: %v", err)
+		return fmt.Errorf("invalid Kubernetes API configuration: %v", err)
 	}
 
 	glog.V(4).Infof("Building service-catalog kubeconfig for url: %v\n", controllerManagerOptions.ServiceCatalogAPIServerURL)
+
+	var serviceCatalogKubeconfig *rest.Config
 	// Build the service-catalog kubeconfig / clientBuilder
-	serviceCatalogKubeconfig, err := clientcmd.BuildConfigFromFlags(
-		controllerManagerOptions.ServiceCatalogAPIServerURL,
-		// controllerManagerOptions.ServiceCatalogKubeconfigPath,
-		"", // TODO: tolerate missing kubeconfig
-	)
+	if controllerManagerOptions.ServiceCatalogAPIServerURL == "" && controllerManagerOptions.ServiceCatalogKubeconfigPath == "" {
+		// explicitly fall back to InClusterConfig, assuming we're talking to an API server which does aggregation
+		// (BuildConfigFromFlags does this, but gives a more generic warning message than we do here)
+		glog.V(4).Infof("Using inClusterConfig to talk to service catalog API server -- make sure your API server is registered with the aggregator")
+		serviceCatalogKubeconfig, err = rest.InClusterConfig()
+	} else {
+		serviceCatalogKubeconfig, err = clientcmd.BuildConfigFromFlags(
+			controllerManagerOptions.ServiceCatalogAPIServerURL,
+			controllerManagerOptions.ServiceCatalogKubeconfigPath)
+	}
 	if err != nil {
 		// TODO: disambiguate API errors
-		return err
+		return fmt.Errorf("failed to get Service Catalog client configuration: %v", err)
 	}
 
+	// due to using both k8s.io/kubernetes and k8s.io/client-go, we need to convert this object over
+
 	glog.V(4).Info("Starting http server and mux")
 	// Start http server and handlers
 	go func() {
@@ -306,7 +316,7 @@ func StartControllers(s *options.ControllerManagerServer,
 		glog.V(1).Info("Starting shared informers")
 		informerFactory.Start(stop)
 	} else {
-		glog.V(1).Infof("Skipping starting service-catalog controller because servicecatalog/v1alpha1 is not available")
+		return fmt.Errorf("unable to start service-catalog controller: servicecatalog/v1alpha1 is not available")
 	}
 
 	select {}