#1 Split plugins in two groups:required by operator and required by user

docs/getting-started.md

@@ -6,9 +6,8 @@ This document describes a getting started guide for **jenkins-operator** and an
 2. [Deploy Jenkins](#deploy-jenkins)
 3. [Configure Seed Jobs and Pipelines](#configure-seed-jobs-and-pipelines)
 4. [Install Plugins](#install-plugins)
-5. [Configure Authorization](#configure-authorization)
+5. [Configure Backup & Restore](#configure-backup-&-restore)
-6. [Configure Backup & Restore](#configure-backup-&-restore)
+6. [Debugging](#debugging)
-7. [Debugging](#debugging)
 
 ## First Steps
 
@@ -256,6 +255,27 @@ When **jenkins-operator-user-configuration-example** ConfigMap is updated Jenkin
 
 ## Install Plugins
 
+### Via CR
+
+Edit CR under `spec.master.plugins`:
+
+```
+apiVersion: jenkins.io/v1alpha1
+kind: Jenkins
+metadata:
+  name: example
+spec:
+  master:
+   image: jenkins/jenkins:lts
+   plugins:
+     configuration-as-code:1.4:
+     - configuration-as-code-support:1.4
+```
+
+Then **jenkins-operator** will automatically install plugins after Jenkins master pod restart.
+
+### Via groovy script
+
 To install a plugin please add **2-install-slack-plugin.groovy** script to the **jenkins-operator-user-configuration-example** ConfigMap:
 
 ```
@@ -318,7 +338,7 @@ data:
 
 Then **jenkins-operator** will automatically trigger **jenkins-operator-user-configuration** Jenkins Job again.
 
-## Configure Backup & Restore (work in progress)
+## Configure Backup & Restore
 
 Not implemented yet.
 

pkg/apis/jenkinsio/v1alpha1/jenkins_types.go

@@ -22,7 +22,10 @@ type JenkinsMaster struct {
 	Image       string                      `json:"image,omitempty"`
 	Annotations map[string]string           `json:"masterAnnotations,omitempty"`
 	Resources   corev1.ResourceRequirements `json:"resources,omitempty"`
-	Plugins     map[string][]string         `json:"plugins,omitempty"`
+	// OperatorPlugins contains plugins required by operator
+	OperatorPlugins map[string][]string `json:"basePlugins,omitempty"`
+	// Plugins contains plugins required by user
+	Plugins map[string][]string `json:"plugins,omitempty"`
 }
 
 // JenkinsStatus defines the observed state of Jenkins

pkg/apis/jenkinsio/v1alpha1/zz_generated.deepcopy.go

@@ -121,6 +121,21 @@ func (in *JenkinsMaster) DeepCopyInto(out *JenkinsMaster) {
 		}
 	}
 	in.Resources.DeepCopyInto(&out.Resources)
+	if in.OperatorPlugins != nil {
+		in, out := &in.OperatorPlugins, &out.OperatorPlugins
+		*out = make(map[string][]string, len(*in))
+		for key, val := range *in {
+			var outVal []string
+			if val == nil {
+				(*out)[key] = nil
+			} else {
+				in, out := &val, &outVal
+				*out = make([]string, len(*in))
+				copy(*out, *in)
+			}
+			(*out)[key] = outVal
+		}
+	}
 	if in.Plugins != nil {
 		in, out := &in.Plugins, &out.Plugins
 		*out = make(map[string][]string, len(*in))

pkg/controller/jenkins/configuration/base/reconcile.go

@@ -85,12 +85,13 @@ func (r *ReconcileJenkinsBaseConfiguration) Reconcile() (reconcile.Result, jenki
 	}
 	r.logger.V(log.VDebug).Info("Jenkins API client set")
 
-	ok, err := r.verifyPlugins(jenkinsClient, plugins.BasePluginsMap)
+	ok, err := r.verifyPlugins(jenkinsClient)
 	if err != nil {
 		return reconcile.Result{}, nil, err
 	}
 	if !ok {
-		r.logger.V(log.VWarn).Info("Please correct Jenkins CR (spec.master.plugins)")
+		r.logger.V(log.VWarn).Info("Please correct Jenkins CR(spec.master.OperatorPlugins or spec.master.plugins)")
+		// TODO inform user via Admin Monitor and don't restart Jenkins
 		return reconcile.Result{Requeue: true}, nil, r.restartJenkinsMasterPod(metaObject)
 	}
 
@@ -137,7 +138,7 @@ func (r *ReconcileJenkinsBaseConfiguration) ensureResourcesRequiredForJenkinsPod
 	return nil
 }
 
-func (r *ReconcileJenkinsBaseConfiguration) verifyPlugins(jenkinsClient jenkinsclient.Jenkins, allRequiredPlugins ...map[string][]plugins.Plugin) (bool, error) {
+func (r *ReconcileJenkinsBaseConfiguration) verifyPlugins(jenkinsClient jenkinsclient.Jenkins) (bool, error) {
 	allPluginsInJenkins, err := jenkinsClient.GetPlugins(fetchAllPlugins)
 	if err != nil {
 		return false, err
@@ -151,7 +152,21 @@ func (r *ReconcileJenkinsBaseConfiguration) verifyPlugins(jenkinsClient jenkinsc
 	}
 	r.logger.V(log.VDebug).Info(fmt.Sprintf("Installed plugins '%+v'", installedPlugins))
 
+	userPlugins := map[string][]plugins.Plugin{}
+	for rootPlugin, dependentPluginNames := range r.jenkins.Spec.Master.Plugins {
+		var dependentPlugins []plugins.Plugin
+		for _, pluginNameWithVersion := range dependentPluginNames {
+			plugin, err := plugins.New(pluginNameWithVersion)
+			if err != nil {
+				return false, err
+			}
+			dependentPlugins = append(dependentPlugins, *plugin)
+		}
+		userPlugins[rootPlugin] = dependentPlugins
+	}
+
 	status := true
+	allRequiredPlugins := []map[string][]plugins.Plugin{plugins.BasePluginsMap, userPlugins}
 	for _, requiredPlugins := range allRequiredPlugins {
 		for rootPluginName, p := range requiredPlugins {
 			rootPlugin, _ := plugins.New(rootPluginName)

pkg/controller/jenkins/configuration/base/resources/scripts_configmap.go

@@ -254,12 +254,19 @@ chmod +x {{ .JenkinsHomePath }}/scripts/*.sh
 {{- $jenkinsHomePath := .JenkinsHomePath }}
 {{- $installPluginsCommand := .InstallPluginsCommand }}
 
-echo "Installing plugins - begin"
+echo "Installing plugins required by Operator - begin"
-{{- range $rootPluginName, $plugins := .Plugins }}
+{{- range $rootPluginName, $plugins := .OperatorPlugins }}
 echo "Installing required plugins for '{{ $rootPluginName }}'"
 {{ $jenkinsHomePath }}/scripts/{{ $installPluginsCommand }} {{ $rootPluginName }} {{ range $index, $plugin := $plugins }}{{ . }} {{ end }}
 {{- end }}
-echo "Installing plugins - end"
+echo "Installing plugins required by Operator - end"
+
+echo "Installing plugins required by user - begin"
+{{- range $rootPluginName, $plugins := .UserPlugins }}
+echo "Installing required plugins for '{{ $rootPluginName }}'"
+{{ $jenkinsHomePath }}/scripts/{{ $installPluginsCommand }} {{ $rootPluginName }} {{ range $index, $plugin := $plugins }}{{ . }} {{ end }}
+{{- end }}
+echo "Installing plugins required by user - end"
 
 /sbin/tini -s -- /usr/local/bin/jenkins.sh
 `))
@@ -271,17 +278,19 @@ func buildConfigMapTypeMeta() metav1.TypeMeta {
 	}
 }
 
-func buildInitBashScript(pluginsToInstall map[string][]string) (*string, error) {
+func buildInitBashScript(jenkins *v1alpha1.Jenkins) (*string, error) {
 	data := struct {
 		JenkinsHomePath          string
 		InitConfigurationPath    string
 		InstallPluginsCommand    string
 		JenkinsScriptsVolumePath string
-		Plugins                  map[string][]string
+		OperatorPlugins          map[string][]string
+		UserPlugins              map[string][]string
 	}{
 		JenkinsHomePath:          jenkinsHomePath,
 		InitConfigurationPath:    jenkinsInitConfigurationVolumePath,
-		Plugins:                  pluginsToInstall,
+		OperatorPlugins:          jenkins.Spec.Master.OperatorPlugins,
+		UserPlugins:              jenkins.Spec.Master.Plugins,
 		InstallPluginsCommand:    installPluginsCommand,
 		JenkinsScriptsVolumePath: jenkinsScriptsVolumePath,
 	}
@@ -302,7 +311,7 @@ func getScriptsConfigMapName(jenkins *v1alpha1.Jenkins) string {
 func NewScriptsConfigMap(meta metav1.ObjectMeta, jenkins *v1alpha1.Jenkins) (*corev1.ConfigMap, error) {
 	meta.Name = getScriptsConfigMapName(jenkins)
 
-	initBashScript, err := buildInitBashScript(jenkins.Spec.Master.Plugins)
+	initBashScript, err := buildInitBashScript(jenkins)
 	if err != nil {
 		return nil, err
 	}

pkg/controller/jenkins/configuration/base/validate.go

@@ -28,34 +28,39 @@ func (r *ReconcileJenkinsBaseConfiguration) Validate(jenkins *v1alpha1.Jenkins)
 
 	}
 
-	if !r.validatePlugins(jenkins.Spec.Master.Plugins) {
+	if !r.validatePlugins(jenkins.Spec.Master.OperatorPlugins, jenkins.Spec.Master.Plugins) {
 		return false, nil
 	}
 
 	return true, nil
 }
 
-func (r *ReconcileJenkinsBaseConfiguration) validatePlugins(pluginsWithVersions map[string][]string) bool {
+func (r *ReconcileJenkinsBaseConfiguration) validatePlugins(pluginsWithVersionSlice ...map[string][]string) bool {
 	valid := true
-	allPlugins := map[string][]plugins.Plugin{}
+	allPlugins := map[plugins.Plugin][]plugins.Plugin{}
 
-	for rootPluginName, dependentPluginNames := range pluginsWithVersions {
+	for _, pluginsWithVersions := range pluginsWithVersionSlice {
-		if _, err := plugins.New(rootPluginName); err != nil {
+		for rootPluginName, dependentPluginNames := range pluginsWithVersions {
-			r.logger.V(log.VWarn).Info(fmt.Sprintf("Invalid root plugin name '%s'", rootPluginName))
+			rootPlugin, err := plugins.New(rootPluginName)
-			valid = false
+			if err != nil {
-		}
+				r.logger.V(log.VWarn).Info(fmt.Sprintf("Invalid root plugin name '%s'", rootPluginName))
-
-		dependentPlugins := []plugins.Plugin{}
-		for _, pluginName := range dependentPluginNames {
-			if p, err := plugins.New(pluginName); err != nil {
-				r.logger.V(log.VWarn).Info(fmt.Sprintf("Invalid dependent plugin name '%s' in root plugin '%s'", pluginName, rootPluginName))
 				valid = false
-			} else {
+			}
-				dependentPlugins = append(dependentPlugins, *p)
+
+			var dependentPlugins []plugins.Plugin
+			for _, pluginName := range dependentPluginNames {
+				if p, err := plugins.New(pluginName); err != nil {
+					r.logger.V(log.VWarn).Info(fmt.Sprintf("Invalid dependent plugin name '%s' in root plugin '%s'", pluginName, rootPluginName))
+					valid = false
+				} else {
+					dependentPlugins = append(dependentPlugins, *p)
+				}
+			}
+
+			if rootPlugin != nil {
+				allPlugins[*rootPlugin] = dependentPlugins
 			}
 		}
-
-		allPlugins[rootPluginName] = dependentPlugins
 	}
 
 	if valid {

pkg/controller/jenkins/configuration/base/validate_test.go

@@ -1,7 +1,6 @@
 package base
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -9,50 +8,60 @@ import (
 )
 
 func TestValidatePlugins(t *testing.T) {
-	data := []struct {
-		plugins        map[string][]string
-		expectedResult bool
-	}{
-		{
-			plugins: map[string][]string{
-				"valid-plugin-name:1.0": {
-					"valid-plugin-name:1.0",
-				},
-			},
-			expectedResult: true,
-		},
-		{
-			plugins: map[string][]string{
-				"invalid-plugin-name": {
-					"invalid-plugin-name",
-				},
-			},
-			expectedResult: false,
-		},
-		{
-			plugins: map[string][]string{
-				"valid-plugin-name:1.0": {
-					"valid-plugin-name:1.0",
-					"valid-plugin-name2:1.0",
-				},
-			},
-			expectedResult: true,
-		},
-		{
-			plugins: map[string][]string{
-				"valid-plugin-name:1.0": {},
-			},
-			expectedResult: true,
-		},
-	}
-
 	baseReconcileLoop := New(nil, nil, logf.ZapLogger(false),
 		nil, false, false)
+	t.Run("happy", func(t *testing.T) {
+		plugins := map[string][]string{
+			"valid-plugin-name:1.0": {
+				"valid-plugin-name:1.0",
+			},
+		}
+		got := baseReconcileLoop.validatePlugins(plugins)
+		assert.Equal(t, true, got)
+	})
+	t.Run("fail, no version in plugin name", func(t *testing.T) {
+		plugins := map[string][]string{
+			"invalid-plugin-name": {
+				"invalid-plugin-name",
+			},
+		}
+		got := baseReconcileLoop.validatePlugins(plugins)
+		assert.Equal(t, false, got)
+	})
+	t.Run("fail, no version in root plugin name", func(t *testing.T) {
+		plugins := map[string][]string{
+			"invalid-plugin-name": {
+				"invalid-plugin-name:1.0",
+			},
+		}
+		got := baseReconcileLoop.validatePlugins(plugins)
+		assert.Equal(t, false, got)
+	})
+	t.Run("fail, no version in plugin name", func(t *testing.T) {
+		plugins := map[string][]string{
+			"invalid-plugin-name:1.0": {
+				"invalid-plugin-name",
+			},
+		}
+		got := baseReconcileLoop.validatePlugins(plugins)
+		assert.Equal(t, false, got)
+	})
+	t.Run("happy", func(t *testing.T) {
+		plugins := map[string][]string{
+			"valid-plugin-name:1.0": {
+				"valid-plugin-name:1.0",
+				"valid-plugin-name2:1.0",
+			},
+		}
+		got := baseReconcileLoop.validatePlugins(plugins)
+		assert.Equal(t, true, got)
+	})
+	t.Run("hapy", func(t *testing.T) {
+		plugins := map[string][]string{
+			"valid-plugin-name:1.0": {},
+		}
+		got := baseReconcileLoop.validatePlugins(plugins)
+		assert.Equal(t, true, got)
+	})
 
-	for index, testingData := range data {
-		t.Run(fmt.Sprintf("Testing %d plugins set", index), func(t *testing.T) {
-			result := baseReconcileLoop.validatePlugins(testingData.plugins)
-			assert.Equal(t, testingData.expectedResult, result)
-		})
-	}
 }

pkg/controller/jenkins/jenkins_controller.go

@@ -212,10 +212,14 @@ func (r *ReconcileJenkins) setDefaults(jenkins *v1alpha1.Jenkins, logger logr.Lo
 		changed = true
 		jenkins.Spec.Master.Image = constants.DefaultJenkinsMasterImage
 	}
-	if len(jenkins.Spec.Master.Plugins) == 0 {
+	if len(jenkins.Spec.Master.OperatorPlugins) == 0 {
 		logger.Info("Setting default base plugins")
 		changed = true
-		jenkins.Spec.Master.Plugins = plugins.BasePlugins()
+		jenkins.Spec.Master.OperatorPlugins = plugins.BasePlugins()
+	}
+	if len(jenkins.Spec.Master.Plugins) == 0 {
+		changed = true
+		jenkins.Spec.Master.Plugins = map[string][]string{"simple-theme-plugin:0.5.1": {}}
 	}
 	_, requestCPUSet := jenkins.Spec.Master.Resources.Requests[corev1.ResourceCPU]
 	_, requestMemporySet := jenkins.Spec.Master.Resources.Requests[corev1.ResourceMemory]

pkg/controller/jenkins/plugins/base_plugins.go

@@ -1,56 +1,72 @@
 package plugins
 
 const (
-	// ApacheComponentsClientPlugin is apache-httpcomponents-client-4-api Jenkins plugin with version
+	apacheComponentsClientPlugin = "apache-httpcomponents-client-4-api:4.5.5-3.0"
-	ApacheComponentsClientPlugin = "apache-httpcomponents-client-4-api:4.5.5-3.0"
+	jackson2ADIPlugin            = "jackson2-api:2.9.8"
-	// Jackson2ADIPlugin is jackson2-api-httpcomponents-client-4-api Jenkins plugin with version
+	credentialsPlugin            = "credentials:2.1.18"
-	Jackson2ADIPlugin = "jackson2-api:2.9.8"
+	cloudBeesFolderPlugin        = "cloudbees-folder:6.7"
+	durableTaskPlugin            = "durable-task:1.28"
+	plainCredentialsPlugin       = "plain-credentials:1.5"
+	structsPlugin                = "structs:1.17"
+	workflowStepAPIPlugin        = "workflow-step-api:2.17"
+	scmAPIPlugin                 = "scm-api:2.3.0"
+	workflowAPIPlugin            = "workflow-api:2.33"
+	workflowSupportPlugin        = "workflow-support:3.0"
+	displayURLAPIPlugin          = "display-url-api:2.3.0"
+	gitClientPlugin              = "git-client:2.7.6"
+	jschPlugin                   = "jsch:0.1.55"
+	junitPlugin                  = "junit:1.26.1"
+	mailerPlugin                 = "mailer:1.23"
+	matrixProjectPlugin          = "matrix-project:1.13"
+	scriptSecurityPlugin         = "script-security:1.50"
+	sshCredentialsPlugin         = "ssh-credentials:1.14"
+	workflowSCMStepPlugin        = "workflow-scm-step:2.7"
 )
 
 // BasePluginsMap contains plugins to install by operator
 var BasePluginsMap = map[string][]Plugin{
 	Must(New("kubernetes:1.13.8")).String(): {
-		Must(New(ApacheComponentsClientPlugin)),
+		Must(New(apacheComponentsClientPlugin)),
-		Must(New("cloudbees-folder:6.7")),
+		Must(New(cloudBeesFolderPlugin)),
-		Must(New("credentials:2.1.18")),
+		Must(New(credentialsPlugin)),
-		Must(New("durable-task:1.28")),
+		Must(New(durableTaskPlugin)),
-		Must(New(Jackson2ADIPlugin)),
+		Must(New(jackson2ADIPlugin)),
 		Must(New("kubernetes-credentials:0.4.0")),
-		Must(New("plain-credentials:1.5")),
+		Must(New(plainCredentialsPlugin)),
-		Must(New("structs:1.17")),
+		Must(New(structsPlugin)),
 		Must(New("variant:1.1")),
-		Must(New("workflow-step-api:2.17")),
+		Must(New(workflowStepAPIPlugin)),
 	},
 	Must(New("workflow-job:2.31")).String(): {
-		Must(New("scm-api:2.3.0")),
+		Must(New(scmAPIPlugin)),
-		Must(New("script-security:1.50")),
+		Must(New(scriptSecurityPlugin)),
-		Must(New("structs:1.17")),
+		Must(New(structsPlugin)),
-		Must(New("workflow-api:2.33")),
+		Must(New(workflowAPIPlugin)),
-		Must(New("workflow-step-api:2.17")),
+		Must(New(workflowStepAPIPlugin)),
-		Must(New("workflow-support:3.0")),
+		Must(New(workflowSupportPlugin)),
 	},
 	Must(New("workflow-aggregator:2.6")).String(): {
 		Must(New("ace-editor:1.1")),
-		Must(New(ApacheComponentsClientPlugin)),
+		Must(New(apacheComponentsClientPlugin)),
 		Must(New("authentication-tokens:1.3")),
 		Must(New("branch-api:2.1.2")),
-		Must(New("cloudbees-folder:6.7")),
+		Must(New(cloudBeesFolderPlugin)),
 		Must(New("credentials-binding:1.17")),
-		Must(New("credentials:2.1.18")),
+		Must(New(credentialsPlugin)),
-		Must(New("display-url-api:2.3.0")),
+		Must(New(displayURLAPIPlugin)),
 		Must(New("docker-commons:1.13")),
 		Must(New("docker-workflow:1.17")),
-		Must(New("durable-task:1.28")),
+		Must(New(durableTaskPlugin)),
-		Must(New("git-client:2.7.6")),
+		Must(New(gitClientPlugin)),
 		Must(New("git-server:1.7")),
 		Must(New("handlebars:1.1.1")),
-		Must(New(Jackson2ADIPlugin)),
+		Must(New(jackson2ADIPlugin)),
 		Must(New("jquery-detached:1.2.1")),
-		Must(New("jsch:0.1.55")),
+		Must(New(jschPlugin)),
-		Must(New("junit:1.26.1")),
+		Must(New(junitPlugin)),
 		Must(New("lockable-resources:2.3")),
-		Must(New("mailer:1.23")),
+		Must(New(mailerPlugin)),
-		Must(New("matrix-project:1.13")),
+		Must(New(matrixProjectPlugin)),
 		Must(New("momentjs:1.1.1")),
 		Must(New("pipeline-build-step:2.7")),
 		Must(New("pipeline-graph-analysis:1.9")),
@@ -64,48 +80,46 @@ var BasePluginsMap = map[string][]Plugin{
 		Must(New("pipeline-stage-step:2.3")),
 		Must(New("pipeline-stage-tags-metadata:1.3.4.1")),
 		Must(New("pipeline-stage-view:2.10")),
-		Must(New("plain-credentials:1.5")),
+		Must(New(plainCredentialsPlugin)),
-		Must(New("scm-api:2.3.0")),
+		Must(New(scmAPIPlugin)),
-		Must(New("script-security:1.50")),
+		Must(New(scriptSecurityPlugin)),
-		Must(New("ssh-credentials:1.14")),
+		Must(New(sshCredentialsPlugin)),
-		Must(New("structs:1.17")),
+		Must(New(structsPlugin)),
-		Must(New("workflow-api:2.33")),
+		Must(New(workflowAPIPlugin)),
 		Must(New("workflow-basic-steps:2.13")),
 		Must(New("workflow-cps-global-lib:2.12")),
 		Must(New("workflow-cps:2.61.1")),
 		Must(New("workflow-durable-task-step:2.27")),
 		Must(New("workflow-job:2.31")),
 		Must(New("workflow-multibranch:2.20")),
-		Must(New("workflow-scm-step:2.7")),
+		Must(New(workflowSCMStepPlugin)),
-		Must(New("workflow-step-api:2.17")),
+		Must(New(workflowStepAPIPlugin)),
-		Must(New("workflow-support:3.0")),
+		Must(New(workflowSupportPlugin)),
 	},
 	Must(New("git:3.9.1")).String(): {
-		Must(New(ApacheComponentsClientPlugin)),
+		Must(New(apacheComponentsClientPlugin)),
-		Must(New("credentials:2.1.18")),
+		Must(New(credentialsPlugin)),
-		Must(New("display-url-api:2.3.0")),
+		Must(New(displayURLAPIPlugin)),
-		Must(New("git-client:2.7.6")),
+		Must(New(gitClientPlugin)),
-		Must(New("jsch:0.1.55")),
+		Must(New(jschPlugin)),
-		Must(New("junit:1.26.1")),
+		Must(New(junitPlugin)),
-		Must(New("mailer:1.23")),
+		Must(New(mailerPlugin)),
-		Must(New("matrix-project:1.13")),
+		Must(New(matrixProjectPlugin)),
-		Must(New("scm-api:2.3.0")),
+		Must(New(scmAPIPlugin)),
-		Must(New("script-security:1.50")),
+		Must(New(scriptSecurityPlugin)),
-		Must(New("ssh-credentials:1.14")),
+		Must(New(sshCredentialsPlugin)),
-		Must(New("structs:1.17")),
+		Must(New(structsPlugin)),
-		Must(New("workflow-api:2.33")),
+		Must(New(workflowAPIPlugin)),
-		Must(New("workflow-scm-step:2.7")),
+		Must(New(workflowSCMStepPlugin)),
-		Must(New("workflow-step-api:2.17")),
+		Must(New(workflowStepAPIPlugin)),
 	},
 	Must(New("job-dsl:1.71")).String(): {
-		Must(New("script-security:1.50")),
+		Must(New(scriptSecurityPlugin)),
-		Must(New("structs:1.17")),
+		Must(New(structsPlugin)),
 	},
-	Must(New("jobConfigHistory:2.19")).String(): {},
 	Must(New("configuration-as-code:1.4")).String(): {
 		Must(New("configuration-as-code-support:1.4")),
 	},
-	Must(New("simple-theme-plugin:0.5.1")).String(): {},
 }
 
 // BasePlugins returns map of plugins to install by operator

pkg/controller/jenkins/plugins/plugin.go

@@ -40,26 +40,22 @@ func Must(plugin *Plugin, err error) Plugin {
 }
 
 // VerifyDependencies checks if all plugins have compatible versions
-func VerifyDependencies(values ...map[string][]Plugin) bool {
+func VerifyDependencies(values ...map[Plugin][]Plugin) bool {
 	// key - plugin name, value array of versions
 	allPlugins := make(map[string][]Plugin)
 	valid := true
 
 	for _, value := range values {
-		for rootPluginNameAndVersion, plugins := range value {
+		for rootPlugin, plugins := range value {
-			if rootPlugin, err := New(rootPluginNameAndVersion); err != nil {
+			allPlugins[rootPlugin.Name] = append(allPlugins[rootPlugin.Name], Plugin{
-				valid = false
+				Name:                     rootPlugin.Name,
-			} else {
+				Version:                  rootPlugin.Version,
-				allPlugins[rootPlugin.Name] = append(allPlugins[rootPlugin.Name], Plugin{
+				rootPluginNameAndVersion: rootPlugin.String()})
-					Name:                     rootPlugin.Name,
-					Version:                  rootPlugin.Version,
-					rootPluginNameAndVersion: rootPluginNameAndVersion})
-			}
 			for _, plugin := range plugins {
 				allPlugins[plugin.Name] = append(allPlugins[plugin.Name], Plugin{
 					Name:                     plugin.Name,
 					Version:                  plugin.Version,
-					rootPluginNameAndVersion: rootPluginNameAndVersion})
+					rootPluginNameAndVersion: rootPlugin.String()})
 			}
 		}
 	}

pkg/controller/jenkins/plugins/plugin_test.go

@@ -1,90 +1,88 @@
 package plugins
 
 import (
-	"fmt"
-	"github.com/jenkinsci/kubernetes-operator/pkg/log"
 	"testing"
 
+	"github.com/jenkinsci/kubernetes-operator/pkg/log"
+
 	"github.com/stretchr/testify/assert"
 )
 
 func TestVerifyDependencies(t *testing.T) {
-	data := []struct {
-		basePlugins    map[string][]Plugin
-		extraPlugins   map[string][]Plugin
-		expectedResult bool
-	}{
-		{
-			basePlugins: map[string][]Plugin{
-				"first-root-plugin:1.0.0": {
-					Must(New("first-plugin:0.0.1")),
-				},
-			},
-			expectedResult: true,
-		},
-		{
-			basePlugins: map[string][]Plugin{
-				"first-root-plugin:1.0.0": {
-					Must(New("first-plugin:0.0.1")),
-				},
-				"second-root-plugin:1.0.0": {
-					Must(New("first-plugin:0.0.1")),
-				},
-			},
-			expectedResult: true,
-		},
-		{
-			basePlugins: map[string][]Plugin{
-				"first-root-plugin:1.0.0": {
-					Must(New("first-plugin:0.0.1")),
-				},
-			},
-			extraPlugins: map[string][]Plugin{
-				"second-root-plugin:2.0.0": {
-					Must(New("first-plugin:0.0.1")),
-				},
-			},
-			expectedResult: true,
-		},
-		{
-			basePlugins: map[string][]Plugin{
-				"first-root-plugin:1.0.0": {
-					Must(New("first-plugin:0.0.1")),
-				},
-				"first-root-plugin:2.0.0": {
-					Must(New("first-plugin:0.0.2")),
-				},
-			},
-			expectedResult: false,
-		},
-		{
-			basePlugins: map[string][]Plugin{
-				"first-root-plugin:1.0.0": {
-					Must(New("first-plugin:0.0.1")),
-				},
-			},
-			extraPlugins: map[string][]Plugin{
-				"first-root-plugin:2.0.0": {
-					Must(New("first-plugin:0.0.2")),
-				},
-			},
-			expectedResult: false,
-		},
-		{
-			basePlugins: map[string][]Plugin{
-				"invalid-plugin-name": {},
-			},
-			expectedResult: false,
-		},
-	}
-
 	debug := false
 	log.SetupLogger(&debug)
 
-	for index, testingData := range data {
+	t.Run("happy, single root plugin with one dependent plugin", func(t *testing.T) {
-		t.Run(fmt.Sprintf("Testing %d data", index), func(t *testing.T) {
+		basePlugins := map[Plugin][]Plugin{
-			result := VerifyDependencies(testingData.basePlugins, testingData.extraPlugins)
+			Must(New("first-root-plugin:1.0.0")): {
-			assert.Equal(t, testingData.expectedResult, result)
+				Must(New("first-plugin:0.0.1")),
-		})
+			},
-	}
+		}
+		got := VerifyDependencies(basePlugins)
+		assert.Equal(t, true, got)
+	})
+	t.Run("happy, two root plugins with one depended plugin with the same version", func(t *testing.T) {
+		basePlugins := map[Plugin][]Plugin{
+			Must(New("first-root-plugin:1.0.0")): {
+				Must(New("first-plugin:0.0.1")),
+			},
+			Must(New("second-root-plugin:1.0.0")): {
+				Must(New("first-plugin:0.0.1")),
+			},
+		}
+		got := VerifyDependencies(basePlugins)
+		assert.Equal(t, true, got)
+	})
+	t.Run("fail, two root plugins have different versions", func(t *testing.T) {
+		basePlugins := map[Plugin][]Plugin{
+			Must(New("first-root-plugin:1.0.0")): {
+				Must(New("first-plugin:0.0.1")),
+			},
+			Must(New("first-root-plugin:2.0.0")): {
+				Must(New("first-plugin:0.0.1")),
+			},
+		}
+		got := VerifyDependencies(basePlugins)
+		assert.Equal(t, false, got)
+	})
+	t.Run("happy, no version collision with two sperate plugins lists", func(t *testing.T) {
+		basePlugins := map[Plugin][]Plugin{
+			Must(New("first-root-plugin:1.0.0")): {
+				Must(New("first-plugin:0.0.1")),
+			},
+		}
+		extraPlugins := map[Plugin][]Plugin{
+			Must(New("second-root-plugin:2.0.0")): {
+				Must(New("first-plugin:0.0.1")),
+			},
+		}
+		got := VerifyDependencies(basePlugins, extraPlugins)
+		assert.Equal(t, true, got)
+	})
+	t.Run("fail, dependent plugins have different versions", func(t *testing.T) {
+		basePlugins := map[Plugin][]Plugin{
+			Must(New("first-root-plugin:1.0.0")): {
+				Must(New("first-plugin:0.0.1")),
+			},
+			Must(New("first-root-plugin:2.0.0")): {
+				Must(New("first-plugin:0.0.2")),
+			},
+		}
+		got := VerifyDependencies(basePlugins)
+		assert.Equal(t, false, got)
+	})
+	t.Run("fail, root and dependent plugins have different versions", func(t *testing.T) {
+		basePlugins := map[Plugin][]Plugin{
+			Must(New("first-root-plugin:1.0.0")): {
+				Must(New("first-plugin:0.0.1")),
+			},
+		}
+		extraPlugins := map[Plugin][]Plugin{
+			Must(New("first-root-plugin:2.0.0")): {
+				Must(New("first-plugin:0.0.2")),
+			},
+		}
+		got := VerifyDependencies(basePlugins, extraPlugins)
+		assert.Equal(t, false, got)
+	})
 }

test/e2e/configuration_test.go

@@ -33,7 +33,7 @@ func TestConfiguration(t *testing.T) {
 
 	verifyJenkinsMasterPodAttributes(t, jenkins)
 	client := verifyJenkinsAPIConnection(t, jenkins)
-	verifyBasePlugins(t, client)
+	verifyPlugins(t, client, jenkins)
 
 	// user
 	waitForJenkinsUserConfigurationToComplete(t, jenkins)
@@ -90,28 +90,48 @@ func verifyJenkinsMasterPodAttributes(t *testing.T, jenkins *v1alpha1.Jenkins) {
 	t.Log("Jenkins pod attributes are valid")
 }
 
-func verifyBasePlugins(t *testing.T, jenkinsClient *gojenkins.Jenkins) {
+func verifyPlugins(t *testing.T, jenkinsClient *gojenkins.Jenkins, jenkins *v1alpha1.Jenkins) {
 	installedPlugins, err := jenkinsClient.GetPlugins(1)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	for rootPluginName, p := range plugins.BasePluginsMap {
+	requiredPlugins := []map[string][]string{plugins.BasePlugins(), jenkins.Spec.Master.Plugins}
-		rootPlugin, err := plugins.New(rootPluginName)
+	for _, p := range requiredPlugins {
-		if err != nil {
+		for rootPluginName, dependentPlugins := range p {
-			t.Fatal(err)
+			rootPlugin, err := plugins.New(rootPluginName)
-		}
+			if err != nil {
-		if found, ok := isPluginValid(installedPlugins, *rootPlugin); !ok {
+				t.Fatal(err)
-			t.Fatalf("Invalid plugin '%s', actual '%+v'", rootPlugin, found)
+			}
-		}
+			if found, ok := isPluginValid(installedPlugins, *rootPlugin); !ok {
-		for _, requiredPlugin := range p {
+				t.Fatalf("Invalid plugin '%s', actual '%+v'", rootPlugin, found)
-			if found, ok := isPluginValid(installedPlugins, requiredPlugin); !ok {
+			}
-				t.Fatalf("Invalid plugin '%s', actual '%+v'", requiredPlugin, found)
+			for _, pluginName := range dependentPlugins {
+				plugin, err := plugins.New(pluginName)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if found, ok := isPluginValid(installedPlugins, *plugin); !ok {
+					t.Fatalf("Invalid plugin '%s', actual '%+v'", rootPlugin, found)
+				}
 			}
 		}
 	}
 
-	t.Log("Base plugins have been installed")
+	t.Log("All plugins have been installed")
+}
+
+func isPluginValid(plugins *gojenkins.Plugins, requiredPlugin plugins.Plugin) (*gojenkins.Plugin, bool) {
+	p := plugins.Contains(requiredPlugin.Name)
+	if p == nil {
+		return p, false
+	}
+
+	if !p.Active || !p.Enabled || p.Deleted {
+		return p, false
+	}
+
+	return p, requiredPlugin.Version == p.Version
 }
 
 func verifyJenkinsSeedJobs(t *testing.T, client *gojenkins.Jenkins, jenkins *v1alpha1.Jenkins) {
@@ -150,16 +170,3 @@ func verifyJenkinsSeedJobs(t *testing.T, client *gojenkins.Jenkins, jenkins *v1a
 	})
 	assert.NoError(t, err)
 }
-
-func isPluginValid(plugins *gojenkins.Plugins, requiredPlugin plugins.Plugin) (*gojenkins.Plugin, bool) {
-	p := plugins.Contains(requiredPlugin.Name)
-	if p == nil {
-		return p, false
-	}
-
-	if !p.Active || !p.Enabled || p.Deleted {
-		return p, false
-	}
-
-	return p, requiredPlugin.Version == p.Version
-}

test/e2e/jenkins.go

@@ -85,6 +85,10 @@ func createJenkinsCR(t *testing.T, namespace string) *v1alpha1.Jenkins {
 			Master: v1alpha1.JenkinsMaster{
 				Image:       "jenkins/jenkins",
 				Annotations: map[string]string{"test": "label"},
+				Plugins: map[string][]string{
+					"audit-trail:2.4":           {},
+					"simple-theme-plugin:0.5.1": {},
+				},
 			},
 			//TODO(bantoniak) add seed job with private key
 			SeedJobs: []v1alpha1.SeedJob{