From 774c20626ae0ebaf2947a19c1a2f4139716ae212 Mon Sep 17 00:00:00 2001 From: Kyle Husmann Date: Thu, 24 Sep 2026 17:46:13 -0700 Subject: [PATCH 1/3] Render order-sensitive ini config files from an ordered list --- charts/rstudio-library/Chart.yaml | 2 +- charts/rstudio-library/NEWS.md | 14 ++ charts/rstudio-library/README.md | 2 +- charts/rstudio-library/templates/_config.tpl | 90 ++++++++- .../rstudio-library/templates/_profiles.tpl | 82 +++++--- other-charts/rstudio-library-test/Chart.lock | 6 +- other-charts/rstudio-library-test/Chart.yaml | 2 +- other-charts/rstudio-library-test/NEWS.md | 4 + other-charts/rstudio-library-test/README.md | 2 +- .../templates/test-profiles.yaml | 2 +- .../tests/config_test.yaml | 180 ++++++++++++++++++ .../tests/profiles_test.yaml | 145 ++++++++++++++ 12 files changed, 482 insertions(+), 49 deletions(-) diff --git a/charts/rstudio-library/Chart.yaml b/charts/rstudio-library/Chart.yaml index 1a5e2bc4d..25a2f2ee4 100644 --- a/charts/rstudio-library/Chart.yaml +++ b/charts/rstudio-library/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: rstudio-library description: Helm library helpers for use by official RStudio charts type: library -version: 0.1.37 +version: 0.1.38 appVersion: 0.1.35 icon: https://raw.githubusercontent.com/rstudio/helm/main/images/posit-icon-fullcolor.svg diff --git a/charts/rstudio-library/NEWS.md b/charts/rstudio-library/NEWS.md index f019f0698..ef3335e98 100644 --- a/charts/rstudio-library/NEWS.md +++ b/charts/rstudio-library/NEWS.md @@ -1,5 +1,19 @@ # Changelog +## 0.1.38 + +- `rstudio-library.config.ini` now accepts a file's contents as a list of single-entry maps, and + renders them in the order written rather than sorted by name. A single-entry item becomes a + `[name]` section when its value is a map, and a `name=value` line when its value is a scalar. + A list item with more than one key still renders as a blank-line separated record, which is what + `/etc/rstudio/r-versions` expects. Previously a list of single-entry maps rendered broken lines + such as `*=map[max-memory-mb:1024]`, with no section headers. +- `rstudio-library.profiles.ini.advanced`, `rstudio-library.profiles.ini.singleFile`, and + `rstudio-library.profiles.json-from-overrides-config` accept the same ordered list form for a + profiles file's sections, including its `job-json-overrides` handling. +- New `rstudio-library.config.entries` helper, which normalizes either form into an ordered list of + entries. + ## 0.1.37 - **DEPRECATED**: Chronicle agent helpers are deprecated. diff --git a/charts/rstudio-library/README.md b/charts/rstudio-library/README.md index 849c8ade9..4e9d11bd0 100644 --- a/charts/rstudio-library/README.md +++ b/charts/rstudio-library/README.md @@ -1,6 +1,6 @@ # rstudio-library -![Version: 0.1.37](https://img.shields.io/badge/Version-0.1.37-informational?style=flat-square) ![Type: library](https://img.shields.io/badge/Type-library-informational?style=flat-square) ![AppVersion: 0.1.35](https://img.shields.io/badge/AppVersion-0.1.35-informational?style=flat-square) +![Version: 0.1.38](https://img.shields.io/badge/Version-0.1.38-informational?style=flat-square) ![Type: library](https://img.shields.io/badge/Type-library-informational?style=flat-square) ![AppVersion: 0.1.35](https://img.shields.io/badge/AppVersion-0.1.35-informational?style=flat-square) #### _Helm library helpers for use by official RStudio charts_ diff --git a/charts/rstudio-library/templates/_config.tpl b/charts/rstudio-library/templates/_config.tpl index 4ace9d3e9..2b4f740fd 100644 --- a/charts/rstudio-library/templates/_config.tpl +++ b/charts/rstudio-library/templates/_config.tpl @@ -43,20 +43,51 @@ {{- end }} {{- end }} -{{- define "rstudio-library.config.ini" -}} -{{- range $file, $keys := . -}} -{{- printf "%s: |" $file | nindent 0 }} -{{- if kindIs "string" $keys }} - {{- $keys | nindent 2 }} +{{- /* + Normalizes config-file contents into an ordered list of entries. + + Accepts either form: + - a map of {name: value}, which Go templates iterate in sorted key order + - a list of single-entry maps, which is iterated in the order it was written + + Templates cannot return values, so the caller passes a dict to write into: + data: the contents to normalize + result: a dict, which this sets an "entries" key on. Each entry is a dict + with a "name" (string) and a "config" (the value written under it) +*/ -}} +{{- define "rstudio-library.config.entries" -}} +{{- $entries := list }} +{{- $data := default (dict) .data }} +{{- if kindIs "slice" $data }} + {{- range $item := $data }} + {{- if not (kindIs "map" $item) }} + {{- fail (print "\n\nEntries written as a list must each be a map of a single name and its value. Instead got '" (kindOf $item) "' : '" (print $item) "'") }} + {{- end }} + {{- range $name, $config := $item }} + {{- $entries = append $entries (dict "name" (toString $name) "config" $config) }} + {{- end }} + {{- end }} {{- else }} -{{- range $parent, $child := $keys -}} - {{/* ini files may have multiple sections with the same name */}} + {{- range $name, $config := $data }} + {{- $entries = append $entries (dict "name" (toString $name) "config" $config) }} + {{- end }} +{{- end }} +{{- $_ := set .result "entries" $entries }} +{{- end -}} + +{{- /* + Renders a single ini entry, passed as a dict of {name: value}: + - a map value becomes a [name] section followed by its key=value pairs + - a list of maps becomes repeated [name] sections (ini files may have more + than one section with the same name) + - anything else becomes a bare name=value line +*/ -}} +{{- define "rstudio-library.config.ini.entry" -}} +{{- range $parent, $child := . -}} {{- $sections := ( (kindIs "slice" $child) | ternary $child ( list $child ))}} {{- range $i, $section := $sections -}} {{- if kindIs "map" $section }} - {{- if not ( kindIs "slice" $keys ) -}} - {{- printf "[%s]" (toString $parent) | nindent 2 }} - {{- end }} + {{- printf "[%s]" (toString $parent) | nindent 2 }} {{- range $key, $val := $section }} {{- printf "%s=%s" (toString $key) (toString $val) | nindent 2 }} {{- end }} @@ -67,6 +98,45 @@ {{- end }} {{- end }} {{- end }} + +{{- /* + Takes a map of {filename: contents} and renders each as an ini file. + + Contents may be: + - a raw string, rendered verbatim + - a map of {name: value}, rendered in sorted key order. Files whose behavior + depends on the order of their sections or entries should use the list form + - a list, rendered in the order it was written. Each item is a map, and is + rendered as either: + - one ordered entry, when the item has a single key: a [name] section if + its value is a map, otherwise a name=value line + - one record of fields followed by a blank line, when the item has more + than one key (the shape /etc/rstudio/r-versions expects) +*/ -}} +{{- define "rstudio-library.config.ini" -}} +{{- range $file, $keys := . -}} +{{- printf "%s: |" $file | nindent 0 }} +{{- if kindIs "string" $keys }} + {{- $keys | nindent 2 }} +{{- else if kindIs "slice" $keys }} +{{- range $item := $keys -}} + {{- if not (kindIs "map" $item) }} + {{- fail (print "\n\nEntries of '" $file "' written as a list must each be a map. Instead got '" (kindOf $item) "' : '" (print $item) "'") }} + {{- end }} + {{- if eq (len (keys $item)) 1 }} + {{- include "rstudio-library.config.ini.entry" $item }} + {{- else }} + {{- range $key, $val := $item }} + {{- printf "%s=%s" (toString $key) (toString $val) | nindent 2 }} + {{- end }} + {{- printf "" | nindent 0 }} + {{- end }} +{{- end }} +{{- else }} +{{- range $parent, $child := $keys -}} + {{- include "rstudio-library.config.ini.entry" (dict (toString $parent) $child) }} +{{- end }} +{{- end }} {{- end }} {{- end }} diff --git a/charts/rstudio-library/templates/_profiles.tpl b/charts/rstudio-library/templates/_profiles.tpl index c9da09936..a2a5187be 100644 --- a/charts/rstudio-library/templates/_profiles.tpl +++ b/charts/rstudio-library/templates/_profiles.tpl @@ -3,7 +3,8 @@ Looks at the "json" key of the job-json-overrides definition Takes a dict: - data: the launcher.kubernetes.profiles.conf configuration as a dict (map of maps) + data: the launcher.kubernetes.profiles.conf configuration, as a map of sections or as an + ordered list of single-entry maps default: optional. The default job-json-overrides to append - Build a unique list of overrides (and a unique list of names for testing uniqueness) @@ -30,8 +31,11 @@ {{- if $data }} {{- $data = $data | deepCopy }} {{- end }} - {{- include "rstudio-library.debug.type-check" (dict "name" "config data" "object" $data "expected" "map" "description" "of section headers and configuration" ) }} - {{- range $key, $config := $data -}} + {{- $normalized := dict }} + {{- include "rstudio-library.config.entries" (dict "data" $data "result" $normalized) }} + {{- range $entry := $normalized.entries -}} + {{- $key := $entry.name -}} + {{- $config := $entry.config -}} {{- include "rstudio-library.debug.type-check" (dict "name" (print "[" $key "] section") "object" $config "expected" "map" "description" "of config data" ) }} {{- if hasKey $config "job-json-overrides" -}} {{- $overrides := get $config "job-json-overrides" -}} @@ -93,19 +97,24 @@ - output the ini file Takes a dict: - data: the launcher.kubernetes.profiles.conf configuration as a dict (map of maps) + data: the launcher.kubernetes.profiles.conf configuration, as a map of sections or as an + ordered list of single-entry maps. The list form renders in the order written default: optional. the default job-json-overrides to append filePath: optional. the default is none */}} {{- define "rstudio-library.profiles.apply-everyone-and-default-to-others" }} - {{- $newDict := dict }} - {{- $everyoneConfig := dict }} {{- $data := .data }} {{- if $data }} {{- $data = $data | deepCopy }} {{- end }} - {{- if hasKey $data "*" }} - {{- $everyoneConfig = get $data "*" }} + {{- $normalized := dict }} + {{- include "rstudio-library.config.entries" (dict "data" $data "result" $normalized) }} + {{- $entries := $normalized.entries }} + {{- $everyoneConfig := dict }} + {{- range $entry := $entries }} + {{- if eq $entry.name "*" }} + {{- $everyoneConfig = $entry.config }} + {{- end }} {{- end }} {{- include "rstudio-library.debug.type-check" (dict "name" "[*] section" "object" $everyoneConfig "expected" "map" "description" "of config values") }} {{- $defaultConfig := default (list) .default }} @@ -127,43 +136,51 @@ {{- end }} {{- $defaultConfig = concat $defaultConfig $everyone }} {{- end }} - {{- /* if default config is defined, ensure that "everyone" is updated by it */ -}} - {{- if ge (len $defaultConfig) 1 }} - {{- $newDict = mergeOverwrite $newDict (dict "*" (dict "job-json-overrides" $defaultConfig)) }} - {{- end }} - {{- /* loop over non-everyone config, prepending the default configuration */ -}} - {{- $others := omit $data "*" }} - {{- range $key, $one := $others }} - {{- include "rstudio-library.debug.type-check" (dict "name" (print "[" $key "] section" ) "object" $one "expected" "map" "description" "of config values") }} - {{- if hasKey $one "job-json-overrides" }} - {{- $oneConfig := get $one "job-json-overrides" }} - {{- include "rstudio-library.debug.type-check" (dict "name" ( print "[" $key "].job-json-overrides" ) "object" $oneConfig "expected" "slice" "description" "of job-json-overrides definitions") }} - {{- range $entry := $oneConfig }} - {{- $_ := set $entry "file" ( print $filePath ($entry.name | nospace) ".json" ) }} + {{- /* walk the sections in order, prepending the default configuration to each */ -}} + {{- $output := list }} + {{- $hasEveryone := false }} + {{- range $entry := $entries }} + {{- $name := $entry.name }} + {{- $config := $entry.config }} + {{- include "rstudio-library.debug.type-check" (dict "name" (print "[" $name "] section" ) "object" $config "expected" "map" "description" "of config values") }} + {{- if eq $name "*" }} + {{- $hasEveryone = true }} + {{- if ge (len $defaultConfig) 1 }} + {{- $config = mergeOverwrite $config (dict "job-json-overrides" $defaultConfig) }} {{- end }} - {{- $oneList := concat $defaultConfig $oneConfig }} - {{- $oneDict := dict $key (dict "job-json-overrides" $oneList) }} - {{- $newDict = mergeOverwrite $newDict $oneDict }} + {{- else if hasKey $config "job-json-overrides" }} + {{- $oneConfig := get $config "job-json-overrides" }} + {{- include "rstudio-library.debug.type-check" (dict "name" ( print "[" $name "].job-json-overrides" ) "object" $oneConfig "expected" "slice" "description" "of job-json-overrides definitions") }} + {{- range $one := $oneConfig }} + {{- $_ := set $one "file" ( print $filePath ($one.name | nospace) ".json" ) }} + {{- end }} + {{- $config = mergeOverwrite $config (dict "job-json-overrides" (concat $defaultConfig $oneConfig)) }} {{- end }} + {{- $output = append $output (dict $name $config) }} + {{- end }} + {{- /* the defaults need an "everyone" section to live in, even if none was written */ -}} + {{- if and (not $hasEveryone) (ge (len $defaultConfig) 1) }} + {{- $output = prepend $output (dict "*" (dict "job-json-overrides" $defaultConfig)) }} {{- end }} {{- /* output the configuration file */ -}} - {{- $output := mergeOverwrite $data $newDict }} {{- include "rstudio-library.profiles.ini.singleFile" $output }} {{- end }} {{/* - Builds a single ini file + Builds a single ini file, from either a map of sections or an ordered list of single-entry maps Modified from rstudio-library.config.ini to: - collapse arrays - via rstudio-library.profiles.ini.collapse-array */}} {{- define "rstudio-library.profiles.ini.singleFile" -}} -{{- range $parent, $child := . -}} +{{- $normalized := dict }} +{{- include "rstudio-library.config.entries" (dict "data" . "result" $normalized) }} +{{- range $entry := $normalized.entries -}} + {{- $parent := $entry.name -}} + {{- $child := $entry.config -}} {{- if kindIs "map" $child }} - {{ if not ( kindIs "slice" . ) -}} [{{ $parent }}] - {{- end }} {{- range $key, $val := $child }} {{- if kindIs "slice" $val }} {{ $key }}={{ include "rstudio-library.profiles.ini.collapse-array" $val }} @@ -192,7 +209,8 @@ {{/* Takes a dict: - - .data : the configuration map of maps + - .data : a map of {filename: contents}. Each file's contents is either a map of sections or + an ordered list of single-entry maps, which renders in the order written - .jobJsonDefaults : an array of {target:target, name:name, json:json} defaults - .filePath : the path from the root of the system to where json overrides files will be mounted */}} @@ -203,7 +221,9 @@ {{- $data := .data }} {{- include "rstudio-library.debug.type-check" (dict "name" "profiles data" "object" $data "expected" "map" "description" "of filenames and config data") }} {{- range $file, $keys := $data -}} -{{- include "rstudio-library.debug.type-check" (dict "name" (print "profiles content for file '" $file "'") "object" $keys "expected" "map" "description" "of section headers and configuration") }} +{{- if not (or (kindIs "map" $keys) (kindIs "slice" $keys)) }} + {{- fail (print "\n\nprofiles content for file '" $file "' must be a 'map' of section headers and configuration, or a 'slice' of single-entry maps. Instead got '" (kindOf $keys) "' : '" (print $keys) "'") }} +{{- end }} {{ $file }}: | {{- include "rstudio-library.profiles.apply-everyone-and-default-to-others" (dict "data" $keys "default" $jobJsonDefaults "filePath" $filePath) }} diff --git a/other-charts/rstudio-library-test/Chart.lock b/other-charts/rstudio-library-test/Chart.lock index c6ecc39de..6d65a8f42 100644 --- a/other-charts/rstudio-library-test/Chart.lock +++ b/other-charts/rstudio-library-test/Chart.lock @@ -1,6 +1,6 @@ dependencies: - name: rstudio-library repository: file://../../charts/rstudio-library - version: 0.1.37 -digest: sha256:d2c41673ebb1b0dfb2c5548e34d62727906636a7ae40a18fbf35cc5fedb53351 -generated: "2026-06-18T14:27:05.729695-04:00" + version: 0.1.38 +digest: sha256:c4509e159bcf8fdb934a720ebc558bafad7db2687414568b81c76183563b6e9d +generated: "2026-09-24T17:46:06.957171-07:00" diff --git a/other-charts/rstudio-library-test/Chart.yaml b/other-charts/rstudio-library-test/Chart.yaml index 99b789a5d..8e0e0fa94 100644 --- a/other-charts/rstudio-library-test/Chart.yaml +++ b/other-charts/rstudio-library-test/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: rstudio-library-test description: Test harness for rstudio-library templates type: application -version: 0.1.1 +version: 0.1.2 appVersion: "0.1.0" maintainers: diff --git a/other-charts/rstudio-library-test/NEWS.md b/other-charts/rstudio-library-test/NEWS.md index fc2579260..5dbbd2651 100644 --- a/other-charts/rstudio-library-test/NEWS.md +++ b/other-charts/rstudio-library-test/NEWS.md @@ -1,5 +1,9 @@ # NEWS +## 0.1.2 + +- Exercise the ordered list form of `rstudio-library.config.ini` and the profiles helpers + ## 0.1.1 - Update rstudio-library dependency to 0.1.37 diff --git a/other-charts/rstudio-library-test/README.md b/other-charts/rstudio-library-test/README.md index 9e86de12b..5fcf5282d 100644 --- a/other-charts/rstudio-library-test/README.md +++ b/other-charts/rstudio-library-test/README.md @@ -2,7 +2,7 @@ # rstudio-library-test -![Version: 0.1.1](https://img.shields.io/badge/Version-0.1.1-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 0.1.0](https://img.shields.io/badge/AppVersion-0.1.0-informational?style=flat-square) +![Version: 0.1.2](https://img.shields.io/badge/Version-0.1.2-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 0.1.0](https://img.shields.io/badge/AppVersion-0.1.0-informational?style=flat-square) Test harness for rstudio-library templates diff --git a/other-charts/rstudio-library-test/templates/test-profiles.yaml b/other-charts/rstudio-library-test/templates/test-profiles.yaml index 3356482bc..f0e370312 100644 --- a/other-charts/rstudio-library-test/templates/test-profiles.yaml +++ b/other-charts/rstudio-library-test/templates/test-profiles.yaml @@ -19,7 +19,7 @@ data: {{- end }} {{- /* Test profiles.ini.singleFile */}} -{{- if and .Values.testProfiles.singleFile (kindIs "map" .Values.testProfiles.singleFile) }} +{{- if and .Values.testProfiles.singleFile (or (kindIs "map" .Values.testProfiles.singleFile) (kindIs "slice" .Values.testProfiles.singleFile)) }} --- apiVersion: v1 kind: ConfigMap diff --git a/other-charts/rstudio-library-test/tests/config_test.yaml b/other-charts/rstudio-library-test/tests/config_test.yaml index ecd27bbf4..28c73587d 100644 --- a/other-charts/rstudio-library-test/tests/config_test.yaml +++ b/other-charts/rstudio-library-test/tests/config_test.yaml @@ -295,3 +295,183 @@ tests: - matchRegex: path: data["empty.ini"] pattern: "\\[empty_section\\]" + + # ======================================== + # INI Ordered List Form Tests + # ======================================== + - it: should render ini sections from a list in the order written + set: + testConfig: + ini: + filename: profiles + config: + - jsmith: + max-memory-mb: 8192 + - "12345": + max-memory-mb: 8192 + - "@contractors": + max-memory-mb: 2048 + - "@analysts": + max-memory-mb: 4096 + - "*": + max-memory-mb: 1024 + asserts: + - equal: + path: data["profiles"] + value: | + [jsmith] + max-memory-mb=8192 + + [12345] + max-memory-mb=8192 + + [@contractors] + max-memory-mb=2048 + + [@analysts] + max-memory-mb=4096 + + [*] + max-memory-mb=1024 + + - it: should render sectionless ini entries from a list in the order written + set: + testConfig: + ini: + filename: repos.conf + config: + - Internal: https://pkgs.example.com/internal + - CRAN: https://packagemanager.posit.co/cran/latest + asserts: + - equal: + path: data["repos.conf"] + value: | + Internal=https://pkgs.example.com/internal + CRAN=https://packagemanager.posit.co/cran/latest + + - it: should render resource profiles from a list in the order written + set: + testConfig: + ini: + filename: launcher.kubernetes.resources.conf + config: + - small: + name: Small + cpus: 1 + - large: + name: Large + cpus: 8 + asserts: + - equal: + path: data["launcher.kubernetes.resources.conf"] + value: | + [small] + cpus=1 + name=Small + + [large] + cpus=8 + name=Large + + - it: should render multi-field list items as blank-line separated records + set: + testConfig: + ini: + filename: r-versions + config: + - Path: /opt/R/4.4.1 + Label: Latest + - Path: /opt/R/4.0.2 + Label: Old + asserts: + - equal: + path: data["r-versions"] + value: | + Label=Latest + Path=/opt/R/4.4.1 + + Label=Old + Path=/opt/R/4.0.2 + + - it: should sort ini sections by name when written as a map + set: + testConfig: + ini: + filename: profiles + config: + # the test chart's values.yaml default, cleared so the map is exactly what is written + section1: null + jsmith: + max-memory-mb: 8192 + "@analysts": + max-memory-mb: 4096 + "*": + max-memory-mb: 1024 + asserts: + - equal: + path: data["profiles"] + value: | + [*] + max-memory-mb=1024 + + [@analysts] + max-memory-mb=4096 + + [jsmith] + max-memory-mb=8192 + + - it: should render a raw string ini file verbatim + set: + testConfig: + ini: + filename: profiles + config: | + [jsmith] + max-memory-mb=8192 + + [*] + max-memory-mb=1024 + asserts: + - equal: + path: data["profiles"] + value: | + [jsmith] + max-memory-mb=8192 + + [*] + max-memory-mb=1024 + + - it: should still repeat a section name given a list of maps under one key + set: + testConfig: + ini: + filename: launcher.conf + config: + section1: null + cluster: + - name: Cluster1 + type: Kubernetes + - name: Cluster2 + type: Kubernetes + asserts: + - equal: + path: data["launcher.conf"] + value: | + [cluster] + name=Cluster1 + type=Kubernetes + + [cluster] + name=Cluster2 + type=Kubernetes + + - it: should fail when a list entry is not a map + set: + testConfig: + ini: + filename: repos.conf + config: + - https://packagemanager.posit.co/cran/latest + asserts: + - failedTemplate: + errorPattern: "Entries of 'repos.conf' written as a list must each be a map" diff --git a/other-charts/rstudio-library-test/tests/profiles_test.yaml b/other-charts/rstudio-library-test/tests/profiles_test.yaml index 1149624d5..49738af23 100644 --- a/other-charts/rstudio-library-test/tests/profiles_test.yaml +++ b/other-charts/rstudio-library-test/tests/profiles_test.yaml @@ -119,6 +119,151 @@ tests: path: data["launcher.kubernetes.profiles.conf"] pattern: "default-cpus=2" + - it: should render advanced profiles sections from a list in the order written + set: + testProfiles: + advanced: + data: + launcher.kubernetes.profiles.conf: + - jsmith: + default-cpus: 8 + - "@contractors": + default-cpus: 2 + - "@analysts": + default-cpus: 4 + - "*": + default-cpus: 1 + jobJsonDefaults: [] + filePath: /etc/rstudio/ + asserts: + - equal: + path: data["launcher.kubernetes.profiles.conf"] + value: | + [jsmith] + default-cpus=8 + + [@contractors] + default-cpus=2 + + [@analysts] + default-cpus=4 + + [*] + default-cpus=1 + + - it: should sort advanced profiles sections by name when written as a map + set: + testProfiles: + advanced: + data: + launcher.kubernetes.profiles.conf: + # the test chart's values.yaml default, cleared so the map is exactly what is written + testuser: null + jsmith: + default-cpus: 8 + "@analysts": + default-cpus: 4 + "*": + default-cpus: 1 + jobJsonDefaults: [] + filePath: /etc/rstudio/ + asserts: + - equal: + path: data["launcher.kubernetes.profiles.conf"] + value: | + [*] + default-cpus=1 + + [@analysts] + default-cpus=4 + + [jsmith] + default-cpus=8 + + - it: should prepend jobJsonDefaults to each ordered section that sets its own overrides + set: + testProfiles: + advanced: + data: + launcher.kubernetes.profiles.conf: + - "@contractors": + job-json-overrides: + - name: contractorNode + target: /spec/template/spec/nodeSelector + json: + tier: contractor + - "*": + default-cpus: 1 + jobJsonDefaults: + - name: sharedVolume + target: /spec/template/spec/volumes/- + json: + name: shared + filePath: /etc/rstudio/ + asserts: + - equal: + path: data["launcher.kubernetes.profiles.conf"] + value: | + [@contractors] + job-json-overrides="/spec/template/spec/volumes/-":"/etc/rstudio/sharedVolume.json","/spec/template/spec/nodeSelector":"/etc/rstudio/contractorNode.json" + + [*] + default-cpus=1 + job-json-overrides="/spec/template/spec/volumes/-":"/etc/rstudio/sharedVolume.json" + + - it: should add an everyone section for jobJsonDefaults when the ordered list has none + set: + testProfiles: + advanced: + data: + launcher.kubernetes.profiles.conf: + - jsmith: + default-cpus: 8 + jobJsonDefaults: + - name: sharedVolume + target: /spec/template/spec/volumes/- + json: + name: shared + filePath: /etc/rstudio/ + asserts: + - equal: + path: data["launcher.kubernetes.profiles.conf"] + value: | + [*] + job-json-overrides="/spec/template/spec/volumes/-":"/etc/rstudio/sharedVolume.json" + + [jsmith] + default-cpus=8 + + - it: should fail when advanced profiles content is neither a map nor a list + set: + testProfiles: + advanced: + data: + launcher.kubernetes.profiles.conf: "not-a-map" + jobJsonDefaults: [] + asserts: + - failedTemplate: + errorPattern: "must be a 'map' of section headers and configuration, or a 'slice' of single-entry maps" + + - it: should render single file sections from a list in the order written + set: + testProfiles: + singleFile: + - user1: + setting: value1 + - "*": + setting: value2 + asserts: + - equal: + path: data["profiles.conf"] + value: | + [user1] + setting=value1 + + [*] + setting=value2 + # ======================================== # profiles.json-from-overrides-config Tests # ======================================== From 0094ed283415fec9f5045e9ad35f3e5c28824b4b Mon Sep 17 00:00:00 2001 From: Kyle Husmann Date: Thu, 24 Sep 2026 17:56:29 -0700 Subject: [PATCH 2/3] Treat a list item naming sections as sections rather than a record --- charts/rstudio-library/NEWS.md | 5 +- charts/rstudio-library/templates/_config.tpl | 27 +++++++--- .../tests/config_test.yaml | 50 +++++++++++++++++++ 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/charts/rstudio-library/NEWS.md b/charts/rstudio-library/NEWS.md index ef3335e98..16b6034b7 100644 --- a/charts/rstudio-library/NEWS.md +++ b/charts/rstudio-library/NEWS.md @@ -6,8 +6,9 @@ renders them in the order written rather than sorted by name. A single-entry item becomes a `[name]` section when its value is a map, and a `name=value` line when its value is a scalar. A list item with more than one key still renders as a blank-line separated record, which is what - `/etc/rstudio/r-versions` expects. Previously a list of single-entry maps rendered broken lines - such as `*=map[max-memory-mb:1024]`, with no section headers. + `/etc/rstudio/r-versions` expects, unless one of its keys names a section - most often a list item + that is missing its own `- `. Previously a list of single-entry maps rendered broken lines such as + `*=map[max-memory-mb:1024]`, with no section headers. - `rstudio-library.profiles.ini.advanced`, `rstudio-library.profiles.ini.singleFile`, and `rstudio-library.profiles.json-from-overrides-config` accept the same ordered list form for a profiles file's sections, including its `job-json-overrides` handling. diff --git a/charts/rstudio-library/templates/_config.tpl b/charts/rstudio-library/templates/_config.tpl index 2b4f740fd..ba29ddbbb 100644 --- a/charts/rstudio-library/templates/_config.tpl +++ b/charts/rstudio-library/templates/_config.tpl @@ -108,10 +108,12 @@ depends on the order of their sections or entries should use the list form - a list, rendered in the order it was written. Each item is a map, and is rendered as either: - - one ordered entry, when the item has a single key: a [name] section if - its value is a map, otherwise a name=value line - - one record of fields followed by a blank line, when the item has more - than one key (the shape /etc/rstudio/r-versions expects) + - one record of fields followed by a blank line, when the item holds more + than one key and none of them name a section (the shape + /etc/rstudio/r-versions expects) + - otherwise, one ordered entry per key: a [name] section if its value is a + map, and a name=value line if not. Keys within an item are still sorted, + so write one key per item to control the order */ -}} {{- define "rstudio-library.config.ini" -}} {{- range $file, $keys := . -}} @@ -123,13 +125,24 @@ {{- if not (kindIs "map" $item) }} {{- fail (print "\n\nEntries of '" $file "' written as a list must each be a map. Instead got '" (kindOf $item) "' : '" (print $item) "'") }} {{- end }} - {{- if eq (len (keys $item)) 1 }} - {{- include "rstudio-library.config.ini.entry" $item }} - {{- else }} + {{- /* A map value names a section, so an item holding one is a group of sections rather than a + record of fields -- most often a list item that is missing its own "- ". Rendering it as + a record would silently emit lines like "name=map[key:value]". */ -}} + {{- $isRecord := gt (len (keys $item)) 1 }} + {{- range $key, $val := $item }} + {{- if kindIs "map" $val }} + {{- $isRecord = false }} + {{- end }} + {{- end }} + {{- if $isRecord }} {{- range $key, $val := $item }} {{- printf "%s=%s" (toString $key) (toString $val) | nindent 2 }} {{- end }} {{- printf "" | nindent 0 }} + {{- else }} + {{- range $key, $val := $item }} + {{- include "rstudio-library.config.ini.entry" (dict (toString $key) $val) }} + {{- end }} {{- end }} {{- end }} {{- else }} diff --git a/other-charts/rstudio-library-test/tests/config_test.yaml b/other-charts/rstudio-library-test/tests/config_test.yaml index 28c73587d..fbaa72065 100644 --- a/other-charts/rstudio-library-test/tests/config_test.yaml +++ b/other-charts/rstudio-library-test/tests/config_test.yaml @@ -475,3 +475,53 @@ tests: asserts: - failedTemplate: errorPattern: "Entries of 'repos.conf' written as a list must each be a map" + + - it: should render many options within an ordered section + set: + testConfig: + ini: + filename: profiles + config: + - "@analysts": + max-memory-mb: 4096 + max-cpus: 4 + session-limit: 10 + - "*": + max-memory-mb: 1024 + asserts: + - equal: + path: data["profiles"] + value: | + [@analysts] + max-cpus=4 + max-memory-mb=4096 + session-limit=10 + + [*] + max-memory-mb=1024 + + - it: should render a list item holding several sections as sections, not as a record + set: + testConfig: + ini: + filename: profiles + config: + # "@contractors" is missing its own "- ", so it lands in the same item + - "@analysts": + max-cpus: 4 + "@contractors": + max-cpus: 2 + - "*": + max-cpus: 1 + asserts: + - equal: + path: data["profiles"] + value: | + [@analysts] + max-cpus=4 + + [@contractors] + max-cpus=2 + + [*] + max-cpus=1 From fbef3baba0ee62492a127a552d6e5f2782c8ea74 Mon Sep 17 00:00:00 2001 From: Kyle Husmann Date: Thu, 24 Sep 2026 18:29:45 -0700 Subject: [PATCH 3/3] Reject list entries and section options that ini cannot represent --- charts/rstudio-library/NEWS.md | 16 ++- charts/rstudio-library/templates/_config.tpl | 88 +++++++++---- .../tests/config_test.yaml | 121 ++++++++++++++++-- .../tests/profiles_test.yaml | 33 +++++ 4 files changed, 217 insertions(+), 41 deletions(-) diff --git a/charts/rstudio-library/NEWS.md b/charts/rstudio-library/NEWS.md index 16b6034b7..416b95f3d 100644 --- a/charts/rstudio-library/NEWS.md +++ b/charts/rstudio-library/NEWS.md @@ -5,10 +5,18 @@ - `rstudio-library.config.ini` now accepts a file's contents as a list of single-entry maps, and renders them in the order written rather than sorted by name. A single-entry item becomes a `[name]` section when its value is a map, and a `name=value` line when its value is a scalar. - A list item with more than one key still renders as a blank-line separated record, which is what - `/etc/rstudio/r-versions` expects, unless one of its keys names a section - most often a list item - that is missing its own `- `. Previously a list of single-entry maps rendered broken lines such as - `*=map[max-memory-mb:1024]`, with no section headers. + A list entry with more than one key still renders as a blank-line separated record, which is what + `/etc/rstudio/r-versions` expects. Previously a list of single-entry maps rendered broken lines + such as `*=map[max-memory-mb:1024]`, with no section headers. +- A list entry that is empty, or that names a section alongside any other key, now fails with a + message naming the sections and showing the `- ` placement to fix it. The most common cause is a + section that is missing its own `- `, which would otherwise render as `name=map[key:value]`. +- An option inside a section must now be a single value. ini files have no nesting, so a map or a + list there had no representation and rendered as `key=map[a:1]` or `key=[a b]`. This applies to + both the map and the list form. Lists at the top level of a file are unaffected: a list of maps + still repeats a section, and a list of values still repeats a key. + `rstudio-library.profiles.ini` is also unaffected, since it defines a meaning for a list inside a + section (it comma-joins). - `rstudio-library.profiles.ini.advanced`, `rstudio-library.profiles.ini.singleFile`, and `rstudio-library.profiles.json-from-overrides-config` accept the same ordered list form for a profiles file's sections, including its `job-json-overrides` handling. diff --git a/charts/rstudio-library/templates/_config.tpl b/charts/rstudio-library/templates/_config.tpl index ba29ddbbb..06e244774 100644 --- a/charts/rstudio-library/templates/_config.tpl +++ b/charts/rstudio-library/templates/_config.tpl @@ -61,7 +61,15 @@ {{- if kindIs "slice" $data }} {{- range $item := $data }} {{- if not (kindIs "map" $item) }} - {{- fail (print "\n\nEntries written as a list must each be a map of a single name and its value. Instead got '" (kindOf $item) "' : '" (print $item) "'") }} + {{- fail (print "\n\nEvery entry written as a list must be a map of a single name and its value. Instead got '" (kindOf $item) "' : '" (print $item) "'") }} + {{- end }} + {{- $names := keys $item | sortAlpha }} + {{- if ne (len $names) 1 }} + {{- $hint := "" }} + {{- range $n := $names }} + {{- $hint = print $hint "\n - " ($n | quote) ":\n ..." }} + {{- end }} + {{- fail (print "\n\nAn entry written as a list holds " (len $names) " keys: " (join ", " $names) "\n\nEach entry names one section, so that the sections keep the order they were\nwritten in. Put '- ' in front of each one:\n" $hint "\n") }} {{- end }} {{- range $name, $config := $item }} {{- $entries = append $entries (dict "name" (toString $name) "config" $config) }} @@ -76,19 +84,34 @@ {{- end -}} {{- /* - Renders a single ini entry, passed as a dict of {name: value}: - - a map value becomes a [name] section followed by its key=value pairs - - a list of maps becomes repeated [name] sections (ini files may have more - than one section with the same name) - - anything else becomes a bare name=value line + Renders a single ini entry. + + Takes a dict: + file: the file name, used in error messages + entry: a map of {name: value}, where the value is either + - a map, which becomes a [name] section followed by its key=value pairs + - a list of maps, which becomes repeated [name] sections (ini files may + have more than one section with the same name) + - anything else, which becomes a bare name=value line + + An option inside a section must be a single value. ini has no nesting, so a map + or a list there has no representation and used to render as "key=map[a:1]" or + "key=[a b]". (rstudio-library.profiles.ini does define a meaning for a list -- + it comma-joins -- which is why it does not share this helper.) */ -}} {{- define "rstudio-library.config.ini.entry" -}} -{{- range $parent, $child := . -}} +{{- $file := .file }} +{{- range $parent, $child := .entry -}} {{- $sections := ( (kindIs "slice" $child) | ternary $child ( list $child ))}} {{- range $i, $section := $sections -}} {{- if kindIs "map" $section }} {{- printf "[%s]" (toString $parent) | nindent 2 }} {{- range $key, $val := $section }} + {{- if or (kindIs "map" $val) (kindIs "slice" $val) }} + {{- $kind := (kindIs "map" $val) | ternary "a map" "a list" }} + {{- $fix := (kindIs "map" $val) | ternary "" "\n\nIf the file expects several values, write them as one value, such as \"a,b\"." }} + {{- fail (print "\n\n'" (toString $key) "' in section [" (toString $parent) "] of '" $file "' is " $kind ", but an option\ninside a section must be a single value. ini files have no nesting, so this\nwould have rendered as '" (toString $key) "=" (toString $val) "'." $fix "\n") }} + {{- end }} {{- printf "%s=%s" (toString $key) (toString $val) | nindent 2 }} {{- end }} {{- printf "" | nindent 0 }} @@ -106,14 +129,13 @@ - a raw string, rendered verbatim - a map of {name: value}, rendered in sorted key order. Files whose behavior depends on the order of their sections or entries should use the list form - - a list, rendered in the order it was written. Each item is a map, and is - rendered as either: - - one record of fields followed by a blank line, when the item holds more - than one key and none of them name a section (the shape - /etc/rstudio/r-versions expects) - - otherwise, one ordered entry per key: a [name] section if its value is a - map, and a name=value line if not. Keys within an item are still sorted, - so write one key per item to control the order + - a list, rendered in the order it was written. Each entry is a non-empty map: + - a key whose value is a map names a section, and must be the only key in + its entry, so that sections keep the order they were written in + - otherwise the entry is a record of fields, rendered as name=value lines + followed by a blank line (the shape /etc/rstudio/r-versions expects) + Keys within one entry are sorted, so the order of a list is the order of its + sections and entries, not of the options inside them */ -}} {{- define "rstudio-library.config.ini" -}} {{- range $file, $keys := . -}} @@ -121,33 +143,45 @@ {{- if kindIs "string" $keys }} {{- $keys | nindent 2 }} {{- else if kindIs "slice" $keys }} -{{- range $item := $keys -}} +{{- range $i, $item := $keys -}} + {{- $where := print "entry " (add $i 1) " of '" $file "'" }} {{- if not (kindIs "map" $item) }} - {{- fail (print "\n\nEntries of '" $file "' written as a list must each be a map. Instead got '" (kindOf $item) "' : '" (print $item) "'") }} + {{- fail (print "\n\n" $where " must be a map of a name and its value. Instead got '" (kindOf $item) "' : '" (print $item) "'") }} {{- end }} - {{- /* A map value names a section, so an item holding one is a group of sections rather than a - record of fields -- most often a list item that is missing its own "- ". Rendering it as - a record would silently emit lines like "name=map[key:value]". */ -}} - {{- $isRecord := gt (len (keys $item)) 1 }} + {{- $names := keys $item | sortAlpha }} + {{- if eq (len $names) 0 }} + {{- fail (print "\n\n" $where " is empty. Every entry written as a list must be a map of a name and its value.") }} + {{- end }} + {{- /* A map value names a section, so it has to be the only key in its entry. Otherwise the + sections in one entry would be sorted against each other, losing the order the list is + there to preserve -- and before this check, they rendered as "name=map[key:value]". + Almost always a list entry that is missing its own "- ". */ -}} + {{- $sections := list }} {{- range $key, $val := $item }} {{- if kindIs "map" $val }} - {{- $isRecord = false }} + {{- $sections = append $sections (toString $key) }} + {{- end }} + {{- end }} + {{- if and $sections (gt (len $names) 1) }} + {{- $hint := "" }} + {{- range $s := $sections }} + {{- $hint = print $hint "\n - " ($s | quote) ":\n ..." }} {{- end }} + {{- fail (print "\n\n" $where " holds more than one key: " (join ", " $names) "\n\nA key whose value is a section must be the only key in its entry, so that the\nsections keep the order they were written in. These name sections: " (join ", " $sections) "\n\nPut '- ' in front of each one:\n\n " $file ":" $hint "\n") }} {{- end }} - {{- if $isRecord }} + {{- if gt (len $names) 1 }} + {{- /* a record of fields, the shape /etc/rstudio/r-versions expects */ -}} {{- range $key, $val := $item }} {{- printf "%s=%s" (toString $key) (toString $val) | nindent 2 }} {{- end }} {{- printf "" | nindent 0 }} {{- else }} - {{- range $key, $val := $item }} - {{- include "rstudio-library.config.ini.entry" (dict (toString $key) $val) }} - {{- end }} + {{- include "rstudio-library.config.ini.entry" (dict "file" $file "entry" $item) }} {{- end }} {{- end }} {{- else }} {{- range $parent, $child := $keys -}} - {{- include "rstudio-library.config.ini.entry" (dict (toString $parent) $child) }} + {{- include "rstudio-library.config.ini.entry" (dict "file" $file "entry" (dict (toString $parent) $child)) }} {{- end }} {{- end }} {{- end }} diff --git a/other-charts/rstudio-library-test/tests/config_test.yaml b/other-charts/rstudio-library-test/tests/config_test.yaml index fbaa72065..0a5367c08 100644 --- a/other-charts/rstudio-library-test/tests/config_test.yaml +++ b/other-charts/rstudio-library-test/tests/config_test.yaml @@ -474,7 +474,7 @@ tests: - https://packagemanager.posit.co/cran/latest asserts: - failedTemplate: - errorPattern: "Entries of 'repos.conf' written as a list must each be a map" + errorPattern: "entry 1 of 'repos.conf' must be a map of a name and its value" - it: should render many options within an ordered section set: @@ -500,28 +500,129 @@ tests: [*] max-memory-mb=1024 - - it: should render a list item holding several sections as sections, not as a record + - it: should fail when one list entry holds several sections set: testConfig: ini: filename: profiles config: - # "@contractors" is missing its own "- ", so it lands in the same item + # "@contractors" is missing its own "- ", so it lands in the same entry - "@analysts": max-cpus: 4 "@contractors": max-cpus: 2 - "*": max-cpus: 1 + asserts: + - failedTemplate: + errorPattern: "entry 1 of 'profiles' holds more than one key: @analysts, @contractors" + + - it: should fail when a list entry mixes a section and a bare value + set: + testConfig: + ini: + filename: profiles + config: + - "@analysts": + max-cpus: 4 + some-key: some-value + asserts: + - failedTemplate: + errorPattern: "These name sections: @analysts" + + - it: should fail when a list entry is empty + set: + testConfig: + ini: + filename: profiles + config: + - {} + asserts: + - failedTemplate: + errorPattern: "entry 1 of 'profiles' is empty" + + - it: should still render a multi-field record, which names no section + set: + testConfig: + ini: + filename: r-versions + config: + - Path: /opt/R/4.4.1 + Label: Latest + Module: r/4.4.1 asserts: - equal: - path: data["profiles"] + path: data["r-versions"] value: | - [@analysts] - max-cpus=4 + Label=Latest + Module=r/4.4.1 + Path=/opt/R/4.4.1 - [@contractors] - max-cpus=2 + - it: should fail when an option inside a section is a map + set: + testConfig: + ini: + filename: profiles + config: + section1: null + "*": + limits: + cpu: 1 + mem: 2 + asserts: + - failedTemplate: + errorPattern: "'limits' in section \\[\\*\\] of 'profiles' is a map" - [*] - max-cpus=1 + - it: should fail when an option inside a section is a list + set: + testConfig: + ini: + filename: profiles + config: + section1: null + "*": + container-images: + - imageA + - imageB + asserts: + - failedTemplate: + errorPattern: "'container-images' in section \\[\\*\\] of 'profiles' is a list" + - failedTemplate: + errorPattern: "write them as one value" + + - it: should still allow a list of maps at the top level of a file + set: + testConfig: + ini: + filename: launcher.conf + config: + section1: null + cluster: + - name: Cluster1 + - name: Cluster2 + asserts: + - equal: + path: data["launcher.conf"] + value: | + [cluster] + name=Cluster1 + + [cluster] + name=Cluster2 + + - it: should still allow a list of scalars at the top level of a file + set: + testConfig: + ini: + filename: some.conf + config: + section1: null + repeated-key: + - a + - b + asserts: + - equal: + path: data["some.conf"] + value: | + repeated-key=a + repeated-key=b diff --git a/other-charts/rstudio-library-test/tests/profiles_test.yaml b/other-charts/rstudio-library-test/tests/profiles_test.yaml index 49738af23..af68fbb02 100644 --- a/other-charts/rstudio-library-test/tests/profiles_test.yaml +++ b/other-charts/rstudio-library-test/tests/profiles_test.yaml @@ -380,3 +380,36 @@ tests: asserts: - hasDocuments: count: 0 + + - it: should fail when one profiles list entry holds several sections + set: + testProfiles: + advanced: + data: + launcher.kubernetes.profiles.conf: + - "@analysts": + default-cpus: 4 + "@contractors": + default-cpus: 2 + jobJsonDefaults: [] + asserts: + - failedTemplate: + errorPattern: "An entry written as a list holds 2 keys: @analysts, @contractors" + + - it: should still comma-join a list inside a profiles section + set: + testProfiles: + advanced: + data: + launcher.kubernetes.profiles.conf: + - "*": + some-key: + - value1 + - value2 + jobJsonDefaults: [] + asserts: + - equal: + path: data["launcher.kubernetes.profiles.conf"] + value: | + [*] + some-key=value1,value2