Merge pull request #18508 from deads2k/controller-04-satoken

Automatic merge from submit-queue (batch tested with PRs 18591, 18508).

use the upstream SA token controller

This patches in our service serving cert CA initialization in the upstream SA token controller when requested.  This makes it easier to run the upstream controllers in 3.10 and doing it in 3.9 makes for a smooth transition on the leases.

Author: openshift-merge-robot

pkg/cmd/server/origin/controller/config.go

@@ -1,20 +1,15 @@
 package controller
 
 import (
-	"fmt"
-	"io/ioutil"
 	"path"
 	"time"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/client-go/util/cert"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	kapi "k8s.io/kubernetes/pkg/apis/core"
-	kcontroller "k8s.io/kubernetes/pkg/controller"
 	serviceaccountadmission "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount"
 
 	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
-	"github.com/openshift/origin/pkg/cmd/server/crypto"
 	"github.com/openshift/origin/pkg/cmd/util/variable"
 )
 
@@ -56,8 +51,6 @@ func getOpenShiftClientEnvVars(options configapi.MasterConfig) ([]kapi.EnvVar, e
 // OpenshiftControllerConfig is the runtime (non-serializable) config object used to
 // launch the set of openshift (not kube) controllers.
 type OpenshiftControllerConfig struct {
-	ServiceAccountTokenControllerOptions ServiceAccountTokenControllerOptions
-
 	ServiceAccountControllerOptions ServiceAccountControllerOptions
 
 	BuildControllerConfig BuildControllerConfig
@@ -115,63 +108,10 @@ func (c *OpenshiftControllerConfig) GetControllerInitializers() (map[string]Init
 	return ret, nil
 }
 
-// NewOpenShiftControllerPreStartInitializers returns list of initializers for controllers
-// that needed to be run before any other controller is started.
-// Typically this has to done for the serviceaccount-token controller as it provides
-// tokens to other controllers.
-func (c *OpenshiftControllerConfig) ServiceAccountContentControllerInit() InitFunc {
-	return c.ServiceAccountTokenControllerOptions.RunController
-}
-
 func BuildOpenshiftControllerConfig(options configapi.MasterConfig) (*OpenshiftControllerConfig, error) {
 	var err error
 	ret := &OpenshiftControllerConfig{}
 
-	_, loopbackClientConfig, err := configapi.GetInternalKubeClient(options.MasterClients.OpenShiftLoopbackKubeConfig, options.MasterClients.OpenShiftLoopbackClientConnectionOverrides)
-	if err != nil {
-		return nil, err
-	}
-
-	ret.ServiceAccountTokenControllerOptions = ServiceAccountTokenControllerOptions{
-		RootClientBuilder: kcontroller.SimpleControllerClientBuilder{
-			ClientConfig: loopbackClientConfig,
-		},
-	}
-	if len(options.ServiceAccountConfig.PrivateKeyFile) > 0 {
-		ret.ServiceAccountTokenControllerOptions.PrivateKey, err = cert.PrivateKeyFromFile(options.ServiceAccountConfig.PrivateKeyFile)
-		if err != nil {
-			return nil, fmt.Errorf("error reading signing key for Service Account Token Manager: %v", err)
-		}
-	}
-	if len(options.ServiceAccountConfig.MasterCA) > 0 {
-		ret.ServiceAccountTokenControllerOptions.RootCA, err = ioutil.ReadFile(options.ServiceAccountConfig.MasterCA)
-		if err != nil {
-			return nil, fmt.Errorf("error reading master ca file for Service Account Token Manager: %s: %v", options.ServiceAccountConfig.MasterCA, err)
-		}
-		if _, err := cert.ParseCertsPEM(ret.ServiceAccountTokenControllerOptions.RootCA); err != nil {
-			return nil, fmt.Errorf("error parsing master ca file for Service Account Token Manager: %s: %v", options.ServiceAccountConfig.MasterCA, err)
-		}
-	}
-	if options.ControllerConfig.ServiceServingCert.Signer != nil && len(options.ControllerConfig.ServiceServingCert.Signer.CertFile) > 0 {
-		certFile := options.ControllerConfig.ServiceServingCert.Signer.CertFile
-		serviceServingCA, err := ioutil.ReadFile(certFile)
-		if err != nil {
-			return nil, fmt.Errorf("error reading ca file for Service Serving Certificate Signer: %s: %v", certFile, err)
-		}
-		if _, err := crypto.CertsFromPEM(serviceServingCA); err != nil {
-			return nil, fmt.Errorf("error parsing ca file for Service Serving Certificate Signer: %s: %v", certFile, err)
-		}
-
-		// if we have a rootCA bundle add that too.  The rootCA will be used when hitting the default master service, since those are signed
-		// using a different CA by default.  The rootCA's key is more closely guarded than ours and if it is compromised, that power could
-		// be used to change the trusted signers for every pod anyway, so we're already effectively trusting it.
-		if len(ret.ServiceAccountTokenControllerOptions.RootCA) > 0 {
-			ret.ServiceAccountTokenControllerOptions.ServiceServingCA = append(ret.ServiceAccountTokenControllerOptions.ServiceServingCA, ret.ServiceAccountTokenControllerOptions.RootCA...)
-			ret.ServiceAccountTokenControllerOptions.ServiceServingCA = append(ret.ServiceAccountTokenControllerOptions.ServiceServingCA, []byte("\n")...)
-		}
-		ret.ServiceAccountTokenControllerOptions.ServiceServingCA = append(ret.ServiceAccountTokenControllerOptions.ServiceServingCA, serviceServingCA...)
-	}
-
 	ret.ServiceAccountControllerOptions = ServiceAccountControllerOptions{
 		ManagedNames: options.ServiceAccountConfig.ManagedNames,
 	}

pkg/cmd/server/origin/controller/serviceaccount.go

@@ -4,9 +4,7 @@ import (
 	"github.com/golang/glog"
 
 	kapiv1 "k8s.io/api/core/v1"
-	"k8s.io/kubernetes/pkg/controller"
 	sacontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
-	"k8s.io/kubernetes/pkg/serviceaccount"
 
 	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
 	serviceaccountcontrollers "github.com/openshift/origin/pkg/serviceaccounts/controllers"
@@ -49,37 +47,6 @@ func (c *ServiceAccountControllerOptions) RunController(ctx ControllerContext) (
 	return true, nil
 }
 
-type ServiceAccountTokenControllerOptions struct {
-	RootCA           []byte
-	ServiceServingCA []byte
-	PrivateKey       interface{}
-
-	RootClientBuilder controller.SimpleControllerClientBuilder
-}
-
-func (c *ServiceAccountTokenControllerOptions) RunController(ctx ControllerContext) (bool, error) {
-	if c.PrivateKey == nil {
-		glog.Infof("Skipped starting Service Account Token Manager, no private key specified")
-		return false, nil
-	}
-
-	controller, err := sacontroller.NewTokensController(
-		ctx.ExternalKubeInformers.Core().V1().ServiceAccounts(),
-		ctx.ExternalKubeInformers.Core().V1().Secrets(),
-		c.RootClientBuilder.ClientOrDie(bootstrappolicy.InfraServiceAccountTokensControllerServiceAccountName),
-		sacontroller.TokensControllerOptions{
-			TokenGenerator:   serviceaccount.JWTTokenGenerator(c.PrivateKey),
-			RootCA:           c.RootCA,
-			ServiceServingCA: c.ServiceServingCA,
-		},
-	)
-	if err != nil {
-		return true, nil
-	}
-	go controller.Run(int(ctx.OpenshiftControllerOptions.ServiceAccountTokenOptions.ConcurrentSyncs), ctx.Stop)
-	return true, nil
-}
-
 func RunServiceAccountPullSecretsController(ctx ControllerContext) (bool, error) {
 	kc := ctx.ClientBuilder.ClientOrDie(bootstrappolicy.InfraServiceAccountPullSecretsControllerServiceAccountName)
 

pkg/cmd/server/start/start_kube_controller_manager.go

@@ -21,8 +21,6 @@ func computeKubeControllerManagerArgs(kubeconfigFile, saPrivateKeyFile, saRootCA
 			"-tokencleaner",
 			// we have to configure this separately until it is generic
 			"-horizontalpodautoscaling",
-			// we carry patches on this. For now....
-			"-serviceaccount-token",
 		}
 	}
 	if _, ok := cmdLineArgs["service-account-private-key-file"]; !ok {

pkg/cmd/server/start/start_master.go

@@ -6,12 +6,10 @@ import (
 	"io"
 	"io/ioutil"
 	"net"
-	"net/http"
 	"os"
 	"path"
 	"path/filepath"
 	"strings"
-	"time"
 
 	"github.com/coreos/go-systemd/daemon"
 	"github.com/golang/glog"
@@ -21,7 +19,6 @@ import (
 
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/apimachinery/pkg/util/wait"
 	utilwait "k8s.io/apimachinery/pkg/util/wait"
 	clientgoclientset "k8s.io/client-go/kubernetes"
 	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -431,19 +428,34 @@ func (m *Master) Start() error {
 		go runEmbeddedScheduler(m.config.MasterClients.OpenShiftLoopbackKubeConfig, m.config.KubernetesMasterConfig.SchedulerConfigFile, m.config.KubernetesMasterConfig.SchedulerArguments)
 
 		go func() {
-			kubeControllerConfigBytes, err := configapilatest.WriteYAML(m.config)
+			kubeControllerConfigShallowCopy := *m.config
+			// this creates using 0700
+			kubeControllerConfigDir, err := ioutil.TempDir("", "openshift-kube-controller-manager-config-")
 			if err != nil {
 				glog.Fatal(err)
 			}
-			// this creates using 0600
-			kubeControllerConfigFile, err := ioutil.TempFile("", "openshift-kube-controler-manager-config.yaml")
+			defer func() {
+				os.RemoveAll(kubeControllerConfigDir)
+			}()
+			if m.config.ControllerConfig.ServiceServingCert.Signer != nil && len(m.config.ControllerConfig.ServiceServingCert.Signer.CertFile) > 0 {
+				caBytes, err := ioutil.ReadFile(m.config.ControllerConfig.ServiceServingCert.Signer.CertFile)
+				if err != nil {
+					glog.Fatal(err)
+				}
+				serviceServingCertSignerCAFile := path.Join(kubeControllerConfigDir, "service-signer.crt")
+				if err := ioutil.WriteFile(serviceServingCertSignerCAFile, caBytes, 0644); err != nil {
+					glog.Fatal(err)
+				}
+
+				// we need to tweak the master config file with a relative ref, but to do that we need to copy it
+				kubeControllerConfigShallowCopy.ControllerConfig.ServiceServingCert.Signer = &configapi.CertInfo{CertFile: "service-signer.crt"}
+			}
+			kubeControllerConfigBytes, err := configapilatest.WriteYAML(&kubeControllerConfigShallowCopy)
 			if err != nil {
 				glog.Fatal(err)
 			}
-			defer func() {
-				os.Remove(kubeControllerConfigFile.Name())
-			}()
-			if err := ioutil.WriteFile(kubeControllerConfigFile.Name(), kubeControllerConfigBytes, 0644); err != nil {
+			masterConfigFile := path.Join(kubeControllerConfigDir, "master-config.yaml")
+			if err := ioutil.WriteFile(masterConfigFile, kubeControllerConfigBytes, 0644); err != nil {
 				glog.Fatal(err)
 			}
 
@@ -452,7 +464,7 @@ func (m *Master) Start() error {
 				m.config.ServiceAccountConfig.PrivateKeyFile,
 				m.config.ServiceAccountConfig.MasterCA,
 				m.config.KubernetesMasterConfig.PodEvictionTimeout,
-				kubeControllerConfigFile.Name(),
+				masterConfigFile,
 				m.config.VolumeConfig.DynamicProvisioningEnabled,
 			)
 		}()
@@ -602,40 +614,6 @@ func startControllers(options configapi.MasterConfig, allocationController origi
 		return err
 	}
 
-	// We need to start the serviceaccount-tokens controller first as it provides token
-	// generation for other controllers.
-	startSATokenController := openshiftControllerConfig.ServiceAccountContentControllerInit()
-	if enabled, err := startSATokenController(controllerContext); err != nil {
-		return fmt.Errorf("Error starting serviceaccount-token controller: %v", err)
-	} else if !enabled {
-		glog.Warningf("Skipping serviceaccount-token controller")
-	} else {
-		glog.Infof("Started serviceaccount-token controller")
-	}
-
-	// The service account controllers require informers in order to create service account tokens
-	// for other controllers, which means we need to start their informers (which use the privileged
-	// loopback client) before the other controllers will run.
-	controllerContext.ExternalKubeInformers.Start(controllerContext.Stop)
-
-	// right now we have controllers which are relying on the ability to make requests before the bootstrap policy is in place
-	// In 3.7, we will be fixed by the post start hook that prevents readiness unless policy is in place
-	// for 3.6, just make sure we don't proceed until the garbage collector can hit discovery
-	// wait for bootstrap permissions to be established.  This check isn't perfect, but it ensures that at least the controllers checking discovery can succeed
-	gcClientset := controllerContext.ClientBuilder.ClientOrDie("generic-garbage-collector")
-	err = wait.PollImmediate(500*time.Millisecond, 30*time.Second, func() (bool, error) {
-		result := gcClientset.Discovery().RESTClient().Get().AbsPath("/apis").Do()
-		var statusCode int
-		result.StatusCode(&statusCode)
-		if statusCode >= http.StatusOK && statusCode < http.StatusMultipleChoices {
-			return true, nil
-		}
-		return false, nil
-	})
-	if err != nil {
-		return err
-	}
-
 	//  the service account passed for the recyclable volume plugins needs to exist.  We want to do this via the init function, but its a kube init function
 	// for the rebase, create that service account here
 	// TODO make this a lot cleaner

test/util/server/server.go

@@ -424,7 +424,7 @@ func StartConfiguredMasterAPI(masterConfig *configapi.MasterConfig) (string, err
 	if masterConfig.KubernetesMasterConfig.ControllerArguments == nil {
 		masterConfig.KubernetesMasterConfig.ControllerArguments = map[string][]string{}
 	}
-	masterConfig.KubernetesMasterConfig.ControllerArguments["controllers"] = append(masterConfig.KubernetesMasterConfig.ControllerArguments["controllers"], "clusterrole-aggregation")
+	masterConfig.KubernetesMasterConfig.ControllerArguments["controllers"] = append(masterConfig.KubernetesMasterConfig.ControllerArguments["controllers"], "serviceaccount-token", "clusterrole-aggregation")
 
 	return StartConfiguredMasterWithOptions(masterConfig)
 }