Skip to content

ROSAENG-61801: add v2alpha1 public API module with passthrough types - #241

Closed
cdoan1 wants to merge 2 commits into
openshift-online:mainfrom
cdoan1:ROSAENG-61801-passthrough-types-rework
Closed

ROSAENG-61801: add v2alpha1 public API module with passthrough types#241
cdoan1 wants to merge 2 commits into
openshift-online:mainfrom
cdoan1:ROSAENG-61801-passthrough-types-rework

Conversation

@cdoan1

@cdoan1 cdoan1 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

New standalone module at api/public/v2alpha1/ with generated passthrough types (HostedClusterSpecPassthrough, NodePoolSpecPassthrough), envelope types (Cluster, NodePool), configuration mirror types, and per-field markers for write-mode, visibility, and feature gates.

Adds platform-api codegen packages: field metadata registry (120 fields), feature gate registry (6 gates), and conversion helpers. Adds Makefile codegen pipeline (make codegen) and updates verify/deps targets.

v1alpha1 internal CRD types are unchanged.

Description

add make targets to create cluster and nodepool passthrough types in api/public/v2alpha1

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • CI/CD or tooling change

Testing

  • Unit tests pass (make test)
  • Integration tests pass (if applicable)
  • Manual verification completed

Checklist

  • My code follows the project's coding conventions
  • I have updated documentation as needed
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass

Summary by CodeRabbit

  • New Features

    • Added public v2alpha1 APIs for managing clusters, node pools, configurations, and machine settings.
    • Added lifecycle states, status reporting, placement references, metadata, and validation for cluster and node pool resources.
    • Added HyperShift hosted-cluster and node-pool configuration passthrough.
    • Added detailed kubelet, machine, networking, authentication, and component configuration options.
    • Added feature-gate definitions and field-level configuration metadata.
    • Added automatic population and normalization of platform-managed cluster values.
  • Developer Experience

    • Added documented commands for generating, validating, and verifying API code.
    • Added automatic handling for generated intermediate files.

@openshift-ci-robot

openshift-ci-robot commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

@cdoan1: This pull request references ROSAENG-61801 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

New standalone module at api/public/v2alpha1/ with generated passthrough types (HostedClusterSpecPassthrough, NodePoolSpecPassthrough), envelope types (Cluster, NodePool), configuration mirror types, and per-field markers for write-mode, visibility, and feature gates.

Adds platform-api codegen packages: field metadata registry (120 fields), feature gate registry (6 gates), and conversion helpers. Adds Makefile codegen pipeline (make codegen) and updates verify/deps targets.

v1alpha1 internal CRD types are unchanged.

Description

add make targets to create cluster and nodepool passthrough types in api/public/v2alpha1

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • CI/CD or tooling change

Testing

  • Unit tests pass (make test)
  • Integration tests pass (if applicable)
  • Manual verification completed

Checklist

  • My code follows the project's coding conventions
  • I have updated documentation as needed
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 31, 2026
@openshift-ci

openshift-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoan1

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

The pull request process is described 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 31, 2026
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The PR adds a public v2alpha1 API module with Cluster, NodePool, configuration, and passthrough types. It adds code-generation targets, feature-gate and conversion helpers, and generated field metadata.

Changes

Public v2alpha1 API and management

Layer / File(s) Summary
Define public v2alpha1 API contracts
api/public/v2alpha1/*
Adds Kubernetes API metadata, Cluster and NodePool resources, configuration types, passthrough specifications, lifecycle phases, statuses, and scheme registration.
Wire public API code generation
Makefile, .gitignore, hack/api-codegen/..., platform-api/go.mod
Adds public API deepcopy, passthrough, registry, aggregate, and verification targets. Updates module wiring and ignores passthrough intermediates.
Add feature gates and field metadata
platform-api/internal/codegen/featuregate/*, platform-api/internal/codegen/registry/*
Adds feature-stage evaluation and generated metadata for write modes, hidden fields, and feature gates.
Add cluster service-set conversion
platform-api/internal/codegen/conversion/cluster.go
Adds service-set field injection and cloud URL rewriting using a cluster identifier.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Makefile
  participant APICodegen as hack/api-codegen
  participant PublicAPI as api/public/v2alpha1
  participant RegistryGenerator as platform-api registry generator
  Makefile->>APICodegen: Generate passthrough types
  APICodegen->>PublicAPI: Write passthrough API types
  Makefile->>PublicAPI: Generate deepcopy code
  Makefile->>RegistryGenerator: Generate field metadata
  RegistryGenerator->>PublicAPI: Read API markers and fields
  Makefile->>PublicAPI: Verify API compilation
  Makefile->>RegistryGenerator: Verify registry compilation
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Ai-Attribution ⚠️ Warning The PR commit identifies AI use with Co-Authored-By: Claude Opus 4.6; it has no Assisted-by or Generated-by Red Hat attribution trailer. Replace the AI Co-Authored-By trailer with the required Red Hat Assisted-by or Generated-by trailer.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the new v2alpha1 public API module and passthrough types, which are the main changes in the pull request.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto ✅ Passed The commit patch adds no MD5, SHA1, DES, RC4, Blowfish, ECB, crypto imports, or crypto calls; new secret fields are declarations/deep copies only, with no secret comparisons.
Container-Privileges ✅ Passed The pull request adds no container or Kubernetes manifest files and no flagged settings; its only “capabilities” matches are API fields and metadata.
No-Sensitive-Data-In-Logs ✅ Passed The PR diff adds no logging or print calls; added code does not format arbitrary values, and the 120 metadata entries contain no credential-like values.
No-Hardcoded-Secrets ✅ Passed Patch scans found no hardcoded keys, tokens, passwords, private keys, credential URLs, or secret-like assignments; long base64 strings occur only as Go module checksums.
No-Injection-Vectors ✅ Passed Changed-file scans found no unsafe eval/exec, pickle, YAML load, os.system, shell=True, innerHTML, or SQL string construction; metadata contains no code-like payloads.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (2)
platform-api/internal/codegen/conversion/cluster.go (1)

26-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Join the URL parts instead of concatenating strings.

If baseURL ends with /, the result contains //. Trim the separator, or use net/url to build the path. Consider also skipping the write when clusterID is empty, because the current code produces a trailing slash URL.

♻️ Proposed fix
+import "strings"
+
 // RewriteCloudURLWithID sets cloudUrl to baseURL/clusterID in a response spec.
 func RewriteCloudURLWithID(spec map[string]interface{}, baseURL, clusterID string) {
-	spec["cloudUrl"] = baseURL + "/" + clusterID
+	spec["cloudUrl"] = strings.TrimSuffix(baseURL, "/") + "/" + clusterID
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@platform-api/internal/codegen/conversion/cluster.go` around lines 26 - 28,
Update RewriteCloudURLWithID to join baseURL and clusterID without duplicate
separators, trimming the existing trailing separator before appending the ID.
Skip updating spec["cloudUrl"] when clusterID is empty so no trailing-slash URL
is produced.
platform-api/internal/codegen/registry/field_metadata.go (1)

5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Avoid coupling platform-api to the hack/api-codegen module.

platform-api/go.mod directly requires and replaces hack/api-codegen, and the generated registry imports hack/api-codegen/pkg/markers. Move the shared metadata types into a library package that both modules import. Keep generator-specific packages under hack/.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@platform-api/internal/codegen/registry/field_metadata.go` around lines 5 -
13, Decouple the generated registry from hack/api-codegen by moving WriteMode,
FieldMeta, and FeatureGateWriteMode into a shared library package outside hack/.
Update the generator and platform-api registry to import that shared package,
remove the re-export aliases and direct module require/replace, and keep
generator-specific implementation packages under hack/.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/public/v2alpha1/go.mod`:
- Around line 8-9: Align the Kubernetes dependencies in go.mod by updating the
k8s.io/api requirement from v0.35.1 to v0.36.0, matching k8s.io/apimachinery and
keeping both modules on the same minor version.

In `@platform-api/internal/codegen/conversion/cluster.go`:
- Around line 12-22: Update the service-set field handling in the conversion
logic around ssf.CloudURL, ssf.Placement, and ssf.CreatorARN: delete each
corresponding client-supplied key from spec before conditionally assigning the
injected value, so empty injected values cannot preserve client data. Replace
the placement interface comparison with type-safe empty-value handling so
decoded non-string values do not bypass injection.
- Line 11: Update InjectClusterServiceSet and the other function in this
conversion flow that assigns into spec to guard against a nil map before any
writes. Return early for nil spec, or otherwise explicitly enforce
initialized-map inputs consistently in both function contracts.

In `@platform-api/internal/codegen/registry/field_metadata.go`:
- Around line 541-545: Remove the incorrect feature-gate marker from the source
field defining spec.tags, then regenerate the artifacts. In
platform-api/internal/codegen/registry/field_metadata.go lines 541-545, remove
FeatureGate: "HyperFleetAutoScaling"; regenerate
platform-api/internal/codegen/registry/field_metadata.json lines 520-524 so
spec.tags has no featureGate or the correct gate.
- Line 22: Update FieldRegistry to attach the existing unreferenced feature
gates to the exact dns, etcd, and secretEncryption field metadata entries.
Preserve the mutable setting for both distinct maxPods and kubelet.maxPods keys,
and keep the current lookup behavior unchanged.

---

Nitpick comments:
In `@platform-api/internal/codegen/conversion/cluster.go`:
- Around line 26-28: Update RewriteCloudURLWithID to join baseURL and clusterID
without duplicate separators, trimming the existing trailing separator before
appending the ID. Skip updating spec["cloudUrl"] when clusterID is empty so no
trailing-slash URL is produced.

In `@platform-api/internal/codegen/registry/field_metadata.go`:
- Around line 5-13: Decouple the generated registry from hack/api-codegen by
moving WriteMode, FieldMeta, and FeatureGateWriteMode into a shared library
package outside hack/. Update the generator and platform-api registry to import
that shared package, remove the re-export aliases and direct module
require/replace, and keep generator-specific implementation packages under
hack/.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift-online/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0d9e61a6-0530-46dd-9662-dca01e488bc6

📥 Commits

Reviewing files that changed from the base of the PR and between c04e8e1 and 765ef34.

⛔ Files ignored due to path filters (4)
  • api/public/v2alpha1/go.sum is excluded by !**/*.sum
  • api/public/v2alpha1/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • hack/api-codegen/go.sum is excluded by !**/*.sum
  • platform-api/go.sum is excluded by !**/*.sum
📒 Files selected for processing (16)
  • .gitignore
  • Makefile
  • api/public/v2alpha1/cluster_types.go
  • api/public/v2alpha1/configuration.go
  • api/public/v2alpha1/go.mod
  • api/public/v2alpha1/groupversion_info.go
  • api/public/v2alpha1/hostedclusterspec.passthrough.go
  • api/public/v2alpha1/nodepool_types.go
  • hack/api-codegen/go.mod
  • hack/api-codegen/pkg/markers/scanner.go
  • platform-api/go.mod
  • platform-api/internal/codegen/conversion/cluster.go
  • platform-api/internal/codegen/featuregate/registry.go
  • platform-api/internal/codegen/featuregate/types.go
  • platform-api/internal/codegen/registry/field_metadata.go
  • platform-api/internal/codegen/registry/field_metadata.json

Comment thread api/public/v2alpha1/go.mod Outdated
Comment thread platform-api/internal/codegen/conversion/cluster.go
Comment thread platform-api/internal/codegen/conversion/cluster.go Outdated
Comment thread platform-api/internal/codegen/registry/field_metadata.go
Comment thread platform-api/internal/codegen/registry/field_metadata.go
@cdoan1
cdoan1 force-pushed the ROSAENG-61801-passthrough-types-rework branch from 765ef34 to ee37bfc Compare July 31, 2026 22:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Makefile`:
- Around line 348-349: Remove the unresolved Git conflict marker immediately
following the codegen target in the Makefile, leaving the existing codegen
dependency declaration valid so make can parse and execute it.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift-online/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 98cd5cae-e52d-48f2-b103-67ec94147ccf

📥 Commits

Reviewing files that changed from the base of the PR and between 765ef34 and ee37bfc.

⛔ Files ignored due to path filters (4)
  • api/public/v2alpha1/go.sum is excluded by !**/*.sum
  • api/public/v2alpha1/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • hack/api-codegen/go.sum is excluded by !**/*.sum
  • platform-api/go.sum is excluded by !**/*.sum
📒 Files selected for processing (16)
  • .gitignore
  • Makefile
  • api/public/v2alpha1/cluster_types.go
  • api/public/v2alpha1/configuration.go
  • api/public/v2alpha1/go.mod
  • api/public/v2alpha1/groupversion_info.go
  • api/public/v2alpha1/hostedclusterspec.passthrough.go
  • api/public/v2alpha1/nodepool_types.go
  • hack/api-codegen/go.mod
  • hack/api-codegen/pkg/markers/scanner.go
  • platform-api/go.mod
  • platform-api/internal/codegen/conversion/cluster.go
  • platform-api/internal/codegen/featuregate/registry.go
  • platform-api/internal/codegen/featuregate/types.go
  • platform-api/internal/codegen/registry/field_metadata.go
  • platform-api/internal/codegen/registry/field_metadata.json
🚧 Files skipped from review as they are similar to previous changes (13)
  • .gitignore
  • hack/api-codegen/pkg/markers/scanner.go
  • api/public/v2alpha1/go.mod
  • platform-api/go.mod
  • platform-api/internal/codegen/registry/field_metadata.go
  • hack/api-codegen/go.mod
  • platform-api/internal/codegen/registry/field_metadata.json
  • api/public/v2alpha1/hostedclusterspec.passthrough.go
  • platform-api/internal/codegen/conversion/cluster.go
  • platform-api/internal/codegen/featuregate/registry.go
  • api/public/v2alpha1/cluster_types.go
  • api/public/v2alpha1/nodepool_types.go
  • api/public/v2alpha1/configuration.go

Comment thread Makefile Outdated
@cdoan1
cdoan1 force-pushed the ROSAENG-61801-passthrough-types-rework branch 2 times, most recently from e1fb57e to 6a247ff Compare July 31, 2026 22:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Makefile`:
- Line 348: Update the codegen target prerequisites so the dependency graph
enforces the documented sequence: codegen-passthrough must complete before
generate-public-deepcopy, then codegen-registry, then codegen-verify. Preserve
the codegen target while expressing these ordering dependencies through the
prerequisite relationships rather than relying on their listed order.

In `@platform-api/internal/codegen/conversion/cluster.go`:
- Line 15: Enforce a non-nil spec map contract for InjectClusterServiceSet and
RewriteCloudURLWithID across every caller. Verify all call sites and initialize
the maps before invoking these helpers, or update both helpers to return and
propagate an error or updated map; do not silently skip writes when spec is nil.

In `@platform-api/internal/codegen/featuregate/registry.go`:
- Around line 41-49: Update GatesForFeatureSet to sort the collected gates slice
before returning it, while preserving the existing stage filtering logic and
return behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift-online/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ddcbc41f-6f0d-4b89-8a35-2850a5751a18

📥 Commits

Reviewing files that changed from the base of the PR and between ee37bfc and e1fb57e.

⛔ Files ignored due to path filters (4)
  • api/public/v2alpha1/go.sum is excluded by !**/*.sum
  • api/public/v2alpha1/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • hack/api-codegen/go.sum is excluded by !**/*.sum
  • platform-api/go.sum is excluded by !**/*.sum
📒 Files selected for processing (16)
  • .gitignore
  • Makefile
  • api/public/v2alpha1/cluster_types.go
  • api/public/v2alpha1/configuration.go
  • api/public/v2alpha1/go.mod
  • api/public/v2alpha1/groupversion_info.go
  • api/public/v2alpha1/hostedclusterspec.passthrough.go
  • api/public/v2alpha1/nodepool_types.go
  • hack/api-codegen/go.mod
  • hack/api-codegen/pkg/markers/scanner.go
  • platform-api/go.mod
  • platform-api/internal/codegen/conversion/cluster.go
  • platform-api/internal/codegen/featuregate/registry.go
  • platform-api/internal/codegen/featuregate/types.go
  • platform-api/internal/codegen/registry/field_metadata.go
  • platform-api/internal/codegen/registry/field_metadata.json
🚧 Files skipped from review as they are similar to previous changes (13)
  • .gitignore
  • api/public/v2alpha1/go.mod
  • platform-api/internal/codegen/registry/field_metadata.json
  • api/public/v2alpha1/hostedclusterspec.passthrough.go
  • hack/api-codegen/go.mod
  • platform-api/internal/codegen/featuregate/types.go
  • api/public/v2alpha1/configuration.go
  • hack/api-codegen/pkg/markers/scanner.go
  • api/public/v2alpha1/nodepool_types.go
  • platform-api/internal/codegen/registry/field_metadata.go
  • api/public/v2alpha1/cluster_types.go
  • platform-api/go.mod
  • api/public/v2alpha1/groupversion_info.go

Comment thread Makefile Outdated
Comment thread platform-api/internal/codegen/conversion/cluster.go
Comment thread platform-api/internal/codegen/featuregate/registry.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
Makefile (1)

348-348: ⚠️ Potential issue | 🟠 Major

Serialize the codegen prerequisite graph.

make -j codegen can run codegen-registry or codegen-verify before earlier generation steps finish. Chain the targets as codegen-passthrough → generate-public-deepcopy → codegen-registry → codegen-verify, then make codegen depend on codegen-verify.

This is the same unresolved issue from the previous review.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` at line 348, Update the Makefile targets so the prerequisites form
the ordered chain codegen-passthrough → generate-public-deepcopy →
codegen-registry → codegen-verify, then make codegen depend only on
codegen-verify to preserve this sequence under parallel make.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@Makefile`:
- Line 348: Update the Makefile targets so the prerequisites form the ordered
chain codegen-passthrough → generate-public-deepcopy → codegen-registry →
codegen-verify, then make codegen depend only on codegen-verify to preserve this
sequence under parallel make.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift-online/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5b1ef97b-e580-4a3c-b845-7c08817c852f

📥 Commits

Reviewing files that changed from the base of the PR and between e1fb57e and 6a247ff.

⛔ Files ignored due to path filters (4)
  • api/public/v2alpha1/go.sum is excluded by !**/*.sum
  • api/public/v2alpha1/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • hack/api-codegen/go.sum is excluded by !**/*.sum
  • platform-api/go.sum is excluded by !**/*.sum
📒 Files selected for processing (16)
  • .gitignore
  • Makefile
  • api/public/v2alpha1/cluster_types.go
  • api/public/v2alpha1/configuration.go
  • api/public/v2alpha1/go.mod
  • api/public/v2alpha1/groupversion_info.go
  • api/public/v2alpha1/hostedclusterspec.passthrough.go
  • api/public/v2alpha1/nodepool_types.go
  • hack/api-codegen/go.mod
  • hack/api-codegen/pkg/markers/scanner.go
  • platform-api/go.mod
  • platform-api/internal/codegen/conversion/cluster.go
  • platform-api/internal/codegen/featuregate/registry.go
  • platform-api/internal/codegen/featuregate/types.go
  • platform-api/internal/codegen/registry/field_metadata.go
  • platform-api/internal/codegen/registry/field_metadata.json
🚧 Files skipped from review as they are similar to previous changes (14)
  • .gitignore
  • hack/api-codegen/go.mod
  • api/public/v2alpha1/go.mod
  • platform-api/go.mod
  • hack/api-codegen/pkg/markers/scanner.go
  • api/public/v2alpha1/groupversion_info.go
  • api/public/v2alpha1/hostedclusterspec.passthrough.go
  • platform-api/internal/codegen/featuregate/registry.go
  • platform-api/internal/codegen/featuregate/types.go
  • api/public/v2alpha1/nodepool_types.go
  • platform-api/internal/codegen/registry/field_metadata.json
  • api/public/v2alpha1/configuration.go
  • api/public/v2alpha1/cluster_types.go
  • platform-api/internal/codegen/conversion/cluster.go

@cdoan1
cdoan1 force-pushed the ROSAENG-61801-passthrough-types-rework branch from 6a247ff to 3958b2a Compare July 31, 2026 22:30
@cdoan1

cdoan1 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

/retest

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
Makefile (1)

328-348: ⚠️ Potential issue | 🟠 Major

Restore the code-generation dependency chain.

Line [348] still declares the generation stages as independent prerequisites. With make -j codegen, codegen-verify can run before generated files are complete. This repeats the previous review finding; the current target definitions still do not encode the required order.

Chain codegen-passthroughgenerate-public-deepcopycodegen-registrycodegen-verify, then make codegen depend on codegen-verify.

Proposed fix
-generate-public-deepcopy: $(CONTROLLER_GEN)
+generate-public-deepcopy: codegen-passthrough $(CONTROLLER_GEN)

-codegen-registry: build-api-codegen
+codegen-registry: generate-public-deepcopy

-codegen-verify:
+codegen-verify: codegen-registry

-codegen: codegen-passthrough generate-public-deepcopy codegen-registry codegen-verify
+codegen: codegen-verify
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 328 - 348, Update the Makefile target dependencies to
enforce the generation order: make generate-public-deepcopy depend on
codegen-passthrough, codegen-registry depend on generate-public-deepcopy, and
codegen-verify depend on codegen-registry. Change codegen to depend only on
codegen-verify so parallel builds cannot verify incomplete generated files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@Makefile`:
- Around line 328-348: Update the Makefile target dependencies to enforce the
generation order: make generate-public-deepcopy depend on codegen-passthrough,
codegen-registry depend on generate-public-deepcopy, and codegen-verify depend
on codegen-registry. Change codegen to depend only on codegen-verify so parallel
builds cannot verify incomplete generated files.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift-online/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7efb556d-7e0a-48af-ae37-c2e62a423142

📥 Commits

Reviewing files that changed from the base of the PR and between 6a247ff and 3958b2a.

⛔ Files ignored due to path filters (4)
  • api/public/v2alpha1/go.sum is excluded by !**/*.sum
  • api/public/v2alpha1/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • hack/api-codegen/go.sum is excluded by !**/*.sum
  • platform-api/go.sum is excluded by !**/*.sum
📒 Files selected for processing (16)
  • .gitignore
  • Makefile
  • api/public/v2alpha1/cluster_types.go
  • api/public/v2alpha1/configuration.go
  • api/public/v2alpha1/go.mod
  • api/public/v2alpha1/groupversion_info.go
  • api/public/v2alpha1/hostedclusterspec.passthrough.go
  • api/public/v2alpha1/nodepool_types.go
  • hack/api-codegen/go.mod
  • hack/api-codegen/pkg/markers/scanner.go
  • platform-api/go.mod
  • platform-api/internal/codegen/conversion/cluster.go
  • platform-api/internal/codegen/featuregate/registry.go
  • platform-api/internal/codegen/featuregate/types.go
  • platform-api/internal/codegen/registry/field_metadata.go
  • platform-api/internal/codegen/registry/field_metadata.json
🚧 Files skipped from review as they are similar to previous changes (15)
  • api/public/v2alpha1/hostedclusterspec.passthrough.go
  • .gitignore
  • hack/api-codegen/pkg/markers/scanner.go
  • api/public/v2alpha1/groupversion_info.go
  • platform-api/go.mod
  • api/public/v2alpha1/go.mod
  • platform-api/internal/codegen/conversion/cluster.go
  • platform-api/internal/codegen/featuregate/registry.go
  • hack/api-codegen/go.mod
  • platform-api/internal/codegen/registry/field_metadata.go
  • api/public/v2alpha1/configuration.go
  • platform-api/internal/codegen/featuregate/types.go
  • api/public/v2alpha1/cluster_types.go
  • platform-api/internal/codegen/registry/field_metadata.json
  • api/public/v2alpha1/nodepool_types.go

@cdoan1
cdoan1 force-pushed the ROSAENG-61801-passthrough-types-rework branch 3 times, most recently from 108b78d to 451484e Compare August 4, 2026 13:50
@cdoan1

cdoan1 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

/test on-demand-e2e

cdoan1 and others added 2 commits August 4, 2026 14:37
…and codegen pipeline

New standalone module at api/public/v2alpha1/ with generated passthrough
types (HostedClusterSpecPassthrough, NodePoolSpecPassthrough), envelope
types (Cluster, NodePool), configuration mirror types, and per-field
markers for write-mode, visibility, and feature gates.

Adds platform-api codegen packages: field metadata registry (120 fields),
feature gate registry (6 gates), and conversion helpers. Adds Makefile
codegen pipeline (make codegen) and updates verify/deps targets.

v1alpha1 internal CRD types are unchanged.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The marker-scanner produced flat registry keys (e.g. "pausedUntil") with
no root-type namespace, so fields with the same JSON name in different
passthrough types silently overwrote each other with non-deterministic
results. Prefix passthrough fields with their root type context
(spec.hostedCluster.* / spec.nodePool.*) to match the paths that
downstream consumers already construct.

Add a verify-codegen Makefile target that re-runs the full codegen
pipeline and fails on git diff, same pattern as verify-clientset, so CI
catches stale generated code.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@typeid

typeid commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the work here. The marker infrastructure is really solid. The scanner, field registry, feature gate system, and the Makefile codegen pipeline are all things we need and the implementation quality is good. I want to step back and talk about the overall structure though, because I think we need to take a different approach to get where we're going.

What we're trying to solve:

We have one Cluster object (same for NodePool) with fields that fall into different buckets:

  • Fields customers can see and edit (displayName, deleteProtection)
  • Fields customers can see but not change after creation (fips, systemReserved)
  • Fields customers can't see at all, platform-managed (accountId, creatorARN, internalId)
  • Fields behind feature gates that only some customers can access

We need the platform-api to enforce all of this at runtime, and we need to generate a customer-facing OpenAPI spec (and eventually a typed Go SDK) that only exposes the visible fields. Internally, the operator and admin tooling need to see and set everything.

The problem with a separate api/public/v2alpha1/ module:

Right now nothing actually imports these types. platform-api uses hyperfleet-operator/api/v1alpha1 and so does the operator. The envelope fields this PR adds (DisplayName, DeleteProtection, Properties, Tags) only exist on the v2alpha1 types, not on the types that actually get written to the database. So we end up with two disconnected type hierarchies:

  • v1alpha1 has the fields the system uses, but lacks the envelope fields and has no markers
  • v2alpha1 has the markers and envelope fields, but nothing reads or writes it

There's no codegen or compile-time link between them, so if they drift there's nothing to catch it. The passthrough types (HostedClusterSpecPassthrough) exist in v2alpha1 but ClusterSpec.HostedCluster still embeds the raw upstream hypershiftv1beta1.HostedClusterSpec, so the markers on the passthrough type aren't actually applied to the field the system uses.

What we should do instead:

The markers and envelope fields need to live on the actual types that get stored in the DB. One set of Go types is the source of truth, and the customer-facing view is derived from it, not maintained separately.

Concretely:

  1. Add the envelope fields and markers to v1alpha1. DisplayName, DeleteProtection, Properties, Tags, ExpirationTimestamp go on ClusterSpec alongside the existing fields. Every field gets a +hyperfleet:write-mode and +k8s:openapi-gen marker.

  2. The passthrough types replace the upstream embed. Instead of HostedCluster hypershiftv1beta1.HostedClusterSpec, it becomes HostedCluster HostedClusterPassthrough, a generated mirror where we own every field and can put markers on it. That's the whole reason passthrough types exist: we can't add markers to types from another module.

  3. The customer-facing types are generated, not hand-written. An OpenAPI generator reads the v1alpha1 types, skips +k8s:openapi-gen=false fields, and produces a public OpenAPI spec. From that spec, oapi-codegen generates the REST types that the customer SDK uses. Customers structurally can't reference hidden fields because those fields were never in the OpenAPI spec.

  4. Drop api/public/v2alpha1/. No separate public module, no second version to maintain, no version confusion (v2alpha1 vs v1alpha1).

  5. Move hyperfleet-operator/api/v1alpha1/ up to api/v1alpha1/. It's imported by 4 modules (platform-api, operator, clientset, test). It's the shared data model for the whole system, not operator-specific. This is optional but worth doing now while nothing external depends on the path. The GVK stored in the database (hyperfleet.io/v1alpha1/Cluster) is based on the group name, not the module path, so existing data is unaffected.

Since we use Postgres via hyperfleet-db rather than a K8s API server, we don't need CRD YAML generation. The CRD-specific markers (+kubebuilder:subresource:status, +kubebuilder:printcolumn, +kubebuilder:resource:scope) aren't useful to us. We do still need +kubebuilder:object:root=true for deepcopy generation and +kubebuilder:validation:* markers for OpenAPI schema constraints (MaxLength, Enum, Required, etc.).

Where the public types end up:

The generated public artifacts live together under api/public/:

api/
├── v1alpha1/                        source of truth (all fields, all markers)
    ├── cluster.go
    ├── nodepool.go
    ├── hostedcluster_passthrough.go   generated mirror of HyperShift
    └── public/   generated (customer-visible only)
        ├── openapi.json                   OpenAPI spec (visible fields only)
        └── types.gen.go                   Go types generated from the spec

The pipeline:

api/v1alpha1/ (source of truth, all fields, all markers)
    |
    v  openapi-gen (skips +openapi-gen=false fields)
    |
api/public/openapi.json (customer-visible schema)
    |
    v  oapi-codegen
    |
api/public/types.gen.go (generated public Go types)

The clientset imports api/public/ for its typed structs. Other language SDKs generate from api/public/openapi.json. No hand-maintained public types module, one source of truth, one set of generated outputs.

On the codegen tooling: The marker scanner, field registry, feature gate system, and the Makefile pipeline are the foundation for all of this. The main change is pointing them at v1alpha1 instead of a separate module. The hard part of building the codegen infrastructure is already done here.

Happy to walk through this together or pair on the restructure.

@cdoan1 cdoan1 closed this Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants