qwc-open/asiakirjat

Watch

Fork 0

The documented environment variable names are the real ones #168

Open

qwc wants to merge 1 commit from fix/env-var-docs into main

qwc commented

2026-10-02 11:12:38 +02:00

Owner

Problem

internal/docs/builtin/docs/reference/configuration.md described environment overrides as derivable from the YAML structure ("Use underscores for nested keys") and gave two examples that do not exist:

Documented	Actual
`ASIAKIRJAT_DATABASE_DRIVER`	`ASIAKIRJAT_DB_DRIVER`
`ASIAKIRJAT_AUTH_LDAP_ENABLED`	`ASIAKIRJAT_LDAP_ENABLED`

The names come from explicit env: struct tags in internal/config/config.go, and applyEnvToStruct only looks up tags that exist. An unrecognised variable is ignored with no warning, so anyone following the documented rule gets a setting that silently has no effect — the worst possible failure mode for a deployment knob.

Four names genuinely do not follow their YAML path, which is why the rule can't work: database.* → DB_, server.log_level → LOG_LEVEL (no SERVER_), auth.initial_admin.* → ADMIN_ (no AUTH_INITIAL_), storage.base_path → STORAGE_PATH (not ..._BASE_PATH).

The same page also suggested:

bind_password: "${LDAP_PASSWORD}"

There is no variable interpolation anywhere in config loading (no os.ExpandEnv), so this stores the literal string ${LDAP_PASSWORD} as the bind password. The LDAP bind then fails with a plain authentication error that points nowhere near the cause.

Changes

Replace the guessing rule with all 44 variables in a config key → variable table, plus a short table highlighting the four irregular names.
Document value parsing as implemented: booleans accept only true/1 (so yes and on are false), unparseable integers are ignored silently, and an empty string is applied rather than falling back to the file.
Name the settings with no override at all — the access section and both project_groups lists, which are slices the env walker never visits.
Replace the ${LDAP_PASSWORD} example, and add a "Keeping secrets out of the config file" section with the Kubernetes secretKeyRef form.
Expand the precedence section: settings resolve independently, and a -config path that doesn't exist is skipped silently (another symptom that reads as "my settings are ignored").
readme.md and CLAUDE.md claimed every setting has an override; narrowed to scalar settings.

Verification

The 44-row table is checked against the code rather than typed by hand — every env: tag in config.go appears exactly once in the table, no extras, and every YAML key named in it exists in config.go. Tests pass for ./internal/config/... and ./internal/docs/....

Docs-only plus two prose lines; no behaviour change.

🤖 Generated with Claude Code

## Problem `internal/docs/builtin/docs/reference/configuration.md` described environment overrides as derivable from the YAML structure ("Use underscores for nested keys") and gave two examples that **do not exist**: | Documented | Actual | |------------|--------| | `ASIAKIRJAT_DATABASE_DRIVER` | `ASIAKIRJAT_DB_DRIVER` | | `ASIAKIRJAT_AUTH_LDAP_ENABLED` | `ASIAKIRJAT_LDAP_ENABLED` | The names come from explicit `env:` struct tags in `internal/config/config.go`, and `applyEnvToStruct` only looks up tags that exist. An unrecognised variable is ignored with no warning, so anyone following the documented rule gets a setting that silently has no effect — the worst possible failure mode for a deployment knob. Four names genuinely do not follow their YAML path, which is why the rule can't work: `database.*` → `DB_`, `server.log_level` → `LOG_LEVEL` (no `SERVER_`), `auth.initial_admin.*` → `ADMIN_` (no `AUTH_INITIAL_`), `storage.base_path` → `STORAGE_PATH` (not `..._BASE_PATH`). The same page also suggested: ```yaml bind_password: "${LDAP_PASSWORD}" ``` There is no variable interpolation anywhere in config loading (no `os.ExpandEnv`), so this stores the literal string `${LDAP_PASSWORD}` as the bind password. The LDAP bind then fails with a plain authentication error that points nowhere near the cause. ## Changes - Replace the guessing rule with **all 44 variables** in a `config key → variable` table, plus a short table highlighting the four irregular names. - Document value parsing as implemented: booleans accept only `true`/`1` (so `yes` and `on` are false), unparseable integers are ignored silently, and an empty string is *applied* rather than falling back to the file. - Name the settings with no override at all — the `access` section and both `project_groups` lists, which are slices the env walker never visits. - Replace the `${LDAP_PASSWORD}` example, and add a "Keeping secrets out of the config file" section with the Kubernetes `secretKeyRef` form. - Expand the precedence section: settings resolve independently, and a `-config` path that doesn't exist is skipped silently (another symptom that reads as "my settings are ignored"). - `readme.md` and `CLAUDE.md` claimed *every* setting has an override; narrowed to scalar settings. ## Verification The 44-row table is checked against the code rather than typed by hand — every `env:` tag in `config.go` appears exactly once in the table, no extras, and every YAML key named in it exists in `config.go`. Tests pass for `./internal/config/...` and `./internal/docs/...`. Docs-only plus two prose lines; no behaviour change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

qwc added 1 commit

2026-10-02 11:12:39 +02:00

The documented environment variable names are the real ones

CI / test (pull_request) Successful in 1m28s

Details

CI / build (pull_request) Successful in 46s

Details

CI / docker (pull_request) Has been skipped

Details

8a7613f6b3

The configuration reference told readers to build variable names by joining
nested YAML keys with underscores, and gave two examples that do not exist:
ASIAKIRJAT_DATABASE_DRIVER and ASIAKIRJAT_AUTH_LDAP_ENABLED. The names are
explicit `env:` struct tags, so the real ones are ASIAKIRJAT_DB_DRIVER and
ASIAKIRJAT_LDAP_ENABLED. An unrecognised variable is ignored without a warning,
so following the old rule produced a setting that silently had no effect.

Replace the guessing rule with the complete list of all 44 variables, call out
the four names that do not follow their YAML path (DB_, LOG_LEVEL, ADMIN_,
STORAGE_PATH), document how booleans, integers and empty strings are parsed,
and name the list-valued settings that have no override at all.

The same page also suggested `bind_password: "${LDAP_PASSWORD}"`. There is no
variable interpolation in the config file, so that stores 18 literal characters
as the password and the bind fails with nothing pointing at the cause. Replace
it with the environment variable, and add a section on keeping secrets out of
the file, including the Kubernetes secretKeyRef form.

readme.md and CLAUDE.md claimed every setting has an override; narrowed to
scalar settings, which is what is true.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marcel M. Otte <marcel.otte@mmo.to>

CI / test (pull_request) Successful in 1m28s

Details

CI / build (pull_request) Successful in 46s

Details

CI / docker (pull_request) Has been skipped

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'fix/env-var-docs:fix/env-var-docs'

git switch 'fix/env-var-docs'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'main'

git merge --no-ff 'fix/env-var-docs'

git switch 'fix/env-var-docs'

git rebase 'main'

git switch 'main'

git merge --ff-only 'fix/env-var-docs'

git switch 'fix/env-var-docs'

git rebase 'main'

git switch 'main'

git merge --no-ff 'fix/env-var-docs'

git switch 'main'

git merge --squash 'fix/env-var-docs'