From ebe750aa8e7ea25a9555dadec4df57ba400825e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20S=C4=99k?= Date: Sat, 19 Jan 2019 13:14:19 +0100 Subject: [PATCH] Move validation to AWS S3 backup provider --- .../jenkins/configuration/base/validate.go | 31 ++----- .../configuration/base/validate_test.go | 76 ---------------- .../jenkins/configuration/user/validate.go | 40 ++------- .../configuration/user/validate_test.go | 86 ------------------- 4 files changed, 16 insertions(+), 217 deletions(-) diff --git a/pkg/controller/jenkins/configuration/base/validate.go b/pkg/controller/jenkins/configuration/base/validate.go index aa5fbe28..ce63cddd 100644 --- a/pkg/controller/jenkins/configuration/base/validate.go +++ b/pkg/controller/jenkins/configuration/base/validate.go @@ -6,6 +6,7 @@ import ( "regexp" virtuslabv1alpha1 "github.com/VirtusLab/jenkins-operator/pkg/apis/virtuslab/v1alpha1" + "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/backup" "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/configuration/base/resources" "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/plugins" "github.com/VirtusLab/jenkins-operator/pkg/log" @@ -42,7 +43,12 @@ func (r *ReconcileJenkinsBaseConfiguration) Validate(jenkins *virtuslabv1alpha1. return valid, err } - if r.jenkins.Spec.Backup == virtuslabv1alpha1.JenkinsBackupTypeAmazonS3 && !r.verifyBackupAmazonS3() { + backupProvider, err := backup.GetBackupProvider(r.jenkins.Spec.Backup) + if err != nil { + return false, err + } + + if !backupProvider.IsConfigurationValidForBasePhase(*r.jenkins, r.logger) { return false, nil } @@ -86,8 +92,8 @@ func (r *ReconcileJenkinsBaseConfiguration) verifyBackup() (bool, error) { } valid := false - for _, backup := range virtuslabv1alpha1.AllowedJenkinsBackups { - if r.jenkins.Spec.Backup == backup { + for _, backupType := range virtuslabv1alpha1.AllowedJenkinsBackups { + if r.jenkins.Spec.Backup == backupType { valid = true } } @@ -114,22 +120,3 @@ func (r *ReconcileJenkinsBaseConfiguration) verifyBackup() (bool, error) { return true, nil } - -func (r *ReconcileJenkinsBaseConfiguration) verifyBackupAmazonS3() bool { - if len(r.jenkins.Spec.BackupAmazonS3.BucketName) == 0 { - r.logger.V(log.VWarn).Info("Bucket name not set in 'spec.backupAmazonS3.bucketName'") - return false - } - - if len(r.jenkins.Spec.BackupAmazonS3.BucketPath) == 0 { - r.logger.V(log.VWarn).Info("Bucket path not set in 'spec.backupAmazonS3.bucketPath'") - return false - } - - if len(r.jenkins.Spec.BackupAmazonS3.Region) == 0 { - r.logger.V(log.VWarn).Info("Region not set in 'spec.backupAmazonS3.region'") - return false - } - - return true -} diff --git a/pkg/controller/jenkins/configuration/base/validate_test.go b/pkg/controller/jenkins/configuration/base/validate_test.go index 10d72579..51602680 100644 --- a/pkg/controller/jenkins/configuration/base/validate_test.go +++ b/pkg/controller/jenkins/configuration/base/validate_test.go @@ -146,79 +146,3 @@ func TestReconcileJenkinsBaseConfiguration_verifyBackup(t *testing.T) { }) } } - -func TestReconcileJenkinsBaseConfiguration_verifyBackupAmazonS3(t *testing.T) { - - tests := []struct { - name string - jenkins *virtuslabv1alpha1.Jenkins - want bool - }{ - { - name: "happy", - jenkins: &virtuslabv1alpha1.Jenkins{ - Spec: virtuslabv1alpha1.JenkinsSpec{ - BackupAmazonS3: virtuslabv1alpha1.JenkinsBackupAmazonS3{ - BucketName: "some-value", - BucketPath: "some-value", - Region: "some-value", - }, - }, - }, - want: true, - }, - { - name: "fail, no bucket name", - jenkins: &virtuslabv1alpha1.Jenkins{ - Spec: virtuslabv1alpha1.JenkinsSpec{ - BackupAmazonS3: virtuslabv1alpha1.JenkinsBackupAmazonS3{ - BucketName: "", - BucketPath: "some-value", - Region: "some-value", - }, - }, - }, - want: false, - }, - { - name: "fail, no bucket path", - jenkins: &virtuslabv1alpha1.Jenkins{ - Spec: virtuslabv1alpha1.JenkinsSpec{ - BackupAmazonS3: virtuslabv1alpha1.JenkinsBackupAmazonS3{ - BucketName: "some-value", - BucketPath: "", - Region: "some-value", - }, - }, - }, - want: false, - }, - { - name: "fail, no region", - jenkins: &virtuslabv1alpha1.Jenkins{ - Spec: virtuslabv1alpha1.JenkinsSpec{ - BackupAmazonS3: virtuslabv1alpha1.JenkinsBackupAmazonS3{ - BucketName: "some-value", - BucketPath: "some-value", - Region: "", - }, - }, - }, - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - r := &ReconcileJenkinsBaseConfiguration{ - k8sClient: fake.NewFakeClient(), - scheme: nil, - logger: logf.ZapLogger(false), - jenkins: tt.jenkins, - local: false, - minikube: false, - } - got := r.verifyBackupAmazonS3() - assert.Equal(t, tt.want, got) - }) - } -} diff --git a/pkg/controller/jenkins/configuration/user/validate.go b/pkg/controller/jenkins/configuration/user/validate.go index 7119f7d2..2083d18a 100644 --- a/pkg/controller/jenkins/configuration/user/validate.go +++ b/pkg/controller/jenkins/configuration/user/validate.go @@ -9,12 +9,10 @@ import ( "strings" virtuslabv1alpha1 "github.com/VirtusLab/jenkins-operator/pkg/apis/virtuslab/v1alpha1" - "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/configuration/base/resources" - "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/constants" + "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/backup" "github.com/VirtusLab/jenkins-operator/pkg/log" "k8s.io/api/core/v1" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" ) @@ -26,7 +24,12 @@ func (r *ReconcileUserConfiguration) Validate(jenkins *virtuslabv1alpha1.Jenkins return valid, err } - return r.verifyBackup() + backupProvider, err := backup.GetBackupProvider(r.jenkins.Spec.Backup) + if err != nil { + return false, err + } + + return backupProvider.IsConfigurationValidForUserPhase(r.k8sClient, *r.jenkins, r.logger) } func (r *ReconcileUserConfiguration) validateSeedJobs(jenkins *virtuslabv1alpha1.Jenkins) (bool, error) { @@ -95,32 +98,3 @@ func validatePrivateKey(privateKey string) error { return nil } - -func (r *ReconcileUserConfiguration) verifyBackup() (bool, error) { - if r.jenkins.Spec.Backup == virtuslabv1alpha1.JenkinsBackupTypeAmazonS3 { - return r.verifyBackupAmazonS3() - } - - return true, nil -} - -func (r *ReconcileUserConfiguration) verifyBackupAmazonS3() (bool, error) { - backupSecretName := resources.GetBackupCredentialsSecretName(r.jenkins) - backupSecret := &corev1.Secret{} - err := r.k8sClient.Get(context.TODO(), types.NamespacedName{Namespace: r.jenkins.Namespace, Name: backupSecretName}, backupSecret) - if err != nil { - return false, err - } - - if len(backupSecret.Data[constants.BackupAmazonS3SecretSecretKey]) == 0 { - r.logger.V(log.VWarn).Info(fmt.Sprintf("Secret '%s' doesn't contains key: %s", backupSecretName, constants.BackupAmazonS3SecretSecretKey)) - return false, nil - } - - if len(backupSecret.Data[constants.BackupAmazonS3SecretAccessKey]) == 0 { - r.logger.V(log.VWarn).Info(fmt.Sprintf("Secret '%s' doesn't contains key: %s", backupSecretName, constants.BackupAmazonS3SecretAccessKey)) - return false, nil - } - - return true, nil -} diff --git a/pkg/controller/jenkins/configuration/user/validate_test.go b/pkg/controller/jenkins/configuration/user/validate_test.go index c38843af..97bce2f4 100644 --- a/pkg/controller/jenkins/configuration/user/validate_test.go +++ b/pkg/controller/jenkins/configuration/user/validate_test.go @@ -6,7 +6,6 @@ import ( "testing" virtuslabv1alpha1 "github.com/VirtusLab/jenkins-operator/pkg/apis/virtuslab/v1alpha1" - "github.com/VirtusLab/jenkins-operator/pkg/controller/jenkins/constants" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" @@ -237,88 +236,3 @@ func TestValidateSeedJobs(t *testing.T) { }) } } - -func TestReconcileUserConfiguration_verifyBackupAmazonS3(t *testing.T) { - tests := []struct { - name string - jenkins *virtuslabv1alpha1.Jenkins - secret *corev1.Secret - want bool - wantErr bool - }{ - { - name: "happy", - jenkins: &virtuslabv1alpha1.Jenkins{ - ObjectMeta: metav1.ObjectMeta{Namespace: "namespace-name", Name: "jenkins-cr-name"}, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: "namespace-name", Name: "jenkins-operator-backup-credentials-jenkins-cr-name"}, - Data: map[string][]byte{ - constants.BackupAmazonS3SecretSecretKey: []byte("some-value"), - constants.BackupAmazonS3SecretAccessKey: []byte("some-value"), - }, - }, - want: true, - wantErr: false, - }, - { - name: "fail, no secret", - jenkins: &virtuslabv1alpha1.Jenkins{ - ObjectMeta: metav1.ObjectMeta{Namespace: "namespace-name", Name: "jenkins-cr-name"}, - }, - want: false, - wantErr: true, - }, - { - name: "fail, no secret key in secret", - jenkins: &virtuslabv1alpha1.Jenkins{ - ObjectMeta: metav1.ObjectMeta{Namespace: "namespace-name", Name: "jenkins-cr-name"}, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: "namespace-name", Name: "jenkins-operator-backup-credentials-jenkins-cr-name"}, - Data: map[string][]byte{ - constants.BackupAmazonS3SecretSecretKey: []byte(""), - constants.BackupAmazonS3SecretAccessKey: []byte("some-value"), - }, - }, - want: false, - wantErr: false, - }, - { - name: "fail, no access key in secret", - jenkins: &virtuslabv1alpha1.Jenkins{ - ObjectMeta: metav1.ObjectMeta{Namespace: "namespace-name", Name: "jenkins-cr-name"}, - }, - secret: &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: "namespace-name", Name: "jenkins-operator-backup-credentials-jenkins-cr-name"}, - Data: map[string][]byte{ - constants.BackupAmazonS3SecretSecretKey: []byte("some-value"), - constants.BackupAmazonS3SecretAccessKey: []byte(""), - }, - }, - want: false, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - r := &ReconcileUserConfiguration{ - k8sClient: fake.NewFakeClient(), - jenkinsClient: nil, - logger: logf.ZapLogger(false), - jenkins: tt.jenkins, - } - if tt.secret != nil { - e := r.k8sClient.Create(context.TODO(), tt.secret) - assert.NoError(t, e) - } - got, err := r.verifyBackupAmazonS3() - if tt.wantErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - assert.Equal(t, tt.want, got) - }) - } -}