From 4e8b0fa72ebb21c0ce8daebbce3034b63f8d91f3 Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Thu, 3 Oct 2019 15:53:39 +0200 Subject: [PATCH] Improved notification tests, refactor ConfigurationType to Phase --- .../jenkins/configuration/base/validate.go | 45 ++++---- .../configuration/base/validate_test.go | 55 +++++---- .../configuration/user/seedjobs/validate.go | 57 +++++----- .../user/seedjobs/validate_test.go | 42 ++++--- pkg/controller/jenkins/jenkins_controller.go | 105 ++++++++---------- .../jenkins/notifications/mailgun.go | 4 +- .../jenkins/notifications/msteams.go | 6 +- .../jenkins/notifications/msteams_test.go | 12 +- .../jenkins/notifications/sender.go | 58 +++++----- pkg/controller/jenkins/notifications/slack.go | 6 +- .../jenkins/notifications/slack_test.go | 12 +- pkg/controller/jenkins/plugins/plugin.go | 11 +- pkg/controller/jenkins/plugins/plugin_test.go | 16 +-- 13 files changed, 222 insertions(+), 207 deletions(-) diff --git a/pkg/controller/jenkins/configuration/base/validate.go b/pkg/controller/jenkins/configuration/base/validate.go index 47c4df7b..676f8d8f 100644 --- a/pkg/controller/jenkins/configuration/base/validate.go +++ b/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) { diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go index ca231a73..3a275e8c 100644 --- a/pkg/controller/jenkins/configuration/base/validate_test.go +++ b/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"}) }) } diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/validate.go b/pkg/controller/jenkins/configuration/user/seedjobs/validate.go index d97dfd56..15798637 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/validate.go +++ b/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 { diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go b/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go index 707db9a8..e1d75f20 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go +++ b/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"}) }) } diff --git a/pkg/controller/jenkins/jenkins_controller.go b/pkg/controller/jenkins/jenkins_controller.go index 528fb6d0..0c085fdd 100644 --- a/pkg/controller/jenkins/jenkins_controller.go +++ b/pkg/controller/jenkins/jenkins_controller.go @@ -125,7 +125,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul logger := r.buildLogger(request.Name) logger.V(log.VDebug).Info("Reconciling Jenkins") - result, err := r.reconcile(request, logger) + result, jenkins, err := r.reconcile(request, logger) if err != nil && apierrors.IsConflict(err) { return reconcile.Result{Requeue: true}, nil } else if err != nil { @@ -151,25 +151,12 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul logger.V(log.VWarn).Info(fmt.Sprintf("Reconcile loop failed %d times with the same error, giving up: %s", reconcileFailLimit, err)) } - jenkins := &v1alpha2.Jenkins{} - err := r.client.Get(context.TODO(), request.NamespacedName, jenkins) - if err != nil { - if apierrors.IsNotFound(err) { - // Request object not found, could have been deleted after reconcile request. - // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. - // Return and don't requeue - return reconcile.Result{}, nil - } - // Error reading the object - requeue the request. - return reconcile.Result{}, errors.WithStack(err) - } - *r.notificationEvents <- notifications.Event{ - Jenkins: *jenkins, - ConfigurationType: notifications.ConfigurationTypeUnknown, - LogLevel: v1alpha2.NotificationLogLevelWarning, - Message: fmt.Sprintf("Reconcile loop failed ten times with the same error, giving up: %s", err), - MessagesVerbose: []string{fmt.Sprintf("Reconcile loop failed ten times with the same error, giving up: %+v", err)}, + Jenkins: *jenkins, + Phase: notifications.PhaseBase, + LogLevel: v1alpha2.NotificationLogLevelWarning, + Message: fmt.Sprintf("Reconcile loop failed %d times with the same error, giving up: %s", reconcileFailLimit, err), + MessagesVerbose: []string{fmt.Sprintf("Reconcile loop failed %d times with the same error, giving up: %+v", reconcileFailLimit, err)}, } return reconcile.Result{Requeue: false}, nil } @@ -190,7 +177,7 @@ func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Resul return result, nil } -func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logger) (reconcile.Result, error) { +func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logger) (reconcile.Result, *v1alpha2.Jenkins, error) { // Fetch the Jenkins instance jenkins := &v1alpha2.Jenkins{} err := r.client.Get(context.TODO(), request.NamespacedName, jenkins) @@ -199,47 +186,47 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg // Request object not found, could have been deleted after reconcile request. // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. // Return and don't requeue - return reconcile.Result{}, nil + return reconcile.Result{}, nil, nil } // Error reading the object - requeue the request. - return reconcile.Result{}, errors.WithStack(err) + return reconcile.Result{}, nil, errors.WithStack(err) } err = r.setDefaults(jenkins, logger) if err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, nil, err } // Reconcile base configuration baseConfiguration := base.New(r.client, r.scheme, logger, jenkins, r.local, r.minikube, &r.clientSet, &r.config) messages, err := baseConfiguration.Validate(jenkins) if err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, nil, err } - if messages != nil { + if len(messages) > 0 { message := "Validation of base configuration failed, please correct Jenkins CR." *r.notificationEvents <- notifications.Event{ - Jenkins: *jenkins, - ConfigurationType: notifications.ConfigurationTypeBase, - LogLevel: v1alpha2.NotificationLogLevelWarning, - Message: message, - MessagesVerbose: messages, + Jenkins: *jenkins, + Phase: notifications.PhaseBase, + LogLevel: v1alpha2.NotificationLogLevelWarning, + Message: message, + MessagesVerbose: messages, } logger.V(log.VWarn).Info(message) for _, msg := range messages { - logger.V(log.VDebug).Info(msg) + logger.V(log.VWarn).Info(msg) } - return reconcile.Result{}, nil // don't requeue + return reconcile.Result{}, nil, nil // don't requeue } result, jenkinsClient, err := baseConfiguration.Reconcile() if err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, nil, err } if result.Requeue { - return result, nil + return result, nil, nil } if jenkinsClient == nil { - return reconcile.Result{Requeue: false}, nil + return reconcile.Result{Requeue: false}, nil, nil } if jenkins.Status.BaseConfigurationCompletedTime == nil { @@ -247,17 +234,17 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg jenkins.Status.BaseConfigurationCompletedTime = &now err = r.client.Update(context.TODO(), jenkins) if err != nil { - return reconcile.Result{}, errors.WithStack(err) + return reconcile.Result{}, nil, errors.WithStack(err) } message := fmt.Sprintf("Base configuration phase is complete, took %s", jenkins.Status.BaseConfigurationCompletedTime.Sub(jenkins.Status.ProvisionStartTime.Time)) *r.notificationEvents <- notifications.Event{ - Jenkins: *jenkins, - ConfigurationType: notifications.ConfigurationTypeBase, - LogLevel: v1alpha2.NotificationLogLevelInfo, - Message: message, - MessagesVerbose: messages, + Jenkins: *jenkins, + Phase: notifications.PhaseBase, + LogLevel: v1alpha2.NotificationLogLevelInfo, + Message: message, + MessagesVerbose: messages, } logger.Info(message) } @@ -266,31 +253,31 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg messages, err = userConfiguration.Validate(jenkins) if err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, nil, err } - if messages != nil { + if len(messages) > 0 { message := fmt.Sprintf("Validation of user configuration failed, please correct Jenkins CR") *r.notificationEvents <- notifications.Event{ - Jenkins: *jenkins, - ConfigurationType: notifications.ConfigurationTypeUser, - LogLevel: v1alpha2.NotificationLogLevelWarning, - Message: message, - MessagesVerbose: messages, + Jenkins: *jenkins, + Phase: notifications.PhaseUser, + LogLevel: v1alpha2.NotificationLogLevelWarning, + Message: message, + MessagesVerbose: messages, } logger.V(log.VWarn).Info(message) for _, msg := range messages { - logger.V(log.VDebug).Info(msg) + logger.V(log.VWarn).Info(msg) } - return reconcile.Result{}, nil // don't requeue + return reconcile.Result{}, nil, nil // don't requeue } result, err = userConfiguration.Reconcile() if err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, nil, err } if result.Requeue { - return result, nil + return result, nil, nil } if jenkins.Status.UserConfigurationCompletedTime == nil { @@ -298,21 +285,21 @@ func (r *ReconcileJenkins) reconcile(request reconcile.Request, logger logr.Logg jenkins.Status.UserConfigurationCompletedTime = &now err = r.client.Update(context.TODO(), jenkins) if err != nil { - return reconcile.Result{}, errors.WithStack(err) + return reconcile.Result{}, nil, errors.WithStack(err) } message := fmt.Sprintf("User configuration phase is complete, took %s", jenkins.Status.UserConfigurationCompletedTime.Sub(jenkins.Status.ProvisionStartTime.Time)) *r.notificationEvents <- notifications.Event{ - Jenkins: *jenkins, - ConfigurationType: notifications.ConfigurationTypeUser, - LogLevel: v1alpha2.NotificationLogLevelInfo, - Message: message, - MessagesVerbose: messages, + Jenkins: *jenkins, + Phase: notifications.PhaseUser, + LogLevel: v1alpha2.NotificationLogLevelInfo, + Message: message, + MessagesVerbose: messages, } logger.Info(message) } - return reconcile.Result{}, nil + return reconcile.Result{}, jenkins, nil } func (r *ReconcileJenkins) buildLogger(jenkinsName string) logr.Logger { diff --git a/pkg/controller/jenkins/notifications/mailgun.go b/pkg/controller/jenkins/notifications/mailgun.go index 8fd3dab3..45a166a2 100644 --- a/pkg/controller/jenkins/notifications/mailgun.go +++ b/pkg/controller/jenkins/notifications/mailgun.go @@ -28,7 +28,7 @@ const content = ` %s - Configuration type: + Phase: %s @@ -83,7 +83,7 @@ func (m MailGun) Send(event Event, config v1alpha2.Notification) error { statusMessage = event.Message } - htmlMessage := fmt.Sprintf(content, m.getStatusColor(event.LogLevel), notificationTitle(event), statusMessage, event.Jenkins.Name, event.ConfigurationType) + htmlMessage := fmt.Sprintf(content, m.getStatusColor(event.LogLevel), notificationTitle(event), statusMessage, event.Jenkins.Name, event.Phase) msg := mg.NewMessage(fmt.Sprintf("Jenkins Operator Notifier <%s>", config.Mailgun.From), notificationTitle(event), "", config.Mailgun.Recipient) msg.SetHtml(htmlMessage) diff --git a/pkg/controller/jenkins/notifications/msteams.go b/pkg/controller/jenkins/notifications/msteams.go index daf6817d..c883fbfb 100644 --- a/pkg/controller/jenkins/notifications/msteams.go +++ b/pkg/controller/jenkins/notifications/msteams.go @@ -102,10 +102,10 @@ func (t Teams) Send(event Event, config v1alpha2.Notification) error { tm.Summary = message } - if event.ConfigurationType != ConfigurationTypeUnknown { + if event.Phase != PhaseUnknown { tm.Sections[0].Facts = append(tm.Sections[0].Facts, TeamsFact{ - Name: configurationTypeFieldName, - Value: string(event.ConfigurationType), + Name: phaseFieldName, + Value: string(event.Phase), }) } diff --git a/pkg/controller/jenkins/notifications/msteams_test.go b/pkg/controller/jenkins/notifications/msteams_test.go index b223a4cd..088f6928 100644 --- a/pkg/controller/jenkins/notifications/msteams_test.go +++ b/pkg/controller/jenkins/notifications/msteams_test.go @@ -27,10 +27,10 @@ func TestTeams_Send(t *testing.T) { Namespace: testNamespace, }, }, - ConfigurationType: testConfigurationType, - Message: testMessage, - MessagesVerbose: testMessageVerbose, - LogLevel: testLoggingLevel, + Phase: testPhase, + Message: testMessage, + MessagesVerbose: testMessageVerbose, + LogLevel: testLoggingLevel, } teams := Teams{k8sClient: fakeClient} @@ -51,8 +51,8 @@ func TestTeams_Send(t *testing.T) { for _, fact := range mainSection.Facts { switch fact.Name { - case configurationTypeFieldName: - assert.Equal(t, fact.Value, string(event.ConfigurationType)) + case phaseFieldName: + assert.Equal(t, fact.Value, string(event.Phase)) case crNameFieldName: assert.Equal(t, fact.Value, event.Jenkins.Name) case messageFieldName: diff --git a/pkg/controller/jenkins/notifications/sender.go b/pkg/controller/jenkins/notifications/sender.go index 0f426530..441d9030 100644 --- a/pkg/controller/jenkins/notifications/sender.go +++ b/pkg/controller/jenkins/notifications/sender.go @@ -2,50 +2,50 @@ package notifications import ( "fmt" + "net/http" + "github.com/jenkinsci/kubernetes-operator/pkg/apis/jenkins/v1alpha2" "github.com/jenkinsci/kubernetes-operator/pkg/event" "github.com/jenkinsci/kubernetes-operator/pkg/log" - "net/http" - "github.com/pkg/errors" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" ) const ( - infoTitleText = "Jenkins Operator reconciliation info" - warnTitleText = "Jenkins Operator reconciliation warning" - messageFieldName = "Message" - loggingLevelFieldName = "Logging Level" - crNameFieldName = "CR Name" - configurationTypeFieldName = "Phase" - namespaceFieldName = "Namespace" - footerContent = "Powered by Jenkins Operator" + infoTitleText = "Jenkins Operator reconciliation info" + warnTitleText = "Jenkins Operator reconciliation warning" + messageFieldName = "Message" + loggingLevelFieldName = "Logging Level" + crNameFieldName = "CR Name" + phaseFieldName = "Phase" + namespaceFieldName = "Namespace" + footerContent = "Powered by Jenkins Operator" ) const ( - // ConfigurationTypeBase is core configuration of Jenkins provided by the Operator - ConfigurationTypeBase ConfigurationType = "base" + // PhaseBase is core configuration of Jenkins provided by the Operator + PhaseBase Phase = "base" - // ConfigurationTypeUser is user-defined configuration of Jenkins - ConfigurationTypeUser ConfigurationType = "user" + // PhaseUser is user-defined configuration of Jenkins + PhaseUser Phase = "user" - // ConfigurationTypeUnknown is untraceable type of configuration - ConfigurationTypeUnknown ConfigurationType = "unknown" + // PhaseUnknown is untraceable type of configuration + PhaseUnknown Phase = "unknown" ) var ( - testConfigurationType = ConfigurationTypeUser - testCrName = "test-cr" - testNamespace = "default" - testMessage = "test-message" - testMessageVerbose = []string{"detail-test-message"} - testLoggingLevel = v1alpha2.NotificationLogLevelWarning + testPhase = PhaseUser + testCrName = "test-cr" + testNamespace = "default" + testMessage = "test-message" + testMessageVerbose = []string{"detail-test-message"} + testLoggingLevel = v1alpha2.NotificationLogLevelWarning client = http.Client{} ) -// ConfigurationType defines the type of configuration -type ConfigurationType string +// Phase defines the type of configuration +type Phase string // StatusColor is useful for better UX type StatusColor string @@ -55,11 +55,11 @@ type LoggingLevel string // Event contains event details which will be sent as a notification type Event struct { - Jenkins v1alpha2.Jenkins - ConfigurationType ConfigurationType - LogLevel v1alpha2.NotificationLogLevel - Message string - MessagesVerbose []string + Jenkins v1alpha2.Jenkins + Phase Phase + LogLevel v1alpha2.NotificationLogLevel + Message string + MessagesVerbose []string } type service interface { diff --git a/pkg/controller/jenkins/notifications/slack.go b/pkg/controller/jenkins/notifications/slack.go index 8db3975e..5ee08a98 100644 --- a/pkg/controller/jenkins/notifications/slack.go +++ b/pkg/controller/jenkins/notifications/slack.go @@ -104,10 +104,10 @@ func (s Slack) Send(event Event, config v1alpha2.Notification) error { mainAttachment.Fields[0].Value = message } - if event.ConfigurationType != ConfigurationTypeUnknown { + if event.Phase != PhaseUnknown { mainAttachment.Fields = append(mainAttachment.Fields, SlackField{ - Title: configurationTypeFieldName, - Value: string(event.ConfigurationType), + Title: phaseFieldName, + Value: string(event.Phase), Short: true, }) } diff --git a/pkg/controller/jenkins/notifications/slack_test.go b/pkg/controller/jenkins/notifications/slack_test.go index 935eaed2..261a6172 100644 --- a/pkg/controller/jenkins/notifications/slack_test.go +++ b/pkg/controller/jenkins/notifications/slack_test.go @@ -27,10 +27,10 @@ func TestSlack_Send(t *testing.T) { Namespace: testNamespace, }, }, - ConfigurationType: testConfigurationType, - Message: testMessage, - MessagesVerbose: testMessageVerbose, - LogLevel: testLoggingLevel, + Phase: testPhase, + Message: testMessage, + MessagesVerbose: testMessageVerbose, + LogLevel: testLoggingLevel, } slack := Slack{k8sClient: fakeClient} @@ -48,8 +48,8 @@ func TestSlack_Send(t *testing.T) { assert.Equal(t, mainAttachment.Title, notificationTitle(event)) for _, field := range mainAttachment.Fields { switch field.Title { - case configurationTypeFieldName: - assert.Equal(t, field.Value, string(event.ConfigurationType)) + case phaseFieldName: + assert.Equal(t, field.Value, string(event.Phase)) case crNameFieldName: assert.Equal(t, field.Value, event.Jenkins.Name) case messageFieldName: diff --git a/pkg/controller/jenkins/plugins/plugin.go b/pkg/controller/jenkins/plugins/plugin.go index ef96fa8a..d4ee39e0 100644 --- a/pkg/controller/jenkins/plugins/plugin.go +++ b/pkg/controller/jenkins/plugins/plugin.go @@ -5,8 +5,6 @@ import ( "regexp" "strings" - "github.com/jenkinsci/kubernetes-operator/pkg/log" - "github.com/pkg/errors" ) @@ -77,10 +75,10 @@ func Must(plugin *Plugin, err error) Plugin { } // VerifyDependencies checks if all plugins have compatible versions -func VerifyDependencies(values ...map[Plugin][]Plugin) bool { +func VerifyDependencies(values ...map[Plugin][]Plugin) []string { + var messages []string // key - plugin name, value array of versions allPlugins := make(map[string][]Plugin) - valid := true for _, value := range values { for rootPlugin, plugins := range value { @@ -105,18 +103,17 @@ func VerifyDependencies(values ...map[Plugin][]Plugin) bool { for _, firstVersion := range versions { for _, secondVersion := range versions { if firstVersion.Version != secondVersion.Version { - log.Log.V(log.VWarn).Info(fmt.Sprintf("Plugin '%s' requires version '%s' but plugin '%s' requires '%s' for plugin '%s'", + messages = append(messages, fmt.Sprintf("Plugin '%s' requires version '%s' but plugin '%s' requires '%s' for plugin '%s'", firstVersion.rootPluginNameAndVersion, firstVersion.Version, secondVersion.rootPluginNameAndVersion, secondVersion.Version, pluginName, )) - valid = false } } } } - return valid + return messages } diff --git a/pkg/controller/jenkins/plugins/plugin_test.go b/pkg/controller/jenkins/plugins/plugin_test.go index 2e050a0e..a4002ddc 100644 --- a/pkg/controller/jenkins/plugins/plugin_test.go +++ b/pkg/controller/jenkins/plugins/plugin_test.go @@ -18,7 +18,7 @@ func TestVerifyDependencies(t *testing.T) { }, } got := VerifyDependencies(basePlugins) - assert.Equal(t, true, got) + assert.Nil(t, got) }) t.Run("happy, two root plugins with one depended plugin with the same version", func(t *testing.T) { basePlugins := map[Plugin][]Plugin{ @@ -30,7 +30,7 @@ func TestVerifyDependencies(t *testing.T) { }, } got := VerifyDependencies(basePlugins) - assert.Equal(t, true, got) + assert.Nil(t, got) }) t.Run("happy, two plugin names with names with underscores", func(t *testing.T) { basePlugins := map[Plugin][]Plugin{ @@ -42,7 +42,7 @@ func TestVerifyDependencies(t *testing.T) { }, } got := VerifyDependencies(basePlugins) - assert.Equal(t, true, got) + assert.Nil(t, got) }) t.Run("happy, two plugin names with uppercase names", func(t *testing.T) { basePlugins := map[Plugin][]Plugin{ @@ -54,7 +54,7 @@ func TestVerifyDependencies(t *testing.T) { }, } got := VerifyDependencies(basePlugins) - assert.Equal(t, true, got) + assert.Nil(t, got) }) t.Run("fail, two root plugins have different versions", func(t *testing.T) { basePlugins := map[Plugin][]Plugin{ @@ -66,7 +66,7 @@ func TestVerifyDependencies(t *testing.T) { }, } got := VerifyDependencies(basePlugins) - assert.Equal(t, false, got) + assert.NotNil(t, got) }) t.Run("happy, no version collision with two sperate plugins lists", func(t *testing.T) { basePlugins := map[Plugin][]Plugin{ @@ -80,7 +80,7 @@ func TestVerifyDependencies(t *testing.T) { }, } got := VerifyDependencies(basePlugins, extraPlugins) - assert.Equal(t, true, got) + assert.Nil(t, got) }) t.Run("fail, dependent plugins have different versions", func(t *testing.T) { basePlugins := map[Plugin][]Plugin{ @@ -92,7 +92,7 @@ func TestVerifyDependencies(t *testing.T) { }, } got := VerifyDependencies(basePlugins) - assert.Equal(t, false, got) + assert.NotNil(t, got) }) t.Run("fail, root and dependent plugins have different versions", func(t *testing.T) { basePlugins := map[Plugin][]Plugin{ @@ -106,6 +106,6 @@ func TestVerifyDependencies(t *testing.T) { }, } got := VerifyDependencies(basePlugins, extraPlugins) - assert.Equal(t, false, got) + assert.NotNil(t, got) }) }