diff --git a/api/v1alpha2/jenkins_types.go b/api/v1alpha2/jenkins_types.go index f71b32d3..190802b4 100644 --- a/api/v1alpha2/jenkins_types.go +++ b/api/v1alpha2/jenkins_types.go @@ -73,6 +73,21 @@ type JenkinsSpec struct { // JenkinsAPISettings defines configuration used by the operator to gain admin access to the Jenkins API JenkinsAPISettings JenkinsAPISettings `json:"jenkinsAPISettings"` + + // +optional + Lifecycle JenkinsLifecycle `json:"lifecycle,omitempty"` +} + +type JenkinsLifecycle struct { + // +optional + Ignore JenkinsLifecycleIgnore `json:"ignore,omitempty"` +} + +type JenkinsLifecycleIgnore struct { + IgnoredVolumes []string `json:"volumes,omitempty"` + IgnoredEnvs []string `json:"envs,omitempty"` + IgnoredAnnotations []string `json:"annotations,omitempty"` + IgnoredLabels []string `json:"labels,omitempty"` } // AuthorizationStrategy defines authorization strategy of the operator for the Jenkins API diff --git a/pkg/configuration/base/pod.go b/pkg/configuration/base/pod.go index 579e085f..2f68f879 100644 --- a/pkg/configuration/base/pod.go +++ b/pkg/configuration/base/pod.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "slices" "github.com/jenkinsci/kubernetes-operator/api/v1alpha2" "github.com/jenkinsci/kubernetes-operator/pkg/configuration/backuprestore" @@ -82,7 +83,7 @@ func (r *JenkinsBaseConfigurationReconciler) checkForPodRecreation(currentJenkin currentJenkinsMasterPod.Labels, r.Configuration.Jenkins.Spec.Master.Labels)) } - if !compareMap(r.Configuration.Jenkins.Spec.Master.Annotations, currentJenkinsMasterPod.ObjectMeta.Annotations) { + if !r.compareAnnotations(currentJenkinsMasterPod) { messages = append(messages, "Jenkins pod annotations have changed") verbose = append(verbose, fmt.Sprintf("Jenkins pod annotations have changed, actual '%+v' required '%+v'", currentJenkinsMasterPod.ObjectMeta.Annotations, r.Configuration.Jenkins.Spec.Master.Annotations)) @@ -146,6 +147,20 @@ func (r *JenkinsBaseConfigurationReconciler) checkForPodRecreation(currentJenkin return reason.NewPodRestart(reason.OperatorSource, messages, verbose...) } +func (r *JenkinsBaseConfigurationReconciler) compareAnnotations(currentJenkinsMasterPod corev1.Pod) bool { + ignoredAnnotations := r.Jenkins.Spec.Lifecycle.Ignore.IgnoredAnnotations + annotations := r.Configuration.Jenkins.Spec.Master.Annotations + + res := make(map[string]string) + for key, val := range annotations { + if slices.Contains(ignoredAnnotations, key) { + continue + } + res[key] = val + } + return compareMap(res, currentJenkinsMasterPod.ObjectMeta.Annotations) +} + func (r *JenkinsBaseConfigurationReconciler) ensureJenkinsMasterPod(meta metav1.ObjectMeta) (reconcile.Result, error) { userAndPasswordHash, err := r.calculateUserAndPasswordHash() if err != nil { diff --git a/pkg/configuration/base/reconcile_test.go b/pkg/configuration/base/reconcile_test.go index 2c39c175..49c6b174 100644 --- a/pkg/configuration/base/reconcile_test.go +++ b/pkg/configuration/base/reconcile_test.go @@ -97,6 +97,114 @@ func TestCompareContainerVolumeMounts(t *testing.T) { }) } +func TestCompareAnnotations(t *testing.T) { + type testCase struct { + name string + jenkinsAnnotations map[string]string + ignoredAnnotations []string + podAnnotations map[string]string + expectedShouldMatch bool + } + + runTest := func(t *testing.T, tc testCase) { + + t.Run(tc.name, func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{ + Annotations: tc.jenkinsAnnotations, + }, + }, + } + + if len(tc.ignoredAnnotations) > 0 { + jenkins.Spec.Lifecycle = v1alpha2.JenkinsLifecycle{ + Ignore: v1alpha2.JenkinsLifecycleIgnore{ + IgnoredAnnotations: tc.ignoredAnnotations, + }, + } + } + + pod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: tc.podAnnotations, + }, + } + + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) + result := reconciler.compareAnnotations(pod) + + assert.Equal(t, tc.expectedShouldMatch, result, + "Expected compareAnnotations to return %v but got %v", tc.expectedShouldMatch, result) + }) + } + + testCases := []testCase{ + { + name: "no annotation - additional annotations - not different", + jenkinsAnnotations: map[string]string{}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"one": "two"}, + expectedShouldMatch: true, + }, + { + name: "one additional annotation - change not ignored - not different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"one": "two", "additional": "annotation"}, + expectedShouldMatch: true, + }, + { + + name: "annotations different - different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"two": "three"}, + expectedShouldMatch: false, + }, + + { + name: "annotations different - ignored - not different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: []string{"one"}, + podAnnotations: map[string]string{"two": "three"}, + expectedShouldMatch: true, + }, + { + name: "one annotation different - change not ignored - different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"one": "different"}, + expectedShouldMatch: false, + }, + { + name: "one annotation different - change ignored - not different", + jenkinsAnnotations: map[string]string{"one": "two"}, + ignoredAnnotations: []string{"one"}, + podAnnotations: map[string]string{"one": "different"}, + expectedShouldMatch: true, + }, + { + name: "one additional annotation - different", + jenkinsAnnotations: map[string]string{"one": "two", "ignore": "me"}, + ignoredAnnotations: nil, + podAnnotations: map[string]string{"one": "two", "ignore": "this"}, + expectedShouldMatch: false, + }, + { + name: "one additional annotation - change ignored - not different", + jenkinsAnnotations: map[string]string{"one": "two", "ignore": "me"}, + ignoredAnnotations: []string{"ignore"}, + podAnnotations: map[string]string{"one": "two", "ignore": "this"}, + expectedShouldMatch: true, + }, + } + + for _, tc := range testCases { + runTest(t, tc) + } +} + func TestCompareVolumes(t *testing.T) { t.Run("defaults", func(t *testing.T) { jenkins := &v1alpha2.Jenkins{} @@ -160,6 +268,74 @@ func TestCompareVolumes(t *testing.T) { assert.True(t, got) }) + + t.Run("different - additional workspace identity volume", func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + Spec: v1alpha2.JenkinsSpec{ + Master: v1alpha2.JenkinsMaster{}, + }, + } + pod := corev1.Pod{ + Spec: corev1.PodSpec{ + ServiceAccountName: "service-account-name", + Volumes: append(resources.GetJenkinsMasterPodBaseVolumes(jenkins), corev1.Volume{Name: "azure-identity-token"}), + }, + } + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) + + got := reconciler.compareVolumes(pod) + + assert.False(t, got) + }) + + t.Run("additional workspace identity volume but ignored", func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + + Spec: v1alpha2.JenkinsSpec{ + Lifecycle: v1alpha2.JenkinsLifecycle{ + Ignore: v1alpha2.JenkinsLifecycleIgnore{ + IgnoredVolumes: []string{"azure-identity-token"}, + }, + }, + }, + } + pod := corev1.Pod{ + Spec: corev1.PodSpec{ + ServiceAccountName: "service-account-name", + Volumes: append(resources.GetJenkinsMasterPodBaseVolumes(jenkins), corev1.Volume{Name: "azure-identity-token"}), + }, + } + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) + + got := reconciler.compareVolumes(pod) + + assert.True(t, got) + }) + + t.Run("additional multiple volumes added but ignored", func(t *testing.T) { + jenkins := &v1alpha2.Jenkins{ + + Spec: v1alpha2.JenkinsSpec{ + Lifecycle: v1alpha2.JenkinsLifecycle{ + Ignore: v1alpha2.JenkinsLifecycleIgnore{ + IgnoredVolumes: []string{"volume-present", "volume-absent"}, + }, + }, + }, + } + pod := corev1.Pod{ + Spec: corev1.PodSpec{ + ServiceAccountName: "service-account-name", + Volumes: append(resources.GetJenkinsMasterPodBaseVolumes(jenkins), corev1.Volume{Name: "volume-present"}), + }, + } + reconciler := New(configuration.Configuration{Jenkins: jenkins}, client.JenkinsAPIConnectionSettings{}) + + got := reconciler.compareVolumes(pod) + + assert.True(t, got) + }) + } func TestJenkinsBaseConfigurationReconciler_verifyPlugins(t *testing.T) { diff --git a/pkg/configuration/base/reconciler.go b/pkg/configuration/base/reconciler.go index e5096513..53112074 100644 --- a/pkg/configuration/base/reconciler.go +++ b/pkg/configuration/base/reconciler.go @@ -294,6 +294,11 @@ func CompareContainerVolumeMounts(expected corev1.Container, actual corev1.Conta func (r *JenkinsBaseConfigurationReconciler) compareVolumes(actualPod corev1.Pod) bool { var toCompare []corev1.Volume for _, volume := range actualPod.Spec.Volumes { + + if r.isVolumeIgnored(volume.Name) { + continue + } + // filter out service account if strings.HasPrefix(volume.Name, actualPod.Spec.ServiceAccountName) { continue @@ -421,3 +426,13 @@ func (r *JenkinsBaseConfigurationReconciler) ensureBaseConfiguration(jenkinsClie }) return reconcile.Result{Requeue: requeue}, err } + +// isVolumeIgnored checks if the given volume name is in the list of ignored volumes +func (r *JenkinsBaseConfigurationReconciler) isVolumeIgnored(volumeName string) bool { + for _, ignoredVolume := range r.Jenkins.Spec.Lifecycle.Ignore.IgnoredVolumes { + if ignoredVolume == volumeName { + return true + } + } + return false +}