Skip to content

feat: simplify policy identification by using name as primary identif…#284

Open
Aliexe-code wants to merge 1 commit into
spiffe:mainfrom
Aliexe-code:feature/250-remove-policy-id
Open

feat: simplify policy identification by using name as primary identif…#284
Aliexe-code wants to merge 1 commit into
spiffe:mainfrom
Aliexe-code:feature/250-remove-policy-id

Conversation

@Aliexe-code

@Aliexe-code Aliexe-code commented Feb 9, 2026

Copy link
Copy Markdown
Contributor

Summary
This PR removes the auto-generated UUID ID field from SPIKE policies and uses the user-provided name as the sole unique identifier, aligning with HashiCorp Vault's approach for better UX and simplicity.

Problem
SPIKE policies had two identifiers:

  • ID: Auto-generated UUID (e.g., 550e8400-e29b-41d4-a716-446655440000)
  • name: User-provided name (e.g., database-policy)

Testing

  • All existing tests pass

  • Build succeeds

  • No breaking changes to existing behavior (IDs are internal only)

  • This change is backward compatible for end users since they always used name in CLI commands. The ID field was only used internally and never exposed to users in the API responses of these endpoints.
    Follow-up Required ⚠️

SDK Dependency: The spike-sdk-go package contains request/response structs that still use ID fields. This PR includes a temporary workaround using the SDK's ID field but treating it as the policy name. A follow-up PR to the SDK is needed to:

@v0lkan

v0lkan commented Apr 10, 2026

Copy link
Copy Markdown
Member

Hey @Aliexe-code ; just saw this; will check it out and get back.

@v0lkan

v0lkan commented Apr 11, 2026

Copy link
Copy Markdown
Member

Quick update:

Overall, it makes sense.

But I have thoughts about

maybe removing policy ids altogether (name in itself uniquely identifies the policy anyway)

an autogenerated ID "can" be helpful; but I'm still not sure.

I'm adding this issue to my current radar to think about it further.

I'll update here.

@Aliexe-code

Copy link
Copy Markdown
Contributor Author

Hey @v0lkan - sounds good, Let me know if you want me to make any changes, or if you'd prefer to remove the ID field entirely. Happy to adjust based on your preference.

@v0lkan

v0lkan commented Jun 10, 2026

Copy link
Copy Markdown
Member

bah! can someone invent compressing spacetime continuum already :D.

No promises, but I want to spend this weekend to go over all these issues.

@v0lkan

v0lkan commented Jul 14, 2026

Copy link
Copy Markdown
Member

Hey @Aliexe-code -- I'm folding this in; but I'll probably create a new PR and reference this one due to the merge conflicts here.

Thanks a lot for your help 🙏 .

@v0lkan

v0lkan commented Jul 14, 2026

Copy link
Copy Markdown
Member

I'm adding you as "co-author" to the commit since the majority of the work is yours.

v0lkan added a commit that referenced this pull request Jul 14, 2026
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

Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com>
Co-authored-by: Ali Mohammed <terminalexe1@gmail.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.

2 participants