ROSAENG-61801: add v2alpha1 public API module with passthrough types - #241
Open
cdoan1 wants to merge 1 commit into
Open
ROSAENG-61801: add v2alpha1 public API module with passthrough types#241cdoan1 wants to merge 1 commit into
cdoan1 wants to merge 1 commit into
Conversation
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. DetailsIn response to this:
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
added
the
jira/valid-reference
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds a public ChangesPublic v2alpha1 API and management
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
Possibly related PRs
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
platform-api/internal/codegen/conversion/cluster.go (1)
26-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winJoin the URL parts instead of concatenating strings.
If
baseURLends with/, the result contains//. Trim the separator, or usenet/urlto build the path. Consider also skipping the write whenclusterIDis 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 tradeoffAvoid coupling
platform-apito thehack/api-codegenmodule.
platform-api/go.moddirectly requires and replaceshack/api-codegen, and the generated registry importshack/api-codegen/pkg/markers. Move the shared metadata types into a library package that both modules import. Keep generator-specific packages underhack/.🤖 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
⛔ Files ignored due to path filters (4)
api/public/v2alpha1/go.sumis excluded by!**/*.sumapi/public/v2alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*hack/api-codegen/go.sumis excluded by!**/*.sumplatform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.gitignoreMakefileapi/public/v2alpha1/cluster_types.goapi/public/v2alpha1/configuration.goapi/public/v2alpha1/go.modapi/public/v2alpha1/groupversion_info.goapi/public/v2alpha1/hostedclusterspec.passthrough.goapi/public/v2alpha1/nodepool_types.gohack/api-codegen/go.modhack/api-codegen/pkg/markers/scanner.goplatform-api/go.modplatform-api/internal/codegen/conversion/cluster.goplatform-api/internal/codegen/featuregate/registry.goplatform-api/internal/codegen/featuregate/types.goplatform-api/internal/codegen/registry/field_metadata.goplatform-api/internal/codegen/registry/field_metadata.json
cdoan1
force-pushed
the
ROSAENG-61801-passthrough-types-rework
branch
from
July 31, 2026 22:12
765ef34 to
ee37bfc
Compare
Contributor
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (4)
api/public/v2alpha1/go.sumis excluded by!**/*.sumapi/public/v2alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*hack/api-codegen/go.sumis excluded by!**/*.sumplatform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.gitignoreMakefileapi/public/v2alpha1/cluster_types.goapi/public/v2alpha1/configuration.goapi/public/v2alpha1/go.modapi/public/v2alpha1/groupversion_info.goapi/public/v2alpha1/hostedclusterspec.passthrough.goapi/public/v2alpha1/nodepool_types.gohack/api-codegen/go.modhack/api-codegen/pkg/markers/scanner.goplatform-api/go.modplatform-api/internal/codegen/conversion/cluster.goplatform-api/internal/codegen/featuregate/registry.goplatform-api/internal/codegen/featuregate/types.goplatform-api/internal/codegen/registry/field_metadata.goplatform-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
cdoan1
force-pushed
the
ROSAENG-61801-passthrough-types-rework
branch
2 times, most recently
from
July 31, 2026 22:19
e1fb57e to
6a247ff
Compare
Contributor
There was a problem hiding this comment.
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
⛔ Files ignored due to path filters (4)
api/public/v2alpha1/go.sumis excluded by!**/*.sumapi/public/v2alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*hack/api-codegen/go.sumis excluded by!**/*.sumplatform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.gitignoreMakefileapi/public/v2alpha1/cluster_types.goapi/public/v2alpha1/configuration.goapi/public/v2alpha1/go.modapi/public/v2alpha1/groupversion_info.goapi/public/v2alpha1/hostedclusterspec.passthrough.goapi/public/v2alpha1/nodepool_types.gohack/api-codegen/go.modhack/api-codegen/pkg/markers/scanner.goplatform-api/go.modplatform-api/internal/codegen/conversion/cluster.goplatform-api/internal/codegen/featuregate/registry.goplatform-api/internal/codegen/featuregate/types.goplatform-api/internal/codegen/registry/field_metadata.goplatform-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
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Makefile (1)
348-348:⚠️ Potential issue | 🟠 MajorSerialize the
codegenprerequisite graph.
make -j codegencan runcodegen-registryorcodegen-verifybefore earlier generation steps finish. Chain the targets ascodegen-passthrough → generate-public-deepcopy → codegen-registry → codegen-verify, then makecodegendepend oncodegen-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
⛔ Files ignored due to path filters (4)
api/public/v2alpha1/go.sumis excluded by!**/*.sumapi/public/v2alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*hack/api-codegen/go.sumis excluded by!**/*.sumplatform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.gitignoreMakefileapi/public/v2alpha1/cluster_types.goapi/public/v2alpha1/configuration.goapi/public/v2alpha1/go.modapi/public/v2alpha1/groupversion_info.goapi/public/v2alpha1/hostedclusterspec.passthrough.goapi/public/v2alpha1/nodepool_types.gohack/api-codegen/go.modhack/api-codegen/pkg/markers/scanner.goplatform-api/go.modplatform-api/internal/codegen/conversion/cluster.goplatform-api/internal/codegen/featuregate/registry.goplatform-api/internal/codegen/featuregate/types.goplatform-api/internal/codegen/registry/field_metadata.goplatform-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
force-pushed
the
ROSAENG-61801-passthrough-types-rework
branch
from
July 31, 2026 22:30
6a247ff to
3958b2a
Compare
Collaborator
Author
|
/retest |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Makefile (1)
328-348:⚠️ Potential issue | 🟠 MajorRestore the code-generation dependency chain.
Line [348] still declares the generation stages as independent prerequisites. With
make -j codegen,codegen-verifycan 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-passthrough→generate-public-deepcopy→codegen-registry→codegen-verify, then makecodegendepend oncodegen-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
⛔ Files ignored due to path filters (4)
api/public/v2alpha1/go.sumis excluded by!**/*.sumapi/public/v2alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*hack/api-codegen/go.sumis excluded by!**/*.sumplatform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.gitignoreMakefileapi/public/v2alpha1/cluster_types.goapi/public/v2alpha1/configuration.goapi/public/v2alpha1/go.modapi/public/v2alpha1/groupversion_info.goapi/public/v2alpha1/hostedclusterspec.passthrough.goapi/public/v2alpha1/nodepool_types.gohack/api-codegen/go.modhack/api-codegen/pkg/markers/scanner.goplatform-api/go.modplatform-api/internal/codegen/conversion/cluster.goplatform-api/internal/codegen/featuregate/registry.goplatform-api/internal/codegen/featuregate/types.goplatform-api/internal/codegen/registry/field_metadata.goplatform-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
force-pushed
the
ROSAENG-61801-passthrough-types-rework
branch
from
July 31, 2026 22:57
3958b2a to
7222502
Compare
…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>
cdoan1
force-pushed
the
ROSAENG-61801-passthrough-types-rework
branch
from
July 31, 2026 23:06
7222502 to
108b78d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Testing
make test)Checklist
Summary by CodeRabbit
New Features
Developer Experience