HYPERFLEET-732 - feat: replace existing IDs with RFC4122 UUID v7 for both Cluster and NodePool IDs#29
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis 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 Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/hold for other PRs of this feature, don't affect PR review. |
There was a problem hiding this comment.
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
📒 Files selected for processing (10)
main.tspmodels-core/cluster/example_cluster.tspmodels-core/nodepool/example_nodepool.tspmodels-gcp/cluster/example_cluster.tspmodels-gcp/nodepool/example_nodepool.tspmodels/common/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yaml
models/common/model.tsp
Outdated
| /** Resource identifier in RFC4122 UUID v7 format (time-ordered, e.g., "550e8400-e29b-41d4-a716-446655440000") */ | ||
| alias Identifier = string; |
There was a problem hiding this comment.
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.
…both Cluster and NodePool IDs
82da243 to
eae1c79
Compare
Summary
Replace existing IDs with RFC4122 UUID v7 for both Cluster and NodePool IDs
Test Plan
No makefile in hyperfleet-api-spec repo
make test-allpassesmake lintpassesmake test-helm(if applicable)Summary by CodeRabbit
Chores
Documentation