From 1d27443f1487ae1e4273a3277bea0f0ed8f59306 Mon Sep 17 00:00:00 2001 From: convee Date: Thu, 10 Sep 2026 23:44:09 +0800 Subject: [PATCH] chart: refuse a half-configured external PostgreSQL `postgresql.embedded.enabled: false` already dropped the bundled StatefulSet, its Service and its ingress NetworkPolicy, but three values kept defaults that only make sense while that server exists: - `database.host` still named `sites-postgres`; - `database.runtimeHost` still resolved that Service's in-cluster DNS name for the tenant-facing runtime Secret; - `SITES_DB_SSLMODE` was hardcoded `disable`. Each one renders, applies and rolls out while the control plane dials a Service this render declined to create, so all three now fail at `helm template` in that mode. `database.sslmode` joins the values surface as a stated choice. The bundled install keeps `disable`, because that image serves no certificate and the hop never leaves the cluster; an external deployment has to say `require`, `verify-ca`, `verify-full` or `disable` rather than inherit "require" from the code while the chart's comment still describes a hop that is gone. --- CHANGELOG.md | 10 +++ README.md | 2 +- charts/site/templates/10-control-plane.yaml | 11 ++- charts/site/values.schema.json | 17 +++-- charts/site/values.yaml | 8 ++ docs/DEPLOYMENT.md | 37 +++++++++ tests/test_helm_package.py | 84 +++++++++++++++++++++ 7 files changed, 160 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea720a0..61e5197 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -116,6 +116,16 @@ All notable changes to this project are documented in this file. The format foll through `SITES_OIDC_MERCHANT_MAP`, and an unmapped value is refused with 403 and a log line. Tenants may still be created on first use, gated by `SITES_OIDC_SIGNUPS_ENABLED` and `SITES_OIDC_EMAIL_DOMAINS`. +- The chart can point the control plane at a PostgreSQL you operate instead of the + StatefulSet it ships. `postgresql.embedded.enabled: false` already dropped that + StatefulSet, its Service and its ingress NetworkPolicy, but three values kept their + bundled defaults: `database.host` still named `sites-postgres`, the tenant-facing + `database.runtimeHost` still resolved that Service's in-cluster DNS name, and + `SITES_DB_SSLMODE` was still hardcoded `disable`. All three are now refused at render + when the bundled server is off, because each one installs cleanly and then dials a + Service that was not created. `database.sslmode` joins the values surface as a stated + choice (`require`, `verify-ca`, `verify-full`, `disable`); the bundled install keeps + `disable`. ### Removed diff --git a/README.md b/README.md index be48033..72bb784 100644 --- a/README.md +++ b/README.md @@ -175,7 +175,7 @@ git clone https://github.com/hullwork/site.git cd site uv sync --locked --extra dev make test-db # starts a throwaway PostgreSQL on 127.0.0.1:55439 -make test # 1060 tests +make test # 1063 tests make test-db-down ``` diff --git a/charts/site/templates/10-control-plane.yaml b/charts/site/templates/10-control-plane.yaml index 608ebc5..2f7013c 100644 --- a/charts/site/templates/10-control-plane.yaml +++ b/charts/site/templates/10-control-plane.yaml @@ -349,13 +349,18 @@ spec: value: {{ .Values.namespaces.control | quote }} - name: SITES_HTTP_PORT value: "8080" +{{- if and (not .Values.postgresql.embedded.enabled) (eq .Values.database.host "sites-postgres") }} +{{- fail "database.host still names the bundled sites-postgres Service while postgresql.embedded.enabled is false. This render does not create that Service, so the control plane would start against a name that resolves to nothing. Set database.host to the endpoint of the database you operate." }} +{{- end }} - name: SITES_DB_HOST value: {{ .Values.database.host | quote }} # Tenant workloads run outside sites-local, so the runtime Secret # must contain a cross-namespace address. The API keeps using the # short control-plane Service name above for its own connections. + # With the bundled server switched off there is no Service to derive + # that address from, so an external deployment states it. - name: SITES_DATA_DB_RUNTIME_HOST - value: {{ default (printf "sites-postgres.%s.svc.cluster.local" .Values.namespaces.control) .Values.database.runtimeHost | quote }} + value: {{ if .Values.postgresql.embedded.enabled }}{{ default (printf "sites-postgres.%s.svc.cluster.local" .Values.namespaces.control) .Values.database.runtimeHost | quote }}{{ else }}{{ required "database.runtimeHost is required when postgresql.embedded.enabled is false: the bundled sites-postgres Service does not exist and tenant workloads run outside the control namespace, so the runtime Secret needs the address they can reach." .Values.database.runtimeHost | quote }}{{ end }} - name: SITES_DB_PORT value: {{ .Values.database.port | quote }} - name: SITES_DB_NAME @@ -381,8 +386,10 @@ spec: # this is one pod talking to another inside the cluster. Point # SITES_DB_HOST at a managed database and drop this variable to get # "require" back, or set "verify-full" once you have a CA to check. + # With the bundled server switched off there is no such hop to reason + # about, so the operator states the mode instead of inheriting it. - name: SITES_DB_SSLMODE - value: disable + value: {{ if .Values.postgresql.embedded.enabled }}disable{{ else }}{{ required "database.sslmode is required when postgresql.embedded.enabled is false. The code default is require, and whether the hop to a database you operate is encrypted is not something this chart can infer. Use require, verify-ca, verify-full, or disable to state that it is not." .Values.database.sslmode }}{{ end }} # Signing key for admin console sessions. sites-api refuses to start # without it, so the integration layer must add a `console-session-key` # key (>=32 characters, different from `token`) to the configured diff --git a/charts/site/values.schema.json b/charts/site/values.schema.json index a14123c..9f3356e 100644 --- a/charts/site/values.schema.json +++ b/charts/site/values.schema.json @@ -130,6 +130,7 @@ "host", "runtimeHost", "port", + "sslmode", "name", "user" ], @@ -141,12 +142,16 @@ "runtimeHost": { "type": "string" }, - "port": { - "type": "integer", - "minimum": 1, - "maximum": 65535 - }, - "name": { + "port": { + "type": "integer", + "minimum": 1, + "maximum": 65535 + }, + "sslmode": { + "type": "string", + "enum": ["", "require", "verify-ca", "verify-full", "disable"] + }, + "name": { "type": "string", "minLength": 1 }, diff --git a/charts/site/values.yaml b/charts/site/values.yaml index 602cb01..34143cf 100644 --- a/charts/site/values.yaml +++ b/charts/site/values.yaml @@ -72,6 +72,14 @@ database: port: 5432 name: sites user: sites + # Rendered as SITES_DB_SSLMODE. Empty keeps the bundled behaviour: `disable`, + # because sites-postgres runs the stock image with no server certificate and + # this is one Pod talking to another inside the cluster. When + # `postgresql.embedded.enabled` is false this value is required, because the + # code default is `require` and a plaintext hop to a database you operate is + # not a property of this chart. Use `require`, `verify-ca`, `verify-full`, or + # `disable` to state that it is not. + sslmode: "" # Node-reachable address for images produced by the optional build plane. A # multi-node or remote cluster must point this at its registry mirror/route. diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 2be41ce..eb8d1bc 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -215,6 +215,43 @@ consumer's integration layer. PostgreSQL is the only metadata backend. Use a dedicated database and account, and pass the password through `SITES_DB_PASSWORD_FILE` rather than an environment variable. +### Embedded or external PostgreSQL + +`postgresql.embedded.enabled` decides whether the release carries its own database. The +default is the single-instance StatefulSet in `charts/site`; setting it to `false` drops +that StatefulSet, its Service and its ingress NetworkPolicy, and the control plane dials +the server you operate instead: + +```yaml +postgresql: + embedded: + enabled: false +database: + host: postgres.example.net # SITES_DB_HOST + runtimeHost: postgres.example.net # SITES_DATA_DB_RUNTIME_HOST, read by tenants + port: 5432 + name: sites + user: sites + sslmode: verify-full # SITES_DB_SSLMODE +``` + +Three values stop being inferable once the bundled server is gone, and the chart refuses +to render rather than install a release that dials nothing: + +- `database.host` defaults to `sites-postgres`, the Service name this mode does not + create. +- `database.runtimeHost` defaults to + `sites-postgres..svc.cluster.local`, the address tenant workloads + use, which exists only while the bundled server does. +- `database.sslmode` is `disable` for the bundled server because that image serves no + certificate and the hop never leaves the cluster. The code default is `require`, so + the chart does not choose for you: state `require`, `verify-ca`, `verify-full`, or + `disable`. + +Credentials come from `existingSecrets.database` in either mode, so an external server +only needs a Secret carrying its own values. The database and role must already exist +there - the bundled StatefulSet is what creates the two it is handed. + `SITES_DB_SSLMODE` defaults to `require`, and `SITES_DATA_DB_SSLMODE` inherits it. Accepted values are `require`, `verify-ca`, `verify-full`, and `disable`. libpq's `prefer` and `allow` are rejected on purpose: both fall back to an unencrypted diff --git a/tests/test_helm_package.py b/tests/test_helm_package.py index ac4bef7..37d4521 100644 --- a/tests/test_helm_package.py +++ b/tests/test_helm_package.py @@ -20,6 +20,16 @@ # rather than through tests/chart.py, so they state it themselves. POD_CIDR = ("--set-string", "clusterNetwork.podCIDR=10.201.0.0/16") +# The complete external-database configuration. The refusal tests below remove +# exactly one part of it, so the guard each one is about is the only thing left +# that can fail. +EXTERNAL_DATABASE = ( + "--set", "postgresql.embedded.enabled=false", + "--set-string", "database.host=postgres.example.net", + "--set-string", "database.runtimeHost=postgres.example.net", + "--set-string", "database.sslmode=verify-full", +) + class HelmPackageContractTests(unittest.TestCase): def test_quickstart_is_a_repository_owned_kubeadm_workflow(self) -> None: @@ -280,6 +290,80 @@ def test_chart_declares_existing_secret_names_and_keys_without_rendering_credent ): self.assertIn(expected, custom) + def test_external_database_replaces_the_bundled_one(self) -> None: + """The bundled server has to disappear, not just lose its address. + + Rewriting `SITES_DB_HOST` on its own leaves the StatefulSet, its Service + and its ingress policy in the release, and the tenant-facing runtime + host still names the Service the same render declined to create. + """ + rendered = subprocess.check_output( + ["helm", "template", "site", str(CHART), *POD_CIDR, *EXTERNAL_DATABASE], + text=True, + ) + documents = [item for item in yaml.safe_load_all(rendered) if item] + named = {(item.get("kind"), item["metadata"]["name"]) for item in documents} + for absent in ("Service", "StatefulSet", "NetworkPolicy"): + self.assertNotIn((absent, "sites-postgres"), named) + api = next( + item for item in documents + if item.get("kind") == "Deployment" + and item["metadata"]["name"] == "sites-api" + ) + environment = { + item["name"]: item.get("value") + for item in api["spec"]["template"]["spec"]["containers"][0]["env"] + } + self.assertEqual("postgres.example.net", environment["SITES_DB_HOST"]) + self.assertEqual( + "postgres.example.net", environment["SITES_DATA_DB_RUNTIME_HOST"] + ) + self.assertEqual("verify-full", environment["SITES_DB_SSLMODE"]) + + def test_the_bundled_database_still_renders_untouched(self) -> None: + """Negative control: none of the guards above may fire by default. + + A guard written as `if .Values.postgresql.embedded.enabled` rather than + the other way round would pass every refusal test and make the chart + uninstallable in its default shape. + """ + rendered = subprocess.check_output( + ["helm", "template", "site", str(CHART), *POD_CIDR], text=True + ) + documents = [item for item in yaml.safe_load_all(rendered) if item] + named = {(item.get("kind"), item["metadata"]["name"]) for item in documents} + self.assertIn(("StatefulSet", "sites-postgres"), named) + self.assertIn(("Service", "sites-postgres"), named) + self.assertIn("value: disable", rendered) + + def test_the_chart_refuses_a_half_configured_external_database(self) -> None: + """Three ways to leave the release dialling something that is not there. + + The default `database.host` is the bundled Service name, so flipping the + switch and stopping is the shape an operator actually reaches; an empty + runtime host points tenant workloads at that same absent Service; and an + unstated SSL mode would inherit the code default of `require` while the + chart's own comment still describes the bundled hop that no longer + exists. + """ + for missing, override in ( + ("database.host", ("--set-string", "database.host=sites-postgres")), + ("database.runtimeHost", ("--set-string", "database.runtimeHost=")), + ("database.sslmode", ("--set-string", "database.sslmode=")), + ): + with self.subTest(missing=missing): + result = subprocess.run( + [ + "helm", "template", "site", str(CHART), + *POD_CIDR, *EXTERNAL_DATABASE, *override, + ], + capture_output=True, + text=True, + check=False, + ) + self.assertNotEqual(0, result.returncode, result.stdout[:400]) + self.assertIn(missing, result.stdout + result.stderr) + def test_chart_renders_after_copy_without_sibling_repositories(self) -> None: if not shutil.which("helm"): self.skipTest("helm is not installed")