fix: do not add barman-certificates projection if not needed

Closes #347

Signed-off-by: Marco Nenciarini <[email protected]>

Author: mnencia

internal/cnpgi/operator/lifecycle.go

@@ -333,6 +333,7 @@ func reconcilePodSpec(
 			Drop: []corev1.Capability{"ALL"},
 		},
 	}
+	sidecarTemplate.RestartPolicy = ptr.To(corev1.ContainerRestartPolicyAlways)
 	sidecarTemplate.Resources = config.resources
 
 	// merge the main container envs if they aren't already set
@@ -368,20 +369,28 @@ func reconcilePodSpec(
 		}
 	}
 
-	if err := injectPluginSidecarPodSpec(spec, &sidecarTemplate, mainContainerName); err != nil {
-		return err
-	}
+	if len(config.certificates) > 0 {
+		sidecarTemplate.VolumeMounts = append(
+			sidecarTemplate.VolumeMounts,
+			corev1.VolumeMount{
+				Name:      barmanCertificatesVolumeName,
+				MountPath: metadata.BarmanCertificatesPath,
+			})
 
-	// inject the volume containing the certificates if needed
-	if !volumeListHasVolume(spec.Volumes, barmanCertificatesVolumeName) {
-		spec.Volumes = append(spec.Volumes, corev1.Volume{
+		spec.Volumes = volumeListEnsureVolume(spec.Volumes, corev1.Volume{
 			Name: barmanCertificatesVolumeName,
 			VolumeSource: corev1.VolumeSource{
 				Projected: &corev1.ProjectedVolumeSource{
 					Sources: config.certificates,
 				},
 			},
 		})
+	} else {
+		spec.Volumes = volumeListRemoveVolume(spec.Volumes, barmanCertificatesVolumeName)
+	}
+
+	if err := injectPluginSidecarPodSpec(spec, &sidecarTemplate, mainContainerName); err != nil {
+		return err
 	}
 
 	return nil
@@ -428,10 +437,6 @@ func InjectPluginVolumePodSpec(spec *corev1.PodSpec, mainContainerName string) {
 }
 
 // injectPluginSidecarPodSpec injects a plugin sidecar into a CNPG Pod spec.
-//
-// If the "injectMainContainerVolumes" flag is true, this will append all the volume
-// mounts that are used in the instance manager Pod to the passed sidecar
-// container, granting it superuser access to the PostgreSQL instance.
 func injectPluginSidecarPodSpec(
 	spec *corev1.PodSpec,
 	sidecar *corev1.Container,
@@ -440,12 +445,11 @@ func injectPluginSidecarPodSpec(
 	sidecar = sidecar.DeepCopy()
 	InjectPluginVolumePodSpec(spec, mainContainerName)
 
-	var volumeMounts []corev1.VolumeMount
 	sidecarContainerFound := false
 	mainContainerFound := false
 	for i := range spec.Containers {
 		if spec.Containers[i].Name == mainContainerName {
-			volumeMounts = spec.Containers[i].VolumeMounts
+			sidecar.VolumeMounts = append(sidecar.VolumeMounts, spec.Containers[i].VolumeMounts...)
 			mainContainerFound = true
 		}
 	}
@@ -457,38 +461,45 @@ func injectPluginSidecarPodSpec(
 	for i := range spec.InitContainers {
 		if spec.InitContainers[i].Name == sidecar.Name {
 			sidecarContainerFound = true
+			spec.InitContainers[i] = *sidecar
 		}
 	}
 
-	if sidecarContainerFound {
-		// The sidecar container was already added
-		return nil
+	if !sidecarContainerFound {
+		spec.InitContainers = append(spec.InitContainers, *sidecar)
 	}
 
-	// Do not modify the passed sidecar definition
-	sidecar.VolumeMounts = append(
-		sidecar.VolumeMounts,
-		corev1.VolumeMount{
-			Name:      barmanCertificatesVolumeName,
-			MountPath: metadata.BarmanCertificatesPath,
-		})
-	sidecar.VolumeMounts = append(sidecar.VolumeMounts, volumeMounts...)
-	sidecar.RestartPolicy = ptr.To(corev1.ContainerRestartPolicyAlways)
-	spec.InitContainers = append(spec.InitContainers, *sidecar)
-
 	return nil
 }
 
-// volumeListHasVolume check if a volume with a known name exists
-// in the volume list
-func volumeListHasVolume(volumes []corev1.Volume, name string) bool {
+// volumeListEnsureVolume makes sure the passed volume is present in the list of volumes.
+// If the volume is already present, it is updated.
+func volumeListEnsureVolume(volumes []corev1.Volume, volume corev1.Volume) []corev1.Volume {
+	volumeFound := false
 	for i := range volumes {
-		if volumes[i].Name == name {
-			return true
+		if volumes[i].Name == volume.Name {
+			volumeFound = true
+			volumes[i] = volume
 		}
 	}
 
-	return false
+	if !volumeFound {
+		volumes = append(volumes, volume)
+	}
+
+	return volumes
+}
+
+// volumeListRemoveVolume removes a volume with a known name from a list of volumes.
+func volumeListRemoveVolume(volumes []corev1.Volume, name string) []corev1.Volume {
+	j := 0
+	for _, volume := range volumes {
+		if volume.Name != name {
+			volumes[j] = volume
+			j++
+		}
+	}
+	return volumes[:j]
 }
 
 // getCNPGJobRole gets the role associated to a CNPG job

internal/cnpgi/operator/lifecycle_test.go

@@ -209,3 +209,44 @@ var _ = Describe("LifecycleImplementation", func() {
 		})
 	})
 })
+
+var _ = Describe("Volume utilities", func() {
+	Describe("volumeListEnsureVolume", func() {
+		It("adds a new volume if not present", func() {
+			volumes := []corev1.Volume{{Name: "vol1"}}
+			newVol := corev1.Volume{Name: "vol2"}
+			result := volumeListEnsureVolume(volumes, newVol)
+			Expect(result).To(HaveLen(2))
+			Expect(result[1]).To(Equal(newVol))
+		})
+
+		It("updates an existing volume", func() {
+			volumes := []corev1.Volume{{Name: "vol1"}}
+			updatedVol := corev1.Volume{Name: "vol1", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}
+			result := volumeListEnsureVolume(volumes, updatedVol)
+			Expect(result).To(HaveLen(1))
+			Expect(result[0]).To(Equal(updatedVol))
+		})
+	})
+
+	Describe("volumeListRemoveVolume", func() {
+		It("removes the specified volume", func() {
+			volumes := []corev1.Volume{
+				{Name: "vol1"},
+				{Name: "vol2"},
+				{Name: "vol3"},
+			}
+			result := volumeListRemoveVolume(volumes, "vol2")
+			Expect(result).To(HaveLen(2))
+			for _, v := range result {
+				Expect(v.Name).NotTo(Equal("vol2"))
+			}
+		})
+
+		It("returns the same list if the volume is not present", func() {
+			volumes := []corev1.Volume{{Name: "vol1"}, {Name: "vol2"}}
+			result := volumeListRemoveVolume(volumes, "vol3")
+			Expect(result).To(Equal(volumes))
+		})
+	})
+})