Skip to content

Commit 95683e5

Browse files
camilamacedo86camilamacedo86
committed
fix(deploymentConfig): merge volumes/volumeMounts by name instead of appending
Generated-by: Cursour/Claude
1 parent 0db26d7 commit 95683e5

3 files changed

Lines changed: 309 additions & 8 deletions

File tree

‎internal/operator-controller/rukpak/render/registryv1/generators/generators.go‎

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -671,29 +671,73 @@ func applyEnvironmentFromConfig(deployment *appsv1.Deployment, config *config.De
671671
}
672672
}
673673

674-
// applyVolumeConfig appends volumes to the deployment's pod spec.
675-
// This follows OLMv0 behavior:
676-
// https://github.com/operator-framework/operator-lifecycle-manager/blob/v0.39.0/pkg/controller/operators/olm/overrides/inject/inject.go#L104-L117
674+
// applyVolumeConfig merges volumes into the deployment's pod spec.
675+
// Volumes from config override existing volumes with the same name.
677676
func applyVolumeConfig(deployment *appsv1.Deployment, config *config.DeploymentConfig) {
678677
if len(config.Volumes) == 0 {
679678
return
680679
}
681680

682-
deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, config.Volumes...)
681+
deployment.Spec.Template.Spec.Volumes = mergeVolumes(deployment.Spec.Template.Spec.Volumes, config.Volumes)
683682
}
684683

685-
// applyVolumeMountConfig appends volume mounts to all containers in the deployment.
686-
// This follows OLMv0 behavior:
687-
// https://github.com/operator-framework/operator-lifecycle-manager/blob/v0.39.0/pkg/controller/operators/olm/overrides/inject/inject.go#L149-L165
684+
// applyVolumeMountConfig merges volume mounts into all containers in the deployment.
685+
// Volume mounts from config override existing volume mounts with the same name.
688686
func applyVolumeMountConfig(deployment *appsv1.Deployment, config *config.DeploymentConfig) {
689687
if len(config.VolumeMounts) == 0 {
690688
return
691689
}
692690

693691
for i := range deployment.Spec.Template.Spec.Containers {
694692
container := &deployment.Spec.Template.Spec.Containers[i]
695-
container.VolumeMounts = append(container.VolumeMounts, config.VolumeMounts...)
693+
container.VolumeMounts = mergeVolumeMounts(container.VolumeMounts, config.VolumeMounts)
694+
}
695+
}
696+
697+
// mergeVolumes merges newVolumes into existing volumes, overriding volumes with matching names.
698+
func mergeVolumes(existing []corev1.Volume, newVolumes []corev1.Volume) []corev1.Volume {
699+
merged := make([]corev1.Volume, len(existing))
700+
copy(merged, existing)
701+
702+
for _, newVolume := range newVolumes {
703+
found := false
704+
for i := range merged {
705+
if merged[i].Name == newVolume.Name {
706+
// Override the existing volume with the new one
707+
merged[i] = newVolume
708+
found = true
709+
break
710+
}
711+
}
712+
if !found {
713+
// Append if this is a new volume
714+
merged = append(merged, newVolume)
715+
}
716+
}
717+
return merged
718+
}
719+
720+
// mergeVolumeMounts merges newVolumeMounts into existing volume mounts, overriding mounts with matching names.
721+
func mergeVolumeMounts(existing []corev1.VolumeMount, newVolumeMounts []corev1.VolumeMount) []corev1.VolumeMount {
722+
merged := make([]corev1.VolumeMount, len(existing))
723+
copy(merged, existing)
724+
725+
for _, newVolumeMount := range newVolumeMounts {
726+
found := false
727+
for i := range merged {
728+
if merged[i].Name == newVolumeMount.Name {
729+
// Override the existing volume mount with the new one
730+
merged[i] = newVolumeMount
731+
found = true
732+
break
733+
}
734+
}
735+
if !found {
736+
// Append if this is a new volume mount
737+
merged = append(merged, newVolumeMount)
738+
}
696739
}
740+
return merged
697741
}
698742

699743
// applyTolerationsConfig appends tolerations to the deployment's pod spec.

‎internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go‎

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3085,6 +3085,205 @@ func Test_BundleCSVDeploymentGenerator_WithDeploymentConfig(t *testing.T) {
30853085
require.Equal(t, "existing_value", dep.Spec.Template.Spec.Containers[0].Env[0].Value)
30863086
},
30873087
},
3088+
{
3089+
name: "merges volumes from deployment config - overrides matching names",
3090+
bundle: &bundle.RegistryV1{
3091+
CSV: clusterserviceversion.Builder().
3092+
WithStrategyDeploymentSpecs(
3093+
v1alpha1.StrategyDeploymentSpec{
3094+
Name: "test-deployment",
3095+
Spec: appsv1.DeploymentSpec{
3096+
Template: corev1.PodTemplateSpec{
3097+
Spec: corev1.PodSpec{
3098+
Containers: []corev1.Container{
3099+
{Name: "manager"},
3100+
},
3101+
Volumes: []corev1.Volume{
3102+
{
3103+
Name: "bundle-emptydir-vol",
3104+
VolumeSource: corev1.VolumeSource{
3105+
EmptyDir: &corev1.EmptyDirVolumeSource{},
3106+
},
3107+
},
3108+
{
3109+
Name: "existing-vol",
3110+
VolumeSource: corev1.VolumeSource{
3111+
EmptyDir: &corev1.EmptyDirVolumeSource{},
3112+
},
3113+
},
3114+
},
3115+
},
3116+
},
3117+
},
3118+
},
3119+
).Build(),
3120+
},
3121+
opts: render.Options{
3122+
InstallNamespace: "test-ns",
3123+
TargetNamespaces: []string{"test-ns"},
3124+
DeploymentConfig: &config.DeploymentConfig{
3125+
Volumes: []corev1.Volume{
3126+
{
3127+
Name: "bundle-emptydir-vol",
3128+
VolumeSource: corev1.VolumeSource{
3129+
ConfigMap: &corev1.ConfigMapVolumeSource{
3130+
LocalObjectReference: corev1.LocalObjectReference{
3131+
Name: "test-cm-vol",
3132+
},
3133+
},
3134+
},
3135+
},
3136+
{
3137+
Name: "config-secret-vol",
3138+
VolumeSource: corev1.VolumeSource{
3139+
Secret: &corev1.SecretVolumeSource{
3140+
SecretName: "test-secret-vol",
3141+
},
3142+
},
3143+
},
3144+
},
3145+
},
3146+
},
3147+
verify: func(t *testing.T, objs []client.Object) {
3148+
require.Len(t, objs, 1)
3149+
dep := objs[0].(*appsv1.Deployment)
3150+
volumes := dep.Spec.Template.Spec.Volumes
3151+
3152+
// Should have 3 volumes total:
3153+
// - bundle-emptydir-vol (overridden to ConfigMap)
3154+
// - existing-vol (unchanged)
3155+
// - config-secret-vol (new)
3156+
require.Len(t, volumes, 3)
3157+
3158+
// Verify bundle-emptydir-vol was overridden (now ConfigMap, not EmptyDir)
3159+
var bundleVol *corev1.Volume
3160+
for i := range volumes {
3161+
if volumes[i].Name == "bundle-emptydir-vol" {
3162+
bundleVol = &volumes[i]
3163+
break
3164+
}
3165+
}
3166+
require.NotNil(t, bundleVol, "bundle-emptydir-vol should exist")
3167+
require.NotNil(t, bundleVol.ConfigMap, "bundle-emptydir-vol should be ConfigMap")
3168+
require.Equal(t, "test-cm-vol", bundleVol.ConfigMap.Name)
3169+
require.Nil(t, bundleVol.EmptyDir, "bundle-emptydir-vol should not be EmptyDir")
3170+
3171+
// Verify existing-vol remains unchanged
3172+
var existingVol *corev1.Volume
3173+
for i := range volumes {
3174+
if volumes[i].Name == "existing-vol" {
3175+
existingVol = &volumes[i]
3176+
break
3177+
}
3178+
}
3179+
require.NotNil(t, existingVol, "existing-vol should exist")
3180+
require.NotNil(t, existingVol.EmptyDir, "existing-vol should still be EmptyDir")
3181+
3182+
// Verify config-secret-vol was added
3183+
var secretVol *corev1.Volume
3184+
for i := range volumes {
3185+
if volumes[i].Name == "config-secret-vol" {
3186+
secretVol = &volumes[i]
3187+
break
3188+
}
3189+
}
3190+
require.NotNil(t, secretVol, "config-secret-vol should exist")
3191+
require.NotNil(t, secretVol.Secret, "config-secret-vol should be Secret")
3192+
require.Equal(t, "test-secret-vol", secretVol.Secret.SecretName)
3193+
},
3194+
},
3195+
{
3196+
name: "merges volumeMounts from deployment config - overrides matching names",
3197+
bundle: &bundle.RegistryV1{
3198+
CSV: clusterserviceversion.Builder().
3199+
WithStrategyDeploymentSpecs(
3200+
v1alpha1.StrategyDeploymentSpec{
3201+
Name: "test-deployment",
3202+
Spec: appsv1.DeploymentSpec{
3203+
Template: corev1.PodTemplateSpec{
3204+
Spec: corev1.PodSpec{
3205+
Containers: []corev1.Container{
3206+
{
3207+
Name: "manager",
3208+
VolumeMounts: []corev1.VolumeMount{
3209+
{
3210+
Name: "bundle-vol",
3211+
MountPath: "/old/path",
3212+
},
3213+
{
3214+
Name: "existing-vol",
3215+
MountPath: "/existing/path",
3216+
},
3217+
},
3218+
},
3219+
},
3220+
},
3221+
},
3222+
},
3223+
},
3224+
).Build(),
3225+
},
3226+
opts: render.Options{
3227+
InstallNamespace: "test-ns",
3228+
TargetNamespaces: []string{"test-ns"},
3229+
DeploymentConfig: &config.DeploymentConfig{
3230+
VolumeMounts: []corev1.VolumeMount{
3231+
{
3232+
Name: "bundle-vol",
3233+
MountPath: "/new/path",
3234+
},
3235+
{
3236+
Name: "config-vol",
3237+
MountPath: "/config/path",
3238+
},
3239+
},
3240+
},
3241+
},
3242+
verify: func(t *testing.T, objs []client.Object) {
3243+
require.Len(t, objs, 1)
3244+
dep := objs[0].(*appsv1.Deployment)
3245+
volumeMounts := dep.Spec.Template.Spec.Containers[0].VolumeMounts
3246+
3247+
// Should have 3 volume mounts total:
3248+
// - bundle-vol (overridden to /new/path)
3249+
// - existing-vol (unchanged)
3250+
// - config-vol (new)
3251+
require.Len(t, volumeMounts, 3)
3252+
3253+
// Verify bundle-vol was overridden
3254+
var bundleMount *corev1.VolumeMount
3255+
for i := range volumeMounts {
3256+
if volumeMounts[i].Name == "bundle-vol" {
3257+
bundleMount = &volumeMounts[i]
3258+
break
3259+
}
3260+
}
3261+
require.NotNil(t, bundleMount, "bundle-vol should exist")
3262+
require.Equal(t, "/new/path", bundleMount.MountPath, "bundle-vol mount path should be overridden")
3263+
3264+
// Verify existing-vol remains unchanged
3265+
var existingMount *corev1.VolumeMount
3266+
for i := range volumeMounts {
3267+
if volumeMounts[i].Name == "existing-vol" {
3268+
existingMount = &volumeMounts[i]
3269+
break
3270+
}
3271+
}
3272+
require.NotNil(t, existingMount, "existing-vol should exist")
3273+
require.Equal(t, "/existing/path", existingMount.MountPath)
3274+
3275+
// Verify config-vol was added
3276+
var configMount *corev1.VolumeMount
3277+
for i := range volumeMounts {
3278+
if volumeMounts[i].Name == "config-vol" {
3279+
configMount = &volumeMounts[i]
3280+
break
3281+
}
3282+
}
3283+
require.NotNil(t, configMount, "config-vol should exist")
3284+
require.Equal(t, "/config/path", configMount.MountPath)
3285+
},
3286+
},
30883287
} {
30893288
t.Run(tc.name, func(t *testing.T) {
30903289
objs, err := generators.BundleCSVDeploymentGenerator(tc.bundle, tc.opts)

‎test/e2e/features/install.feature‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,3 +535,61 @@ Feature: Install ClusterExtension
535535
nodeSelector:
536536
kubernetes.io/os: linux
537537
"""
538+
539+
@DeploymentConfig
540+
Scenario: deploymentConfig volumes and volumeMounts are applied to the operator deployment
541+
When ClusterExtension is applied
542+
"""
543+
apiVersion: olm.operatorframework.io/v1
544+
kind: ClusterExtension
545+
metadata:
546+
name: ${NAME}
547+
spec:
548+
namespace: ${TEST_NAMESPACE}
549+
serviceAccount:
550+
name: olm-sa
551+
config:
552+
configType: Inline
553+
inline:
554+
deploymentConfig:
555+
volumes:
556+
- name: custom-config-volume
557+
configMap:
558+
name: test-configmap
559+
- name: custom-secret-volume
560+
secret:
561+
secretName: test-secret
562+
volumeMounts:
563+
- name: custom-config-volume
564+
mountPath: /custom/config
565+
- name: custom-secret-volume
566+
mountPath: /custom/secret
567+
source:
568+
sourceType: Catalog
569+
catalog:
570+
packageName: test
571+
selector:
572+
matchLabels:
573+
"olm.operatorframework.io/metadata.name": test-catalog
574+
"""
575+
Then ClusterExtension is rolled out
576+
And ClusterExtension is available
577+
And resource "deployment/test-operator" matches
578+
"""
579+
spec:
580+
template:
581+
spec:
582+
volumes:
583+
- name: custom-config-volume
584+
configMap:
585+
name: test-configmap
586+
- name: custom-secret-volume
587+
secret:
588+
secretName: test-secret
589+
containers:
590+
- volumeMounts:
591+
- name: custom-config-volume
592+
mountPath: /custom/config
593+
- name: custom-secret-volume
594+
mountPath: /custom/secret
595+
"""

0 commit comments

Comments
 (0)