Wire 4 missing secrets through Salt GPG pillar pipeline

ldraney commented

2026-06-13 08:44:46 +00:00

Owner

Summary

4 Terraform variables were declared but never wired through the Salt GPG secrets pipeline, blocking tofu plan
Sources: k8s live secrets, PR #410 body, Keycloak client config

Changes

salt/pillar/secrets/platform.sls — add GPG-encrypted entries for telegram_landscaping_chat_id, grafana_oidc_client_secret, admin_oidc_client_secret, admin_auth_secret
Makefile — add all 4 to TF_SECRET_VARS allowlist

Test Plan

make tofu-secrets renders all 22 vars
tofu plan succeeds without missing variable errors
Hetzner edge node shows 3 resources to create

Review Checklist

No secrets committed in plaintext
GPG encryption uses correct Salt Master key
Commit messages are descriptive

Fixes #419 (unblocks tofu plan for hetzner edge node apply)
PR #410 introduced telegram_landscaping_chat_id without pillar entry
PR #395 introduced grafana_oidc_client_secret, admin_oidc_client_secret, admin_auth_secret without pillar entries

## Summary - 4 Terraform variables were declared but never wired through the Salt GPG secrets pipeline, blocking `tofu plan` - Sources: k8s live secrets, PR #410 body, Keycloak client config ## Changes - `salt/pillar/secrets/platform.sls` — add GPG-encrypted entries for `telegram_landscaping_chat_id`, `grafana_oidc_client_secret`, `admin_oidc_client_secret`, `admin_auth_secret` - `Makefile` — add all 4 to `TF_SECRET_VARS` allowlist ## Test Plan - [ ] `make tofu-secrets` renders all 22 vars - [ ] `tofu plan` succeeds without missing variable errors - [ ] Hetzner edge node shows 3 resources to create ## Review Checklist - [ ] No secrets committed in plaintext - [ ] GPG encryption uses correct Salt Master key - [ ] Commit messages are descriptive ## Related Notes - Fixes #419 (unblocks tofu plan for hetzner edge node apply) - PR #410 introduced `telegram_landscaping_chat_id` without pillar entry - PR #395 introduced `grafana_oidc_client_secret`, `admin_oidc_client_secret`, `admin_auth_secret` without pillar entries

ldraney added 1 commit

2026-06-13 08:44:47 +00:00

Wire 4 missing secrets through Salt GPG pillar pipeline

ci/woodpecker/push/terraform Pipeline was successful

Details

ci/woodpecker/pr/terraform Pipeline was successful

Details

ci/woodpecker/pull_request_closed/terraform Pipeline was successful

Details

cee8e420e0

Add telegram_landscaping_chat_id, grafana_oidc_client_secret,
admin_oidc_client_secret, and admin_auth_secret to the pillar and
Makefile TF_SECRET_VARS. These variables were declared in Terraform
by PRs #410 and #395 but never wired through the secrets pipeline,
blocking tofu plan.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented

2026-06-13 08:47:14 +00:00

Author

Owner

PR #422 Review

DOMAIN REVIEW

Tech stack: Salt GPG pillar (YAML + GPG encryption), Makefile, Terraform/OpenTofu variable wiring.

Salt/GPG analysis:

All 4 new GPG-encrypted entries (telegram_landscaping_chat_id, grafana_oidc_client_secret, admin_oidc_client_secret, admin_auth_secret) use the correct Salt Master GPG key ID (4GgPRz4dNyQ / 81A03D1CF874DC90), matching all existing entries in platform.sls.
Each entry uses the correct | (literal block scalar) YAML syntax for multi-line PGP blocks.
Proper blank line after -----BEGIN PGP MESSAGE----- header in each block.
Entries are appended at the end of the secrets: > platform: mapping, maintaining correct YAML indentation (4 spaces for key, 6 spaces for PGP content).

Terraform variable verification:

All 4 variables are already declared in terraform/variables.tf (lines 176, 206, 212, 218) and their respective module variables.tf files.
All 4 are referenced in terraform/main.tf module calls.
The PR correctly wires the missing link: pillar entries exist, TF declarations exist, but the Makefile allowlist and GPG entries were the gap. This PR closes that gap.

Makefile analysis:

Line continuation (\) correctly added to the edge_ssh_public_key line (previously the last entry, now continued).
telegram_landscaping_chat_id gets its own continued line.
grafana_oidc_client_secret admin_oidc_client_secret admin_auth_secret are on the final line (no trailing \), which is correct for the last entry.

BLOCKERS

None.

NITS

secrets_registry.sls missing entries (non-blocking but important): The 4 new secrets do not have corresponding metadata entries in salt/pillar/secrets_registry.sls. The registry tracks origin, description, rotation schedule, and backup locations for every secret. Existing secrets like telegram_chat_id (line 129) and keycloak_admin_password (line 92) have entries. The 4 new secrets should get registry entries as a follow-up to maintain audit trail parity. Suggested entries:
- telegram_landscaping_chat_id: origin config, rotation 0, notes referencing PR #410
- grafana_oidc_client_secret: origin external, provider Keycloak admin, rotation 0
- admin_oidc_client_secret: origin external, provider Keycloak admin, rotation 0
- admin_auth_secret: origin generated, rotation 90
Makefile formatting (cosmetic): The last 3 secrets are packed onto a single line (grafana_oidc_client_secret admin_oidc_client_secret admin_auth_secret), while the rest of the list uses 1-2 vars per line. Consider splitting for readability, e.g.:
```
telegram_landscaping_chat_id \
grafana_oidc_client_secret admin_oidc_client_secret \
admin_auth_secret
```

SOP COMPLIANCE

Branch named after issue (419-... expected, got wire-missing-secrets)
PR body follows template (Summary, Changes, Test Plan, Review Checklist, Related Notes)
Related Notes references parent issue #419
Closes #419 / Fixes #419 not present in PR body (uses "Fixes #419" in Related Notes narrative but not as a standalone keyword that Forgejo will auto-close)
No secrets committed in plaintext (all values are GPG-encrypted PGP blocks)
No unnecessary file changes (2 files changed, both directly relevant)
GPG encryption uses correct Salt Master key (verified key ID match)

PROCESS OBSERVATIONS

This PR fixes a gap introduced across two prior PRs (#410, #395) that added Terraform variable declarations without wiring the corresponding Salt pillar entries. Consider adding a checklist item to the PR template: "If adding a new TF variable sourced from secrets, confirm pillar entry and Makefile allowlist entry exist."
The secrets_registry.sls gap is a documentation debt item. Each time a secret is added to platform.sls without a registry entry, the audit trail degrades. A pre-merge check (or CI lint) that compares platform.sls keys against secrets_registry.sls keys would catch this automatically.

VERDICT: APPROVED

The core changes are correct: GPG-encrypted entries use the right key, YAML structure is valid, Makefile allowlist additions are syntactically correct, and all 4 Terraform variables are already declared and referenced. The branch naming nit and missing registry entries are non-blocking -- the registry entries should be addressed in a follow-up.

## PR #422 Review ### DOMAIN REVIEW **Tech stack:** Salt GPG pillar (YAML + GPG encryption), Makefile, Terraform/OpenTofu variable wiring. **Salt/GPG analysis:** - All 4 new GPG-encrypted entries (`telegram_landscaping_chat_id`, `grafana_oidc_client_secret`, `admin_oidc_client_secret`, `admin_auth_secret`) use the correct Salt Master GPG key ID (`4GgPRz4dNyQ` / `81A03D1CF874DC90`), matching all existing entries in `platform.sls`. - Each entry uses the correct `|` (literal block scalar) YAML syntax for multi-line PGP blocks. - Proper blank line after `-----BEGIN PGP MESSAGE-----` header in each block. - Entries are appended at the end of the `secrets:` > `platform:` mapping, maintaining correct YAML indentation (4 spaces for key, 6 spaces for PGP content). **Terraform variable verification:** - All 4 variables are already declared in `terraform/variables.tf` (lines 176, 206, 212, 218) and their respective module `variables.tf` files. - All 4 are referenced in `terraform/main.tf` module calls. - The PR correctly wires the missing link: pillar entries exist, TF declarations exist, but the Makefile allowlist and GPG entries were the gap. This PR closes that gap. **Makefile analysis:** - Line continuation (`\`) correctly added to the `edge_ssh_public_key` line (previously the last entry, now continued). - `telegram_landscaping_chat_id` gets its own continued line. - `grafana_oidc_client_secret admin_oidc_client_secret admin_auth_secret` are on the final line (no trailing `\`), which is correct for the last entry. ### BLOCKERS None. ### NITS 1. **secrets_registry.sls missing entries (non-blocking but important):** The 4 new secrets do not have corresponding metadata entries in `salt/pillar/secrets_registry.sls`. The registry tracks origin, description, rotation schedule, and backup locations for every secret. Existing secrets like `telegram_chat_id` (line 129) and `keycloak_admin_password` (line 92) have entries. The 4 new secrets should get registry entries as a follow-up to maintain audit trail parity. Suggested entries: - `telegram_landscaping_chat_id`: origin `config`, rotation 0, notes referencing PR #410 - `grafana_oidc_client_secret`: origin `external`, provider `Keycloak admin`, rotation 0 - `admin_oidc_client_secret`: origin `external`, provider `Keycloak admin`, rotation 0 - `admin_auth_secret`: origin `generated`, rotation 90 2. **Makefile formatting (cosmetic):** The last 3 secrets are packed onto a single line (`grafana_oidc_client_secret admin_oidc_client_secret admin_auth_secret`), while the rest of the list uses 1-2 vars per line. Consider splitting for readability, e.g.: ``` telegram_landscaping_chat_id \ grafana_oidc_client_secret admin_oidc_client_secret \ admin_auth_secret ``` ### SOP COMPLIANCE - [ ] Branch named after issue (`419-...` expected, got `wire-missing-secrets`) - [x] PR body follows template (Summary, Changes, Test Plan, Review Checklist, Related Notes) - [x] Related Notes references parent issue #419 - [ ] `Closes #419` / `Fixes #419` not present in PR body (uses "Fixes #419" in Related Notes narrative but not as a standalone keyword that Forgejo will auto-close) - [x] No secrets committed in plaintext (all values are GPG-encrypted PGP blocks) - [x] No unnecessary file changes (2 files changed, both directly relevant) - [x] GPG encryption uses correct Salt Master key (verified key ID match) ### PROCESS OBSERVATIONS - This PR fixes a gap introduced across two prior PRs (#410, #395) that added Terraform variable declarations without wiring the corresponding Salt pillar entries. Consider adding a checklist item to the PR template: "If adding a new TF variable sourced from secrets, confirm pillar entry and Makefile allowlist entry exist." - The `secrets_registry.sls` gap is a documentation debt item. Each time a secret is added to `platform.sls` without a registry entry, the audit trail degrades. A pre-merge check (or CI lint) that compares `platform.sls` keys against `secrets_registry.sls` keys would catch this automatically. ### VERDICT: APPROVED The core changes are correct: GPG-encrypted entries use the right key, YAML structure is valid, Makefile allowlist additions are syntactically correct, and all 4 Terraform variables are already declared and referenced. The branch naming nit and missing registry entries are non-blocking -- the registry entries should be addressed in a follow-up.