Skip to content

feat: central vault ingot unlocks against - #37

Open
alanshaw wants to merge 1 commit into
mainfrom
ash/feat/central-openbao
Open

alanshaw wants to merge 1 commit into
mainfrom
ash/feat/central-openbao

Conversation

@alanshaw

Copy link
Copy Markdown
Member

Adds an OpenBao instance "central" that Ingot's OpenBao instance unlocks against.

@alanshaw
alanshaw requested a lite review from Copilot August 26, 2026 15:03

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Comment thread README.md
| ingot | 15130 | S3 gateway over Forge |
| ingot-postgres | 15131 | Postgres for ingot's registry/metadata |
| ingot-openbao | 15132 | Region KEK vault for ingot (OpenBao, raft storage) |
| central-openbao | 15210 | Seal root for appliance OpenBaos (transit auto-unseal) |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am wondering if we really need to exercise OpenBao unlocking in the ephemeral dev deployments created by Smelt. It feels like unneeded complexity to me.

The real dev/staging/prod deployment will not use Smelt, so we are gaining very little by adding the central OpenBao here.

The real process for setting up "local OpenBao unsealed by central OpenBao" is a bit complex, see https://github.com/fil-forge/infra-central/blob/main/docs/decisions/2026-08-region-onboarding.md, fil-forge/infra-central#50 and fil-forge/infra-central#49. (Note: I haven't exercised this process yet; the final version may be slightly different if I need to fix any bugs.)

@bajtos bajtos Aug 27, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My proposal: keep it simple in Smelt, let Ingot's OpenBao unseal automatically using a locally stored secret.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It just means it's impossible to test that revoked central access means an appliance can no longer read/serve encrypted data or that adding an appliance to the network in the first instance is blocked by it being authorised by the central vault.

Perhaps that is the useful functionality and tests we should add to this PR to make it worthwhile?

@bajtos bajtos Aug 27, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see your point.

I am not sure if this is the right place for testing these two features. In the current direction, Smelt is used for ephemeral deployments only. Long-living deployments will use infra-central & infra-nodes. (Why? The first nudge towards that direction comes from #11 (comment). More importantly, Smelt deploys both the central and the regional services together, while the real deployments need to manage them differently.)

Do we need to periodically test whether "local OpenBao unsealed via central OpenBao" still works as a concept? I don't see much value in that.

Also: Revoking the unsealing key in the central OpenBao is not enough to retire a node - the node can keep operating as long as it does not restart. See https://linear.app/filecoin-foundation/issue/FIL-1090 for more details.

adding an appliance to the network in the first instance is blocked by it being authorised by the central vault.

It all depends on how the appliance is configured. In particular, how the node operator configures their OpenBao instance.

  • The "canonical" recipe lives in infra-nodes, not Smelt.
  • Unless we have full control of the entire appliance stack, including all dependencies like OpenBao, we cannot rely just on the fact that our canonical recipe gives us this guarantee. We would need to implement some sort of audit to verify that the node is using the central OpenBao for unsealing.

Anyhow, I don't feel too strongly about this. If you see value in adding the central vault to Smelt deployments, then go for it.

hannahhoward added a commit that referenced this pull request Sep 27, 2026
## Summary
- Postgres, OpenBao, Redis, DynamoDB Local and smtp4dev now read
`POSTGRES_IMAGE`, `OPENBAO_IMAGE`, `REDIS_IMAGE`, `DYNAMODB_LOCAL_IMAGE`
and `SMTP4DEV_IMAGE`, with today's tag as each default. A benchmark host
can pin them by digest from the environment instead of a compose
override file.
- With the variables unset, `docker compose config` output is
byte-identical to `main` for every tracked manifest and for a mixed
postgres+s3 / sqlite+filesystem manifest.
- Setting each variable changes the `image:` of exactly the expected
services and nothing else.

## Deliverables
| File | Status | Summary |
|------|--------|---------|
| `systems/ingot/compose.yml` | Modified | `ingot-openbao`,
`ingot-openbao-init` read `OPENBAO_IMAGE`; `ingot-postgres` reads
`POSTGRES_IMAGE` |
| `systems/hilt/compose.yml` | Modified | `hilt-vault` reads
`OPENBAO_IMAGE`; `hilt-postgres` reads `POSTGRES_IMAGE` |
| `systems/upload/compose.yml` | Modified | `postgres` reads
`POSTGRES_IMAGE` |
| `systems/swarf/compose.yml` | Modified | `swarf-postgres` reads
`POSTGRES_IMAGE` |
| `systems/plc/compose.yml` | Modified | `plc-postgres` reads
`POSTGRES_IMAGE` |
| `systems/common/compose.yml` | Modified | `dynamodb-local` reads
`DYNAMODB_LOCAL_IMAGE`; `email` reads `SMTP4DEV_IMAGE` |
| `systems/indexing/indexer/compose.yml` | Modified | `redis` reads
`REDIS_IMAGE` |
| `pkg/generate/compose.go` | Modified | `piri-postgres` and
`piri-postgres-init` emit `${POSTGRES_IMAGE:-postgres:16-alpine}` |
| `pkg/generate/generate_test.go` | Modified | Asserts the generated
postgres services use the interpolated image |
| `images_test.go` | New | Pins each third-party service to its variable
and default; fails if any image in the root stack has no override
variable |
| `.env` | Modified | Commented "Third-party" block listing the
variables |

## Test plan
- `go vet ./...` and `go test ./...`: 6 packages ok (4 without test
files), 0 failures.
- New tests fail against `main`'s compose files (checked by restoring
`systems/common/compose.yml` from `main`:
`TestIncludedComposeImagesAreInterpolated` reports `dynamodb-local` and
`email`).
- Render diff: `go run ./cmd/smelt generate` then `docker compose
config` (Compose 2.31) for `smelt.yml`,
`manifests/piri-1-postgres-filesystem.yml` and a mixed two-node
manifest, on `main` and on this branch, variables unset: 3/3 identical.
- Per variable, set to a dummy ref, compared with the unset render; only
`image:` lines differ:

| Variable | Services changed |
|---|---|
| `POSTGRES_IMAGE` | hilt-postgres, ingot-postgres, piri-postgres,
piri-postgres-init, plc-postgres, postgres, swarf-postgres |
| `OPENBAO_IMAGE` | hilt-vault, ingot-openbao, ingot-openbao-init |
| `REDIS_IMAGE` | redis |
| `DYNAMODB_LOCAL_IMAGE` | dynamodb-local |
| `SMTP4DEV_IMAGE` | email |

- No shell scripts touched, so no shellcheck run applies.

## Decisions taken where the spec was silent
- Names are unprefixed (`POSTGRES_IMAGE`, not `SMELT_POSTGRES_IMAGE`) to
follow the existing `<NAME>_IMAGE` convention in `.env`. A `SMELT_`
prefix would avoid collisions with a variable already set in a
developer's shell; maintainers may prefer it.
- `systems/telemetry/compose.yml` keeps its literal tags: it is not
included by the root `compose.yml`, and the new guard test covers only
the root stack.
- The `.env` block also lists `MINIO_IMAGE`, which the stack already
reads but `.env` did not mention.
- If the open central-OpenBao PR (#37) lands first, its OpenBao service
should take `OPENBAO_IMAGE` too; the guard test will flag it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants