Fix issue with gerrit/github conns using same sec name

If a least two Gerrit or Github connections use the same secret, then the Volume and VolumeMount get duplicated and kube fails to apply the resource. This change fixes that behavior by ensuring to avoid adding duplicated Volume and VolumeMount. Change-Id: Ia500e3c51823670f18ef3533894535892f6261c5
softwarefactory-project · Aug 13, 2024 · e86018a · e86018a
1 parent 121eca5
commit e86018a
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 58 deletions.
diff --git a/controllers/libs/utils/utils.go b/controllers/libs/utils/utils.go
@@ -38,6 +38,7 @@ func Int32Ptr(i int32) *int32 { return &i }
 func BoolPtr(b bool) *bool    { return &b }
 
 var Execmod int32 = 493 // decimal for 0755 octal
+var Readmod int32 = 256 // decimal for 0400 octal
 
 // LogI logs a message with the INFO log Level
 func LogI(msg string) {

diff --git a/controllers/logserver.go b/controllers/logserver.go
@@ -160,7 +160,6 @@ func (r *SFController) DeployLogserver() bool {
 	// Setup the main container
 	sts.Spec.Template.Spec.Containers[0].VolumeMounts = volumeMounts
 
-	var mod int32 = 256 // decimal for 0400 octal
 	sts.Spec.Template.Spec.Volumes = []apiv1.Volume{
 		{
 			Name: logserverIdent + "-config-vol",
@@ -178,7 +177,7 @@ func (r *SFController) DeployLogserver() bool {
 			VolumeSource: apiv1.VolumeSource{
 				Secret: &apiv1.SecretVolumeSource{
 					SecretName:  logserverIdent + "-keys",
-					DefaultMode: &mod,
+					DefaultMode: &utils.Readmod,
 				},
 			},
 		},

diff --git a/controllers/nodepool.go b/controllers/nodepool.go
@@ -409,7 +409,6 @@ func (r *SFController) DeployNodepoolBuilder(statsdExporterVolume apiv1.Volume,
 	builderExtraConfigData["httpd-build-logs-dir.conf"] = httpdBuildLogsDirConfig
 	r.EnsureConfigMap("nodepool-builder-extra-config", builderExtraConfigData)
 
-	var mod int32 = 256 // decimal for 0400 octal
 	// get statsd relay if defined
 	var relayAddress *string
 	if r.cr.Spec.Nodepool.StatsdTarget != "" {
@@ -427,7 +426,7 @@ func (r *SFController) DeployNodepoolBuilder(statsdExporterVolume apiv1.Volume,
 			VolumeSource: apiv1.VolumeSource{
 				Secret: &apiv1.SecretVolumeSource{
 					SecretName:  "nodepool-builder-ssh-key",
-					DefaultMode: &mod,
+					DefaultMode: &utils.Readmod,
 				},
 			},
 		},
@@ -440,7 +439,7 @@ func (r *SFController) DeployNodepoolBuilder(statsdExporterVolume apiv1.Volume,
 						Key:  "pub",
 						Path: "pub",
 					}},
-					DefaultMode: &mod,
+					DefaultMode: &utils.Readmod,
 				},
 			},
 		},

diff --git a/controllers/zuul.go b/controllers/zuul.go
@@ -17,6 +17,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/utils/ptr"
+	"k8s.io/utils/strings/slices"
 
 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	sfv1 "github.com/softwarefactory-project/sf-operator/api/v1"
@@ -105,32 +106,6 @@ func mkZuulLoggingMount(service string) apiv1.VolumeMount {
 	}
 }
 
-func mkZuulConnectionsSecretsMount(r *SFController) []apiv1.VolumeMount {
-	zuulConnectionMounts := []apiv1.VolumeMount{}
-	for _, connection := range r.cr.Spec.Zuul.GitHubConns {
-		secretName := connection.Secrets
-		if connection.AppID > 0 {
-			zuulConnectionMounts = append(zuulConnectionMounts, apiv1.VolumeMount{
-				Name:      secretName,
-				MountPath: "/var/lib/zuul/" + secretName + "/app_key",
-				SubPath:   "app_key",
-			})
-		}
-	}
-
-	for _, conn := range r.cr.Spec.Zuul.GerritConns {
-		if conn.Sshkey != "" {
-			keyMount := apiv1.VolumeMount{
-				Name:      "zuul-ssh-key-" + conn.Sshkey,
-				MountPath: "/var/lib/zuul-" + conn.Sshkey + "/",
-			}
-			zuulConnectionMounts = append(zuulConnectionMounts, keyMount)
-		}
-	}
-
-	return zuulConnectionMounts
-}
-
 func getZuulImage(service string) string {
 	switch srv := service; srv {
 	case "zuul-scheduler":
@@ -147,6 +122,35 @@ func getZuulImage(service string) string {
 }
 
 func (r *SFController) mkZuulContainer(service string, corporateCMExists bool) apiv1.Container {
+
+	mkZuulConnectionsSecretsMount := func(r *SFController) []apiv1.VolumeMount {
+		added := []string{}
+		zuulConnectionMounts := []apiv1.VolumeMount{}
+		for _, conn := range r.cr.Spec.Zuul.GitHubConns {
+			if conn.AppID > 0 && !slices.Contains(added, conn.Secrets) {
+				zuulConnectionMounts = append(zuulConnectionMounts, apiv1.VolumeMount{
+					Name:      conn.Secrets,
+					MountPath: "/var/lib/zuul/" + conn.Secrets + "/app_key",
+					SubPath:   "app_key",
+				})
+				added = append(added, conn.Secrets)
+			}
+		}
+
+		for _, conn := range r.cr.Spec.Zuul.GerritConns {
+			if conn.Sshkey != "" && !slices.Contains(added, conn.Sshkey) {
+				keyMount := apiv1.VolumeMount{
+					Name:      "zuul-ssh-key-" + conn.Sshkey,
+					MountPath: "/var/lib/zuul-" + conn.Sshkey + "/",
+				}
+				zuulConnectionMounts = append(zuulConnectionMounts, keyMount)
+				added = append(added, conn.Sshkey)
+			}
+		}
+
+		return zuulConnectionMounts
+	}
+
 	volumeMounts := []apiv1.VolumeMount{
 		{
 			Name:      "zuul-config",
@@ -242,7 +246,32 @@ func (r *SFController) mkZuulContainer(service string, corporateCMExists bool) a
 }
 
 func mkZuulVolumes(service string, r *SFController, corporateCMExists bool) []apiv1.Volume {
-	var mod int32 = 256 // decimal for 0400 octal
+	mkZuulConnectionSecretsVolumes := func(r *SFController) []apiv1.Volume {
+		added := []string{}
+		volumes := []apiv1.Volume{}
+		for _, conn := range r.cr.Spec.Zuul.GitHubConns {
+			if conn.Secrets != "" && !slices.Contains(added, conn.Secrets) {
+				volumes = append(volumes, base.MkVolumeSecret(conn.Secrets))
+				added = append(added, conn.Secrets)
+			}
+		}
+		for _, conn := range r.cr.Spec.Zuul.GerritConns {
+			if conn.Sshkey != "" && !slices.Contains(added, conn.Sshkey) {
+				keyVol := apiv1.Volume{
+					Name: "zuul-ssh-key-" + conn.Sshkey,
+					VolumeSource: apiv1.VolumeSource{
+						Secret: &apiv1.SecretVolumeSource{
+							SecretName:  conn.Sshkey,
+							DefaultMode: &utils.Readmod,
+						},
+					},
+				}
+				volumes = append(volumes, keyVol)
+				added = append(added, conn.Sshkey)
+			}
+		}
+		return volumes
+	}
 
 	// create extra config config map
 	r.EnsureConfigMap("zuul-extra", map[string]string{
@@ -267,7 +296,7 @@ func mkZuulVolumes(service string, r *SFController, corporateCMExists bool) []ap
 			VolumeSource: apiv1.VolumeSource{
 				Secret: &apiv1.SecretVolumeSource{
 					SecretName:  "zuul-ssh-key",
-					DefaultMode: &mod,
+					DefaultMode: &utils.Readmod,
 				},
 			},
 		},
@@ -295,22 +324,7 @@ func mkZuulVolumes(service string, r *SFController, corporateCMExists bool) []ap
 		volumes = append(volumes, toolingVol)
 	}
 
-	for _, conn := range r.cr.Spec.Zuul.GerritConns {
-		if conn.Sshkey != "" {
-			keyVol := apiv1.Volume{
-				Name: "zuul-ssh-key-" + conn.Sshkey,
-				VolumeSource: apiv1.VolumeSource{
-					Secret: &apiv1.SecretVolumeSource{
-						SecretName:  conn.Sshkey,
-						DefaultMode: &mod,
-					},
-				},
-			}
-			volumes = append(volumes, keyVol)
-		}
-	}
-
-	volumes = append(volumes, mkZuulGitHubSecretsVolumes(r)...)
+	volumes = append(volumes, mkZuulConnectionSecretsVolumes(r)...)
 
 	if corporateCMExists {
 		volumes = append(volumes, base.MkVolumeCM(service+"-corporate-ca-certs", "corporate-ca-certs"))
@@ -405,15 +419,6 @@ func (r *SFController) getZuulLoggingString(service string) string {
 	return loggingcm.Data[service+"-logging.yaml"]
 }
 
-func mkZuulGitHubSecretsVolumes(r *SFController) []apiv1.Volume {
-	gitConnectionSecretVolumes := []apiv1.Volume{}
-	for _, connection := range r.cr.Spec.Zuul.GitHubConns {
-		secretName := connection.Secrets
-		gitConnectionSecretVolumes = append(gitConnectionSecretVolumes, base.MkVolumeSecret(secretName))
-	}
-	return gitConnectionSecretVolumes
-}
-
 func (r *SFController) EnsureZuulScheduler(cfg *ini.File) bool {
 	sections := utils.IniGetSectionNamesByPrefix(cfg, "connection")
 	authSections := utils.IniGetSectionNamesByPrefix(cfg, "auth")

diff --git a/doc/reference/CHANGELOG.md b/doc/reference/CHANGELOG.md
@@ -16,6 +16,9 @@ All notable changes to this project will be documented in this file.
 ### Deprecated
 ### Removed
 ### Fixed
+
+- Fail to deploy Zuul when multiple connections use the same Secret name (Gerrit and Github)
+
 ### Security
 
 ## [v0.0.34] - 2024-07-24

diff --git a/roles/health-check/zuul-connections/tasks/main.yaml b/roles/health-check/zuul-connections/tasks/main.yaml
@@ -6,9 +6,15 @@
         hostname: dummy-gerrit.local
         username: zuul
         sshkey: dummygerritsecret
+      - name: dummy-gerrit-conn-dup
+        hostname: dummy-gerrit.local
+        username: zuul
+        sshkey: dummygerritsecret
     dummy_githubconns:
       - name: dummy-github-conn
         secrets: githubconnectionsecret
+      - name: dummy-github-conn-dup
+        secrets: githubconnectionsecret
     dummy_gitlabconns:
       - name: dummy-gitlab-conn
         secrets: gitlabconnectionsecret
@@ -109,7 +115,9 @@
       register: this
       until:
         - "'dummy-gerrit-conn' in this.content"
+        - "'dummy-gerrit-conn-dup' in this.content"
         - "'dummy-github-conn' in this.content"
+        - "'dummy-github-conn-dup' in this.content"
         - "'dummy-gitlab-conn' in this.content"
         - "'dummy-git-conn' in this.content"
         - "'dummy-pagure-conn' in this.content"
@@ -150,7 +158,9 @@
       register: this
       until:
         - "'dummy-gerrit-conn' not in this.content"
+        - "'dummy-gerrit-conn-dup' not in this.content"
         - "'dummy-github-conn' not in this.content"
+        - "'dummy-github-conn-dup' not in this.content"
         - "'dummy-gitlab-conn' not in this.content"
         - "'dummy-git-conn' not in this.content"
         - "'dummy-pagure-conn' not in this.content"