Improved notification tests, refactor ConfigurationType to Phase

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

@@ -6,13 +6,11 @@ import (
 	"regexp"
 	"strings"
 
+	docker "github.com/docker/distribution/reference"
 	"github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2"
 	"github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/configuration/base/resources"
 	"github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/constants"
 	"github.com/jenkinsci/kubernetes-operator/pkg/controller/jenkins/plugins"
-	"github.com/jenkinsci/kubernetes-operator/pkg/log"
-
-	docker "github.com/docker/distribution/reference"
 	stackerr "github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -27,38 +25,38 @@ var (
 func (r *ReconcileJenkinsBaseConfiguration) Validate(jenkins *v1alpha2.Jenkins) ([]string, error) {
 	var messages []string
 
-	if msg := r.validateReservedVolumes(); msg != nil {
+	if msg := r.validateReservedVolumes(); len(msg) > 0 {
 		messages = append(messages, msg...)
 	}
 
 	if msg, err := r.validateVolumes(); err != nil {
 		return nil, err
-	} else if msg != nil {
+	} else if len(msg) > 0 {
 		messages = append(messages, msg...)
 	}
 
 	for _, container := range jenkins.Spec.Master.Containers {
-		if msg := r.validateContainer(container); msg != nil {
+		if msg := r.validateContainer(container); len(msg) > 0 {
 			messages = append(messages, msg...)
 		}
 	}
 
-	if msg := r.validatePlugins(plugins.BasePlugins(), jenkins.Spec.Master.BasePlugins, jenkins.Spec.Master.Plugins); msg != nil {
+	if msg := r.validatePlugins(plugins.BasePlugins(), jenkins.Spec.Master.BasePlugins, jenkins.Spec.Master.Plugins); len(msg) > 0 {
 		messages = append(messages, msg...)
 	}
 
-	if msg := r.validateJenkinsMasterPodEnvs(); msg != nil {
+	if msg := r.validateJenkinsMasterPodEnvs(); len(msg) > 0 {
 		messages = append(messages, msg...)
 	}
 
 	if msg, err := r.validateCustomization(r.jenkins.Spec.GroovyScripts.Customization, "spec.groovyScripts"); err != nil {
 		return nil, err
-	} else if msg != nil {
+	} else if len(msg) > 0 {
 		messages = append(messages, msg...)
 	}
 	if msg, err := r.validateCustomization(r.jenkins.Spec.ConfigurationAsCode.Customization, "spec.configurationAsCode"); err != nil {
 		return nil, err
-	} else if msg != nil {
+	} else if len(msg) > 0 {
 		messages = append(messages, msg...)
 	}
 
@@ -72,7 +70,7 @@ func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecrets() ([]string
 		if err != nil {
 			return nil, err
 		}
-		if msg != nil {
+		if len(msg) > 0 {
 			messages = append(messages, msg...)
 		}
 	}
@@ -112,19 +110,19 @@ func (r *ReconcileJenkinsBaseConfiguration) validateVolumes() ([]string, error)
 		case volume.ConfigMap != nil:
 			if msg, err := r.validateConfigMapVolume(volume); err != nil {
 				return nil, err
-			} else if msg != nil {
+			} else if len(msg) > 0 {
 				messages = append(messages, msg...)
 			}
 		case volume.Secret != nil:
 			if msg, err := r.validateSecretVolume(volume); err != nil {
 				return nil, err
-			} else if msg != nil {
+			} else if len(msg) > 0 {
 				messages = append(messages, msg...)
 			}
 		case volume.PersistentVolumeClaim != nil:
 			if msg, err := r.validatePersistentVolumeClaim(volume); err != nil {
 				return nil, err
-			} else if msg != nil {
+			} else if len(msg) > 0 {
 				messages = append(messages, msg...)
 			}
 		default: //TODO add support for rest of volumes
@@ -211,7 +209,7 @@ func (r *ReconcileJenkinsBaseConfiguration) validateContainer(container v1alpha2
 		messages = append(messages, "Image pull policy not set")
 	}
 
-	if msg := r.validateContainerVolumeMounts(container); msg != nil {
+	if msg := r.validateContainerVolumeMounts(container); len(msg) > 0 {
 		messages = append(messages, msg...)
 	}
 
@@ -307,19 +305,19 @@ func (r *ReconcileJenkinsBaseConfiguration) validatePlugins(requiredBasePlugins
 		}
 	}
 
-	if !plugins.VerifyDependencies(allPlugins) {
-		messages = append(messages, "Invalid dependencies")
+	if msg := plugins.VerifyDependencies(allPlugins); len(msg) > 0 {
+		messages = append(messages, msg...)
 	}
 
-	if !r.verifyBasePlugins(requiredBasePlugins, basePlugins) {
-		messages = append(messages, "Failed to verify base plugins")
+	if msg := r.verifyBasePlugins(requiredBasePlugins, basePlugins); len(msg) > 0 {
+		messages = append(messages, msg...)
 	}
 
 	return messages
 }
 
-func (r *ReconcileJenkinsBaseConfiguration) verifyBasePlugins(requiredBasePlugins []plugins.Plugin, basePlugins []v1alpha2.Plugin) bool {
-	valid := true
+func (r *ReconcileJenkinsBaseConfiguration) verifyBasePlugins(requiredBasePlugins []plugins.Plugin, basePlugins []v1alpha2.Plugin) []string {
+	var messages []string
 
 	for _, requiredBasePlugin := range requiredBasePlugins {
 		found := false
@@ -330,12 +328,11 @@ func (r *ReconcileJenkinsBaseConfiguration) verifyBasePlugins(requiredBasePlugin
 			}
 		}
 		if !found {
-			valid = false
-			r.logger.V(log.VWarn).Info(fmt.Sprintf("Missing plugin '%s' in spec.master.basePlugins", requiredBasePlugin.Name))
+			messages = append(messages, fmt.Sprintf("Missing plugin '%s' in spec.master.basePlugins", requiredBasePlugin.Name))
 		}
 	}
 
-	return valid
+	return messages
 }
 
 func (r *ReconcileJenkinsBaseConfiguration) validateCustomization(customization v1alpha2.Customization, name string) ([]string, error) {

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

@@ -48,7 +48,7 @@ func TestValidatePlugins(t *testing.T) {
 
 		got := baseReconcileLoop.validatePlugins(requiredBasePlugins, basePlugins, userPlugins)
 
-		assert.NotNil(t, got)
+		assert.Equal(t, got, []string{"invalid plugin name 'INVALID?:0.0.1', must follow pattern '(?i)^[0-9a-z-_]+$'"})
 	})
 	t.Run("invalid user plugin version", func(t *testing.T) {
 		var requiredBasePlugins []plugins.Plugin
@@ -57,7 +57,7 @@ func TestValidatePlugins(t *testing.T) {
 
 		got := baseReconcileLoop.validatePlugins(requiredBasePlugins, basePlugins, userPlugins)
 
-		assert.NotNil(t, got)
+		assert.Equal(t, got, []string{"invalid plugin version 'simple-plugin:invalid', must follow pattern '^[0-9\\\\.]+$'"})
 	})
 	t.Run("valid base plugin", func(t *testing.T) {
 		var requiredBasePlugins []plugins.Plugin
@@ -75,7 +75,7 @@ func TestValidatePlugins(t *testing.T) {
 
 		got := baseReconcileLoop.validatePlugins(requiredBasePlugins, basePlugins, userPlugins)
 
-		assert.NotNil(t, got)
+		assert.Equal(t, got, []string{"invalid plugin name 'INVALID?:0.0.1', must follow pattern '(?i)^[0-9a-z-_]+$'"})
 	})
 	t.Run("invalid base plugin version", func(t *testing.T) {
 		var requiredBasePlugins []plugins.Plugin
@@ -84,7 +84,7 @@ func TestValidatePlugins(t *testing.T) {
 
 		got := baseReconcileLoop.validatePlugins(requiredBasePlugins, basePlugins, userPlugins)
 
-		assert.NotNil(t, got)
+		assert.Equal(t, got, []string{"invalid plugin version 'simple-plugin:invalid', must follow pattern '^[0-9\\\\.]+$'"})
 	})
 	t.Run("valid user and base plugin version", func(t *testing.T) {
 		var requiredBasePlugins []plugins.Plugin
@@ -129,7 +129,7 @@ func TestValidatePlugins(t *testing.T) {
 
 		got := baseReconcileLoop.validatePlugins(requiredBasePlugins, basePlugins, userPlugins)
 
-		assert.NotNil(t, got)
+		assert.Equal(t, got, []string{"Missing plugin 'simple-plugin' in spec.master.basePlugins"})
 	})
 }
 
@@ -186,7 +186,8 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T
 			&jenkins, false, false, nil, nil)
 
 		got, _ := baseReconcileLoop.validateImagePullSecrets()
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Secret test-ref not found defined in spec.master.imagePullSecrets", "Secret 'test-ref' defined in spec.master.imagePullSecrets doesn't have 'docker-server' key.", "Secret 'test-ref' defined in spec.master.imagePullSecrets doesn't have 'docker-username' key.", "Secret 'test-ref' defined in spec.master.imagePullSecrets doesn't have 'docker-password' key.", "Secret 'test-ref' defined in spec.master.imagePullSecrets doesn't have 'docker-email' key."})
 	})
 
 	t.Run("no docker email", func(t *testing.T) {
@@ -219,7 +220,8 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T
 			&jenkins, false, false, nil, nil)
 
 		got, _ := baseReconcileLoop.validateImagePullSecrets()
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Secret 'test-ref' defined in spec.master.imagePullSecrets doesn't have 'docker-email' key."})
 	})
 
 	t.Run("no docker password", func(t *testing.T) {
@@ -252,7 +254,8 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T
 			&jenkins, false, false, nil, nil)
 
 		got, _ := baseReconcileLoop.validateImagePullSecrets()
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Secret 'test-ref' defined in spec.master.imagePullSecrets doesn't have 'docker-password' key."})
 	})
 
 	t.Run("no docker username", func(t *testing.T) {
@@ -285,7 +288,8 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T
 			&jenkins, false, false, nil, nil)
 
 		got, _ := baseReconcileLoop.validateImagePullSecrets()
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Secret 'test-ref' defined in spec.master.imagePullSecrets doesn't have 'docker-username' key."})
 	})
 
 	t.Run("no docker server", func(t *testing.T) {
@@ -318,7 +322,8 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T
 			&jenkins, false, false, nil, nil)
 
 		got, _ := baseReconcileLoop.validateImagePullSecrets()
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Secret 'test-ref' defined in spec.master.imagePullSecrets doesn't have 'docker-server' key."})
 	})
 }
 
@@ -374,7 +379,8 @@ func TestValidateJenkinsMasterPodEnvs(t *testing.T) {
 		baseReconcileLoop := New(nil, nil, logf.ZapLogger(false),
 			&jenkins, false, false, nil, nil)
 		got := baseReconcileLoop.validateJenkinsMasterPodEnvs()
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Jenkins Master container env 'JENKINS_HOME' cannot be overridden"})
 	})
 	t.Run("missing -Djava.awt.headless=true in JAVA_OPTS env", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -396,7 +402,8 @@ func TestValidateJenkinsMasterPodEnvs(t *testing.T) {
 		baseReconcileLoop := New(nil, nil, logf.ZapLogger(false),
 			&jenkins, false, false, nil, nil)
 		got := baseReconcileLoop.validateJenkinsMasterPodEnvs()
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Jenkins Master container env 'JAVA_OPTS' doesn't have required flag '-Djava.awt.headless=true'"})
 	})
 	t.Run("missing -Djenkins.install.runSetupWizard=false in JAVA_OPTS env", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -418,7 +425,8 @@ func TestValidateJenkinsMasterPodEnvs(t *testing.T) {
 		baseReconcileLoop := New(nil, nil, logf.ZapLogger(false),
 			&jenkins, false, false, nil, nil)
 		got := baseReconcileLoop.validateJenkinsMasterPodEnvs()
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Jenkins Master container env 'JAVA_OPTS' doesn't have required flag '-Djenkins.install.runSetupWizard=false'"})
 	})
 }
 
@@ -455,7 +463,8 @@ func TestValidateReservedVolumes(t *testing.T) {
 		baseReconcileLoop := New(nil, nil, logf.ZapLogger(false),
 			&jenkins, false, false, nil, nil)
 		got := baseReconcileLoop.validateReservedVolumes()
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Jenkins Master pod volume 'jenkins-home' is reserved please choose different one"})
 	})
 }
 
@@ -545,7 +554,8 @@ func TestValidateContainerVolumeMounts(t *testing.T) {
 		baseReconcileLoop := New(nil, nil, logf.ZapLogger(false),
 			&jenkins, false, false, nil, nil)
 		got := baseReconcileLoop.validateContainerVolumeMounts(jenkins.Spec.Master.Containers[0])
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Not found volume for 'missing-volume' volume mount in container ''"})
 	})
 }
 
@@ -618,7 +628,8 @@ func TestValidateConfigMapVolume(t *testing.T) {
 		got, err := baseReconcileLoop.validateConfigMapVolume(volume)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"ConfigMap 'configmap-name' not found for volume '{volume-name {nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil &ConfigMapVolumeSource{LocalObjectReference:LocalObjectReference{Name:configmap-name,},Items:[],DefaultMode:nil,Optional:*false,} nil nil nil nil nil nil nil nil}}'"})
 	})
 }
 
@@ -687,7 +698,8 @@ func TestValidateSecretVolume(t *testing.T) {
 		got, err := baseReconcileLoop.validateSecretVolume(volume)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Secret 'secret-name' not found for volume '{volume-name {nil nil nil nil nil &SecretVolumeSource{SecretName:secret-name,Items:[],DefaultMode:nil,Optional:*false,} nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil}}'"})
 	})
 }
 
@@ -731,7 +743,8 @@ func TestValidateCustomization(t *testing.T) {
 		got, err := baseReconcileLoop.validateCustomization(customization, "spec.groovyScripts")
 
 		assert.NoError(t, err)
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"spec.groovyScripts.secret.name is set but spec.groovyScripts.configurations is empty"})
 	})
 	t.Run("secret and configmap exists", func(t *testing.T) {
 		customization := v1alpha2.Customization{
@@ -784,7 +797,8 @@ func TestValidateCustomization(t *testing.T) {
 		got, err := baseReconcileLoop.validateCustomization(customization, "spec.groovyScripts")
 
 		assert.NoError(t, err)
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"Secret 'secretName' configured in spec.groovyScripts.secret.name not found"})
 	})
 	t.Run("secret exists and configmap not exists", func(t *testing.T) {
 		customization := v1alpha2.Customization{
@@ -806,6 +820,7 @@ func TestValidateCustomization(t *testing.T) {
 		got, err := baseReconcileLoop.validateCustomization(customization, "spec.groovyScripts")
 
 		assert.NoError(t, err)
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"ConfigMap 'configmap-name' configured in spec.groovyScripts.configurations[0] not found"})
 	})
 }

pkg/controller/jenkins/configuration/user/seedjobs/validate.go

@@ -8,9 +8,6 @@ import (
 	"strings"
 
 	"github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2"
-	"github.com/jenkinsci/kubernetes-operator/pkg/log"
-
-	"github.com/go-logr/logr"
 	stackerr "github.com/pkg/errors"
 	"github.com/robfig/cron"
 	"k8s.io/api/core/v1"
@@ -22,41 +19,39 @@ import (
 func (s *SeedJobs) ValidateSeedJobs(jenkins v1alpha2.Jenkins) ([]string, error) {
 	var messages []string
 
-	if msg := s.validateIfIDIsUnique(jenkins.Spec.SeedJobs); msg != nil {
+	if msg := s.validateIfIDIsUnique(jenkins.Spec.SeedJobs); len(msg) > 0 {
 		messages = append(messages, msg...)
 	}
 
 	for _, seedJob := range jenkins.Spec.SeedJobs {
-		logger := s.logger.WithValues("seedJob", seedJob.ID).V(log.VWarn)
-
 		if len(seedJob.ID) == 0 {
-			messages = append(messages, "id can't be empty")
+			messages = append(messages, fmt.Sprintf("seedJob `%s` id can't be empty", seedJob.ID))
 		}
 
 		if len(seedJob.RepositoryBranch) == 0 {
-			messages = append(messages, "repository branch can't be empty")
+			messages = append(messages, fmt.Sprintf("seedJob `%s` repository branch can't be empty", seedJob.ID))
 		}
 
 		if len(seedJob.RepositoryURL) == 0 {
-			messages = append(messages, "repository URL branch can't be empty")
+			messages = append(messages, fmt.Sprintf("seedJob `%s` repository URL branch can't be empty", seedJob.ID))
 		}
 
 		if len(seedJob.Targets) == 0 {
-			messages = append(messages, "targets can't be empty")
+			messages = append(messages, fmt.Sprintf("seedJob `%s` targets can't be empty", seedJob.ID))
 		}
 
 		if _, ok := v1alpha2.AllowedJenkinsCredentialMap[string(seedJob.JenkinsCredentialType)]; !ok {
-			messages = append(messages, "unknown credential type")
+			messages = append(messages, fmt.Sprintf("seedJob `%s` unknown credential type", seedJob.ID))
 		}
 
 		if (seedJob.JenkinsCredentialType == v1alpha2.BasicSSHCredentialType ||
 			seedJob.JenkinsCredentialType == v1alpha2.UsernamePasswordCredentialType) && len(seedJob.CredentialID) == 0 {
-			messages = append(messages, "credential ID can't be empty")
+			messages = append(messages, fmt.Sprintf("seedJob `%s` credential ID can't be empty", seedJob.ID))
 		}
 
 		// validate repository url match private key
 		if strings.Contains(seedJob.RepositoryURL, "git@") && seedJob.JenkinsCredentialType == v1alpha2.NoJenkinsCredentialCredentialType {
-			messages = append(messages, "Jenkins credential must be set while using ssh repository url")
+			messages = append(messages, fmt.Sprintf("seedJob `%s` Jenkins credential must be set while using ssh repository url", seedJob.ID))
 		}
 
 		if seedJob.JenkinsCredentialType == v1alpha2.BasicSSHCredentialType || seedJob.JenkinsCredentialType == v1alpha2.UsernamePasswordCredentialType {
@@ -64,38 +59,48 @@ func (s *SeedJobs) ValidateSeedJobs(jenkins v1alpha2.Jenkins) ([]string, error)
 			namespaceName := types.NamespacedName{Namespace: jenkins.Namespace, Name: seedJob.CredentialID}
 			err := s.k8sClient.Get(context.TODO(), namespaceName, secret)
 			if err != nil && apierrors.IsNotFound(err) {
-				messages = append(messages, fmt.Sprintf("required secret '%s' with Jenkins credential not found", seedJob.CredentialID))
+				messages = append(messages, fmt.Sprintf("seedJob `%s` required secret '%s' with Jenkins credential not found", seedJob.ID, seedJob.CredentialID))
 			} else if err != nil {
 				return nil, stackerr.WithStack(err)
 			}
 
 			if seedJob.JenkinsCredentialType == v1alpha2.BasicSSHCredentialType {
-				if msg := validateBasicSSHSecret(logger, *secret); msg != nil {
-					messages = append(messages, msg...)
+				if msg := validateBasicSSHSecret(*secret); len(msg) > 0 {
+					for _, m := range msg {
+						messages = append(messages, fmt.Sprintf("seedJob `%s` %s", seedJob.ID, m))
+					}
 				}
 			}
 			if seedJob.JenkinsCredentialType == v1alpha2.UsernamePasswordCredentialType {
-				if msg := validateUsernamePasswordSecret(logger, *secret); msg != nil {
-					messages = append(messages, msg...)
+				if msg := validateUsernamePasswordSecret(*secret); len(msg) > 0 {
+					for _, m := range msg {
+						messages = append(messages, fmt.Sprintf("seedJob `%s` %s", seedJob.ID, m))
+					}
 				}
 			}
 		}
 
 		if len(seedJob.BuildPeriodically) > 0 {
-			if msg := s.validateSchedule(seedJob, seedJob.BuildPeriodically, "buildPeriodically"); msg != nil {
-				messages = append(messages, msg...)
+			if msg := s.validateSchedule(seedJob, seedJob.BuildPeriodically, "buildPeriodically"); len(msg) > 0 {
+				for _, m := range msg {
+					messages = append(messages, fmt.Sprintf("seedJob `%s` %s", seedJob.ID, m))
+				}
 			}
 		}
 
 		if len(seedJob.PollSCM) > 0 {
-			if msg := s.validateSchedule(seedJob, seedJob.PollSCM, "pollSCM"); msg != nil {
-				messages = append(messages, msg...)
+			if msg := s.validateSchedule(seedJob, seedJob.PollSCM, "pollSCM"); len(msg) > 0 {
+				for _, m := range msg {
+					messages = append(messages, fmt.Sprintf("seedJob `%s` %s", seedJob.ID, m))
+				}
 			}
 		}
 
 		if seedJob.GitHubPushTrigger {
-			if msg := s.validateGitHubPushTrigger(jenkins); msg != nil {
-				messages = append(messages, msg...)
+			if msg := s.validateGitHubPushTrigger(jenkins); len(msg) > 0 {
+				for _, m := range msg {
+					messages = append(messages, fmt.Sprintf("seedJob `%s` %s", seedJob.ID, m))
+				}
 			}
 		}
 	}
@@ -146,7 +151,7 @@ func (s *SeedJobs) validateIfIDIsUnique(seedJobs []v1alpha2.SeedJob) []string {
 	return messages
 }
 
-func validateBasicSSHSecret(logger logr.InfoLogger, secret v1.Secret) []string {
+func validateBasicSSHSecret(secret v1.Secret) []string {
 	var messages []string
 	username, exists := secret.Data[UsernameSecretKey]
 	if !exists {
@@ -170,7 +175,7 @@ func validateBasicSSHSecret(logger logr.InfoLogger, secret v1.Secret) []string {
 	return messages
 }
 
-func validateUsernamePasswordSecret(logger logr.InfoLogger, secret v1.Secret) []string {
+func validateUsernamePasswordSecret(secret v1.Secret) []string {
 	var messages []string
 	username, exists := secret.Data[UsernameSecretKey]
 	if !exists {

pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go

@@ -96,7 +96,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `` id can't be empty"})
 	})
 	t.Run("Valid with private key and secret", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -162,7 +163,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` private key 'privateKey' invalid in secret 'deploy-keys': failed to decode PEM block"})
 	})
 	t.Run("Invalid with PrivateKey and empty Secret data", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -195,7 +197,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` required data 'privateKey' not found in secret 'deploy-keys'", "seedJob `example` private key 'privateKey' invalid in secret 'deploy-keys': failed to decode PEM block"})
 	})
 	t.Run("Invalid with ssh RepositoryURL and empty PrivateKey", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -217,7 +220,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string([]string{"seedJob `example` required secret 'jenkins-operator-e2e' with Jenkins credential not found", "seedJob `example` required data 'username' not found in secret ''", "seedJob `example` required data 'username' is empty in secret ''", "seedJob `example` required data 'privateKey' not found in secret ''", "seedJob `example` required data 'privateKey' not found in secret ''", "seedJob `example` private key 'privateKey' invalid in secret '': failed to decode PEM block"}))
 	})
 	t.Run("Invalid without targets", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -237,7 +241,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` targets can't be empty"})
 	})
 	t.Run("Invalid without repository URL", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -257,7 +262,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` repository URL branch can't be empty"})
 	})
 	t.Run("Invalid without repository branch", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -277,7 +283,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` repository branch can't be empty"})
 	})
 	t.Run("Valid with username and password", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -343,7 +350,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` required data 'username' is empty in secret 'deploy-keys'"})
 	})
 	t.Run("Invalid with empty password", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -376,7 +384,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` required data 'password' is empty in secret 'deploy-keys'"})
 	})
 	t.Run("Invalid without username", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -408,7 +417,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` required data 'username' not found in secret 'deploy-keys'", "seedJob `example` required data 'username' is empty in secret 'deploy-keys'"})
 	})
 	t.Run("Invalid without password", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -440,7 +450,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` required data 'password' not found in secret 'deploy-keys'", "seedJob `example` required data 'password' is empty in secret 'deploy-keys'"})
 	})
 	t.Run("Invalid with wrong cron spec", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -463,7 +474,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` `buildPeriodically` schedule 'invalid-cron-spec' is invalid cron spec in `example`"})
 	})
 	t.Run("Valid with good cron spec", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -510,7 +522,8 @@ func TestValidateSeedJobs(t *testing.T) {
 		result, err := seedJobs.ValidateSeedJobs(jenkins)
 
 		assert.NoError(t, err)
-		assert.NotNil(t, result)
+
+		assert.Equal(t, result, []string{"seedJob `example` githubPushTrigger is set. This function requires `github` plugin installed in .Spec.Master.Plugins because seed jobs Push Trigger function needs it"})
 	})
 	t.Run("Invalid with set githubPushTrigger and not installed github plugin", func(t *testing.T) {
 		jenkins := v1alpha2.Jenkins{
@@ -557,6 +570,7 @@ func TestValidateIfIDIsUnique(t *testing.T) {
 		}
 		ctrl := New(nil, nil, logf.ZapLogger(false))
 		got := ctrl.validateIfIDIsUnique(seedJobs)
-		assert.NotNil(t, got)
+
+		assert.Equal(t, got, []string{"'first' seed job ID is not unique"})
 	})
 }