Skip to content

fix: support extraVolumes/extraVolumeMounts, fix WH CA handling (OP-388) #2800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kristina-solovyova wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: support extraVolumes/extraVolumeMounts, fix WH CA handling (OP-388) #2800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .ainav/config/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ Key config categories:

Capacity-planner operator constraints — shared `capacityPlannerConstraints.*` (`maxCoresPerContainer`, `driveSharing.computeTo{Tlc,Qlc}DriveCoreRatio`, `fullDrives.computeToDriveCoreRatio`) and clusterCapacity-only `clusterCapacity.*` (`tlcCapacityPerCoreGiB`, `qlcCapacityPerCoreGiB`) — are documented in `doc/operator/deployment/cluster-capacity.md` (Helm constraints table).

`manager.extraVolumes`/`extraVolumeMounts` add volumes/mounts to the operator's own pod (reserved: `tmpdir` volume, `/tmp` mount, guarded via Helm `fail`).

## API Types (CRDs)

**Path**: `pkg/weka-k8s-api/api/v1alpha1/`
Expand All @@ -54,6 +56,10 @@ Capacity-planner operator constraints — shared `capacityPlannerConstraints.*`
| `metrics.go` | Metrics types |
| `condition/conditions.go` | Status conditions |

WekaCluster (`spec.podConfig`), WekaClient (`spec`), and WekaContainer (`spec`, propagated not
admission-validated) also carry `extraVolumes`/`extraVolumeMounts` for mounting extra volumes into
pods (e.g. a CA bundle) — see `doc/operator/deployment/extra-volumes.md`.

Generated docs: `doc/api_dump/*.md`

## Validation & Admission
Expand All @@ -78,6 +84,11 @@ spec field wins when non-zero (0 is treated as unset), else the Helm-level defau
`PROTECTION_REDUNDANCY_LEVEL` / `PROTECTION_HOT_SPARE`, values `protection.*`) fills it. Same helper
is used in `FormCluster` so validation and formation agree.

Extra-volumes validators (`extra_volumes.go` shared core + `cluster_extra_volumes.go` /
`client_extra_volumes.go`) reject reserved names/paths and malformed `extraVolumes` JSON;
`client_wekahome_cacert_unverifiable.go` warns when a Weka Home CA cert cannot be verified.
For the focused validator map, see [validation.md](validation.md).

## Adding Configuration

1. Add field to `internal/config/env.go`
Expand Down
24 changes: 24 additions & 0 deletions .ainav/config/validation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Validation & Admission

**Path**: `internal/validation/` + `internal/admission/`

Admission-webhook validators implement the `Validator` interface (`validator.go`), are
listed per-CRD in `registry.go`, and get a default severity in `admission/defaults.go`.
`doc.go` holds the sizing-mode glossary and the rule-ownership map — read it before adding a rule that
touches drive counts, container counts, or core sizing, so one condition is not reported twice.
Add a rule = implement + register + add to the defaults table. Reuse the shared helpers rather than
re-deriving: `role_specs.go` (`rolesForTemplate` — the six per-role sizing fields, one table for every
per-role validator), `template_cores.go` (`templateCoreSides` — drive/compute core totals plus the
planner-managed exclusion), `drive_role_nodes.go` (`listDriveRoleNodes` / `driveRoleNodeInfos`).
clusterCapacity validators:
`cluster_capacity_chunk_feasibility.go` (greenfield per-FD TLC share ≥ 384 GiB; skipped once the
cluster has TLC-bearing drive containers) and `cluster_capacity_protection.go` (min SW≥3, RL≥2, HS≥0 /
hotSpare optional — the `3+2+0` floor from `allocator.MinProtectionFloor`).
Protection values are resolved via `DriveSharingConfig.EffectiveProtection` (env.go): a per-cluster
spec field wins when non-zero (0 is treated as unset), else the Helm-level default (`PROTECTION_STRIPE_WIDTH` /
`PROTECTION_REDUNDANCY_LEVEL` / `PROTECTION_HOT_SPARE`, values `protection.*`) fills it. Same helper
is used in `FormCluster` so validation and formation agree.

Extra-volumes validators (`extra_volumes.go` shared core + `cluster_extra_volumes.go` /
`client_extra_volumes.go`) reject reserved names/paths and malformed `extraVolumes` JSON;
`client_wekahome_cacert_unverifiable.go` warns when a Weka Home CA cert cannot be verified.
16 changes: 16 additions & 0 deletions .ainav/controllers/wekaclient.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,19 @@ an `auto`/`dedicated`/`dedicated_ht` container therefore replaces the `CPUReques
capacity planner charges and the DRA claim is sized from — and if only one side of a pair is named
the other follows it, so QoS stays Guaranteed but the planner's node accounting drifts. That is
accepted: naming a resource here means taking responsibility for it.

## Weka Home CA cert

Clients phone Weka Home directly — they do not relay through a backend except as a fallback after
a direct attempt fails. The cluster-wide `weka_cloud_ca_cert_path` override replicates to joining
machines, but the certificate **file** does not, so a client of a private-CA cluster must carry it
locally or its telemetry breaks.

`domain.GetWekaHomeClientCacertSecret` (`internal/pkg/domain/wekahome.go`) resolves which Secret to
mount: client's own `spec.wekaHome.cacertSecret`, then the target cluster's own value (same
namespace only — a cross-namespace Secret name would not resolve, which raises a throttled
`WekaHomeCacertSecretNotInherited` warning event), then the operator-wide env default. The resolved name is carried on `UpdatableClientSpec`
so a change reaches already-running containers, not only ones created afterwards.

Clients are `--restricted` and can never set the override themselves; they only need the file
present. Full behaviour: `doc/operator/operations/weka-home-tls.md`.
9 changes: 9 additions & 0 deletions .ainav/controllers/wekacluster.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,12 @@ Docs: `doc/operator/deployment/act-as-daemonset.md`, `cluster-capacity.md`.
- Manages cluster-wide secrets and credentials
- Coordinates NFS/S3 protocol configuration
- Exposes management via K8s services/ingress

## Weka Home CA cert scope

`configureWekaHome` (`steps_post_cluster.go`) sets `weka_cloud_ca_cert_path` from a drive container.
That override is **cluster-wide**: it replicates to every joining machine, but the certificate file
does not, and an explicit CA replaces the OS trust store rather than adding to it. Setting
`spec.wekaHome.cacertSecret` therefore obliges every machine that joins — including clients this
operator does not manage — to place a PEM at `/opt/weka/k8s-runtime/vars/wh-cacert/cert.pem`. See
`doc/operator/operations/weka-home-tls.md`.
1 change: 1 addition & 0 deletions .ainav/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Entry point for AI navigation. Max 3 hops to any information.
| Controllers | [controllers/index.md](controllers/index.md) | Reconciliation logic, lifecycle management |
| Operations | [operations/index.md](operations/index.md) | Manual ops, policies, CSI, drivers |
| Config | [config/index.md](config/index.md) | Helm values, env vars, API types |
| Validation | [config/validation.md](config/validation.md) | Admission validators, severity defaults |
| Services | [services/index.md](services/index.md) | Weka API, K8s utils, node agent |
| Examples | [examples/index.md](examples/index.md) | YAML examples for clusters, clients, policies |
| **Tasks** | [tasks.md](tasks.md) | How to add/modify features |
Expand Down
20 changes: 17 additions & 3 deletions charts/weka-operator/resources/weka_runtime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3189,11 +3189,25 @@ async def configure_persistency():
mount --make-rshared /opt/weka/external-mounts/shared-netns
fi

if [ -f /var/run/secrets/weka-operator/wekahome-cacert/cert.pem ]; then
if [ -d /var/run/secrets/weka-operator/wekahome-cacert ]; then
rm -rf /opt/weka/k8s-runtime/vars/wh-cacert
mkdir -p /opt/weka/k8s-runtime/vars/wh-cacert/
cp /var/run/secrets/weka-operator/wekahome-cacert/cert.pem /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem
chmod 400 /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem
# Secret data-key names are arbitrary, so concatenate every mounted PEM rather
# than assuming one is named cert.pem (the glob skips the ..data/..2025_* dotfiles).
for f in /var/run/secrets/weka-operator/wekahome-cacert/*; do
[ -f "$f" ] || continue
cat "$f" >> /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem
echo "" >> /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem
done
# Test for actual PEM content, not file size: the separator above writes a newline
# per key, so a secret holding only empty or non-PEM values still yields a non-empty
# file. An explicit CA replaces the system trust store, so pointing
# weka_cloud_ca_cert_path at a certificate-less file breaks Weka Home silently.
if grep -q "BEGIN CERTIFICATE" /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem 2>/dev/null; then
chmod 400 /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem
else
rm -rf /opt/weka/k8s-runtime/vars/wh-cacert
fi
Comment on lines +3192 to +3210

@claude claude Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things here.

1. Concatenating every file in the secret dir will inline a private key if the user points cacertSecret at a kubernetes.io/tls secret. That's a very natural mistake — a TLS secret is the obvious thing to reach for, and it has tls.crt + tls.key. The BEGIN CERTIFICATE check passes (thanks to tls.crt), so tls.key lands verbatim inside /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem, which is then pointed at by weka_cloud_ca_cert_path. Worth filtering to certificate blocks, or at least skipping files containing a private key:

for f in /var/run/secrets/weka-operator/wekahome-cacert/*; do
    [ -f "$f" ] || continue
    grep -q "BEGIN CERTIFICATE" "$f" || continue
    grep -q "PRIVATE KEY" "$f" && continue
    cat "$f" >> /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem
    echo "" >> /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem
done

That also removes the need for the post-hoc content test, since a dir with no PEM certs produces no file at all.

2. chmod 400 happens only after the loop, so the file exists with the default umask (typically 0644) for the duration of the concatenation. Harmless for a public CA bundle, but combined with (1) it's a real window on a private key. Set the mode before writing:

: > /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem
chmod 400 /opt/weka/k8s-runtime/vars/wh-cacert/cert.pem

(then drop the trailing chmod, keeping only the rm -rf on the no-certs path).

Nit on the comment: "the glob skips the ..data/..2025_* dotfiles" — true, but the reason is that shell globs don't match leading dots, not the date; the ..2025_* naming will read as stale in a year. # the glob skips the secret's ..data/..timestamp dotfiles is enough.

fi

if [ -d /host-binds/shared-configs ]; then
Expand Down
12 changes: 12 additions & 0 deletions charts/weka-operator/templates/manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,18 @@ spec:
{{- with .Values.dnsPolicy }}
dnsPolicy: {{ .k8sNetwork | default "" }}
{{- end }}
{{- range .Values.manager.extraVolumes }}
{{- if eq .name "tmpdir" }}{{ fail "manager.extraVolumes: the volume name \"tmpdir\" is reserved by the operator" }}{{ end }}
{{- end }}
{{- range .Values.manager.extraVolumeMounts }}
{{- if eq .mountPath "/tmp" }}{{ fail "manager.extraVolumeMounts: the mount path \"/tmp\" is reserved by the operator" }}{{ end }}
{{- end }}
volumes:
Comment on lines +66 to 72

@claude claude Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /tmp guard is exact-match only, so mountPath: /tmp/foo (or /tmp/) sails through and shadows part of the operator's own tmpdir emptyDir. Same on-or-under rule the Go side uses (IsReservedMountPath) would be more consistent:

      {{- range .Values.manager.extraVolumeMounts }}
      {{- if or (eq .mountPath "/tmp") (hasPrefix "/tmp/" .mountPath) }}{{ fail "manager.extraVolumeMounts: the mount path \"/tmp\" is reserved by the operator" }}{{ end }}
      {{- end }}

Also nothing here catches duplicate names or duplicate mount paths within the user's own lists — Kubernetes will reject the Deployment, but with a much less obvious message than the two fails above. Low priority given the failure is at least loud.

- name: tmpdir
emptyDir: { }
{{- with .Values.manager.extraVolumes }}
{{- toYaml . | nindent 8 }}
{{- end }}
containers:
- args:
- --secure-listen-address=0.0.0.0:8443
Expand Down Expand Up @@ -518,6 +527,9 @@ spec:
volumeMounts:
- mountPath: /tmp
name: tmpdir
{{- with .Values.manager.extraVolumeMounts }}
{{- toYaml . | nindent 12 }}
{{- end }}
name: manager
securityContext:
allowPrivilegeEscalation: false
Expand Down
5 changes: 5 additions & 0 deletions charts/weka-operator/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ manager:
nodeSelector: {}
tolerations: []
labels: {}
# -- extra volumes added to the operator Deployment pod spec (standard PodSpec `volumes` shape).
# The reserved name is `tmpdir`.
extraVolumes: []
# -- extra volumeMounts for the `manager` container. The reserved path is `/tmp`.
extraVolumeMounts: []
loggerSettings:
level: 0
format: raw
Expand Down
70 changes: 61 additions & 9 deletions doc/api_dump/api-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1974,6 +1974,24 @@
"description": "affinity per container role\ntakes precedence over the `affinity` field",
"optional": true,
"pointer": true
},
{
"name": "ExtraVolumes",
"jsonName": "extraVolumes",
"type": "k8s:runtime.RawExtension",
"description": "extra volumes added to every weka pod of this cluster, in the same shape as a PodSpec's\n`volumes`. Names must not collide with operator-managed volumes; see\ndoc/operator/deployment/extra-volumes.md for the reserved names and paths.",
"optional": true,
"pointer": true
},
{
"name": "ExtraVolumeMounts",
"jsonName": "extraVolumeMounts",
"type": {
"type": "array",
"items": "k8s:v1.VolumeMount"
},
"description": "mounts for `extraVolumes`, applied to the weka container only (not init containers)",
"optional": true
}
],
"usedBy": [
Expand Down Expand Up @@ -3549,6 +3567,24 @@
},
"optional": true
},
{
"name": "ExtraVolumes",
"jsonName": "extraVolumes",
"type": "k8s:runtime.RawExtension",
"description": "extra volumes added to every client pod, in the same shape as a PodSpec's `volumes`.\nNames must not collide with operator-managed volumes; see\ndoc/operator/deployment/extra-volumes.md for the reserved names and paths.",
"optional": true,
"pointer": true
},
{
"name": "ExtraVolumeMounts",
"jsonName": "extraVolumeMounts",
"type": {
"type": "array",
"items": "k8s:v1.VolumeMount"
},
"description": "mounts for `extraVolumes`, applied to the weka container only (not init containers)",
"optional": true
},
{
"name": "WekaSecretRef",
"jsonName": "wekaSecretRef",
Expand Down Expand Up @@ -3696,15 +3732,6 @@
"optional": true,
"pointer": true
},
{
"name": "WekaHomeConfig",
"jsonName": "wekaHomeConfig",
"type": {
"$ref": "#/definitions/WekahomeClientConfig"
},
"description": "DEPRECATED, kept for compatibility with old API clients, not taking any action, to be removed on new API version",
"optional": true
},
{
"name": "WekaHome",
"jsonName": "wekaHome",
Expand Down Expand Up @@ -3906,6 +3933,13 @@
"$ref": "#/definitions/ClientPrinterColumns"
},
"optional": true
},
{
"name": "LastAppliedPodConfigHash",
"jsonName": "lastAppliedPodConfigHash",
"type": "string",
"description": "Pod config version this client has adopted. Mirrors the WekaCluster field: it gates\nthe first-deploy adoption that lets tracking start on pods predating the annotation\nwithout rolling them.",
"optional": true
}
],
"usedBy": [
Expand Down Expand Up @@ -5316,6 +5350,24 @@
},
"optional": true
},
{
"name": "ExtraVolumes",
"jsonName": "extraVolumes",
"type": "k8s:runtime.RawExtension",
"description": "extra volumes added to this pod, in the same shape as a PodSpec's `volumes`.\nPropagated from the owning WekaCluster/WekaClient; names must not collide with\noperator-managed volumes.",
"optional": true,
"pointer": true
},
{
"name": "ExtraVolumeMounts",
"jsonName": "extraVolumeMounts",
"type": {
"type": "array",
"items": "k8s:v1.VolumeMount"
},
"description": "mounts for `extraVolumes`, applied to the weka container only (not init containers)",
"optional": true
},
{
"name": "Instructions",
"jsonName": "instructions",
Expand Down
4 changes: 3 additions & 1 deletion doc/api_dump/wekaclient.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
| agentPort | int | if not set (0), weka will find a free port from the portRange |
| portRange | *PortRange | used for dynamic port allocation |
| nodeSelector | map[string]string | |
| extraVolumes | *runtime.RawExtension | extra volumes added to every client pod, in the same shape as a PodSpec's `volumes`.<br>Names must not collide with operator-managed volumes; see<br>doc/operator/deployment/extra-volumes.md for the reserved names and paths. |
| extraVolumeMounts | []v1.VolumeMount | mounts for `extraVolumes`, applied to the weka container only (not init containers) |
| wekaSecretRef | string | |
| network | Network | |
| driversDistService | string | |
Expand All @@ -64,7 +66,6 @@
| resources | *PodResourcesSpec | experimental: pod resources to be proxied as-is to the pod spec |
| hugepages | int | hugepages, value in megabytes |
| hugepagesOffset | *int | value in megabytes to offset |
| wekaHomeConfig | WekahomeClientConfig | DEPRECATED, kept for compatibility with old API clients, not taking any action, to be removed on new API version |
| wekaHome | *WekahomeClientConfig | |

@claude claude Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads-up on something riding along in the pkg/weka-k8s-api submodule bump rather than in this PR's own Go code: the deprecated wekaHomeConfig field is removed from WekaClientSpec (also visible in doc/api_dump/api-schema.json at the WekaHomeConfig/wekaHomeConfig deletion).

Its doc comment said "kept for compatibility with old API clients … to be removed on new API version" — but this removes it from v1alpha1, i.e. within the same API version. Consequences for anyone still setting it:

  • Plain kubectl apply / client-go: pruned silently (behaviour unchanged, since the field was already a no-op).
  • Server-side apply, and kubectl apply --validate=strict: hard rejection on the unknown field.
  • Any stored object still carrying it gets it pruned on next write.

If that's intentional and coordinated, fine — but it's an unrelated breaking API change in a PR titled fix:, so it deserves an explicit callout in the PR description / release notes rather than only showing up as a line deleted from a generated doc.

| upgradePolicy | UpgradePolicy | |
| allowHotUpgrade | bool | |
Expand All @@ -85,6 +86,7 @@
| status | WekaClientStatusEnum | |
| stats | *ClientMetrics | |
| printer | ClientPrinterColumns | |
| lastAppliedPodConfigHash | string | Pod config version this client has adopted. Mirrors the WekaCluster field: it gates<br>the first-deploy adoption that lets tracking start on pods predating the annotation<br>without rolling them. |

---

Expand Down
2 changes: 2 additions & 0 deletions doc/api_dump/wekacluster.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@
| roleTopologySpreadConstraints | *RoleTopologySpreadConstraints | takes precedence over the `topologySpreadConstraints` |
| affinity | *runtime.RawExtension | advanced scheduling constraints |
| roleAffinity | *RoleAffinity | affinity per container role<br>takes precedence over the `affinity` field |
| extraVolumes | *runtime.RawExtension | extra volumes added to every weka pod of this cluster, in the same shape as a PodSpec's<br>`volumes`. Names must not collide with operator-managed volumes; see<br>doc/operator/deployment/extra-volumes.md for the reserved names and paths. |
| extraVolumeMounts | []v1.VolumeMount | mounts for `extraVolumes`, applied to the weka container only (not init containers) |

---

Expand Down
2 changes: 2 additions & 0 deletions doc/api_dump/wekacontainer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@
| group | string | |
| serviceAccountName | string | |
| additionalSecrets | map[string]string | |
| extraVolumes | *runtime.RawExtension | extra volumes added to this pod, in the same shape as a PodSpec's `volumes`.<br>Propagated from the owning WekaCluster/WekaClient; names must not collide with<br>operator-managed volumes. |
| extraVolumeMounts | []v1.VolumeMount | mounts for `extraVolumes`, applied to the weka container only (not init containers) |
| instructions | *Instructions | |
| dropAffinityConstraints | bool | |
| uploadResultsTo | string | |
Expand Down
Loading
Loading