feat(policy): use name as the sole policy identifier - #292
Merged
Conversation
Folds in the substantive work from PR #284 (issue #250) onto current main, resolving the drift from the spike-sdk-go v0.19.9 migration. SPIKE policies are uniquely identified by their user-provided name, so the auto-generated UUID id added drift and a practically unused column. This removes it end to end: - Drop the id column from the policies table; name is now the PRIMARY KEY. Upsert, load, and delete queries all key on name. - Persist, state, memory backend, and CLI operate on name; delete and load-by-name replace the id-based paths. - google/uuid is no longer imported directly (demoted to indirect). Conflict resolutions preserve main's post-migration idioms: - persist StorePolicy/DeletePolicy keep the withSerializableTx helper while switching to name-based args - CLI keeps the context.Context threading; findPolicyByName uses the ctx-aware ListAllPolicies The SDK still exposes the identifier as PolicyReadRequest.ID and PolicyDeleteRequest.ID, so request.ID carries the policy name at the route boundary. This bridge is documented inline; issue #250 tracks the pending SDK field rename. (TODO markers from the original PR were converted to plain comments per the no-TODO-in-main rule.) make test and make audit both pass; govulncheck reports 0 affecting vulnerabilities. Spec: TBD Co-authored-by: Ali Mohammed <terminalexe1@gmail.com> Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com>
Member
Author
|
Supersedes and closes: #284 |
Member
Author
|
reminder. Issue #250 is still open and it is related to this and possibly the next thing to be done. |
This was referenced Jul 14, 2026
v0lkan
added a commit
that referenced
this pull request
Jul 16, 2026
* fix(bare-metal): preflight-check SPIRE binaries with actionable guidance Running `make start` without spire-server/spire-agent on PATH failed several steps in with a bare "spire-server: command not found" from the agent-token script, after already building the SPIKE binaries and starting the SPIRE server. Add a SPIRE preflight to start.sh, right after the domain check, that fails fast with a clear message: which binaries are missing, how to build and install them (./hack/bare-metal/build/build-spire.sh), how to add an existing install to PATH, and how to verify. It is gated by the same SPIKE_SKIP_* flags as the steps that use each binary, so external-SPIRE workflows that skip those steps are not blocked. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * build(bare-metal): add `make build-spire` target for SPIRE binaries Wrap ./hack/bare-metal/build/build-spike.sh's SPIRE counterpart in a make target, matching how the rest of the bare-metal workflow is driven (`make start`, `make build`, `make bootstrap`). Point the start.sh SPIRE preflight message at `make build-spire` instead of the raw script path so the guidance uses the conventional entry point. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * fix(bare-metal): build-spire.sh cleans up its clone; gitignore /spire/ build-spire.sh cloned SPIRE into ./spire, moved the built binaries to /usr/local/bin, and left the 45MB clone behind. Since ./spire was not gitignored, it showed up as untracked and broke `make audit` (the `gofmt -l .` step scans the SPIRE source). Clone into a temporary directory instead and remove it on exit via a trap, so the working tree stays clean even if the build fails. Add `set -euo pipefail` for fail-fast behavior, and gitignore /spire/ as a belt-and-suspenders guard against a repo-root clone from older runs or a manual git clone. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * fix(bare-metal): preflight-check SPIKE binaries with actionable guidance start.sh invoked spike and demo as bare PATH commands with no check, so a missing binary surfaced as a raw "spike: command not found" halfway through startup, after SPIRE and the Keepers were already running. The only existing check ran when SPIKE_SKIP_SPIKE_BUILD was set, and it did not cover demo. Replace it with a single check_spike_binaries preflight that runs right after the build step (on a fresh clone the binaries do not exist until the build produces them) and verifies all five binaries (spike, nexus, keeper, bootstrap, demo) are on PATH. On failure it exits before any SPIRE or SPIKE process starts, with concrete guidance: `make build`, a copy-pasteable export PATH line, and a verify command. Also update the closing hint to run 'spike' instead of the stale './spike'. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * fix(bare-metal): detect PATH-shadowed SPIKE binaries in the preflight The binary names (spike, nexus, keeper, bootstrap, demo) are generic enough to collide with tools a developer already has on PATH, and the docs recommend appending the repo's bin directory to PATH, so an existing lookalike wins silently. The preflight then reported green while the run was guaranteed to break later: the SPIRE entries pin the exact binary path and hash (unix:path and unix:sha256 selectors), so a lookalike cannot obtain an identity and the failure surfaces as cryptic attestation errors. Teach check_spike_binaries to also verify that each name resolves to the binary in ./bin (via the same-file test, so symlinks are fine). On mismatch it fails fast, naming the conflicting paths and suggesting a prepend-style export PATH fix. When SPIKE_SKIP_REGISTER_ENTRIES is set, the entries may legitimately point elsewhere, so the mismatch downgrades to a warning and the run continues. Keeping PATH-based invocation is deliberate: having the binaries on PATH is the user-facing convenience, and the harness sharing the same resolution keeps one consistent story. This check only makes the conflict case loud instead of silent. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * fix(nexus): validate policy names, not UUIDs, in get/delete guards Policies use the name as their sole identifier since #292, and the ID field of PolicyReadRequest and PolicyDeleteRequest carries the policy name; the route handlers already treat it that way. The request guards, however, still called net.RespondErrOnBadPolicyID, which enforces UUID format, so every `spike policy get <name>` and `spike policy delete <name>` was rejected with "400 Bad Request" before reaching its handler. Validate with net.RespondErrOnBadName instead, the same validator the put guard applies to policy names on create. Input validation stays in place; only the format it enforces changes. The SDK-side field rename (ID to Name) is tracked in issue #250. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * fix(pilot): drop the vestigial ID field from human policy output Policies are keyed by name and no longer have IDs since #292, but the human formatters for `spike policy list` and `spike policy get` still printed an "ID:" line, now always empty. Remove the line from both, update the format tests accordingly, and rewrite the stale doc example of `spike policy list`, which showed fields the list endpoint never returns. JSON and YAML outputs still contain an empty "id" field because the SDK's PolicyListItem and Policy types carry it; that goes away with the SDK field rename tracked in issue #250. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * fix(bare-metal): align start.sh policy validation with the CLI output The policy checks in start.sh grepped for output formats that no longer exist: the tabular "ID NAME" header (gone since #274 standardized the CLI output) and "Permissions:" lines, which `spike policy list` has not printed since the block format landed. The checks failed on every run and were masked by the debug-mode "continuing anyway" path. Match the current contract instead: expect one "Name: <policy>" block per policy from `spike policy list`, and verify permissions through `spike policy get`, which is where permissions are shown and which also exercises the get-by-name path end to end. On failure, dump the list and get outputs to make the next format drift easy to diagnose. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> --------- Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com>
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
1 participant
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.
Folds in the substantive work from PR #284 (issue #250) onto current main, resolving the drift from the spike-sdk-go v0.19.9 migration.
SPIKE policies are uniquely identified by their user-provided name, so the auto-generated UUID id added drift and a practically unused column. This removes it end to end:
Conflict resolutions preserve main's post-migration idioms:
The SDK still exposes the identifier as PolicyReadRequest.ID and PolicyDeleteRequest.ID, so request.ID carries the policy name at the route boundary. This bridge is documented inline; issue #250 tracks the pending SDK field rename. (TODO markers from the original PR were converted to plain comments per the no-TODO-in-main rule.)
make test and make audit both pass; govulncheck reports 0 affecting vulnerabilities.
cc: @Aliexe-code
Spec: TBD