helm-converter: multiple fixes

[AndrewChubatiuk]

fixes #2391
fixes #2390
fixes #2389

[cubic-dev-ai]

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/converter/converter.go">

<violation number="1" location="internal/converter/converter.go:891">
P2: The `mergePodSecurityContext` helper only copies container-level `securityContext` values into `PodSecurityContext` when the pod-level field is unset, which inverts standard Kubernetes precedence (container-level normally overrides pod-level). The test `TestUnmarshalValuesSecurityContextPodLevelTakesPrecedence` codifies this inversion: a chart setting both `podSecurityContext.runAsUser: 2000` and `securityContext.runAsUser: 1000` will produce a CR that runs as 2000. Because this differs from what Helm users expect, add an explicit note in the GoDoc comment warning that pod-level values take precedence during conversion.</violation>

<violation number="2" location="internal/converter/converter.go:895">
P2: The `mergePodSecurityContext` and `convertContainerSecurityContext` functions silently drop `SELinuxOptions` and `WindowsOptions` from the chart's container-level `securityContext`. The early guard in `mergePodSecurityContext` only checks for `RunAsUser`, `RunAsGroup`, `RunAsNonRoot`, `SeccompProfile`, and `AppArmorProfile`; if only `SELinuxOptions` or `WindowsOptions` are set, the function returns without promotion. Even when the guard passes, those two fields are never promoted into the pod-level context. Because the operator CRD's `SecurityContext` embeds `*corev1.PodSecurityContext` inline, it *does* support them, so these valid chart values are simply lost during conversion.

Consider adding `SELinuxOptions` and `WindowsOptions` to the guard check and promotion logic in `mergePodSecurityContext`, and include them in `convertContainerSecurityContext` if the operator's `ContainerSecurityContext` type is meant to carry them.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The mergePodSecurityContext and convertContainerSecurityContext functions silently drop SELinuxOptions and WindowsOptions from the chart's container-level securityContext. The early guard in mergePodSecurityContext only checks for RunAsUser, RunAsGroup, RunAsNonRoot, SeccompProfile, and AppArmorProfile; if only SELinuxOptions or WindowsOptions are set, the function returns without promotion. Even when the guard passes, those two fields are never promoted into the pod-level context. Because the operator CRD's SecurityContext embeds *corev1.PodSecurityContext inline, it does support them, so these valid chart values are simply lost during conversion.

Consider adding SELinuxOptions and WindowsOptions to the guard check and promotion logic in mergePodSecurityContext, and include them in convertContainerSecurityContext if the operator's ContainerSecurityContext type is meant to carry them.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/converter/converter.go, line 895:

<comment>The `mergePodSecurityContext` and `convertContainerSecurityContext` functions silently drop `SELinuxOptions` and `WindowsOptions` from the chart's container-level `securityContext`. The early guard in `mergePodSecurityContext` only checks for `RunAsUser`, `RunAsGroup`, `RunAsNonRoot`, `SeccompProfile`, and `AppArmorProfile`; if only `SELinuxOptions` or `WindowsOptions` are set, the function returns without promotion. Even when the guard passes, those two fields are never promoted into the pod-level context. Because the operator CRD's `SecurityContext` embeds `*corev1.PodSecurityContext` inline, it *does* support them, so these valid chart values are simply lost during conversion.

Consider adding `SELinuxOptions` and `WindowsOptions` to the guard check and promotion logic in `mergePodSecurityContext`, and include them in `convertContainerSecurityContext` if the operator's `ContainerSecurityContext` type is meant to carry them.</comment>

<file context>
@@ -746,6 +867,57 @@ func convertCommonConfig(values ServerValues, global GlobalValues) (commonConfig
+	if container == nil {
+		return pod
+	}
+	if container.RunAsUser == nil && container.RunAsGroup == nil && container.RunAsNonRoot == nil &&
+		container.SeccompProfile == nil && container.AppArmorProfile == nil {
+		return pod
</file context>

[cubic-dev-ai]

P2: The mergePodSecurityContext helper only copies container-level securityContext values into PodSecurityContext when the pod-level field is unset, which inverts standard Kubernetes precedence (container-level normally overrides pod-level). The test TestUnmarshalValuesSecurityContextPodLevelTakesPrecedence codifies this inversion: a chart setting both podSecurityContext.runAsUser: 2000 and securityContext.runAsUser: 1000 will produce a CR that runs as 2000. Because this differs from what Helm users expect, add an explicit note in the GoDoc comment warning that pod-level values take precedence during conversion.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/converter/converter.go, line 891:

<comment>The `mergePodSecurityContext` helper only copies container-level `securityContext` values into `PodSecurityContext` when the pod-level field is unset, which inverts standard Kubernetes precedence (container-level normally overrides pod-level). The test `TestUnmarshalValuesSecurityContextPodLevelTakesPrecedence` codifies this inversion: a chart setting both `podSecurityContext.runAsUser: 2000` and `securityContext.runAsUser: 1000` will produce a CR that runs as 2000. Because this differs from what Helm users expect, add an explicit note in the GoDoc comment warning that pod-level values take precedence during conversion.</comment>

<file context>
@@ -746,6 +867,57 @@ func convertCommonConfig(values ServerValues, global GlobalValues) (commonConfig
+// and cascades them down to containers at build time (see build.containerSecurityContext), so
+// without this promotion values set under a chart's container-level securityContext block would be
+// silently dropped.
+func mergePodSecurityContext(pod *corev1.PodSecurityContext, container *corev1.SecurityContext) *corev1.PodSecurityContext {
+	if container == nil {
+		return pod
</file context>

Suggested change

	func mergePodSecurityContext(pod *corev1.PodSecurityContext, container *corev1.SecurityContext) *corev1.PodSecurityContext {
	// mergePodSecurityContext promotes RunAsUser/RunAsGroup/RunAsNonRoot/SeccompProfile/AppArmorProfile
	// from the chart's container-level securityContext into the pod-level one when not already set
	// there. The operator's CRD only exposes these fields via spec.securityContext.podSecurityContext
	// and cascades them down to containers at build time (see build.containerSecurityContext), so
	// without this promotion values set under a chart's container-level securityContext block would be
	// silently dropped.
	// NOTE: because the CRD lacks container-level fields for these, pod-level values take precedence
	// over container-level values here. This inverts native Kubernetes semantics where
	// container.securityContext overrides podSecurityContext.
	func mergePodSecurityContext(pod *corev1.PodSecurityContext, container *corev1.SecurityContext) *corev1.PodSecurityContext {