Skip to content

feat: add OwnerMigrationStore adapter for vault org_id migration#21691

Open
prashantkumar1982 wants to merge 1 commit intodevelopfrom
feat/pr5-owner-migration-adapter
Open

feat: add OwnerMigrationStore adapter for vault org_id migration#21691
prashantkumar1982 wants to merge 1 commit intodevelopfrom
feat/pr5-owner-migration-adapter

Conversation

@prashantkumar1982
Copy link
Contributor

@prashantkumar1982 prashantkumar1982 commented Mar 25, 2026

Summary

  • Adds OwnerMigrationReadStore and OwnerMigrationWriteStore adapters in core/services/ocr2/plugins/vault/owner_migration_store.go that implement the existing ReadKVStore / WriteKVStore interfaces with transparent dual-owner (org_id / workflow_owner) migration logic.
  • Reads perform dual-owner lookup (org_id first, workflow_owner fallback); writes always go under org_id with lazy migration of legacy entries; list operations merge and deduplicate metadata across both owners.
  • Includes 46 comprehensive unit tests covering all operations, migration scenarios, edge cases, and error propagation.

This is a standalone component in the JWT auth rollout plan — not wired into the plugin yet. No behavior change.

Add a transparent adapter layer (OwnerMigrationReadStore / OwnerMigrationWriteStore)
that sits between the vault plugin and the KV store to handle the migration of
secrets from workflow_owner-keyed entries to org_id-keyed entries.

The adapter implements the same ReadKVStore / WriteKVStore interfaces and provides:
- Dual-owner lookup on reads (org_id first, workflow_owner fallback)
- Metadata merging and deduplication across both owners for list operations
- org_id-based writes for all new/updated secrets
- Lazy migration: deletes legacy workflow_owner entries on update
- Dual-owner deletion with cleanup of both owners
- Pass-through for pending queue operations (not owner-scoped)

This is a standalone component (not wired into the plugin yet) with comprehensive
unit tests covering all operations and migration scenarios.
@github-actions
Copy link
Contributor

👋 prashantkumar1982, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

@github-actions
Copy link
Contributor

I see you updated files related to core. Please run make gocs in the root directory to add a changeset as well as in the text include at least one of the following tags:

  • #added For any new functionality added.
  • #breaking_change For any functionality that requires manual action for the node to boot.
  • #bugfix For bug fixes.
  • #changed For any change to the existing functionality.
  • #db_update For any feature that introduces updates to database schema.
  • #deprecation_notice For any upcoming deprecation functionality.
  • #internal For changesets that need to be excluded from the final changelog.
  • #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
  • #removed For any functionality/config that is removed.
  • #updated For any functionality that is updated.
  • #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

@github-actions
Copy link
Contributor

✅ No conflicts with other open PRs targeting develop

@trunk-io
Copy link

trunk-io bot commented Mar 25, 2026

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube IDE SonarQube IDE


var _ ReadKVStore = (*OwnerMigrationReadStore)(nil)

func NewOwnerMigrationReadStore(inner ReadKVStore, orgID, workflowOwner string) *OwnerMigrationReadStore {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how this would be transparent since this implies that we'll have one read store per owner/org ID. Atm we typically instantiate one readStore for every plugin function call

}

// org_id entries take priority in deduplication.
addEntries(orgMd)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a risk that the merged list will end up having more than the max allowed number of secrets?

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