#199 Generalize Jenkins container command validation

jenkinsci · Jan 18, 2020 · bf07fa5 · bf07fa5
1 parent bf3a4bc
commit bf07fa5
Show file tree

Hide file tree

Showing 5 changed files with 178 additions and 5 deletions.
diff --git a/pkg/apis/jenkins/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/jenkins/v1alpha2/zz_generated.deepcopy.go
diff --git a/pkg/controller/jenkins/configuration/base/reconcile.go b/pkg/controller/jenkins/configuration/base/reconcile.go
@@ -484,11 +484,6 @@ func (r *ReconcileJenkinsBaseConfiguration) ensureJenkinsMasterPod(meta metav1.O
 	currentJenkinsMasterPod, err := r.getJenkinsMasterPod()
 	if err != nil && apierrors.IsNotFound(err) {
 		jenkinsMasterPod := resources.NewJenkinsMasterPod(meta, r.Configuration.Jenkins)
-		if !reflect.DeepEqual(jenkinsMasterPod.Spec.Containers[0].Command, resources.GetJenkinsMasterContainerBaseCommand()) {
-			r.logger.Info(fmt.Sprintf("spec.master.containers[%s].command has been overridden make sure the command looks like: '%v', otherwise the operator won't configure default user and install plugins",
-				resources.JenkinsMasterContainerName, []string{"bash", "-c", fmt.Sprintf("%s/%s && <custom-command-here> && /sbin/tini -s -- /usr/local/bin/jenkins.sh",
-					resources.JenkinsScriptsVolumePath, resources.InitScriptName)}))
-		}
 		*r.Notifications <- event.Event{
 			Jenkins: *r.Configuration.Jenkins,
 			Phase:   event.PhaseBase,

diff --git a/pkg/controller/jenkins/configuration/base/validate.go b/pkg/controller/jenkins/configuration/base/validate.go
@@ -69,6 +69,38 @@ func (r *ReconcileJenkinsBaseConfiguration) Validate(jenkins *v1alpha2.Jenkins)
 	return messages, nil
 }
 
+func (r *ReconcileJenkinsBaseConfiguration) validateJenkinsMasterContainerCommand() []string {
+	masterContainer := r.Configuration.GetJenkinsMasterContainer()
+	if masterContainer == nil {
+		return []string{}
+	}
+
+	jenkinsOperatorInitScript := fmt.Sprintf("%s/%s && ", resources.JenkinsScriptsVolumePath, resources.InitScriptName)
+	correctCommand := []string{
+		"bash",
+		"-c",
+		fmt.Sprintf("%s<optional-custom-command> && exec <command-which-start-jenkins>", jenkinsOperatorInitScript),
+	}
+	invalidCommandMessage := []string{fmt.Sprintf("spec.master.containers[%s].command is invalid, make sure it looks like '%v', otherwise the operator won't configure default user and install plugins. 'exec' is required to propagate signals to the Jenkins.", masterContainer.Name, correctCommand)}
+	if len(masterContainer.Command) != 3 {
+		return invalidCommandMessage
+	}
+	if masterContainer.Command[0] != correctCommand[0] {
+		return invalidCommandMessage
+	}
+	if masterContainer.Command[1] != correctCommand[1] {
+		return invalidCommandMessage
+	}
+	if !strings.HasPrefix(masterContainer.Command[2], jenkinsOperatorInitScript) {
+		return invalidCommandMessage
+	}
+	if !strings.Contains(masterContainer.Command[2], "exec") {
+		return invalidCommandMessage
+	}
+
+	return []string{}
+}
+
 func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecrets() ([]string, error) {
 	var messages []string
 	for _, sr := range r.Configuration.Jenkins.Spec.Master.ImagePullSecrets {

diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go
@@ -840,3 +840,123 @@ func TestValidateCustomization(t *testing.T) {
 		assert.Equal(t, got, []string{"ConfigMap 'configmap-name' configured in spec.groovyScripts.configurations[0] not found"})
 	})
 }
+
+func TestValidateJenkinsMasterContainerCommand(t *testing.T) {
+	log.SetupLogger(true)
+	t.Run("no Jenkins master container", func(t *testing.T) {
+		jenkins := &v1alpha2.Jenkins{}
+		baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{})
+
+		got := baseReconcileLoop.validateJenkinsMasterContainerCommand()
+
+		assert.Empty(t, got)
+	})
+	t.Run("empty command", func(t *testing.T) {
+		jenkins := &v1alpha2.Jenkins{
+			Spec: v1alpha2.JenkinsSpec{
+				Master: v1alpha2.JenkinsMaster{
+					Containers: []v1alpha2.Container{
+						{
+							Name: resources.JenkinsMasterContainerName,
+						},
+					},
+				},
+			},
+		}
+		baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{})
+
+		got := baseReconcileLoop.validateJenkinsMasterContainerCommand()
+
+		assert.Len(t, got, 1)
+	})
+	t.Run("command has 3 lines but it's values are invalid", func(t *testing.T) {
+		jenkins := &v1alpha2.Jenkins{
+			Spec: v1alpha2.JenkinsSpec{
+				Master: v1alpha2.JenkinsMaster{
+					Containers: []v1alpha2.Container{
+						{
+							Name: resources.JenkinsMasterContainerName,
+							Command: []string{
+								"invalid",
+								"invalid",
+								"invalid",
+							},
+						},
+					},
+				},
+			},
+		}
+		baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{})
+
+		got := baseReconcileLoop.validateJenkinsMasterContainerCommand()
+
+		assert.Len(t, got, 1)
+	})
+	t.Run("valid command", func(t *testing.T) {
+		jenkins := &v1alpha2.Jenkins{
+			Spec: v1alpha2.JenkinsSpec{
+				Master: v1alpha2.JenkinsMaster{
+					Containers: []v1alpha2.Container{
+						{
+							Name:    resources.JenkinsMasterContainerName,
+							Command: resources.GetJenkinsMasterContainerBaseCommand(),
+						},
+					},
+				},
+			},
+		}
+		baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{})
+
+		got := baseReconcileLoop.validateJenkinsMasterContainerCommand()
+
+		assert.Len(t, got, 0)
+	})
+	t.Run("custom valid command", func(t *testing.T) {
+		jenkins := &v1alpha2.Jenkins{
+			Spec: v1alpha2.JenkinsSpec{
+				Master: v1alpha2.JenkinsMaster{
+					Containers: []v1alpha2.Container{
+						{
+							Name: resources.JenkinsMasterContainerName,
+							Command: []string{
+								"bash",
+								"-c",
+								fmt.Sprintf("%s/%s && my-extra-command.sh && exec /sbin/tini -s -- /usr/local/bin/jenkins.sh",
+									resources.JenkinsScriptsVolumePath, resources.InitScriptName),
+							},
+						},
+					},
+				},
+			},
+		}
+		baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{})
+
+		got := baseReconcileLoop.validateJenkinsMasterContainerCommand()
+
+		assert.Len(t, got, 0)
+	})
+	t.Run("no exec command for the Jenkins", func(t *testing.T) {
+		jenkins := &v1alpha2.Jenkins{
+			Spec: v1alpha2.JenkinsSpec{
+				Master: v1alpha2.JenkinsMaster{
+					Containers: []v1alpha2.Container{
+						{
+							Name: resources.JenkinsMasterContainerName,
+							Command: []string{
+								"bash",
+								"-c",
+								fmt.Sprintf("%s/%s && my-extra-command.sh && /sbin/tini -s -- /usr/local/bin/jenkins.sh",
+									resources.JenkinsScriptsVolumePath, resources.InitScriptName),
+							},
+						},
+					},
+				},
+			},
+		}
+		baseReconcileLoop := New(configuration.Configuration{Jenkins: jenkins}, log.Log, client.JenkinsAPIConnectionSettings{})
+
+		got := baseReconcileLoop.validateJenkinsMasterContainerCommand()
+
+		assert.Len(t, got, 1)
+	})
+}
diff --git a/pkg/controller/jenkins/configuration/configuration.go b/pkg/controller/jenkins/configuration/configuration.go
@@ -150,3 +150,12 @@ func (c *Configuration) Exec(podName, containerName string, command []string) (s
 
 	return
 }
+
+// GetJenkinsMasterContainer returns the Jenkins master container from the CR
+func (c *Configuration) GetJenkinsMasterContainer() *v1alpha2.Container {
+	if len(c.Jenkins.Spec.Master.Containers) > 0 {
+		// the first container is the Jenkins master, it is forced jenkins_controller.go
+		return &c.Jenkins.Spec.Master.Containers[0]
+	}
+	return nil
+}