Skip to content

HYPERFLEET-732 - feat: replace existing IDs with RFC4122 UUID v7 for both Cluster and NodePool IDs#29

Open
86254860 wants to merge 1 commit intoopenshift-hyperfleet:mainfrom
86254860:HYPERFLEET-732-replace-id-with-uuid
Open

HYPERFLEET-732 - feat: replace existing IDs with RFC4122 UUID v7 for both Cluster and NodePool IDs#29
86254860 wants to merge 1 commit intoopenshift-hyperfleet:mainfrom
86254860:HYPERFLEET-732-replace-id-with-uuid

Conversation

@86254860
Copy link

@86254860 86254860 commented Mar 24, 2026

Summary

Replace existing IDs with RFC4122 UUID v7 for both Cluster and NodePool IDs

  • HYPERFLEET-732

Test Plan

No makefile in hyperfleet-api-spec repo

  • Unit tests added/updated
  • make test-all passes
  • make lint passes
  • Helm chart changes validated with make test-helm (if applicable)
  • Deployed to a development cluster and verified (if Helm/config changes)
  • E2E tests passed (if cross-component or major changes)

Summary by CodeRabbit

  • Chores

    • Bumped API version to 1.0.5
  • Documentation

    • Updated API docs, OpenAPI/Swagger metadata, and schema examples to use UUID-style resource identifiers for Cluster and NodePool examples (replacing prior placeholder IDs and hrefs)

@openshift-ci openshift-ci bot requested review from vkareh and xueli181114 March 24, 2026 10:27
@openshift-ci
Copy link

openshift-ci bot commented Mar 24, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yasun1 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link

coderabbitai bot commented Mar 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d8e8e2a7-e783-45b1-8008-dadd668a9a0e

📥 Commits

Reviewing files that changed from the base of the PR and between 82da243 and eae1c79.

📒 Files selected for processing (10)
  • main.tsp
  • models-core/cluster/example_cluster.tsp
  • models-core/nodepool/example_nodepool.tsp
  • models-gcp/cluster/example_cluster.tsp
  • models-gcp/nodepool/example_nodepool.tsp
  • models/common/model.tsp
  • schemas/core/openapi.yaml
  • schemas/core/swagger.yaml
  • schemas/gcp/openapi.yaml
  • schemas/gcp/swagger.yaml
✅ Files skipped from review due to trivial changes (7)
  • main.tsp
  • models/common/model.tsp
  • models-gcp/cluster/example_cluster.tsp
  • models-core/nodepool/example_nodepool.tsp
  • models-gcp/nodepool/example_nodepool.tsp
  • schemas/core/swagger.yaml
  • schemas/core/openapi.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
  • models-core/cluster/example_cluster.tsp
  • schemas/gcp/swagger.yaml
  • schemas/gcp/openapi.yaml

Walkthrough

This pull request bumps the API version from 1.0.4 to 1.0.5 in service metadata and OpenAPI/Swagger files, updates example resource identifiers across TypeSpec models and schema examples to UUIDs (Cluster: 019466a0-8f8e-7abc-9def-0123456789ab; NodePool: 019466a1-2b3c-7def-8abc-456789abcdef), and adds a new exported Identifier type alias documented as an RFC4122 UUID v7. No routes, service logic, or control flow were changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and directly summarizes the main change: replacing existing IDs with RFC4122 UUID v7 for both Cluster and NodePool, which aligns perfectly with all file modifications across the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@86254860
Copy link
Author

/hold for other PRs of this feature, don't affect PR review.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@models/common/model.tsp`:
- Around line 6-7: The Identifier alias currently defined as "alias Identifier =
string;" doesn't enforce a UUID v7 format and the example shows a UUIDv4; change
Identifier to a constrained/branded type that enforces UUID v7 (e.g., a branded
type or a string refinement with a UUID v7 regex that requires the '7' in the
version nibble), update the documentation comment example to a valid UUIDv7
string (with '7' in the version position), and use the new Identifier type in
any places that previously referenced the free-form string so consumers get
compile-time/type-check guarantees; reference the Identifier alias in
models/common/model.tsp and adjust any validation helpers that construct/parse
Identifiers to validate against the UUID v7 pattern.

In `@schemas/gcp/swagger.yaml`:
- Around line 701-703: The example UUIDs use a v4 version nibble ("-41d4-..."
e.g. id value "550e8400-e29b-41d4-a716-446655440000" and matching href) but the
API contract requires RFC4122 UUID v7; update all example UUID strings across
the schema files (including occurrences in symbols like href and id under
cluster examples) so the third group begins with "7" (e.g., replace the "41d4"
nibble with a v7 equivalent), ensuring consistency in gcp/swagger.yaml,
gcp/openapi.yaml, core/swagger.yaml and core/openapi.yaml.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6802d8fd-546d-489c-96ba-0bff2ea8420f

📥 Commits

Reviewing files that changed from the base of the PR and between ebeb009 and 82da243.

📒 Files selected for processing (10)
  • main.tsp
  • models-core/cluster/example_cluster.tsp
  • models-core/nodepool/example_nodepool.tsp
  • models-gcp/cluster/example_cluster.tsp
  • models-gcp/nodepool/example_nodepool.tsp
  • models/common/model.tsp
  • schemas/core/openapi.yaml
  • schemas/core/swagger.yaml
  • schemas/gcp/openapi.yaml
  • schemas/gcp/swagger.yaml

Comment on lines 6 to 7
/** Resource identifier in RFC4122 UUID v7 format (time-ordered, e.g., "550e8400-e29b-41d4-a716-446655440000") */
alias Identifier = string;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Identifier does not enforce UUID v7 and the documented example is UUIDv4.

The alias is still an unconstrained string, so the contract does not guarantee UUID input. Also, the example 550e8400-e29b-41d4-a716-446655440000 is v4 (4 in the version position), not v7, which conflicts with the PR objective and propagates incorrect examples downstream.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@models/common/model.tsp` around lines 6 - 7, The Identifier alias currently
defined as "alias Identifier = string;" doesn't enforce a UUID v7 format and the
example shows a UUIDv4; change Identifier to a constrained/branded type that
enforces UUID v7 (e.g., a branded type or a string refinement with a UUID v7
regex that requires the '7' in the version nibble), update the documentation
comment example to a valid UUIDv7 string (with '7' in the version position), and
use the new Identifier type in any places that previously referenced the
free-form string so consumers get compile-time/type-check guarantees; reference
the Identifier alias in models/common/model.tsp and adjust any validation
helpers that construct/parse Identifiers to validate against the UUID v7
pattern.

@86254860 86254860 force-pushed the HYPERFLEET-732-replace-id-with-uuid branch from 82da243 to eae1c79 Compare March 25, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant