From e049218b78a6e64d3d0e5f4458ac51fd85928213 Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Fri, 12 Jul 2019 13:11:42 +0200 Subject: [PATCH 1/6] #45 Add support for imagePullSecrets parameter --- pkg/apis/jenkins/v1alpha2/jenkins_types.go | 7 + .../jenkins/v1alpha2/zz_generated.deepcopy.go | 5 + .../jenkins/configuration/base/reconcile.go | 6 + .../configuration/base/reconcile_test.go | 23 +++ .../configuration/base/resources/pod.go | 1 + .../jenkins/configuration/base/validate.go | 41 ++++ .../configuration/base/validate_test.go | 193 ++++++++++++++++++ 7 files changed, 276 insertions(+) diff --git a/pkg/apis/jenkins/v1alpha2/jenkins_types.go b/pkg/apis/jenkins/v1alpha2/jenkins_types.go index abc35481..a961b7a9 100644 --- a/pkg/apis/jenkins/v1alpha2/jenkins_types.go +++ b/pkg/apis/jenkins/v1alpha2/jenkins_types.go @@ -210,6 +210,13 @@ type JenkinsMaster struct { // memory: 600Mi Containers []Container `json:"containers,omitempty"` + // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. + // If specified, these secrets will be passed to individual puller implementations for them to use. For example, + // in the case of docker, only DockerConfig type secrets are honored. + // More info: https://kubernetes.io/docs/concepts/containers/images#specifying-imagepullsecrets-on-a-pod + // +optional + ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"` + // List of volumes that can be mounted by containers belonging to the pod. // More info: https://kubernetes.io/docs/concepts/storage/volumes // +optional diff --git a/pkg/apis/jenkins/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/jenkins/v1alpha2/zz_generated.deepcopy.go index ab88bd8c..33b5ff35 100644 --- a/pkg/apis/jenkins/v1alpha2/zz_generated.deepcopy.go +++ b/pkg/apis/jenkins/v1alpha2/zz_generated.deepcopy.go @@ -322,6 +322,11 @@ func (in *JenkinsMaster) DeepCopyInto(out *JenkinsMaster) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.ImagePullSecrets != nil { + in, out := &in.ImagePullSecrets, &out.ImagePullSecrets + *out = make([]v1.LocalObjectReference, len(*in)) + copy(*out, *in) + } if in.Volumes != nil { in, out := &in.Volumes, &out.Volumes *out = make([]v1.Volume, len(*in)) diff --git a/pkg/controller/jenkins/configuration/base/reconcile.go b/pkg/controller/jenkins/configuration/base/reconcile.go index 7cb591a4..ed8ac2f1 100644 --- a/pkg/controller/jenkins/configuration/base/reconcile.go +++ b/pkg/controller/jenkins/configuration/base/reconcile.go @@ -527,6 +527,12 @@ func (r *ReconcileJenkinsBaseConfiguration) isRecreatePodNeeded(currentJenkinsMa return true } + if !reflect.DeepEqual(r.jenkins.Spec.Master.ImagePullSecrets, currentJenkinsMasterPod.Spec.ImagePullSecrets) { + r.logger.Info(fmt.Sprintf("Jenkins Pod ImagePullSecrets has changed, actual '%+v' required '%+v', recreating pod", + currentJenkinsMasterPod.Spec.ImagePullSecrets, r.jenkins.Spec.Master.ImagePullSecrets)) + return true + } + if !reflect.DeepEqual(r.jenkins.Spec.Master.NodeSelector, currentJenkinsMasterPod.Spec.NodeSelector) { r.logger.Info(fmt.Sprintf("Jenkins pod node selector has changed, actual '%+v' required '%+v', recreating pod", currentJenkinsMasterPod.Spec.NodeSelector, r.jenkins.Spec.Master.NodeSelector)) diff --git a/pkg/controller/jenkins/configuration/base/reconcile_test.go b/pkg/controller/jenkins/configuration/base/reconcile_test.go index 82386155..a81aaf06 100644 --- a/pkg/controller/jenkins/configuration/base/reconcile_test.go +++ b/pkg/controller/jenkins/configuration/base/reconcile_test.go @@ -124,6 +124,29 @@ func TestGetJenkinsOpts(t *testing.T) { assert.Contains(t, opts, "httpPort") assert.Equal(t, opts["httpPort"], "8080") }) + + t.Run("JENKINS_OPTS have --httpPort=--8080 argument", func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + Containers: []v1alpha2.Container{ + { + Env: []corev1.EnvVar{ + {Name: "JENKINS_OPTS", Value: "--httpPort=--8080"}, + }, + }, + }, + }, + }, + } + + opts := GetJenkinsOpts(jenkins) + + assert.Equal(t, 1, len(opts)) + assert.NotContains(t, opts, "prefix") + assert.Contains(t, opts, "httpPort") + assert.Equal(t, opts["httpPort"], "--8080") + }) } func TestCompareContainerVolumeMounts(t *testing.T) { diff --git a/pkg/controller/jenkins/configuration/base/resources/pod.go b/pkg/controller/jenkins/configuration/base/resources/pod.go index 1a40a4b9..fe899575 100644 --- a/pkg/controller/jenkins/configuration/base/resources/pod.go +++ b/pkg/controller/jenkins/configuration/base/resources/pod.go @@ -288,6 +288,7 @@ func NewJenkinsMasterPod(objectMeta metav1.ObjectMeta, jenkins *v1alpha2.Jenkins Containers: newContainers(jenkins), Volumes: append(GetJenkinsMasterPodBaseVolumes(jenkins), jenkins.Spec.Master.Volumes...), SecurityContext: jenkins.Spec.Master.SecurityContext, + ImagePullSecrets: jenkins.Spec.Master.ImagePullSecrets, }, } } diff --git a/pkg/controller/jenkins/configuration/base/validate.go b/pkg/controller/jenkins/configuration/base/validate.go index f3de634a..b49f370c 100644 --- a/pkg/controller/jenkins/configuration/base/validate.go +++ b/pkg/controller/jenkins/configuration/base/validate.go @@ -2,6 +2,7 @@ package base import ( "context" + "errors" "fmt" "regexp" @@ -60,6 +61,46 @@ func (r *ReconcileJenkinsBaseConfiguration) Validate(jenkins *v1alpha2.Jenkins) return true, nil } +func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecrets() (bool, error) { + var err error + valid := true + ips := r.jenkins.Spec.Master.ImagePullSecrets + for _, sr := range ips { + valid, err = r.validateImagePullSecret(sr.Name) + if err != nil || !valid { + return valid, err + } + } + + return valid, err +} + +func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecret(name string) (bool, error) { + secret := &corev1.Secret{} + err := r.k8sClient.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: r.jenkins.ObjectMeta.Namespace}, secret) + if err != nil && apierrors.IsNotFound(err) { + r.logger.V(log.VWarn).Info(fmt.Sprintf("Secret '%s' not found", name)) + return false, nil + } else if err != nil && !apierrors.IsNotFound(err) { + return false, stackerr.WithStack(err) + } + + if secret.Data["docker-server"] == nil { + return false, errors.New("docker server not set") + } + if secret.Data["docker-username"] == nil { + return false, errors.New("docker username not set") + } + if secret.Data["docker-password"] == nil { + return false, errors.New("docker password not set") + } + if secret.Data["docker-email"] == nil { + return false, errors.New("docker email not set") + } + + return true, nil +} + func (r *ReconcileJenkinsBaseConfiguration) validateVolumes() (bool, error) { valid := true for _, volume := range r.jenkins.Spec.Master.Volumes { diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go index 07b7c208..cac463b2 100644 --- a/pkg/controller/jenkins/configuration/base/validate_test.go +++ b/pkg/controller/jenkins/configuration/base/validate_test.go @@ -132,6 +132,199 @@ func TestValidatePlugins(t *testing.T) { }) } +func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T) { + t.Run("happy", func(t *testing.T) { + lor := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ref", + }, + Data: map[string][]byte{ + "docker-server": []byte("test_server"), + "docker-username": []byte("test_user"), + "docker-password": []byte("test_password"), + "docker-email": []byte("test_email"), + }, + } + + jenkins := v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: lor.ObjectMeta.Name}, + }, + }, + }, + } + + fakeClient := fake.NewFakeClient() + err := fakeClient.Create(context.TODO(), lor) + assert.NoError(t, err) + + baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), + &jenkins, false, false, nil, nil) + + got, err := baseReconcileLoop.validateImagePullSecrets() + assert.Equal(t, got, true) + assert.NoError(t, err) + }) + + t.Run("no secret", func(t *testing.T) { + jenkins := v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: "test-ref"}, + }, + }, + }, + } + + fakeClient := fake.NewFakeClient() + + baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), + &jenkins, false, false, nil, nil) + + got, _ := baseReconcileLoop.validateImagePullSecrets() + assert.Equal(t, got, false) + }) + + t.Run("no docker email", func(t *testing.T) { + lor := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ref", + }, + Data: map[string][]byte{ + "docker-server": []byte("test_server"), + "docker-username": []byte("test_user"), + "docker-password": []byte("test_password"), + }, + } + + jenkins := v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: lor.ObjectMeta.Name}, + }, + }, + }, + } + + fakeClient := fake.NewFakeClient() + err := fakeClient.Create(context.TODO(), lor) + assert.NoError(t, err) + + baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), + &jenkins, false, false, nil, nil) + + got, err := baseReconcileLoop.validateImagePullSecrets() + assert.Equal(t, got, false) + assert.Error(t, err) + }) + + t.Run("no docker password", func(t *testing.T) { + lor := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ref", + }, + Data: map[string][]byte{ + "docker-server": []byte("test_server"), + "docker-username": []byte("test_user"), + "docker-email": []byte("test_email"), + }, + } + + jenkins := v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: lor.ObjectMeta.Name}, + }, + }, + }, + } + + fakeClient := fake.NewFakeClient() + err := fakeClient.Create(context.TODO(), lor) + assert.NoError(t, err) + + baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), + &jenkins, false, false, nil, nil) + + got, err := baseReconcileLoop.validateImagePullSecrets() + assert.Equal(t, got, false) + assert.Error(t, err) + }) + + t.Run("no docker username", func(t *testing.T) { + lor := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ref", + }, + Data: map[string][]byte{ + "docker-server": []byte("test_server"), + "docker-password": []byte("test_password"), + "docker-email": []byte("test_email"), + }, + } + + jenkins := v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: lor.ObjectMeta.Name}, + }, + }, + }, + } + + fakeClient := fake.NewFakeClient() + err := fakeClient.Create(context.TODO(), lor) + assert.NoError(t, err) + + baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), + &jenkins, false, false, nil, nil) + + got, err := baseReconcileLoop.validateImagePullSecrets() + assert.Equal(t, got, false) + assert.Error(t, err) + }) + + t.Run("no docker server", func(t *testing.T) { + lor := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ref", + }, + Data: map[string][]byte{ + "docker-username": []byte("test_user"), + "docker-password": []byte("test_password"), + "docker-email": []byte("test_email"), + }, + } + + jenkins := v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + ImagePullSecrets: []corev1.LocalObjectReference{ + {Name: lor.ObjectMeta.Name}, + }, + }, + }, + } + + fakeClient := fake.NewFakeClient() + err := fakeClient.Create(context.TODO(), lor) + assert.NoError(t, err) + + baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), + &jenkins, false, false, nil, nil) + + got, err := baseReconcileLoop.validateImagePullSecrets() + assert.Equal(t, got, false) + assert.Error(t, err) + }) +} + func TestValidateJenkinsMasterPodEnvs(t *testing.T) { t.Run("happy", func(t *testing.T) { jenkins := v1alpha2.Jenkins{ From 7849ddc6f5beb0dc490739e8fa926737ff79d3c0 Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Fri, 12 Jul 2019 14:15:46 +0200 Subject: [PATCH 2/6] Improve docs --- docs/getting-started.md | 86 +++++++++++++++++-- docs/migration-guide-v1alphav1-to-v1alpha2.md | 2 +- 2 files changed, 81 insertions(+), 7 deletions(-) diff --git a/docs/getting-started.md b/docs/getting-started.md index 4e63cd7b..a1bf705d 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -5,12 +5,13 @@ This document describes a getting started guide for **jenkins-operator** and an 1. [First Steps](#first-steps) 2. [Deploy Jenkins](#deploy-jenkins) 3. [Configure Seed Jobs and Pipelines](#configure-seed-jobs-and-pipelines) -4. [Install Plugins](#install-plugins) -5. [Configure Backup & Restore](#configure-backup-and-restore) -6. [AKS](#aks) -7. [Jenkins login credentials](#jenkins-login-credentials) -8. [Override default Jenkins container command](#override-default-Jenkins-container-command) -9. [Debugging](#debugging) +4. [Pulling custom Jenkins image from Docker Registry](#Pulling custom Jenkins image from Docker Registry) +5. [Install Plugins](#install-plugins) +6. [Configure Backup & Restore](#configure-backup-and-restore) +7. [AKS](#aks) +8. [Jenkins login credentials](#jenkins-login-credentials) +9. [Override default Jenkins container command](#override-default-Jenkins-container-command) +10. [Debugging](#debugging) ## First Steps @@ -293,6 +294,79 @@ data: password: password_or_token ``` +## Pulling custom Jenkins image from Docker Registry +Since **0.2.0** version it's possible to use custom prebuilt Jenkins Docker Image using `imagePullSecrets` annotation support. + +Please follow the instructions on [creating a secret with a docker config](https://kubernetes.io/docs/concepts/containers/images/?origin_team=T42NTAGHM#creating-a-secret-with-a-docker-config). + +### Docker Hub Configuration +To use Docker Hub additional steps are required. + +Edit the previously created secret: +```bash +kubectl edit secret +``` + +The `data..dockerconfigjson` key's value needs to be replaced with a modified version. + +After modifications it needs to be encoded as Base64 value before setting the `.dockerconfigjson` key:q. + +Example config file to modify and use: +``` +{ + "auths":{ + "https://index.docker.io/v1/":{ + "username":"user", + "password":"password", + "email":"yourdockeremail@gmail.com", + "auth":"base64 of string user:password" + }, + "auth.docker.io":{ + "username":"user", + "password":"password", + "email":"yourdockeremail@gmail.com", + "auth":"base64 of string user:password" + }, + "registry.docker.io":{ + "username":"user", + "password":"password", + "email":"yourdockeremail@gmail.com", + "auth":"base64 of string user:password" + }, + "docker.io":{ + "username":"user", + "password":"password", + "email":"yourdockeremail@gmail.com", + "auth":"base64 of string user:password" + }, + "https://registry-1.docker.io/v2/": { + "username":"user", + "password":"password", + "email":"yourdockeremail@gmail.com", + "auth":"base64 of string user:password" + }, + "registry-1.docker.io/v2/": { + "username":"user", + "password":"password", + "email":"yourdockeremail@gmail.com", + "auth":"base64 of string user:password" + }, + "registry-1.docker.io": { + "username":"user", + "password":"password", + "email":"yourdockeremail@gmail.com", + "auth":"base64 of string user:password" + }, + "https://registry-1.docker.io": { + "username":"user", + "password":"password", + "email":"yourdockeremail@gmail.com", + "auth":"base64 of string user:password" + } + } +} +``` + ## Jenkins Customisation Jenkins can be customized using groovy scripts or configuration as code plugin. All custom configuration is stored in diff --git a/docs/migration-guide-v1alphav1-to-v1alpha2.md b/docs/migration-guide-v1alphav1-to-v1alpha2.md index 6b70f3df..7c7ac93a 100644 --- a/docs/migration-guide-v1alphav1-to-v1alpha2.md +++ b/docs/migration-guide-v1alphav1-to-v1alpha2.md @@ -308,7 +308,7 @@ To use default CRD file: kubectl -n apply -f https://github.com/jenkinsci/kubernetes-operator/blob/master/deploy/crds/jenkins_v1alpha2_jenkins_crd.yaml ``` -## Update RBAC to new verison +## Update RBAC to new version New operator version requires updated RBAC permissions: From feae4e6e3d11f131e69ea99f3c0f3ffac58fe022 Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Fri, 12 Jul 2019 14:53:22 +0200 Subject: [PATCH 3/6] #45 Improve feature --- docs/getting-started.md | 10 +++---- .../jenkins/configuration/base/validate.go | 27 ++++++++--------- .../configuration/base/validate_test.go | 30 +++++++++---------- 3 files changed, 33 insertions(+), 34 deletions(-) diff --git a/docs/getting-started.md b/docs/getting-started.md index a1bf705d..7860a4e7 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -5,7 +5,7 @@ This document describes a getting started guide for **jenkins-operator** and an 1. [First Steps](#first-steps) 2. [Deploy Jenkins](#deploy-jenkins) 3. [Configure Seed Jobs and Pipelines](#configure-seed-jobs-and-pipelines) -4. [Pulling custom Jenkins image from Docker Registry](#Pulling custom Jenkins image from Docker Registry) +4. [Pulling Docker images from private repositories](#Pulling Docker images from private repositories) 5. [Install Plugins](#install-plugins) 6. [Configure Backup & Restore](#configure-backup-and-restore) 7. [AKS](#aks) @@ -294,8 +294,8 @@ data: password: password_or_token ``` -## Pulling custom Jenkins image from Docker Registry -Since **0.2.0** version it's possible to use custom prebuilt Jenkins Docker Image using `imagePullSecrets` annotation support. +## Pulling Docker images from private repositories +To pull Docker Image from private repository you can use `imagePullSecrets`. Please follow the instructions on [creating a secret with a docker config](https://kubernetes.io/docs/concepts/containers/images/?origin_team=T42NTAGHM#creating-a-secret-with-a-docker-config). @@ -304,10 +304,10 @@ To use Docker Hub additional steps are required. Edit the previously created secret: ```bash -kubectl edit secret +kubectl -n edit secret ``` -The `data..dockerconfigjson` key's value needs to be replaced with a modified version. +The `.dockerconfigjson` key's value needs to be replaced with a modified version. After modifications it needs to be encoded as Base64 value before setting the `.dockerconfigjson` key:q. diff --git a/pkg/controller/jenkins/configuration/base/validate.go b/pkg/controller/jenkins/configuration/base/validate.go index b49f370c..c96da282 100644 --- a/pkg/controller/jenkins/configuration/base/validate.go +++ b/pkg/controller/jenkins/configuration/base/validate.go @@ -2,7 +2,6 @@ package base import ( "context" - "errors" "fmt" "regexp" @@ -62,40 +61,40 @@ func (r *ReconcileJenkinsBaseConfiguration) Validate(jenkins *v1alpha2.Jenkins) } func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecrets() (bool, error) { - var err error - valid := true - ips := r.jenkins.Spec.Master.ImagePullSecrets - for _, sr := range ips { - valid, err = r.validateImagePullSecret(sr.Name) + for _, sr := range r.jenkins.Spec.Master.ImagePullSecrets { + valid, err := r.validateImagePullSecret(sr.Name) if err != nil || !valid { - return valid, err + return true, err } } - - return valid, err + return false, nil } func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecret(name string) (bool, error) { secret := &corev1.Secret{} err := r.k8sClient.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: r.jenkins.ObjectMeta.Namespace}, secret) if err != nil && apierrors.IsNotFound(err) { - r.logger.V(log.VWarn).Info(fmt.Sprintf("Secret '%s' not found", name)) + r.logger.V(log.VWarn).Info(fmt.Sprintf("Secret %s not found defined in spec.master.imagePullSecrets", name)) return false, nil } else if err != nil && !apierrors.IsNotFound(err) { return false, stackerr.WithStack(err) } if secret.Data["docker-server"] == nil { - return false, errors.New("docker server not set") + r.logger.V(log.VWarn).Info("Docker Server is empty") + return false, nil } if secret.Data["docker-username"] == nil { - return false, errors.New("docker username not set") + r.logger.V(log.VWarn).Info("Docker Username is empty") + return false, nil } if secret.Data["docker-password"] == nil { - return false, errors.New("docker password not set") + r.logger.V(log.VWarn).Info("Docker Password is empty") + return false, nil } if secret.Data["docker-email"] == nil { - return false, errors.New("docker email not set") + r.logger.V(log.VWarn).Info("Docker Email is empty") + return false, nil } return true, nil diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go index cac463b2..6deba48b 100644 --- a/pkg/controller/jenkins/configuration/base/validate_test.go +++ b/pkg/controller/jenkins/configuration/base/validate_test.go @@ -134,7 +134,7 @@ func TestValidatePlugins(t *testing.T) { func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T) { t.Run("happy", func(t *testing.T) { - lor := &corev1.Secret{ + secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ref", }, @@ -150,14 +150,14 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T Spec: v1alpha2.JenkinsSpec{ Master: v1alpha2.JenkinsMaster{ ImagePullSecrets: []corev1.LocalObjectReference{ - {Name: lor.ObjectMeta.Name}, + {Name: secret.ObjectMeta.Name}, }, }, }, } fakeClient := fake.NewFakeClient() - err := fakeClient.Create(context.TODO(), lor) + err := fakeClient.Create(context.TODO(), secret) assert.NoError(t, err) baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), @@ -189,7 +189,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T }) t.Run("no docker email", func(t *testing.T) { - lor := &corev1.Secret{ + secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ref", }, @@ -204,14 +204,14 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T Spec: v1alpha2.JenkinsSpec{ Master: v1alpha2.JenkinsMaster{ ImagePullSecrets: []corev1.LocalObjectReference{ - {Name: lor.ObjectMeta.Name}, + {Name: secret.ObjectMeta.Name}, }, }, }, } fakeClient := fake.NewFakeClient() - err := fakeClient.Create(context.TODO(), lor) + err := fakeClient.Create(context.TODO(), secret) assert.NoError(t, err) baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), @@ -223,7 +223,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T }) t.Run("no docker password", func(t *testing.T) { - lor := &corev1.Secret{ + secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ref", }, @@ -238,14 +238,14 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T Spec: v1alpha2.JenkinsSpec{ Master: v1alpha2.JenkinsMaster{ ImagePullSecrets: []corev1.LocalObjectReference{ - {Name: lor.ObjectMeta.Name}, + {Name: secret.ObjectMeta.Name}, }, }, }, } fakeClient := fake.NewFakeClient() - err := fakeClient.Create(context.TODO(), lor) + err := fakeClient.Create(context.TODO(), secret) assert.NoError(t, err) baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), @@ -257,7 +257,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T }) t.Run("no docker username", func(t *testing.T) { - lor := &corev1.Secret{ + secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ref", }, @@ -272,14 +272,14 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T Spec: v1alpha2.JenkinsSpec{ Master: v1alpha2.JenkinsMaster{ ImagePullSecrets: []corev1.LocalObjectReference{ - {Name: lor.ObjectMeta.Name}, + {Name: secret.ObjectMeta.Name}, }, }, }, } fakeClient := fake.NewFakeClient() - err := fakeClient.Create(context.TODO(), lor) + err := fakeClient.Create(context.TODO(), secret) assert.NoError(t, err) baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), @@ -291,7 +291,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T }) t.Run("no docker server", func(t *testing.T) { - lor := &corev1.Secret{ + secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-ref", }, @@ -306,14 +306,14 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T Spec: v1alpha2.JenkinsSpec{ Master: v1alpha2.JenkinsMaster{ ImagePullSecrets: []corev1.LocalObjectReference{ - {Name: lor.ObjectMeta.Name}, + {Name: secret.ObjectMeta.Name}, }, }, }, } fakeClient := fake.NewFakeClient() - err := fakeClient.Create(context.TODO(), lor) + err := fakeClient.Create(context.TODO(), secret) assert.NoError(t, err) baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), From 74b8ec98ec1c3efa35d310dc90d4982ca3ab3c9c Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Mon, 15 Jul 2019 09:04:39 +0200 Subject: [PATCH 4/6] Fix validation bug --- pkg/controller/jenkins/configuration/base/resources/pod.go | 2 +- pkg/controller/jenkins/configuration/base/validate.go | 7 ++++--- pkg/controller/jenkins/configuration/base/validate_test.go | 4 ---- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pkg/controller/jenkins/configuration/base/resources/pod.go b/pkg/controller/jenkins/configuration/base/resources/pod.go index fe899575..64d911f7 100644 --- a/pkg/controller/jenkins/configuration/base/resources/pod.go +++ b/pkg/controller/jenkins/configuration/base/resources/pod.go @@ -288,7 +288,7 @@ func NewJenkinsMasterPod(objectMeta metav1.ObjectMeta, jenkins *v1alpha2.Jenkins Containers: newContainers(jenkins), Volumes: append(GetJenkinsMasterPodBaseVolumes(jenkins), jenkins.Spec.Master.Volumes...), SecurityContext: jenkins.Spec.Master.SecurityContext, - ImagePullSecrets: jenkins.Spec.Master.ImagePullSecrets, + ImagePullSecrets: jenkins.Spec.Master.ImagePullSecrets, }, } } diff --git a/pkg/controller/jenkins/configuration/base/validate.go b/pkg/controller/jenkins/configuration/base/validate.go index c96da282..61f801e0 100644 --- a/pkg/controller/jenkins/configuration/base/validate.go +++ b/pkg/controller/jenkins/configuration/base/validate.go @@ -61,13 +61,14 @@ func (r *ReconcileJenkinsBaseConfiguration) Validate(jenkins *v1alpha2.Jenkins) } func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecrets() (bool, error) { + var err error for _, sr := range r.jenkins.Spec.Master.ImagePullSecrets { valid, err := r.validateImagePullSecret(sr.Name) if err != nil || !valid { - return true, err + return false, nil } } - return false, nil + return true, err } func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecret(name string) (bool, error) { @@ -92,7 +93,7 @@ func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecret(name string) r.logger.V(log.VWarn).Info("Docker Password is empty") return false, nil } - if secret.Data["docker-email"] == nil { + if secret.Data["docker-email"] == nil { r.logger.V(log.VWarn).Info("Docker Email is empty") return false, nil } diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go index 6deba48b..7a8aedbf 100644 --- a/pkg/controller/jenkins/configuration/base/validate_test.go +++ b/pkg/controller/jenkins/configuration/base/validate_test.go @@ -219,7 +219,6 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T got, err := baseReconcileLoop.validateImagePullSecrets() assert.Equal(t, got, false) - assert.Error(t, err) }) t.Run("no docker password", func(t *testing.T) { @@ -253,7 +252,6 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T got, err := baseReconcileLoop.validateImagePullSecrets() assert.Equal(t, got, false) - assert.Error(t, err) }) t.Run("no docker username", func(t *testing.T) { @@ -287,7 +285,6 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T got, err := baseReconcileLoop.validateImagePullSecrets() assert.Equal(t, got, false) - assert.Error(t, err) }) t.Run("no docker server", func(t *testing.T) { @@ -321,7 +318,6 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T got, err := baseReconcileLoop.validateImagePullSecrets() assert.Equal(t, got, false) - assert.Error(t, err) }) } From 811e816b2f34c28623210de4bee784175c86b06c Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Mon, 15 Jul 2019 09:14:53 +0200 Subject: [PATCH 5/6] Fixed tests --- pkg/controller/jenkins/configuration/base/validate.go | 2 +- .../jenkins/configuration/base/validate_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/controller/jenkins/configuration/base/validate.go b/pkg/controller/jenkins/configuration/base/validate.go index 61f801e0..ee7a8822 100644 --- a/pkg/controller/jenkins/configuration/base/validate.go +++ b/pkg/controller/jenkins/configuration/base/validate.go @@ -93,7 +93,7 @@ func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecret(name string) r.logger.V(log.VWarn).Info("Docker Password is empty") return false, nil } - if secret.Data["docker-email"] == nil { + if secret.Data["docker-email"] == nil { r.logger.V(log.VWarn).Info("Docker Email is empty") return false, nil } diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go index 7a8aedbf..3dcb258c 100644 --- a/pkg/controller/jenkins/configuration/base/validate_test.go +++ b/pkg/controller/jenkins/configuration/base/validate_test.go @@ -217,7 +217,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), &jenkins, false, false, nil, nil) - got, err := baseReconcileLoop.validateImagePullSecrets() + got, _ := baseReconcileLoop.validateImagePullSecrets() assert.Equal(t, got, false) }) @@ -250,7 +250,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), &jenkins, false, false, nil, nil) - got, err := baseReconcileLoop.validateImagePullSecrets() + got, _ := baseReconcileLoop.validateImagePullSecrets() assert.Equal(t, got, false) }) @@ -283,7 +283,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), &jenkins, false, false, nil, nil) - got, err := baseReconcileLoop.validateImagePullSecrets() + got, _ := baseReconcileLoop.validateImagePullSecrets() assert.Equal(t, got, false) }) @@ -316,7 +316,7 @@ func TestReconcileJenkinsBaseConfiguration_validateImagePullSecrets(t *testing.T baseReconcileLoop := New(fakeClient, nil, logf.ZapLogger(false), &jenkins, false, false, nil, nil) - got, err := baseReconcileLoop.validateImagePullSecrets() + got, _ := baseReconcileLoop.validateImagePullSecrets() assert.Equal(t, got, false) }) } From 0fed26b7330d7f271e8489e2b380467f71ea14a1 Mon Sep 17 00:00:00 2001 From: Jakub Al-Khalili Date: Mon, 15 Jul 2019 09:56:47 +0200 Subject: [PATCH 6/6] Improve tests & docs --- docs/getting-started.md | 2 +- pkg/controller/jenkins/configuration/base/validate.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/getting-started.md b/docs/getting-started.md index 7860a4e7..6907e1b3 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -5,7 +5,7 @@ This document describes a getting started guide for **jenkins-operator** and an 1. [First Steps](#first-steps) 2. [Deploy Jenkins](#deploy-jenkins) 3. [Configure Seed Jobs and Pipelines](#configure-seed-jobs-and-pipelines) -4. [Pulling Docker images from private repositories](#Pulling Docker images from private repositories) +4. [Pulling Docker images from private repositories](#pulling-docker-images-from-private-repositories) 5. [Install Plugins](#install-plugins) 6. [Configure Backup & Restore](#configure-backup-and-restore) 7. [AKS](#aks) diff --git a/pkg/controller/jenkins/configuration/base/validate.go b/pkg/controller/jenkins/configuration/base/validate.go index ee7a8822..4ff7f27f 100644 --- a/pkg/controller/jenkins/configuration/base/validate.go +++ b/pkg/controller/jenkins/configuration/base/validate.go @@ -82,19 +82,19 @@ func (r *ReconcileJenkinsBaseConfiguration) validateImagePullSecret(name string) } if secret.Data["docker-server"] == nil { - r.logger.V(log.VWarn).Info("Docker Server is empty") + r.logger.V(log.VWarn).Info(fmt.Sprintf("Secret '%s' defined in spec.master.imagePullSecrets doesn't have 'docker-server' key.", name)) return false, nil } if secret.Data["docker-username"] == nil { - r.logger.V(log.VWarn).Info("Docker Username is empty") + r.logger.V(log.VWarn).Info(fmt.Sprintf("Secret '%s' defined in spec.master.imagePullSecrets doesn't have 'docker-username' key.", name)) return false, nil } if secret.Data["docker-password"] == nil { - r.logger.V(log.VWarn).Info("Docker Password is empty") + r.logger.V(log.VWarn).Info(fmt.Sprintf("Secret '%s' defined in spec.master.imagePullSecrets doesn't have 'docker-password' key.", name)) return false, nil } if secret.Data["docker-email"] == nil { - r.logger.V(log.VWarn).Info("Docker Email is empty") + r.logger.V(log.VWarn).Info(fmt.Sprintf("Secret '%s' defined in spec.master.imagePullSecrets doesn't have 'docker-email' key.", name)) return false, nil }