Envtest secure serving by default, 1.20 flags

This introduces secure serving by default, transparently switching
configs to connect to secure endpoints.  Similarly, the insecure
endpoints are now disabled by default -- they must be explicitly
disabled if you want to use them (note that this will only work on API
servers <=1.19).

This also turns on flags around the tokenrequest endpoint which are
required to be on in 1.20, and have been available for all the versions
that we currently support (back to 1.16).

There's a whole new set of control plane APIs for provisioning users,
and the old "just get a single account" APIs of the internal package are
mostly deprecated.

The default `Environment.Config` is *NOT* deprecated however -- this is
intended to continue working as normal in order to avoid breaking
everyone and to keep things in envtest working easily -- most tests will
be fine with an admin user.

For users that want RBAC, individual users may be provisioned with the
ControlPlane.AddUser method.

Author: DirectXMan12

.golangci.yml

@@ -24,7 +24,6 @@ linters:
   - deadcode
   - errcheck
   - varcheck
-  - goconst
   - unparam
   - ineffassign
   - nakedret
@@ -33,3 +32,5 @@ linters:
   - dupl
   - goimports
   - golint
+  # disabled:
+  # - goconst is overly aggressive

examples/scratch-env/go.mod

@@ -4,7 +4,7 @@ go 1.15
 
 require (
 	github.com/spf13/pflag v1.0.5
-	k8s.io/client-go v0.19.2
+	k8s.io/client-go v0.21.0-beta.1
 	sigs.k8s.io/controller-runtime v0.0.0-00010101000000-000000000000
 )
 

examples/scratch-env/go.sum

@@ -2,15 +2,11 @@ package main
 
 import (
 	goflag "flag"
-	"fmt"
-	"io"
 	"io/ioutil"
 	"os"
 
 	flag "github.com/spf13/pflag"
 
-	"k8s.io/client-go/tools/clientcmd"
-	kcapi "k8s.io/client-go/tools/clientcmd/api"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -22,25 +18,6 @@ var (
 	attachControlPlaneOut = flag.Bool("debug-env", false, "attach to test env (apiserver & etcd) output -- just a convinience flag to force KUBEBUILDER_ATTACH_CONTROL_PLANE_OUTPUT=true")
 )
 
-func writeKubeConfig(kubeConfig *kcapi.Config, kubeconfigFile *os.File) error {
-	defer kubeconfigFile.Close()
-
-	contents, err := clientcmd.Write(*kubeConfig)
-	if err != nil {
-		return fmt.Errorf("unable to serialize kubeconfig file: %w", err)
-	}
-
-	amt, err := kubeconfigFile.Write(contents)
-	if err != nil {
-		return fmt.Errorf("unable to write kubeconfig file: %w", err)
-	}
-	if amt != len(contents) {
-		fmt.Errorf("unable to write all of the kubeconfig file: %w", io.ErrShortWrite)
-	}
-
-	return nil
-}
-
 // have a separate function so we can return an exit code w/o skipping defers
 func runMain() int {
 	loggerOpts := &zap.Options{
@@ -68,41 +45,50 @@ func runMain() int {
 	cfg, err := env.Start()
 	if err != nil {
 		log.Error(err, "unable to start the test environment")
+		// shut down the environment in case we started it and failed while
+		// installing CRDs or provisioning users.
+		if err := env.Stop(); err != nil {
+			log.Error(err, "unable to stop the test environment after an error (this might be expected, but just though you should know)")
+		}
 		return 1
 	}
 
 	log.Info("apiserver running", "host", cfg.Host)
 
+	// NB(directxman12): this group is unfortunately named, but various
+	// kubernetes versions require us to use it to get "admin" access.
+	user, err := env.ControlPlane.AddUser(envtest.User{
+		Name:   "envtest-admin",
+		Groups: []string{"system:masters"},
+	}, nil)
+	if err != nil {
+		log.Error(err, "unable to provision admin user, continuing on without it")
+		return 1
+	}
+
 	// TODO(directxman12): add support for writing to a new context in an existing file
 	kubeconfigFile, err := ioutil.TempFile("", "scratch-env-kubeconfig-")
 	if err != nil {
 		log.Error(err, "unable to create kubeconfig file, continuing on without it")
-	} else {
-		defer os.Remove(kubeconfigFile.Name())
-
-		log := log.WithValues("path", kubeconfigFile.Name())
-		log.V(1).Info("Writing kubeconfig")
-
-		// TODO(directxman12): this config isn't quite fully specified, but I
-		// think it's the best we can do for now -- I don't see any obvious
-		// "rest.Config --> clientcmdapi.Config" helper
-		kubeConfig := kcapi.NewConfig()
-		kubeConfig.Clusters["scratch-env"] = &kcapi.Cluster{
-			Server: fmt.Sprintf("http://%s", cfg.Host),
-		}
-		kcCtx := kcapi.NewContext()
-		kcCtx.Cluster = "scratch-env"
-		kubeConfig.Contexts["scratch-env"] = kcCtx
-		kubeConfig.CurrentContext = "scratch-env"
-
-		if err := writeKubeConfig(kubeConfig, kubeconfigFile); err != nil {
-			log.Error(err, "unable to save kubeconfig")
-			return 1
-		}
+		return 1
+	}
+	defer os.Remove(kubeconfigFile.Name())
 
-		log.Info("Wrote kubeconfig")
+	log := log.WithValues("path", kubeconfigFile.Name())
+	log.V(1).Info("Writing kubeconfig")
+
+	kubeConfig, err := user.KubeConfig()
+	if err != nil {
+		log.Error(err, "unable to create kubeconfig")
+	}
+
+	if _, err := kubeconfigFile.Write(kubeConfig); err != nil {
+		log.Error(err, "unable to save kubeconfig")
+		return 1
 	}
 
+	log.Info("Wrote kubeconfig")
+
 	if opts := env.WebhookInstallOptions; opts.LocalServingPort != 0 {
 		log.Info("webhooks configured for", "host", opts.LocalServingHost, "port", opts.LocalServingPort, "dir", opts.LocalServingCertDir)
 	}

examples/scratch-env/main.go

@@ -52,8 +52,14 @@ var _ = BeforeSuite(func(done Done) {
 	cfg, err = testenv.Start()
 	Expect(err).NotTo(HaveOccurred())
 
-	clientTransport = &http.Transport{}
-	cfg.Transport = clientTransport
+	cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
+		// NB(directxman12): we can't set Transport *and* use TLS options,
+		// so we grab the transport right after it gets created so that we can
+		// type-assert on it (hopefully)?
+		// hopefully this doesn't break 🤞
+		clientTransport = rt.(*http.Transport)
+		return rt
+	}
 
 	clientset, err = kubernetes.NewForConfig(cfg)
 	Expect(err).NotTo(HaveOccurred())

pkg/cluster/cluster_suite_test.go

@@ -68,8 +68,14 @@ var _ = BeforeSuite(func(done Done) {
 	cfg, err = testenv.Start()
 	Expect(err).NotTo(HaveOccurred())
 
-	clientTransport = &http.Transport{}
-	cfg.Transport = clientTransport
+	cfg.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
+		// NB(directxman12): we can't set Transport *and* use TLS options,
+		// so we grab the transport right after it gets created so that we can
+		// type-assert on it (hopefully)?
+		// hopefully this doesn't break 🤞
+		clientTransport = rt.(*http.Transport)
+		return rt
+	}
 
 	clientset, err = kubernetes.NewForConfig(cfg)
 	Expect(err).NotTo(HaveOccurred())

pkg/controller/controller_suite_test.go

@@ -95,7 +95,7 @@ func InstallCRDs(config *rest.Config, options CRDInstallOptions) ([]client.Objec
 
 	// Read the CRD yamls into options.CRDs
 	if err := readCRDFiles(&options); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to read CRD files: %w", err)
 	}
 
 	if err := modifyConversionWebhooks(options.CRDs, options.Scheme, options.WebhookOptions); err != nil {
@@ -104,12 +104,12 @@ func InstallCRDs(config *rest.Config, options CRDInstallOptions) ([]client.Objec
 
 	// Create the CRDs in the apiserver
 	if err := CreateCRDs(config, options.CRDs); err != nil {
-		return options.CRDs, err
+		return options.CRDs, fmt.Errorf("unable to create CRD instances: %w", err)
 	}
 
 	// Wait for the CRDs to appear as Resources in the apiserver
 	if err := WaitForCRDs(config, options.CRDs, options); err != nil {
-		return options.CRDs, err
+		return options.CRDs, fmt.Errorf("something went wrong waiting for CRDs to appear as API resources: %w", err)
 	}
 
 	return options.CRDs, nil
@@ -281,7 +281,7 @@ func UninstallCRDs(config *rest.Config, options CRDInstallOptions) error {
 func CreateCRDs(config *rest.Config, crds []client.Object) error {
 	cs, err := client.New(config, client.Options{})
 	if err != nil {
-		return err
+		return fmt.Errorf("unable to create client: %w", err)
 	}
 
 	// Create each CRD
@@ -292,10 +292,10 @@ func CreateCRDs(config *rest.Config, crds []client.Object) error {
 		switch {
 		case apierrors.IsNotFound(err):
 			if err := cs.Create(context.TODO(), crd); err != nil {
-				return err
+				return fmt.Errorf("unable to create CRD %q: %w", crd.GetName(), err)
 			}
 		case err != nil:
-			return err
+			return fmt.Errorf("unable to get CRD %q to check if it exists: %w", crd.GetName(), err)
 		default:
 			log.V(1).Info("CRD already exists, updating", "crd", crd.GetName())
 			if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {

pkg/envtest/crd.go

@@ -91,6 +91,12 @@ type APIServer = controlplane.APIServer
 // Etcd is the re-exported Etcd type from the internal testing package
 type Etcd = controlplane.Etcd
 
+// User represents a Kubernetes user to provision for auth purposes.
+type User = controlplane.User
+
+// AuthenticatedUser represets a Kubernetes user that's been provisioned.
+type AuthenticatedUser = controlplane.AuthenticatedUser
+
 // Environment creates a Kubernetes test environment that will start / stop the Kubernetes control plane and
 // install extension APIs
 type Environment struct {
@@ -181,27 +187,6 @@ func (te *Environment) Stop() error {
 	return te.ControlPlane.Stop()
 }
 
-// getAPIServerFlags returns flags to be used with the Kubernetes API server.
-// it returns empty slice for api server defined defaults to be applied if no args specified
-func (te Environment) getAPIServerFlags() []string {
-	// Set default API server flags if not set.
-	if len(te.KubeAPIServerFlags) == 0 {
-		return []string{}
-	}
-	// Check KubeAPIServerFlags contains service-cluster-ip-range, if not, set default value to service-cluster-ip-range
-	containServiceClusterIPRange := false
-	for _, flag := range te.KubeAPIServerFlags {
-		if strings.Contains(flag, "service-cluster-ip-range") {
-			containServiceClusterIPRange = true
-			break
-		}
-	}
-	if !containServiceClusterIPRange {
-		te.KubeAPIServerFlags = append(te.KubeAPIServerFlags, "--service-cluster-ip-range=10.0.0.0/24")
-	}
-	return te.KubeAPIServerFlags
-}
-
 // Start starts a local Kubernetes server and updates te.ApiserverPort with the port it is listening on
 func (te *Environment) Start() (*rest.Config, error) {
 	if te.useExistingCluster() {
@@ -214,12 +199,17 @@ func (te *Environment) Start() (*rest.Config, error) {
 			var err error
 			te.Config, err = config.GetConfig()
 			if err != nil {
-				return nil, err
+				return nil, fmt.Errorf("unable to get configuration for existing cluster: %w", err)
 			}
 		}
 	} else {
 		if te.ControlPlane.APIServer == nil {
-			te.ControlPlane.APIServer = &controlplane.APIServer{Args: te.getAPIServerFlags()}
+			te.ControlPlane.APIServer = &controlplane.APIServer{
+				// NB(directxman12): we still pass these in so that things work if the
+				// user manually specifies them, but in most cases we expect them to
+				// be nil so that we use the new .Configure() logic.
+				Args: te.KubeAPIServerFlags,
+			}
 		}
 		if te.ControlPlane.Etcd == nil {
 			te.ControlPlane.Etcd = &controlplane.Etcd{}
@@ -249,8 +239,9 @@ func (te *Environment) Start() (*rest.Config, error) {
 		}
 		if os.Getenv(envKubectlBin) == "" {
 			// we can't just set the path manually (it's behind a function), so set the environment variable instead
+			// TODO(directxman12): re-evaluate this post pkg/internal/testing refactor
 			if err := os.Setenv(envKubectlBin, te.getBinAssetPath("kubectl")); err != nil {
-				return nil, err
+				return nil, fmt.Errorf("unable to override kubectl environment path: %w", err)
 			}
 		}
 
@@ -264,16 +255,22 @@ func (te *Environment) Start() (*rest.Config, error) {
 
 		log.V(1).Info("starting control plane", "api server flags", te.ControlPlane.APIServer.Args)
 		if err := te.startControlPlane(); err != nil {
-			return nil, err
+			return nil, fmt.Errorf("unable to start control plane itself: %w", err)
 		}
 
 		// Create the *rest.Config for creating new clients
-		te.Config = &rest.Config{
-			Host: te.ControlPlane.APIURL().Host,
+		baseConfig := &rest.Config{
 			// gotta go fast during tests -- we don't really care about overwhelming our test API server
 			QPS:   1000.0,
 			Burst: 2000.0,
 		}
+
+		adminInfo := User{Name: "admin", Groups: []string{"system:masters"}}
+		adminUser, err := te.ControlPlane.AddUser(adminInfo, baseConfig)
+		if err != nil {
+			return te.Config, fmt.Errorf("unable to provision admin user: %w", err)
+		}
+		te.Config = adminUser.Config()
 	}
 
 	// Set the default scheme if nil.
@@ -294,13 +291,13 @@ func (te *Environment) Start() (*rest.Config, error) {
 	te.CRDInstallOptions.WebhookOptions = te.WebhookInstallOptions
 	crds, err := InstallCRDs(te.Config, te.CRDInstallOptions)
 	if err != nil {
-		return te.Config, err
+		return te.Config, fmt.Errorf("unable to install CRDs onto control plane: %w", err)
 	}
 	te.CRDs = crds
 
 	log.V(1).Info("installing webhooks")
 	if err := te.WebhookInstallOptions.Install(te.Config); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to install webhooks onto control plane: %w", err)
 	}
 	return te.Config, nil
 }

pkg/envtest/server.go

@@ -158,6 +158,26 @@ func (c *TinyCA) NewServingCert(names ...string) (CertPair, error) {
 	})
 }
 
+// ClientInfo describes some Kubernetes user for the purposes of creating
+// client certificates.
+type ClientInfo struct {
+	// Name is the user name (embedded as the cert's CommonName)
+	Name string
+	// Groups are the groups to which this user belongs (embedded as the cert's
+	// Organization)
+	Groups []string
+}
+
+// NewClientCert produces a new CertPair suitable for use with Kubernetes
+// client cert auth with an API server validating based on this CA.
+func (c *TinyCA) NewClientCert(user ClientInfo) (CertPair, error) {
+	return c.makeCert(certutil.Config{
+		CommonName:   user.Name,
+		Organization: user.Groups,
+		Usages:       []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+	})
+}
+
 func resolveNames(names []string) ([]string, []net.IP, error) {
 	dnsNames := []string{}
 	ips := []net.IP{}