hubcio commented on code in PR #4035: URL: https://github.com/apache/iggy/pull/4035#discussion_r3913835468
########## helm/charts/iggy/examples/cluster-3-node.yaml: ########## @@ -0,0 +1,106 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Shared roster for a three-node Iggy cluster. Install one release per node, +# overriding only the identity: +# +# for i in 0 1 2; do +# helm upgrade --install "iggy-n$i" ./helm/charts/iggy \ +# -f ./helm/charts/iggy/examples/cluster-3-node.yaml \ +# --set "server.cluster.selfReplicaId=$i" \ +# --set "server.nodeSelector.kubernetes\.io/hostname=node-$i" +# done +# +# Replace the `ip` values with the IPs of the nodes you pin each release to. +# The server binds this address verbatim for replica traffic, so with +# hostNetwork it must be the node's own IP. +# +# Every secret below has to be byte-identical on every node, so they live in one +# Secret that all three releases reference rather than in this file. Create it +# once, in the release namespace, before the first install: +# +# kubectl create secret generic iggy-cluster-secrets \ +# --from-literal=clusterSharedSecret="$(head -c 32 /dev/urandom | base64)" \ +# --from-literal=jwtEncodingSecret="$(head -c 32 /dev/urandom | base64)" \ +# --from-literal=jwtDecodingSecret="$(head -c 32 /dev/urandom | base64)" \ Review Comment: these two `head -c 32 /dev/urandom` calls give the jwt keys different values, so every token from `/users/login` fails with 401 - the server only warns about the mismatch. use one value for both, as the readme does. ########## helm/charts/iggy/templates/service.yaml: ########## @@ -32,11 +32,21 @@ spec: - name: quic port: {{ .Values.server.ports.quic }} targetPort: quic - protocol: TCP + protocol: UDP - name: tcp port: {{ .Values.server.ports.tcp }} targetPort: tcp protocol: TCP + - name: websocket + port: {{ .Values.server.ports.websocket }} + targetPort: websocket + protocol: TCP + {{- if .Values.server.cluster.enabled }} + - name: tcp-replica Review Comment: nothing dials this port through the service - replicas connect straight to the roster `ip:tcpReplica`. drop it, or a `LoadBalancer` service exposes the consensus port with auth off by default. ########## helm/charts/iggy/templates/deployment.yaml: ########## @@ -105,6 +145,12 @@ spec: - name: IGGY_NODE_ADVERTISED_ADDRESS value: {{ .Values.server.advertisedAddress | default (printf "%s.%s.svc.cluster.local" (include "iggy.fullname" .) .Release.Namespace) | quote }} Review Comment: in cluster mode the server ignores `IGGY_NODE_ADVERTISED_ADDRESS` and publishes the roster `ip` or `advertisedAddress`, so `server.advertisedAddress` silently does nothing there. skip this env when `cluster.enabled` and fail the render if `server.advertisedAddress` is set with it. ########## helm/charts/iggy/templates/deployment.yaml: ########## @@ -19,6 +19,16 @@ {{- if hasKey .Values.server "podSecurityContext" }} {{- fail "server.podSecurityContext has been moved to podSecurityContext (root level). Please update your values." }} {{- end }} + {{- if gt (int .Values.server.replicaCount) 1 }} + {{- fail "server.replicaCount > 1 is refused: the replicas share one PVC at one subPath and one advertised address, so they would run as independent servers writing the same data directory, and the server takes no lock against that. Run cluster mode instead: one release per node, each with server.cluster.enabled and its own server.cluster.selfReplicaId. See the Cluster Mode section of the chart README." }} + {{- end }} + {{- if .Values.autoscaling.enabled }} + {{- fail "autoscaling.enabled is refused: horizontally scaling the server Deployment produces the same shared-data-directory corruption as server.replicaCount > 1. Cluster size is a roster decision, not an HPA one." }} Review Comment: with this guard `hpa.yaml` can only render for a ui-only install, where it targets a server deployment that does not exist. delete it, the `autoscaling` values and the matching assert in `test-helm.sh`. ########## helm/charts/iggy/values.yaml: ########## @@ -100,6 +105,107 @@ server: # -- PVC storage size size: 8Gi + # -- Server-side encryption of message payloads and state commands, using + # AES-256-GCM. Every node of a cluster must hold the identical key, or it + # cannot read data another node wrote. + encryption: + # -- Enable encryption at rest + enabled: false + # -- 32-byte key, base64 encoded. Ignored when `existingSecret.name` is set. + # Prefer `existingSecret` outside development: a value here is stored in the + # Helm release and readable by anyone who can read it. + key: "" + existingSecret: + # -- Name of an existing Secret holding the encryption key + name: "" + # -- Key inside that Secret + key: encryptionKey + + # -- Secrets used to sign and validate HTTP bearer tokens. Left unset, each + # node generates a random secret on every start, which invalidates tokens + # across restarts and keeps them node-local. Setting the identical secret on + # every node makes bearers valid cluster-wide and activates follower to + # primary HTTP forwarding; `cluster.auth.enabled` derives the same thing from + # the replica PSK, so it is an alternative rather than an addition. + jwt: + # -- Encoding secret. Ignored when `existingSecret.name` is set. + encodingSecret: "" + # -- Decoding secret. Ignored when `existingSecret.name` is set. + decodingSecret: "" + existingSecret: + # -- Name of an existing Secret holding the JWT secrets + name: "" + # -- Key inside that Secret holding the encoding secret + encodingSecretKey: jwtEncodingSecret + # -- Key inside that Secret holding the decoding secret + decodingSecretKey: jwtDecodingSecret + + cluster: + # -- Enable cluster (VSR consensus) mode. One Helm release per node: every + # release shares the same `nodes` roster and overrides only `selfReplicaId`. + # See the Cluster Mode section of the chart README. + enabled: false + # -- Cluster name, byte-identical on every node. Hashed into the on-disk + # cluster id on first boot, so changing it later means starting from an + # empty data directory. + name: iggy-cluster + # -- Which `nodes` entry this release runs, matched against `replicaId`. + selfReplicaId: 0 + # -- Replica-to-replica authentication on the consensus port. When enabled + # every peer must complete an authenticated handshake or be rejected, and a + # shared secret becomes mandatory. Enabling it on a running cluster is a + # coordinated-restart change, not a rolling one. + auth: + # -- Require the authenticated replica handshake + enabled: false + # -- Cluster-wide pre-shared key, at least 32 bytes of CSPRNG output, + # byte-identical on every node. Ignored when `existingSecret.name` is set. + sharedSecret: "" + # -- Retiring key, accepted for verification only while a rotation is in + # flight. Leave empty outside a rotation. + previousSharedSecret: "" + existingSecret: + # -- Name of an existing Secret holding the pre-shared keys + name: "" + # -- Key inside that Secret holding the active shared secret + sharedSecretKey: clusterSharedSecret + # -- Key inside that Secret holding the retiring shared secret + previousSharedSecretKey: clusterPreviousSharedSecret + + # -- Cluster roster, mirroring the server's `[[cluster.nodes]]` config. + # Every node runs the identical list. `ip` is the replica-plane address: + # this node's consensus listener binds it verbatim and every peer dials it + # verbatim, so it must be a literal IP that the pod itself owns. With + # `server.hostNetwork` that is the node IP. `ports.tcpReplica` is required + # on every entry; the remaining ports default to `server.ports`. + nodes: [] + # nodes: + # - name: iggy-node-0 + # ip: 10.0.1.5 + # replicaId: 0 + # # -- Optional client-facing address, a literal IP or a DNS hostname. + # advertisedAddress: "" + # ports: + # tcp: 8090 + # quic: 8080 + # http: 3000 + # websocket: 8092 + # tcpReplica: 9090 + + # -- Run the server pod in the host network namespace. Required for cluster + # mode, where the replica listener binds the roster IP verbatim. + hostNetwork: false + + # -- Deployment update strategy. Empty lets the chart choose: `Recreate` when + # `hostNetwork` is set, because a rolling update would wait forever for a + # replacement pod that cannot bind host ports the outgoing pod still holds, + # and the Kubernetes default otherwise. + strategy: {} + + # -- Extra command-line arguments appended to the server entrypoint, e.g. + # `["--replica-id", "0"]` when running a cluster node. Review Comment: the chart already passes `--replica-id` when `cluster.enabled`, so this example makes clap exit with `cannot be used multiple times`, and outside cluster mode a non-zero id is refused anyway. use a different example flag. ########## helm/charts/iggy/values.yaml: ########## @@ -32,14 +32,19 @@ server: # -- Image pull policy pullPolicy: Always # -- Server image tag (overrides chart appVersion) - tag: "0.7.0" + tag: "0.9.0-edge.6" Review Comment: this duplicates `appVersion`, which the template already falls back to. if you drop it, `test-helm.sh` needs to read `Chart.yaml` instead, since it extracts this tag. ########## scripts/ci/test-helm.sh: ########## @@ -266,11 +259,57 @@ validate() { fi grep -q 'name: supersecret' "$HELM_RENDER_DIR/existing-secret.yaml" + helm template iggy "$CHART_DIR" \ Review Comment: this only renders the `existingSecret` path, so the chart-generated secret in `server-secrets.yaml` is never asserted. add a render with an inline `server.encryption.key` and check the secret and its `secretKeyRef`. ########## helm/charts/iggy/Chart.yaml: ########## @@ -20,8 +20,8 @@ apiVersion: v2 name: iggy description: A Helm chart for Apache Iggy server and web-ui type: application -version: 0.5.0 -appVersion: "0.7.0" +version: 0.6.0 +appVersion: "0.9.0-edge.6" Review Comment: `0.9.0-edge.6` was built from 996ac04c8, before `node.advertised_address` landed in #3923, so this image logs `IGGY_NODE_ADVERTISED_ADDRESS` as an unknown variable and the advertised-address handling is inert. pin the next edge tag or say so in the readme. ########## helm/charts/iggy/templates/server-secrets.yaml: ########## @@ -0,0 +1,44 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +{{- if and .Values.server.enabled (include "iggy.createsSecret" .) }} +apiVersion: v1 +kind: Secret +metadata: + name: {{ include "iggy.secretName" . }} + labels: + {{- include "iggy.labels" . | nindent 4 }} +type: Opaque +stringData: Review Comment: changing any of these through `helm upgrade` never restarts the pod, since the pod template only holds `secretKeyRef`s - the old secret stays live until a manual rollout restart. add a checksum annotation over this secret to the pod template. ########## helm/charts/iggy/examples/cluster-3-node.yaml: ########## @@ -0,0 +1,106 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Shared roster for a three-node Iggy cluster. Install one release per node, +# overriding only the identity: +# +# for i in 0 1 2; do +# helm upgrade --install "iggy-n$i" ./helm/charts/iggy \ +# -f ./helm/charts/iggy/examples/cluster-3-node.yaml \ +# --set "server.cluster.selfReplicaId=$i" \ +# --set "server.nodeSelector.kubernetes\.io/hostname=node-$i" +# done +# +# Replace the `ip` values with the IPs of the nodes you pin each release to. +# The server binds this address verbatim for replica traffic, so with +# hostNetwork it must be the node's own IP. +# +# Every secret below has to be byte-identical on every node, so they live in one +# Secret that all three releases reference rather than in this file. Create it +# once, in the release namespace, before the first install: +# +# kubectl create secret generic iggy-cluster-secrets \ +# --from-literal=clusterSharedSecret="$(head -c 32 /dev/urandom | base64)" \ +# --from-literal=jwtEncodingSecret="$(head -c 32 /dev/urandom | base64)" \ +# --from-literal=jwtDecodingSecret="$(head -c 32 /dev/urandom | base64)" \ +# --from-literal=encryptionKey="$(head -c 32 /dev/urandom | base64)" +# +# The JWT encoding and decoding secrets are an HMAC key pair, so they hold the +# same value; generate them once and reuse it if you set them by hand. + +ui: + enabled: false + +server: Review Comment: persistence is off here, so each node runs on `emptyDir` and every pod replacement wipes the replica, which then refetches everything from its peers. set `persistence.enabled: true` with a node-local storage class, since the pod is pinned. ########## helm/charts/iggy/README.md.gotmpl: ########## @@ -101,6 +101,142 @@ If Prometheus Operator is installed and you want monitoring, set `server.serviceMonitor.enabled=true` in `custom-values.yaml` or pass it on the command line with `--set server.serviceMonitor.enabled=true`. +## Cluster Mode + +The server runs a Viewstamped Replication cluster when `cluster.enabled` is set +and each node is told which roster entry it is. The chart models this as **one +release per node**: every release is handed the same roster and overrides only +its own identity. + +`helm/charts/iggy/examples/cluster-3-node.yaml` is a ready three-node roster. +Point the `ip` values at the nodes you will pin the releases to, then: + +```bash +for i in 0 1 2; do + helm upgrade --install "iggy-n$i" ./helm/charts/iggy \ + -f ./helm/charts/iggy/examples/cluster-3-node.yaml \ + --set "server.cluster.selfReplicaId=$i" \ + --set "server.nodeSelector.kubernetes\.io/hostname=node-$i" +done +``` + +The chart turns `server.cluster` into the server's `IGGY_CLUSTER_*` environment +variables and passes `--replica-id`, so no configuration file is mounted. The +values mirror the server's `[[cluster.nodes]]` config one for one. + +### Why hostNetwork and node pinning are required + +The server's consensus listener binds `cluster.nodes[*].ip` **verbatim**, unlike +the client listeners, which keep their own bind address and take only the port +from the roster. A pod therefore has to own the address its roster entry names. +Pod IPs are neither stable nor known before install, and a Service ClusterIP is +not an address a pod can bind, so the workable layout today is `hostNetwork: +true` with each release pinned to a node and its roster `ip` set to that node's +IP. Node IPs are known up front and survive a pod restart, which is what lets a +replica come back and rejoin. + +The cost is real: the server pod shares the node's network namespace and its +ports. Weigh it before running this in a shared cluster. + +If a release lands on a node whose IP is not the one in its roster entry, the +server refuses to start rather than misbehaving quietly: + +```text +Error: ShardJoinFailures { failures: [ShardJoinFailure { shard_id: 0, + kind: Error(Iggy(CannotBindToSocket("10.0.1.11:9090"))) }] } +``` + +Check the `nodeSelector` on that release against the `ip` in its roster entry. + +### Upgrades + +Host ports make a rolling update impossible: the replacement pod cannot bind +ports the outgoing pod still holds, so it stays `Pending` while the Deployment +waits for it to become ready. The chart therefore switches the server to the +`Recreate` strategy whenever `hostNetwork` is set, which takes the node down +for the length of the restart. Roll **one release at a time** and let the +cluster regain quorum before starting the next. + +Turning `hostNetwork` on for a release that already exists is rejected by the +API server, because a Deployment cannot move from `RollingUpdate` to `Recreate` +in place: + +```text +Deployment.apps "iggy-n0" is invalid: spec.strategy.rollingUpdate: Forbidden: + may not be specified when strategy `type` is 'Recreate' +``` + +Uninstall that release and install it again. Set `server.strategy` explicitly if Review Comment: a deployment can switch to `Recreate` in place - this only fails under helm 4 server-side apply, which cannot drop the server-defaulted `rollingUpdate`. `helm upgrade --server-side=false` does the switch fine on kind, so document that instead of uninstall. ########## helm/charts/iggy/templates/deployment.yaml: ########## @@ -55,16 +77,34 @@ spec: {{- toYaml .Values.securityContext | nindent 12 }} image: "{{ .Values.server.image.repository }}:{{ .Values.server.image.tag | default .Chart.AppVersion }}" imagePullPolicy: {{ .Values.server.image.pullPolicy }} + {{- if or .Values.server.cluster.enabled .Values.server.extraArgs }} + args: + {{- if .Values.server.cluster.enabled }} + - "--replica-id" + - {{ .Values.server.cluster.selfReplicaId | quote }} + {{- end }} + {{- range .Values.server.extraArgs }} + - {{ . | quote }} + {{- end }} + {{- end }} ports: - name: http containerPort: {{ .Values.server.ports.http }} protocol: TCP - name: tcp containerPort: {{ .Values.server.ports.tcp }} protocol: TCP + - name: websocket + containerPort: {{ .Values.server.ports.websocket }} + protocol: TCP - name: quic containerPort: {{ .Values.server.ports.quic }} + protocol: UDP + {{- if .Values.server.cluster.enabled }} + - name: tcp-replica + containerPort: {{ .Values.server.ports.tcpReplica }} Review Comment: in cluster mode these ports come from `server.ports`, not from this node's roster entry, so a node with other roster ports gets wrong container ports, probes and service targets. take them from the `selfReplicaId` entry instead. ########## helm/charts/iggy/examples/cluster-3-node.yaml: ########## @@ -0,0 +1,106 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Shared roster for a three-node Iggy cluster. Install one release per node, +# overriding only the identity: +# +# for i in 0 1 2; do +# helm upgrade --install "iggy-n$i" ./helm/charts/iggy \ +# -f ./helm/charts/iggy/examples/cluster-3-node.yaml \ +# --set "server.cluster.selfReplicaId=$i" \ +# --set "server.nodeSelector.kubernetes\.io/hostname=node-$i" +# done +# +# Replace the `ip` values with the IPs of the nodes you pin each release to. +# The server binds this address verbatim for replica traffic, so with +# hostNetwork it must be the node's own IP. +# +# Every secret below has to be byte-identical on every node, so they live in one +# Secret that all three releases reference rather than in this file. Create it +# once, in the release namespace, before the first install: +# +# kubectl create secret generic iggy-cluster-secrets \ Review Comment: root credentials must match on every release as well - each node creates root locally from its own `IGGY_ROOT_*` env, so a different password on one release only works on that node. add them to this secret or say so. ########## helm/charts/iggy/templates/_helpers.tpl: ########## @@ -97,3 +97,182 @@ Create the name of the service account to use {{- default "default" .Values.serviceAccount.name }} {{- end }} {{- end }} + +{{/* +Validate the cluster roster and fail the render with an actionable message. +Every check here is one the server would otherwise only reject at boot, after +the pod is already scheduled. +*/}} +{{- define "iggy.validateCluster" -}} + {{- $cluster := .Values.server.cluster }} + {{- if not $cluster.nodes }} + {{- fail "server.cluster.enabled is true but server.cluster.nodes is empty. Every node runs the identical roster; see the Cluster Mode section of the chart README." }} + {{- end }} + {{- $count := len $cluster.nodes }} + {{- $seen := dict }} + {{- range $cluster.nodes }} + {{- if not .name }} + {{- fail "every server.cluster.nodes entry needs a name" }} + {{- end }} + {{- if not .ip }} Review Comment: only emptiness is checked, so a hostname renders and the server dies at boot parsing `hostname:9090`. reject anything outside `[0-9a-fA-F.:]` and the wildcards `0.0.0.0` and `::`, keeping ipv6 allowed. ########## helm/charts/iggy/templates/_helpers.tpl: ########## @@ -97,3 +97,182 @@ Create the name of the service account to use {{- default "default" .Values.serviceAccount.name }} {{- end }} {{- end }} + +{{/* +Validate the cluster roster and fail the render with an actionable message. +Every check here is one the server would otherwise only reject at boot, after +the pod is already scheduled. +*/}} +{{- define "iggy.validateCluster" -}} + {{- $cluster := .Values.server.cluster }} + {{- if not $cluster.nodes }} Review Comment: a fresh cluster boot refuses to start without both `IGGY_ROOT_USERNAME` and `IGGY_ROOT_PASSWORD`, so `users.root.createSecret: false` with no `existingSecret` renders fine and dies at boot. fail here when neither is set. ########## helm/charts/iggy/templates/_helpers.tpl: ########## @@ -97,3 +97,182 @@ Create the name of the service account to use {{- default "default" .Values.serviceAccount.name }} {{- end }} {{- end }} + +{{/* +Validate the cluster roster and fail the render with an actionable message. +Every check here is one the server would otherwise only reject at boot, after +the pod is already scheduled. +*/}} +{{- define "iggy.validateCluster" -}} + {{- $cluster := .Values.server.cluster }} + {{- if not $cluster.nodes }} + {{- fail "server.cluster.enabled is true but server.cluster.nodes is empty. Every node runs the identical roster; see the Cluster Mode section of the chart README." }} + {{- end }} + {{- $count := len $cluster.nodes }} + {{- $seen := dict }} + {{- range $cluster.nodes }} + {{- if not .name }} + {{- fail "every server.cluster.nodes entry needs a name" }} + {{- end }} + {{- if not .ip }} + {{- fail (printf "server.cluster.nodes entry %q has no ip. The replica listener binds this address verbatim, so it must be a literal IP the pod owns." .name) }} + {{- end }} + {{- if kindIs "invalid" .replicaId }} + {{- fail (printf "server.cluster.nodes entry %q has no replicaId" .name) }} + {{- end }} + {{- $id := int .replicaId }} + {{- if or (lt $id 0) (ge $id $count) }} + {{- fail (printf "server.cluster.nodes entry %q has replicaId %d, which is outside 0..%d for a %d-node roster" .name $id (sub $count 1) $count) }} + {{- end }} + {{- if hasKey $seen (printf "%d" $id) }} + {{- fail (printf "server.cluster.nodes has two entries with replicaId %d; ids must be unique" $id) }} + {{- end }} + {{- $_ := set $seen (printf "%d" $id) .name }} + {{- if not (and .ports .ports.tcpReplica) }} + {{- fail (printf "server.cluster.nodes entry %q has no ports.tcpReplica. In cluster mode the server takes every listener port from the roster and refuses to start without it." .name) }} + {{- end }} + {{- end }} + {{- if not (hasKey $seen (printf "%d" (int $cluster.selfReplicaId))) }} + {{- fail (printf "server.cluster.selfReplicaId is %d but no server.cluster.nodes entry declares that replicaId. Each release picks its own identity out of the shared roster." (int $cluster.selfReplicaId)) }} + {{- end }} +{{- end }} + +{{/* +Render the roster as IGGY_CLUSTER_* environment variables. The server accepts +the whole cluster config this way, so the chart needs no config file mount. +*/}} +{{- define "iggy.clusterEnv" -}} + {{- $ports := .Values.server.ports }} +- name: IGGY_CLUSTER_ENABLED + value: "true" +- name: IGGY_CLUSTER_NAME + value: {{ .Values.server.cluster.name | quote }} + {{- range .Values.server.cluster.nodes }} + {{- $id := int .replicaId }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_NAME + value: {{ .name | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_IP + value: {{ .ip | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_REPLICA_ID + value: {{ $id | quote }} + {{- if .advertisedAddress }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_ADVERTISED_ADDRESS + value: {{ .advertisedAddress | quote }} + {{- end }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_TCP + value: {{ (default $ports.tcp (and .ports .ports.tcp)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_QUIC + value: {{ (default $ports.quic (and .ports .ports.quic)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_HTTP + value: {{ (default $ports.http (and .ports .ports.http)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_WEBSOCKET + value: {{ (default $ports.websocket (and .ports .ports.websocket)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_TCP_REPLICA + value: {{ .ports.tcpReplica | quote }} + {{- end }} +{{- end }} + +{{/* +Name of the Secret holding the cluster-wide secrets the chart generates from +inline values. Each of encryption, JWT and replica auth may instead point at a +Secret the operator made, which is the path a multi-node cluster wants: the +values have to be byte-identical on every node, so they belong in one object +every release references rather than in each release's values. +*/}} +{{- define "iggy.secretName" -}} + {{- printf "%s-secrets" (include "iggy.fullname" .) }} +{{- end }} + +{{/* +True when any secret has to be generated by the chart, i.e. an inline value is +set for a feature that is switched on and no existing Secret was named for it. +*/}} +{{- define "iggy.createsSecret" -}} + {{- $server := .Values.server }} + {{- $create := false }} + {{- if and $server.encryption.enabled (not $server.encryption.existingSecret.name) }} + {{- $create = true }} + {{- end }} + {{- if and (or $server.jwt.encodingSecret $server.jwt.decodingSecret) (not $server.jwt.existingSecret.name) }} + {{- $create = true }} + {{- end }} + {{- if and $server.cluster.auth.enabled (not $server.cluster.auth.existingSecret.name) }} + {{- $create = true }} + {{- end }} + {{- if $create }}true{{ end }} +{{- end }} + +{{/* +Validate the secret configuration. The server enforces all of this at boot, so +catching it during render only saves a scheduling round trip, but a cluster +whose PSK differs between nodes fails as a handshake rejection rather than as +anything that names the cause. +*/}} +{{- define "iggy.validateSecrets" -}} + {{- $server := .Values.server }} + {{- if $server.encryption.enabled }} + {{- if and (not $server.encryption.key) (not $server.encryption.existingSecret.name) }} + {{- fail "server.encryption.enabled is true but no key was given. Set server.encryption.key to a base64-encoded 32-byte key, or point server.encryption.existingSecret.name at a Secret holding one." }} + {{- end }} + {{- end }} + {{- if $server.cluster.auth.enabled }} + {{- if not $server.cluster.enabled }} + {{- fail "server.cluster.auth.enabled is true but server.cluster.enabled is false. Replica authentication only applies to the consensus port, which exists in cluster mode." }} + {{- end }} + {{- if and (not $server.cluster.auth.sharedSecret) (not $server.cluster.auth.existingSecret.name) }} + {{- fail "server.cluster.auth.enabled is true but no shared secret was given. Set server.cluster.auth.sharedSecret, or point server.cluster.auth.existingSecret.name at a Secret holding one. Every node needs the byte-identical value." }} + {{- end }} + {{- if and $server.cluster.auth.sharedSecret (lt (len $server.cluster.auth.sharedSecret) 32) }} Review Comment: a `previousSharedSecret` equal to `sharedSecret`, or shorter than 32 bytes, renders fine and the server rejects it at boot. mirror both checks here. ########## helm/charts/iggy/templates/_helpers.tpl: ########## @@ -97,3 +97,182 @@ Create the name of the service account to use {{- default "default" .Values.serviceAccount.name }} {{- end }} {{- end }} + +{{/* +Validate the cluster roster and fail the render with an actionable message. +Every check here is one the server would otherwise only reject at boot, after +the pod is already scheduled. +*/}} +{{- define "iggy.validateCluster" -}} + {{- $cluster := .Values.server.cluster }} + {{- if not $cluster.nodes }} + {{- fail "server.cluster.enabled is true but server.cluster.nodes is empty. Every node runs the identical roster; see the Cluster Mode section of the chart README." }} + {{- end }} + {{- $count := len $cluster.nodes }} + {{- $seen := dict }} + {{- range $cluster.nodes }} + {{- if not .name }} + {{- fail "every server.cluster.nodes entry needs a name" }} + {{- end }} + {{- if not .ip }} + {{- fail (printf "server.cluster.nodes entry %q has no ip. The replica listener binds this address verbatim, so it must be a literal IP the pod owns." .name) }} + {{- end }} + {{- if kindIs "invalid" .replicaId }} + {{- fail (printf "server.cluster.nodes entry %q has no replicaId" .name) }} + {{- end }} + {{- $id := int .replicaId }} + {{- if or (lt $id 0) (ge $id $count) }} + {{- fail (printf "server.cluster.nodes entry %q has replicaId %d, which is outside 0..%d for a %d-node roster" .name $id (sub $count 1) $count) }} + {{- end }} + {{- if hasKey $seen (printf "%d" $id) }} + {{- fail (printf "server.cluster.nodes has two entries with replicaId %d; ids must be unique" $id) }} + {{- end }} + {{- $_ := set $seen (printf "%d" $id) .name }} + {{- if not (and .ports .ports.tcpReplica) }} + {{- fail (printf "server.cluster.nodes entry %q has no ports.tcpReplica. In cluster mode the server takes every listener port from the roster and refuses to start without it." .name) }} + {{- end }} + {{- end }} + {{- if not (hasKey $seen (printf "%d" (int $cluster.selfReplicaId))) }} + {{- fail (printf "server.cluster.selfReplicaId is %d but no server.cluster.nodes entry declares that replicaId. Each release picks its own identity out of the shared roster." (int $cluster.selfReplicaId)) }} + {{- end }} +{{- end }} + +{{/* +Render the roster as IGGY_CLUSTER_* environment variables. The server accepts +the whole cluster config this way, so the chart needs no config file mount. +*/}} +{{- define "iggy.clusterEnv" -}} + {{- $ports := .Values.server.ports }} +- name: IGGY_CLUSTER_ENABLED + value: "true" +- name: IGGY_CLUSTER_NAME + value: {{ .Values.server.cluster.name | quote }} + {{- range .Values.server.cluster.nodes }} + {{- $id := int .replicaId }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_NAME + value: {{ .name | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_IP + value: {{ .ip | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_REPLICA_ID + value: {{ $id | quote }} + {{- if .advertisedAddress }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_ADVERTISED_ADDRESS + value: {{ .advertisedAddress | quote }} + {{- end }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_TCP + value: {{ (default $ports.tcp (and .ports .ports.tcp)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_QUIC + value: {{ (default $ports.quic (and .ports .ports.quic)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_HTTP + value: {{ (default $ports.http (and .ports .ports.http)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_WEBSOCKET + value: {{ (default $ports.websocket (and .ports .ports.websocket)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_TCP_REPLICA + value: {{ .ports.tcpReplica | quote }} + {{- end }} +{{- end }} + +{{/* +Name of the Secret holding the cluster-wide secrets the chart generates from +inline values. Each of encryption, JWT and replica auth may instead point at a +Secret the operator made, which is the path a multi-node cluster wants: the +values have to be byte-identical on every node, so they belong in one object +every release references rather than in each release's values. +*/}} +{{- define "iggy.secretName" -}} + {{- printf "%s-secrets" (include "iggy.fullname" .) }} +{{- end }} + +{{/* +True when any secret has to be generated by the chart, i.e. an inline value is +set for a feature that is switched on and no existing Secret was named for it. +*/}} +{{- define "iggy.createsSecret" -}} + {{- $server := .Values.server }} + {{- $create := false }} + {{- if and $server.encryption.enabled (not $server.encryption.existingSecret.name) }} + {{- $create = true }} + {{- end }} + {{- if and (or $server.jwt.encodingSecret $server.jwt.decodingSecret) (not $server.jwt.existingSecret.name) }} + {{- $create = true }} + {{- end }} + {{- if and $server.cluster.auth.enabled (not $server.cluster.auth.existingSecret.name) }} + {{- $create = true }} + {{- end }} + {{- if $create }}true{{ end }} +{{- end }} + +{{/* +Validate the secret configuration. The server enforces all of this at boot, so +catching it during render only saves a scheduling round trip, but a cluster +whose PSK differs between nodes fails as a handshake rejection rather than as +anything that names the cause. +*/}} +{{- define "iggy.validateSecrets" -}} + {{- $server := .Values.server }} + {{- if $server.encryption.enabled }} + {{- if and (not $server.encryption.key) (not $server.encryption.existingSecret.name) }} + {{- fail "server.encryption.enabled is true but no key was given. Set server.encryption.key to a base64-encoded 32-byte key, or point server.encryption.existingSecret.name at a Secret holding one." }} + {{- end }} + {{- end }} + {{- if $server.cluster.auth.enabled }} + {{- if not $server.cluster.enabled }} + {{- fail "server.cluster.auth.enabled is true but server.cluster.enabled is false. Replica authentication only applies to the consensus port, which exists in cluster mode." }} + {{- end }} + {{- if and (not $server.cluster.auth.sharedSecret) (not $server.cluster.auth.existingSecret.name) }} + {{- fail "server.cluster.auth.enabled is true but no shared secret was given. Set server.cluster.auth.sharedSecret, or point server.cluster.auth.existingSecret.name at a Secret holding one. Every node needs the byte-identical value." }} + {{- end }} + {{- if and $server.cluster.auth.sharedSecret (lt (len $server.cluster.auth.sharedSecret) 32) }} + {{- fail (printf "server.cluster.auth.sharedSecret is %d bytes; the server requires at least 32 bytes of CSPRNG output." (len $server.cluster.auth.sharedSecret)) }} + {{- end }} + {{- end }} +{{- end }} + +{{/* +Environment entries for the secret-backed settings, each sourced from whichever +Secret owns it. +*/}} +{{- define "iggy.secretEnv" -}} + {{- $server := .Values.server }} + {{- $generated := include "iggy.secretName" . }} + {{- if $server.encryption.enabled }} +- name: IGGY_SYSTEM_ENCRYPTION_ENABLED + value: "true" +- name: IGGY_SYSTEM_ENCRYPTION_KEY + valueFrom: + secretKeyRef: + name: {{ default $generated $server.encryption.existingSecret.name }} + key: {{ ternary $server.encryption.existingSecret.key "encryptionKey" (ne $server.encryption.existingSecret.name "") }} + {{- end }} + {{- if or $server.jwt.existingSecret.name $server.jwt.encodingSecret }} +- name: IGGY_HTTP_JWT_ENCODING_SECRET + valueFrom: + secretKeyRef: + name: {{ default $generated $server.jwt.existingSecret.name }} + key: {{ ternary $server.jwt.existingSecret.encodingSecretKey "jwtEncodingSecret" (ne $server.jwt.existingSecret.name "") }} Review Comment: with `existingSecret.name` set both refs are required, so a secret holding only one jwt key leaves the pod in `CreateContainerConfigError`, though the server mirrors a single secret itself. make the decoding ref `optional: true`. ########## helm/charts/iggy/templates/_helpers.tpl: ########## @@ -97,3 +97,182 @@ Create the name of the service account to use {{- default "default" .Values.serviceAccount.name }} {{- end }} {{- end }} + +{{/* +Validate the cluster roster and fail the render with an actionable message. +Every check here is one the server would otherwise only reject at boot, after +the pod is already scheduled. +*/}} +{{- define "iggy.validateCluster" -}} + {{- $cluster := .Values.server.cluster }} + {{- if not $cluster.nodes }} + {{- fail "server.cluster.enabled is true but server.cluster.nodes is empty. Every node runs the identical roster; see the Cluster Mode section of the chart README." }} + {{- end }} + {{- $count := len $cluster.nodes }} + {{- $seen := dict }} + {{- range $cluster.nodes }} + {{- if not .name }} + {{- fail "every server.cluster.nodes entry needs a name" }} + {{- end }} + {{- if not .ip }} + {{- fail (printf "server.cluster.nodes entry %q has no ip. The replica listener binds this address verbatim, so it must be a literal IP the pod owns." .name) }} + {{- end }} + {{- if kindIs "invalid" .replicaId }} + {{- fail (printf "server.cluster.nodes entry %q has no replicaId" .name) }} + {{- end }} + {{- $id := int .replicaId }} + {{- if or (lt $id 0) (ge $id $count) }} + {{- fail (printf "server.cluster.nodes entry %q has replicaId %d, which is outside 0..%d for a %d-node roster" .name $id (sub $count 1) $count) }} + {{- end }} + {{- if hasKey $seen (printf "%d" $id) }} + {{- fail (printf "server.cluster.nodes has two entries with replicaId %d; ids must be unique" $id) }} + {{- end }} + {{- $_ := set $seen (printf "%d" $id) .name }} + {{- if not (and .ports .ports.tcpReplica) }} + {{- fail (printf "server.cluster.nodes entry %q has no ports.tcpReplica. In cluster mode the server takes every listener port from the roster and refuses to start without it." .name) }} + {{- end }} + {{- end }} + {{- if not (hasKey $seen (printf "%d" (int $cluster.selfReplicaId))) }} + {{- fail (printf "server.cluster.selfReplicaId is %d but no server.cluster.nodes entry declares that replicaId. Each release picks its own identity out of the shared roster." (int $cluster.selfReplicaId)) }} + {{- end }} +{{- end }} + +{{/* +Render the roster as IGGY_CLUSTER_* environment variables. The server accepts +the whole cluster config this way, so the chart needs no config file mount. +*/}} +{{- define "iggy.clusterEnv" -}} + {{- $ports := .Values.server.ports }} +- name: IGGY_CLUSTER_ENABLED + value: "true" +- name: IGGY_CLUSTER_NAME + value: {{ .Values.server.cluster.name | quote }} + {{- range .Values.server.cluster.nodes }} + {{- $id := int .replicaId }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_NAME + value: {{ .name | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_IP + value: {{ .ip | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_REPLICA_ID + value: {{ $id | quote }} + {{- if .advertisedAddress }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_ADVERTISED_ADDRESS + value: {{ .advertisedAddress | quote }} + {{- end }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_TCP + value: {{ (default $ports.tcp (and .ports .ports.tcp)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_QUIC + value: {{ (default $ports.quic (and .ports .ports.quic)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_HTTP + value: {{ (default $ports.http (and .ports .ports.http)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_WEBSOCKET + value: {{ (default $ports.websocket (and .ports .ports.websocket)) | quote }} +- name: IGGY_CLUSTER_NODES_{{ $id }}_PORTS_TCP_REPLICA + value: {{ .ports.tcpReplica | quote }} + {{- end }} +{{- end }} + +{{/* +Name of the Secret holding the cluster-wide secrets the chart generates from +inline values. Each of encryption, JWT and replica auth may instead point at a +Secret the operator made, which is the path a multi-node cluster wants: the +values have to be byte-identical on every node, so they belong in one object +every release references rather than in each release's values. +*/}} +{{- define "iggy.secretName" -}} + {{- printf "%s-secrets" (include "iggy.fullname" .) }} +{{- end }} + +{{/* +True when any secret has to be generated by the chart, i.e. an inline value is +set for a feature that is switched on and no existing Secret was named for it. +*/}} +{{- define "iggy.createsSecret" -}} + {{- $server := .Values.server }} + {{- $create := false }} + {{- if and $server.encryption.enabled (not $server.encryption.existingSecret.name) }} + {{- $create = true }} + {{- end }} + {{- if and (or $server.jwt.encodingSecret $server.jwt.decodingSecret) (not $server.jwt.existingSecret.name) }} + {{- $create = true }} + {{- end }} + {{- if and $server.cluster.auth.enabled (not $server.cluster.auth.existingSecret.name) }} + {{- $create = true }} + {{- end }} + {{- if $create }}true{{ end }} +{{- end }} + +{{/* +Validate the secret configuration. The server enforces all of this at boot, so +catching it during render only saves a scheduling round trip, but a cluster +whose PSK differs between nodes fails as a handshake rejection rather than as +anything that names the cause. +*/}} +{{- define "iggy.validateSecrets" -}} + {{- $server := .Values.server }} + {{- if $server.encryption.enabled }} + {{- if and (not $server.encryption.key) (not $server.encryption.existingSecret.name) }} Review Comment: an inline key is not checked here, so a bad value renders and the server fails with `Invalid encryption key`. add `ne (len (b64dec $key)) 32` - sprig `b64dec` returns an error string on bad input, so length catches both. ########## helm/charts/iggy/templates/deployment.yaml: ########## @@ -19,6 +19,16 @@ {{- if hasKey .Values.server "podSecurityContext" }} {{- fail "server.podSecurityContext has been moved to podSecurityContext (root level). Please update your values." }} {{- end }} + {{- if gt (int .Values.server.replicaCount) 1 }} + {{- fail "server.replicaCount > 1 is refused: the replicas share one PVC at one subPath and one advertised address, so they would run as independent servers writing the same data directory, and the server takes no lock against that. Run cluster mode instead: one release per node, each with server.cluster.enabled and its own server.cluster.selfReplicaId. See the Cluster Mode section of the chart README." }} + {{- end }} + {{- if .Values.autoscaling.enabled }} + {{- fail "autoscaling.enabled is refused: horizontally scaling the server Deployment produces the same shared-data-directory corruption as server.replicaCount > 1. Cluster size is a roster decision, not an HPA one." }} + {{- end }} + {{- if .Values.server.cluster.enabled }} Review Comment: cluster mode renders fine with `hostNetwork: false` even though the values and readme call it required, and the pod then dies at boot with `CannotBindToSocket`. fail here unless an explicit opt-out is set. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
