feat(ansible): allow the grafana_alloy API token via git-ignored inventory - #63
Merged
Merged
Conversation
…ntory Give GC_API_TOKEN the same dual-home treatment decdn_node gives rpc_url (#39 parity): set grafana_alloy_api_token in host_vars secret.yml and the role authors /etc/grafana-alloy.env itself — token-only, root 0600, JSON-encoded for systemd; leave it empty to keep the operator-provisioned host path. A <source> <sha256> provenance record next to the file drives two fail-loud guards (missing-secret adoption after an inventory deploy; hand-edited file meeting an inventory token without the overwrite opt-in), the disable path removes its own record behind the managed-by marker, and an out-of-band host edit restarts the agent before the record lands.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved provenance, rotation, path-collision, write-order, and token-leakage issues block approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds inventory-backed Grafana Alloy API-token provisioning with provenance tracking, overwrite safeguards, documentation, and Molecule coverage.
Changes:
- Adds token authoring, checksum records, rotation detection, and validation.
- Updates role and repository documentation.
- Adds validation cases and a dedicated token scenario.
File summaries
| File | Summary |
|---|---|
ansible/roles/grafana_alloy/tasks/validate-paths.yml |
Validates checksum-file paths. |
ansible/roles/grafana_alloy/tasks/preflight.yml |
Adds token and provenance gates. |
ansible/roles/grafana_alloy/tasks/main.yml |
Authors, restarts, and records token files. |
ansible/roles/grafana_alloy/README.md |
Documents token workflows. |
ansible/roles/grafana_alloy/files/grafana-alloy.env.example |
Documents dual credential paths. |
ansible/roles/grafana_alloy/defaults/main.yml |
Defines token and provenance options. |
ansible/README.md |
Updates setup and scenario documentation. |
ansible/molecule/validation/converge.yml |
Adds token validation cases. |
ansible/molecule/grafana-cloud-token/verify.yml |
Verifies authoring, provenance, and rotation. |
ansible/molecule/grafana-cloud-token/molecule.yml |
Defines the new scenario. |
ansible/molecule/grafana-cloud-token/files/alloy-stub |
Supplies the test binary stub. |
ansible/molecule/grafana-cloud-token/converge.yml |
Configures the token scenario. |
ansible/inventory/host_vars/decdn-node-1/secret.yml.example |
Shows the token variable. |
Review details
Suppressed comments (5)
ansible/roles/grafana_alloy/README.md:94
- This documentation promises that rotation is just an inventory edit plus deploy, but the current provenance guard rejects that edit whenever the new token changes the file checksum (before the rewrite runs). Update the documented workflow together with the guard, otherwise operators following this section will hit the overwrite assertion on every rotation.
With it set, the role authors `/etc/grafana-alloy.env` itself — as a **token-only**
file (`GC_API_TOKEN="…"` at `root:root 0600`, JSON-encoded so quotes/backslashes
survive), rewritten wholesale on every converge. Runtime behaviour is identical:
`config.alloy` still reads `sys.env("GC_API_TOKEN")`; only who fills the file
changes. Rotation is an inventory edit + deploy instead of per-host shell work,
and provenance tracking (a `<source> <sha256>` record next to the file) catches
ansible/roles/grafana_alloy/tasks/main.yml:283
- This copy runs before
alloy validate, service start, and the provenance-record write. If a later task fails after a token rotation changes the file, the old record remains; the next retry sees aninventorychecksum mismatch and demandsgrafana_alloy_overwrite_host_file, even though the mismatch was caused by this role. Defer the authoring write until failure-prone validation succeeds, or track the pending role write so retries do not classify it as foreign.
- name: Write the token-only secret environment file (inventory-supplied)
ansible.builtin.copy:
dest: "{{ grafana_alloy_secret_file }}"
owner: root
group: root
ansible/roles/grafana_alloy/tasks/main.yml:283
- This inventory path reads
grafana_alloy_api_tokenon the Ansible control machine and sends it to the target viacopy, so it necessarily transits the control machine. That contradicts the PR description's claim that the value never does; clarify that claim (it is only true for the empty-variable, host-provisioned path) so operators understand the different threat model.
- name: Write the token-only secret environment file (inventory-supplied)
ansible.builtin.copy:
dest: "{{ grafana_alloy_secret_file }}"
owner: root
group: root
ansible/roles/grafana_alloy/tasks/preflight.yml:460
- This condition treats a normal inventory-token rotation as a foreign edit: after the first inventory converge, changing the token necessarily makes the live file checksum differ from the recorded checksum, so this assert aborts before the
copytask can rotate it. The newgrafana-cloud-tokenverify play exercises exactly that path and will fail here; adjust the provenance/rotation design (or explicitly require and document an opt-in for rotation) while preserving the out-of-band-edit guard.
- _ga_recorded_sum | length > 0
- >-
_ga_record_source != 'inventory'
or _ga_recorded_sum != (_ga_secret_stat.stat.checksum | default(''))
ansible/roles/grafana_alloy/tasks/preflight.yml:460
- This condition deliberately skips files with no provenance record, so an existing hand-edited/unknown file is overwritten without requiring
grafana_alloy_overwrite_host_file. That can discard its extraGC_*settings; it also contradicts the PR contract and README, which say unknown/untracked existing files require the explicit opt-in. Treat an empty provenance record as foreign for this guard (and remove or revise the now-unneeded warning path).
- grafana_alloy_api_token | length > 0
- _ga_secret_stat.stat.exists
- _ga_recorded_sum | length > 0
- >-
_ga_record_source != 'inventory'
or _ga_recorded_sum != (_ga_secret_stat.stat.checksum | default(''))
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Three issues from automated review of the inventory-token path: * reject grafana_alloy_env_checksum_file == grafana_alloy_secret_file in validate-paths: equal paths would make preflight slurp the operator's credential file as if it were the provenance record, and let the post- converge record task overwrite the credentials with "<source> <sha256>". * scope the env-file content greps to the host-provisioned posture: with grafana_alloy_api_token set the rewrite is token-only, so keys found on the current file prove nothing — demand every non-token connection setting from inventory before any mutation instead. * stop interpolating part of the API token into a preflight assert failure message; assert output lands in plain Ansible/CI logs.
…kens The new token-via-inventory scenarios carry deliberately fake API tokens (molecule-test-token & co.) as plain YAML literals named grafana_alloy_api_token, which trips KICS' "Passwords And Secrets - Generic Token" HIGH query and fails the fail-on-high gate with 5 findings. Annotate exactly those five fixture lines with # kics-scan ignore-line so molecule stays scanned and any future real secret in it still lights up.
Refuse unknown credential overwrites, constrain and validate provenance records, preserve provenance across disable, recover interrupted rotations with a restart, and add behavioral Molecule coverage for each state transition.
altanoruc
force-pushed
the
feat/ga-token-via-inventory
branch
from
September 17, 2026 18:47
d3d1523 to
f2762d2
Compare
Review of #63 found four ways the inventory token path could deploy green while the agent ran on wrong or stale credentials. Token shape gate anchored on `$`, which in Python also matches just before a trailing newline — so `grafana_alloy_api_token: |` (the natural way to wrap a long token) passed a gate whose own fail_msg promises "no newline", and systemd delivered the value with a literal \n. Anchor on \Z, as roles/decdn_node does at :997/:1072/:1089. stat omits `checksum` when a file is unreadable to the connection, so `stat.checksum | default('')` made the adoption guard — the gate whose whole job is catching a vanished secret.yml — pass vacuously, and skipped the provenance record write in silence. Fail closed in the 0600 gate instead and drop the three defaults it was guarding; the record write now fails loudly rather than leaving the host permanently untracked. validate-env-record hardcoded `python3` (bypassing interpreter_python = auto_silent) and folded every non-zero rc into "your record is corrupt", whose suggested remedy disarms the guard. Use the discovered interpreter and let any rc outside {0,1} fail as itself. With no record there is nothing to compare, so a host-side token edit cannot be detected: `state: started` is a no-op on a running unit and the record is then seeded from the new bytes, hiding that edit forever. The documented hand-back procedure lands operators in exactly that state. Keep the deliberate no-restart behaviour (bouncing the fleet on the upgrade run is what it avoids) but say so, scoped to an already-running agent. Also: reject placeholder tokens carried over from the .example templates (shaped like real tokens, so nothing downstream could tell), require a real boolean for grafana_alloy_overwrite_host_file rather than let "yes"/"on"/"1" grant a credential-destroying authorisation by coercion, normalise the token's `| length` tests so an empty `grafana_alloy_api_token:` key fails in the role's own voice, and include `.msg` in the config-validation failure so a missing binary stops reporting an empty reason next to a wrong diagnosis. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… cells
The grafana-cloud-token verify play failed in CI on
`_ga_untracked_overwrite_applied_report.skipped`. The cause is not flakiness:
`ansible_check_mode` reflects ONLY the --check CLI flag, never the `check_mode:`
keyword (verified at block and play level on ansible-core 2.21). Inside the
dry-run block the role therefore sees False, skips its check-mode branch and
takes the real-run one, so both `.skipped` predicates asserted the opposite of
what they read. Assert the contract the play can actually prove — a dry run
mutates neither the credential file nor the record — and document why the
message wording is out of reach here (operators meet it via `make check`).
grafana-cloud's out-of-band-edit play omitted grafana_alloy_batch_send_size,
which converge.yml sets to 512 against a role default of 1024. The config
re-rendered, notified Restart alloy on its own, and the PID assertion passed —
so the play stayed green with the provenance signal it exists to test deleted.
Mirror converge exactly and assert the config checksum is unchanged across the
re-run, so future var drift fails instead of masking.
New coverage for the cells that had none:
- preflight's all-or-nothing gate, whose failure mode is a token-only env file,
sys.env resolving to "", a passing `alloy validate`, a running unit and not one
metric shipped — both the unconditional and the logs-gated half
- both foreign-provenance cells of the overwrite gate: a record reading "host"
(the normal host->inventory migration) and one whose checksum no longer
matches, each driven with a token that differs from the file so the
`desired == actual` escape hatch cannot mask the regression
- a symlinked provenance record, mirroring ga-symlink-secret — the record's stat
gate runs before the slurp, and that ordering is the whole defence against
pulling an arbitrary root-only file onto the control machine
- a newline-terminated token, and a stringly-typed overwrite knob
- the rendered config's sys.env("GC_API_TOKEN") indirection and the OTLP
username fallback, which the leak probe alone cannot prove
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…relaxed AGENTS.md still told every agent reading the repo that the API token is host-provisioned, full stop — the exact statement #63 relaxes, in the file read first. grafana-alloy.env.example contradicted itself: its header forbade carrying the token "through inventory" while line 26 of the same file now permits it. And preflight's smuggle gate justified itself with "inventory is committed", which ansible/.gitignore:9 makes untrue for the secret.yml the token rides in. Document the migration path onto the inventory path, which had none: the ordered procedure, `make check` first, and — the part that was missing everywhere — setting grafana_alloy_overwrite_host_file back to false afterwards. Left true it permanently disables the clobber guard on that host, so a later hand-edit is destroyed silently instead of stopping the deploy. Note the blind spot the hand-back direction leaves, matching the role's new warning. Also correct two counts AGENTS.md had drifted on (the collection ships three roles, not two — galaxy/build.sh:19; the suite is glob-discovered, not "six scenarios"), drop the claim that the Helm chart was "Secret-oriented from the start" when it ships no Alloy at all, and add the three new user-facing variables to the collection changelog. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two roles write secrets with `copy: content=…` — the node's decdn.env and, as of this branch, Alloy's grafana-alloy.env. Without pipelining the module arguments, plaintext token included, are staged into ~/.ansible/tmp on the target before the atomic move. `no_log` covers task output, not that file, so AGENTS.md hard rule 1 was leaning on a gap the inventory token path widens. Molecule already sets ANSIBLE_PIPELINING per scenario; production did not. Requires requiretty off in sudoers, which is the default on the Debian/Ubuntu targets this project supports. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 24, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
GC_API_TOKENgets the same dual-home treatmentdecdn_rpc_urlreceived in #39. Settinggrafana_alloy_api_token(git-ignoredhost_vars/<node>/secret.yml, like everything else sensitive) makesgrafana_alloyauthor/etc/grafana-alloy.envitself — a single JSON-encodedGC_API_TOKEN="…"line at root:root 0600 (no_log). Leaving it empty keeps today's operator-provisioned posture unchanged; the value never transits the control machine.Provenance machinery (#39 parity)
After every enabled converge the role records
<source> <sha256>of the env file into/etc/grafana-alloy.env.sha256(root 0600), recorded last — after handler flush and service start, checksum-guarded under--check. Preflight uses it for two fail-loud guards:inventory+ checksum matchesgrafana_alloy_overwrite_host_file: truebefore rewritingAuthoring is all-or-nothing: extra hand-added
GC_*lines are discarded by a rewrite, so preflight's per-key gates force migrating them to their inventory variables first. An out-of-band edit on the host path now notifies a restart before the flush/record (same ordering asdecdn_node:1408/:1611). Teardown removes the record behind the managed-by marker, so disable→enable re-seeds cleanly. Checksums pinned to sha256 explicitly so the.sha256cross-checks againstsha256sum.Zero template changes —
config.alloy.j2keepssys.env("GC_API_TOKEN"); only who fills the file changes.Tests
validation: rescue matcher updated for the renamed existence assert; new casesga-token-var-shape(whitespace token) andga-collision(foreign fixture + token var, no overwrite knob); expected-tags multiset extendedmolecule/grafana-cloud-token: no staged env file, token via playbook vars; verifies 0600 root:root regular file, exact single-line content,<source> <sha256>record vs live stat checksum, no token literal outside the env file, running service, idempotence, plus an end-state rotation pass.exampletemplates cross-reference the other pathOut of scope
Helm chart (Secret-oriented by design) and
lint-alloymatrix (renders config only).Local gates:
make lint,lint-ansible(production profile),lint-alloy(real binary, 8 combos), moleculegrafana-cloud-token/grafana-cloud/validation— all green.