Moves functionality from gitrepo to gitjob controller

charts/fleet/templates/rbac_gitjob.yaml

@@ -43,6 +43,26 @@ rules:
       - 'events'
     verbs:
       - '*'
+  - apiGroups:
+      - ""
+    resources:
+      - serviceaccounts
+    verbs:
+      - "create"
+  - apiGroups:
+      - rbac.authorization.k8s.io
+    resources:
+      - roles
+    verbs:
+      - escalate
+      - create
+      - bind
+  - apiGroups:
+      - rbac.authorization.k8s.io
+    resources:
+      - rolebindings
+    verbs:
+      - create
 
 ---
 apiVersion: rbac.authorization.k8s.io/v1

integrationtests/controller/gitrepo/gitrepo_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 
 	corev1 "k8s.io/api/core/v1"
-	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -58,29 +57,6 @@ var _ = Describe("GitRepo", func() {
 			Expect(err).NotTo(HaveOccurred())
 		})
 
-		It("creates RBAC resources", func() {
-			Expect(gitrepo.Spec.PollingInterval).To(BeNil())
-
-			Eventually(func() bool {
-				ns := types.NamespacedName{
-					Name:      fmt.Sprintf("git-%s", gitrepoName),
-					Namespace: namespace,
-				}
-
-				if err := k8sClient.Get(ctx, ns, &corev1.ServiceAccount{}); err != nil {
-					return false
-				}
-				if err := k8sClient.Get(ctx, ns, &rbacv1.Role{}); err != nil {
-					return false
-				}
-				if err := k8sClient.Get(ctx, ns, &rbacv1.RoleBinding{}); err != nil {
-					return false
-				}
-
-				return true
-			}).Should(BeTrue())
-		})
-
 		It("updates the gitrepo status", func() {
 			org := gitrepo.ResourceVersion
 			Eventually(func() bool {

integrationtests/gitjob/controller/controller_test.go

@@ -1,13 +1,19 @@
 package controller
 
 import (
+	"strings"
+	"time"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 	"github.com/rancher/wrangler/v2/pkg/name"
 
 	batchv1 "k8s.io/api/batch/v1"
+	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -43,6 +49,24 @@ var _ = Describe("GitJob controller", func() {
 
 			Expect(job.Spec.Template.Spec.Containers).To(HaveLen(1))
 			Expect(job.Spec.Template.Spec.Containers[0].Args).To(ContainElements("fleet", "apply"))
+
+			// it should create RBAC resources for that gitRepo
+			Eventually(func() bool {
+				saName := name.SafeConcatName("git", gitRepo.Name)
+				ns := types.NamespacedName{Name: saName, Namespace: gitRepo.Namespace}
+
+				if err := k8sClient.Get(ctx, ns, &corev1.ServiceAccount{}); err != nil {
+					return false
+				}
+				if err := k8sClient.Get(ctx, ns, &rbacv1.Role{}); err != nil {
+					return false
+				}
+				if err := k8sClient.Get(ctx, ns, &rbacv1.RoleBinding{}); err != nil {
+					return false
+				}
+
+				return true
+			}).Should(BeTrue())
 		})
 
 		When("a job completes successfully", func() {
@@ -54,7 +78,8 @@ var _ = Describe("GitJob controller", func() {
 				// simulate job was successful
 				Eventually(func() error {
 					err := k8sClient.Get(ctx, types.NamespacedName{Name: jobName, Namespace: gitRepoNamespace}, &job)
-					Expect(err).ToNot(HaveOccurred())
+					// We could be checking this when the job is still not created
+					Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred())
 					job.Status.Succeeded = 1
 					job.Status.Conditions = []batchv1.JobCondition{
 						{
@@ -226,6 +251,112 @@ var _ = Describe("GitJob controller", func() {
 			}).Should(BeTrue())
 		})
 	})
+
+	When("creating a gitRepo that references a nonexistent helm secret", func() {
+		var (
+			gitRepo                v1alpha1.GitRepo
+			gitRepoName            string
+			helmSecretNameForPaths string
+			helmSecretName         string
+		)
+
+		JustBeforeEach(func() {
+			gitRepoName = "test-no-for-paths-secret"
+			gitRepo = createGitRepo(gitRepoName)
+			gitRepo.Spec.HelmSecretNameForPaths = helmSecretNameForPaths
+			gitRepo.Spec.HelmSecretName = helmSecretName
+			// Create should return an error
+			err := k8sClient.Create(ctx, &gitRepo)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(simulateGitPollerUpdatingCommitInStatus(gitRepo, commit)).ToNot(HaveOccurred())
+		})
+
+		AfterEach(func() {
+			err := k8sClient.Delete(ctx, &gitRepo)
+			Expect(err).ToNot(HaveOccurred())
+			// reset the logs buffer so we don't read logs from previous tests
+			logsBuffer.Reset()
+		})
+
+		Context("helmSecretNameForPaths secret does not exist", func() {
+			BeforeEach(func() {
+				helmSecretNameForPaths = "secret-does-not-exist"
+				helmSecretName = ""
+			})
+			It("logs an error about HelmSecretNameForPaths not being found", func() {
+				Eventually(func() bool {
+					strLogs := logsBuffer.String()
+					return strings.Contains(strLogs, `failed to look up HelmSecretNameForPaths, error: Secret \"secret-does-not-exist\" not found`)
+				}).Should(BeTrue())
+			})
+
+			It("doesn't create RBAC resources", func() {
+				Consistently(func() bool {
+					saName := name.SafeConcatName("git", gitRepo.Name)
+					ns := types.NamespacedName{Name: saName, Namespace: gitRepo.Namespace}
+
+					if err := k8sClient.Get(ctx, ns, &corev1.ServiceAccount{}); !errors.IsNotFound(err) {
+						return false
+					}
+					if err := k8sClient.Get(ctx, ns, &rbacv1.Role{}); !errors.IsNotFound(err) {
+						return false
+					}
+					if err := k8sClient.Get(ctx, ns, &rbacv1.RoleBinding{}); !errors.IsNotFound(err) {
+						return false
+					}
+					return true
+				}, time.Second*5, time.Second*1).Should(BeTrue())
+			})
+
+			It("doesn't create the job", func() {
+				Consistently(func() bool {
+					jobName := name.SafeConcatName(gitRepoName, name.Hex(repo+commit, 5))
+					newJob := &batchv1.Job{}
+					err := k8sClient.Get(ctx, types.NamespacedName{Name: jobName, Namespace: gitRepo.Namespace}, newJob)
+					return errors.IsNotFound(err)
+				}, time.Second*5, time.Second*1).Should(BeTrue())
+			})
+		})
+		Context("helmSecretName secret does not exist", func() {
+			BeforeEach(func() {
+				helmSecretNameForPaths = ""
+				helmSecretName = "secret-does-not-exist"
+			})
+			It("logs an error about HelmSecretName not being found", func() {
+				Eventually(func() bool {
+					strLogs := logsBuffer.String()
+					return strings.Contains(strLogs, `failed to look up helmSecretName, error: Secret \"secret-does-not-exist\" not found`)
+				}).Should(BeTrue())
+			})
+
+			It("doesn't create RBAC resources", func() {
+				Consistently(func() bool {
+					saName := name.SafeConcatName("git", gitRepo.Name)
+					ns := types.NamespacedName{Name: saName, Namespace: gitRepo.Namespace}
+
+					if err := k8sClient.Get(ctx, ns, &corev1.ServiceAccount{}); !errors.IsNotFound(err) {
+						return false
+					}
+					if err := k8sClient.Get(ctx, ns, &rbacv1.Role{}); !errors.IsNotFound(err) {
+						return false
+					}
+					if err := k8sClient.Get(ctx, ns, &rbacv1.RoleBinding{}); !errors.IsNotFound(err) {
+						return false
+					}
+					return true
+				}, time.Second*5, time.Second*1).Should(BeTrue())
+			})
+
+			It("doesn't create the job", func() {
+				Consistently(func() bool {
+					jobName := name.SafeConcatName(gitRepoName, name.Hex(repo+commit, 5))
+					newJob := &batchv1.Job{}
+					err := k8sClient.Get(ctx, types.NamespacedName{Name: jobName, Namespace: gitRepo.Namespace}, newJob)
+					return errors.IsNotFound(err)
+				}, time.Second*5, time.Second*1).Should(BeTrue())
+			})
+		})
+	})
 })
 
 func simulateIncreaseForceSyncGeneration(gitRepo v1alpha1.GitRepo) error {

integrationtests/gitjob/controller/suite_test.go

@@ -1,6 +1,7 @@
 package controller
 
 import (
+	"bytes"
 	"context"
 	"path/filepath"
 	"testing"
@@ -20,18 +21,20 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 )
 
 const (
 	timeout = 30 * time.Second
 )
 
 var (
-	cfg       *rest.Config
-	testEnv   *envtest.Environment
-	ctx       context.Context
-	cancel    context.CancelFunc
-	k8sClient client.Client
+	cfg        *rest.Config
+	testEnv    *envtest.Environment
+	ctx        context.Context
+	cancel     context.CancelFunc
+	k8sClient  client.Client
+	logsBuffer bytes.Buffer
 )
 
 func TestGitJobController(t *testing.T) {
@@ -67,6 +70,10 @@ var _ = BeforeSuite(func() {
 	ctlr := gomock.NewController(GinkgoT())
 	gitPollerMock := mocks.NewMockGitPoller(ctlr)
 
+	// redirect logs to a buffer that we can read in the tests
+	GinkgoWriter.TeeTo(&logsBuffer)
+	ctrl.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
+
 	// do nothing if gitPoller is called. gitPoller calls are tested in unit tests
 	gitPollerMock.EXPECT().AddOrModifyGitRepoPollJob(gomock.Any(), gomock.Any()).AnyTimes()
 	gitPollerMock.EXPECT().CleanUpGitRepoPollJobs(gomock.Any()).AnyTimes()

internal/cmd/controller/errorutil/errorutil.go

@@ -0,0 +1,10 @@
+package errorutil
+
+import apierrors "k8s.io/apimachinery/pkg/api/errors"
+
+func IgnoreConflict(err error) error {
+	if apierrors.IsConflict(err) {
+		return nil
+	}
+	return err
+}

internal/cmd/controller/gitops/reconciler/gitjob_controller.go

@@ -100,16 +100,27 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		Name:      jobName(&gitRepo),
 	}, &job)
 	if err != nil && !errors.IsNotFound(err) {
-		return ctrl.Result{}, fmt.Errorf("error retrieving git job: %v", err)
+		return ctrl.Result{}, fmt.Errorf("error retrieving git job: %w", err)
 	}
 
 	if errors.IsNotFound(err) && gitRepo.Status.Commit != "" {
+		if err := r.validateExternalSecretExist(ctx, &gitRepo); err != nil {
+			nsname := types.NamespacedName{Namespace: gitRepo.Namespace, Name: gitRepo.Name}
+			return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, nsname, gitRepo.Status, err)
+		}
+		logger.V(1).Info("Creating Git job resources")
+		if err := r.createJobRBAC(ctx, &gitRepo); err != nil {
+			return ctrl.Result{}, fmt.Errorf("failed to create RBAC resources for git job: %w", err)
+		}
+		if err := r.createTargetsConfigMap(ctx, &gitRepo); err != nil {
+			return ctrl.Result{}, fmt.Errorf("failed to create targets config map for git job: %w", err)
+		}
 		if err := r.createJob(ctx, &gitRepo); err != nil {
-			return ctrl.Result{}, fmt.Errorf("error creating git job: %v", err)
+			return ctrl.Result{}, fmt.Errorf("error creating git job: %w", err)
 		}
 	} else if gitRepo.Status.Commit != "" {
 		if err = r.deleteJobIfNeeded(ctx, &gitRepo, &job); err != nil {
-			return ctrl.Result{}, fmt.Errorf("error deleting git job: %v", err)
+			return ctrl.Result{}, fmt.Errorf("error deleting git job: %w", err)
 		}
 	}
 
@@ -118,7 +129,7 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			logger.V(1).Info("conflict updating status, retrying", "message", err)
 			return ctrl.Result{Requeue: true}, nil // just retry, but don't show an error
 		}
-		return ctrl.Result{}, fmt.Errorf("error updating git job status: %v", err)
+		return ctrl.Result{}, fmt.Errorf("error updating git job status: %w", err)
 	}
 
 	return ctrl.Result{}, nil
@@ -141,6 +152,66 @@ func generationOrCommitChangedPredicate() predicate.Predicate {
 	}
 }
 
+func (r *GitJobReconciler) createJobRBAC(ctx context.Context, gitrepo *v1alpha1.GitRepo) error {
+	// No update needed, values are the same. So we ignore AlreadyExists.
+	saName := name.SafeConcatName("git", gitrepo.Name)
+	sa := grutil.NewServiceAccount(gitrepo.Namespace, saName)
+	if err := controllerutil.SetControllerReference(gitrepo, sa, r.Scheme); err != nil {
+		return err
+	}
+	if err := r.Create(ctx, sa); err != nil && !errors.IsAlreadyExists(err) {
+		return err
+	}
+
+	role := grutil.NewRole(gitrepo.Namespace, saName)
+	if err := controllerutil.SetControllerReference(gitrepo, role, r.Scheme); err != nil {
+		return err
+	}
+	if err := r.Create(ctx, role); err != nil && !errors.IsAlreadyExists(err) {
+		return err
+	}
+
+	rb := grutil.NewRoleBinding(gitrepo.Namespace, saName)
+	if err := controllerutil.SetControllerReference(gitrepo, rb, r.Scheme); err != nil {
+		return err
+	}
+	if err := r.Create(ctx, rb); err != nil && !errors.IsAlreadyExists(err) {
+		return err
+	}
+
+	return nil
+}
+
+func (r *GitJobReconciler) createTargetsConfigMap(ctx context.Context, gitrepo *v1alpha1.GitRepo) error {
+	configMap, err := grutil.NewTargetsConfigMap(gitrepo)
+	if err != nil {
+		return err
+	}
+	if err := controllerutil.SetControllerReference(gitrepo, configMap, r.Scheme); err != nil {
+		return err
+	}
+	data := configMap.BinaryData
+	_, err = controllerutil.CreateOrUpdate(ctx, r.Client, configMap, func() error {
+		configMap.BinaryData = data
+		return nil
+	})
+
+	return err
+}
+
+func (r *GitJobReconciler) validateExternalSecretExist(ctx context.Context, gitrepo *v1alpha1.GitRepo) error {
+	if gitrepo.Spec.HelmSecretNameForPaths != "" {
+		if err := r.Get(ctx, types.NamespacedName{Namespace: gitrepo.Namespace, Name: gitrepo.Spec.HelmSecretNameForPaths}, &corev1.Secret{}); err != nil {
+			return fmt.Errorf("failed to look up HelmSecretNameForPaths, error: %w", err)
+		}
+	} else if gitrepo.Spec.HelmSecretName != "" {
+		if err := r.Get(ctx, types.NamespacedName{Namespace: gitrepo.Namespace, Name: gitrepo.Spec.HelmSecretName}, &corev1.Secret{}); err != nil {
+			return fmt.Errorf("failed to look up helmSecretName, error: %w", err)
+		}
+	}
+	return nil
+}
+
 func (r *GitJobReconciler) createJob(ctx context.Context, gitRepo *v1alpha1.GitRepo) error {
 	job, err := r.newJob(ctx, gitRepo)
 	if err != nil {