Support migrated-namespace and migrated-uid annotations by mangelajo · Pull Request #190 · jumpstarter-dev/jumpstarter-controller

mangelajo · Nov 7, 2025

Enable Client/Exporter migration across clusters using annotations to override namespace and UID in authentication tokens. When these annotations are present, tokens authenticate using the migrated values instead of the current object's namespace/UID.

Annotations:

jumpstarter.dev/migrated-namespace
jumpstarter.dev/migrated-uid

Fixes-Issue: jumpstarter-dev/jumpstarter#25

Summary by CodeRabbit

New Features
- Added support for migration annotations that can override namespace and UID used when constructing internal resource subjects.
- Client and exporter subject generation now respects migrated namespace/UID when present.
Tests
- Added unit tests covering migrated-annotation precedence, empty-value handling, and username behavior for client and exporter subjects.
Chores
- Adjusted linter configuration to include duplication checks for API test files.

coderabbitai · Nov 7, 2025

Walkthrough

Internal subject construction now uses a new helper that derives namespace and UID from object metadata and optional migration annotations; client and exporter InternalSubject implementations call this helper before composing their subject strings. Two new exported annotation constants were added and unit tests cover the new behavior.

Changes

Cohort / File(s)	Summary
Constants added `api/v1alpha1/groupversion_info.go`	Add exported annotation keys `AnnotationMigratedNamespace` = `"jumpstarter.dev/migrated-namespace"` and `AnnotationMigratedUID` = `"jumpstarter.dev/migrated-uid"`.
Helper function added `api/v1alpha1/common_helpers.go`	Add `getNamespaceAndUID(namespace string, uid types.UID, annotations map[string]string) (string, string)` which returns namespace and UID, overriding with migrated annotation values when present and non-empty.
InternalSubject updates `api/v1alpha1/client_helpers.go`, `api/v1alpha1/exporter_helpers.go`	InternalSubject now calls `getNamespaceAndUID(...)` to obtain namespace and uid prior to building subject strings (`client:<namespace>:<Name>:<uid>` and `exporter:<namespace>:<name>:<uid>`). No public signatures changed.
Tests added `api/v1alpha1/*_test.go`	Add table-driven tests for `getNamespaceAndUID`, and unit tests for Client and Exporter `InternalSubject` and `Usernames` covering migrated-annotation overrides, empty values, and username inclusion.
Lint config `.golangci.yml`	Add dupl linter exclusion rule for api test files (pattern `api/.*_test\.go`).
Module file `go.mod`	Present in manifest (no logic changes indicated).

Sequence Diagram(s)

sequenceDiagram
  participant Caller as InternalSubject()
  participant Obj as Object (Namespace, UID, Annotations)
  participant Helper as getNamespaceAndUID
  participant Builder as Subject Builder

  Caller->>Obj: read Namespace, UID, Annotations
  Caller->>Helper: getNamespaceAndUID(namespace, uid, annotations)
  Note right of Helper `#E8F6EF`: If migration annotations present\nand non-empty, return overrides
  Helper-->>Caller: namespace', uid'
  Caller->>Builder: format subject with prefix, namespace', name, uid'
  Builder-->>Caller: "client:<ns>:<Name>:<uid>" / "exporter:<ns>:<name>:<uid>"

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Inspect getNamespaceAndUID for correct handling of nil/empty annotations and UID string conversion.
Verify annotation constant values and usage sites match exactly.
Review tests for adequate coverage of edge cases (empty migrated values, partial overrides).

Possibly related PRs

Always allow clients to use the internal authenticator #103 — modifies InternalSubject/Usernames logic for Client and Exporter and appears to address the same migration-annotation behavior.

Poem

🐇 I sniffed the annotations, moonlit and bright,
Swapped namespaces gently in the night,
UIDs hopped over, settled in place,
Subjects stitched neatly, a soft little trace,
I thumped my foot — migration's in sight!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Support migrated-namespace and migrated-uid annotations' accurately summarizes the main objective of the changeset: adding support for migration annotations to override namespace and UID values in authentication tokens.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch migrated-uuid

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

♻️ Duplicate comments (1)

api/v1alpha1/client_helpers.go (1)

6-17: Duplicate annotation logic (see exporter_helpers.go).

This annotation-checking logic is identical to the code in exporter_helpers.go (lines 13-24). Please refer to the review comment on that file for the recommended refactoring to eliminate this duplication.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88c8e4b and 5ed410e.

📒 Files selected for processing (2)

api/v1alpha1/client_helpers.go (1 hunks)
api/v1alpha1/exporter_helpers.go (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

GitHub Check: lint-go
GitHub Check: e2e-test-operator
GitHub Check: deploy-kind
GitHub Check: e2e-tests-release-0-7
GitHub Check: e2e-tests (ubuntu-24.04-arm)
GitHub Check: tests

api/v1alpha1/exporter_helpers.go

Enable Client/Exporter migration across clusters using annotations to override namespace and UID in authentication tokens. When these annotations are present, tokens authenticate using the migrated values instead of the current object's namespace/UID. Annotations: - jumpstarter.dev/migrated-namespace - jumpstarter.dev/migrated-uid Fixes-Issue: #188

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ed410e and 55af940.

📒 Files selected for processing (4)

api/v1alpha1/client_helpers.go (1 hunks)
api/v1alpha1/common_helpers.go (1 hunks)
api/v1alpha1/exporter_helpers.go (1 hunks)
api/v1alpha1/groupversion_info.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

api/v1alpha1/client_helpers.go
api/v1alpha1/exporter_helpers.go

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter-controller PR: 190
File: api/v1alpha1/exporter_helpers.go:16-24
Timestamp: 2025-11-14T15:47:36.325Z
Learning: In the jumpstarter-controller project, migration annotations (jumpstarter.dev/migrated-namespace and jumpstarter.dev/migrated-uid) that override namespace and UID values in authentication tokens are acceptable without additional validation webhooks because the security model assumes only administrators have write access to Exporter and Client resources via K8s RBAC.

📚 Learning: 2025-11-14T15:47:36.325Z

Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter-controller PR: 190
File: api/v1alpha1/exporter_helpers.go:16-24
Timestamp: 2025-11-14T15:47:36.325Z
Learning: In the jumpstarter-controller project, migration annotations (jumpstarter.dev/migrated-namespace and jumpstarter.dev/migrated-uid) that override namespace and UID values in authentication tokens are acceptable without additional validation webhooks because the security model assumes only administrators have write access to Exporter and Client resources via K8s RBAC.

Applied to files:

api/v1alpha1/common_helpers.go
api/v1alpha1/groupversion_info.go

🧬 Code graph analysis (1)

api/v1alpha1/common_helpers.go (1)

api/v1alpha1/groupversion_info.go (2)

AnnotationMigratedNamespace (40-40)

AnnotationMigratedUID (43-43)

🪛 golangci-lint (2.5.0)

api/v1alpha1/common_helpers.go

[error] 22-22: File is not properly formatted

(gofmt)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: e2e-tests (ubuntu-24.04-arm)
GitHub Check: e2e-tests (ubuntu-24.04)
GitHub Check: e2e-tests-release-0-7
GitHub Check: e2e-test-operator
GitHub Check: deploy-kind
GitHub Check: tests
GitHub Check: lint-go

🔇 Additional comments (2)

api/v1alpha1/groupversion_info.go (1)

38-44: LGTM! Well-defined migration annotation constants.

The constants are properly named, documented, and follow Kubernetes annotation naming conventions.

api/v1alpha1/common_helpers.go (1)

5-21: LGTM! Solid implementation with proper defensive checks.

The helper function correctly handles namespace and UID overrides from migration annotations. The nil check for annotations and non-empty validation for annotation values provide appropriate defensive programming.

Based on learnings.

api/v1alpha1/common_helpers.go

NickCao

LGTM

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

api/v1alpha1/client_helpers_test.go (1)
77-77: Consider defining stringPtr locally for test file independence.

The stringPtr helper is currently defined only in exporter_helpers_test.go (lines 87-89). While Go's test compilation makes it accessible across test files in the same package, defining it in both test files would make each file independently understandable.

You could add the helper to this file as well:
// Helper function to create string pointers
func stringPtr(s string) *string {
	return &s
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 55af940 and 2865b81.

📒 Files selected for processing (5)

.golangci.yml (1 hunks)
api/v1alpha1/client_helpers_test.go (1 hunks)
api/v1alpha1/common_helpers.go (1 hunks)
api/v1alpha1/common_helpers_test.go (1 hunks)
api/v1alpha1/exporter_helpers_test.go (1 hunks)

✅ Files skipped from review due to trivial changes (1)

api/v1alpha1/common_helpers_test.go

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter-controller PR: 190
File: api/v1alpha1/exporter_helpers.go:16-24
Timestamp: 2025-11-14T15:47:36.325Z
Learning: In the jumpstarter-controller project, migration annotations (jumpstarter.dev/migrated-namespace and jumpstarter.dev/migrated-uid) that override namespace and UID values in authentication tokens are acceptable without additional validation webhooks because the security model assumes only administrators have write access to Exporter and Client resources via K8s RBAC.

📚 Learning: 2025-11-14T15:47:36.325Z

Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter-controller PR: 190
File: api/v1alpha1/exporter_helpers.go:16-24
Timestamp: 2025-11-14T15:47:36.325Z
Learning: In the jumpstarter-controller project, migration annotations (jumpstarter.dev/migrated-namespace and jumpstarter.dev/migrated-uid) that override namespace and UID values in authentication tokens are acceptable without additional validation webhooks because the security model assumes only administrators have write access to Exporter and Client resources via K8s RBAC.

Applied to files:

api/v1alpha1/common_helpers.go

🧬 Code graph analysis (3)

api/v1alpha1/common_helpers.go (1)

api/v1alpha1/groupversion_info.go (2)

AnnotationMigratedNamespace (40-40)

AnnotationMigratedUID (43-43)

api/v1alpha1/client_helpers_test.go (2)

api/v1alpha1/client_types.go (2)

Client (44-50)

ClientSpec (28-30)

api/v1alpha1/groupversion_info.go (2)

AnnotationMigratedNamespace (40-40)

AnnotationMigratedUID (43-43)

api/v1alpha1/exporter_helpers_test.go (2)

api/v1alpha1/groupversion_info.go (2)

AnnotationMigratedNamespace (40-40)

AnnotationMigratedUID (43-43)

api/v1alpha1/exporter_types.go (1)

ExporterSpec (28-30)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: tests
GitHub Check: e2e-tests (ubuntu-24.04)
GitHub Check: e2e-tests (ubuntu-24.04-arm)
GitHub Check: e2e-tests-release-0-7
GitHub Check: lint-go
GitHub Check: e2e-test-operator
GitHub Check: deploy-kind

🔇 Additional comments (7)

api/v1alpha1/common_helpers.go (1)

7-21: LGTM! Clean implementation of migration annotation support.

The helper function correctly handles namespace and UID derivation with proper nil and empty-value checks. The logic ensures that empty migration annotations are ignored (lines 12, 15), preventing accidental overrides with blank values.

.golangci.yml (1)

28-30: LGTM! Sensible exclusion for test files.

Allowing code duplication in test files is appropriate since test cases often follow similar patterns for consistency and clarity.

api/v1alpha1/client_helpers_test.go (2)

10-60: Excellent test coverage for InternalSubject.

The three test cases comprehensively cover the migration annotation behavior:

Default behavior without annotations

Override behavior with both annotations present

Fallback behavior when annotations are empty

The expected subject format is correctly validated in each scenario.

62-84: LGTM! Usernames method is thoroughly tested.

Both scenarios (with and without custom username) are properly validated. The test correctly verifies that the internal subject is always included and that custom usernames are appended when present.

api/v1alpha1/exporter_helpers_test.go (3)

10-60: Excellent test coverage for Exporter's InternalSubject.

The test suite comprehensively validates the migration annotation behavior across all scenarios, mirroring the client tests appropriately. The expected subject format with the "exporter:" prefix is correctly validated.

62-84: LGTM! Usernames behavior is well-tested.

Both test cases properly validate the username augmentation logic, ensuring the internal subject and optional custom username are correctly included.

87-89: Good test helper definition.

The stringPtr helper is used across multiple test files in this package. The current placement works due to Go's test file compilation, and the .golangci.yml configuration explicitly allows this duplication pattern.

mangelajo requested a review from NickCao November 7, 2025 11:08

coderabbitai bot reviewed Nov 7, 2025

View reviewed changes

api/v1alpha1/exporter_helpers.go Outdated Show resolved Hide resolved

api/v1alpha1/exporter_helpers.go Outdated Show resolved Hide resolved

mangelajo force-pushed the migrated-uuid branch from 5ed410e to 55af940 Compare November 14, 2025 16:44

coderabbitai bot reviewed Nov 14, 2025

View reviewed changes

api/v1alpha1/common_helpers.go Outdated Show resolved Hide resolved

Add unit testing to migrated-ns/uid

2865b81

NickCao approved these changes Nov 14, 2025

View reviewed changes

coderabbitai bot reviewed Nov 14, 2025

View reviewed changes

mangelajo merged commit 8193ed2 into main Nov 14, 2025
9 checks passed

mangelajo deleted the migrated-uuid branch November 14, 2025 17:31

mangelajo mentioned this pull request Jan 15, 2026

Support portable authentication tokens with backward compatibility #189

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support migrated-namespace and migrated-uid annotations#190

Support migrated-namespace and migrated-uid annotations#190
mangelajo merged 2 commits intomainjumpstarter-dev/jumpstarter-controller:mainfrom
migrated-uuidjumpstarter-dev/jumpstarter-controller:migrated-uuidCopy head branch name to clipboard

mangelajo commented Nov 7, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 7, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

NickCao left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Search code, repositories, users, issues, pull requests...

Conversation

mangelajo commented Nov 7, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NickCao left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mangelajo commented Nov 7, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 7, 2025 •

edited

Loading