From 1a8fd8828bcdd112016a924ce3e18c9d6a1961bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20S=C4=99k?= Date: Sat, 16 Mar 2019 20:19:22 +0100 Subject: [PATCH] #14 Add username/password authentication for seed jobs - validate if seed job ids are unique --- .../configuration/user/seedjobs/validate.go | 121 ++++++++++-------- .../user/seedjobs/validate_test.go | 47 +++++-- .../jenkins/configuration/user/validate.go | 2 +- 3 files changed, 101 insertions(+), 69 deletions(-) diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/validate.go b/pkg/controller/jenkins/configuration/user/seedjobs/validate.go index 1626b4d8..730782f8 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/validate.go +++ b/pkg/controller/jenkins/configuration/user/seedjobs/validate.go @@ -18,71 +18,72 @@ import ( ) // ValidateSeedJobs verify seed jobs configuration -func (r *SeedJobs) ValidateSeedJobs(jenkins *v1alpha1.Jenkins) (bool, error) { +func (r *SeedJobs) ValidateSeedJobs(jenkins v1alpha1.Jenkins) (bool, error) { valid := true - // TODO id must be unique - if jenkins.Spec.SeedJobs != nil { - for _, seedJob := range jenkins.Spec.SeedJobs { - logger := r.logger.WithValues("seedJob", fmt.Sprintf("%+v", seedJob)).V(log.VWarn) + if !r.validateIfIDIsUnique(jenkins.Spec.SeedJobs) { + valid = false + } - if len(seedJob.ID) == 0 { - logger.Info("id can't be empty") - valid = false - } + for _, seedJob := range jenkins.Spec.SeedJobs { + logger := r.logger.WithValues("seedJob", fmt.Sprintf("%+v", seedJob)).V(log.VWarn) - if len(seedJob.RepositoryBranch) == 0 { - logger.Info("repository branch can't be empty") - valid = false - } + if len(seedJob.ID) == 0 { + logger.Info("id can't be empty") + valid = false + } - if len(seedJob.RepositoryURL) == 0 { - logger.Info("repository URL branch can't be empty") - valid = false - } + if len(seedJob.RepositoryBranch) == 0 { + logger.Info("repository branch can't be empty") + valid = false + } - if len(seedJob.Targets) == 0 { - logger.Info("targets can't be empty") - valid = false - } + if len(seedJob.RepositoryURL) == 0 { + logger.Info("repository URL branch can't be empty") + valid = false + } - if _, ok := v1alpha1.AllowedJenkinsCredentialMap[string(seedJob.JenkinsCredentialType)]; !ok { - logger.Info("unknown credential type") + if len(seedJob.Targets) == 0 { + logger.Info("targets can't be empty") + valid = false + } + + if _, ok := v1alpha1.AllowedJenkinsCredentialMap[string(seedJob.JenkinsCredentialType)]; !ok { + logger.Info("unknown credential type") + return false, nil + } + + if (seedJob.JenkinsCredentialType == v1alpha1.BasicSSHCredentialType || + seedJob.JenkinsCredentialType == v1alpha1.UsernamePasswordCredentialType) && len(seedJob.CredentialID) == 0 { + logger.Info("credential ID can't be empty") + valid = false + } + + // validate repository url match private key + if strings.Contains(seedJob.RepositoryURL, "git@") && seedJob.JenkinsCredentialType == v1alpha1.NoJenkinsCredentialCredentialType { + logger.Info("Jenkins credential must be set while using ssh repository url") + valid = false + } + + if seedJob.JenkinsCredentialType == v1alpha1.BasicSSHCredentialType || seedJob.JenkinsCredentialType == v1alpha1.UsernamePasswordCredentialType { + secret := &v1.Secret{} + namespaceName := types.NamespacedName{Namespace: jenkins.Namespace, Name: seedJob.CredentialID} + err := r.k8sClient.Get(context.TODO(), namespaceName, secret) + if err != nil && apierrors.IsNotFound(err) { + logger.Info(fmt.Sprintf("required secret '%s' with Jenkins credential not found", seedJob.CredentialID)) return false, nil + } else if err != nil { + return false, stackerr.WithStack(err) } - if (seedJob.JenkinsCredentialType == v1alpha1.BasicSSHCredentialType || - seedJob.JenkinsCredentialType == v1alpha1.UsernamePasswordCredentialType) && len(seedJob.CredentialID) == 0 { - logger.Info("credential ID can't be empty") - valid = false - } - - // validate repository url match private key - if strings.Contains(seedJob.RepositoryURL, "git@") && seedJob.JenkinsCredentialType == v1alpha1.NoJenkinsCredentialCredentialType { - logger.Info("Jenkins credential must be set while using ssh repository url") - valid = false - } - - if seedJob.JenkinsCredentialType == v1alpha1.BasicSSHCredentialType || seedJob.JenkinsCredentialType == v1alpha1.UsernamePasswordCredentialType { - secret := &v1.Secret{} - namespaceName := types.NamespacedName{Namespace: jenkins.Namespace, Name: seedJob.CredentialID} - err := r.k8sClient.Get(context.TODO(), namespaceName, secret) - if err != nil && apierrors.IsNotFound(err) { - logger.Info(fmt.Sprintf("required secret '%s' with Jenkins credential not found", seedJob.CredentialID)) - return false, nil - } else if err != nil { - return false, stackerr.WithStack(err) + if seedJob.JenkinsCredentialType == v1alpha1.BasicSSHCredentialType { + if ok := validateBasicSSHSecret(logger, *secret); !ok { + valid = false } - - if seedJob.JenkinsCredentialType == v1alpha1.BasicSSHCredentialType { - if ok := validateBasicSSHSecret(logger, *secret); !ok { - valid = false - } - } - if seedJob.JenkinsCredentialType == v1alpha1.UsernamePasswordCredentialType { - if ok := validateUsernamePasswordSecret(logger, *secret); !ok { - valid = false - } + } + if seedJob.JenkinsCredentialType == v1alpha1.UsernamePasswordCredentialType { + if ok := validateUsernamePasswordSecret(logger, *secret); !ok { + valid = false } } } @@ -90,6 +91,18 @@ func (r *SeedJobs) ValidateSeedJobs(jenkins *v1alpha1.Jenkins) (bool, error) { return valid, nil } +func (r *SeedJobs) validateIfIDIsUnique(seedJobs []v1alpha1.SeedJob) bool { + ids := map[string]bool{} + for _, seedJob := range seedJobs { + if _, found := ids[seedJob.ID]; found { + r.logger.V(log.VWarn).Info(fmt.Sprintf("'%s' seed job ID is not unique", seedJob.ID)) + return false + } + ids[seedJob.ID] = true + } + return true +} + func validateBasicSSHSecret(logger logr.InfoLogger, secret v1.Secret) bool { valid := true username, exists := secret.Data[UsernameSecretKey] diff --git a/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go b/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go index f1ee651f..f584f8c2 100644 --- a/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go +++ b/pkg/controller/jenkins/configuration/user/seedjobs/validate_test.go @@ -57,7 +57,7 @@ func TestValidateSeedJobs(t *testing.T) { Namespace: "default", } t.Run("Valid with public repository and without private key", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -79,7 +79,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, true, result) }) t.Run("Invalid without id", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -99,7 +99,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Valid with private key and secret", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -132,7 +132,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, true, result) }) t.Run("Invalid private key in secret", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -165,7 +165,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Invalid with PrivateKey and empty Secret data", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -198,7 +198,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Invalid with ssh RepositoryURL and empty PrivateKey", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -220,7 +220,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Invalid without targets", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -240,7 +240,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Invalid without repository URL", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -260,7 +260,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Invalid without repository branch", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -280,7 +280,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Valid with username and password", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -313,7 +313,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, true, result) }) t.Run("Invalid with empty username", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -346,7 +346,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Invalid with empty password", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -379,7 +379,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Invalid without username", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -411,7 +411,7 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) t.Run("Invalid without password", func(t *testing.T) { - jenkins := &v1alpha1.Jenkins{ + jenkins := v1alpha1.Jenkins{ Spec: v1alpha1.JenkinsSpec{ SeedJobs: []v1alpha1.SeedJob{ { @@ -443,3 +443,22 @@ func TestValidateSeedJobs(t *testing.T) { assert.Equal(t, false, result) }) } + +func TestValidateIfIDIsUnique(t *testing.T) { + t.Run("happy", func(t *testing.T) { + seedJobs := []v1alpha1.SeedJob{ + {ID: "first"}, {ID: "second"}, + } + ctrl := New(nil, nil, logf.ZapLogger(false)) + got := ctrl.validateIfIDIsUnique(seedJobs) + assert.Equal(t, true, got) + }) + t.Run("duplicated ids", func(t *testing.T) { + seedJobs := []v1alpha1.SeedJob{ + {ID: "first"}, {ID: "first"}, + } + ctrl := New(nil, nil, logf.ZapLogger(false)) + got := ctrl.validateIfIDIsUnique(seedJobs) + assert.Equal(t, false, got) + }) +} diff --git a/pkg/controller/jenkins/configuration/user/validate.go b/pkg/controller/jenkins/configuration/user/validate.go index bd8f6a69..5cf122ef 100644 --- a/pkg/controller/jenkins/configuration/user/validate.go +++ b/pkg/controller/jenkins/configuration/user/validate.go @@ -8,5 +8,5 @@ import ( // Validate validates Jenkins CR Spec section func (r *ReconcileUserConfiguration) Validate(jenkins *v1alpha1.Jenkins) (bool, error) { seedJobs := seedjobs.New(r.jenkinsClient, r.k8sClient, r.logger) - return seedJobs.ValidateSeedJobs(jenkins) + return seedJobs.ValidateSeedJobs(*jenkins) }