Tests for testing/controlplane components

This adds fairly comprehensive tests for the control plane components,
which either didn't exist before the refactor or existed very loosely in
the form of general integration tests.

Author: DirectXMan12

pkg/internal/testing/controlplane/apiserver.go

@@ -15,6 +15,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/internal/testing/process"
 )
 
+// TODO: listenaddr needs to be exposes publically so we can set insecureserving
+
 const (
 	// saKeyFile is the name of the service account signing private key file
 	saKeyFile = "sa-signer.key"
@@ -144,6 +146,13 @@ func (s *APIServer) Configure() *process.Arguments {
 // Start starts the apiserver, waits for it to come up, and returns an error,
 // if occurred.
 func (s *APIServer) Start() error {
+	if err := s.prepare(); err != nil {
+		return err
+	}
+	return s.processState.Start(s.Out, s.Err)
+}
+
+func (s *APIServer) prepare() error {
 	if s.processState == nil {
 		if err := s.setProcessState(); err != nil {
 			return err
@@ -152,7 +161,7 @@ func (s *APIServer) Start() error {
 	if err := s.Authn.Start(); err != nil {
 		return err
 	}
-	return s.processState.Start(s.Out, s.Err)
+	return nil
 }
 
 // configurePorts configures the serving ports for this API server.
@@ -398,7 +407,9 @@ func (s *APIServer) populateAPIServerCerts() error {
 // the CertDir if necessary.
 func (s *APIServer) Stop() error {
 	if s.processState != nil {
-		return s.processState.Stop()
+		if err := s.processState.Stop(); err != nil {
+			return err
+		}
 	}
 	return s.Authn.Stop()
 }
@@ -425,3 +436,23 @@ var APIServerDefaultArgs = []string{
 	"--allow-privileged=true",
 	// NB(directxman12): we also enable RBAC if nothing else was enabled
 }
+
+// PrepareAPIServer is an internal-only (NEVER SHOULD BE EXPOSED)
+// function that sets up the API server just before starting it,
+// without actually starting it.  This saves time on tests.
+//
+// NB(directxman12): do not expose this outside of internal -- it's unsafe to
+// use, because things like port allocation could race even more than they
+// currently do if you later call start!
+func PrepareAPIServer(s *APIServer) error {
+	return s.prepare()
+}
+
+// APIServerArguments is an internal-only (NEVER SHOULD BE EXPOSED)
+// function that sets up the API server just before starting it,
+// without actually starting it.  It's public to make testing easier.
+//
+// NB(directxman12): do not expose this outside of internal
+func APIServerArguments(s *APIServer) []string {
+	return s.processState.Args
+}

pkg/internal/testing/controlplane/apiserver_test.go

@@ -0,0 +1,279 @@
+package controlplane_test
+
+import (
+	"errors"
+	"net/url"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"k8s.io/client-go/rest"
+
+	. "sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane"
+	"sigs.k8s.io/controller-runtime/pkg/internal/testing/process"
+)
+
+var _ = Describe("APIServer", func() {
+	var server *APIServer
+	BeforeEach(func() {
+		server = &APIServer{
+			EtcdURL: &url.URL{},
+		}
+	})
+	JustBeforeEach(func() {
+		Expect(PrepareAPIServer(server)).To(Succeed())
+	})
+	Describe("setting up serving hosts & ports", func() {
+		Context("when URL is set", func() {
+			BeforeEach(func() {
+				server.URL = &url.URL{Scheme: "http", Host: "localhost:8675", Path: "/some-path"}
+			})
+
+			Context("when insecure serving is also set", func() {
+				BeforeEach(func() {
+					server.InsecureServing = &process.ListenAddr{
+						Address: "localhost",
+						Port:    "1234",
+					}
+				})
+
+				It("should override the existing insecure serving", func() {
+					Expect(server.InsecureServing).To(Equal(&process.ListenAddr{
+						Address: "localhost",
+						Port:    "8675",
+					}))
+				})
+			})
+
+			It("should set insecure serving off of that", func() {
+				Expect(server.InsecureServing).To(Equal(&process.ListenAddr{
+					Address: "localhost",
+					Port:    "8675",
+				}))
+			})
+
+			It("should keep URL as-is", func() {
+				Expect(server.URL.String()).To(Equal("http://localhost:8675/some-path"))
+			})
+		})
+
+		Context("when URL is not set but InsecureServing is set", func() {
+			BeforeEach(func() {
+				server.InsecureServing = &process.ListenAddr{}
+			})
+
+			Context("when host and port are set", func() {
+				BeforeEach(func() {
+					server.InsecureServing.Address = "localhost"
+					server.InsecureServing.Port = "8675"
+				})
+				It("should set URL from InsecureServing", func() {
+					Expect(server.URL.String()).To(Equal("http://localhost:8675"))
+				})
+
+				It("should leave InsecureServing as-is if address and port are filled out", func() {
+					Expect(server.InsecureServing).To(Equal(&process.ListenAddr{
+						Address: "localhost",
+						Port:    "8675",
+					}))
+				})
+			})
+
+			Context("when address and port are not filled out", func() {
+				BeforeEach(func() {
+					server.InsecureServing = &process.ListenAddr{}
+				})
+				It("should default an insecure port", func() {
+					Expect(server.InsecureServing.Port).NotTo(BeEmpty())
+				})
+				It("should set URL from InsecureServing", func() {
+					Expect(server.URL.String()).To(Equal("http://" + server.InsecureServing.Address + ":" + server.InsecureServing.Port))
+				})
+			})
+		})
+
+		Context("when neither URL or InsecureServing are set", func() {
+			It("should not default either of them", func() {
+				Expect(server.URL).To(BeNil(), "no URL should be set")
+				Expect(server.InsecureServing).To(BeNil(), "no insecure serving details should be set")
+			})
+		})
+
+		Context("when SecureServing host & port are set", func() {
+			BeforeEach(func() {
+				server.Address = "localhost"
+				server.Port = "8675"
+			})
+
+			It("should leave SecureServing as-is", func() {
+				Expect(server.SecureServing.Address).To(Equal("localhost"))
+				Expect(server.SecureServing.Port).To(Equal("8675"))
+			})
+		})
+
+		Context("when SecureServing is not set", func() {
+			It("should be defaulted with a random port", func() {
+				Expect(server.Port).NotTo(Equal(0))
+			})
+		})
+	})
+
+	It("should default authn if not set", func() {
+		Expect(server.Authn).NotTo(BeNil())
+	})
+
+	Describe("argument defaulting", func() {
+		// NB(directxman12): most of the templating vs configure logic is tested
+		// in arguments/arguments_test.go, so just test secure vs insecure port logic here
+
+		Context("when insecure serving is set, on a binary that supports it", func() {
+			BeforeEach(func() {
+				server.InsecureServing = &process.ListenAddr{
+					Address: "localhost",
+					Port:    "8675",
+				}
+				server.Path = "./testdata/fake-1.19-apiserver.sh"
+			})
+			It("should set the insecure-port and insecure-bind-address fields from insecureserving", func() {
+				Expect(APIServerArguments(server)).To(ContainElements(
+					"--insecure-port=8675",
+					"--insecure-bind-address=localhost",
+				))
+			})
+		})
+
+		Context("when insecureserving is disabled, on binaries with no insecure-port flag", func() {
+			BeforeEach(func() {
+				server.Path = "./testdata/fake-1.20-apiserver.sh"
+			})
+			It("should not try to explicitly disable the insecure port", func() {
+				Expect(APIServerArguments(server)).NotTo(ContainElement(HavePrefix("--insecure-port")))
+			})
+		})
+
+		Context("when insecureserving is disabled, on binaries with an insecure-port flag", func() {
+			BeforeEach(func() {
+				server.Path = "./testdata/fake-1.19-apiserver.sh"
+			})
+			It("should explicitly disable the insecure port", func() {
+				Expect(APIServerArguments(server)).To(ContainElement("--insecure-port=0"))
+			})
+		})
+
+		Context("when given legacy-style template arguments", func() {
+			BeforeEach(func() {
+				server.Args = []string{"--foo=bar", "--baz={{ .Port }}"}
+			})
+			It("should use the passed in args with the minimal required defaults", func() {
+				Expect(APIServerArguments(server)).To(ConsistOf(
+					"--foo=bar",
+					MatchRegexp(`--baz=\d+`),
+					"--service-cluster-ip-range=10.0.0.0/24",
+					MatchRegexp("--client-ca-file=.+"),
+					"--authorization-mode=RBAC",
+				))
+			})
+		})
+	})
+
+	Describe("setting up auth", func() {
+		var auth *fakeAuthn
+		BeforeEach(func() {
+			auth = &fakeAuthn{
+				setFlag: true,
+			}
+			server.Authn = auth
+		})
+		It("should configure with the cert dir", func() {
+			Expect(auth.workDir).To(Equal(server.CertDir))
+		})
+		It("should pass its args to be configured", func() {
+			Expect(server.Configure().Get("configure-called").Get(nil)).To(ConsistOf("true"))
+		})
+
+		Context("when configuring auth errors out", func() {
+			It("should fail to configure", func() {
+				server := &APIServer{
+					EtcdURL: &url.URL{},
+					SecureServing: SecureServing{
+						Authn: auth,
+					},
+				}
+				auth.configureErr = errors.New("Oh no")
+				Expect(PrepareAPIServer(server)).NotTo(Succeed())
+			})
+		})
+	})
+
+	Describe("managing", func() {
+		// some of these tests are combined for speed reasons -- starting the apiserver
+		// takes a while, relatively speaking
+
+		var (
+			auth *fakeAuthn
+			etcd *Etcd
+		)
+		BeforeEach(func() {
+			etcd = &Etcd{}
+			Expect(etcd.Start()).To(Succeed())
+			server.EtcdURL = etcd.URL
+
+			auth = &fakeAuthn{}
+			server.Authn = auth
+		})
+		AfterEach(func() {
+			Expect(etcd.Stop()).To(Succeed())
+		})
+
+		Context("after starting", func() {
+			BeforeEach(func() {
+				Expect(server.Start()).To(Succeed())
+			})
+
+			It("should stop successfully, and stop auth", func() {
+				Expect(server.Stop()).To(Succeed())
+				Expect(auth.stopCalled).To(BeTrue())
+			})
+		})
+
+		It("should fail to start when auth fails to start", func() {
+			auth.startErr = errors.New("Oh no")
+			Expect(server.Start()).NotTo(Succeed())
+		})
+
+		It("should start successfully & start auth", func() {
+			Expect(server.Start()).To(Succeed())
+			defer func() { Expect(server.Stop()).To(Succeed()) }()
+			Expect(auth.startCalled).To(BeTrue())
+		})
+	})
+})
+
+type fakeAuthn struct {
+	workDir string
+
+	startCalled bool
+	stopCalled  bool
+	setFlag     bool
+
+	configureErr error
+	startErr     error
+}
+
+func (f *fakeAuthn) Configure(workDir string, args *process.Arguments) error {
+	f.workDir = workDir
+	if f.setFlag {
+		args.Set("configure-called", "true")
+	}
+	return f.configureErr
+}
+func (f *fakeAuthn) Start() error {
+	f.startCalled = true
+	return f.startErr
+}
+func (f *fakeAuthn) AddUser(user User, baseCfg *rest.Config) (*rest.Config, error) {
+	return nil, nil
+}
+func (f *fakeAuthn) Stop() error {
+	f.stopCalled = true
+	return nil
+}

pkg/internal/testing/controlplane/etcd_test.go

@@ -0,0 +1,20 @@
+package controlplane_test
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	. "sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane"
+)
+
+var _ = Describe("etcd", func() {
+	// basic coherence test
+	It("should start and stop successfully", func() {
+		etcd := &Etcd{}
+		Expect(etcd.Start()).To(Succeed())
+		defer func() {
+			Expect(etcd.Stop()).To(Succeed())
+		}()
+		Expect(etcd.URL).NotTo(BeNil())
+	})
+})

pkg/internal/testing/controlplane/kubectl_test.go

@@ -51,7 +51,7 @@ var _ = Describe("KubeConfigFromREST", func() {
 
 	BeforeEach(func() {
 		restCfg = &rest.Config{
-			Host:    "some-host:8675",
+			Host:    "https://some-host:8675",
 			APIPath: "/some-prefix",
 			TLSClientConfig: rest.TLSClientConfig{
 				CertData: []byte("cert"),
@@ -91,6 +91,7 @@ var _ = Describe("KubeConfigFromREST", func() {
 
 	Context("when no TLS is enabled", func() {
 		BeforeEach(func() {
+			restCfg.Host = "http://some-host:8675"
 			restCfg.TLSClientConfig = rest.TLSClientConfig{}
 		})