feat!(config): update env vars being passed in

[henryjarend]

This majorly overhauls what environment variables are being passed in so that we can better align with the default config files that are present in the curator codebase

[austinderrick]

I rendered this branch against the chart's own test-values.yaml and then checked it against all 133 Curator HelmReleases in iac-interworks. I like the direction, but a few things look like they'll break every site as written, and the rollout order matters more here than usual. My questions are at the bottom.

What helm template produces today

Rendering templates/deployment.yaml on this branch with prod-shaped values:

- name: APP_KEY
  valueFrom:
    secretKeyRef:
      name: prod-admin
      value: app-key          # should be `key:`; `key` is required
- name: AWS_ACCESS_KEY_ID
  valueFrom:
    secretKeyRef: {name: null, key: null}
- name: AWS_SECRET_ACCESS_KEY
  valueFrom:
    secretKeyRef: {name: null, key: null}

Three separate problems:

1. secretKeyRef.value is not a field, and the required key is missing, so the API server rejects the pod spec.
2. Both guards are always true. {{ if .Values.curator.app.appKeySecret }} tests a map with two nil children, and that map has length 2, so it's truthy. The same is true of persistence.s3.accessKeyIdSecret, so the AWS blocks render with null secret names on every S3 site.
3. All 133 sites already supply APP_KEY through curator.envFromSecret, so the new block also duplicates a var that was already there.

Point 2 is the one that worries me, because it lands everywhere. Every release sets persistence.s3.enabled: true with only bucket and region, no credential secrets, and no serviceAccount annotations, so they all take the node-role path your values comment describes.

Unquoted scalars break the same way. --set curator.mail.port=587 renders value: 587, and Kubernetes requires a string:

- name: MAIL_PORT
  value: 587

Same for powerbi.cacheExpirySeconds, session.secureCookie, app.debug, and the cms.* toggles. MEMCACHED_PORT is the only one with | quote.

Related: {{ if .Values.curator.X }} also means false can never be emitted. --set curator.app.debug=false produces no APP_DEBUG at all, which I didn't expect until I rendered it. That matters for the vars whose Curator defaults are true: APP_DEBUG, ENABLE_CSRF, POWER_BI_CACHE_ENABLED. kindIs "invalid" or hasKey would fix it.

The image reads different names than the chart now sends

This is the part I most want to check with you. The premise is aligning with the config files in the Curator repo, but the container doesn't use most of those files. docker/Dockerfile#L113-L119 overlays docker/config/{app,cache,cms,database,filesystems,logging,session}.php on top of config/, so for those seven files the repo defaults never load.

Dropping S3_BUCKET and S3_REGION is the sharp edge:

• docker/config/filesystems.php#L15-L16 reads $_ENV['S3_REGION'] and $_ENV['S3_BUCKET'] with no fallback, so the disk gets a null bucket.
• docker/config/cms.php#L2 builds every media and uploads URL from $_ENV['S3_BUCKET'] ?? 'curator-bucket', so all of them repoint to a bucket that doesn't exist.
• Nothing reads AWS_ENDPOINT. The image uses S3_ENDPOINT and keys use_path_style_endpoint off it, and config/filesystems.php has no endpoint key at all.

Several other new vars do nothing against the shipped image. The cache overlay hardcodes the driver and port and reads CACHE_HOST, so CACHE_DRIVER, MEMCACHED_HOST and MEMCACHED_PORT go nowhere. The same goes for the cms.* toggles, the session.* vars, the logging.* vars, APP_DEBUG and DB_CONNECTION.

FILESYSTEM_DISK is a different case, because Curator never reads it. Winter uses FILESYSTEM_DRIVER, so curator.filesystems.disk and curator.cms.filesystemDriver are two knobs for one setting, and one of them does nothing.

The mail.*, queue.connection and powerbi.* additions do work, because the Apache image leaves those files alone. It's worth knowing that Dockerfile.frankenphp copies the whole docker/config/ directory instead, and there mail.php hardcodes host and credentials while queue.php hardcodes sync. No site runs that image today, so it's a trap for later, not a problem now.

Rollout order

The two halves of this change ship by different mechanisms. I had this backwards at first:

• The image tag is Renovate-managed and automerged. renovate.json#L135-L141 sets minimumReleaseAge: null and ignoreTests: true for ghcr.io/interworks/curator, and renovate.json#L48-L65 automerges prod and qa in the Thursday 4-6am window.
• The chart version is hand-edited. All 133 releases still pin 2.6.0 even though 2.7.3 shipped on 5 August, and 124 of the files carry the comment # make sure that this is the latest version, for example amgen's dev release.

So if we teach the image the new names first, Renovate pushes it to every site while they're all still on chart 2.6.0 sending the old ones, and storage breaks without anyone merging an iac PR. Chart first just moves the breakage to the manual bump. The order I'd suggest:

1. Teach the image to accept both names, $_ENV['AWS_BUCKET'] ?? $_ENV['S3_BUCKET'] and so on. That's harmless under chart 2.6.0, and it rolls out on its own.
2. Let that tag reach prod on the next Thursday window.
3. Land this chart PR with the renames and the fixes above, and cut a version.
4. Bump the 133 pins, dev first.
5. Drop the old names from the image a release later.

Smaller things

• henry-and-paige/prod-helmrelease.yaml#L87-L105 mounts its own filesystems.php through curator.config with $_ENV['S3_BUCKET'] hardcoded, so that site breaks on the rename even after the image is fixed. It's the only one today, but 22 sites use curator.config for something.
• Four sites set MAIL_EHLO_DOMAIN through curator.env, so they'll now be setting it twice. curator.env renders last and still wins, but it needs a migration note.
• One site sets DB_HOST_READ through curator.env, and docker/config/database.php supports it. That one might deserve a real value alongside the MaxScale logic.
• There's no DB_PORT knob, now that curator.database.host invites external databases. The entrypoint parses host:port out of DB_HOST, but the config reads DB_PORT on its own.
• Six sites override session.php through curator.config, and the only change in each is 'lifetime' => 1440. SESSION_LIFETIME would retire all six blobs. It's not in this PR, while three session vars the image ignores are.
• charts/curator/values.schema.json still validates only environment. iac-interworks/scripts/validate-helm-values.sh skips Curator charts on the grounds that they're schema-validated at deploy time, so nothing checks these keys on either side. This PR triples the values surface, which feels like the moment to fill that schema in.
• The first test is named "renders identical env vars for sites that upgrade with no values.yaml changes", but it asserts MEMCACHED_HOST and FILESYSTEM_DISK, which didn't exist before. helm-unittest renders without schema validation, so both malformed secretKeyRef blocks sail through. A kubeconform or kubectl --dry-run=client step would have caught three of the four blockers.
• # TODO account for old value location is still in _env.tpl, and that TODO is the upgrade path for all 133 live sites.
• commitlint is red.

Questions

1. Do you want to fix the image configs, or keep the old names in the chart? If it's the image, are you up for the both-names step so an automerged tag can't land ahead of the chart bump?
2. Should the vars the image ignores stay in as groundwork, or come out until the docker/config/ overlays go away? I lean toward cutting them, since values.yaml documents behavior that won't happen.
3. Who owns bumping the 133 pins when this ships, and should that be scripted?

Happy to send the mechanical fixes (quoting, nil guards, secretKeyRef.key) as a PR into your branch if that's easier. The curatorDbEndpoint change and the per-config-file structure are both good, and I'd like to see them land.