ROSAENG-61801: feat: enable api-management v2 - #284
Merged
openshift-merge-bot[bot] merged 14 commits intoAug 6, 2026
Merged
Conversation
…el api/ Module path changes from .../hyperfleet-operator/api to .../api. Updates all go.mod replace directives, Go imports, Makefile targets, Tekton pipelines, Containerfiles, and documentation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move passthrough types (HostedClusterSpecPassthrough, NodePoolSpecPassthrough) and configuration types into api/v1alpha1/ as the single source of truth. Replace raw HyperShift embeds with passthrough types in ClusterSpec and NodePoolSpec, add envelope fields with per-field markers, and wire JSON roundtrip conversion in the operator render code. - Add envelope fields to ClusterSpec (DisplayName, DeleteProtection, Properties, Tags, AccountID, InternalID) and NodePoolSpec (DisplayName, AutoRepair, Labels, AccountID, InternalPoolID) with write-mode markers - Change HostedCluster/NodePool field types to passthrough types - Add JSON roundtrip conversion in render package for passthrough→HyperShift - Add CRD validation bounds (MaxItems, MaxProperties) to satisfy CEL cost - Add codegen pipeline files (registry, featuregate, conversion) to platform-api - Update marker scanner to recognize *Passthrough root types - Add hack/api-codegen replace directive to platform-api/go.mod - Create api/v1alpha1/public/ placeholder for future generated types - Regenerate deepcopy, CRDs; all tests pass Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
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
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 change moves API types into a standalone ChangesAPI contracts and code generation
Operator and platform integration
Estimated code review effort: 4 (Complex) | ~60 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The platform-api go.mod now has a replace directive for hack/api-codegen, so the Containerfile and Tekton prefetch-input paths need to include it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hyperfleet-operator/config/crd/bases/hyperfleet.io_nodepools.yaml (1)
1883-1889: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftRestore the NodePool scaling conflict validation.
The schema now accepts both
spec.nodePool.replicasandspec.nodePool.autoScaling. These fields define competing desired-size controls. Reject this combination at admission time.Add the equivalent
XValidationmarker to the passthrough source or generator so regeneration preserves the CEL rule.🤖 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 `@hyperfleet-operator/config/crd/bases/hyperfleet.io_nodepools.yaml` around lines 1883 - 1889, The NodePool schema must reject configurations that set both spec.nodePool.replicas and spec.nodePool.autoScaling. Add the equivalent XValidation marker in the passthrough source or generator for the NodePool spec, using a CEL rule that enforces these fields are not simultaneously present, so regenerated CRDs retain the admission validation.
🧹 Nitpick comments (5)
platform-api/internal/codegen/featuregate/registry.go (1)
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrevent mutation of the gate registry.
HyperFleetFeatureGatesis an exported package-level map. Any importing package can add, change, or delete an entry. The registry decides which API fields are exposed and writable, so a mutation changes gating for the whole process. Concurrent reads and a write also race, because Go maps are not safe for concurrent use.Make the map unexported and expose read-only accessors.
♻️ Proposed refactor
-// HyperFleetFeatureGates is the registry of all feature gates. -var HyperFleetFeatureGates = map[string]FeatureGateInfo{ +// hyperFleetFeatureGates is the registry of all feature gates. +var hyperFleetFeatureGates = map[string]FeatureGateInfo{+// Gate returns the registered information for a gate. +func Gate(gate string) (FeatureGateInfo, bool) { + info, exists := hyperFleetFeatureGates[gate] + return info, exists +} + // IsGateEnabled returns true if the given gate is enabled for the feature set. func IsGateEnabled(gate string, featureSet FeatureSet) bool { - info, exists := HyperFleetFeatureGates[gate] + info, exists := hyperFleetFeatureGates[gate] if !exists { return false } return featureSet.Includes(info.Stage) }Update
GatesForFeatureSetto readhyperFleetFeatureGates, and update any external reference to the old exported name.🤖 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/featuregate/registry.go` around lines 5 - 6, Make the feature-gate registry map unexported by renaming HyperFleetFeatureGates to hyperFleetFeatureGates, update GatesForFeatureSet to use the private map, and replace all external references with read-only accessor usage rather than exposing mutable map state.platform-api/internal/codegen/featuregate/types.go (1)
6-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument that
Includesdepends on this declaration order.
Includescompares stages numerically. The comparison is correct only while the constants stay ordered from least to most permissive. A new stage inserted betweenGAandTechPreviewwould silently change which gates are enabled for every feature set. Record the invariant next to the constants.♻️ Proposed comment
const ( + // Stages must stay ordered from least to most permissive. + // FeatureSet.Includes relies on this numeric order. GA FeatureStage = iota TechPreview DevPreview )🤖 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/featuregate/types.go` around lines 6 - 10, Add a concise comment immediately above the `GA`, `TechPreview`, and `DevPreview` constants documenting that `Includes` compares their numeric values and therefore requires the stages to remain ordered from least to most permissive; preserve the existing declaration order.hack/api-codegen/pkg/markers/scanner.go (1)
100-110: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEmpty prefixes leave registry keys un-namespaced in a flat map.
rootTypePrefixreturns""for every root type it does not recognize, andscanDiriteratesdirCachein nondeterministic order. Two root types with the same JSON field name then produce one key, and the surviving entry depends on map order. The generatedfield_metadata.goalready shows unprefixed keys such asmaxPodsnext tokubelet.maxPods, which is the observable output of this behavior.
hack/api-codegen/pkg/markers/scanner.go#L100-L110: replace the string-prefix checks with an explicit prefix table, and return an error for aPassthroughtype that has no mapped prefix.hack/api-codegen/pkg/markers/scanner.go#L82-L92: propagate that error out ofscanDirso an unmapped root type fails generation instead of writing flat keys.🤖 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 `@hack/api-codegen/pkg/markers/scanner.go` around lines 100 - 110, The rootTypePrefix/scanDir flow silently produces unnamespaced registry keys for unmapped Passthrough root types. In hack/api-codegen/pkg/markers/scanner.go:100-110, replace prefix checks with an explicit type-to-prefix table and return an error when a Passthrough type lacks a mapping; in hack/api-codegen/pkg/markers/scanner.go:82-92, propagate that error from scanDir so generation stops before writing flat keys.README.md (1)
25-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
api/to the directory table.The module layout block now documents
api/go.modas a standalone module. The directory table near the top of the file listsplatform-api/,hyperfleet-operator/,hyperfleet-db/, andtest/, but notapi/. Add a row so a reader finds the CRD types module in both places.🤖 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 `@README.md` around lines 25 - 27, Update the README directory table to add an api/ row describing the standalone CRD types module, matching the existing table format and the api/go.mod entry in the module layout block.api/v1alpha1/cluster_types.go (1)
88-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse one required/optional marker style in this file.
HostedClusteruses+kubebuilder:validation:Required. Every other field inClusterSpecuses+optional, andCluster(Lines 153-164) uses+required. Switch to+requiredfor consistency. Both markers produce the same CRD output, so this change is cosmetic.♻️ Proposed marker change
// HostedCluster contains the upstream HyperShift fields, mirrored as // passthrough types with per-field visibility and write-mode markers. - // +kubebuilder:validation:Required + // +required HostedCluster HostedClusterSpecPassthrough `json:"hostedCluster"`🤖 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 `@api/v1alpha1/cluster_types.go` around lines 88 - 91, Change the kubebuilder marker immediately above the HostedCluster field in ClusterSpec from +kubebuilder:validation:Required to +required, matching the marker style used by the surrounding ClusterSpec and Cluster fields.
🤖 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/v1alpha1/cluster_types.go`:
- Around line 75-80: Update the CreatorARN validation pattern on the CreatorARN
field to accept the commercial, China, and GovCloud AWS partitions, matching the
repository’s established ARN pattern; leave the field’s other markers and
comment unchanged.
In `@api/v1alpha1/hostedclusterspec.passthrough.go`:
- Around line 96-103: Update the passthrough generator input that defines
Configuration to use the HyperFleet-owned ClusterConfiguration type instead of
hypershiftv1beta1.ClusterConfiguration, preserving the existing JSON tag and
markers. Do not modify the generated hostedclusterspec.passthrough.go file
directly.
In `@api/v1alpha1/manifest_types.go`:
- Around line 58-60: Restrict write access to Manifest objects exposed through
the manifest-editor-role: bind that role only to principals trusted with
equivalent management-cluster authority, or enforce an admission allow-list
covering permitted groups, kinds, namespaces, resources, and targets before
ApplyDesires forwards content and resource to kube-applier-aws. Preserve read
access and existing Manifest behavior for authorized callers.
In `@api/v1alpha1/nodepool_types.go`:
- Around line 55-65: Update the NodePool request decoding and conversion flow to
reject customer-supplied AccountID and InternalPoolID values before constructing
the NodePool resource. Use a customer-facing request type that omits these
fields or invoke the existing service-set validation on NodePoolSpec before
conversion, while preserving valid request handling.
In `@CLAUDE.md`:
- Around line 38-40: Update the CRD ownership entry in the “Key Conventions”
section of CLAUDE.md to state that CRD types live in the standalone api module
and are imported by hyperfleet-operator and platform-api. Remove the outdated
ownership wording while preserving the surrounding conventions.
In `@hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml`:
- Around line 89-96: Complete the truncated passthrough field descriptions in
hostedclusterspec.passthrough.go by preserving all lines from the corresponding
upstream doc comments, including the affected configuration, etcd,
imageContentSources, issuerURL, secretEncryption, olmCatalogPlacement, and
serviceAccountSigningKey fields. Then regenerate the CRD schema so hostedCluster
descriptions in hyperfleet.io_clusters.yaml contain the full sentences.
- Around line 3504-3510: Restore the upstream validation, immutability, format,
default, and bounds markers in api/v1alpha1/hostedclusterspec.passthrough.go for
every corresponding HostedClusterSpec field, including fips, clusterID, infraID,
issuerURL, capabilities, and the listed URL, RFC3339, hostname, collection, and
property constraints. Document any intentionally omitted constraint in the
passthrough struct, then regenerate the Cluster CRD so its schema preserves the
upstream behavior.
- Around line 8349-8358: Update the generator input for the hostedCluster
passthrough schema to define defaults for both required fields, using false for
fips and an empty object for sshKey; then regenerate the CRDs so the generated
schema reflects those defaults instead of editing this YAML directly.
In `@hyperfleet-operator/internal/render/cluster.go`:
- Around line 181-191: Update hostedCluster and the API conversion boundary
around toHostedClusterSpec so only the explicit public HostedCluster
specification is persisted and rendered; do not passthrough the complete
ClusterSpec. Strip or reject service-set fields including Platform, Etcd,
ServiceAccountSigningKey, SecretEncryption, and OperatorConfiguration before
storage and before rendering, while preserving the existing conversion error
handling.
In `@Makefile`:
- Around line 287-290: Update the generate target to run object generation
inside the api module by changing the command to execute cd api before invoking
CONTROLLER_GEN with paths="./..."; leave the existing hyperfleet-operator CRD
generation unchanged.
In `@platform-api/internal/codegen/conversion/cluster.go`:
- Around line 34-40: Update RewriteCloudURLWithID to normalize the URL separator
when combining baseURL and clusterID, preventing duplicate slashes if baseURL
already ends with one; add the strings import and preserve the existing nil-spec
behavior and cloudUrl assignment.
In `@platform-api/internal/codegen/registry/field_metadata.go`:
- Around line 1-22: Remove the unused generated registry under the registry
package, including its re-exported aliases, constants, and FieldRegistry
definition; do not add generation changes unless choosing instead to establish a
single target that consistently updates both registry copies.
- Around line 580-584: Remove the HyperFleetAutoScaling feature-gate marker from
the Tags field in api/v1alpha1/cluster_types.go, leaving the generated field
metadata unchanged. Ensure spec.tags remains mutable and included in the default
feature set.
In `@platform-api/pkg/types/cluster.go`:
- Line 6: Update ClusterCRToPlatform and the writeJSON serialization path so
Cluster responses omit the internal ClusterSpec fields accountId and internalId,
while preserving all customer-visible fields; alternatively, align the public
schema and visibility metadata to explicitly support those fields.
---
Outside diff comments:
In `@hyperfleet-operator/config/crd/bases/hyperfleet.io_nodepools.yaml`:
- Around line 1883-1889: The NodePool schema must reject configurations that set
both spec.nodePool.replicas and spec.nodePool.autoScaling. Add the equivalent
XValidation marker in the passthrough source or generator for the NodePool spec,
using a CEL rule that enforces these fields are not simultaneously present, so
regenerated CRDs retain the admission validation.
---
Nitpick comments:
In `@api/v1alpha1/cluster_types.go`:
- Around line 88-91: Change the kubebuilder marker immediately above the
HostedCluster field in ClusterSpec from +kubebuilder:validation:Required to
+required, matching the marker style used by the surrounding ClusterSpec and
Cluster fields.
In `@hack/api-codegen/pkg/markers/scanner.go`:
- Around line 100-110: The rootTypePrefix/scanDir flow silently produces
unnamespaced registry keys for unmapped Passthrough root types. In
hack/api-codegen/pkg/markers/scanner.go:100-110, replace prefix checks with an
explicit type-to-prefix table and return an error when a Passthrough type lacks
a mapping; in hack/api-codegen/pkg/markers/scanner.go:82-92, propagate that
error from scanDir so generation stops before writing flat keys.
In `@platform-api/internal/codegen/featuregate/registry.go`:
- Around line 5-6: Make the feature-gate registry map unexported by renaming
HyperFleetFeatureGates to hyperFleetFeatureGates, update GatesForFeatureSet to
use the private map, and replace all external references with read-only accessor
usage rather than exposing mutable map state.
In `@platform-api/internal/codegen/featuregate/types.go`:
- Around line 6-10: Add a concise comment immediately above the `GA`,
`TechPreview`, and `DevPreview` constants documenting that `Includes` compares
their numeric values and therefore requires the stages to remain ordered from
least to most permissive; preserve the existing declaration order.
In `@README.md`:
- Around line 25-27: Update the README directory table to add an api/ row
describing the standalone CRD types module, matching the existing table format
and the api/go.mod entry in the module layout block.
🪄 Autofix
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: 04266c7d-689e-4667-97b3-648432d4937b
⛔ Files ignored due to path filters (10)
api/go.sumis excluded by!**/*.sumapi/v1alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*clientset/generated/fake/register.gois excluded by!**/generated/**clientset/generated/scheme/register.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/cluster.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/fake/fake_cluster.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/fake/fake_nodepool.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/nodepool.gois excluded by!**/generated/**hyperfleet-operator/api/v1alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*platform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (70)
.tekton/rosa-hyperfleet-api-pull-request.yaml.tekton/rosa-hyperfleet-api-push.yaml.tekton/rosa-hyperfleet-operator-pull-request.yaml.tekton/rosa-hyperfleet-operator-push.yamlCLAUDE.mdMakefileREADME.mdapi/go.modapi/v1alpha1/cluster_types.goapi/v1alpha1/configuration.goapi/v1alpha1/groupversion_info.goapi/v1alpha1/hostedclusterspec.passthrough.goapi/v1alpha1/install/install.goapi/v1alpha1/managementcluster_types.goapi/v1alpha1/manifest_types.goapi/v1alpha1/nodepool_types.goapi/v1alpha1/placement_types.goapi/v1alpha1/public/.gitkeepclientset/docs/architecture.mdclientset/go.modclientset/wrappers/wire_wrappers_generated.goclientset/wrappers/wrappers_test.gohack/api-codegen/pkg/markers/scanner.gohack/api-codegen/pkg/markers/scanner_test.gohyperfleet-operator/Containerfilehyperfleet-operator/PROJECThyperfleet-operator/cmd/manager/main.gohyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yamlhyperfleet-operator/config/crd/bases/hyperfleet.io_nodepools.yamlhyperfleet-operator/go.modhyperfleet-operator/internal/controller/cluster_controller.gohyperfleet-operator/internal/controller/cluster_controller_test.gohyperfleet-operator/internal/controller/manifest_controller.gohyperfleet-operator/internal/controller/manifest_controller_test.gohyperfleet-operator/internal/controller/nodepool_controller.gohyperfleet-operator/internal/controller/nodepool_controller_test.gohyperfleet-operator/internal/controller/placement_controller.gohyperfleet-operator/internal/controller/placement_controller_test.gohyperfleet-operator/internal/controller/suite_test.gohyperfleet-operator/internal/dynamo/statusstream/manager.gohyperfleet-operator/internal/render/cluster.gohyperfleet-operator/internal/render/cluster_test.gohyperfleet-operator/internal/render/convert.gohyperfleet-operator/internal/render/nodepool.gohyperfleet-operator/internal/render/nodepool_test.gohyperfleet-operator/test/cluster_test.gohyperfleet-operator/test/helpers_test.gohyperfleet-operator/test/manifest_test.gohyperfleet-operator/test/suite_test.goplatform-api/Containerfileplatform-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.jsonplatform-api/pkg/clients/hyperfleetdb/client.goplatform-api/pkg/clients/hyperfleetdb/client_test.goplatform-api/pkg/clients/hyperfleetdb/convert.goplatform-api/pkg/clients/hyperfleetdb/convert_test.goplatform-api/pkg/handlers/cluster_test.goplatform-api/pkg/handlers/management_cluster.goplatform-api/pkg/handlers/zoa_test.goplatform-api/pkg/types/cluster.goplatform-api/pkg/types/nodepool.goplatform-api/pkg/zoa/jobbuilder.goplatform-api/pkg/zoa/reconciler.goplatform-api/pkg/zoa/reconciler_test.gotest/e2e-sdk/sdk_sanity_test.gotest/go.mod
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hyperfleet-operator/config/crd/bases/hyperfleet.io_nodepools.yaml (1)
1883-1889: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftRestore the NodePool scaling conflict validation.
The schema now accepts both
spec.nodePool.replicasandspec.nodePool.autoScaling. These fields define competing desired-size controls. Reject this combination at admission time.Add the equivalent
XValidationmarker to the passthrough source or generator so regeneration preserves the CEL rule.🤖 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 `@hyperfleet-operator/config/crd/bases/hyperfleet.io_nodepools.yaml` around lines 1883 - 1889, The NodePool schema must reject configurations that set both spec.nodePool.replicas and spec.nodePool.autoScaling. Add the equivalent XValidation marker in the passthrough source or generator for the NodePool spec, using a CEL rule that enforces these fields are not simultaneously present, so regenerated CRDs retain the admission validation.
🧹 Nitpick comments (5)
platform-api/internal/codegen/featuregate/registry.go (1)
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrevent mutation of the gate registry.
HyperFleetFeatureGatesis an exported package-level map. Any importing package can add, change, or delete an entry. The registry decides which API fields are exposed and writable, so a mutation changes gating for the whole process. Concurrent reads and a write also race, because Go maps are not safe for concurrent use.Make the map unexported and expose read-only accessors.
♻️ Proposed refactor
-// HyperFleetFeatureGates is the registry of all feature gates. -var HyperFleetFeatureGates = map[string]FeatureGateInfo{ +// hyperFleetFeatureGates is the registry of all feature gates. +var hyperFleetFeatureGates = map[string]FeatureGateInfo{+// Gate returns the registered information for a gate. +func Gate(gate string) (FeatureGateInfo, bool) { + info, exists := hyperFleetFeatureGates[gate] + return info, exists +} + // IsGateEnabled returns true if the given gate is enabled for the feature set. func IsGateEnabled(gate string, featureSet FeatureSet) bool { - info, exists := HyperFleetFeatureGates[gate] + info, exists := hyperFleetFeatureGates[gate] if !exists { return false } return featureSet.Includes(info.Stage) }Update
GatesForFeatureSetto readhyperFleetFeatureGates, and update any external reference to the old exported name.🤖 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/featuregate/registry.go` around lines 5 - 6, Make the feature-gate registry map unexported by renaming HyperFleetFeatureGates to hyperFleetFeatureGates, update GatesForFeatureSet to use the private map, and replace all external references with read-only accessor usage rather than exposing mutable map state.platform-api/internal/codegen/featuregate/types.go (1)
6-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument that
Includesdepends on this declaration order.
Includescompares stages numerically. The comparison is correct only while the constants stay ordered from least to most permissive. A new stage inserted betweenGAandTechPreviewwould silently change which gates are enabled for every feature set. Record the invariant next to the constants.♻️ Proposed comment
const ( + // Stages must stay ordered from least to most permissive. + // FeatureSet.Includes relies on this numeric order. GA FeatureStage = iota TechPreview DevPreview )🤖 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/featuregate/types.go` around lines 6 - 10, Add a concise comment immediately above the `GA`, `TechPreview`, and `DevPreview` constants documenting that `Includes` compares their numeric values and therefore requires the stages to remain ordered from least to most permissive; preserve the existing declaration order.hack/api-codegen/pkg/markers/scanner.go (1)
100-110: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEmpty prefixes leave registry keys un-namespaced in a flat map.
rootTypePrefixreturns""for every root type it does not recognize, andscanDiriteratesdirCachein nondeterministic order. Two root types with the same JSON field name then produce one key, and the surviving entry depends on map order. The generatedfield_metadata.goalready shows unprefixed keys such asmaxPodsnext tokubelet.maxPods, which is the observable output of this behavior.
hack/api-codegen/pkg/markers/scanner.go#L100-L110: replace the string-prefix checks with an explicit prefix table, and return an error for aPassthroughtype that has no mapped prefix.hack/api-codegen/pkg/markers/scanner.go#L82-L92: propagate that error out ofscanDirso an unmapped root type fails generation instead of writing flat keys.🤖 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 `@hack/api-codegen/pkg/markers/scanner.go` around lines 100 - 110, The rootTypePrefix/scanDir flow silently produces unnamespaced registry keys for unmapped Passthrough root types. In hack/api-codegen/pkg/markers/scanner.go:100-110, replace prefix checks with an explicit type-to-prefix table and return an error when a Passthrough type lacks a mapping; in hack/api-codegen/pkg/markers/scanner.go:82-92, propagate that error from scanDir so generation stops before writing flat keys.README.md (1)
25-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
api/to the directory table.The module layout block now documents
api/go.modas a standalone module. The directory table near the top of the file listsplatform-api/,hyperfleet-operator/,hyperfleet-db/, andtest/, but notapi/. Add a row so a reader finds the CRD types module in both places.🤖 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 `@README.md` around lines 25 - 27, Update the README directory table to add an api/ row describing the standalone CRD types module, matching the existing table format and the api/go.mod entry in the module layout block.api/v1alpha1/cluster_types.go (1)
88-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse one required/optional marker style in this file.
HostedClusteruses+kubebuilder:validation:Required. Every other field inClusterSpecuses+optional, andCluster(Lines 153-164) uses+required. Switch to+requiredfor consistency. Both markers produce the same CRD output, so this change is cosmetic.♻️ Proposed marker change
// HostedCluster contains the upstream HyperShift fields, mirrored as // passthrough types with per-field visibility and write-mode markers. - // +kubebuilder:validation:Required + // +required HostedCluster HostedClusterSpecPassthrough `json:"hostedCluster"`🤖 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 `@api/v1alpha1/cluster_types.go` around lines 88 - 91, Change the kubebuilder marker immediately above the HostedCluster field in ClusterSpec from +kubebuilder:validation:Required to +required, matching the marker style used by the surrounding ClusterSpec and Cluster fields.
🤖 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/v1alpha1/cluster_types.go`:
- Around line 75-80: Update the CreatorARN validation pattern on the CreatorARN
field to accept the commercial, China, and GovCloud AWS partitions, matching the
repository’s established ARN pattern; leave the field’s other markers and
comment unchanged.
In `@api/v1alpha1/hostedclusterspec.passthrough.go`:
- Around line 96-103: Update the passthrough generator input that defines
Configuration to use the HyperFleet-owned ClusterConfiguration type instead of
hypershiftv1beta1.ClusterConfiguration, preserving the existing JSON tag and
markers. Do not modify the generated hostedclusterspec.passthrough.go file
directly.
In `@api/v1alpha1/manifest_types.go`:
- Around line 58-60: Restrict write access to Manifest objects exposed through
the manifest-editor-role: bind that role only to principals trusted with
equivalent management-cluster authority, or enforce an admission allow-list
covering permitted groups, kinds, namespaces, resources, and targets before
ApplyDesires forwards content and resource to kube-applier-aws. Preserve read
access and existing Manifest behavior for authorized callers.
In `@api/v1alpha1/nodepool_types.go`:
- Around line 55-65: Update the NodePool request decoding and conversion flow to
reject customer-supplied AccountID and InternalPoolID values before constructing
the NodePool resource. Use a customer-facing request type that omits these
fields or invoke the existing service-set validation on NodePoolSpec before
conversion, while preserving valid request handling.
In `@CLAUDE.md`:
- Around line 38-40: Update the CRD ownership entry in the “Key Conventions”
section of CLAUDE.md to state that CRD types live in the standalone api module
and are imported by hyperfleet-operator and platform-api. Remove the outdated
ownership wording while preserving the surrounding conventions.
In `@hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml`:
- Around line 89-96: Complete the truncated passthrough field descriptions in
hostedclusterspec.passthrough.go by preserving all lines from the corresponding
upstream doc comments, including the affected configuration, etcd,
imageContentSources, issuerURL, secretEncryption, olmCatalogPlacement, and
serviceAccountSigningKey fields. Then regenerate the CRD schema so hostedCluster
descriptions in hyperfleet.io_clusters.yaml contain the full sentences.
- Around line 3504-3510: Restore the upstream validation, immutability, format,
default, and bounds markers in api/v1alpha1/hostedclusterspec.passthrough.go for
every corresponding HostedClusterSpec field, including fips, clusterID, infraID,
issuerURL, capabilities, and the listed URL, RFC3339, hostname, collection, and
property constraints. Document any intentionally omitted constraint in the
passthrough struct, then regenerate the Cluster CRD so its schema preserves the
upstream behavior.
- Around line 8349-8358: Update the generator input for the hostedCluster
passthrough schema to define defaults for both required fields, using false for
fips and an empty object for sshKey; then regenerate the CRDs so the generated
schema reflects those defaults instead of editing this YAML directly.
In `@hyperfleet-operator/internal/render/cluster.go`:
- Around line 181-191: Update hostedCluster and the API conversion boundary
around toHostedClusterSpec so only the explicit public HostedCluster
specification is persisted and rendered; do not passthrough the complete
ClusterSpec. Strip or reject service-set fields including Platform, Etcd,
ServiceAccountSigningKey, SecretEncryption, and OperatorConfiguration before
storage and before rendering, while preserving the existing conversion error
handling.
In `@Makefile`:
- Around line 287-290: Update the generate target to run object generation
inside the api module by changing the command to execute cd api before invoking
CONTROLLER_GEN with paths="./..."; leave the existing hyperfleet-operator CRD
generation unchanged.
In `@platform-api/internal/codegen/conversion/cluster.go`:
- Around line 34-40: Update RewriteCloudURLWithID to normalize the URL separator
when combining baseURL and clusterID, preventing duplicate slashes if baseURL
already ends with one; add the strings import and preserve the existing nil-spec
behavior and cloudUrl assignment.
In `@platform-api/internal/codegen/registry/field_metadata.go`:
- Around line 1-22: Remove the unused generated registry under the registry
package, including its re-exported aliases, constants, and FieldRegistry
definition; do not add generation changes unless choosing instead to establish a
single target that consistently updates both registry copies.
- Around line 580-584: Remove the HyperFleetAutoScaling feature-gate marker from
the Tags field in api/v1alpha1/cluster_types.go, leaving the generated field
metadata unchanged. Ensure spec.tags remains mutable and included in the default
feature set.
In `@platform-api/pkg/types/cluster.go`:
- Line 6: Update ClusterCRToPlatform and the writeJSON serialization path so
Cluster responses omit the internal ClusterSpec fields accountId and internalId,
while preserving all customer-visible fields; alternatively, align the public
schema and visibility metadata to explicitly support those fields.
---
Outside diff comments:
In `@hyperfleet-operator/config/crd/bases/hyperfleet.io_nodepools.yaml`:
- Around line 1883-1889: The NodePool schema must reject configurations that set
both spec.nodePool.replicas and spec.nodePool.autoScaling. Add the equivalent
XValidation marker in the passthrough source or generator for the NodePool spec,
using a CEL rule that enforces these fields are not simultaneously present, so
regenerated CRDs retain the admission validation.
---
Nitpick comments:
In `@api/v1alpha1/cluster_types.go`:
- Around line 88-91: Change the kubebuilder marker immediately above the
HostedCluster field in ClusterSpec from +kubebuilder:validation:Required to
+required, matching the marker style used by the surrounding ClusterSpec and
Cluster fields.
In `@hack/api-codegen/pkg/markers/scanner.go`:
- Around line 100-110: The rootTypePrefix/scanDir flow silently produces
unnamespaced registry keys for unmapped Passthrough root types. In
hack/api-codegen/pkg/markers/scanner.go:100-110, replace prefix checks with an
explicit type-to-prefix table and return an error when a Passthrough type lacks
a mapping; in hack/api-codegen/pkg/markers/scanner.go:82-92, propagate that
error from scanDir so generation stops before writing flat keys.
In `@platform-api/internal/codegen/featuregate/registry.go`:
- Around line 5-6: Make the feature-gate registry map unexported by renaming
HyperFleetFeatureGates to hyperFleetFeatureGates, update GatesForFeatureSet to
use the private map, and replace all external references with read-only accessor
usage rather than exposing mutable map state.
In `@platform-api/internal/codegen/featuregate/types.go`:
- Around line 6-10: Add a concise comment immediately above the `GA`,
`TechPreview`, and `DevPreview` constants documenting that `Includes` compares
their numeric values and therefore requires the stages to remain ordered from
least to most permissive; preserve the existing declaration order.
In `@README.md`:
- Around line 25-27: Update the README directory table to add an api/ row
describing the standalone CRD types module, matching the existing table format
and the api/go.mod entry in the module layout block.
🪄 Autofix
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: 04266c7d-689e-4667-97b3-648432d4937b
⛔ Files ignored due to path filters (10)
api/go.sumis excluded by!**/*.sumapi/v1alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*clientset/generated/fake/register.gois excluded by!**/generated/**clientset/generated/scheme/register.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/cluster.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/fake/fake_cluster.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/fake/fake_nodepool.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/nodepool.gois excluded by!**/generated/**hyperfleet-operator/api/v1alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*platform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (70)
.tekton/rosa-hyperfleet-api-pull-request.yaml.tekton/rosa-hyperfleet-api-push.yaml.tekton/rosa-hyperfleet-operator-pull-request.yaml.tekton/rosa-hyperfleet-operator-push.yamlCLAUDE.mdMakefileREADME.mdapi/go.modapi/v1alpha1/cluster_types.goapi/v1alpha1/configuration.goapi/v1alpha1/groupversion_info.goapi/v1alpha1/hostedclusterspec.passthrough.goapi/v1alpha1/install/install.goapi/v1alpha1/managementcluster_types.goapi/v1alpha1/manifest_types.goapi/v1alpha1/nodepool_types.goapi/v1alpha1/placement_types.goapi/v1alpha1/public/.gitkeepclientset/docs/architecture.mdclientset/go.modclientset/wrappers/wire_wrappers_generated.goclientset/wrappers/wrappers_test.gohack/api-codegen/pkg/markers/scanner.gohack/api-codegen/pkg/markers/scanner_test.gohyperfleet-operator/Containerfilehyperfleet-operator/PROJECThyperfleet-operator/cmd/manager/main.gohyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yamlhyperfleet-operator/config/crd/bases/hyperfleet.io_nodepools.yamlhyperfleet-operator/go.modhyperfleet-operator/internal/controller/cluster_controller.gohyperfleet-operator/internal/controller/cluster_controller_test.gohyperfleet-operator/internal/controller/manifest_controller.gohyperfleet-operator/internal/controller/manifest_controller_test.gohyperfleet-operator/internal/controller/nodepool_controller.gohyperfleet-operator/internal/controller/nodepool_controller_test.gohyperfleet-operator/internal/controller/placement_controller.gohyperfleet-operator/internal/controller/placement_controller_test.gohyperfleet-operator/internal/controller/suite_test.gohyperfleet-operator/internal/dynamo/statusstream/manager.gohyperfleet-operator/internal/render/cluster.gohyperfleet-operator/internal/render/cluster_test.gohyperfleet-operator/internal/render/convert.gohyperfleet-operator/internal/render/nodepool.gohyperfleet-operator/internal/render/nodepool_test.gohyperfleet-operator/test/cluster_test.gohyperfleet-operator/test/helpers_test.gohyperfleet-operator/test/manifest_test.gohyperfleet-operator/test/suite_test.goplatform-api/Containerfileplatform-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.jsonplatform-api/pkg/clients/hyperfleetdb/client.goplatform-api/pkg/clients/hyperfleetdb/client_test.goplatform-api/pkg/clients/hyperfleetdb/convert.goplatform-api/pkg/clients/hyperfleetdb/convert_test.goplatform-api/pkg/handlers/cluster_test.goplatform-api/pkg/handlers/management_cluster.goplatform-api/pkg/handlers/zoa_test.goplatform-api/pkg/types/cluster.goplatform-api/pkg/types/nodepool.goplatform-api/pkg/zoa/jobbuilder.goplatform-api/pkg/zoa/reconciler.goplatform-api/pkg/zoa/reconciler_test.gotest/e2e-sdk/sdk_sanity_test.gotest/go.mod
🛑 Comments failed to post (1)
api/v1alpha1/manifest_types.go (1)
58-60: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect Manifest writers and the controller path that converts templates to desires. rg -n -i -C 6 'manifests|kind:[[:space:]]*(Role|ClusterRole)|resources:' \ -g '*.yaml' -g '*.yml' -g '*.go' . rg -n -C 8 'ResourceTemplate|ManifestSpec|ApplyDesire|managementCluster' \ -g '*.go' api hyperfleet-operator platform-apiRepository: openshift-online/rosa-hyperfleet-api
Length of output: 50393
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- relevant files ---' git ls-files | rg -i '(rbac|role|clusterrole|manifest|controller|reconciler|dynamo|desire)' | head -200 printf '%s\n' '--- Manifest API and controller references ---' rg -n -C 5 'type Manifest|type ResourceTemplate|ManifestSpec|ResourceTemplate|BuildManifest|ApplyDesire|targetItem|ManagementCluster' \ api/v1alpha1 hyperfleet-operator/internal platform-api/pkg \ -g '*.go' | head -500 printf '%s\n' '--- authorization and admission configuration ---' rg -n -i -C 5 'manifests|hyperfleet.io|rolebinding|clusterrolebinding|authorization|admission|validat' \ . -g '*.yaml' -g '*.yml' -g '*.go' -g '*.json' \ | head -500Repository: openshift-online/rosa-hyperfleet-api
Length of output: 50392
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- Manifest RBAC ---' for f in \ hyperfleet-operator/config/rbac/manifest_admin_role.yaml \ hyperfleet-operator/config/rbac/manifest_editor_role.yaml \ hyperfleet-operator/config/rbac/manifest_viewer_role.yaml \ hyperfleet-operator/config/rbac/cluster_admin_role.yaml \ hyperfleet-operator/config/rbac/cluster_editor_role.yaml \ hyperfleet-operator/config/rbac/cluster_viewer_role.yaml \ hyperfleet-operator/config/rbac/role.yaml \ hyperfleet-operator/charts/templates/clusterrole.yaml \ hyperfleet-operator/charts/templates/clusterrolebinding.yaml do if test -f "$f"; then echo "### $f" cat -n "$f" fi done printf '%s\n' '--- API type definitions ---' rg -n -C 12 'type (Manifest|ManifestSpec|ResourceTemplate) struct' api/v1alpha1/manifest_types.go printf '%s\n' '--- Manifest controller implementation ---' cat -n hyperfleet-operator/internal/controller/manifest_controller.go printf '%s\n' '--- Manifest design documentation ---' cat -n hyperfleet-operator/docs/manifest-controller.md | head -260Repository: openshift-online/rosa-hyperfleet-api
Length of output: 45823
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path import re rbac = {} for name in [ "manifest_admin_role.yaml", "manifest_editor_role.yaml", "manifest_viewer_role.yaml", ]: p = Path("hyperfleet-operator/config/rbac") / name text = p.read_text() rbac[name] = { "writes_manifests": bool(re.search( r"resources:\s*\n\s*-\s*manifests\b.*?verbs:\s*\n((?:\s*-\s*[^\n]+\n)+)", text, re.S)), "has_create": bool(re.search(r"^\s*-\s*create\s*$", text, re.M)), "has_update": bool(re.search(r"^\s*-\s*update\s*$", text, re.M)), } controller = Path("hyperfleet-operator/internal/controller/manifest_controller.go").read_text() api = Path("api/v1alpha1/manifest_types.go").read_text() docs = Path("hyperfleet-operator/docs/manifest-controller.md").read_text() print("manifest_rbac:", rbac) print("controller_passes_raw_content:", "KubeContent: &res.Content" in controller and "UpsertApplyDesire(ctx, specsPrefix, desire)" in controller) print("controller_passes_user_resource:", "Resource: res.Resource" in controller) print("api_has_only_min_resource_validation:", "validation:MinLength=1" in api and "Resource is the plural resource name" in api) print("docs_no_admission_validation:", "No admission-time validation" in docs) print("docs_describes_arbitrary_apply:", "Deploys arbitrary Kubernetes resources" in docs) PY printf '%s\n' '--- bindings to manifest editor/admin roles ---' rg -n -C 5 'manifest-(editor|admin|viewer)-role|roleRef:|kind:[[:space:]]*(RoleBinding|ClusterRoleBinding)' \ hyperfleet-operator/config hyperfleet-operator/charts \ -g '*.yaml' -g '*.yml' || trueRepository: openshift-online/rosa-hyperfleet-api
Length of output: 7638
Authorization Bypass (CWE-269): Improper Privilege Management
Reachability: External · Exploitability: Moderate
Reachability path
● Entry platform-api/pkg/zoa/reconciler_test.go:515 Run │ ▼ ● Hop platform-api/pkg/zoa/reconciler.go:385 ManifestPhaseApplied │ ▼ ● Sink api/v1alpha1/manifest_types.goRestrict Manifest write access to trusted principals.
manifest-editor-rolegrants create and update access to arbitraryManifestobjects. The operator forwardscontentandresourceinto ApplyDesires thatkube-applier-awsapplies to the management cluster. Bind this role only to principals with equivalent management-cluster authority, or add an admission allow-list for permitted groups, kinds, namespaces, resources, and targets.🤖 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 `@api/v1alpha1/manifest_types.go` around lines 58 - 60, Restrict write access to Manifest objects exposed through the manifest-editor-role: bind that role only to principals trusted with equivalent management-cluster authority, or enforce an admission allow-list covering permitted groups, kinds, namespaces, resources, and targets before ApplyDesires forwards content and resource to kube-applier-aws. Preserve read access and existing Manifest behavior for authorized callers.Source: Path instructions
Set GOCACHE to /workspace/.cache/go-build so the non-root USER 1001 can write build cache during the container image build. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cdoan1
added
the
do-not-merge/work-in-progress
13 tasks
…argets Port field validation from ROSAENG-61802 branch: FieldValidator with write-mode enforcement wired into cluster and nodepool handlers. Add codegen Makefile targets (codegen-passthrough, codegen-registry, codegen-verify, verify-codegen) refactored for api/v1alpha1 layout. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
Makefile (1)
293-299: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRun object generation inside the
apimodule.The object-generation recipe in Lines 293-299 runs from the repository root with
paths="./api/...". If the root has nogo.modorgo.work,controller-gencannot load the standaloneapimodule. This blocks the dependent codegen targets in Lines 331-339 and 342-345.This is the same issue reported in the previous review. Move only the object-generation command into
api, or provide a root workspace.Proposed fix
generate: - $(CONTROLLER_GEN) object paths="./api/..." + cd api && $(CONTROLLER_GEN) object paths="./..."Run this verification script:
#!/bin/bash set -euo pipefail echo "=== module and workspace files ===" fd -t f -g 'go.mod' -g 'go.work' -g 'go.work.sum' | sort echo "=== Go module context ===" for dir in . api hyperfleet-operator platform-api; do if [ -d "$dir" ]; then printf '%s: ' "$dir" (cd "$dir" && GOTOOLCHAIN=local go env GOMOD GOWORK 2>&1) || true fi done echo "=== relevant Makefile recipes ===" sed -n '284,345p' MakefileAlso applies to: 331-339, 342-345
🤖 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 293 - 299, Update the object-generation recipe under generate to run $(CONTROLLER_GEN) from the api directory, while preserving its existing object paths and dependencies. Move only this command into api so dependent generate-clientset and related codegen targets can load the standalone api module; do not add a root workspace or alter unrelated recipes.
🤖 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 `@hack/api-codegen/README.md`:
- Around line 71-78: Add the text language identifier to the fenced
dependency-tree block in the README by changing its opening fence to ```text,
while leaving the tree content unchanged.
In `@Makefile`:
- Line 340: Keep the Makefile target codegen dependent only on codegen-verify,
leaving passthrough generation manual. Update Makefile lines 116-117 to state
that verification covers automatic outputs and passthrough generation is manual,
and align hack/api-codegen/README.md lines 63-67 with these target behaviors and
descriptions.
In `@platform-api/pkg/handlers/cluster.go`:
- Around line 312-321: Handle the return value of json.Encoder.Encode in both
writeValidationErrors helpers: platform-api/pkg/handlers/cluster.go lines
312-321 and platform-api/pkg/handlers/nodepool.go lines 266-275. When encoding
fails, log the error through the respective handler logger instead of discarding
it, while preserving the existing validation response behavior.
In `@platform-api/pkg/validation/field_validator_test.go`:
- Around line 3-253: Convert the tests in the FieldValidator suite from
testing.T to Ginkgo/Gomega, replacing test functions and assertions with Ginkgo
specs and Gomega matchers while preserving all existing scenarios and
expectations. Add the package-level Ginkgo RunSpecs bootstrap, and update module
dependencies so Ginkgo remains at v2.28.1 and Gomega v1.42.1 is declared
directly.
In `@platform-api/pkg/validation/field_validator.go`:
- Around line 131-139: Update ValidateUpdate and the request-to-resource
conversion paths to preserve existing values for registry.ServiceSet fields
instead of allowing zero or omitted fields to clear them. Merge only
customer-owned fields, or use field-presence information to distinguish omission
from an explicit write and reject explicit writes, while keeping
platform-managed values from req.Spec from replacing the existing spec.
- Around line 140-153: The update handlers ApplyPlatformUpdateToClusterCR and
ApplyPlatformUpdateToNodePoolCR must preserve immutable fields omitted from
partial updates. Merge incoming specs with the existing spec before replacement,
or validate the merged union using correctly prefixed nested paths so
registry.Immutable entries such as
spec.hostedCluster.configuration.machineConfig.fips are checked. Add regression
tests covering omitted immutable fields for both handlers.
---
Duplicate comments:
In `@Makefile`:
- Around line 293-299: Update the object-generation recipe under generate to run
$(CONTROLLER_GEN) from the api directory, while preserving its existing object
paths and dependencies. Move only this command into api so dependent
generate-clientset and related codegen targets can load the standalone api
module; do not add a root workspace or alter unrelated recipes.
🪄 Autofix
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: eb11b37d-a248-4125-9d12-aa8d62109a48
📒 Files selected for processing (7)
.gitignoreMakefilehack/api-codegen/README.mdplatform-api/pkg/handlers/cluster.goplatform-api/pkg/handlers/nodepool.goplatform-api/pkg/validation/field_validator.goplatform-api/pkg/validation/field_validator_test.go
…, and codegen enhancements - Scanner verbose mode for marker debugging - Service-set field preservation via snapshot pattern in update handlers - AccountID/InternalID population on create for clusters and node pools - Registry relocated from platform-api/internal/codegen to hack/api-codegen/pkg - OpenAPI merge command and generator enhancements (passthrough collapse, ref targets) - imageContentSources changed to mutable with openapi-gen=true - Configuration field uses local ClusterConfiguration type - OpenAPI spec moved to api/v1alpha1/public/openapi.yaml - CRD regenerated with allowDangerousTypes=true - Makefile codegen targets updated for api/v1alpha1 layout Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
platform-api/pkg/clients/hyperfleetdb/convert.go (1)
76-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the parameter order of the two conversion functions.
PlatformCreateToClusterCRtakes(clusterID, accountID string, ...).PlatformCreateToNodePoolCRtakes(accountID, internalPoolID string, ...). Both leading parameters are plain strings, so a caller that swaps them still compiles. The result is an account ID stored as an internal ID and an internal ID used as the tenant label.Use the same order in both functions, or introduce distinct named string types for the account ID and the internal ID so the compiler rejects a swap.
Also applies to: 152-155
🤖 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/pkg/clients/hyperfleetdb/convert.go` around lines 76 - 79, Align PlatformCreateToClusterCR with PlatformCreateToNodePoolCR by using the same accountID-then-internalID parameter order, and update all call sites accordingly. Ensure the assignments to spec.AccountID and spec.InternalID remain mapped to the correct arguments, including the corresponding conversion logic around PlatformCreateToNodePoolCR.platform-api/pkg/clients/hyperfleetdb/convert_test.go (1)
41-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case where the request spec already carries
AccountIDand the internal ID.Both tests start from a spec that leaves
AccountID,InternalID, andInternalPoolIDempty, so they prove only that the fields are populated. The important guarantee of this change is that the server value wins. Add a case that setsSpec.AccountIDandSpec.InternalID(orSpec.InternalPoolID) to attacker-supplied values in the request, then assert that the converted resource holds the server-provided values instead.Also applies to: 74-80
🤖 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/pkg/clients/hyperfleetdb/convert_test.go` around lines 41 - 47, Extend the conversion tests around the existing np.Spec.AccountID and np.Spec.InternalPoolID assertions with a request whose spec pre-populates AccountID and InternalID or InternalPoolID with different attacker-supplied values. Assert that the converted resource retains the server-provided values, confirming they override request values while preserving the existing empty-field coverage.hack/api-codegen/pkg/openapi/generator.go (1)
153-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThree hardcoded maps describe the same type set and can drift apart.
typeToRegistryPrefix(Lines 157-165),refTargets(Lines 231-236), and the two literal slices insidecollapsePassthroughTypes(Lines 244 and 269) all enumerate the same passthrough and configuration types. A change to one map does not force a change to the others. For example, a new nested configuration type needs an entry in the prefix map, an entry inrefTargets, and possibly a slice entry, and nothing detects an omission.Consider one table keyed by type name that holds the registry prefix, the passthrough flag, and the child field-to-definition mapping. Derive all three current behaviors from that table.
Also applies to: 229-236
🤖 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 `@hack/api-codegen/pkg/openapi/generator.go` around lines 153 - 165, Replace the separate type lists in typeToRegistryPrefix, refTargets, and collapsePassthroughTypes with one type-name-keyed metadata table containing each type’s registry prefix, passthrough status, and child field-to-definition mappings. Update refTargets and both collapsePassthroughTypes paths to derive their current behavior from this table, preserving existing mappings while ensuring adding a type requires only one entry.hack/api-codegen/pkg/openapi/generator_test.go (1)
9-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd assertions for the new collapse behavior, and fix the directory variable name.
This test covers hidden-field pruning only. The same change adds
collapsePassthroughTypes, which is untested here. Add assertions for:
HostedClusterSpecPassthroughandNodePoolSpecPassthroughhaveAdditionalProperties.Allows == true.ClusterSpec.Properties["hostedCluster"].Refpoints toHostedClusterSpecPassthrough.- A non-
$refproperty of the passthrough type, for exampleautoNode, has no nestedPropertiesand noRequiredentries.The variable
v2alpha1Dirat Line 14 holds thev1alpha1path. Rename it tov1alpha1Dir.🤖 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 `@hack/api-codegen/pkg/openapi/generator_test.go` around lines 9 - 72, Extend TestConfigurationUsesLocalType to assert collapsePassthroughTypes behavior: verify HostedClusterSpecPassthrough and NodePoolSpecPassthrough allow additional properties, confirm ClusterSpec’s hostedCluster property references HostedClusterSpecPassthrough, and verify a non-$ref property such as autoNode has no nested Properties or Required entries. Rename the incorrectly named v2alpha1Dir variable and update its uses to v1alpha1Dir.
🤖 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/v1alpha1/public/openapi.yaml`:
- Around line 2957-3013: Regenerate HostedClusterSpecPassthrough from the
current markers so it includes the visible etcd, networking, and platform
properties and requires etcd, fips, networking, and platform. Remove autoNode
from its required list while preserving it as optional; leave hidden service-set
fields and the empty NodePoolSpecPassthrough unchanged.
In `@hack/api-codegen/cmd/openapi-merge/main.go`:
- Around line 216-227: The YAML line-processing logic must avoid bufio.Scanner’s
token limit and handle errors explicitly. In
hack/api-codegen/cmd/openapi-merge/main.go:216-227, replace the scanner-based
splitting around the current prefix-writing logic with a non-truncating
implementation that preserves blank-line handling; in
hack/api-codegen/cmd/openapi-merge/main.go:252-259, make the same replacement
for the other scanner-based splitter. Ensure both paths propagate any read or
processing errors instead of ignoring them.
In `@hack/api-codegen/pkg/openapi/generator.go`:
- Around line 183-187: Update filterHiddenFields around typeToRegistryPrefix so
an unmapped definition produces an explicit error instead of continuing without
pruning; propagate that error through Generate and its callers. Preserve pruning
for mapped definitions, and use the registry-derived prefix when available
rather than silently falling back to the definition name.
In `@hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml`:
- Around line 503-522: Update ClusterConfiguration and its generated CRD schema
for apiServer, authentication, featureGate, image, ingress, network, oauth,
proxy, and scheduler: remove these placeholder fields if unsupported, or mark
their source definitions with +kubebuilder:pruning:PreserveUnknownFields and
regenerate the CRD so nested configuration is retained.
In `@platform-api/pkg/handlers/nodepool.go`:
- Around line 198-208: Update the nodepool update flow around
ApplyPlatformUpdateToNodePoolCR to preserve every omitted service-set field,
including ClusterName, Release, Platform, AccountID, and InternalPoolID, rather
than restoring only two fields; preferably merge only mutable fields. Apply the
same nested service-set preservation logic in the cluster update handler.
---
Nitpick comments:
In `@hack/api-codegen/pkg/openapi/generator_test.go`:
- Around line 9-72: Extend TestConfigurationUsesLocalType to assert
collapsePassthroughTypes behavior: verify HostedClusterSpecPassthrough and
NodePoolSpecPassthrough allow additional properties, confirm ClusterSpec’s
hostedCluster property references HostedClusterSpecPassthrough, and verify a
non-$ref property such as autoNode has no nested Properties or Required entries.
Rename the incorrectly named v2alpha1Dir variable and update its uses to
v1alpha1Dir.
In `@hack/api-codegen/pkg/openapi/generator.go`:
- Around line 153-165: Replace the separate type lists in typeToRegistryPrefix,
refTargets, and collapsePassthroughTypes with one type-name-keyed metadata table
containing each type’s registry prefix, passthrough status, and child
field-to-definition mappings. Update refTargets and both
collapsePassthroughTypes paths to derive their current behavior from this table,
preserving existing mappings while ensuring adding a type requires only one
entry.
In `@platform-api/pkg/clients/hyperfleetdb/convert_test.go`:
- Around line 41-47: Extend the conversion tests around the existing
np.Spec.AccountID and np.Spec.InternalPoolID assertions with a request whose
spec pre-populates AccountID and InternalID or InternalPoolID with different
attacker-supplied values. Assert that the converted resource retains the
server-provided values, confirming they override request values while preserving
the existing empty-field coverage.
In `@platform-api/pkg/clients/hyperfleetdb/convert.go`:
- Around line 76-79: Align PlatformCreateToClusterCR with
PlatformCreateToNodePoolCR by using the same accountID-then-internalID parameter
order, and update all call sites accordingly. Ensure the assignments to
spec.AccountID and spec.InternalID remain mapped to the correct arguments,
including the corresponding conversion logic around PlatformCreateToNodePoolCR.
🪄 Autofix
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: a9df3855-ed12-4470-86ab-175103e22808
⛔ Files ignored due to path filters (1)
api/v1alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*
📒 Files selected for processing (22)
.gitignoreMakefileREADME.mdapi/v1alpha1/hostedclusterspec.passthrough.goapi/v1alpha1/public/openapi.yamlhack/api-codegen/cmd/marker-scanner/main.gohack/api-codegen/cmd/openapi-merge/main.gohack/api-codegen/pkg/markers/gated_writemode_test.gohack/api-codegen/pkg/markers/scanner.gohack/api-codegen/pkg/markers/scanner_test.gohack/api-codegen/pkg/markers/types.gohack/api-codegen/pkg/openapi/generator.gohack/api-codegen/pkg/openapi/generator_test.gohack/api-codegen/pkg/registry/field_metadata.gohack/api-codegen/pkg/registry/field_metadata.jsonhyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yamlplatform-api/pkg/clients/hyperfleetdb/convert.goplatform-api/pkg/clients/hyperfleetdb/convert_test.goplatform-api/pkg/handlers/cluster.goplatform-api/pkg/handlers/nodepool.goplatform-api/pkg/validation/field_validator.goplatform-api/pkg/validation/field_validator_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- .gitignore
- README.md
- api/v1alpha1/hostedclusterspec.passthrough.go
- platform-api/pkg/validation/field_validator.go
- hack/api-codegen/pkg/markers/scanner_test.go
- platform-api/pkg/validation/field_validator_test.go
- platform-api/pkg/handlers/cluster.go
Comment on lines
+503
to
+522
| properties: | ||
| apiServer: | ||
| description: apiServer contains advanced network settings | ||
| for the API server. | ||
| type: object | ||
| authentication: | ||
| description: authentication contains configuration for the | ||
| cluster authentication. | ||
| type: object | ||
| featureGate: | ||
| description: featureGate contains the desired configuration | ||
| for feature gates. | ||
| type: object | ||
| image: | ||
| description: image contains the configuration for internal | ||
| registry. | ||
| type: object | ||
| ingress: | ||
| description: ingress contains the configuration for ingress. | ||
| type: object |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the opaque configuration sub-objects are declared, populated, or rendered anywhere.
set -uo pipefail
echo "=== local ClusterConfiguration type ==="
fd -t f 'configuration.go' | while IFS= read -r f; do
echo "--- $f"
cat -n "$f"
done
echo "=== producers/consumers of configuration sub-objects ==="
rg -n -C4 '\.Configuration\b|Configuration:\s*&|APIServer:|Authentication:|FeatureGate:|Ingress:|Network:|OAuth:|Proxy:|Scheduler:' \
--glob '*.go' -g '!**/vendor/**' | head -120
echo "=== preserve-unknown-fields markers present in api types ==="
rg -n 'PreserveUnknownFields|x-kubernetes-preserve-unknown-fields' --glob '*.go' --glob '*.yaml' -g '!**/vendor/**' | head -40Repository: openshift-online/rosa-hyperfleet-api
Length of output: 9263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== generated schema sections ==="
sed -n '475,535p' hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml
sed -n '630,685p' hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml
echo "=== schema-level preservation markers ==="
rg -n -C3 'x-kubernetes-preserve-unknown-fields|hostedCluster|configuration:' \
hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml | head -160
echo "=== source markers and placeholder declarations ==="
rg -n -C3 'openapi-gen=false|PreserveUnknownFields|type (APIServerNetworkConfiguration|ClusterAuthentication|FeatureGateConfiguration|ImageConfiguration|IngressConfiguration|NetworkConfiguration|OAuthConfiguration|SchedulerConfiguration|ProxyConfiguration) struct' \
api hyperfleet-operator --glob '*.go' --glob '*.yaml' --glob '*.yml' 2>/dev/null | head -160
echo "=== all configuration field usages ==="
rg -n -C3 'Configuration\.(APIServer|Authentication|FeatureGate|Image|Ingress|Network|OAuth|Proxy|Scheduler)|Configuration:\s*&|\.Configuration\b' \
--glob '*.go' -g '!**/vendor/**' | head -240Repository: openshift-online/rosa-hyperfleet-api
Length of output: 25398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
crd = Path("hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml").read_text()
source = Path("api/v1alpha1/configuration.go").read_text()
fields = ["apiServer", "authentication", "featureGate", "image", "ingress",
"network", "oauth", "proxy", "scheduler"]
# Inspect each configuration child schema block until the next sibling.
start = crd.index(" configuration:")
end = crd.index(" controlPlaneRelease:", start)
schema = crd[start:end]
for field in fields:
match = re.search(
rf"^ {re.escape(field)}:\n(?P<body>.*?)(?=^ \w|\\Z)",
schema, re.MULTILINE | re.DOTALL,
)
assert match, field
body = match.group("body")
assert re.search(r"^\s+type: object$", body, re.MULTILINE), field
assert "x-kubernetes-preserve-unknown-fields: true" not in body, field
assert "properties:" not in body, field
print(f"{field}: empty object schema; unknown fields are not preserved")
# Model the relevant structural-schema pruning operation for a concrete payload.
payload = {
"configuration": {
"network": {"networkType": " OVNKubernetes "},
"proxy": {"httpProxy": "http://proxy.example"},
}
}
for field in ("network", "proxy"):
payload["configuration"][field] = {}
assert payload == {"configuration": {"network": {}, "proxy": {}}}
print("network.networkType and proxy.httpProxy are removed by this schema shape")
# Confirm the source uses placeholder types and that no fields are declared in them.
for typ in [
"APIServerNetworkConfiguration", "ClusterAuthentication",
"FeatureGateConfiguration", "ImageConfiguration", "IngressConfiguration",
"NetworkConfiguration", "OAuthConfiguration",
"SchedulerConfiguration", "ProxyConfiguration",
]:
match = re.search(rf"type {typ} struct \{{(?P<body>.*?)\}}", source, re.DOTALL)
assert match and not match.group("body").strip(), typ
print(f"{typ}: empty placeholder type")
PYRepository: openshift-online/rosa-hyperfleet-api
Length of output: 785
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== scheduler schema ==="
sed -n '650,680p' hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml
python3 - <<'PY'
from pathlib import Path
import re
crd = Path("hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml").read_text()
start = crd.index(" configuration:")
end = crd.index(" controlPlaneRelease:", start)
schema = crd[start:end]
fields = ["apiServer", "authentication", "featureGate", "image", "ingress",
"network", "oauth", "proxy", "scheduler"]
# Split on exact YAML sibling indentation instead of a generic word boundary.
children = re.split(r"(?m)^ (?=[A-Za-z][A-Za-z0-9]*:\n)", schema)
blocks = {}
for child in children[1:]:
name = child.split(":", 1)[0]
blocks[name] = child
for field in fields:
body = blocks[field]
assert re.search(r"(?m)^\s+type: object$", body), field
assert "x-kubernetes-preserve-unknown-fields: true" not in body, field
assert "properties:" not in body, field
print(f"{field}: empty object schema; unknown fields are not preserved")
payload = {
"configuration": {
"network": {"networkType": "OVNKubernetes"},
"proxy": {"httpProxy": "http://proxy.example"},
"scheduler": {"policy": "default"},
}
}
for field in ("network", "proxy", "scheduler"):
payload["configuration"][field] = {}
assert payload == {"configuration": {"network": {}, "proxy": {}, "scheduler": {}}}
print("nested fields under network, proxy, and scheduler are pruned by this schema shape")
PYRepository: openshift-online/rosa-hyperfleet-api
Length of output: 2292
Remove or preserve the placeholder configuration objects.
The CRD defines apiServer, authentication, featureGate, image, ingress, network, oauth, proxy, and scheduler as empty object schemas. The Kubernetes API server prunes fields inside these objects because the schema defines no properties and does not set x-kubernetes-preserve-unknown-fields: true.
These fields use empty placeholder types. If they are not supported, remove them from ClusterConfiguration. If they must accept opaque configuration, add +kubebuilder:pruning:PreserveUnknownFields and regenerate the CRD.
🤖 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 `@hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml` around
lines 503 - 522, Update ClusterConfiguration and its generated CRD schema for
apiServer, authentication, featureGate, image, ingress, network, oauth, proxy,
and scheduler: remove these placeholder fields if unsupported, or mark their
source definitions with +kubebuilder:pruning:PreserveUnknownFields and
regenerate the CRD so nested configuration is retained.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml`:
- Around line 523-659: Add the requested validation bounds to the source
definitions for KubeletConfig and MachineConfigSpec: set MaxItems on every
listed array, MaxProperties on the six kubelet map fields, and MaxLength on
FileSpec.Contents, SystemdUnit.Contents, and SystemdDropin.Contents. Resolve the
FIPS source of truth between MachineConfigSpec.FIPS and
HostedClusterSpecPassthrough.FIPS, then remove or retain the duplicate
consistently; regenerate the CRD so these validations appear in the schema.
In `@platform-api/pkg/clients/hyperfleetdb/convert.go`:
- Around line 78-79: Update ClusterCRToPlatform and NodePoolCRToPlatform to
explicitly map only customer-visible Spec fields instead of copying complete CR
Specs; omit service-managed accountID, internalID, and internalPoolID from the
REST response objects while preserving all supported customer fields. Add JSON
regression assertions covering these hidden fields’ absence in both converted
responses.
🪄 Autofix
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: b728cef6-c8d8-4540-84e1-52c7054a35f8
⛔ Files ignored due to path filters (1)
api/v1alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*
📒 Files selected for processing (22)
.gitignoreMakefileREADME.mdapi/v1alpha1/hostedclusterspec.passthrough.goapi/v1alpha1/public/openapi.yamlhack/api-codegen/cmd/marker-scanner/main.gohack/api-codegen/cmd/openapi-merge/main.gohack/api-codegen/pkg/markers/gated_writemode_test.gohack/api-codegen/pkg/markers/scanner.gohack/api-codegen/pkg/markers/scanner_test.gohack/api-codegen/pkg/markers/types.gohack/api-codegen/pkg/openapi/generator.gohack/api-codegen/pkg/openapi/generator_test.gohack/api-codegen/pkg/registry/field_metadata.gohack/api-codegen/pkg/registry/field_metadata.jsonhyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yamlplatform-api/pkg/clients/hyperfleetdb/convert.goplatform-api/pkg/clients/hyperfleetdb/convert_test.goplatform-api/pkg/handlers/cluster.goplatform-api/pkg/handlers/nodepool.goplatform-api/pkg/validation/field_validator.goplatform-api/pkg/validation/field_validator_test.go
🚧 Files skipped from review as they are similar to previous changes (8)
- .gitignore
- README.md
- platform-api/pkg/handlers/cluster.go
- platform-api/pkg/handlers/nodepool.go
- api/v1alpha1/hostedclusterspec.passthrough.go
- hack/api-codegen/pkg/markers/scanner.go
- platform-api/pkg/validation/field_validator_test.go
- platform-api/pkg/validation/field_validator.go
…/v1alpha1/public - Add --rest-output-dir and --rest-package flags to conversion-gen CLI to decouple REST type output from conversion functions - Fix generator: non-struct types (ClusterPhase etc.) correctly qualified, runtime.RawExtension import support, REST-local type references unqualified, inlined enrichCRD to avoid redeclaration across files - REST types (visible fields only) generated to api/v1alpha1/public/ (package public), importable by SDK and clientset - Conversion functions (Project/Unproject) generated to platform-api/pkg/conversion/ - ServiceSetFields struct generated to platform-api/pkg/conversion/types.go - Makefile: codegen-conversion, verify-conversion targets - Regenerated OpenAPI spec to reflect current marker state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-api The generated ServiceSetFields type references configv1.URL directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cdoan1
changed the title
…r on unmapped hidden fields Add MaxProperties/MaxItems/MaxLength constraints to KubeletConfig maps, MachineConfigSpec arrays, and content string fields. Remove MachineConfigSpec.FIPS (duplicate of HostedClusterSpecPassthrough.FIPS). Make filterHiddenFields return an error when an unmapped definition has properties matching hidden registry entries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…vice-set fields Replace full spec replacement with JSON merge in ApplyPlatformUpdateToClusterCR and ApplyPlatformUpdateToNodePoolCR. Omitted fields (zero/nil with omitempty) are preserved in the existing spec, eliminating the need for manual restoration and covering all nested service-set fields in passthrough types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove bufio.Scanner's 64KB token limit from YAML line processing in jsonSchemaToYAML and splitLines. Use strings.Split which has no size limit and needs no error handling for in-memory data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Mark Replicas as +k8s:openapi-gen=true and +hyperfleet:write-mode=mutable so customers can set desired node count. Regenerate field registry, REST types, OpenAPI spec, and conversion types to reflect the change. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
/test on-demand-e2e |
typeid
reviewed
Aug 6, 2026
typeid
reviewed
Aug 6, 2026
typeid
reviewed
Aug 6, 2026
typeid
reviewed
Aug 6, 2026
…ge, buildFieldPath, test compilation Address blocking and non-blocking review comments from typeid on PR openshift-online#284: Blocking: - Replace typed mergeSpec with raw JSON MergeSpecJSON to prevent data loss on non-omitempty passthrough fields (hostedCluster, nodePool) - Merge only APIServer into existing Configuration instead of overwriting Non-blocking: - Fix buildFieldPath to produce correct registry paths for KubeletConfig, MachineConfigSpec, and ClusterConfiguration nested types - Regenerate ServiceSetFields with correct types ([]string, []FileSpec, etc.) - Fix integration tests: HostedClusterSpec → HostedClusterSpecPassthrough, add missing defaultClusterExpiration arg to NewClusterHandler - Remove stale public/machineconfigspec_types.go hidden fields - Fix gofmt in convert.go Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Blocking 1 — mergeSpec data loss on non-omitempty fields:
Blocking 2 — Configuration overwrite drops customer kubelet/machineConfig:
Non-blocking fixes: |
Collaborator
Author
|
/test on-demand-e2e |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdoan1, typeid 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-merge-bot
Bot
merged commit Aug 6, 2026
ff8e995
into
openshift-online:main
14 checks passed
13 tasks
This was referenced Aug 11, 2026
ROSAENG-62084: refactor: complete auto generated files and map nodepool namespace to cluster_id
#313
Merged
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
3 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.
Description
Consolidate 61801, 61802, 61803, 61805, and conversion of openapi to go types in api/v1alpha1/public
visibility/write-mode/feature-gate markers.
to customers.
- codegen-registry — scans markers → field_metadata.go/.json
- codegen-conversion — generates REST types in api/v1alpha1/public/ (visible fields only) and Project/Unproject JSON roundtrip functions in platform-api/pkg/conversion/
- verify-conversion — CI target to detect stale generated code
Type of Change
Testing
make test)Checklist
Summary by CodeRabbit
New Features
Validation
Documentation