Skip to content

ROSAENG-65009: fix: passthrough codegen roundtrip rework - #323

Merged
openshift-merge-bot[bot] merged 3 commits into
openshift-online:mainfrom
cdoan1:ROSAENG-65009-passthrough-codegen-roundtrip-rework
Aug 12, 2026
Merged

ROSAENG-65009: fix: passthrough codegen roundtrip rework#323
openshift-merge-bot[bot] merged 3 commits into
openshift-online:mainfrom
cdoan1:ROSAENG-65009-passthrough-codegen-roundtrip-rework

Conversation

@cdoan1

@cdoan1 cdoan1 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Description

Problem

The passthrough codegen pipeline had several gaps preventing a clean round-trip (make codegen-registry && make codegen-passthrough && git diff --exit-code):

  1. passthrough-gen embedded a stale copy of field_metadata.json via //go:embed and silently fell back to it when -registry wasn't passed
  2. The marker scanner skipped zz_generated.passthrough.go due to a blanket zz_generated* file filter, so it couldn't read curated markers from the passthrough file
  3. conversion-gen had the same file filter issue, causing it to miss the passthrough types entirely
  4. conversion-gen's parseTypes() had cyclomatic complexity 31 (lint limit 30)

Changes

  • Remove stale embedded registry — deleted hack/api-codegen/cmd/passthrough-gen/field_metadata.json, removed //go:embed, made -registry flag mandatory
  • Fix file filters — added exception for zz_generated.passthrough.go in both scanner.go and conversion-gen's generator.go
  • Reduce cyclomatic complexity — extracted parseStructType() from parseTypes() in conversion-gen
  • Fix Makefile dependency chain — codegen-passthrough now depends on codegen-registry (not build-api-codegen); removed circular dependency where codegen-registry depended on codegen-passthrough
  • Regenerated downstream outputs — registry (122→162 fields), REST types, OpenAPI spec, CRD manifests, ServiceSetFields
  • Added design doc — docs/api/passthrough-design.md documenting the full pipeline, curation workflow, and marker types

Verification

  • make build — all modules compile
  • make lint — 0 issues across all modules
  • make test-unit — 23 packages pass
  • Pipeline is stable: running make codegen-registry codegen-passthrough twice produces identical output

Type of Change

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

Testing

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

Checklist

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

Summary by CodeRabbit

  • API Changes

    • Updated hosted cluster passthrough settings with clearer required and optional fields.
    • Removed the unsupported hosted cluster Etcd setting.
    • Removed the 50-item limit for image content sources.
    • Updated node pool passthrough settings to require cluster name, release, and platform information.
    • Refined published API schemas to reflect the supported passthrough configuration.
  • Documentation

    • Added documentation describing passthrough configuration generation and regeneration workflows.

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

openshift-ci-robot commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@cdoan1: This pull request references ROSAENG-65009 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 task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Description

Type of Change

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

Testing

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

Checklist

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

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

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

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

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

Use the following commands to manage reviews:

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

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: a6ebdcdb-aaf5-4e8b-a479-e7f05edff8cf

📥 Commits

Reviewing files that changed from the base of the PR and between 518af2f and 635990f.

📒 Files selected for processing (2)
  • Makefile
  • docs/api/passthrough-design.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/api/passthrough-design.md
  • Makefile

Walkthrough

The change narrows passthrough API schemas, adds field registry metadata, makes passthrough generation registry-driven, updates Makefile dependencies, and expands platform conversion field coverage.

Changes

Passthrough code generation

Layer / File(s) Summary
Passthrough API contracts
api/v1alpha1/public/*passthrough_types.go, api/v1alpha1/public/openapi.yaml
Passthrough types and OpenAPI schemas retain selected fields and update required, optional, and validation markers.
Field registry metadata
hack/api-codegen/pkg/registry/*
Hosted-cluster and node-pool field paths now include write modes, visibility, and feature-gate metadata.
Registry-driven generation
Makefile, hack/api-codegen/cmd/passthrough-gen/main.go, hack/api-codegen/pkg/{conversion,markers}/*, docs/api/passthrough-design.md
The generator requires registry input, parses the generated passthrough source, and uses updated Make targets for registry-first and clobber regeneration.
Platform conversion fields
platform-api/pkg/conversion/types.go, platform-api/go.mod
ServiceSetFields now includes additional platform-managed fields and uses a direct github.com/openshift/api dependency.

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

Sequence Diagram(s)

sequenceDiagram
  participant Makefile
  participant codegen-registry
  participant passthrough-gen
  participant GeneratedPassthrough
  Makefile->>codegen-registry: Generate field_metadata.json
  Makefile->>passthrough-gen: Pass registry file
  passthrough-gen->>GeneratedPassthrough: Parse passthrough declarations
  passthrough-gen-->>Makefile: Generate passthrough output
Loading
🚥 Pre-merge checks | ✅ 9 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Ai-Attribution ⚠️ Warning All three PR commits identify Claude Opus 4.6 with Co-Authored-By trailers; no Assisted-by or Generated-by trailer is present. Remove the AI Co-Authored-By trailers and add the required Red Hat Assisted-by or Generated-by attribution.
✅ Passed checks (9 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto ✅ Passed The PR diff adds no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, custom crypto, or secret/token comparisons; touched Go code is codegen and field metadata.
Container-Privileges ✅ Passed The cumulative PR diff changes no container or Kubernetes privilege settings; added lines contain no privileged:true, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation.
No-Sensitive-Data-In-Logs ✅ Passed The diff adds no logging of passwords, tokens, PII, hostnames, or customer data; generator logs only registry paths, counts, type names, and existing errors.
No-Hardcoded-Secrets ✅ Passed Added-line scans found no API keys, tokens, passwords, private-key material, credential URLs, secret assignments, or base64-like config literals; secret-related additions are field names and types...
No-Injection-Vectors ✅ Passed The PR diff adds no SQL concatenation, shell=True, eval/exec, pickle.loads, yaml.load, os.system, or dangerouslySetInnerHTML; new Make commands use fixed paths.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the passthrough codegen roundtrip rework, which matches the main changes in the pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@cdoan1
cdoan1 force-pushed the ROSAENG-65009-passthrough-codegen-roundtrip-rework branch from 6492ffd to 36b394f Compare August 12, 2026 04:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Makefile (1)

364-372: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Include passthrough output in the verification workflow.

codegen-verify only refreshes the registry and builds packages. verify-codegen only checks deepcopy and registry diffs. A stale zz_generated.passthrough.go can therefore pass CI, despite the documented round-trip guarantee.

  • Makefile#L364-L372: make verification run codegen-passthrough and diff api/v1alpha1/zz_generated.passthrough.go.
  • docs/api/passthrough-design.md#L151-L161: retain the CI verification claim only after the Makefile verifies passthrough output.
🤖 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 364 - 372, The codegen verification workflow must
regenerate and validate passthrough output. In Makefile lines 364-372, update
codegen-verify to run codegen-passthrough and update verify-codegen to diff
api/v1alpha1/zz_generated.passthrough.go alongside the existing generated files.
In docs/api/passthrough-design.md lines 151-161, retain the CI verification
claim after the Makefile checks passthrough output.
🤖 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/hostedclusterspecpassthrough_types.go`:
- Around line 14-40: Regenerate the public API deepcopy output for the changed
types, including Cluster, NodePool, and other public API root types, so their
generated DeepCopyObject methods implement runtime.Object and the public API
package builds successfully. Commit the regenerated output alongside the
HostedClusterSpecPassthrough API change.

In `@docs/api/passthrough-design.md`:
- Around line 96-106: Update the “Type overrides” section to describe the
implemented -type-overrides behavior, removing “planned” wording and references
to it as a remaining gap. Retain the invocation and explain that overrides are
applied during type resolution before import collection; update the
corresponding remaining-gaps entry as well.

In `@Makefile`:
- Around line 348-358: Update the codegen-passthrough-clobber target
prerequisites to include codegen-registry before deleting the generated
passthrough file and invoking passthrough-gen, ensuring the generator uses
refreshed registry metadata.

---

Outside diff comments:
In `@Makefile`:
- Around line 364-372: The codegen verification workflow must regenerate and
validate passthrough output. In Makefile lines 364-372, update codegen-verify to
run codegen-passthrough and update verify-codegen to diff
api/v1alpha1/zz_generated.passthrough.go alongside the existing generated files.
In docs/api/passthrough-design.md lines 151-161, retain the CI verification
claim after the Makefile checks passthrough output.
🪄 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: 4a6696bd-2b6e-4489-bbc0-15c2980b62a7

📥 Commits

Reviewing files that changed from the base of the PR and between 93162f5 and 36b394f.

⛔ Files ignored due to path filters (3)
  • api/v1alpha1/public/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • api/v1alpha1/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • api/v1alpha1/zz_generated.passthrough.go is excluded by !**/zz_generated*
📒 Files selected for processing (17)
  • Makefile
  • api/v1alpha1/public/hostedclusterspecpassthrough_types.go
  • api/v1alpha1/public/nodepoolspecpassthrough_types.go
  • api/v1alpha1/public/openapi.yaml
  • docs/api/passthrough-design.md
  • hack/api-codegen/cmd/passthrough-gen/field_metadata.json
  • hack/api-codegen/cmd/passthrough-gen/main.go
  • hack/api-codegen/pkg/conversion/generator.go
  • hack/api-codegen/pkg/markers/scanner.go
  • hack/api-codegen/pkg/markers/scanner_test.go
  • hack/api-codegen/pkg/passthrough/loader.go
  • hack/api-codegen/pkg/passthrough/loader_test.go
  • hack/api-codegen/pkg/passthrough/types.go
  • hack/api-codegen/pkg/registry/field_metadata.go
  • hack/api-codegen/pkg/registry/field_metadata.json
  • hyperfleet-operator/config/crd/bases/hyperfleet.io_clusters.yaml
  • platform-api/pkg/conversion/types.go
💤 Files with no reviewable changes (4)
  • hack/api-codegen/cmd/passthrough-gen/field_metadata.json
  • api/v1alpha1/public/openapi.yaml
  • api/v1alpha1/public/nodepoolspecpassthrough_types.go
  • hack/api-codegen/pkg/markers/scanner_test.go

Comment thread api/v1alpha1/public/hostedclusterspecpassthrough_types.go
Comment thread docs/api/passthrough-design.md Outdated
Comment thread Makefile
@cdoan1
cdoan1 force-pushed the ROSAENG-65009-passthrough-codegen-roundtrip-rework branch 2 times, most recently from 362f8e1 to d750955 Compare August 12, 2026 04:26

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 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 `@api/v1alpha1/public/hostedclusterspecpassthrough_types.go`:
- Around line 14-15: Regenerate and commit the complete deepcopy output for the
api/v1alpha1/public package so Cluster, NodePool, ManagementCluster, Manifest,
Placement, and their list types implement DeepCopyObject. Ensure the generated
files include all methods required for the package to compile alongside the
Release field change.
- Around line 54-55: Add the missing MaxItems=255 validation marker to
ImageContentSources in the passthrough schema, matching
HostedClusterSpec.ImageContentSources, then regenerate the OpenAPI and CRD
schemas.
🪄 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: 5c15ba3c-6bd6-462f-8f14-dabbd547b37b

📥 Commits

Reviewing files that changed from the base of the PR and between 362f8e1 and d750955.

⛔ Files ignored due to path filters (1)
  • api/v1alpha1/public/zz_generated.deepcopy.go is excluded by !**/zz_generated*
📒 Files selected for processing (3)
  • api/v1alpha1/public/hostedclusterspecpassthrough_types.go
  • api/v1alpha1/public/nodepoolspecpassthrough_types.go
  • platform-api/pkg/conversion/types.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • platform-api/pkg/conversion/types.go

Comment thread api/v1alpha1/public/hostedclusterspecpassthrough_types.go
Comment thread api/v1alpha1/public/hostedclusterspecpassthrough_types.go
@cdoan1

cdoan1 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

/retest

@cdoan1
cdoan1 force-pushed the ROSAENG-65009-passthrough-codegen-roundtrip-rework branch 2 times, most recently from 8579232 to 518af2f Compare August 12, 2026 13:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

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

Inline comments:
In `@hack/api-codegen/cmd/passthrough-gen/main.go`:
- Around line 29-35: Update the passthrough generation flow around flag
validation and registry loading so codegen-passthrough-clobber invokes or
otherwise depends on codegen-registry before reading field_metadata.json,
ensuring marker metadata is regenerated rather than stale and handling the
absent-file case successfully. Use the existing codegen-registry target or
function instead of duplicating registry generation logic.
🪄 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: 8390526d-221f-4ebe-b135-a653eba1ac0f

📥 Commits

Reviewing files that changed from the base of the PR and between d750955 and 518af2f.

⛔ Files ignored due to path filters (2)
  • api/v1alpha1/public/zz_generated.deepcopy.go is excluded by !**/zz_generated*
  • api/v1alpha1/zz_generated.passthrough.go is excluded by !**/zz_generated*
📒 Files selected for processing (8)
  • Makefile
  • api/v1alpha1/public/hostedclusterspecpassthrough_types.go
  • hack/api-codegen/cmd/passthrough-gen/main.go
  • hack/api-codegen/pkg/passthrough/loader.go
  • hack/api-codegen/pkg/registry/field_metadata.go
  • hack/api-codegen/pkg/registry/field_metadata.json
  • platform-api/go.mod
  • platform-api/pkg/conversion/types.go
💤 Files with no reviewable changes (1)
  • hack/api-codegen/pkg/passthrough/loader.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • hack/api-codegen/pkg/registry/field_metadata.go
  • hack/api-codegen/pkg/registry/field_metadata.json
  • api/v1alpha1/public/hostedclusterspecpassthrough_types.go
  • platform-api/pkg/conversion/types.go

Comment thread hack/api-codegen/cmd/passthrough-gen/main.go
cdoan1 and others added 3 commits August 12, 2026 07:30
Documents the passthrough round-trip pipeline, curation workflow,
marker types, safe defaults, and five remaining gaps to fix on main.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove stale embedded registry from passthrough-gen, make -registry mandatory
- Fix scanner and conversion-gen file filters to read zz_generated.passthrough.go
- Add -type-overrides flag to passthrough-gen for local type substitution
- Extract parseStructType() to reduce cyclomatic complexity in conversion-gen
- Fix Makefile dependency chain: codegen-passthrough depends on codegen-registry
- Regenerate downstream outputs (registry, deepcopy, REST types, OpenAPI, CRDs)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Type override feature removed — already implemented separately in main.
Update design doc to reflect resolved gaps and remove stale references.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cdoan1
cdoan1 force-pushed the ROSAENG-65009-passthrough-codegen-roundtrip-rework branch from 635990f to 9eaa448 Compare August 12, 2026 14:30
@cdoan1

cdoan1 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

/retest

@cdoan1

cdoan1 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

/honk

@openshift-ci

openshift-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

@cdoan1:
goose image

Details

In response to this:

/honk

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 kubernetes-sigs/prow repository.

@gdbranco

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2026
@openshift-ci

openshift-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoan1, gdbranco

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-merge-bot
openshift-merge-bot Bot merged commit f0366b5 into openshift-online:main Aug 12, 2026
13 checks passed
@cdoan1
cdoan1 deleted the ROSAENG-65009-passthrough-codegen-roundtrip-rework branch August 12, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants