diff --git a/charts/rstudio-workbench/Chart.yaml b/charts/rstudio-workbench/Chart.yaml index e22a7ffa..3de079c5 100644 --- a/charts/rstudio-workbench/Chart.yaml +++ b/charts/rstudio-workbench/Chart.yaml @@ -1,6 +1,6 @@ name: rstudio-workbench description: Official Helm chart for Posit Workbench -version: 0.22.2 +version: 0.23.0 apiVersion: v2 appVersion: 2026.09.0 icon: https://raw.githubusercontent.com/rstudio/helm/main/images/posit-icon-fullcolor.svg @@ -13,7 +13,7 @@ maintainers: url: https://github.com/sol-eng dependencies: - name: rstudio-library - version: 0.1.35 + version: 0.1.38 repository: https://helm.rstudio.com annotations: artifacthub.io/images: | diff --git a/charts/rstudio-workbench/NEWS.md b/charts/rstudio-workbench/NEWS.md index c0249b87..da34224a 100644 --- a/charts/rstudio-workbench/NEWS.md +++ b/charts/rstudio-workbench/NEWS.md @@ -1,5 +1,117 @@ # Changelog +## 0.23.0 + +- Config files whose behavior depends on the order of their sections or entries can now be written + as a list, putting `- ` in front of each section or entry, and are rendered in the order written. + This covers `config.server.profiles`, `config.server.launcher\.*\.resources\.conf`, + `config.profiles.launcher\.*\.profiles\.conf` (and the deprecated `config.server` location of + those files), and `config.session.repos\.conf`: + + ```yaml + config: + server: + profiles: + - "*": + max-memory-mb: 1024 + - "@analysts": + max-memory-mb: 4096 + session: + repos.conf: + - Internal: https://pkgs.example.com/internal + - CRAN: https://packagemanager.posit.co/cran/latest + ``` + + Written as a map, these files are still rendered as before - sorted by name - and the chart now + prints a `WARNING` in `NOTES.txt` saying so, since sorting silently changes what they do. The map + form is not going away; it is simply the wrong form for these four files. The raw string form + (`profiles: |`) keeps the written order and is unaffected. +- A list of values for one option is now written the way the file's parser expects. Files read by + boost `program_options` - `rserver.conf`, `rsession.conf`, the `launcher..conf` plugin + files, `jupyter.conf`, `vscode.conf`, `positron.conf` - repeat the key, one line per value, as + the chart always did at the top level of a file: + + ```yaml + config: + server: + rserver.conf: + www-allow-origin: [a.example.com, b.example.com] # www-allow-origin=a.example.com + # www-allow-origin=b.example.com + ``` + + Files read by boost `property_tree`, which rejects a repeated key - `profiles`, `launcher.conf`, + `logging.conf`, `repos.conf`, and the `launcher.*.resources.conf` and `launcher.*.profiles.conf` + files - comma-join them (`container-images=a,b`), as `config.profiles` always did. Either way the + same applies inside a `[section]`, where a list previously rendered as `key=[a b]`. An empty list + (`container-images: []`) renders nothing rather than `container-images=[]`. + + `config.session.pip\.conf` is the exception: pip is not Workbench, and it writes several values + as one value continued over indented lines, which the chart does not produce. A repeated key + makes pip refuse the whole file, so a list of values in `pip.conf` now fails the render with a + message saying to write the file as a string. It previously rendered `extra-index-url=[a b]`. + + `config.secret`, `config.sessionSecret`, `config.startupCustom` and `config.startupUserProvisioning` + keep repeating the key at the top level of a file, as before. `config.sssd.conf` comma-joins + (`domains=a,b`), which is sssd's own syntax; it previously rendered `domains=[a b]`. +- `config.session` files are now rendered according to their format rather than the shape of the + value written. `r-versions` and `notifications.conf` are DCF (`Key: Value`, records separated by + a blank line), `*.json` files are JSON, and everything else stays ini. Previously all of them + were rendered as ini, so `r-versions` came out as `Key=Value` and Workbench discarded it, + logging `does not point to a valid directory` for each line. Resolves + https://github.com/rstudio/helm/issues/948. A file written as a raw string is still passed + through unchanged. + + **Your R version list may change on upgrade.** If you wrote `r-versions` as a list of records, + Workbench has been ignoring it and scanning for R on its own; it now reads your entries. Entries + whose `Path` no longer exists are logged and skipped. The same goes for `*.json` session files + written as a map, which Workbench has been ignoring as invalid JSON. +- **BREAKING**: a file written as a list must give each section or entry its own `- `, holding a + single key. This rejects shapes that previously rendered something unusable: several sections + crammed into one entry (a missing `- `), which rendered as `name=map[key:value]`; and a multi-field + record, which was only ever used for `config.session.r-versions` and emitted `Key=Value` where + Workbench parses that file as DCF (`Key: Value`) - see https://github.com/rstudio/helm/issues/948. + Write `r-versions` as a string (`r-versions: |`), which is passed through unchanged. An entry with + no value (`- "*":` with nothing under it) also fails; it rendered as `*=`. +- **BREAKING**: a list entry holding several options in a file without sections, such as + `rsession.conf: [{session-timeout-minutes: 60, session-save-action-default: none}]`, now fails + where it used to render one line per option. Write the file as a map instead. +- **BREAKING**: an option's value must be a single value or a list of single values. ini files have + no nesting, so a map there (`limits: {cpu: 1}`) rendered as `limits=map[cpu:1]`; it now fails. +- `config.session.repos\.conf` now defaults to a list, so that its order is kept. A `repos.conf` + you write as a map still gets the chart's `CRAN` entry when it has none, as it did when the + default was a map. A list replaces the default, so it must name a `CRAN` entry itself - Workbench + ignores the whole file without one - and the chart now fails if it does not. A string without a + `CRAN=` line prints a `WARNING` instead. With a map, Helm itself also logs `destination for + rstudio-workbench.config.session.repos.conf is a table. Ignoring non-table value (...)` on every + command: that is the chart's default list being set aside in favor of your map, it is harmless, + and it goes away once the file is written as a list. +- `config.server.rserver\.conf`, `launcher\.conf`, `launcher\.kubernetes\.conf`, and + `positron\.conf` written as a list now fail with a message saying to write them as a map. The + chart merges its own settings into these files, which a list silently dropped - `launcher\.conf` + lost the `[server]` section the launcher needs. `rserver\.conf` written as a string now fails with + the same message instead of a template type error. The other three still accept a string, which + is used as the whole file and so replaces those settings too (`kubernetes-namespace` and + `use-templating`, the rootless `secure-cookie-key-file`, the Positron `exe`); the message and + README now say so, where before they only said a string was accepted. +- `launcher.*.profiles.conf` no longer starts with a blank line. Profiles files now render through + the same helper as every other ini file, which places the blank line between sections rather than + before the first one. Nothing reads it - the file is parsed with an ini parser that skips blank + lines - but it changes the rendered file, so the config checksum shifts and pods restart once on + upgrade. +- The chart now warns when any other ini file is written as a *list*. Helm merges a map with the + chart's defaults for a file but replaces them with a list, so the list form drops any default the + chart ships for that file. Write those files as a map unless you need to control section order. + Files that are not ini (`r-versions`, `notifications.conf`, `*.json`) are exempt, since a list is + how you legitimately write those. +- `config.server` files are now rendered by format too, the same way `config.session` already is. + A single table in the chart gives each configuration file its format, and both ConfigMaps read + from it. `config.server.*.json` files written as a map are now rendered as JSON rather than ini. +- The chart now warns when a configuration file it does not recognize is written as a map or a + list. Such a file is still rendered as ini with repeated keys, which is what the chart has always + done, but ini is a guess for a file the chart knows nothing about, and a wrong guess renders a + file that looks fine and is ignored by the product. Give the file's contents as text to render it + exactly as written. `Renviron.site` is recognized and does not warn. + ## 0.22.2 - Bump Workbench version to 2026.09.0 diff --git a/charts/rstudio-workbench/README.md b/charts/rstudio-workbench/README.md index ca399c43..e9a1882a 100644 --- a/charts/rstudio-workbench/README.md +++ b/charts/rstudio-workbench/README.md @@ -1,6 +1,6 @@ # Posit Workbench -![Version: 0.22.2](https://img.shields.io/badge/Version-0.22.2-informational?style=flat-square) ![AppVersion: 2026.09.0](https://img.shields.io/badge/AppVersion-2026.09.0-informational?style=flat-square) +![Version: 0.23.0](https://img.shields.io/badge/Version-0.23.0-informational?style=flat-square) ![AppVersion: 2026.09.0](https://img.shields.io/badge/AppVersion-2026.09.0-informational?style=flat-square) #### _Official Helm chart for Posit Workbench_ @@ -24,11 +24,11 @@ To ensure a stable production deployment: ## Installing the chart -To install the chart with the release name `my-release` at version 0.22.2: +To install the chart with the release name `my-release` at version 0.23.0: ```{.bash} helm repo add rstudio https://helm.rstudio.com -helm upgrade --install my-release rstudio/rstudio-workbench --version=0.22.2 +helm upgrade --install my-release rstudio/rstudio-workbench --version=0.23.0 ``` To explore other chart versions, look at: @@ -259,10 +259,17 @@ The files are converted into configuration files in the necessary format via go- ```yaml config: server: - rserver.conf: | + logging.conf: | verbatim-file=format ``` +A string is used as the whole file, so it also replaces whatever the chart would have added to that +file. That matters for the files the chart merges its own settings into: `launcher.conf` (the +`[server]` `secure-cookie-key-file` entry when `pod.runAsRoot` is `false`), `launcher.kubernetes.conf` +(`kubernetes-namespace` and `use-templating`) and `positron.conf` (`exe` when the Positron init +container is enabled) accept a string but you must include those settings yourself; `rserver.conf` +must be a map. `config.profiles` files must be a map or a list, not a string. + The names of files are dynamically used, so you can add new files as needed. Beware that some files have default values, so moving them can have adverse effects. Also, if you use a different mounting paradigm, you need to change the `XDG_CONFIG_DIRS` environment variable. @@ -312,7 +319,7 @@ the `XDG_CONFIG_DIRS` environment variable. - `supervisord` service / unit definition `.conf` files. - Use the `.ini` file format by default. - Mounted at:
`/startup/custom` - - As with all configuration files above, you can override with a verbatim string if desired: + - As with all configuration files above (other than `config.profiles`), you can override with a verbatim string if desired: - Located at:
`config.startupCustom.<< name of file >>` Helm values: ```yaml config: @@ -338,19 +345,108 @@ pip can be configured with `config.session.pip.conf`: trusted-host: packagemanager.posit.co ``` +`pip.conf` is read by pip, not by Workbench, and pip writes an option with several values (such as +`extra-index-url`) as one value continued over indented lines. The chart does not produce that +form, and a repeated key makes pip refuse the whole file, so a list of values in `pip.conf` fails +the render. Write the file as a string instead: + + ```yaml + config: + session: + pip.conf: | + [global] + index-url = https://packagemanager.posit.co/pypi/latest/simple + extra-index-url = + https://pkgs.example.com/internal/simple + https://pkgs.example.com/other/simple + ``` + #### R repositories -R package repositories can be configured with `config.session.repos.conf`: +R package repositories can be configured with `config.session.repos.conf`. R reads the file in +order and uses that order to break ties when a package version is in more than one repository, so +write the repositories as a list, putting `- ` in front of each one: ```yaml config: session: repos.conf: - CRAN: https://packagemanager.posit.co/cran/__linux__/jammy/latest + - Internal: https://pkgs.example.com/internal + - CRAN: https://packagemanager.posit.co/cran/__linux__/jammy/latest +``` + +Becomes: + +_/etc/rstudio/repos.conf_ + +```ini +Internal=https://pkgs.example.com/internal +CRAN=https://packagemanager.posit.co/cran/__linux__/jammy/latest ``` +:::{.callout-warning} +A map does not keep the order you wrote it in: the chart renders map keys alphabetically, so an +internal repository can't be put ahead of CRAN, and the chart warns when `repos.conf` is a map. +::: + +:::{.callout-important} +`repos.conf` must contain an entry named `CRAN`. Workbench ignores the whole file when nothing is +named `CRAN`, so the other repositories are lost too - the only sign is `is missing CRAN entry` in +the session log. The entry does not have to be CRAN itself; point it at your own mirror if that is +what sessions should use. + +- Written as a **list**, your `repos.conf` **replaces** the chart default, so it must name `CRAN` + itself. The chart fails if it does not. +- Written as a **map** with no `CRAN` entry, it gets the chart's default `CRAN` entry added, as it + always has. +- Written as a **string**, it is used as-is, and the chart warns if it has no `CRAN=` line. + +To configure repositories somewhere else entirely, set `config.session.repos\.conf: null` and the +chart renders no file. +::: + For more information about configuring CRAN repositories in Workbench, see the [Posit Workbench Administrator Guide's - Package Installation > CRAN repositories](https://docs.posit.co/ide/server-pro/rstudio_pro_sessions/package_installation.html#cran-repositories) section. +#### R versions + +`/etc/rstudio/r-versions` is not an ini file. It is DCF: `Key: Value`, with a blank line between +each R version. The chart renders it with a DCF renderer, so write each R version as one list +entry and its fields as that entry's keys: + +```yaml +config: + session: + r-versions: + - Path: /opt/R/4.1.3 + Label: Custom 4.1.3 + Repo: https://packagemanager.posit.co/cran/__linux__/jammy/latest + - Path: /opt/R/4.2.3 + Label: Custom 4.2.3 +``` + +A raw string also works, and is passed through unchanged. + +:::{.callout-note} +Configuration files are rendered according to their format, not the shape of the value you write. +`r-versions` and `notifications.conf` are DCF, `*.json` files are JSON, and everything else the +chart recognizes (`rserver.conf`, `repos.conf`, `rsession.conf`, `pip.conf`, ...) is ini. Before +chart 0.23.0 every `config.session` file was rendered as ini, so `r-versions` came out with `=` +and Workbench ignored it entirely - see [#948](https://github.com/rstudio/helm/issues/948). + +A file the chart does not recognize is rendered as ini and prints a warning on install, since ini +is a guess for a file it knows nothing about. Give that file's contents as text to render it +exactly as written: + +```yaml +config: + session: + my-file: | + whatever the file needs to say +``` +::: + +See [Extended R version definitions](https://docs.posit.co/ide/server-pro/admin/r/using_multiple_versions_of_r.html#extended-r-version-definitions) in the Administrator Guide for the full list of fields. + ## User provisioning Provisioning users in Workbench containers is challenging. Session images create users automatically (with @@ -410,6 +506,28 @@ Sections define whether a set of configurations is applied to a user's jobs base The product reads configuration from top to bottom and "last-in-wins" for a given configuration value. +Because these files are read in order, write their sections as a list, putting `- ` in front of +each section header. A map does not keep the order you wrote it in: the chart renders map keys +alphabetically, so, for example, a user named `12345` would lose their own settings to every group +they belong to. The map form still renders, sorted, and the chart warns about it. + +This applies to `/etc/rstudio/profiles`, `launcher.*.profiles.conf` (under `config.profiles`, or +the deprecated `config.server` location), and `launcher.*.resources.conf` (where the session +launcher lists resource profiles in file order and pre-selects the first one). + +Which form to use is decided by the file, not by preference: **an order-sensitive file wants a +list, every other ini file wants a map.** Helm merges a map with the chart's defaults for a file +but replaces them with a list, so writing an order-agnostic file as a list drops whatever the +chart ships for it. The chart warns in both directions, and fails outright for `rserver.conf`, +`launcher.conf`, `launcher.kubernetes.conf`, and `positron.conf` written as a list, since the chart +merges settings of its own into those. Files that are not ini - `r-versions`, +`notifications.conf`, and `*.json` - are exempt, since a list is how you legitimately write those. + +Only the sections are ordered. The options written inside a section are still rendered +alphabetically, which is what these files expect - they are resolved section by section, not +option by option. A file that depends on the order of options within a section needs the raw +string form. + ### `/etc/rstudio/profiles` The `/etc/rstudio/profiles` file enables you to tailor the behavior of sessions on a per-user or per-group basis. See the [Posit Workbench Administrator Guide - User and Group Profiles](https://docs.posit.co/ide/server-pro/rstudio_pro_sessions/user_and_group_profiles.html) page for more information. @@ -420,9 +538,11 @@ In the `values.yaml`, define the content of `/etc/rstudio/profiles` in `config.s config: server: profiles: - "*": - session-limit: 5 - session-timeout-minutes: 60 + - "*": + session-limit: 5 + session-timeout-minutes: 60 + - "@analysts": + session-limit: 10 ``` Becomes: @@ -433,6 +553,9 @@ _/etc/rstudio/profiles_ [*] session-limit=5 session-timeout-minutes=60 + +[@analysts] +session-limit=10 ``` ### `/etc/rstudio/launcher.kubernetes.profiles.conf` @@ -447,7 +570,7 @@ The `/etc/rstudio/launcher.kubernetes.profiles.conf` contains the configuration - value2 ``` -- The `[*]` section has arrays "appended" to user and group sections, along with "defaults" defined by the chart. +- The `[*]` section receives the "defaults" defined by the chart (the session image settings), and its `job-json-overrides` are prepended to every other section that defines its own. Other keys are not merged across sections - a user or group section overrides `[*]` for that key, which is how the product resolves profiles. For example: @@ -455,14 +578,14 @@ For example: config: profiles: launcher.kubernetes.profiles.conf: - "*": - some-key: - - value1 - - value2 - myuser: - some-key: - - value4 - - value5 + - "*": + some-key: + - value1 + - value2 + - myuser: + some-key: + - value4 + - value5 ``` Becomes: @@ -471,9 +594,10 @@ _/etc/rstudio/launcher.kubernetes.profiles.conf_ ```ini [*] -some-key: value1,value2 +some-key=value1,value2 + [myuser] -some-key: value1,value2,value3,value4 +some-key=value4,value5 ``` :::{.callout-note} @@ -764,11 +888,11 @@ When combining `sealedSecret.enabled=true` with rootless mode (`pod.runAsRoot=fa | config.defaultMode.userProvisioning | int | 0600 | default mode for userProvisioning config | | config.existingSecrets | list | `[]` | a list of existing Kubernetes Secrets to project into `/mnt/secret-configmap/rstudio/`. Each item should have `name` (secret name) and `items` (list of keys to mount with their paths). Mounted with 0600 permissions by default. | | config.pam | object | `{}` | a map of pam config files. Will be mounted into the container directly / per file, in order to avoid overwriting system pam files | -| config.profiles | object | `{}` | a map of server-scoped config files (akin to `config.server`), but with specific behavior that supports profiles. See README for more information. | +| config.profiles | object | `{}` | a map of server-scoped config files (akin to `config.server`), but with specific behavior that supports profiles. `launcher.*.profiles.conf` is read in order, so write its sections as a list of single-entry maps. See README for more information. | | config.secret | string | `nil` | a map of secret, server-scoped config files (database.conf, databricks.conf, openid-client-secret). Mounted to `/mnt/secret-configmap/rstudio/` with 0600 permissions | -| config.server | object | [RStudio Workbench Configuration Reference](https://docs.rstudio.com/ide/server-pro/rstudio_server_configuration/rstudio_server_configuration.html). See defaults with `helm show values` | a map of server config files. Mounted to `/mnt/configmap/rstudio/` | +| config.server | object | [RStudio Workbench Configuration Reference](https://docs.rstudio.com/ide/server-pro/rstudio_server_configuration/rstudio_server_configuration.html). See defaults with `helm show values` | a map of server config files. Mounted to `/mnt/configmap/rstudio/`. Each file's contents may be a map, a raw string, or - for files read in order, such as `profiles` and `launcher.*.resources.conf` - a list of single-entry maps. See README for more information. | | config.serverDcf | object | `{"launcher-mounts":[]}` | a map of server-scoped config files (akin to `config.server`), but with .dcf file formatting (i.e. `launcher-mounts`, `launcher-env`, etc.) | -| config.session | object | `{"notifications.conf":{},"repos.conf":{"CRAN":"https://packagemanager.posit.co/cran/__linux__/jammy/latest"},"rsession.conf":{},"rstudio-prefs.json":"{}\n"}` | a map of session-scoped config files. Mounted to `/mnt/session-configmap/rstudio/` on both server and session, by default. | +| config.session | object | `{"notifications.conf":{},"repos.conf":[{"CRAN":"https://packagemanager.posit.co/cran/__linux__/jammy/latest"}],"rsession.conf":{},"rstudio-prefs.json":"{}\n"}` | a map of session-scoped config files. Mounted to `/mnt/session-configmap/rstudio/` on both server and session, by default. Each file's contents may be a map, a raw string, or - for files read in order, such as `repos.conf` - a list of single-entry maps. See README for more information. | | config.sessionSecret | object | `{}` | a map of secret, session-scoped config files (odbc.ini, etc.). Mounted to `/mnt/session-secret/` on both server and session, by default | | config.sssd | object | `{"conf":{},"enabled":true}` | Bundled SSSD daemon for legacy LDAP/Active Directory user provisioning. On by default; automatically skipped when the pod runs unprivileged (`pod.runAsRoot: false`), since SSSD requires root. Modern provisioning (SCIM / native) does not require SSSD. | | config.sssd.conf | object | `{}` | a map of sssd config files, mounted to `/etc/sssd/conf.d/` with 0600 permissions. Replaces the deprecated `config.userProvisioning`. | diff --git a/charts/rstudio-workbench/README.md.gotmpl b/charts/rstudio-workbench/README.md.gotmpl index 4eda4cab..ef9b3481 100644 --- a/charts/rstudio-workbench/README.md.gotmpl +++ b/charts/rstudio-workbench/README.md.gotmpl @@ -205,10 +205,17 @@ The files are converted into configuration files in the necessary format via go- ```yaml config: server: - rserver.conf: | + logging.conf: | verbatim-file=format ``` +A string is used as the whole file, so it also replaces whatever the chart would have added to that +file. That matters for the files the chart merges its own settings into: `launcher.conf` (the +`[server]` `secure-cookie-key-file` entry when `pod.runAsRoot` is `false`), `launcher.kubernetes.conf` +(`kubernetes-namespace` and `use-templating`) and `positron.conf` (`exe` when the Positron init +container is enabled) accept a string but you must include those settings yourself; `rserver.conf` +must be a map. `config.profiles` files must be a map or a list, not a string. + The names of files are dynamically used, so you can add new files as needed. Beware that some files have default values, so moving them can have adverse effects. Also, if you use a different mounting paradigm, you need to change the `XDG_CONFIG_DIRS` environment variable. @@ -258,7 +265,7 @@ the `XDG_CONFIG_DIRS` environment variable. - `supervisord` service / unit definition `.conf` files. - Use the `.ini` file format by default. - Mounted at:
`/startup/custom` - - As with all configuration files above, you can override with a verbatim string if desired: + - As with all configuration files above (other than `config.profiles`), you can override with a verbatim string if desired: - Located at:
`config.startupCustom.<< name of file >>` Helm values: ```yaml config: @@ -284,19 +291,108 @@ pip can be configured with `config.session.pip.conf`: trusted-host: packagemanager.posit.co ``` +`pip.conf` is read by pip, not by Workbench, and pip writes an option with several values (such as +`extra-index-url`) as one value continued over indented lines. The chart does not produce that +form, and a repeated key makes pip refuse the whole file, so a list of values in `pip.conf` fails +the render. Write the file as a string instead: + + ```yaml + config: + session: + pip.conf: | + [global] + index-url = https://packagemanager.posit.co/pypi/latest/simple + extra-index-url = + https://pkgs.example.com/internal/simple + https://pkgs.example.com/other/simple + ``` + #### R repositories -R package repositories can be configured with `config.session.repos.conf`: +R package repositories can be configured with `config.session.repos.conf`. R reads the file in +order and uses that order to break ties when a package version is in more than one repository, so +write the repositories as a list, putting `- ` in front of each one: ```yaml config: session: repos.conf: - CRAN: https://packagemanager.posit.co/cran/__linux__/jammy/latest + - Internal: https://pkgs.example.com/internal + - CRAN: https://packagemanager.posit.co/cran/__linux__/jammy/latest +``` + +Becomes: + +_/etc/rstudio/repos.conf_ + +```ini +Internal=https://pkgs.example.com/internal +CRAN=https://packagemanager.posit.co/cran/__linux__/jammy/latest ``` +:::{.callout-warning} +A map does not keep the order you wrote it in: the chart renders map keys alphabetically, so an +internal repository can't be put ahead of CRAN, and the chart warns when `repos.conf` is a map. +::: + +:::{.callout-important} +`repos.conf` must contain an entry named `CRAN`. Workbench ignores the whole file when nothing is +named `CRAN`, so the other repositories are lost too - the only sign is `is missing CRAN entry` in +the session log. The entry does not have to be CRAN itself; point it at your own mirror if that is +what sessions should use. + +- Written as a **list**, your `repos.conf` **replaces** the chart default, so it must name `CRAN` + itself. The chart fails if it does not. +- Written as a **map** with no `CRAN` entry, it gets the chart's default `CRAN` entry added, as it + always has. +- Written as a **string**, it is used as-is, and the chart warns if it has no `CRAN=` line. + +To configure repositories somewhere else entirely, set `config.session.repos\.conf: null` and the +chart renders no file. +::: + For more information about configuring CRAN repositories in Workbench, see the [Posit Workbench Administrator Guide's - Package Installation > CRAN repositories](https://docs.posit.co/ide/server-pro/rstudio_pro_sessions/package_installation.html#cran-repositories) section. +#### R versions + +`/etc/rstudio/r-versions` is not an ini file. It is DCF: `Key: Value`, with a blank line between +each R version. The chart renders it with a DCF renderer, so write each R version as one list +entry and its fields as that entry's keys: + +```yaml +config: + session: + r-versions: + - Path: /opt/R/4.1.3 + Label: Custom 4.1.3 + Repo: https://packagemanager.posit.co/cran/__linux__/jammy/latest + - Path: /opt/R/4.2.3 + Label: Custom 4.2.3 +``` + +A raw string also works, and is passed through unchanged. + +:::{.callout-note} +Configuration files are rendered according to their format, not the shape of the value you write. +`r-versions` and `notifications.conf` are DCF, `*.json` files are JSON, and everything else the +chart recognizes (`rserver.conf`, `repos.conf`, `rsession.conf`, `pip.conf`, ...) is ini. Before +chart 0.23.0 every `config.session` file was rendered as ini, so `r-versions` came out with `=` +and Workbench ignored it entirely - see [#948](https://github.com/rstudio/helm/issues/948). + +A file the chart does not recognize is rendered as ini and prints a warning on install, since ini +is a guess for a file it knows nothing about. Give that file's contents as text to render it +exactly as written: + +```yaml +config: + session: + my-file: | + whatever the file needs to say +``` +::: + +See [Extended R version definitions](https://docs.posit.co/ide/server-pro/admin/r/using_multiple_versions_of_r.html#extended-r-version-definitions) in the Administrator Guide for the full list of fields. + ## User provisioning Provisioning users in Workbench containers is challenging. Session images create users automatically (with @@ -356,6 +452,28 @@ Sections define whether a set of configurations is applied to a user's jobs base The product reads configuration from top to bottom and "last-in-wins" for a given configuration value. +Because these files are read in order, write their sections as a list, putting `- ` in front of +each section header. A map does not keep the order you wrote it in: the chart renders map keys +alphabetically, so, for example, a user named `12345` would lose their own settings to every group +they belong to. The map form still renders, sorted, and the chart warns about it. + +This applies to `/etc/rstudio/profiles`, `launcher.*.profiles.conf` (under `config.profiles`, or +the deprecated `config.server` location), and `launcher.*.resources.conf` (where the session +launcher lists resource profiles in file order and pre-selects the first one). + +Which form to use is decided by the file, not by preference: **an order-sensitive file wants a +list, every other ini file wants a map.** Helm merges a map with the chart's defaults for a file +but replaces them with a list, so writing an order-agnostic file as a list drops whatever the +chart ships for it. The chart warns in both directions, and fails outright for `rserver.conf`, +`launcher.conf`, `launcher.kubernetes.conf`, and `positron.conf` written as a list, since the chart +merges settings of its own into those. Files that are not ini - `r-versions`, +`notifications.conf`, and `*.json` - are exempt, since a list is how you legitimately write those. + +Only the sections are ordered. The options written inside a section are still rendered +alphabetically, which is what these files expect - they are resolved section by section, not +option by option. A file that depends on the order of options within a section needs the raw +string form. + ### `/etc/rstudio/profiles` The `/etc/rstudio/profiles` file enables you to tailor the behavior of sessions on a per-user or per-group basis. See the [Posit Workbench Administrator Guide - User and Group Profiles](https://docs.posit.co/ide/server-pro/rstudio_pro_sessions/user_and_group_profiles.html) page for more information. @@ -366,9 +484,11 @@ In the `values.yaml`, define the content of `/etc/rstudio/profiles` in `config.s config: server: profiles: - "*": - session-limit: 5 - session-timeout-minutes: 60 + - "*": + session-limit: 5 + session-timeout-minutes: 60 + - "@analysts": + session-limit: 10 ``` Becomes: @@ -379,6 +499,9 @@ _/etc/rstudio/profiles_ [*] session-limit=5 session-timeout-minutes=60 + +[@analysts] +session-limit=10 ``` ### `/etc/rstudio/launcher.kubernetes.profiles.conf` @@ -393,7 +516,7 @@ The `/etc/rstudio/launcher.kubernetes.profiles.conf` contains the configuration - value2 ``` -- The `[*]` section has arrays "appended" to user and group sections, along with "defaults" defined by the chart. +- The `[*]` section receives the "defaults" defined by the chart (the session image settings), and its `job-json-overrides` are prepended to every other section that defines its own. Other keys are not merged across sections - a user or group section overrides `[*]` for that key, which is how the product resolves profiles. For example: @@ -401,14 +524,14 @@ For example: config: profiles: launcher.kubernetes.profiles.conf: - "*": - some-key: - - value1 - - value2 - myuser: - some-key: - - value4 - - value5 + - "*": + some-key: + - value1 + - value2 + - myuser: + some-key: + - value4 + - value5 ``` Becomes: @@ -417,9 +540,10 @@ _/etc/rstudio/launcher.kubernetes.profiles.conf_ ```ini [*] -some-key: value1,value2 +some-key=value1,value2 + [myuser] -some-key: value1,value2,value3,value4 +some-key=value4,value5 ``` :::{.callout-note} diff --git a/charts/rstudio-workbench/templates/NOTES.txt b/charts/rstudio-workbench/templates/NOTES.txt index b26b72ca..62adc34b 100644 --- a/charts/rstudio-workbench/templates/NOTES.txt +++ b/charts/rstudio-workbench/templates/NOTES.txt @@ -22,6 +22,112 @@ kubectl -n {{ $.Release.Namespace }} get secret {{ include "rstudio-workbench.fu ``` {{- end }} +{{- /* Two form warnings, both keyed off the filename dispatch in _helpers.tpl. + + An order-sensitive file wants the list form: a map sorts its sections by name, silently + changing what the file does. An order-agnostic file wants the map form: Helm merges a map + with the chart's defaults for that file, but *replaces* them with a list. + + Only ini files are considered. r-versions and notifications.conf are DCF and .json files + are JSON; a list is how you legitimately write those. */}} +{{- $wantsList := list }} +{{- $wantsMap := list }} +{{- $assumedIni := list }} +{{- range $scope := (list "server" "session" "profiles") }} + {{- range $file, $contents := (get $.Values.config $scope | default dict) }} + {{- $path := printf "config.%s.%s" $scope ($file | replace "." "\\.") }} + {{- $format := include "rstudio-workbench.config.fileFormat" (dict "scope" $scope "file" $file) }} + {{- $ordered := include "rstudio-workbench.config.fileOrdered" (dict "scope" $scope "file" $file) }} + {{- if not (has $format (list "dcf" "json" "unknown")) }} + {{- if and $ordered (kindIs "map" $contents) }} + {{- $wantsList = append $wantsList $path }} + {{- else if and (not $ordered) (kindIs "slice" $contents) }} + {{- $wantsMap = append $wantsMap $path }} + {{- end }} + {{- else if and (eq $format "unknown") (or (kindIs "map" $contents) (kindIs "slice" $contents)) }} + {{- $assumedIni = append $assumedIni $path }} + {{- end }} + {{- end }} +{{- end }} +{{- if $wantsList }} + +WARNING: the following configuration files are written as maps, which does not keep the order you wrote them in +{{- range $wantsList | sortAlpha }} + - `.Values.{{ . }}` +{{- end }} + Workbench reads these files in order, so sorting their sections by name changes how they behave. + Write them as a list instead, putting `- ` in front of each section or entry: + + config: + server: + profiles: + - "*": + max-memory-mb: 1024 + - "@analysts": + max-memory-mb: 4096 + + A map is fine for every other config file; these are the ones where the order matters. + {{- $repos := get .Values.config.session "repos.conf" }} + {{- if and (kindIs "map" $repos) (not (hasKey $repos "CRAN")) }} + + `repos\.conf` has no `CRAN` entry, so the chart added its default one. A list replaces the + chart's default, so name a `CRAN` entry yourself when you convert it. + {{- end }} +{{- end }} +{{- if $wantsMap }} + +WARNING: the following configuration files are written as lists, which replaces the chart's defaults for them instead of merging with them +{{- range $wantsMap | sortAlpha }} + - `.Values.{{ . }}` +{{- end }} + Helm merges a map with the chart's defaults for a file, but a list replaces them: any default the + chart ships for these files is dropped, and only what you wrote is rendered. Nothing in these + files depends on the order of their sections, so write them as a map unless you specifically + need to control that order - and then include any chart defaults you still want in your list. +{{- end }} + +{{- if $assumedIni }} + +WARNING: the chart does not recognize the following configuration files +{{- range $assumedIni | sortAlpha }} + - `.Values.{{ . }}` +{{- end }} + They will be rendered as ini: `Key=Value` under `[section]` headings. If that is not the right + format, give the file's contents as text instead and the chart will use them as they are: + + config: + session: + my-file: | + whatever the file needs to say + + Open an issue if this is a file the chart should know how to build. +{{- end }} + +{{- /* Workbench discards repos.conf entirely when it has no entry named CRAN + (SessionOptions.cpp, parseReposConfig), taking the admin's own repositories with it. + configmap-session.yaml fails for the map and list forms; a string is passed through + untouched, so only warn for it. */}} +{{- $repos := get .Values.config.session "repos.conf" }} +{{- if kindIs "string" $repos }} + {{- if and $repos (not (regexMatch "(?m)^[ \t]*CRAN[ \t]*=" $repos)) }} + +WARNING: `.Values.config.session.repos\.conf` has no `CRAN` entry + - Workbench ignores the whole file when no entry is named `CRAN`, so none of these repositories + will be used, and the session log will show "is missing CRAN entry". + - Name one of your repositories `CRAN`. It does not have to be CRAN itself - point it at your + own mirror if that is what you want sessions to use: + + config: + session: + repos.conf: | + CRAN=https://mirror.example.com/cran + Internal=https://pkgs.example.com/internal + + - To configure repositories somewhere else instead, set `config.session.repos\.conf: null` and + the chart will not render the file at all. + {{- end }} +{{- end }} + {{- if hasKey .Values.config.server "launcher.kubernetes.profiles.conf" }} WARNING: `.Values.config.server.launcher\.kubernetes\.profiles\.conf` is deprecated @@ -40,9 +146,11 @@ Please consider removing this configuration value. {{- end }} {{- if and .Values.launcher.useTemplates .Values.launcher.enabled }} - {{- if hasKey .Values.config.profiles "launcher.kubernetes.profiles.conf" }} - {{- range $k,$v := (get .Values.config.profiles "launcher.kubernetes.profiles.conf") }} - {{- if hasKey $v "job-json-overrides" }} + {{- if hasKey (default (dict) .Values.config.profiles) "launcher.kubernetes.profiles.conf" }} + {{- $normalized := dict }} + {{- include "rstudio-library.config.entries" (dict "data" (get .Values.config.profiles "launcher.kubernetes.profiles.conf") "result" $normalized) }} + {{- range $entry := $normalized.entries }} + {{- if hasKey $entry.config "job-json-overrides" }} {{- fail "\n\n`profiles` has `job-json-overrides` defined. This cannot be used with `launcher.useTemplates=true`.\n\nPlease move `job-json-overrides` to the corresponding `launcher.templateValues`, or set `launcher.useTemplates=false`.\n\nNote: `launcher.useTemplates=true` was made the default in chart version 0.9.0" }} {{- end }} {{- end }} diff --git a/charts/rstudio-workbench/templates/_helpers.tpl b/charts/rstudio-workbench/templates/_helpers.tpl index b7198c23..ed4905f7 100644 --- a/charts/rstudio-workbench/templates/_helpers.tpl +++ b/charts/rstudio-workbench/templates/_helpers.tpl @@ -715,3 +715,143 @@ app.kubernetes.io/instance: {{ .Release.Name }} {{- define "rstudio-workbench.xdg-config-dirs" -}} {{ trimSuffix ":" ( join ":" (list .Values.xdgConfigDirs (join ":" .Values.xdgConfigDirsExtra) ) ) }} {{- end -}} + +{{- /* + The CRAN entry of the chart's default repos.conf, which values.yaml writes as a list. A map + written by the admin replaces that list rather than merging with it, so configmap-session.yaml + adds this back to a map with no CRAN, as the chart did when the default was a map. Must match + values.yaml; tests/configmap_test.yaml asserts both render the same URL. +*/ -}} +{{- define "rstudio-workbench.config.defaultCran" -}} +https://packagemanager.posit.co/cran/__linux__/jammy/latest +{{- end -}} + +{{- /* + ========================================================================== + Config file table - the one place that knows anything about config filenames + ========================================================================== + + One row per file: scope | filename pattern | format | ordered. The first matching row wins. + + scope config.server / config.session / config.profiles, or * for any + pattern regex matched against the filename + format which parser Workbench reads the file with, which decides both the renderer and + how an option with several values is written: + ini_ptree boost property_tree read_ini. [section] headings; a repeated key is an + error, so a list of values is comma-joined (a,b,c) + ini_popt boost program_options. A list of values repeats the key, one line per + value, and a comma is part of the value (www-allow-origin, + server-add-header) + gcfg Go gcfg, as ini with repeated keys + renviron R's Renviron, as ini with repeated keys + pip Python configparser (pip.conf is not read by Workbench at all). Several + values are written as a newline-continued value, which this chart does + not produce, and a repeated key makes pip refuse the whole file, so a + list of values fails with "write the file as a string" + dcf Key: Value, records separated by a blank line + json JSON + ordered yes when the file's behavior depends on the order of its sections or entries, so it + wants the list form. Read only by NOTES.txt + + A file matching no row is "unknown". Its contents are best given as a string, which is passed + through untouched whatever the format. Written as a map or a list it is still built as ini with + repeated keys, because that is what the chart has always done, but NOTES.txt says so: guessing + ini for a file we do not recognize is how `r-versions` came out as `Key=Value`, which Workbench + silently ignores (#948). + + Consumers: configmap-general.yaml and configmap-session.yaml pick the renderer, NOTES.txt + raises the form warnings. Add a file here and all of them follow. Not every scope goes through + here, and none of these get NOTES warnings: config.secret, config.sessionSecret, + config.startupCustom and config.startupUserProvisioning render as ini with repeated keys (what + the chart always did for them), and config.sssd.conf as ini with comma-joined lists (sssd's own + syntax for several values). +*/ -}} +{{- define "rstudio-workbench.config.fileTable" -}} +server | ^profiles$ | ini_ptree | yes +server | ^launcher\..+\.resources\.conf$ | ini_ptree | yes +profiles | ^launcher\..+\.profiles\.conf$ | ini_ptree | yes +server | ^launcher\..+\.profiles\.conf$ | ini_ptree | yes +* | ^repos\.conf$ | ini_ptree | yes +* | ^launcher\.conf$ | ini_ptree | no +* | ^logging\.conf$ | ini_ptree | no +* | ^r-versions$ | dcf | no +* | ^notifications\.conf$ | dcf | no +* | \.json$ | json | no +* | ^chronicle-local\.gcfg$ | gcfg | no +* | ^Renviron\.site$ | renviron | no +session | ^pip\.conf$ | pip | no +* | \.conf$ | ini_popt | no +{{- end -}} + +{{- /* + Looks a file up in the table. Takes `scope`, `file`, and `column` (2 for format, 3 for + ordered); returns that column of the first matching row, or "" when no row matches. +*/ -}} +{{- define "rstudio-workbench.config.fileLookup" -}} +{{- $scope := .scope -}} +{{- $file := .file -}} +{{- $column := .column -}} +{{- $hit := "" -}} +{{- $found := false -}} +{{- range $line := splitList "\n" (include "rstudio-workbench.config.fileTable" .) -}} + {{- if and (not $found) (contains "|" $line) -}} + {{- $col := splitList "|" $line -}} + {{- if and (or (eq (trim (index $col 0)) "*") (eq (trim (index $col 0)) $scope)) (regexMatch (trim (index $col 1)) $file) -}} + {{- $found = true -}} + {{- $hit = trim (index $col $column) -}} + {{- end -}} + {{- end -}} +{{- end -}} +{{- $hit -}} +{{- end -}} + +{{- /* The file's format from the table, or "unknown". Takes `scope` and `file`. */ -}} +{{- define "rstudio-workbench.config.fileFormat" -}} +{{- include "rstudio-workbench.config.fileLookup" (dict "scope" .scope "file" .file "column" 2) | default "unknown" -}} +{{- end -}} + +{{- /* "yes" when the table marks the file as ordered, otherwise "". Takes `scope` and `file`. */ -}} +{{- define "rstudio-workbench.config.fileOrdered" -}} +{{- if eq (include "rstudio-workbench.config.fileLookup" (dict "scope" .scope "file" .file "column" 3)) "yes" }}yes{{ end -}} +{{- end -}} + +{{- /* + Renders one config scope, picking a renderer per file from the table above rather than + treating every file as ini. The files in these directories are not all the same format: + `r-versions` and `notifications.conf` are DCF, `*.json` files are JSON, and the rest of + what the chart recognizes is ini of one flavor or another. + + Takes `scope` and `data`. A file the table does not list falls back to ini with repeated + keys, which is a guess; NOTES.txt warns about it so the guess is at least visible. +*/ -}} +{{- define "rstudio-workbench.config.files" -}} +{{- $scope := .scope }} +{{- $ini := dict }} +{{- $dcf := dict }} +{{- $json := dict }} +{{- range $file, $contents := .data }} + {{- $format := include "rstudio-workbench.config.fileFormat" (dict "scope" $scope "file" $file) }} + {{- if or (kindIs "string" $contents) (empty $contents) }} + {{- /* Already the finished file. Every renderer has to pass a string through untouched and + the ini one does; the JSON one would re-encode it and turn `{}` into `"{}"`. Empty + renders to nothing whichever bucket it lands in. */ -}} + {{- $_ := set $ini $file $contents }} + {{- else if eq $format "dcf" }} + {{- $_ := set $dcf $file $contents }} + {{- else if eq $format "json" }} + {{- $_ := set $json $file $contents }} + {{- else }} + {{- $_ := set $ini $file $contents }} + {{- end }} +{{- end }} +{{- /* One file at a time, in the sorted order rstudio-library.config.ini would use, so that each + gets its own way of writing several values: ini_ptree comma-joins, pip refuses, everything + else (ini_popt, gcfg, renviron, unknown) repeats the key. */ -}} +{{- range $file := keys $ini | sortAlpha }} + {{- $format := include "rstudio-workbench.config.fileFormat" (dict "scope" $scope "file" $file) }} + {{- $multi := get (dict "ini_ptree" "join" "pip" "reject") $format | default "repeat" }} + {{- include "rstudio-library.config.ini.files" (dict "files" (dict $file (get $ini $file)) "multi" $multi) }} +{{- end }} +{{- if $dcf }}{{- include "rstudio-library.config.dcf" $dcf }}{{- end }} +{{- if $json }}{{- include "rstudio-library.config.json" $json }}{{- end }} +{{- end }} diff --git a/charts/rstudio-workbench/templates/configmap-general.yaml b/charts/rstudio-workbench/templates/configmap-general.yaml index bedde656..1e6a8b77 100644 --- a/charts/rstudio-workbench/templates/configmap-general.yaml +++ b/charts/rstudio-workbench/templates/configmap-general.yaml @@ -1,3 +1,21 @@ +{{- /* The chart adds its own settings to these files by merging a map over what was written. + rserver.conf must be a map for that merge to work at all. The rest may be a string, which + is taken as the whole file - and so replaces the chart's settings too, which the message + says. A list would silently drop them, which for launcher.conf includes the [server] + section the launcher needs to start. Nothing in them depends on order, so there is no + reason to write them as a list. */}} +{{- $stringReplaces := dict + "launcher.conf" "the [server] secure-cookie-key-file setting the chart adds when pod.runAsRoot is false" + "launcher.kubernetes.conf" "the kubernetes-namespace and use-templating settings the chart adds" + "positron.conf" "the exe setting the chart adds when the Positron init container is enabled" }} +{{- range $file := list "rserver.conf" "launcher.conf" "launcher.kubernetes.conf" "positron.conf" }} + {{- $contents := get $.Values.config.server $file }} + {{- $allowed := eq $file "rserver.conf" | ternary (list "map") (list "map" "string") }} + {{- if and $contents (not (has (kindOf $contents) $allowed)) }} + {{- $orString := eq $file "rserver.conf" | ternary "" (print "\nA string is also accepted and is used as the whole file, so it replaces\n" (get $stringReplaces $file) "; include those yourself if you still want them.\n") }} + {{- fail (print "\n\nconfig.server." $file " is a " (kindOf $contents) ", but must be a map of options.\n\nThe chart merges its own settings into " $file ", which only works on a map.\nNothing in the file depends on order, so write it as a map:\n\n config:\n server:\n " $file ":\n option-name: value\n" $orString) }} + {{- end }} +{{- end }} {{- /* Define the default values that will be merged over */}} {{- $defaultVersion := .Values.versionOverride | default $.Chart.AppVersion }} {{- $sessionTag := .Values.session.image.tag | default (printf "R%s-python%s-%s" .Values.session.image.rVersion .Values.session.image.pythonVersion .Values.session.image.os ) }} @@ -149,17 +167,37 @@ data: {{- with .Values.chronicle.localConfig }} {{- $overrideDict = mergeOverwrite $overrideDict (dict "chronicle-local.gcfg" .) }} {{- end }} -{{ include "rstudio-library.config.ini" $overrideDict | indent 2 }} +{{ include "rstudio-workbench.config.files" (dict "scope" "server" "data" $overrideDict) | indent 2 }} {{/* helper variables to make things here a bit more sane */}} -{{- $profilesConfig := .Values.config.profiles }} -{{- $profilesConfig = mergeOverwrite (dict "launcher.kubernetes.profiles.conf" $defaultProfilesConfig) $profilesConfig }} +{{- $profilesConfig := default (dict) .Values.config.profiles | deepCopy }} +{{- /* Apply the default [*] session image settings to launcher.kubernetes.profiles.conf. When it is + written as an ordered list, mergeOverwrite would drop the defaults along with the map, so + merge them into the [*] entry in place instead (prepending one if the admin wrote none). */}} +{{- $writtenProfiles := get $profilesConfig "launcher.kubernetes.profiles.conf" }} +{{- if kindIs "slice" $writtenProfiles }} + {{- $hasEveryone := false }} + {{- range $item := $writtenProfiles }} + {{- /* anything but a map, and a [*] whose body is not a map, is left for the profiles helper + to reject with its own message */}} + {{- if and (kindIs "map" $item) (hasKey $item "*") (kindIs "map" (get $item "*")) }} + {{- $hasEveryone = true }} + {{- $_ := set $item "*" (mergeOverwrite (deepCopy $defaultProfiles) (get $item "*")) }} + {{- end }} + {{- end }} + {{- if not $hasEveryone }} + {{- $writtenProfiles = prepend $writtenProfiles (dict "*" $defaultProfiles) }} + {{- end }} + {{- $_ := set $profilesConfig "launcher.kubernetes.profiles.conf" $writtenProfiles }} +{{- else }} + {{- $profilesConfig = mergeOverwrite (dict "launcher.kubernetes.profiles.conf" $defaultProfilesConfig) $profilesConfig }} +{{- end }} {{- $useNewerOverrides := and (not (hasKey .Values.config.server "launcher.kubernetes.profiles.conf")) (not .Values.launcher.useTemplates) }} {{- $jobJsonFilePath := "/mnt/job-json-overrides-new/" }} {{- /* $defaultOverrides should be empty from above if we are using templates */ -}} {{- $profilesDict := dict "data" ($profilesConfig | deepCopy) "filePath" ($jobJsonFilePath) "jobJsonDefaults" ($defaultOverrides) }} {{- if not (hasKey .Values.config.server "launcher.kubernetes.profiles.conf") }} {{/* generate the profiles configuration */}} - {{- include "rstudio-library.profiles.ini.advanced" $profilesDict | nindent 2 }} + {{- include "rstudio-library.profiles.ini" $profilesDict | nindent 2 }} {{- end }} {{- /* generate the server configuration (dcf files) minus launcher-mounts */}} {{- include "rstudio-library.config.dcf" ( omit .Values.config.serverDcf "launcher-mounts" ) | nindent 2 }} diff --git a/charts/rstudio-workbench/templates/configmap-secret.yaml b/charts/rstudio-workbench/templates/configmap-secret.yaml index bdadab08..30b7e6e9 100644 --- a/charts/rstudio-workbench/templates/configmap-secret.yaml +++ b/charts/rstudio-workbench/templates/configmap-secret.yaml @@ -14,7 +14,7 @@ metadata: namespace: {{ $.Release.Namespace }} spec: encryptedData: - {{- include "rstudio-library.config.ini" .Values.config.secret | nindent 4 }} + {{- include "rstudio-library.config.ini.files" (dict "files" .Values.config.secret "multi" "repeat") | nindent 4 }} {{- /* do not auto-generate value as the secret will not be encrypted */}} {{- if .Values.launcherPem.existingSecret }} launcher.pem: | @@ -53,7 +53,7 @@ metadata: name: {{ include "rstudio-workbench.fullname" . }}-secret namespace: {{ $.Release.Namespace }} stringData: - {{- include "rstudio-library.config.ini" .Values.config.secret | nindent 2 }} + {{- include "rstudio-library.config.ini.files" (dict "files" .Values.config.secret "multi" "repeat") | nindent 2 }} {{- if not .Values.launcherPem.existingSecret }} launcher.pem: | {{- include "rstudio-workbench.launcherPem" . | nindent 4 }} diff --git a/charts/rstudio-workbench/templates/configmap-session.yaml b/charts/rstudio-workbench/templates/configmap-session.yaml index f71a28d9..ec503675 100644 --- a/charts/rstudio-workbench/templates/configmap-session.yaml +++ b/charts/rstudio-workbench/templates/configmap-session.yaml @@ -1,3 +1,32 @@ +{{- /* Workbench discards repos.conf entirely when it has no entry named CRAN + (SessionOptions.cpp, parseReposConfig), taking the admin's own repositories with it, and + nothing in the rendered file shows it. + - A map is the form repos.conf had when the chart's default was a map, and Helm merged the + two, so the default CRAN entry was always there. The default is now a list, which a map + replaces, so add the entry back to keep that working. NOTES.txt still warns that the + map form sorts the entries. + - A list replaces the default by design, so it has to name CRAN itself. Fail rather than + render a file Workbench will ignore. + - A string is passed through untouched; NOTES.txt warns about it instead. */}} +{{- $sessionConfig := .Values.config.session | deepCopy }} +{{- $repos := get $sessionConfig "repos.conf" }} +{{- if and $repos (kindIs "map" $repos) (not (hasKey $repos "CRAN")) }} + {{- $_ := set $repos "CRAN" (include "rstudio-workbench.config.defaultCran" .) }} +{{- else if and $repos (kindIs "slice" $repos) }} + {{- $hasCran := false }} + {{- $allMaps := true }} + {{- range $item := $repos }} + {{- if not (kindIs "map" $item) }} + {{- $allMaps = false }} + {{- else if hasKey $item "CRAN" }} + {{- $hasCran = true }} + {{- end }} + {{- end }} + {{- /* an entry that is not a map is the renderer's error to report; its message is the useful one */}} + {{- if and $allMaps (not $hasCran) }} + {{- fail "\n\nconfig.session.repos.conf has no entry named CRAN.\n\nWorkbench ignores the whole file without one, so none of these repositories would be\nused. Written as a list, repos.conf replaces the chart's default CRAN entry, so it must\nname its own. It does not have to be CRAN itself - name your own mirror CRAN if that\nis what sessions should use:\n\n config:\n session:\n repos.conf:\n - CRAN: https://mirror.example.com/cran\n - Internal: https://pkgs.example.com/internal\n\nTo configure repositories somewhere else instead, set config.session.repos.conf: null\nand the chart will not render the file at all.\n" }} + {{- end }} +{{- end }} --- apiVersion: v1 kind: ConfigMap @@ -5,7 +34,7 @@ metadata: name: {{ include "rstudio-workbench.fullname" . }}-session namespace: {{ $.Release.Namespace }} data: - {{- include "rstudio-library.config.ini" .Values.config.session | nindent 2 }} + {{- include "rstudio-workbench.config.files" (dict "scope" "session" "data" $sessionConfig) | nindent 2 }} {{- if .Values.config.sessionSecret }} --- {{- if .Values.sealedSecret.enabled }} @@ -19,7 +48,7 @@ metadata: spec: encryptedData: data: - {{- include "rstudio-library.config.ini" .Values.config.sessionSecret | nindent 6 }} + {{- include "rstudio-library.config.ini.files" (dict "files" .Values.config.sessionSecret "multi" "repeat") | nindent 6 }} {{- else }} apiVersion: v1 kind: Secret @@ -27,7 +56,7 @@ metadata: name: {{ include "rstudio-workbench.fullname" . }}-session-secret namespace: {{ $.Release.Namespace }} stringData: - {{- include "rstudio-library.config.ini" .Values.config.sessionSecret | nindent 2 }} + {{- include "rstudio-library.config.ini.files" (dict "files" .Values.config.sessionSecret "multi" "repeat") | nindent 2 }} {{- end }} {{- end }} @@ -47,7 +76,7 @@ metadata: name: {{ include "rstudio-workbench.fullname" . }}-session namespace: {{ $targetNamespace }} data: - {{- include "rstudio-library.config.ini" .Values.config.session | nindent 2 }} + {{- include "rstudio-workbench.config.files" (dict "scope" "session" "data" $sessionConfig) | nindent 2 }} {{- if .Values.config.sessionSecret }} --- {{- if .Values.sealedSecret.enabled }} @@ -61,7 +90,7 @@ metadata: spec: encryptedData: data: - {{- include "rstudio-library.config.ini" .Values.config.sessionSecret | nindent 6 }} + {{- include "rstudio-library.config.ini.files" (dict "files" .Values.config.sessionSecret "multi" "repeat") | nindent 6 }} {{- else }} apiVersion: v1 kind: Secret @@ -69,7 +98,7 @@ metadata: name: {{ include "rstudio-workbench.fullname" . }}-session-secret namespace: {{ $targetNamespace }} stringData: - {{- include "rstudio-library.config.ini" .Values.config.sessionSecret | nindent 2 }} + {{- include "rstudio-library.config.ini.files" (dict "files" .Values.config.sessionSecret "multi" "repeat") | nindent 2 }} {{- end }} {{- end }} {{- end }} diff --git a/charts/rstudio-workbench/templates/configmap-startup.yaml b/charts/rstudio-workbench/templates/configmap-startup.yaml index 46833595..9c87ad3f 100644 --- a/charts/rstudio-workbench/templates/configmap-startup.yaml +++ b/charts/rstudio-workbench/templates/configmap-startup.yaml @@ -43,7 +43,7 @@ metadata: name: {{ include "rstudio-workbench.fullname" . }}-start-user namespace: {{ $.Release.Namespace }} data: - {{- include "rstudio-library.config.ini" $userStartup | nindent 2 }} + {{- include "rstudio-library.config.ini.files" (dict "files" $userStartup "multi" "repeat") | nindent 2 }} {{- end }} {{- if .Values.config.startupCustom }} --- @@ -53,5 +53,5 @@ metadata: name: {{ include "rstudio-workbench.fullname" . }}-start-custom namespace: {{ $.Release.Namespace }} data: - {{- include "rstudio-library.config.ini" .Values.config.startupCustom | nindent 2}} + {{- include "rstudio-library.config.ini.files" (dict "files" .Values.config.startupCustom "multi" "repeat") | nindent 2}} {{- end }} diff --git a/charts/rstudio-workbench/tests/configmap_test.yaml b/charts/rstudio-workbench/tests/configmap_test.yaml index f338e5e9..29a9556b 100644 --- a/charts/rstudio-workbench/tests/configmap_test.yaml +++ b/charts/rstudio-workbench/tests/configmap_test.yaml @@ -458,3 +458,635 @@ tests: - matchRegex: path: data["rserver.conf"] pattern: "(?m)^user-provisioning-enabled=0$" + + # -- Ordered (list form) config files. See https://github.com/rstudio/helm/issues/944 + - it: should render config.server order-sensitive files in the order written + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + profiles: + - jsmith: + max-memory-mb: 8192 + - "12345": + max-memory-mb: 8192 + - "@contractors": + max-memory-mb: 2048 + - "@analysts": + max-memory-mb: 4096 + - "*": + max-memory-mb: 1024 + launcher.kubernetes.resources.conf: + - small: + name: Small + cpus: 1 + - large: + name: Large + cpus: 8 + 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 + - equal: + path: data["launcher.kubernetes.resources.conf"] + value: | + [small] + cpus=1 + name=Small + + [large] + cpus=8 + name=Large + + - it: should sort config.server order-sensitive files by name when written as a map + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + profiles: + 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 config.server file verbatim + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + profiles: | + [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 render config.session repos.conf in the order written + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + repos.conf: + - 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 + + # r-versions is DCF, not ini, so it has to be written as a string. See #948. + - it: should pass an r-versions string through unchanged + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + r-versions: | + Path: /opt/R/4.4.1 + Label: Latest + + Path: /opt/R/4.0.2 + Label: Old + asserts: + - equal: + path: data["r-versions"] + value: | + Path: /opt/R/4.4.1 + Label: Latest + + Path: /opt/R/4.0.2 + Label: Old + + - it: should render config.profiles sections in the order written, merging the default everyone section + template: configmap-general.yaml + documentIndex: 0 + set: + launcher: + useTemplates: false + config: + profiles: + launcher.kubernetes.profiles.conf: + - jsmith: + max-cpus: 8 + - "@analysts": + max-cpus: 4 + - "*": + max-cpus: 1 + asserts: + - matchRegex: + path: data["launcher.kubernetes.profiles.conf"] + pattern: "\\[jsmith\\]\\nmax-cpus=8\\n\\n\\[@analysts\\]\\nmax-cpus=4\\n\\n\\[\\*\\]\\n" + - matchRegex: + path: data["launcher.kubernetes.profiles.conf"] + pattern: "\\[\\*\\]\\nallow-unknown-images=1\\ncontainer-images=posit/workbench-session:.*\\ndefault-container-image=posit/workbench-session:.*\\njob-json-overrides=.*\\nmax-cpus=1" + + - it: should prepend an everyone section to ordered config.profiles that omits one + template: configmap-general.yaml + documentIndex: 0 + set: + launcher: + useTemplates: false + config: + profiles: + launcher.kubernetes.profiles.conf: + - jsmith: + max-cpus: 8 + asserts: + - matchRegex: + path: data["launcher.kubernetes.profiles.conf"] + pattern: "\\[\\*\\]\\nallow-unknown-images=1\\ncontainer-images=.*\\ndefault-container-image=.*\\njob-json-overrides=.*\\n\\n\\[jsmith\\]\\nmax-cpus=8" + + # -- config.session files are routed by format, not by value shape. See #948 + - it: should render r-versions as DCF when written as records + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + r-versions: + - 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 render notifications.conf as DCF + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + notifications.conf: + - StartTime: 2026-01-01 + EndTime: 2026-01-02 + Message: Maintenance window + asserts: + - matchRegex: + path: data["notifications.conf"] + pattern: "StartTime: 2026-01-01" + - notMatchRegex: + path: data["notifications.conf"] + pattern: "StartTime=" + + - it: should render a structured json session file as JSON + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + rstudio-prefs.json: + save_workspace: never + asserts: + - matchRegex: + path: data["rstudio-prefs.json"] + pattern: '"save_workspace": "never"' + + - it: should pass a json session file written as a string through unchanged + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + rstudio-prefs.json: | + {"save_workspace": "never"} + asserts: + - equal: + path: data["rstudio-prefs.json"] + value: | + {"save_workspace": "never"} + + - it: should still render repos.conf as ini + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + repos.conf: + - 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 + + # -- Guard: the README tells admins to write these files as a list, and Helm *replaces* a list + # rather than merging it. So none of them may gain a chart default without someone deciding what + # happens to admins who already use the list form. If this fails, that decision is now due. + # + # The file list below restates NOTES.txt's order-sensitive patterns, which is the source of truth + # — keep them in step. A *map* default would also be caught automatically by "should not warn + # about the map form on a default install" in notes_test.yaml; this catches a *list* default too, + # which warns about nothing and so would otherwise be silent. + - it: should ship no chart default for the files documented as lists + template: configmap-general.yaml + documentIndex: 0 + asserts: + - notExists: + path: data["profiles"] + - notExists: + path: data["launcher.kubernetes.resources.conf"] + - notExists: + path: data["launcher.local.resources.conf"] + - notExists: + path: data["launcher.slurm.resources.conf"] + + # config.session.repos.conf is the exception: it does ship a default, deliberately as a list so + # the map form warns. An admin's own value replaces it, which is why NOTES.txt guards for CRAN. + - it: should ship the repos.conf default as a list, not a map + template: configmap-session.yaml + documentIndex: 0 + asserts: + - equal: + path: data["repos.conf"] + value: | + CRAN=https://packagemanager.posit.co/cran/__linux__/jammy/latest + + # config.profiles does receive chart defaults, so the list form has to merge rather than replace. + # Covered by "should render config.profiles sections in the order written" above; this pins that + # the defaults exist, so that test cannot silently become vacuous. + - it: should ship chart defaults for config.profiles + template: configmap-general.yaml + documentIndex: 0 + asserts: + - matchRegex: + path: data["launcher.kubernetes.profiles.conf"] + pattern: "default-container-image=posit/workbench-session:" + + # -- Files the chart merges its own settings into must be maps + - it: should fail with a clear message when rserver.conf is a list + template: configmap-general.yaml + set: + config: + server: + rserver.conf: + - www-port: 8787 + asserts: + - failedTemplate: + errorPattern: "config.server.rserver.conf is a slice, but must be a map of options" + + - it: should fail with a clear message when rserver.conf is a string + template: configmap-general.yaml + set: + config: + server: + rserver.conf: | + www-port=8787 + asserts: + - failedTemplate: + errorPattern: "config.server.rserver.conf is a string, but must be a map of options" + + - it: should fail rather than drop the chart's [server] section when launcher.conf is a list + template: configmap-general.yaml + set: + config: + server: + launcher.conf: + - cluster: + name: Kubernetes + asserts: + - failedTemplate: + errorPattern: "config.server.launcher.conf is a slice(.|\\n)*A string is also accepted" + + - it: should fail rather than drop the chart's namespace when launcher.kubernetes.conf is a list + template: configmap-general.yaml + set: + config: + server: + launcher.kubernetes.conf: + - kubernetes-namespace: other + asserts: + - failedTemplate: + errorPattern: "config.server.launcher.kubernetes.conf is a slice" + + - it: should pass a launcher.conf string through + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + launcher.conf: | + [server] + address=127.0.0.1 + asserts: + - equal: + path: data["launcher.conf"] + value: | + [server] + address=127.0.0.1 + + - it: should report a non-map profiles list entry with the library's message + template: configmap-general.yaml + set: + config: + profiles: + launcher.kubernetes.profiles.conf: + - oops + asserts: + - failedTemplate: + errorPattern: "Every entry written as a list must be a map" + + # -- repos.conf must name CRAN; Workbench discards the whole file otherwise + - it: should fail when a repos.conf list has no CRAN entry + template: configmap-session.yaml + set: + config: + session: + repos.conf: + - Internal: https://pkgs.example.com/internal + asserts: + - failedTemplate: + errorPattern: "config.session.repos.conf has no entry named CRAN" + + - it: should render a repos.conf list with CRAN in the order written + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + repos.conf: + - Internal: https://pkgs.example.com/internal + - CRAN: https://mirror.example.com/cran + asserts: + - equal: + path: data["repos.conf"] + value: | + Internal=https://pkgs.example.com/internal + CRAN=https://mirror.example.com/cran + + - it: should not render repos.conf when it is null + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + repos.conf: null + asserts: + - notExists: + path: data["repos.conf"] + + # -- How several values are written follows the file's format in the table + - it: should comma-join a list of values in an ini_ptree file + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + logging.conf: + "*": + log-level: info + some-option: [a, b] + asserts: + - matchRegex: + path: data["logging.conf"] + pattern: "(?m)^some-option=a,b$" + + - it: should repeat the key for a list of values in an unrecognized file + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + custom.ini: + option: [a, b] + asserts: + - equal: + path: data["custom.ini"] + value: | + option=a + option=b + + - it: should render the deprecated server-scope launcher.kubernetes.profiles.conf in the order written + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + launcher.kubernetes.profiles.conf: + - "@analysts": + max-cpus: 4 + - "*": + max-cpus: 1 + container-images: [a, b] + asserts: + - equal: + path: data["launcher.kubernetes.profiles.conf"] + value: | + [@analysts] + max-cpus=4 + + [*] + container-images=a,b + max-cpus=1 + + # -- Every launcher..profiles.conf is the same file format, in either scope + - it: should render a non-kubernetes launcher profiles file in config.server like config.profiles + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + launcher.slurm.profiles.conf: + "*": + max-cpus: 1 + some-list: [a, b] + asserts: + - equal: + path: data["launcher.slurm.profiles.conf"] + value: | + [*] + max-cpus=1 + some-list=a,b + + # -- A string for a file the chart merges settings into replaces those settings. Pinned so the + # message and README, which say so, stay true. + - it: should let a launcher.kubernetes.conf string replace the chart's namespace setting + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + launcher.kubernetes.conf: | + use-templating=0 + asserts: + - equal: + path: data["launcher.kubernetes.conf"] + value: | + use-templating=0 + - notMatchRegex: + path: data["launcher.kubernetes.conf"] + pattern: "kubernetes-namespace" + + - it: should say what a string replaces when launcher.kubernetes.conf is a list + template: configmap-general.yaml + set: + config: + server: + launcher.kubernetes.conf: + - kubernetes-namespace: other + asserts: + - failedTemplate: + errorPattern: "A string is also accepted and is used as the whole file, so it replaces" + - failedTemplate: + errorPattern: "kubernetes-namespace and use-templating" + + - it: should not offer a string for rserver.conf + template: configmap-general.yaml + set: + config: + server: + rserver.conf: + - www-port: 8787 + asserts: + - failedTemplate: + errorPattern: "config.server.rserver.conf is a slice, but must be a map of options" + + # -- pip.conf is not a Workbench file; a repeated key makes pip refuse it + - it: should fail a list of values in pip.conf + template: configmap-session.yaml + set: + config: + session: + pip.conf: + global: + extra-index-url: [https://b/simple, https://c/simple] + asserts: + - failedTemplate: + errorPattern: "'extra-index-url' in section \\[global\\] of 'pip.conf' is a list of values" + - failedTemplate: + errorPattern: "Write the whole file as a string \\(pip.conf: \\|\\)" + + - it: should report a non-map everyone section in an ordered profiles file with the profiles message + template: configmap-general.yaml + set: + launcher: + useTemplates: false + config: + profiles: + launcher.kubernetes.profiles.conf: + - "*": oops + asserts: + - failedTemplate: + errorPattern: "\\[\\*\\] section must be a 'map' of config values. Instead got 'string' : 'oops'" + + - it: should render the default profiles when config.profiles is null + template: configmap-general.yaml + documentIndex: 0 + set: + config: + profiles: null + asserts: + - matchRegex: + path: data["launcher.kubernetes.profiles.conf"] + pattern: "(?m)^\\[\\*\\]$" + + - it: should report a bare scalar in a repos.conf list as a shape error, not a missing CRAN + template: configmap-session.yaml + set: + config: + session: + repos.conf: + - https://packagemanager.posit.co/cran/latest + asserts: + - failedTemplate: + errorPattern: "entry 1 of 'repos.conf' must be a map of a name and its value" + + - it: should fail a DCF list entry that is not a map + template: configmap-session.yaml + set: + config: + session: + r-versions: + - /opt/R/4.4.1 + asserts: + - failedTemplate: + errorPattern: "entry 1 of 'r-versions' must be a map of fields" + + - it: should render nothing for an empty list of values + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + profiles: + - "*": + container-images: [] + max-cpus: 1 + rserver.conf: + www-allow-origin: [] + asserts: + - equal: + path: data["profiles"] + value: | + [*] + max-cpus=1 + - notMatchRegex: + path: data["rserver.conf"] + pattern: "www-allow-origin" + + # -- The scopes that do not go through the file table keep main's repeated keys, except sssd + - it: should repeat the key for a list of values in config.secret, as before + template: configmap-secret.yaml + documentIndex: 0 + set: + config: + secret: + database.conf: + provider: postgresql + some-multi: [a, b] + asserts: + - matchRegex: + path: stringData["database.conf"] + pattern: "(?m)^some-multi=a\\nsome-multi=b$" diff --git a/charts/rstudio-workbench/tests/legacy_config_test.yaml b/charts/rstudio-workbench/tests/legacy_config_test.yaml new file mode 100644 index 00000000..541efa9b --- /dev/null +++ b/charts/rstudio-workbench/tests/legacy_config_test.yaml @@ -0,0 +1,213 @@ +suite: Workbench legacy config shapes +# Config written the way it was before the ordered list form existed (chart 0.22 and earlier) +# must render exactly as it did then. Each expected value below was rendered from `main` before +# #945/#953. The two known, intended differences are not pinned here: launcher.*.profiles.conf +# lost a leading blank line, and r-versions written as records became valid DCF (#948). +templates: + - configmap-general.yaml + - configmap-session.yaml +tests: + # rserver.conf is read by boost program_options, where www-allow-origin and server-add-header + # are multitoken: several values repeat the key, and a comma is part of the value. + - it: should repeat the key for a list of values in rserver.conf + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + rserver.conf: + www-enable-origin-check: 1 + www-allow-origin: [a.example.com, b.example.com] + server-add-header: ["X-Frame-Options: DENY", "X-Custom: 1"] + asserts: + - matchRegex: + path: data["rserver.conf"] + pattern: "(?m)^server-add-header=X-Frame-Options: DENY\\nserver-add-header=X-Custom: 1$" + - matchRegex: + path: data["rserver.conf"] + pattern: "(?m)^www-allow-origin=a.example.com\\nwww-allow-origin=b.example.com\\nwww-enable-origin-check=1$" + + # The two tests below pin the same URL: the first as values.yaml ships it, the second as the + # template adds it back (rstudio-workbench.config.defaultCran). Update both together. + - it: should render the chart's default CRAN entry + template: configmap-session.yaml + documentIndex: 0 + asserts: + - equal: + path: data["repos.conf"] + value: | + CRAN=https://packagemanager.posit.co/cran/__linux__/jammy/latest + + - it: should add the chart's CRAN entry to a repos.conf map without one + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + repos.conf: + Internal: https://pkgs.example.com/internal + asserts: + - equal: + path: data["repos.conf"] + value: | + CRAN=https://packagemanager.posit.co/cran/__linux__/jammy/latest + Internal=https://pkgs.example.com/internal + + - it: should add the CRAN entry in the second namespace's session ConfigMap too + template: configmap-session.yaml + documentIndex: 1 + set: + launcher: + namespace: sessions + config: + session: + repos.conf: + Internal: https://pkgs.example.com/internal + asserts: + - equal: + path: metadata.namespace + value: sessions + - equal: + path: data["repos.conf"] + value: | + CRAN=https://packagemanager.posit.co/cran/__linux__/jammy/latest + Internal=https://pkgs.example.com/internal + + - it: should keep the admin's own CRAN entry in a repos.conf map + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + repos.conf: + RSPM: https://packagemanager.example.com/cran/latest + CRAN: https://mirror.example.com/cran + asserts: + - equal: + path: data["repos.conf"] + value: | + CRAN=https://mirror.example.com/cran + RSPM=https://packagemanager.example.com/cran/latest + + - it: should render a profiles map sorted, as before + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + profiles: + "*": + session-limit: 5 + session-timeout-minutes: 60 + "@analysts": + session-limit: 10 + asserts: + - equal: + path: data["profiles"] + value: | + [*] + session-limit=5 + session-timeout-minutes=60 + + [@analysts] + session-limit=10 + + - it: should repeat a section for a list of maps in launcher.conf, merged with the defaults + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + launcher.conf: + cluster: + - name: Kubernetes + type: Kubernetes + - name: Local + type: Local + asserts: + - equal: + path: data["launcher.conf"] + value: | + [cluster] + name=Kubernetes + type=Kubernetes + + [cluster] + name=Local + type=Local + + [server] + address=127.0.0.1 + admin-group=rstudio-server + authorization-enabled=1 + enable-debug-logging=0 + port=5559 + server-user=rstudio-server + thread-pool-size=4 + + - it: should render a pip.conf map as before + template: configmap-session.yaml + documentIndex: 0 + set: + config: + session: + pip.conf: + "global": + index-url: https://packagemanager.posit.co/pypi/latest/simple + trusted-host: packagemanager.posit.co + asserts: + - equal: + path: data["pip.conf"] + value: | + [global] + index-url=https://packagemanager.posit.co/pypi/latest/simple + trusted-host=packagemanager.posit.co + + - it: should merge map-form server files with their chart defaults, as before + template: configmap-general.yaml + documentIndex: 0 + set: + config: + server: + logging.conf: + "*": + log-level: debug + "@rserver": + log-level: info + jupyter.conf: + jupyter-exe: /opt/python/bin/jupyter + vscode.conf: + enabled: 0 + launcher.kubernetes.resources.conf: + small: + name: Small + cpus: 1 + mem-mb: 512 + asserts: + - equal: + path: data["logging.conf"] + value: | + [*] + log-level=debug + logger-type=stderr + + [@rserver] + log-level=info + - equal: + path: data["jupyter.conf"] + value: | + default-session-cluster=Kubernetes + jupyter-exe=/opt/python/bin/jupyter + labs-enabled=1 + - equal: + path: data["vscode.conf"] + value: | + enabled=0 + session-timeout-kill-hours=12 + - equal: + path: data["launcher.kubernetes.resources.conf"] + value: | + [small] + cpus=1 + mem-mb=512 + name=Small diff --git a/charts/rstudio-workbench/tests/notes_test.yaml b/charts/rstudio-workbench/tests/notes_test.yaml index 0175f9a5..80aa7a56 100644 --- a/charts/rstudio-workbench/tests/notes_test.yaml +++ b/charts/rstudio-workbench/tests/notes_test.yaml @@ -51,3 +51,333 @@ tests: asserts: - failedTemplate: errorPattern: "session.image.tagPrefix.*has been removed.*session.image.os" + + # -- Order-sensitive config files written as maps. See https://github.com/rstudio/helm/issues/944 + - it: should warn when order-sensitive config files are written as maps + set: + config: + server: + profiles: + "*": + max-memory-mb: 1024 + "@analysts": + max-memory-mb: 4096 + launcher.kubernetes.resources.conf: + small: + cpus: 1 + large: + cpus: 8 + profiles: + launcher.kubernetes.profiles.conf: + "*": + max-cpus: 1 + jsmith: + max-cpus: 8 + session: + repos.conf: + CRAN: https://packagemanager.posit.co/cran/latest + Internal: https://pkgs.example.com/internal + asserts: + - matchRegexRaw: + pattern: "WARNING: the following configuration files are written as maps" + - matchRegexRaw: + pattern: "`\\.Values\\.config\\.server\\.profiles`" + - matchRegexRaw: + pattern: "`\\.Values\\.config\\.server\\.launcher\\\\\\.kubernetes\\\\\\.resources\\\\\\.conf`" + - matchRegexRaw: + pattern: "`\\.Values\\.config\\.profiles\\.launcher\\\\\\.kubernetes\\\\\\.profiles\\\\\\.conf`" + - matchRegexRaw: + pattern: "`\\.Values\\.config\\.session\\.repos\\\\\\.conf`" + + - it: should not warn when order-sensitive config files are written as lists + set: + config: + server: + profiles: + - "*": + max-memory-mb: 1024 + launcher.kubernetes.resources.conf: + - small: + cpus: 1 + profiles: + launcher.kubernetes.profiles.conf: + - "*": + max-cpus: 1 + session: + repos.conf: + - CRAN: https://packagemanager.posit.co/cran/latest + asserts: + - notMatchRegexRaw: + pattern: "WARNING: the following configuration files are written as maps" + + - it: should not warn when an order-sensitive config file is written as a raw string + set: + config: + server: + profiles: | + [*] + max-memory-mb=1024 + asserts: + - notMatchRegexRaw: + pattern: "WARNING: the following configuration files are written as maps" + + - it: should not warn for config files whose order does not matter + set: + config: + server: + rserver.conf: + www-port: 8787 + session: + rsession.conf: + session-timeout-minutes: 60 + asserts: + - notMatchRegexRaw: + pattern: "WARNING: the following configuration files are written as maps" + + - it: should warn for a single-entry map too, now that the gate is gone + set: + config: + server: + profiles: + "*": + max-memory-mb: 1024 + asserts: + - matchRegexRaw: + pattern: "WARNING: the following configuration files are written as maps" + + # Workbench discards repos.conf entirely when nothing is named CRAN. A map gets the chart's + # CRAN entry and a list fails in configmap-session.yaml, so only a string reaches this warning. + - it: should warn when a repos.conf string has no CRAN entry + set: + config: + session: + repos.conf: | + Internal=https://pkgs.example.com/internal + asserts: + - matchRegexRaw: + pattern: "has no `CRAN` entry" + - matchRegexRaw: + pattern: "Workbench ignores the whole file" + + - it: should not warn when a repos.conf list includes CRAN + set: + config: + session: + repos.conf: + - Internal: https://pkgs.example.com/internal + - CRAN: https://mirror.example.com/cran + asserts: + - notMatchRegexRaw: + pattern: "has no `CRAN` entry" + + - it: should not warn when a repos.conf string includes CRAN + set: + config: + session: + repos.conf: | + Internal=https://pkgs.example.com/internal + CRAN=https://mirror.example.com/cran + asserts: + - notMatchRegexRaw: + pattern: "has no `CRAN` entry" + + - it: should not warn about CRAN when repos.conf is suppressed + set: + config: + session: + repos.conf: null + asserts: + - notMatchRegexRaw: + pattern: "has no `CRAN` entry" + + - it: should note that the chart added CRAN to a repos.conf map without one + set: + config: + session: + repos.conf: + Internal: https://pkgs.example.com/internal + asserts: + - matchRegexRaw: + pattern: "written as maps" + - matchRegexRaw: + pattern: "has no `CRAN` entry, so the chart added its default one" + + - it: should not note an added CRAN when a repos.conf map has its own + set: + config: + session: + repos.conf: + Internal: https://pkgs.example.com/internal + CRAN: https://mirror.example.com/cran + asserts: + - notMatchRegexRaw: + pattern: "chart added its default one" + + # The deprecated server-scope location of launcher.kubernetes.profiles.conf is read in order too + - it: should warn when the server-scope launcher.kubernetes.profiles.conf is written as a map + set: + config: + server: + launcher.kubernetes.profiles.conf: + "*": + max-cpus: 1 + asserts: + - matchRegexRaw: + pattern: "written as maps(.|\\n)*config\\.server\\.launcher\\\\\\.kubernetes\\\\\\.profiles\\\\\\.conf" + + - it: should not say a list replaces defaults for the server-scope launcher.kubernetes.profiles.conf + set: + config: + server: + launcher.kubernetes.profiles.conf: + - "*": + max-cpus: 1 + asserts: + - notMatchRegexRaw: + pattern: "written as lists" + + - it: should not promise to remove the map form + set: + config: + server: + profiles: + "*": + max-memory-mb: 1024 + asserts: + - notMatchRegexRaw: + pattern: "will be removed" + + - it: should not warn about CRAN on a default install + asserts: + - notMatchRegexRaw: + pattern: "has no `CRAN` entry" + + # Derives from NOTES.txt's own list rather than restating it: if a map default is ever added for + # an order-sensitive file, a default install starts warning about the chart's own values. + - it: should not warn about the map form on a default install + asserts: + - notMatchRegexRaw: + pattern: "WARNING: the following configuration files are written as maps" + + # -- The inverse warning: an order-agnostic ini file written as a list replaces the chart's + # defaults for that file rather than merging with them. + - it: should warn when an order-agnostic ini file is written as a list + set: + config: + server: + logging.conf: + - "*": + log-level: warn + asserts: + - matchRegexRaw: + pattern: "WARNING: the following configuration files are written as lists" + - matchRegexRaw: + pattern: "`\\.Values\\.config\\.server\\.logging\\\\\\.conf`" + + - it: should not warn when an order-agnostic ini file is written as a map + set: + config: + server: + logging.conf: + "*": + log-level: warn + asserts: + - notMatchRegexRaw: + pattern: "written as lists" + + # r-versions and notifications.conf are DCF, and .json files are JSON; a list is how you + # legitimately write those, so the ini form warnings must not apply to them. + - it: should not warn about the list form for a DCF session file + set: + config: + session: + r-versions: + - Path: /opt/R/4.4.1 + Label: Latest + asserts: + - notMatchRegexRaw: + pattern: "written as lists" + + - it: should not warn about the list form for a json session file + set: + config: + session: + rstudio-prefs.json: + save_workspace: never + asserts: + - notMatchRegexRaw: + pattern: "written as lists" + + - it: should not warn about the list form on a default install + asserts: + - notMatchRegexRaw: + pattern: "written as lists" + + # -- A file matching no row in rstudio-workbench.config.fileTable still renders as ini, which is + # a guess; the chart says so rather than guessing quietly. + - it: should warn when a file the chart does not recognize is written as a map + set: + config: + session: + my-thing: + FOO: bar + asserts: + - matchRegexRaw: + pattern: "WARNING: the chart does not recognize the following configuration files" + - matchRegexRaw: + pattern: "`\\.Values\\.config\\.session\\.my-thing`" + + - it: should not warn when an unrecognized file is given as text + set: + config: + session: + my-thing: | + FOO=bar + asserts: + - notMatchRegexRaw: + pattern: "does not recognize the following" + + - it: should not warn for files the table does list + set: + config: + server: + otel.conf: + exporter: otlp + session: + Renviron.site: + TZ: UTC + r-versions: + - Path: /opt/R/4.4.1 + Label: Latest + asserts: + - notMatchRegexRaw: + pattern: "does not recognize the following" + + - it: should not warn about unrecognized files on a default install + asserts: + - notMatchRegexRaw: + pattern: "does not recognize the following" + + - it: should warn about the map form for a non-kubernetes launcher profiles file in config.server + set: + config: + server: + launcher.slurm.profiles.conf: + "*": + max-cpus: 1 + asserts: + - matchRegexRaw: + pattern: "written as maps(.|\\n)*config\\.server\\.launcher\\\\.slurm\\\\.profiles\\\\.conf" + - notMatchRegexRaw: + pattern: "written as lists" + + - it: should not warn about pip.conf written as a map + set: + config: + session: + pip.conf: + global: + index-url: https://a/simple + asserts: + - notMatchRegexRaw: + pattern: "WARNING: the (following configuration files|chart does not recognize)" diff --git a/charts/rstudio-workbench/values.yaml b/charts/rstudio-workbench/values.yaml index bd8e75e4..9372be27 100644 --- a/charts/rstudio-workbench/values.yaml +++ b/charts/rstudio-workbench/values.yaml @@ -579,9 +579,15 @@ config: value: "" # -- a map of session-scoped config files. Mounted to `/mnt/session-configmap/rstudio/` on both server and session, by default. + # Each file's contents may be a map, a raw string, or - for files read in order, such as `repos.conf` - a list of single-entry maps. See README for more information. session: + # Written as a list because repos.conf is read in order. A list of your own + # replaces this default entirely, and must include a CRAN entry - Workbench + # ignores the whole file without one, so the chart fails. A map of your own gets + # this CRAN entry added when it has none. The URL is repeated in the chart's + # rstudio-workbench.config.defaultCran template; change both together. repos.conf: - CRAN: https://packagemanager.posit.co/cran/__linux__/jammy/latest + - CRAN: https://packagemanager.posit.co/cran/__linux__/jammy/latest rsession.conf: {} notifications.conf: {} rstudio-prefs.json: | @@ -608,7 +614,8 @@ config: enabled: true # -- a map of sssd config files, mounted to `/etc/sssd/conf.d/` with 0600 permissions. Replaces the deprecated `config.userProvisioning`. conf: {} - # -- a map of server config files. Mounted to `/mnt/configmap/rstudio/` + # -- a map of server config files. Mounted to `/mnt/configmap/rstudio/`. + # Each file's contents may be a map, a raw string, or - for files read in order, such as `profiles` and `launcher.*.resources.conf` - a list of single-entry maps. See README for more information. # @default -- [RStudio Workbench Configuration Reference](https://docs.rstudio.com/ide/server-pro/rstudio_server_configuration/rstudio_server_configuration.html). See defaults with `helm show values` server: rserver.conf: @@ -695,7 +702,8 @@ config: # HELP license_days_left the number of days left on the license # TYPE license_days gauge license_days_left #license-days-left# - # -- a map of server-scoped config files (akin to `config.server`), but with specific behavior that supports profiles. See README for more information. + # -- a map of server-scoped config files (akin to `config.server`), but with specific behavior that supports profiles. + # `launcher.*.profiles.conf` is read in order, so write its sections as a list of single-entry maps. See README for more information. profiles: {} # -- a map of server-scoped config files (akin to `config.server`), but with .dcf file formatting (i.e. `launcher-mounts`, `launcher-env`, etc.) serverDcf: