Merge pull request #1486 from DirectXMan12/feature/envtest-secure-port

⚠️ Envtest refactor, support for 1.20+ API servers (secure serving)

.golangci.yml

@@ -24,12 +24,13 @@ linters:
   - deadcode
   - errcheck
   - varcheck
-  - goconst
   - unparam
   - ineffassign
   - nakedret
   - gocyclo
-  - lll
   - dupl
   - goimports
   - golint
+  # disabled:
+  # - goconst is overly aggressive
+  # - lll generally just complains about flag help & error strings that are human-readable

examples/scratch-env/go.mod

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

examples/scratch-env/main.go

@@ -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,34 +45,45 @@ 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())
+		return 1
+	}
+	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),
+		kubeConfig, err := user.KubeConfig()
+		if err != nil {
+			log.Error(err, "unable to create kubeconfig")
 		}
-		kcCtx := kcapi.NewContext()
-		kcCtx.Cluster = "scratch-env"
-		kubeConfig.Contexts["scratch-env"] = kcCtx
-		kubeConfig.CurrentContext = "scratch-env"
 
-		if err := writeKubeConfig(kubeConfig, kubeconfigFile); err != nil {
+		if _, err := kubeconfigFile.Write(kubeConfig); err != nil {
 			log.Error(err, "unable to save kubeconfig")
 			return 1
 		}

pkg/cluster/cluster_suite_test.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/controller/controller_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/envtest/crd.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 {