#14 Add username/password authentication for seed jobs

- validate if seed job ids are unique

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 !r.validateIfIDIsUnique(jenkins.Spec.SeedJobs) {
-	if jenkins.Spec.SeedJobs != nil {
+		valid = false
-		for _, seedJob := range jenkins.Spec.SeedJobs {
+	}
-			logger := r.logger.WithValues("seedJob", fmt.Sprintf("%+v", seedJob)).V(log.VWarn)
 
-			if len(seedJob.ID) == 0 {
+	for _, seedJob := range jenkins.Spec.SeedJobs {
-				logger.Info("id can't be empty")
+		logger := r.logger.WithValues("seedJob", fmt.Sprintf("%+v", seedJob)).V(log.VWarn)
-				valid = false
-			}
 
-			if len(seedJob.RepositoryBranch) == 0 {
+		if len(seedJob.ID) == 0 {
-				logger.Info("repository branch can't be empty")
+			logger.Info("id can't be empty")
-				valid = false
+			valid = false
-			}
+		}
 
-			if len(seedJob.RepositoryURL) == 0 {
+		if len(seedJob.RepositoryBranch) == 0 {
-				logger.Info("repository URL branch can't be empty")
+			logger.Info("repository branch can't be empty")
-				valid = false
+			valid = false
-			}
+		}
 
-			if len(seedJob.Targets) == 0 {
+		if len(seedJob.RepositoryURL) == 0 {
-				logger.Info("targets can't be empty")
+			logger.Info("repository URL branch can't be empty")
-				valid = false
+			valid = false
-			}
+		}
 
-			if _, ok := v1alpha1.AllowedJenkinsCredentialMap[string(seedJob.JenkinsCredentialType)]; !ok {
+		if len(seedJob.Targets) == 0 {
-				logger.Info("unknown credential type")
+			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 ||
+			if seedJob.JenkinsCredentialType == v1alpha1.BasicSSHCredentialType {
-				seedJob.JenkinsCredentialType == v1alpha1.UsernamePasswordCredentialType) && len(seedJob.CredentialID) == 0 {
+				if ok := validateBasicSSHSecret(logger, *secret); !ok {
-				logger.Info("credential ID can't be empty")
+					valid = false
-				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 seedJob.JenkinsCredentialType == v1alpha1.UsernamePasswordCredentialType {
-					if ok := validateBasicSSHSecret(logger, *secret); !ok {
+				if ok := validateUsernamePasswordSecret(logger, *secret); !ok {
-						valid = false
+					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]

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)
+	})
+}

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)
 }