From a53adc4326f4dd88d98962c028c92f599f27e926 Mon Sep 17 00:00:00 2001 From: Andrey Zavilgelsky Date: Fri, 11 Sep 2026 14:57:37 +0300 Subject: [PATCH] feat(helm): allow a per-set sssd.conf for LoginSet and NodeSet `sssdConfRef` is a per-set field on both CRs, but the chart can only fill it from the cluster-wide `sssd.secretRef`: `slurm.sssdConf.name` reads `.Values.sssd.secretRef` and never sees the set it is rendering. Every LoginSet and NodeSet in a release therefore resolves identities from the same directory. That is limiting as soon as one release has to serve two directories: two LoginSets, or a LoginSet and a NodeSet, each resolving identities from its own domain. A single shared `sssd.conf` would give every set every domain, which is exactly what such a split is meant to avoid. The only workaround today is to patch `sssdConfRef` on the CR after install, and that patch is silently lost when the global `sssd.secretRef` changes, the CR is recreated, or the release is reinstalled. `loginsets..sssd.secretRef` and `nodesets..sssd.secretRef` now replace the cluster-wide reference as a whole: a set that names its own secret gets `key` from that same block, defaulting to `sssd.conf`, never inherited from the cluster-wide value. Composing a reference out of one field from each source would be a quiet way to read the wrong key of the right secret, so a `key` without a `name` fails the render instead. A set that says nothing keeps the cluster-wide value, which stays the default and the only value a release needs. Helm unit tests cover six cases per template: the cluster-wide value when the set declares none, the per-set value winning over it, the per-set value bringing its own key rather than inheriting one, a set that clears `sssd` with null, a set that gives a key without a name, and two sets in one release resolving from different secrets. Signed-off-by: Andrey Zavilgelsky Changelog: Added - `loginsets..sssd.secretRef` and `nodesets..sssd.secretRef` override the cluster-wide `sssd.secretRef`, so sets in one release can resolve identities from different directories --- helm/slurm/README.md | 6 +- .../slurm/templates/loginset/loginset-cr.yaml | 11 +- helm/slurm/templates/nodeset/nodeset-cr.yaml | 9 +- helm/slurm/tests/loginset_test.yaml | 103 +++++++++++++++ helm/slurm/tests/nodeset_test.yaml | 117 ++++++++++++++++++ helm/slurm/values.yaml | 16 +++ 6 files changed, 257 insertions(+), 5 deletions(-) diff --git a/helm/slurm/README.md b/helm/slurm/README.md index 005637945..4c6200517 100644 --- a/helm/slurm/README.md +++ b/helm/slurm/README.md @@ -105,7 +105,7 @@ Kubernetes: `>= 1.29.0-0` | jwtKey.annotations | object | `{}` | Annotations to add to the secret upon creation. | | jwtKey.create | bool | `true` | The secret will be created when true. | | jwtKey.secretRef | secretKeyRef | `{}` | Reference to the secret. | -| loginsetDefaults | object | `{"enabled":true,"extraSshdConfig":null,"initconf":{"image":{"digest":null,"repository":"docker.io/library/alpine","tag":"latest"},"resources":{}},"login":{"env":[],"image":{"digest":null,"repository":"ghcr.io/slinkyproject/login","tag":"26.05-ubuntu26.04"},"resources":{},"securityContext":{"privileged":false},"volumeMounts":[]},"metadata":{},"podSpec":{"affinity":{},"initContainers":[],"nodeSelector":{"kubernetes.io/os":"linux"},"resources":{},"tolerations":[],"volumes":[]},"replicas":1,"rootSshAuthorizedKeys":null,"service":{"metadata":{},"spec":{"type":"LoadBalancer"}},"strategy":{}}` | Defines defaults for the LoginSet map values. | +| loginsetDefaults | object | `{"enabled":true,"extraSshdConfig":null,"initconf":{"image":{"digest":null,"repository":"docker.io/library/alpine","tag":"latest"},"resources":{}},"login":{"env":[],"image":{"digest":null,"repository":"ghcr.io/slinkyproject/login","tag":"26.05-ubuntu26.04"},"resources":{},"securityContext":{"privileged":false},"volumeMounts":[]},"metadata":{},"podSpec":{"affinity":{},"initContainers":[],"nodeSelector":{"kubernetes.io/os":"linux"},"resources":{},"tolerations":[],"volumes":[]},"replicas":1,"rootSshAuthorizedKeys":null,"service":{"metadata":{},"spec":{"type":"LoadBalancer"}},"sssd":{"secretRef":{}},"strategy":{}}` | Defines defaults for the LoginSet map values. | | loginsetDefaults.enabled | bool | `true` | Enable use of this LoginSet. | | loginsetDefaults.extraSshdConfig | string | `nil` | Extra configuration lines appended to `/etc/ssh/sshd_config`. Ref: https://manpages.ubuntu.com/manpages/resolute/man5/sshd_config.5.html | | loginsetDefaults.initconf.image | string \| object | `{"digest":null,"repository":"docker.io/library/alpine","tag":"latest"}` | The image to use. Ref: https://kubernetes.io/docs/concepts/containers/images/#image-names | @@ -128,11 +128,12 @@ Kubernetes: `>= 1.29.0-0` | loginsetDefaults.service | object | `{"metadata":{},"spec":{"type":"LoadBalancer"}}` | The service configuration. | | loginsetDefaults.service.metadata | object | `{}` | Labels and annotations. Ref: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/ | | loginsetDefaults.service.spec | corev1.ServiceSpec | `{"type":"LoadBalancer"}` | Extend the service template, and/or override certain configurations. Ref: https://kubernetes.io/docs/concepts/services-networking/service/ | +| loginsetDefaults.sssd.secretRef | secretKeyRef | `{}` | Per-set `sssd.conf`, replacing the cluster-wide `sssd.secretRef` as a whole. Give it a `name`; `key` defaults to `sssd.conf` and is never inherited from the cluster-wide value. Use it when sets must resolve identities from different directories. | | loginsetDefaults.strategy | object | `{}` | Deployment strategy configuration. Ref: https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy | | loginsets | map[string]object | `{}` | Slurm LoginSet (sackd, sshd, sssd) configurations. | | nameOverride | string | `nil` | Overrides the name of the release. | | namespaceOverride | string | `nil` | Overrides the namespace of the release. | -| nodesetDefaults | object | `{"enabled":true,"extraConf":null,"extraConfMap":{},"logfile":{"image":{"digest":null,"repository":"docker.io/library/alpine","tag":"latest"},"resources":{}},"metadata":{},"ordinalPadding":0,"oversubscribeNode":false,"partition":{"config":null,"configMap":{},"enabled":false},"pinToNode":false,"podSpec":{"affinity":{},"initContainers":[],"nodeSelector":{"kubernetes.io/os":"linux"},"resources":{},"tolerations":[],"volumes":[]},"pruneSlurmNodeRecords":"Never","replicas":1,"scalingMode":"StatefulSet","slurmd":{"args":[],"env":[],"image":{"digest":null,"repository":"ghcr.io/slinkyproject/slurmd","tag":"26.05-ubuntu26.04"},"resources":{},"volumeMounts":[]},"ssh":{"enabled":false,"extraSshdConfig":null},"updateStrategy":{"rollingUpdate":{"maxUnavailable":"25%"},"scheduledUpdate":{},"type":"RollingUpdate"},"workloadDisruptionProtection":true}` | Defines defaults for the NodeSet map values. | +| nodesetDefaults | object | `{"enabled":true,"extraConf":null,"extraConfMap":{},"logfile":{"image":{"digest":null,"repository":"docker.io/library/alpine","tag":"latest"},"resources":{}},"metadata":{},"ordinalPadding":0,"oversubscribeNode":false,"partition":{"config":null,"configMap":{},"enabled":false},"pinToNode":false,"podSpec":{"affinity":{},"initContainers":[],"nodeSelector":{"kubernetes.io/os":"linux"},"resources":{},"tolerations":[],"volumes":[]},"pruneSlurmNodeRecords":"Never","replicas":1,"scalingMode":"StatefulSet","slurmd":{"args":[],"env":[],"image":{"digest":null,"repository":"ghcr.io/slinkyproject/slurmd","tag":"26.05-ubuntu26.04"},"resources":{},"volumeMounts":[]},"ssh":{"enabled":false,"extraSshdConfig":null},"sssd":{"secretRef":{}},"updateStrategy":{"rollingUpdate":{"maxUnavailable":"25%"},"scheduledUpdate":{},"type":"RollingUpdate"},"workloadDisruptionProtection":true}` | Defines defaults for the NodeSet map values. | | nodesetDefaults.enabled | bool | `true` | Enable use of this NodeSet. | | nodesetDefaults.extraConf | string | `nil` | Raw extra configuration added to the `--conf` argument. Ref: https://slurm.schedmd.com/slurmd.html#OPT_conf-%3Cnode-parameters%3E Ref: https://slurm.schedmd.com/slurm.conf.html#SECTION_NODE-CONFIGURATION | | nodesetDefaults.extraConfMap | map[string]string \| map[string][]string | `{}` | Extra configuration added to the `--conf` option. If `extraConf` is not empty, it takes precedence. Ref: https://slurm.schedmd.com/slurmd.html#OPT_conf-%3Cnode-parameters%3E Ref: https://slurm.schedmd.com/slurm.conf.html#SECTION_NODE-CONFIGURATION | @@ -162,6 +163,7 @@ Kubernetes: `>= 1.29.0-0` | nodesetDefaults.slurmd.volumeMounts | list | `[]` | List of volume mounts to use. Ref: https://kubernetes.io/docs/concepts/storage/volumes/ | | nodesetDefaults.ssh.enabled | bool | `false` | Enable SSH access to worker pods with pam_slurm_adopt. Ref: https://slurm.schedmd.com/pam_slurm_adopt.html | | nodesetDefaults.ssh.extraSshdConfig | string | `nil` | Extra configuration lines appended to `/etc/ssh/sshd_config`. Ref: https://manpages.ubuntu.com/manpages/resolute/man5/sshd_config.5.html | +| nodesetDefaults.sssd.secretRef | secretKeyRef | `{}` | Per-set `sssd.conf`, replacing the cluster-wide `sssd.secretRef` as a whole. Give it a `name`; `key` defaults to `sssd.conf` and is never inherited from the cluster-wide value. Use it when sets must resolve identities from different directories. | | nodesetDefaults.updateStrategy.rollingUpdate.maxUnavailable | string | `"25%"` | Maximum number of pods that can be unavailable during update. Can be an absolute number (ex: 5) or a percentage (ex: 25%). | | nodesetDefaults.updateStrategy.type | string | `"RollingUpdate"` | The strategy type. Can be one of: RollingUpdate; OnDelete, ScheduledUpdate. | | nodesetDefaults.workloadDisruptionProtection | bool | `true` | Use a Pod Disruption Budget to protect pods in this NodeSet when Slurm jobs are running on them Ref: https://kubernetes.io/docs/tasks/run-application/configure-pdb/ | diff --git a/helm/slurm/templates/loginset/loginset-cr.yaml b/helm/slurm/templates/loginset/loginset-cr.yaml index 81aab03e8..3d22e7bfa 100644 --- a/helm/slurm/templates/loginset/loginset-cr.yaml +++ b/helm/slurm/templates/loginset/loginset-cr.yaml @@ -32,9 +32,16 @@ spec: extraSshdConfig: | {{- . | nindent 4 }} {{- end }}{{- /* with $loginset.extraSshdConfig */}} + {{- $sssdRef := ($loginset.sssd | default dict).secretRef | default dict }} + {{- if and $sssdRef.key (not $sssdRef.name) }} + {{- fail (printf "loginset `%s` sets sssd.secretRef.key without sssd.secretRef.name." $key) }} + {{- end }} + {{- if not $sssdRef.name }} + {{- $sssdRef = dict "name" (include "slurm.sssdConf.name" $) "key" (include "slurm.sssdConf.key" $) }} + {{- end }} sssdConfRef: - name: {{ include "slurm.sssdConf.name" $ }} - key: {{ include "slurm.sssdConf.key" $ }} + name: {{ $sssdRef.name }} + key: {{ $sssdRef.key | default "sssd.conf" }} {{- with $loginset.rootSshAuthorizedKeys }} rootSshAuthorizedKeys: | {{- . | nindent 4 }} diff --git a/helm/slurm/templates/nodeset/nodeset-cr.yaml b/helm/slurm/templates/nodeset/nodeset-cr.yaml index a628823b3..8ced5e71b 100644 --- a/helm/slurm/templates/nodeset/nodeset-cr.yaml +++ b/helm/slurm/templates/nodeset/nodeset-cr.yaml @@ -42,7 +42,14 @@ spec: {{- end }}{{- /* with $nodeset.partition */}} {{- with $nodeset.ssh }} {{- if .enabled }} - {{- $_ := set $nodeset.ssh "sssdConfRef" (dict "name" (include "slurm.sssdConf.name" $) "key" (include "slurm.sssdConf.key" $)) }} + {{- $sssdRef := ($nodeset.sssd | default dict).secretRef | default dict }} + {{- if and $sssdRef.key (not $sssdRef.name) }} + {{- fail (printf "nodeset `%s` sets sssd.secretRef.key without sssd.secretRef.name." $key) }} + {{- end }} + {{- if not $sssdRef.name }} + {{- $sssdRef = dict "name" (include "slurm.sssdConf.name" $) "key" (include "slurm.sssdConf.key" $) }} + {{- end }} + {{- $_ := set $nodeset.ssh "sssdConfRef" (dict "name" $sssdRef.name "key" ($sssdRef.key | default "sssd.conf")) }} ssh: {{- toYaml . | nindent 4 }} {{- end }}{{- /* if .enabled */}} diff --git a/helm/slurm/tests/loginset_test.yaml b/helm/slurm/tests/loginset_test.yaml index ee3f095c0..64681f2d4 100644 --- a/helm/slurm/tests/loginset_test.yaml +++ b/helm/slurm/tests/loginset_test.yaml @@ -283,3 +283,106 @@ tests: - equal: path: spec.initconf.image value: registry.example.com/org/initconf@sha256:abcdef0123456789 + - it: should use the cluster-wide sssd secretRef when the set has none + set: + sssd: + secretRef: + name: global-sssd + key: sssd.conf + loginsets: + slinky: + enabled: true + asserts: + - equal: + path: spec.sssdConfRef.name + value: global-sssd + - equal: + path: spec.sssdConfRef.key + value: sssd.conf + - it: should prefer the per-loginset sssd secretRef over the cluster-wide one + set: + sssd: + secretRef: + name: global-sssd + key: sssd.conf + loginsets: + slinky: + enabled: true + sssd: + secretRef: + name: my-sssd-secret + key: my-sssd.conf + asserts: + - equal: + path: spec.sssdConfRef.name + value: my-sssd-secret + - equal: + path: spec.sssdConfRef.key + value: my-sssd.conf + - it: should take the whole reference from the set, not one field from each + set: + sssd: + secretRef: + name: global-sssd + key: global.conf + loginsets: + slinky: + enabled: true + sssd: + secretRef: + name: my-sssd-secret + asserts: + - equal: + path: spec.sssdConfRef.name + value: my-sssd-secret + - equal: + path: spec.sssdConfRef.key + value: sssd.conf + - it: should fail when the set gives a secret key without a name + set: + loginsets: + slinky: + enabled: true + sssd: + secretRef: + key: my-sssd.conf + asserts: + - failedTemplate: + errorMessage: "loginset `slinky` sets sssd.secretRef.key without sssd.secretRef.name." + - it: should fall back to the cluster-wide sssd secretRef when the set clears it + set: + sssd: + secretRef: + name: global-sssd + key: global.conf + loginsets: + slinky: + enabled: true + sssd: null + asserts: + - equal: + path: spec.sssdConfRef.name + value: global-sssd + - equal: + path: spec.sssdConfRef.key + value: global.conf + - it: should give each loginset its own sssd secretRef + set: + loginsets: + slinky: + enabled: true + slinky2: + enabled: true + sssd: + secretRef: + name: my-sssd-secret + key: my-sssd.conf + asserts: + - equal: + path: spec.sssdConfRef.name + value: test-release-slurm-sssd-conf + documentIndex: 0 + - equal: + path: spec.sssdConfRef.name + value: my-sssd-secret + documentIndex: 1 diff --git a/helm/slurm/tests/nodeset_test.yaml b/helm/slurm/tests/nodeset_test.yaml index 5f2fbfbc8..7ed5ad30d 100644 --- a/helm/slurm/tests/nodeset_test.yaml +++ b/helm/slurm/tests/nodeset_test.yaml @@ -253,3 +253,120 @@ tests: - equal: path: spec.logfile.image value: registry.example.com/org/logfile@sha256:abcdef0123456789 + - it: should use the cluster-wide sssd secretRef when the set has none + set: + sssd: + secretRef: + name: global-sssd + key: sssd.conf + nodesets: + slinky: + enabled: true + ssh: + enabled: true + asserts: + - equal: + path: spec.ssh.sssdConfRef.name + value: global-sssd + - equal: + path: spec.ssh.sssdConfRef.key + value: sssd.conf + - it: should prefer the per-nodeset sssd secretRef over the cluster-wide one + set: + sssd: + secretRef: + name: global-sssd + key: sssd.conf + nodesets: + slinky: + enabled: true + ssh: + enabled: true + sssd: + secretRef: + name: my-sssd-secret + key: my-sssd.conf + asserts: + - equal: + path: spec.ssh.sssdConfRef.name + value: my-sssd-secret + - equal: + path: spec.ssh.sssdConfRef.key + value: my-sssd.conf + - it: should take the whole reference from the set, not one field from each + set: + sssd: + secretRef: + name: global-sssd + key: global.conf + nodesets: + slinky: + enabled: true + ssh: + enabled: true + sssd: + secretRef: + name: my-sssd-secret + asserts: + - equal: + path: spec.ssh.sssdConfRef.name + value: my-sssd-secret + - equal: + path: spec.ssh.sssdConfRef.key + value: sssd.conf + - it: should fail when the set gives a secret key without a name + set: + nodesets: + slinky: + enabled: true + ssh: + enabled: true + sssd: + secretRef: + key: my-sssd.conf + asserts: + - failedTemplate: + errorMessage: "nodeset `slinky` sets sssd.secretRef.key without sssd.secretRef.name." + - it: should fall back to the cluster-wide sssd secretRef when the set clears it + set: + sssd: + secretRef: + name: global-sssd + key: global.conf + nodesets: + slinky: + enabled: true + ssh: + enabled: true + sssd: null + asserts: + - equal: + path: spec.ssh.sssdConfRef.name + value: global-sssd + - equal: + path: spec.ssh.sssdConfRef.key + value: global.conf + - it: should give each nodeset its own sssd secretRef + set: + nodesets: + slinky: + enabled: true + ssh: + enabled: true + slinky2: + enabled: true + ssh: + enabled: true + sssd: + secretRef: + name: my-sssd-secret + key: my-sssd.conf + asserts: + - equal: + path: spec.ssh.sssdConfRef.name + value: test-release-slurm-sssd-conf + documentIndex: 0 + - equal: + path: spec.ssh.sssdConfRef.name + value: my-sssd-secret + documentIndex: 1 diff --git a/helm/slurm/values.yaml b/helm/slurm/values.yaml index 8a6823349..bc533bdbe 100644 --- a/helm/slurm/values.yaml +++ b/helm/slurm/values.yaml @@ -600,6 +600,14 @@ sssd: # -- (object) Defines defaults for the LoginSet map values. loginsetDefaults: + sssd: + # -- (secretKeyRef) Per-set `sssd.conf`, replacing the cluster-wide + # `sssd.secretRef` as a whole. Give it a `name`; `key` defaults to + # `sssd.conf` and is never inherited from the cluster-wide value. Use it + # when sets must resolve identities from different directories. + secretRef: {} + # name: my-sssd-secret + # key: sssd.conf # -- Enable use of this LoginSet. enabled: true # -- Number of replicas to deploy. @@ -719,6 +727,14 @@ loginsets: {} # -- (object) Defines defaults for the NodeSet map values. nodesetDefaults: + sssd: + # -- (secretKeyRef) Per-set `sssd.conf`, replacing the cluster-wide + # `sssd.secretRef` as a whole. Give it a `name`; `key` defaults to + # `sssd.conf` and is never inherited from the cluster-wide value. Use it + # when sets must resolve identities from different directories. + secretRef: {} + # name: my-sssd-secret + # key: sssd.conf # -- Enable use of this NodeSet. enabled: true # -- Scaling mode: "StatefulSet" (fixed replica count) or "DaemonSet" (one pod per matching node).