Parametrize job cluster role and image (k3s-io#214)

* Add pre-requisite checks to build scripts * Improved switches descriptions * Add temporary dapper Dockerfile to ignored files * Implemented ability to parameterize job cluster role used by jobs managing helm charts * Implemented ability to parameterize job image used by jobs managing helm charts * Skip lint dot-imports check on *_test.go files --------- Signed-off-by: Piotr Minkina <[email protected]>
ceccone · Dec 12, 2023 · cf4b87d · cf4b87d
1 parent 4fd2026
commit cf4b87d
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 18 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,6 +1,8 @@
 .DS_Store
 /.dapper
 /.idea
+/Dockerfile.dapper*
+!/Dockerfile.dapper
 /bin
 /dist
 helm-controller
diff --git a/.golangci.json b/.golangci.json
@@ -36,6 +36,10 @@
 			{
 				"linters": "revive",
 				"text": "unused-parameter"
+			},
+			{
+				"path": "_test\\.go$",
+				"text": "dot-imports: should not use dot imports"
 			}
 		]
 	}

diff --git a/main.go b/main.go
@@ -26,13 +26,15 @@ var (
 )
 
 type HelmController struct {
-	Kubeconfig     string `short:"k" usage:"Kubernetes config files, e.g. $HOME/.kube/config" env:"KUBECONFIG"`
-	MasterURL      string `short:"m" usage:"Kubernetes cluster master URL" env:"MASTERURL"`
-	Namespace      string `short:"n" usage:"Namespace to watch, empty means it will watch CRDs in all namespaces." env:"NAMESPACE"`
-	Threads        int    `short:"t" usage:"Threadiness level to set, defaults to 2." default:"2" env:"THREADS"`
-	ControllerName string `usage:"Unique name to identify this controller that is added to all HelmCharts tracked by this controller" default:"helm-controller" env:"CONTROLLER_NAME"`
-	NodeName       string `usage:"Name of the node this controller is running on" env:"NODE_NAME"`
-	PprofPort      int    `usage:"Port to publish HTTP server runtime profiling data in the format expected by the pprof visualization tool. Only enabled if in debug mode" default:"6060"`
+	Kubeconfig      string `short:"k" usage:"Kubernetes config files, e.g. $HOME/.kube/config. May be set via KUBECONFIG env var." env:"KUBECONFIG"`
+	MasterURL       string `short:"m" usage:"Kubernetes cluster master URL. May be set via MASTERURL env var." env:"MASTERURL"`
+	Namespace       string `short:"n" usage:"Namespace to watch, empty means it will watch CRDs in all namespaces. May be set via NAMESPACE env var." env:"NAMESPACE"`
+	Threads         int    `short:"t" usage:"Threadiness level to set. May be set via THREADS env var." default:"2" env:"THREADS"`
+	ControllerName  string `usage:"Unique name to identify this controller that is added to all HelmCharts tracked by this controller. May be set via CONTROLLER_NAME env var." default:"helm-controller" env:"CONTROLLER_NAME"`
+	NodeName        string `usage:"Name of the node this controller is running on. May be set via NODE_NAME env var." env:"NODE_NAME"`
+	JobClusterRole  string `usage:"Name of the cluster role to use for jobs created to manage helm charts. May be set via JOB_CLUSTER_ROLE env var." default:"cluster-admin" env:"JOB_CLUSTER_ROLE"`
+	DefaultJobImage string `usage:"Default image to use by jobs managing helm charts. May be set via DEFAULT_JOB_IMAGE env var." env:"DEFAULT_JOB_IMAGE"`
+	PprofPort       int    `usage:"Port to publish HTTP server runtime profiling data in the format expected by the pprof visualization tool. Only enabled if in debug mode." default:"6060"`
 }
 
 func (a *HelmController) Run(cmd *cobra.Command, args []string) error {
@@ -60,8 +62,10 @@ func (a *HelmController) Run(cmd *cobra.Command, args []string) error {
 	}
 
 	opts := common.Options{
-		Threadiness: a.Threads,
-		NodeName:    a.NodeName,
+		Threadiness:     a.Threads,
+		NodeName:        a.NodeName,
+		JobClusterRole:  a.JobClusterRole,
+		DefaultJobImage: a.DefaultJobImage,
 	}
 
 	if err := opts.Validate(); err != nil {

diff --git a/pkg/controllers/chart/chart.go b/pkg/controllers/chart/chart.go
@@ -60,6 +60,7 @@ var (
 
 type Controller struct {
 	systemNamespace string
+	jobClusterRole  string
 	managedBy       string
 	helms           helmcontroller.HelmChartController
 	helmCache       helmcontroller.HelmChartCache
@@ -72,8 +73,12 @@ type Controller struct {
 	apiServerPort   string
 }
 
-func Register(ctx context.Context,
-	systemNamespace, managedBy, apiServerPort string,
+func Register(
+	ctx context.Context,
+	systemNamespace,
+	managedBy,
+	jobClusterRole string,
+	apiServerPort string,
 	k8s kubernetes.Interface,
 	apply apply.Apply,
 	recorder record.EventRecorder,
@@ -90,6 +95,7 @@ func Register(ctx context.Context,
 
 	c := &Controller{
 		systemNamespace: systemNamespace,
+		jobClusterRole:  jobClusterRole,
 		managedBy:       managedBy,
 		helms:           helms,
 		helmCache:       helmCache,
@@ -346,7 +352,7 @@ func (c *Controller) getJobAndRelatedResources(chart *v1.HelmChart) (*batch.Job,
 		valuesSecret,
 		contentConfigMap,
 		serviceAccount(chart),
-		roleBinding(chart),
+		roleBinding(chart, c.jobClusterRole),
 	}, nil
 }
 
@@ -537,7 +543,7 @@ func valuesSecretAddConfig(secret *corev1.Secret, config *v1.HelmChartConfig) {
 	}
 }
 
-func roleBinding(chart *v1.HelmChart) *rbac.ClusterRoleBinding {
+func roleBinding(chart *v1.HelmChart, jobClusterRole string) *rbac.ClusterRoleBinding {
 	return &rbac.ClusterRoleBinding{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "rbac.authorization.k8s.io/v1",
@@ -549,7 +555,7 @@ func roleBinding(chart *v1.HelmChart) *rbac.ClusterRoleBinding {
 		RoleRef: rbac.RoleRef{
 			Kind:     "ClusterRole",
 			APIGroup: "rbac.authorization.k8s.io",
-			Name:     "cluster-admin",
+			Name:     jobClusterRole,
 		},
 		Subjects: []rbac.Subject{
 			{

diff --git a/pkg/controllers/chart/chart_test.go b/pkg/controllers/chart/chart_test.go
@@ -53,6 +53,14 @@ func TestDeleteJob(t *testing.T) {
 	assert.Equal("helm-delete-traefik", job.Name)
 }
 
+func TestInstallJobImage(t *testing.T) {
+	assert := assert.New(t)
+	chart := NewChart()
+	chart.Spec.JobImage = "custom-job-image"
+	job, _, _ := job(chart, "6443")
+	assert.Equal("custom-job-image", job.Spec.Template.Spec.Containers[0].Image)
+}
+
 func TestInstallArgs(t *testing.T) {
 	assert := assert.New(t)
 	stringArgs := strings.Join(args(NewChart()), " ")

diff --git a/pkg/controllers/common/options.go b/pkg/controllers/common/options.go
@@ -4,8 +4,10 @@ import "fmt"
 
 // Options defines options that can be set on initializing the Helm Controller
 type Options struct {
-	Threadiness int
-	NodeName    string
+	Threadiness     int
+	NodeName        string
+	JobClusterRole  string
+	DefaultJobImage string
 }
 
 func (opts Options) Validate() error {

diff --git a/pkg/controllers/controllers.go b/pkg/controllers/controllers.go
@@ -72,9 +72,15 @@ func Register(ctx context.Context, systemNamespace, controllerName string, cfg c
 		controllerName = "helm-controller"
 	}
 
+	// apply custom DefaultJobImage option to Helm before starting charts controller
+	if opts.DefaultJobImage != "" {
+		chart.DefaultJobImage = opts.DefaultJobImage
+	}
+
 	chart.Register(ctx,
 		systemNamespace,
 		controllerName,
+		opts.JobClusterRole,
 		"6443",
 		appCtx.K8s,
 		appCtx.Apply,
@@ -91,6 +97,8 @@ func Register(ctx context.Context, systemNamespace, controllerName string, cfg c
 		appCtx.Core.Secret())
 
 	klog.Infof("Starting helm controller with %d threads", opts.Threadiness)
+	klog.Infof("Using cluster role '%s' for jobs managing helm charts", opts.JobClusterRole)
+	klog.Infof("Using default image '%s' for jobs managing helm charts", chart.DefaultJobImage)
 
 	if len(systemNamespace) == 0 {
 		klog.Info("Starting helm controller with no namespace")

diff --git a/scripts/e2e b/scripts/e2e
@@ -7,6 +7,11 @@ K3S_VERSION=v1.25.9-k3s1
 
 cd $(dirname $0)/..
 
+if [[ ! -f 'bin/helm-controller-image.txt' ]]; then
+    echo "Run 'make package' first."
+    exit 1
+fi
+
 setup_k8s(){
     # Using k3s with embedded helm controller disabled
     docker pull rancher/k3s:$K3S_VERSION

diff --git a/scripts/package b/scripts/package
@@ -8,6 +8,11 @@ SUFFIX="-${ARCH}"
 
 cd $(dirname $0)/..
 
+if [[ ! -f 'bin/helm-controller' ]]; then
+    echo "Run 'make build' first."
+    exit 1
+fi
+
 TAG=${TAG:-${VERSION}${SUFFIX}}
 REPO=${REPO:-rancher}
 

diff --git a/test/framework/controller.go b/test/framework/controller.go
@@ -22,6 +22,11 @@ func (f *Framework) setupController(ctx context.Context) error {
 		return err
 	}
 
+	_, err = f.ClientSet.RbacV1().ClusterRoles().Create(ctx, f.getCr(), metav1.CreateOptions{})
+	if err != nil {
+		return err
+	}
+
 	_, err = f.ClientSet.RbacV1().ClusterRoleBindings().Create(ctx, f.getCrb(), metav1.CreateOptions{})
 	if err != nil {
 		return err
@@ -94,7 +99,10 @@ func (f *Framework) getDeployment() *appsv1.Deployment {
 							Name:    f.Name,
 							Image:   getImage(),
 							Command: []string{"helm-controller"},
-							Args:    []string{"--namespace", "helm-controller"},
+							Args: []string{
+								"--namespace", "helm-controller",
+								"--job-cluster-role", f.Name,
+							},
 						},
 					},
 				},
@@ -110,6 +118,25 @@ func getImage() string {
 	return "rancher/helm-controller:latest"
 }
 
+func (f *Framework) getCr() *v1.ClusterRole {
+	return &v1.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: f.Name,
+		},
+		Rules: []v1.PolicyRule{
+			{
+				APIGroups: []string{"*"},
+				Resources: []string{"*"},
+				Verbs:     []string{"*"},
+			},
+			{
+				NonResourceURLs: []string{"*"},
+				Verbs:           []string{"*"},
+			},
+		},
+	}
+}
+
 func (f *Framework) getCrb() *v1.ClusterRoleBinding {
 	return &v1.ClusterRoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
@@ -118,7 +145,7 @@ func (f *Framework) getCrb() *v1.ClusterRoleBinding {
 		RoleRef: v1.RoleRef{
 			APIGroup: "rbac.authorization.k8s.io",
 			Kind:     "ClusterRole",
-			Name:     "cluster-admin",
+			Name:     f.Name,
 		},
 		Subjects: []v1.Subject{
 			{