Use controller roles for k8sManager in integration tests

This also introduces helpers to create and start the manager, and to create other client with different permissions in testEnvs. Issue: cloudfoundry#2660 Co-authored-by: Danail Branekov <[email protected]>
wanddynosios · Jul 13, 2023 · d1c595d · d1c595d
1 parent f423b49
commit d1c595d
Show file tree

Hide file tree

Showing 59 changed files with 1,064 additions and 1,375 deletions.
diff --git a/api/repositories/image_repository_test.go b/api/repositories/image_repository_test.go
@@ -5,11 +5,13 @@ import (
 	"context"
 	"errors"
 	"io"
+	"path/filepath"
 
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
 	"code.cloudfoundry.org/korifi/api/repositories"
 	"code.cloudfoundry.org/korifi/api/repositories/fake"
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
+	"code.cloudfoundry.org/korifi/tests/helpers"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -18,16 +20,16 @@ import (
 
 var _ = Describe("ImageRepository", func() {
 	var (
-		imagePusher         *fake.ImagePusher
-		privilegedK8sClient k8sclient.Interface
-		imageSource         io.Reader
-		imageRepo           *repositories.ImageRepository
-		imageName           string
-		imageRef            string
-		tags                []string
-		uploadErr           error
-		org                 *korifiv1alpha1.CFOrg
-		space               *korifiv1alpha1.CFSpace
+		imagePusher *fake.ImagePusher
+		k8sClient   k8sclient.Interface
+		imageSource io.Reader
+		imageRepo   *repositories.ImageRepository
+		imageName   string
+		imageRef    string
+		tags        []string
+		uploadErr   error
+		org         *korifiv1alpha1.CFOrg
+		space       *korifiv1alpha1.CFSpace
 	)
 
 	BeforeEach(func() {
@@ -38,7 +40,7 @@ var _ = Describe("ImageRepository", func() {
 		imageSource = bytes.NewBufferString("")
 
 		var err error
-		privilegedK8sClient, err = k8sclient.NewForConfig(k8sConfig)
+		k8sClient, err = k8sclient.NewForConfig(helpers.SetupTestEnvUser(testEnv, filepath.Join("helm", "korifi", "api", "role.yaml")))
 		Expect(err).NotTo(HaveOccurred())
 
 		org = createOrgWithCleanup(ctx, prefixedGUID("org"))
@@ -47,7 +49,7 @@ var _ = Describe("ImageRepository", func() {
 		tags = []string{"foo", "bar"}
 
 		imageRepo = repositories.NewImageRepository(
-			privilegedK8sClient,
+			k8sClient,
 			userClientFactory,
 			imagePusher,
 			[]string{"push-secret-name"},

diff --git a/api/repositories/package_repository_test.go b/api/repositories/package_repository_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"path/filepath"
 	"time"
 
 	apierrors "code.cloudfoundry.org/korifi/api/errors"
@@ -12,6 +13,7 @@ import (
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
 	"code.cloudfoundry.org/korifi/controllers/cleanup"
 	"code.cloudfoundry.org/korifi/controllers/controllers/workloads"
+	"code.cloudfoundry.org/korifi/tests/helpers"
 	"code.cloudfoundry.org/korifi/tests/matchers"
 	"code.cloudfoundry.org/korifi/tools"
 	"code.cloudfoundry.org/korifi/tools/image"
@@ -26,7 +28,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/kubernetes/scheme"
 )
 
 var _ = Describe("PackageRepository", func() {
@@ -36,7 +37,7 @@ var _ = Describe("PackageRepository", func() {
 		org         *korifiv1alpha1.CFOrg
 		space       *korifiv1alpha1.CFSpace
 		app         *korifiv1alpha1.CFApp
-		mgrCancel   context.CancelFunc
+		stopManager context.CancelFunc
 	)
 
 	BeforeEach(func() {
@@ -53,13 +54,9 @@ var _ = Describe("PackageRepository", func() {
 		space = createSpaceWithCleanup(ctx, org.Name, prefixedGUID("space"))
 		app = createApp(space.Name)
 
-		k8sManager, err := ctrl.NewManager(k8sConfig, ctrl.Options{
-			Scheme:             scheme.Scheme,
-			MetricsBindAddress: "0",
-		})
-		Expect(err).NotTo(HaveOccurred())
+		k8sManager := helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml"))
 
-		k8sInterface, err := kubernetes.NewForConfig(k8sConfig)
+		k8sInterface, err := kubernetes.NewForConfig(k8sManager.GetConfig())
 		Expect(err).NotTo(HaveOccurred())
 
 		err = (workloads.NewCFPackageReconciler(
@@ -72,17 +69,11 @@ var _ = Describe("PackageRepository", func() {
 		)).SetupWithManager(k8sManager)
 		Expect(err).NotTo(HaveOccurred())
 
-		var mgrCtx context.Context
-		mgrCtx, mgrCancel = context.WithCancel(ctx)
-		go func() {
-			defer GinkgoRecover()
-			err = k8sManager.Start(mgrCtx)
-			Expect(err).NotTo(HaveOccurred())
-		}()
+		stopManager = helpers.StartK8sManager(k8sManager)
 	})
 
 	AfterEach(func() {
-		mgrCancel()
+		stopManager()
 	})
 
 	Describe("CreatePackage", func() {

diff --git a/api/repositories/repositories_suite_test.go b/api/repositories/repositories_suite_test.go
@@ -49,7 +49,6 @@ var (
 	k8sClient             client.WithWatch
 	namespaceRetriever    repositories.NamespaceRetriever
 	userClientFactory     authorization.UserK8sClientFactory
-	k8sConfig             *rest.Config
 	userName              string
 	authInfo              authorization.Info
 	rootNamespace         string
@@ -78,19 +77,19 @@ var _ = BeforeSuite(func() {
 	}
 
 	var err error
-	k8sConfig, err = testEnv.Start()
+	_, err = testEnv.Start()
 	Expect(err).NotTo(HaveOccurred())
 
 	err = korifiv1alpha1.AddToScheme(scheme.Scheme)
 	Expect(err).NotTo(HaveOccurred())
 	err = buildv1alpha2.AddToScheme(scheme.Scheme)
 	Expect(err).NotTo(HaveOccurred())
 
-	k8sClient, err = client.NewWithWatch(k8sConfig, client.Options{Scheme: scheme.Scheme})
+	k8sClient, err = client.NewWithWatch(testEnv.Config, client.Options{Scheme: scheme.Scheme})
 	Expect(err).NotTo(HaveOccurred())
 	Expect(k8sClient).NotTo(BeNil())
 
-	dynamicClient, err := dynamic.NewForConfig(k8sConfig)
+	dynamicClient, err := dynamic.NewForConfig(testEnv.Config)
 	Expect(err).NotTo(HaveOccurred())
 	Expect(dynamicClient).NotTo(BeNil())
 	namespaceRetriever = repositories.NewNamespaceRetriever(dynamicClient)
@@ -118,16 +117,16 @@ var _ = BeforeEach(func() {
 	builderName = "kpack-image-builder"
 	runnerName = "statefulset-runner"
 	tokenInspector := authorization.NewTokenReviewer(k8sClient)
-	certInspector := authorization.NewCertInspector(k8sConfig)
+	certInspector := authorization.NewCertInspector(testEnv.Config)
 	baseIDProvider := authorization.NewCertTokenIdentityProvider(tokenInspector, certInspector)
 	idProvider = authorization.NewCachingIdentityProvider(baseIDProvider, cache.NewExpiring())
 	nsPerms = authorization.NewNamespacePermissions(k8sClient, idProvider)
 
-	httpClient, err := rest.HTTPClientFor(k8sConfig)
+	httpClient, err := rest.HTTPClientFor(testEnv.Config)
 	Expect(err).NotTo(HaveOccurred())
-	mapper, err := apiutil.NewDynamicRESTMapper(k8sConfig, httpClient)
+	mapper, err := apiutil.NewDynamicRESTMapper(testEnv.Config, httpClient)
 	Expect(err).NotTo(HaveOccurred())
-	userClientFactory = authorization.NewUnprivilegedClientFactory(k8sConfig, mapper, k8s.NewDefaultBackoff())
+	userClientFactory = authorization.NewUnprivilegedClientFactory(testEnv.Config, mapper, k8s.NewDefaultBackoff())
 
 	Expect(k8sClient.Create(context.Background(), &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: rootNamespace}})).To(Succeed())
 	createRoleBinding(context.Background(), userName, rootNamespaceUserRole.Name, rootNamespace)

diff --git a/controllers/api/v1alpha1/cfapp_webhook_test.go b/controllers/api/v1alpha1/cfapp_webhook_test.go
@@ -40,7 +40,7 @@ var _ = Describe("CFAppMutatingWebhook", func() {
 	})
 
 	JustBeforeEach(func() {
-		Expect(k8sClient.Create(ctx, cfApp)).To(Succeed())
+		Expect(adminClient.Create(ctx, cfApp)).To(Succeed())
 	})
 
 	It("adds a label matching metadata.name", func() {

diff --git a/controllers/api/v1alpha1/cfbuild_webhook_test.go b/controllers/api/v1alpha1/cfbuild_webhook_test.go
@@ -53,7 +53,7 @@ var _ = Describe("CFBuildMutatingWebhook", func() {
 	})
 
 	JustBeforeEach(func() {
-		Expect(k8sClient.Create(ctx, cfBuild)).To(Succeed())
+		Expect(adminClient.Create(ctx, cfBuild)).To(Succeed())
 	})
 
 	It("sets labels with the guids of the related app and package", func() {

diff --git a/controllers/api/v1alpha1/cforg_types_test.go b/controllers/api/v1alpha1/cforg_types_test.go
@@ -28,7 +28,7 @@ var _ = Describe("CF Org", func() {
 		})
 
 		JustBeforeEach(func() {
-			createErr = k8sClient.Create(ctx, cfOrg)
+			createErr = adminClient.Create(ctx, cfOrg)
 		})
 
 		It("accepts a valid name", func() {
@@ -37,7 +37,7 @@ var _ = Describe("CF Org", func() {
 
 		When("an org with the same display name already exists", func() {
 			BeforeEach(func() {
-				Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFOrg{
+				Expect(adminClient.Create(ctx, &korifiv1alpha1.CFOrg{
 					ObjectMeta: metav1.ObjectMeta{
 						Namespace: namespace,
 						Name:      uuid.NewString(),

diff --git a/controllers/api/v1alpha1/cfpackage_webhook_test.go b/controllers/api/v1alpha1/cfpackage_webhook_test.go
@@ -35,7 +35,7 @@ var _ = Describe("CFPackageMutatingWebhook", func() {
 	})
 
 	BeforeEach(func() {
-		Expect(k8sClient.Create(ctx, cfPackage)).To(Succeed())
+		Expect(adminClient.Create(ctx, cfPackage)).To(Succeed())
 	})
 
 	It("sets a label with the app guid", func() {

diff --git a/controllers/api/v1alpha1/cfprocess_webhook_test.go b/controllers/api/v1alpha1/cfprocess_webhook_test.go
@@ -46,7 +46,7 @@ var _ = Describe("CFProcessMutatingWebhook", func() {
 	})
 
 	JustBeforeEach(func() {
-		Expect(k8sClient.Create(context.Background(), cfProcess)).To(Succeed())
+		Expect(adminClient.Create(context.Background(), cfProcess)).To(Succeed())
 	})
 
 	Describe("labels", func() {

diff --git a/controllers/api/v1alpha1/cfroute_webhook_test.go b/controllers/api/v1alpha1/cfroute_webhook_test.go
@@ -33,7 +33,7 @@ var _ = Describe("CFRouteMutatingWebhook Integration Tests", func() {
 					Name: "a" + uuid.NewString() + ".com",
 				},
 			}
-			Expect(k8sClient.Create(ctx, cfDomain)).To(Succeed())
+			Expect(adminClient.Create(ctx, cfDomain)).To(Succeed())
 
 			cfRoute = &korifiv1alpha1.CFRoute{
 				ObjectMeta: metav1.ObjectMeta{
@@ -52,12 +52,12 @@ var _ = Describe("CFRouteMutatingWebhook Integration Tests", func() {
 		})
 
 		JustBeforeEach(func() {
-			Expect(k8sClient.Create(ctx, cfRoute)).To(Succeed())
+			Expect(adminClient.Create(ctx, cfRoute)).To(Succeed())
 		})
 
 		AfterEach(func() {
-			Expect(k8sClient.Delete(ctx, cfRoute)).To(Succeed())
-			Expect(k8sClient.Delete(ctx, cfDomain)).To(Succeed())
+			Expect(adminClient.Delete(ctx, cfRoute)).To(Succeed())
+			Expect(adminClient.Delete(ctx, cfDomain)).To(Succeed())
 		})
 
 		It("adds labels with guids of the domain and route", func() {

diff --git a/controllers/api/v1alpha1/cfspace_types_test.go b/controllers/api/v1alpha1/cfspace_types_test.go
@@ -28,9 +28,9 @@ var _ = Describe("CF Space", func() {
 					DisplayName: uuid.NewString(),
 				},
 			}
-			Expect(k8sClient.Create(ctx, cfOrg)).To(Succeed())
+			Expect(adminClient.Create(ctx, cfOrg)).To(Succeed())
 
-			Expect(k8sClient.Create(ctx, &corev1.Namespace{
+			Expect(adminClient.Create(ctx, &corev1.Namespace{
 				ObjectMeta: metav1.ObjectMeta{Name: cfOrg.Name},
 			})).To(Succeed())
 
@@ -46,7 +46,7 @@ var _ = Describe("CF Space", func() {
 		})
 
 		JustBeforeEach(func() {
-			createErr = k8sClient.Create(ctx, cfSpace)
+			createErr = adminClient.Create(ctx, cfSpace)
 		})
 
 		It("accepts a valid name", func() {
@@ -55,7 +55,7 @@ var _ = Describe("CF Space", func() {
 
 		When("a space with the same display name already exists", func() {
 			BeforeEach(func() {
-				Expect(k8sClient.Create(ctx, &korifiv1alpha1.CFSpace{
+				Expect(adminClient.Create(ctx, &korifiv1alpha1.CFSpace{
 					ObjectMeta: metav1.ObjectMeta{
 						Namespace: cfOrg.Name,
 						Name:      uuid.NewString(),