Allow more complex maps in launcher's securityContext than just key-value pairs

[jacpete]

Description

This is a followup to #753 (comment).

Other job.tpl files may need a similar fix. This is for Workbench which has the same pattern:

helm/charts/rstudio-workbench/files/job.tpl

Lines 133 to 146 in c6c1b08

	{{- if .Job.container.supplementalGroupIds }}
	{{- $groupIds := list }}
	{{- range .Job.container.supplementalGroupIds }}
	{{- $groupIds = append $groupIds . }}
	{{- end }}
	{{- $_ := set $securityContext "supplementalGroups" (cat "[" ($groupIds | join ", ") "]") }}
	{{- $securityContext := mergeOverwrite $securityContext $templateData.pod.securityContext }}
	{{- end }}
	{{- if $securityContext }}
	securityContext:
	{{- range $key, $val := $securityContext }}
	{{ $key }}: {{ $val }}
	{{- end }}
	{{- end }}

Through issue #753 and PR #755, we were able to identify an issue where our launcher jobs either ignore more complex mappings like adding an appArmorProfile or seccompProfile to launcher.templateValues.pod.securityContext or result in an error in executing the template when the launcher runs.

While this hasn't come up as an issue with a customer for Workbench yet, since this section of the job.tpl in the Workbench chart matched the bad code in Connect, I wanted to get it flagged for review.

There are two potential issues with this section of code:

1. The current method for templating the securityContext section does not allow for more complex mapping structures in the values.yaml than key-value pairs currently. Our code is currently:
   

   helm/charts/rstudio-workbench/files/job.tpl

   Lines 141 to 146 in c6c1b08

   	{{- if $securityContext }}
   	securityContext:
   	{{- range $key, $val := $securityContext }}
   	{{ $key }}: {{ $val }}
   	{{- end }}
   	{{- end }}

   
   but I would expect this to fail similarly to what we saw in Connect in a situation where the you have more than key-value pairs in your values.yaml:

securityContext:
  appArmorProfile:
    type: RuntimeDefault
  seccompProfile:
    type: RuntimeDefault
  capabilities:
    drop:
      - ALL
  allowPrivilegeEscalation: false
  privileged: false

Example error from Connect issue: "json: cannot unmarshal string into Go struct field PodSecurityContext.spec.template.spec.securityContext.appArmorProfile of type v1.AppArmorProfile"

2. The second issue is that the mergeOverwrite only happens if .Job.container.supplementalGroupIds exists.
   

   helm/charts/rstudio-workbench/files/job.tpl

   Lines 133 to 140 in c6c1b08

   	{{- if .Job.container.supplementalGroupIds }}
   	{{- $groupIds := list }}
   	{{- range .Job.container.supplementalGroupIds }}
   	{{- $groupIds = append $groupIds . }}
   	{{- end }}
   	{{- $_ := set $securityContext "supplementalGroups" (cat "[" ($groupIds | join ", ") "]") }}
   	{{- $securityContext := mergeOverwrite $securityContext $templateData.pod.securityContext }}
   	{{- end }}

   
   Essentially, the $templateData.pod.securityContext is getting ignored if {{- if .Job.container.supplementalGroupIds }} is false as it is in this example. This bug makes everything run without error, but the pod level securityContext wasn't being correctly applied.

Potential fix

I believe the following replacement for that section of code could fix both these issues. First, by changing the range function to uses toYaml instead when expanding the $securityContext mapping. And second, moving the mergeOverwrite line out of the .Job.container.supplementalGroupIds if statement and into the $securityContext if statement instead.
Original:

helm/charts/rstudio-workbench/files/job.tpl

Lines 133 to 146 in c6c1b08

	{{- if .Job.container.supplementalGroupIds }}
	{{- $groupIds := list }}
	{{- range .Job.container.supplementalGroupIds }}
	{{- $groupIds = append $groupIds . }}
	{{- end }}
	{{- $_ := set $securityContext "supplementalGroups" (cat "[" ($groupIds | join ", ") "]") }}
	{{- $securityContext := mergeOverwrite $securityContext $templateData.pod.securityContext }}
	{{- end }}
	{{- if $securityContext }}
	securityContext:
	{{- range $key, $val := $securityContext }}
	{{ $key }}: {{ $val }}
	{{- end }}
	{{- end }}

Replacement:

      {{- if .Job.container.supplementalGroupIds }}
        {{- $groupIds := list }}
        {{- range .Job.container.supplementalGroupIds }}
          {{- $groupIds = append $groupIds . }}
        {{- end }}
        {{- $_ := set $securityContext "supplementalGroups" (cat "[" ($groupIds | join ", ") "]") }}
      {{- end }}
      {{- if $securityContext }}
      {{- $securityContext := mergeOverwrite $securityContext $templateData.pod.securityContext }}
      securityContext:
        {{- toYaml $securityContext | nindent 8 }}
      {{- end }}

My notes for testing this on Connect can be found at #755 (comment) if it is helpful. I'm just not sure if I'll get a chance to verify this on Workbench in a timely manner, so I wanted to make sure this was flagged for review.