Merge pull request #17 from aehrc/fix/guard-scaled-readonly-override

Author: edeati

charts/ontoserver/README.md

@@ -83,7 +83,8 @@ The chart supports four deployment combinations controlled by `ontoserver.deploy
 > on any scaled deployment**: setting `ontoserver.deployment.scaled` makes the server reject writes
 > regardless of what else is configured, so a scaled read-write server is not a topology that exists.
 > The chart therefore refuses to render `type: scaled` with `isReadOnly: false` rather than deploy
-> something that cannot work.
+> something that cannot work, and rejects any `ontoserver.config` or `ontoserver.secretConfig` entry
+> that attempts to re-enable writes on a scaled deployment.
 >
 > Load content with a **single-instance read-write** deployment, then publish it to a syndication
 > server and serve it from a **scaled read-only** cluster. See

charts/ontoserver/templates/validate-values.yaml

@@ -124,6 +124,28 @@
   {{- fail "Scaled deployments must set ontoserver.deployment.isReadOnly: true. Ontoserver forces read-only on a scaled deployment, and each replica has its own Lucene index, so content written through the round-robin Service is only indexed on the replica that served the write and $expand/$validate-code fail on the others. Load content with a single-instance read-write deployment, then serve it scaled and read-only." }}
 {{- end }}
 
+{{- /* The chart sets ontoserver.internal.deployment.scaledReadOnly itself on scaled deployments.
+     The config passthroughs render after that entry in the container's env list, and a later
+     duplicate wins, so a passthrough copy would silently override the chart and re-enable the
+     unsupported scaled read-write topology while still passing the isReadOnly guard above.
+     Reject it instead of rendering a manifest that contradicts itself.
+
+     Matching is normalised because Spring resolves a property from the environment in several
+     forms: the dotted name, an older all-lowercase spelling of it, and the uppercase underscored
+     form. Lowercasing and folding _ and - to . collapses all of them to one comparison.
+
+     Not checked, because it cannot be: existingSecretConfig and externalSecret contents are
+     opaque at render time. They arrive via envFrom, which Kubernetes applies *before* the
+     container's own env, so the chart's entry still wins there. */ -}}
+{{- $reserved := "ontoserver.internal.deployment.scaledreadonly" }}
+{{- range $section, $entries := dict "ontoserver.config" (.Values.ontoserver.config | default dict) "ontoserver.secretConfig" (.Values.ontoserver.secretConfig | default dict) }}
+  {{- range $key, $value := $entries }}
+    {{- if eq ($key | lower | replace "_" "." | replace "-" ".") $reserved }}
+      {{- fail (printf "%s must not set %s. Scaled read-write is not supported: Ontoserver forces read-only on a scaled deployment and the chart sets this itself, so a passthrough copy would only produce a manifest whose env list contradicts itself. Remove the entry. To serve content read-write, use a single-instance deployment." $section $key) }}
+    {{- end }}
+  {{- end }}
+{{- end }}
+
 {{- if and (eq .Values.ontoserver.deployment.kind "StatefulSet") .Values.ontoserver.deployment.persistence.files.existingVolume.enabled }}
   {{- fail "StatefulSet deployments do not support ontoserver.deployment.persistence.files.existingVolume. Each replica needs its own volume; use dynamic provisioning via storageClass or a Deployment for single-instance prebound volumes." }}
 {{- end }}

charts/ontoserver/tests/validate_values_test.yaml

@@ -204,6 +204,59 @@ tests:
       - failedTemplate:
           errorMessage: "Scaled deployments must set ontoserver.deployment.isReadOnly: true. Ontoserver forces read-only on a scaled deployment, and each replica has its own Lucene index, so content written through the round-robin Service is only indexed on the replica that served the write and $expand/$validate-code fail on the others. Load content with a single-instance read-write deployment, then serve it scaled and read-only."
 
+  # The chart sets scaledReadOnly itself, and the config passthroughs render after it, so a
+  # duplicate would win at runtime and re-enable scaled read-write while still passing the
+  # isReadOnly guard. Each accepted spelling must be rejected.
+  - it: rejects a scaledReadOnly override in ontoserver.config
+    values:
+      - fixtures/scaled-values.yaml
+    set:
+      ontoserver.config:
+        ontoserver.internal.deployment.scaledReadOnly: "false"
+    asserts:
+      - failedTemplate:
+          errorPattern: "ontoserver\\.config must not set .*Scaled read-write is not supported"
+
+  - it: rejects the older lowercase scaledReadonly spelling
+    values:
+      - fixtures/scaled-values.yaml
+    set:
+      ontoserver.config:
+        ontoserver.internal.deployment.scaledReadonly: "false"
+    asserts:
+      - failedTemplate:
+          errorPattern: "must not set .*Scaled read-write is not supported"
+
+  - it: rejects the uppercase underscored environment form
+    values:
+      - fixtures/scaled-values.yaml
+    set:
+      ontoserver.config:
+        ONTOSERVER_INTERNAL_DEPLOYMENT_SCALEDREADONLY: "false"
+    asserts:
+      - failedTemplate:
+          errorPattern: "must not set ONTOSERVER_INTERNAL_DEPLOYMENT_SCALEDREADONLY"
+
+  - it: rejects a scaledReadOnly override in ontoserver.secretConfig
+    values:
+      - fixtures/scaled-values.yaml
+    set:
+      ontoserver.secretConfig:
+        ontoserver.internal.deployment.scaledReadOnly: "false"
+    asserts:
+      - failedTemplate:
+          errorPattern: "ontoserver\\.secretConfig must not set"
+
+  # The guard must not catch unrelated passthrough entries.
+  - it: allows other ontoserver.config entries on a scaled deployment
+    values:
+      - fixtures/scaled-values.yaml
+    set:
+      ontoserver.config:
+        ontoserver.fhir.closureTable.max: "500"
+    asserts:
+      - notFailedTemplate: {}
+
   # A single-instance deployment is the supported way to load content, so read-write there
   # must stay legal — the guard has to key on `type`, not on isReadOnly alone.
   - it: allows a single read-write deployment