Skip to content

fix(openbao): grant actual-budget the missing vault ACL paths - #2345

Merged
devantler merged 2 commits into
mainfrom
claude/openbao-actual-budget-acl
Jun 30, 2026
Merged

devantler merged 2 commits into
mainfrom
claude/openbao-actual-budget-acl

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Daily AI Assistant

Problem

Since #2331 merged (~07:00Z 2026-06-29), the seed-actual-budget-enablebanking PushSecret has been Errored on a clean 403:

set secret failed: could not verify if secret exists in store: cannot read secret data from Vault:
URL: GET http://openbao-active.openbao.svc.cluster.local:8200/v1/secret/data/apps/actual-budget/enablebanking
Code: 403. * permission denied

#2331 added the Enable Banking seed PushSecret (infrastructure/vault-seed/push-secret-seed-actual-budget-enablebanking.yaml) and the app's actual-budget-enablebanking ExternalSecret, but never granted the matching OpenBao ACL paths in infrastructure/vault-config/job.yaml. So:

  • the seed can't write secret/data/apps/actual-budget/enablebanking (the live 403 above — vault-seed-write has no actual-budget path), and
  • the app read would 403 too once seeded (there is no app-actual-budget-readonly policy on the shared external-secrets role bundle).

This is the persistent, non-flake failure surfacing in the merge-queue deploy-prod HealthCheck wait (distinct from the Cilium first-packet-drop noise tracked by #2337).

Fix

Three additive grants, mirroring the existing umami/crossview apps:

  1. New app-actual-budget-readonly policy — read on secret/data/apps/actual-budget/*.
  2. actual-budget data + metadata write paths on vault-seed-write — so the seed PushSecret can write the placeholder.
  3. app-actual-budget-readonly added to the shared external-secrets Kubernetes-auth role bundle — so the app's ExternalSecret can read it back.

All three are additive allow-list entries; no existing access is altered.

Validation

  • ksail workload validate --skip-helm-render — 474 files, exit 0 (both local + prod overlays).
  • Embedded heredocs balanced; policy blocks copied verbatim from the umami/crossview pattern.

Blast radius / promotion

Touches prod OpenBao ACLs (additive only). On promotion the vault-config Job re-runs and applies the new policies; the seed PushSecret should then sync and the app ExternalSecret resolve. Left as a draft for the maintainer to promote and watch the first reconcile.

Fixes the actual-budget 403 introduced by #2331.

#2331 added the actual-budget Enable Banking seed PushSecret + app
ExternalSecret but never granted the matching OpenBao ACL paths, so since
the ~07:00Z merge the seed PushSecret has been Errored on a 403 (permission
denied writing secret/data/apps/actual-budget/enablebanking) and the app
read would 403 too once seeded.

Add the three missing, additive grants, mirroring the umami/crossview apps:
- a new app-actual-budget-readonly policy (read on apps/actual-budget/*),
- actual-budget data+metadata write paths on vault-seed-write (so the seed
  PushSecret can write the placeholder), and
- app-actual-budget-readonly on the shared external-secrets role bundle (so
  the app's ExternalSecret can read it back).

All additive allow-list grants — no existing access changes.
@coderabbitai

coderabbitai Bot commented Jun 29, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The vault-config Kubernetes Job adds Vault ACLs for actual-budget. It introduces a new read-only policy and extends vault-seed-write to allow create, update, and read access on the app’s data and metadata paths.

Changes

Vault ACL provisioning for actual-budget

Layer / File(s) Summary
Vault policies for actual-budget
k8s/bases/infrastructure/vault-config/job.yaml
Adds app-actual-budget-readonly with read access to secret/data/apps/actual-budget/*, and extends vault-seed-write with create/update/read on secret/data/apps/actual-budget/* and secret/metadata/apps/actual-budget/*.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding missing Vault ACL paths for actual-budget.
Description check ✅ Passed The description matches the changeset and explains the added OpenBao ACL grants for actual-budget.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/openbao-actual-budget-acl

Comment @coderabbitai help to get the list of available commands.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Assistant

Live finding (2026-06-30): this PR is the fix for a deterministic merge-queue deadlock, not just a secondary wedge cause.

seed-actual-budget-enablebanking has been Errored ~24h (created 06-29 07:01Z, 403 permission denied on secret/data/apps/actual-budget/enablebanking). That stalls the infrastructure Kustomization's 20m health check on every merge_group deploy → the whole platform queue (currently 9 PRs) evicts one-by-one and prod GitOps delivery is frozen. The umami/coroot errors in the deploy logs are diagnose-flux noise, not the gate (full triage: #2337).

This PR's vault-config/job.yaml grant makes its own deploy self-healing, so the queue would eventually self-heal once this PR fronts — but only after ~6 × 20m of thrashing. Recommend merging this ahead of the queue (admin / re-order to front) to break the wedge now, then re-queue the evicted trusted PRs (#2335 dependabot, #2332/#2336 own, #2333/#2340 renovate). Green + threads-resolved; ready to go.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Jun 29, 2026
@botantler-1
botantler-1 Bot enabled auto-merge June 30, 2026 01:18
@botantler-1
botantler-1 Bot added this pull request to the merge queue Jun 30, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
k8s/bases/infrastructure/vault-config/job.yaml (2)

803-805: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the duplicate actual-budget policy stanzas
k8s/bases/infrastructure/vault-config/job.yaml lists secret/data/apps/actual-budget/* twice and secret/metadata/apps/actual-budget/* twice. Keep one copy of each so the policy stays easier to audit.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@k8s/bases/infrastructure/vault-config/job.yaml` around lines 803 - 805, The
Vault policy in the job manifest has duplicate stanzas for the actual-budget
paths, so remove the repeated entries and keep only one
`secret/data/apps/actual-budget/*` block and one
`secret/metadata/apps/actual-budget/*` block. Update the policy definition in
the section that currently contains these repeated path rules so the resulting
Vault policy remains consistent and easier to audit.

666-679: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Delete the duplicate app-actual-budget-readonly policy block.
bao policy write app-actual-budget-readonly appears twice with the same secret/data/apps/actual-budget/* read rule, so one copy is redundant and risks the two blocks drifting apart later.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@k8s/bases/infrastructure/vault-config/job.yaml` around lines 666 - 679, The
duplicated app-actual-budget-readonly policy definition should be removed so the
policy is defined only once. In the vault config job manifest, locate the
repeated bao policy write app-actual-budget-readonly block that grants read
access to secret/data/apps/actual-budget/* and delete the redundant copy,
keeping a single canonical policy stanza to avoid future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@k8s/bases/infrastructure/vault-config/job.yaml`:
- Around line 803-805: The Vault policy in the job manifest has duplicate
stanzas for the actual-budget paths, so remove the repeated entries and keep
only one `secret/data/apps/actual-budget/*` block and one
`secret/metadata/apps/actual-budget/*` block. Update the policy definition in
the section that currently contains these repeated path rules so the resulting
Vault policy remains consistent and easier to audit.
- Around line 666-679: The duplicated app-actual-budget-readonly policy
definition should be removed so the policy is defined only once. In the vault
config job manifest, locate the repeated bao policy write
app-actual-budget-readonly block that grants read access to
secret/data/apps/actual-budget/* and delete the redundant copy, keeping a single
canonical policy stanza to avoid future drift.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ab1354ca-9cfa-47b6-bf54-c3c736bb88a6

📥 Commits

Reviewing files that changed from the base of the PR and between a43969e and e45f304.

📒 Files selected for processing (1)
  • k8s/bases/infrastructure/vault-config/job.yaml

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 30, 2026
@devantler
devantler added this pull request to the merge queue Jun 30, 2026
Merged via the queue into main with commit 92f37d2 Jun 30, 2026
15 checks passed
@devantler
devantler deleted the claude/openbao-actual-budget-acl branch June 30, 2026 17:26
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Jun 30, 2026
@botantler-1

botantler-1 Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.88.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant