Draft: HYPERFLEET-732 - feat: add RFC4122 UUID identifiers to Cluster and NodePool resources

[86254860]

Summary:

Add RFC4122 UUID field alongside existing KSUID identifiers to support platform integrations requiring standard UUID format (e.g., Hypershift spec.clusterID).

Changes:

• Add uuid column to clusters and node_pools tables with backfill migrations
• Auto-generate UUIDs in BeforeCreate hooks using google/uuid library
• Update presenters to serialize UUIDs in API responses
• Add comprehensive unit and integration tests for UUID generation and uniqueness
• Update OpenAPI spec with uuid field and documentation
• Document dual identifier system (KSUID for API paths, UUID for integrations)

Summary by CodeRabbit

• New Features

  • Added RFC4122 UUIDs to Cluster and NodePool resources alongside existing IDs for external integrations.

• Documentation

  • Updated API docs and OpenAPI examples to describe the dual-identifier system and clarify which identifier to use.

• Chores

  • Added database migrations to backfill and enforce UUID columns; bumped OpenAPI spec version.

• Tests

  • Added unit and integration tests validating UUID generation, format, uniqueness, immutability, and presenter error handling.

[openshift-ci]

[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 rafabene 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:

• OWNERS

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

[coderabbitai]

Walkthrough

This PR introduces a dual-identifier system: a persistent, immutable RFC4122 uuid field is added to Cluster and NodePool alongside the existing KSUID id. OpenAPI docs and operator guide were updated. Model BeforeCreate hooks now idempotently generate id and uuid when missing. Database migrations add nullable uuid columns, backfill with gen_random_uuid(), set NOT NULL, and create unique indexes scoped to non-deleted rows. Presenters convert stored UUID strings to OpenAPI UUID types and now error on malformed UUIDs. Unit and integration tests cover generation, format, uniqueness, and idempotency.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client
    participant API as API Handler
    participant Model as Cluster/NodePool BeforeCreate
    participant DB as Database
    participant Presenter as API Presenter

    Client->>API: POST /clusters or /node_pools
    API->>Model: Construct resource struct
    Model->>Model: Generate ID (KSUID) if empty
    Model->>Model: Generate UUID (RFC4122) if empty
    API->>DB: INSERT resource
    DB->>DB: Enforce unique indexes (id, uuid)
    DB-->>API: Return persisted row
    API->>Presenter: Build response
    Presenter->>Presenter: Parse string UUID -> openapi UUID type
    Presenter-->>API: Return formatted response (id, uuid)
    API-->>Client: HTTP 201 + response

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 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.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title clearly identifies the main change: adding RFC4122 UUID identifiers to Cluster and NodePool resources, which is the primary focus across all modified files.

_{✏️ 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.}

[coderabbitai]

🧹 Nitpick comments (2)

pkg/api/cluster_types_test.go (1)

245-271: Test function name is slightly misleading.

The test TestCluster_BeforeCreate_UUIDImmutable actually verifies that UUID is regenerated on repeated BeforeCreate calls (lines 263-264). The name suggests immutability, but the test confirms the opposite behavior. Consider renaming to something like TestCluster_BeforeCreate_UUIDRegeneratedOnRepeatedCall for clarity.

The test logic itself is correct and the inline comment explains the expected behavior well.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/api/cluster_types_test.go` around lines 245 - 271, Rename the misleading
test function TestCluster_BeforeCreate_UUIDImmutable to reflect its actual
behavior (e.g., TestCluster_BeforeCreate_UUIDRegeneratedOnRepeatedCall); update
the function declaration and any references or test-suite calls to that symbol
so the test name matches that it verifies UUID/ID are regenerated on a second
BeforeCreate invocation (Cluster.BeforeCreate).

pkg/api/node_pool_types_test.go (1)

286-312: Misleading test name: test verifies regeneration, not immutability.

The test TestNodePool_BeforeCreate_UUIDImmutable actually tests that UUID is regenerated on each BeforeCreate call. The name suggests immutability but the assertions verify the opposite. Consider renaming to TestNodePool_BeforeCreate_UUIDRegeneration to match the actual behavior being tested.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/api/node_pool_types_test.go` around lines 286 - 312, Rename the misnamed
test function TestNodePool_BeforeCreate_UUIDImmutable to reflect its actual
assertion of regeneration (suggested name:
TestNodePool_BeforeCreate_UUIDRegeneration); update the test function
declaration and any references to it so the function name and any comments match
that it verifies UUID and ID are regenerated on successive BeforeCreate calls
(referencing the NodePool struct and its BeforeCreate method to locate the
test).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/api/cluster_types_test.go`:
- Around line 245-271: Rename the misleading test function
TestCluster_BeforeCreate_UUIDImmutable to reflect its actual behavior (e.g.,
TestCluster_BeforeCreate_UUIDRegeneratedOnRepeatedCall); update the function
declaration and any references or test-suite calls to that symbol so the test
name matches that it verifies UUID/ID are regenerated on a second BeforeCreate
invocation (Cluster.BeforeCreate).

In `@pkg/api/node_pool_types_test.go`:
- Around line 286-312: Rename the misnamed test function
TestNodePool_BeforeCreate_UUIDImmutable to reflect its actual assertion of
regeneration (suggested name: TestNodePool_BeforeCreate_UUIDRegeneration);
update the test function declaration and any references to it so the function
name and any comments match that it verifies UUID and ID are regenerated on
successive BeforeCreate calls (referencing the NodePool struct and its
BeforeCreate method to locate the test).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0619b736-1b59-470f-8a2d-083b2d534de1

📥 Commits

Reviewing files that changed from the base of the PR and between 34ae499 and 080aee5.

📒 Files selected for processing (13)

• docs/api-operator-guide.md
• openapi/openapi.yaml
• pkg/api/cluster_types.go
• pkg/api/cluster_types_test.go
• pkg/api/node_pool_types.go
• pkg/api/node_pool_types_test.go
• pkg/api/presenters/cluster.go
• pkg/api/presenters/node_pool.go
• pkg/db/migrations/202603230001_add_cluster_uuid.go
• pkg/db/migrations/202603230002_add_nodepool_uuid.go
• pkg/db/migrations/migration_structs.go
• test/integration/clusters_test.go
• test/integration/node_pools_test.go

[86254860]

/hold This method might be replaced by the method that change the existing ID field to RFC4122.