Code refactoring

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

@@ -38,23 +38,12 @@ func New(k8sClient k8s.Client, jenkinsClient jenkinsclient.Jenkins, logger logr.
 func (r *ReconcileUserConfiguration) Reconcile() (*reconcile.Result, error) {
 	// reconcile seed jobs
 	result, err := r.reconcileSeedJobs()
-	if err != nil {
+	if err != nil || result != nil {
 		return result, err
 	}
-	if result != nil {
-		return result, nil
-	}
 
 	// reconcile custom groovy scripts
-	result, err = r.reconcileCustomGroovy()
-	if err != nil {
-		return result, err
-	}
-	if result != nil {
-		return result, nil
-	}
-
-	return nil, nil
+	return r.reconcileCustomGroovy()
 }
 
 func (r *ReconcileUserConfiguration) reconcileSeedJobs() (*reconcile.Result, error) {

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

@@ -122,6 +122,7 @@ func (s *SeedJobs) privateKeyFromSecret(namespace string, seedJob virtuslabv1alp
 	return "", nil
 }
 
+// FIXME(antoniaklja) use mask-password plugin for params.PRIVATE_KEY
 // seedJobConfigXML this is the XML representation of seed job
 var seedJobConfigXML = `
 <flow-definition plugin="workflow-job@2.30">

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

@@ -3,12 +3,10 @@ package seedjobs
 import (
 	"context"
 	"fmt"
-	"os"
 	"testing"
 
 	virtuslabv1alpha1 "github.com/VirtusLab/jenkins-operator/pkg/apis/virtuslab/v1alpha1"
 	"github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/client"
-	"github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/jobs"
 
 	"github.com/bndr/gojenkins"
 	"github.com/golang/mock/gomock"
@@ -22,11 +20,6 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 )
 
-func TestMain(m *testing.M) {
-	virtuslabv1alpha1.SchemeBuilder.AddToScheme(scheme.Scheme)
-	os.Exit(m.Run())
-}
-
 func TestEnsureSeedJobs(t *testing.T) {
 	// given
 	logger := logf.ZapLogger(false)
@@ -36,10 +29,13 @@ func TestEnsureSeedJobs(t *testing.T) {
 
 	jenkinsClient := client.NewMockJenkins(ctrl)
 	fakeClient := fake.NewFakeClient()
+	err := virtuslabv1alpha1.SchemeBuilder.AddToScheme(scheme.Scheme)
+	assert.NoError(t, err)
 
 	jenkins := jenkinsCustomResource()
-	err := fakeClient.Create(ctx, jenkins)
+	err = fakeClient.Create(ctx, jenkins)
 	assert.NoError(t, err)
+	buildNumber := int64(1)
 
 	for reconcileAttempt := 1; reconcileAttempt <= 2; reconcileAttempt++ {
 		logger.Info(fmt.Sprintf("Reconcile attempt #%d", reconcileAttempt))
@@ -58,7 +54,7 @@ func TestEnsureSeedJobs(t *testing.T) {
 				GetJob(ConfigureSeedJobsName).
 				Return(&gojenkins.Job{
 					Raw: &gojenkins.JobResponse{
-						NextBuildNumber: int64(1),
+						NextBuildNumber: buildNumber,
 					},
 				}, nil)
 
@@ -80,7 +76,7 @@ func TestEnsureSeedJobs(t *testing.T) {
 				GetBuild(ConfigureSeedJobsName, gomock.Any()).
 				Return(&gojenkins.Build{
 					Raw: &gojenkins.BuildResponse{
-						Result: jobs.SuccessStatus,
+						Result: string(virtuslabv1alpha1.BuildSuccessStatus),
 					},
 				}, nil)
 		}
@@ -91,15 +87,27 @@ func TestEnsureSeedJobs(t *testing.T) {
 		err = fakeClient.Get(ctx, types.NamespacedName{Name: jenkins.Name, Namespace: jenkins.Namespace}, jenkins)
 		assert.NoError(t, err)
 
+		assert.Equal(t, 1, len(jenkins.Status.Builds), "There is one running job")
+		build := jenkins.Status.Builds[0]
+		assert.Equal(t, buildNumber, build.Number)
+		assert.Equal(t, ConfigureSeedJobsName, build.JobName)
+		assert.NotNil(t, build.CreateTime)
+		assert.NotEmpty(t, build.Hash)
+		assert.NotNil(t, build.LastUpdateTime)
+		assert.Equal(t, 0, build.Retires)
+
 		// first run - should create job and schedule build
 		if reconcileAttempt == 1 {
 			assert.False(t, done)
+			assert.Equal(t, string(virtuslabv1alpha1.BuildRunningStatus), string(build.Status))
 		}
 
 		// second run - should update and finish job
 		if reconcileAttempt == 2 {
 			assert.True(t, done)
+			assert.Equal(t, string(virtuslabv1alpha1.BuildSuccessStatus), string(build.Status))
 		}
+
 	}
 }
 

pkg/controller/jenkins/configuration/user/theme/theme.go

@@ -1,6 +1,7 @@
 package theme
 
 // SetThemeGroovyScript it's a groovy script which set custom jenkins theme
+// TODO move to base configuration
 var SetThemeGroovyScript = `
 import jenkins.*
 import jenkins.model.*

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

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -11,18 +12,17 @@ import (
 	"github.com/VirtusLab/jenkins-operator/pkg/log"
 
 	"k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 )
 
 // Validate validates Jenkins CR Spec section
-func (r *ReconcileUserConfiguration) Validate(jenkins *virtuslabv1alpha1.Jenkins) bool {
-	if !r.validateSeedJobs(jenkins) {
-		return false
-	}
-	return true
+func (r *ReconcileUserConfiguration) Validate(jenkins *virtuslabv1alpha1.Jenkins) (bool, error) {
+	return r.validateSeedJobs(jenkins)
 }
 
-func (r *ReconcileUserConfiguration) validateSeedJobs(jenkins *virtuslabv1alpha1.Jenkins) bool {
+func (r *ReconcileUserConfiguration) validateSeedJobs(jenkins *virtuslabv1alpha1.Jenkins) (bool, error) {
+	valid := true
 	if jenkins.Spec.SeedJobs != nil {
 		for _, seedJob := range jenkins.Spec.SeedJobs {
 			logger := r.logger.WithValues("seedJob", fmt.Sprintf("%+v", seedJob)).V(log.VWarn)
@@ -30,14 +30,14 @@ func (r *ReconcileUserConfiguration) validateSeedJobs(jenkins *virtuslabv1alpha1
 			// validate seed job id is not empty
 			if len(seedJob.ID) == 0 {
 				logger.Info("seed job id can't be empty")
-				return false
+				valid = false
 			}
 
 			// validate repository url match private key
 			if strings.Contains(seedJob.RepositoryURL, "git@") {
 				if seedJob.PrivateKey.SecretKeyRef == nil {
 					logger.Info("private key can't be empty while using ssh repository url")
-					return false
+					valid = false
 				}
 			}
 
@@ -46,43 +46,44 @@ func (r *ReconcileUserConfiguration) validateSeedJobs(jenkins *virtuslabv1alpha1
 				deployKeySecret := &v1.Secret{}
 				namespaceName := types.NamespacedName{Namespace: jenkins.Namespace, Name: seedJob.PrivateKey.SecretKeyRef.Name}
 				err := r.k8sClient.Get(context.TODO(), namespaceName, deployKeySecret)
-				//TODO(bantoniak) handle error properly
-				if err != nil {
-					logger.Info("couldn't read private key secret")
-					return false
+				if err != nil && apierrors.IsNotFound(err) {
+					logger.Info("secret not found")
+					valid = false
+				} else if err != nil {
+					return false, err
 				}
 
 				privateKey := string(deployKeySecret.Data[seedJob.PrivateKey.SecretKeyRef.Key])
 				if privateKey == "" {
 					logger.Info("private key is empty")
-					return false
+					valid = false
 				}
 
-				if !validatePrivateKey(privateKey) {
-					logger.Info("private key is invalid")
-					return false
+				if err := validatePrivateKey(privateKey); err != nil {
+					logger.Info(fmt.Sprintf("private key is invalid: %s", err))
+					valid = false
 				}
 			}
 		}
 	}
-	return true
+	return valid, nil
 }
 
-func validatePrivateKey(privateKey string) bool {
+func validatePrivateKey(privateKey string) error {
 	block, _ := pem.Decode([]byte(privateKey))
 	if block == nil {
-		return false
+		return errors.New("failed to decode PEM block")
 	}
 
 	priv, err := x509.ParsePKCS1PrivateKey(block.Bytes)
 	if err != nil {
-		return false
+		return err
 	}
 
 	err = priv.Validate()
 	if err != nil {
-		return false
+		return err
 	}
 
-	return true
+	return nil
 }

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

@@ -190,9 +190,13 @@ func TestValidateUserConfiguration(t *testing.T) {
 	for _, testingData := range data {
 		t.Run(fmt.Sprintf("Testing '%s'", testingData.description), func(t *testing.T) {
 			fakeClient := fake.NewFakeClient()
-			fakeClient.Create(context.TODO(), testingData.secret)
+			if testingData.secret != nil {
+				err := fakeClient.Create(context.TODO(), testingData.secret)
+				assert.NoError(t, err)
+			}
 			userReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), nil)
-			result := userReconcileLoop.Validate(testingData.jenkins)
+			result, err := userReconcileLoop.Validate(testingData.jenkins)
+			assert.NoError(t, err)
 			assert.Equal(t, testingData.expectedResult, result)
 		})
 	}