ROSAENG-61801: feat: adding passthrough types /api/v1alpha1 - #193
ROSAENG-61801: feat: adding passthrough types /api/v1alpha1#193cdoan1 wants to merge 13 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdoan1 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
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:
WalkthroughChangesThe PR adds a standalone API code-generation module with marker scanning, field registries, feature-gate filtering, passthrough and conversion generation, OpenAPI output, and request validation. It also adds API configuration types, generated passthrough types, CLI commands, tests, documentation, dependencies, and Makefile workflows. API codegen and validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
cdoan1
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
hack/api-codegen/cmd/conversion-gen/main.go-41-48 (1)
41-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject empty
--input-dirsentries. Trailing or doubled commas pass""tofilepath.Abs, which resolves to the current working directory; trim entries and fail fast on empties.🤖 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/cmd/conversion-gen/main.go` around lines 41 - 48, Update the inputDirs parsing loop to trim whitespace from each entry before calling filepath.Abs, and fail fast when a trimmed entry is empty so trailing or doubled commas are rejected instead of resolving to the current directory. Preserve absolute-path conversion for valid entries and keep the existing error handling through log.Fatalf.hack/api-codegen/pkg/openapi/generator_test.go-12-13 (1)
12-13: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse a test-owned temp file.
/tmp/openapi-test.jsoncan collide with concurrent test runs, and the cleanup path ignoresos.Removeerrors. Usefilepath.Join(t.TempDir(), "openapi.json")instead.🤖 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 12 - 13, Update the test setup around tmpFile to create the file with filepath.Join(t.TempDir(), "openapi.json") instead of a shared /tmp path. Remove the manual defer cleanup, since t.TempDir() owns lifecycle cleanup and avoids concurrent test collisions.Source: Path instructions
hack/api-codegen/cmd/verify-configuration/main.go-68-85 (1)
68-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate the write-mode value, not just marker presence.
A typo such as
+hyperfleet:write-mode=mutablpasses because any containing string setshasWriteMode. Parse the marker and allow-listmutable,immutable, andservice-set.🤖 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/cmd/verify-configuration/main.go` around lines 68 - 85, Update the marker handling in the field and inline-comment scans to parse the value following +hyperfleet:write-mode= instead of treating marker presence as valid. Set hasWriteMode only for mutable, immutable, or service-set; reject unknown or misspelled values while preserving the existing visibility-marker handling.hack/api-codegen/pkg/validation/validator.go-92-124 (1)
92-124: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNon-deterministic error ordering.
req.Fieldsis a map, so thefor fieldPath := range req.Fieldsloop yields errors in a randomized order. Callers surfacingValidationErrors.Error()to users (or logging it) will see field ordering vary run-to-run for the same invalid request. Consider sorting collected field paths (or the resultingerrorsslice) before returning.🤖 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/validation/validator.go` around lines 92 - 124, Make Validator.Validate produce deterministic ValidationErrors by sorting the field paths from req.Fields before iterating, or sorting the collected errors before returning. Preserve the existing feature-gate and validateWriteMode checks while ensuring identical invalid requests always return errors in a stable field order.hack/api-codegen/pkg/featuregate/registry.go-50-62 (1)
50-62: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNon-deterministic gate order.
GatesForFeatureSetappends togateswhile iterating theHyperFleetFeatureGatesmap, so the returned order is randomized per run. This feeds directly intoSummarizeFeatureSet's%vformatting incrd_filter.go, producing non-reproducible summary text across runs. Sort the result before returning.🤖 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/featuregate/registry.go` around lines 50 - 62, Sort the gates collected by GatesForFeatureSet before returning them, ensuring deterministic ordering for SummarizeFeatureSet output while preserving the existing stage filtering behavior.hack/api-codegen/pkg/featuregate/crd_variant.go-26-70 (1)
26-70: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSwallowed
Closeerror on the error path.At lines 57-64,
f.Close()is invoked without checking its return value before returning the primary error. Per the Go error-handling guideline for this repo, error returns should not be discarded — a failure to close (e.g. flush error) after a partial write could leave a corrupted/truncated output file without any signal.🔧 Proposed fix
if err := encoder.Encode(&crd); err != nil { - f.Close() + _ = f.Close() return fmt.Errorf("writing YAML: %w", err) } if err := encoder.Close(); err != nil { - f.Close() + _ = f.Close() return fmt.Errorf("closing YAML encoder: %w", err) }As per path instructions,
**/*.goshould "Never ignore error returns."🤖 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/featuregate/crd_variant.go` around lines 26 - 70, Update the error paths in GenerateVariant so every f.Close() call checks and handles its returned error instead of discarding it, including failures from encoder.Encode and encoder.Close. Preserve the original operation error while incorporating any close failure into the returned error, and keep the existing successful close flow unchanged.Source: Path instructions
hack/api-codegen/pkg/passthrough/loader.go-250-260 (1)
250-260: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPotential panic when
typeName == "Spec".
base := strings.TrimSuffix(typeName, "Spec")yields""whentypeNameis exactly"Spec", and sincebase != typeNamethe code proceeds torunes[0] = ...on an empty slice, panicking. Unlikely in practice given current type names, but there's no guard.🐛 Proposed fix
base := strings.TrimSuffix(typeName, "Spec") - if base == typeName { + if base == typeName || base == "" { return "" }🤖 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/passthrough/loader.go` around lines 250 - 260, Guard deriveFieldPrefix after trimming the “Spec” suffix so an empty base returns an empty prefix before indexing runes. Preserve the existing behavior for non-Spec type names and valid suffixed types.hack/api-codegen/pkg/openapi/generator.go-190-231 (1)
190-231: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFields without any JSON tag are silently dropped from the schema instead of falling back to the Go field name.
jsonName == "" || jsonName == "-"is treated as "excluded," but Go'sencoding/jsonsemantics only exclude on explicitjson:"-"; a field with no tag at all is still marshaled using its Go field name. Combined withextractJSONTagreturning""for untagged fields, such fields silently vanish from the generated OpenAPI schema with no warning.🐛 Proposed fix
jsonName := g.extractJSONTag(field) - if jsonName == "" || jsonName == "-" { + if jsonName == "-" { continue } + if jsonName == "" { + jsonName = name.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 `@hack/api-codegen/pkg/openapi/generator.go` around lines 190 - 231, Update the field-name handling in the schema-generation loop around extractJSONTag and generateFieldSchema so an untagged field falls back to its Go identifier name, matching encoding/json behavior. Continue excluding fields only when the explicit JSON tag is "-", and preserve existing handling for tagged names and required fields.
🧹 Nitpick comments (19)
hack/api-codegen/cmd/featuregate-info/main.go (1)
12-47: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueHandle CLI output errors consistently. Both commands discard errors from
fmt.Print*/fmt.Fprint*, which can report a broken pipe or failed output stream.
hack/api-codegen/cmd/featuregate-info/main.go#L12-L47: route output through a helper that exits non-zero when writing fails.hack/api-codegen/cmd/verify-configuration/main.go#L15-L115: use the same checked-output pattern for diagnostics and success messages.As per path instructions, “Never ignore error returns.”
🤖 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/cmd/featuregate-info/main.go` around lines 12 - 47, Route every fmt.Print*/fmt.Fprint* call in hack/api-codegen/cmd/featuregate-info/main.go (lines 12-47) through a shared checked-output helper that exits non-zero on write failure; apply the same pattern to all diagnostic and success output in hack/api-codegen/cmd/verify-configuration/main.go (lines 15-115), ensuring no output error returns are ignored.Source: Path instructions
hack/api-codegen/pkg/validation/gated_writemode_test.go (1)
162-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssertion doesn't verify error content, only that an error occurred.
errorReason(e.g."immutable","service-set") is set per test case to document the expected failure category, but the check only verifieserrStris non-empty rather thanstrings.Contains(errStr, tt.errorReason). This means the carefully-crafted precedence scenarios (e.g. "Multiple gates - first match wins") wouldn't actually catch a regression that returns the wrong kind of validation error (e.g. "service-set" instead of "immutable") as long as some error is returned.validator_test.goalready uses the correctstrings.Containspattern nearby.✅ Proposed fix
+ "strings" "testing" ... } else if tt.errorReason != "" { - errStr := err.Error() - if errStr == "" || len(errStr) == 0 { - t.Errorf("Expected error containing %q, got empty error", tt.errorReason) - } + if !strings.Contains(err.Error(), tt.errorReason) { + t.Errorf("Expected error containing %q, got %v", tt.errorReason, err) + }🤖 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/validation/gated_writemode_test.go` around lines 162 - 172, Update the error assertion in the gated writemode test’s expectError branch to verify that err.Error() contains tt.errorReason using strings.Contains. Preserve the existing nil and empty-error checks, and add the required strings import if it is not already present.hack/api-codegen/pkg/featuregate/crd_filter.go (1)
11-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winField-inclusion predicate duplicated across the package.
The "skip hidden → no gate means always include → otherwise check
IsGateEnabled" logic is repeated verbatim inFilterCRDFieldsandFieldsForFeatureSet, and reimplemented a third time incrd_variant.go'sshouldIncludeField. Extracting a singlefunc isFieldIncluded(meta registry.FieldMeta, fs FeatureSet) boolhelper would remove the duplication and prevent the three copies from silently diverging.🤖 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/featuregate/crd_filter.go` around lines 11 - 58, Extract the shared field-inclusion predicate into an isFieldIncluded(meta registry.FieldMeta, fs FeatureSet) bool helper, covering hidden fields, ungated fields, and IsGateEnabled checks. Update FilterCRDFields, FieldsForFeatureSet, and crd_variant.go’s shouldIncludeField to call this helper, removing their duplicated logic while preserving current behavior.hack/api-codegen/pkg/featuregate/crd_variant.go (2)
26-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated read-parse-filter logic between
GenerateVariantandWriteVariantToWriter.Both functions repeat the same "read file →
yaml.Unmarshal→ buildfilterContext→filterCRDNode" sequence. Consider extracting a sharedloadAndFilterCRD(inputPath string, featureSet FeatureSet) (*yaml.Node, error)helper used by both, and byGenerateVariant's error-checked file write andWriteVariantToWriter's writer-based encode.Also applies to: 204-235
🤖 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/featuregate/crd_variant.go` around lines 26 - 47, Extract the duplicated read, YAML unmarshal, filterContext creation, and filterCRDNode invocation from GenerateVariant and WriteVariantToWriter into a shared loadAndFilterCRD helper returning the filtered yaml.Node and errors. Update both callers to use this helper while preserving GenerateVariant’s file output and WriteVariantToWriter’s writer-based encoding behavior.
161-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame inclusion predicate duplicated here as in
crd_filter.go.
shouldIncludeFieldre-implements the Hidden/FeatureGate inclusion check already present incrd_filter.go'sFilterCRDFields/FieldsForFeatureSet. Worth consolidating into a shared helper (see companion comment oncrd_filter.go).🤖 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/featuregate/crd_variant.go` around lines 161 - 180, The field inclusion predicate is duplicated between CRDVariantGenerator.shouldIncludeField and crd_filter.go. Consolidate the Hidden and FeatureGate checks into a shared helper, then update shouldIncludeField and the existing FilterCRDFields/FieldsForFeatureSet path to reuse it while preserving structural-field handling and current inclusion behavior.hack/api-codegen/pkg/featuregate/crd_variant_test.go (1)
105-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a regression case for schema-keyword vs. field-name path collisions.
The gated field in this test (
tags) has its entire subtree removed, so it doesn't exercise the scenario where a real nested field shares a name with an OpenAPI schema keyword (e.g.type) at the same nesting depth — see the critical-severity comment oncrd_variant.go'sfilterCRDNode. Once that's fixed, a test case here would help lock in the fix.🤖 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/featuregate/crd_variant_test.go` around lines 105 - 207, Extend TestCRDVariantGenerator_GenerateVariant with a nested gated schema field whose real property name collides with an OpenAPI keyword such as “type”, then assert the generated default and TechPreview variants preserve or remove that field correctly. Ensure the fixture exercises filterCRDNode at the same nesting depth for both the schema keyword and the field-name path.hack/api-codegen/pkg/validation/example_test.go (1)
54-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExample doesn't exercise real validator behavior.
Unlike the other
Example*functions in this file,ExampleValidator_Validate_immutablejust prints a static string rather than actually validating a request — it documents intent but doesn't prove the immutable write-mode path works.gated_writemode_test.goshows the pattern for constructing aValidatorwith a custom registry entry that has an immutable field; mirroring that here (with a fabricated field, similar to thespec.name/registry.Immutablecases used elsewhere) would make this example an actual, trustworthy demonstration.🤖 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/validation/example_test.go` around lines 54 - 60, The ExampleValidator_Validate_immutable example must exercise Validator behavior instead of printing a static message. Mirror the custom-registry setup from gated_writemode_test.go, defining a fabricated immutable field with the appropriate registry and write-mode markers, then validate create and update requests to demonstrate that setting is allowed on create but rejected on update.hack/api-codegen/pkg/passthrough/integration_test.go (1)
94-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReinvents
strings.Contains.
contains/findSubstringduplicate the standard library'sstrings.Contains.♻️ Proposed fix
-// Check for expected type definitions -if !contains(contentStr, "type HostedClusterSpecPassthrough struct") { +if !strings.Contains(contentStr, "type HostedClusterSpecPassthrough struct") { t.Error("Missing HostedClusterSpecPassthrough type definition") } ... -func contains(s, substr string) bool { - return len(s) > 0 && len(substr) > 0 && len(s) >= len(substr) && findSubstring(s, substr) -} - -func findSubstring(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -}(remember to add
"strings"to the import block)🤖 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/passthrough/integration_test.go` around lines 94 - 105, Replace the custom contains/findSubstring helpers with the standard library’s strings.Contains, adding the strings import and updating all callers of contains accordingly; remove both redundant helper functions.hack/api-codegen/pkg/passthrough/loader_test.go (2)
1-1: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTests requiring live module resolution aren't separated from pure unit tests.
TestLoadHyperShiftTypes,TestGenerateTypeDef, andTestGenerateall callNewGeneratorFromImportPath, which shells out togo listand needs network/module-cache access to resolvegithub.com/openshift/hypershift/api, unlike the properly-labeledintegration_test.go.
hack/api-codegen/pkg/passthrough/loader_test.go#L10-30: tagTestLoadHyperShiftTypeswith a build constraint (e.g.//go:build integration) or move it alongsideintegration_test.go.hack/api-codegen/pkg/passthrough/loader_test.go#L168-203: same forTestGenerateTypeDef.hack/api-codegen/pkg/passthrough/generator_test.go#L1-67: same forTestGenerate.🤖 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/passthrough/loader_test.go` at line 1, Separate the live module-resolution tests from default unit-test runs by adding the existing integration build constraint to TestLoadHyperShiftTypes, TestGenerateTypeDef, and TestGenerate. Apply the constraint consistently to the files containing these tests, preserving their current test logic and allowing them to run only when integration tests are explicitly selected.
10-30: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNetwork/module-resolution dependent tests lack a build tag.
Same pattern as
generator_test.go: these tests require livego listresolution of the HyperShift module. See consolidated comment.Also applies to: 168-203
🤖 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/passthrough/loader_test.go` around lines 10 - 30, Add the same network/module-resolution build tag used by generator_test.go to the tests in loader_test.go, including TestLoadHyperShiftTypes and the additional tests around lines 168–203, so they are excluded unless explicitly enabled.hack/api-codegen/pkg/passthrough/generator_test.go (1)
1-67: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNetwork/module-resolution dependent test lacks a build tag.
TestGeneratecallsNewGeneratorFromImportPathwhich shells out togo listforgithub.com/openshift/hypershift/api/..., requiring network/module-cache access — the same pattern flagged inloader_test.go. See consolidated comment.🤖 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/passthrough/generator_test.go` around lines 1 - 67, Mark TestGenerate with the repository’s established build tag for network/module-resolution-dependent tests, matching the convention used in loader_test.go. Ensure the tag is placed so default test runs exclude this test while explicitly tagged runs still execute it.hack/api-codegen/pkg/passthrough/gomod.go (2)
19-32: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
go listinvocation has no timeout/cancellation.
exec.CommandwithCombinedOutput()can hang indefinitely ifgo liststalls (e.g. slow/unreachable module proxy), blocking the codegen run with no way to cancel it.As per path instructions, Go code should use
context.Context for cancellation and timeouts.🔧 Proposed fix
+import "context" +import "time" + func ResolvePackageDir(importPath string) (string, error) { - cmd := exec.Command("go", "list", "-f", "{{.Dir}}", importPath) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "go", "list", "-f", "{{.Dir}}", importPath) output, err := cmd.CombinedOutput()🤖 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/passthrough/gomod.go` around lines 19 - 32, Update ResolvePackageDir to accept or obtain a context.Context and invoke go list with exec.CommandContext, applying the established timeout policy so stalled module resolution can be cancelled. Preserve the existing output trimming and error messages, including command output when execution fails.Source: Path instructions
44-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
"hypershift"prefix contradicts the generic function name.
NewGeneratorFromImportPathis written as a general-purpose constructor from any import path, but the alias is built as"hypershift" + lastSegmentregardless of the actual package. If this is ever invoked for a non-HyperShift import path, the generated alias would be misleading/wrong (e.g..../config/v1→ alias"hypershiftv1").♻️ Proposed fix
- // Extract package alias from import path (last segment) - parts := strings.Split(importPath, "/") - packageAlias := "hypershift" + parts[len(parts)-1] // e.g., "hypershiftv1beta1" + // Extract package alias from the last two import path segments to + // disambiguate versioned packages (e.g. "hypershiftv1beta1"). + parts := strings.Split(importPath, "/") + packageAlias := parts[len(parts)-1] + if len(parts) >= 2 { + packageAlias = parts[len(parts)-2] + parts[len(parts)-1] + }🤖 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/passthrough/gomod.go` around lines 44 - 63, Update NewGeneratorFromImportPath so SourcePackageAlias is derived generically from the import path’s final segment without prepending the hardcoded “hypershift” prefix. Preserve the existing import-path resolution and generator initialization behavior.hack/api-codegen/pkg/passthrough/loader.go (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate struct-tag parsing and directory-filtering logic between
passthroughandopenapipackages. Both packages independently hand-roll the same small helpers instead of sharing a common internal package, and both tag parsers reimplement whatreflect.StructTagalready provides correctly.
hack/api-codegen/pkg/passthrough/loader.go#L262-277: replaceparseStructTagwithreflect.StructTag(tag).Get("json")and move it to a shared helper package.hack/api-codegen/pkg/openapi/generator.go#L316-356: replaceextractJSONTag/isRequired's manualstrings.Fieldsparsing with the same sharedreflect.StructTag-based helper.hack/api-codegen/pkg/passthrough/loader.go#L21-26: extract the_test.go/zz_generated*skip predicate used byparser.ParseDirinto a shared helper.hack/api-codegen/pkg/openapi/generator.go#L115-120: reuse the same shared skip predicate instead of the duplicated inline closure.🤖 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/passthrough/loader.go` at line 1, Create a shared internal helper package for JSON struct-tag extraction and generated/test-file filtering, then update passthrough’s parseStructTag and openapi’s extractJSONTag/isRequired to use reflect.StructTag-based parsing while preserving required-field behavior. Replace both parser.ParseDir skip closures with the shared predicate, including _test.go and zz_generated* exclusions, and remove the duplicated local helpers.hack/api-codegen/pkg/passthrough/generator.go (1)
86-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDebug
.rawartifact is written unconditionally on every run.The raw pre-format output is written on every successful
Generate()call, not just on formatting failure, leaving a strayzz_generated.passthrough.go.rawfile alongside the real output in every generated directory.♻️ Proposed fix
- // Write unformatted first for debugging - outputFile := filepath.Join(outputDir, "zz_generated.passthrough.go") - if err := os.WriteFile(outputFile+".raw", buf.Bytes(), 0644); err != nil { - return fmt.Errorf("writing raw output file: %w", err) - } - // Format the generated code + outputFile := filepath.Join(outputDir, "zz_generated.passthrough.go") formatted, err := format.Source(buf.Bytes()) if err != nil { + // Dump the raw output only when formatting fails, to aid debugging. + _ = os.WriteFile(outputFile+".raw", buf.Bytes(), 0644) return fmt.Errorf("formatting generated code: %w", err) }🤖 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/passthrough/generator.go` around lines 86 - 90, Update Generate so the raw output artifact is only written when formatting fails, rather than unconditionally before formatting. Move or defer the os.WriteFile call for outputFile+".raw" into the formatting-error path while preserving the existing error context and normal formatted output behavior.hyperfleet-operator/api/v1alpha1/configuration.go (1)
186-224: 📐 Maintainability & Code Quality | 🔵 TrivialNine configuration sub-types are still stubs.
APIServerNetworkConfiguration,ClusterAuthentication,FeatureGateConfiguration,ImageConfiguration,IngressConfiguration,NetworkConfiguration,OAuthConfiguration,SchedulerConfiguration,ProxyConfigurationare empty placeholders. Since they're referenced fromClusterConfiguration(hidden/service-set), they won't block this PR, but flagging so follow-up work is tracked.Want me to open a tracking issue for filling these in with granular markers similar to
KubeletConfig/MachineConfigSpec?🤖 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/api/v1alpha1/configuration.go` around lines 186 - 224, The nine configuration types—APIServerNetworkConfiguration, ClusterAuthentication, FeatureGateConfiguration, ImageConfiguration, IngressConfiguration, NetworkConfiguration, OAuthConfiguration, SchedulerConfiguration, and ProxyConfiguration—remain empty stubs. Define their configuration fields with granular markers consistent with KubeletConfig and MachineConfigSpec, preserving their existing references from ClusterConfiguration.hack/api-codegen/pkg/markers/generator.go (1)
129-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnrecognized
WriteModevalues are silently dropped instead of failing generation.Neither switch has a
defaultcase; an unexpectedWriteMode(e.g. a scanner bug or marker typo) results in a generatedFieldMetaentry missing itsWriteMode, which is harder to notice than a build failure.♻️ Proposed fix
switch meta.WriteMode { case Mutable: field.WriteMode = "Mutable" case Immutable: field.WriteMode = "Immutable" case ServiceSet: field.WriteMode = "ServiceSet" + default: + return fmt.Errorf("unknown write mode %q for field %q", meta.WriteMode, path) }🤖 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/generator.go` around lines 129 - 154, Add default handling to both WriteMode switches in the generator logic around meta.WriteMode and gated.WriteMode, and fail generation immediately when an unrecognized value is encountered. Ensure the failure identifies the invalid WriteMode and relevant field or feature-gate context instead of silently producing an empty WriteMode.hack/api-codegen/pkg/markers/json.go (1)
20-26: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDuplicate
jsonFieldstruct + no validation ofwriteModevalues on load.
jsonFieldis redefined identically in bothGenerateJSONandLoadRegistryFromJSONBytes. Additionally,LoadRegistryFromJSONBytescastsfield.WriteModedirectly toWriteModewithout checking it's one ofMutable/Immutable/ServiceSet— a typo in a hand-edited registry JSON (or a future marker-scanner bug) would load silently instead of failing.♻️ Proposed fix
+type jsonField struct { + FieldPath string `json:"fieldPath"` + WriteMode string `json:"writeMode,omitempty"` + FeatureGate string `json:"featureGate,omitempty"` + Hidden bool `json:"hidden,omitempty"` + FeatureGateAwareWriteModes []FeatureGateWriteMode `json:"featureGateAwareWriteModes,omitempty"` +} + func LoadRegistryFromJSONBytes(data []byte) (FieldRegistry, error) { - type jsonField struct { ... } var fields []jsonField if err := json.Unmarshal(data, &fields); err != nil { return nil, fmt.Errorf("unmarshaling JSON: %w", err) } registry := make(FieldRegistry) for _, field := range fields { + wm := WriteMode(field.WriteMode) + if wm != "" && wm != Mutable && wm != Immutable && wm != ServiceSet { + return nil, fmt.Errorf("field %q: unknown writeMode %q", field.FieldPath, field.WriteMode) + } registry[field.FieldPath] = FieldMeta{ FieldPath: field.FieldPath, - WriteMode: WriteMode(field.WriteMode), + WriteMode: wm, ...Also applies to: 71-97
🤖 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/json.go` around lines 20 - 26, Deduplicate the identical jsonField definition by declaring it once and reusing it in both GenerateJSON and LoadRegistryFromJSONBytes. In LoadRegistryFromJSONBytes, validate each non-empty field.WriteMode against the supported WriteMode values Mutable, Immutable, and ServiceSet before converting it; return an error for any unknown value instead of loading it silently.hack/api-codegen/pkg/conversion/generator.go (1)
200-221: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded type/prefix dispatch tables duplicated and fragile.
buildFieldPathspecial-cases exactlyHostedCluster/NodePoolprefixes for passthrough types (Lines 208-217), and thev1alpha1Typeslist inqualifyType(Lines 271-284) is duplicated verbatim insidegenerateRESTType(Lines 409-412). Both must be manually kept in sync as new passthrough types or v1alpha1 configuration types are added — the same failure mode that produced the registry Hidden-flag drift found elsewhere in this PR. Consider deriving the passthrough prefix from a struct/registry convention rather than name prefixes, and hoisting thev1alpha1Typeslist to a single package-level slice reused by both call sites.Also applies to: 268-308, 395-419
🤖 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/conversion/generator.go` around lines 200 - 221, Remove the duplicated type/prefix dispatch in Generator.buildFieldPath by deriving passthrough registry prefixes from the existing struct or registry convention instead of hardcoded HostedCluster and NodePool name checks. Hoist the v1alpha1Types list used by qualifyType and generateRESTType into one package-level slice, then reuse it from both call sites so additions remain synchronized.
🤖 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/field_metadata.json`:
- Around line 317-320: Synchronize the six affected field metadata entries,
including spec.hostedCluster.autoNode and the entries at the other referenced
locations, with the canonical registry so each preserves its hidden-field
designation. Update the metadata definitions to include the matching hidden
setting while retaining their existing fieldPath and writeMode values.
In `@hack/api-codegen/go.mod`:
- Around line 1-48: Align the API dependency versions in the codegen module’s
go.mod with those declared by hyperfleet-operator/api/go.mod, updating both
github.com/openshift/hypershift/api and the indirect github.com/openshift/api
pins. Ensure passthrough.NewGeneratorFromImportPath uses the same module graph
as the consumer so generated types include the newer upstream fields.
In `@hack/api-codegen/pkg/conversion/generator.go`:
- Around line 504-547: Update the ServiceSetFields collection around fieldsMap
to deduplicate using a resource-qualified key derived from the full field path,
including the spec.hostedCluster or spec.nodePool segment, rather than only the
leaf JSON tag. Derive distinct GoName and JSONTag values from that qualified
path so each resource’s release, platform, and pausedUntil fields remain
separate, and ensure generateUnprojectFunction uses the corresponding qualified
members for each resource.
In `@hack/api-codegen/pkg/featuregate/crd_filter.go`:
- Around line 11-33: Make FilterCRDFields return fields in a deterministic order
by sorting the collected includedFields before returning it. Preserve the
existing hidden-field and feature-gate filtering behavior, and add the required
standard-library import for sorting.
In `@hack/api-codegen/pkg/passthrough/generator.go`:
- Around line 106-156: Update Generator.collectImports to derive qualified-type
imports from g.parsedFiles and each source *ast.File.Imports declaration,
resolving selector qualifiers to their actual import paths instead of matching
only configv1, corev1, and metav1 prefixes. Preserve the existing source-package
import handling and deduplication, and ensure every referenced external
qualifier is included in the generated imports.
In `@hack/api-codegen/pkg/passthrough/loader.go`:
- Around line 217-248: Update Generator.getMarkersForField so a registry entry
with an empty meta.WriteMode still receives the safe default
+hyperfleet:write-mode=service-set marker. Preserve explicitly configured write
modes and the existing openapi-gen and feature-gate markers, and add coverage in
loader_test.go for the registered-field case.
In `@hack/api-codegen/pkg/registry/field_metadata.go`:
- Around line 363-366: Mark the six affected spec.hostedCluster fields—autoNode,
channel, configuration, fips, operatorConfiguration, and pausedUntil—with
Hidden: true in their registry entries. Keep their existing FieldPath and
WriteMode values unchanged so generateRESTType excludes them from visible REST
fields.
In `@hack/api-codegen/pkg/validation/validator.go`:
- Around line 162-177: Update the registry.Immutable branch in Validate so an
OperationUpdate request with nil ExistingFields returns a validation error
instead of allowing the immutable field through. Preserve the existing
immutable-field error when ExistingFields is populated and the field already
exists, while retaining the allowed behavior for genuinely new fields.
In `@hack/tools/go.mod`:
- Line 3: Update the Go version directive in hack/tools/go.mod from 1.26.3 to
1.26.5, preserving the rest of the module configuration.
In `@hyperfleet-operator/api/v1alpha1/cluster_types.go`:
- Around line 75-81: Update the kubebuilder validation pattern on CreatorARN to
anchor both the beginning and end of the value, requiring the entire string—not
just its prefix—to match the intended AWS ARN pattern. Preserve the existing ARN
prefix validation while rejecting trailing or otherwise unexpected content.
In `@Makefile`:
- Around line 272-275: Update the codegen-verify target after the existing build
commands to invoke verify-configuration with $(V1ALPHA1_DIR)/configuration.go,
ensuring configuration markers are validated as part of verification.
In `@platform-api/internal/codegen/registry/field_metadata.json`:
- Line 1: Synchronize the passthrough-gen field metadata registry with the
platform-api registry by marking HostedClusterSpec fields autoNode, channel,
configuration, fips, operatorConfiguration, and pausedUntil as hidden. Update
the passthrough-gen copy, or regenerate both registries from a shared canonical
source, while preserving the existing metadata for all other fields.
- Around line 317-321: Synchronize the six affected hidden field metadata
entries between platform-api/internal/codegen/registry/field_metadata.json and
hack/api-codegen/cmd/passthrough-gen/field_metadata.json, including the
spec.hostedCluster.autoNode entry and the entries at the referenced ranges.
Ensure each matching fieldPath has identical hidden configuration in both files.
---
Minor comments:
In `@hack/api-codegen/cmd/conversion-gen/main.go`:
- Around line 41-48: Update the inputDirs parsing loop to trim whitespace from
each entry before calling filepath.Abs, and fail fast when a trimmed entry is
empty so trailing or doubled commas are rejected instead of resolving to the
current directory. Preserve absolute-path conversion for valid entries and keep
the existing error handling through log.Fatalf.
In `@hack/api-codegen/cmd/verify-configuration/main.go`:
- Around line 68-85: Update the marker handling in the field and inline-comment
scans to parse the value following +hyperfleet:write-mode= instead of treating
marker presence as valid. Set hasWriteMode only for mutable, immutable, or
service-set; reject unknown or misspelled values while preserving the existing
visibility-marker handling.
In `@hack/api-codegen/pkg/featuregate/crd_variant.go`:
- Around line 26-70: Update the error paths in GenerateVariant so every
f.Close() call checks and handles its returned error instead of discarding it,
including failures from encoder.Encode and encoder.Close. Preserve the original
operation error while incorporating any close failure into the returned error,
and keep the existing successful close flow unchanged.
In `@hack/api-codegen/pkg/featuregate/registry.go`:
- Around line 50-62: Sort the gates collected by GatesForFeatureSet before
returning them, ensuring deterministic ordering for SummarizeFeatureSet output
while preserving the existing stage filtering behavior.
In `@hack/api-codegen/pkg/openapi/generator_test.go`:
- Around line 12-13: Update the test setup around tmpFile to create the file
with filepath.Join(t.TempDir(), "openapi.json") instead of a shared /tmp path.
Remove the manual defer cleanup, since t.TempDir() owns lifecycle cleanup and
avoids concurrent test collisions.
In `@hack/api-codegen/pkg/openapi/generator.go`:
- Around line 190-231: Update the field-name handling in the schema-generation
loop around extractJSONTag and generateFieldSchema so an untagged field falls
back to its Go identifier name, matching encoding/json behavior. Continue
excluding fields only when the explicit JSON tag is "-", and preserve existing
handling for tagged names and required fields.
In `@hack/api-codegen/pkg/passthrough/loader.go`:
- Around line 250-260: Guard deriveFieldPrefix after trimming the “Spec” suffix
so an empty base returns an empty prefix before indexing runes. Preserve the
existing behavior for non-Spec type names and valid suffixed types.
In `@hack/api-codegen/pkg/validation/validator.go`:
- Around line 92-124: Make Validator.Validate produce deterministic
ValidationErrors by sorting the field paths from req.Fields before iterating, or
sorting the collected errors before returning. Preserve the existing
feature-gate and validateWriteMode checks while ensuring identical invalid
requests always return errors in a stable field order.
---
Nitpick comments:
In `@hack/api-codegen/cmd/featuregate-info/main.go`:
- Around line 12-47: Route every fmt.Print*/fmt.Fprint* call in
hack/api-codegen/cmd/featuregate-info/main.go (lines 12-47) through a shared
checked-output helper that exits non-zero on write failure; apply the same
pattern to all diagnostic and success output in
hack/api-codegen/cmd/verify-configuration/main.go (lines 15-115), ensuring no
output error returns are ignored.
In `@hack/api-codegen/pkg/conversion/generator.go`:
- Around line 200-221: Remove the duplicated type/prefix dispatch in
Generator.buildFieldPath by deriving passthrough registry prefixes from the
existing struct or registry convention instead of hardcoded HostedCluster and
NodePool name checks. Hoist the v1alpha1Types list used by qualifyType and
generateRESTType into one package-level slice, then reuse it from both call
sites so additions remain synchronized.
In `@hack/api-codegen/pkg/featuregate/crd_filter.go`:
- Around line 11-58: Extract the shared field-inclusion predicate into an
isFieldIncluded(meta registry.FieldMeta, fs FeatureSet) bool helper, covering
hidden fields, ungated fields, and IsGateEnabled checks. Update FilterCRDFields,
FieldsForFeatureSet, and crd_variant.go’s shouldIncludeField to call this
helper, removing their duplicated logic while preserving current behavior.
In `@hack/api-codegen/pkg/featuregate/crd_variant_test.go`:
- Around line 105-207: Extend TestCRDVariantGenerator_GenerateVariant with a
nested gated schema field whose real property name collides with an OpenAPI
keyword such as “type”, then assert the generated default and TechPreview
variants preserve or remove that field correctly. Ensure the fixture exercises
filterCRDNode at the same nesting depth for both the schema keyword and the
field-name path.
In `@hack/api-codegen/pkg/featuregate/crd_variant.go`:
- Around line 26-47: Extract the duplicated read, YAML unmarshal, filterContext
creation, and filterCRDNode invocation from GenerateVariant and
WriteVariantToWriter into a shared loadAndFilterCRD helper returning the
filtered yaml.Node and errors. Update both callers to use this helper while
preserving GenerateVariant’s file output and WriteVariantToWriter’s writer-based
encoding behavior.
- Around line 161-180: The field inclusion predicate is duplicated between
CRDVariantGenerator.shouldIncludeField and crd_filter.go. Consolidate the Hidden
and FeatureGate checks into a shared helper, then update shouldIncludeField and
the existing FilterCRDFields/FieldsForFeatureSet path to reuse it while
preserving structural-field handling and current inclusion behavior.
In `@hack/api-codegen/pkg/markers/generator.go`:
- Around line 129-154: Add default handling to both WriteMode switches in the
generator logic around meta.WriteMode and gated.WriteMode, and fail generation
immediately when an unrecognized value is encountered. Ensure the failure
identifies the invalid WriteMode and relevant field or feature-gate context
instead of silently producing an empty WriteMode.
In `@hack/api-codegen/pkg/markers/json.go`:
- Around line 20-26: Deduplicate the identical jsonField definition by declaring
it once and reusing it in both GenerateJSON and LoadRegistryFromJSONBytes. In
LoadRegistryFromJSONBytes, validate each non-empty field.WriteMode against the
supported WriteMode values Mutable, Immutable, and ServiceSet before converting
it; return an error for any unknown value instead of loading it silently.
In `@hack/api-codegen/pkg/passthrough/generator_test.go`:
- Around line 1-67: Mark TestGenerate with the repository’s established build
tag for network/module-resolution-dependent tests, matching the convention used
in loader_test.go. Ensure the tag is placed so default test runs exclude this
test while explicitly tagged runs still execute it.
In `@hack/api-codegen/pkg/passthrough/generator.go`:
- Around line 86-90: Update Generate so the raw output artifact is only written
when formatting fails, rather than unconditionally before formatting. Move or
defer the os.WriteFile call for outputFile+".raw" into the formatting-error path
while preserving the existing error context and normal formatted output
behavior.
In `@hack/api-codegen/pkg/passthrough/gomod.go`:
- Around line 19-32: Update ResolvePackageDir to accept or obtain a
context.Context and invoke go list with exec.CommandContext, applying the
established timeout policy so stalled module resolution can be cancelled.
Preserve the existing output trimming and error messages, including command
output when execution fails.
- Around line 44-63: Update NewGeneratorFromImportPath so SourcePackageAlias is
derived generically from the import path’s final segment without prepending the
hardcoded “hypershift” prefix. Preserve the existing import-path resolution and
generator initialization behavior.
In `@hack/api-codegen/pkg/passthrough/integration_test.go`:
- Around line 94-105: Replace the custom contains/findSubstring helpers with the
standard library’s strings.Contains, adding the strings import and updating all
callers of contains accordingly; remove both redundant helper functions.
In `@hack/api-codegen/pkg/passthrough/loader_test.go`:
- Line 1: Separate the live module-resolution tests from default unit-test runs
by adding the existing integration build constraint to TestLoadHyperShiftTypes,
TestGenerateTypeDef, and TestGenerate. Apply the constraint consistently to the
files containing these tests, preserving their current test logic and allowing
them to run only when integration tests are explicitly selected.
- Around line 10-30: Add the same network/module-resolution build tag used by
generator_test.go to the tests in loader_test.go, including
TestLoadHyperShiftTypes and the additional tests around lines 168–203, so they
are excluded unless explicitly enabled.
In `@hack/api-codegen/pkg/passthrough/loader.go`:
- Line 1: Create a shared internal helper package for JSON struct-tag extraction
and generated/test-file filtering, then update passthrough’s parseStructTag and
openapi’s extractJSONTag/isRequired to use reflect.StructTag-based parsing while
preserving required-field behavior. Replace both parser.ParseDir skip closures
with the shared predicate, including _test.go and zz_generated* exclusions, and
remove the duplicated local helpers.
In `@hack/api-codegen/pkg/validation/example_test.go`:
- Around line 54-60: The ExampleValidator_Validate_immutable example must
exercise Validator behavior instead of printing a static message. Mirror the
custom-registry setup from gated_writemode_test.go, defining a fabricated
immutable field with the appropriate registry and write-mode markers, then
validate create and update requests to demonstrate that setting is allowed on
create but rejected on update.
In `@hack/api-codegen/pkg/validation/gated_writemode_test.go`:
- Around line 162-172: Update the error assertion in the gated writemode test’s
expectError branch to verify that err.Error() contains tt.errorReason using
strings.Contains. Preserve the existing nil and empty-error checks, and add the
required strings import if it is not already present.
In `@hyperfleet-operator/api/v1alpha1/configuration.go`:
- Around line 186-224: The nine configuration
types—APIServerNetworkConfiguration, ClusterAuthentication,
FeatureGateConfiguration, ImageConfiguration, IngressConfiguration,
NetworkConfiguration, OAuthConfiguration, SchedulerConfiguration, and
ProxyConfiguration—remain empty stubs. Define their configuration fields with
granular markers consistent with KubeletConfig and MachineConfigSpec, preserving
their existing references from ClusterConfiguration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift-online/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: de1710c6-e62b-423b-8a55-cc8d66c64571
⛔ Files ignored due to path filters (2)
hack/api-codegen/go.sumis excluded by!**/*.sumhyperfleet-operator/api/v1alpha1/zz_generated.passthrough.go.rawis excluded by!**/zz_generated*
📒 Files selected for processing (55)
Makefilehack/api-codegen/README.mdhack/api-codegen/cmd/conversion-gen/main.gohack/api-codegen/cmd/crd-variants/main.gohack/api-codegen/cmd/featuregate-info/main.gohack/api-codegen/cmd/marker-scanner/main.gohack/api-codegen/cmd/openapi-gen/main.gohack/api-codegen/cmd/passthrough-gen/field_metadata.jsonhack/api-codegen/cmd/passthrough-gen/main.gohack/api-codegen/cmd/verify-configuration/main.gohack/api-codegen/go.modhack/api-codegen/pkg/conversion/generator.gohack/api-codegen/pkg/conversion/generator_test.gohack/api-codegen/pkg/conversion/mirror_types.gohack/api-codegen/pkg/conversion/mirror_types_test.gohack/api-codegen/pkg/featuregate/crd_filter.gohack/api-codegen/pkg/featuregate/crd_variant.gohack/api-codegen/pkg/featuregate/crd_variant_test.gohack/api-codegen/pkg/featuregate/featuregate_test.gohack/api-codegen/pkg/featuregate/registry.gohack/api-codegen/pkg/featuregate/types.gohack/api-codegen/pkg/markers/gated_writemode_test.gohack/api-codegen/pkg/markers/generator.gohack/api-codegen/pkg/markers/json.gohack/api-codegen/pkg/markers/json_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/openapi/types.gohack/api-codegen/pkg/passthrough/generator.gohack/api-codegen/pkg/passthrough/generator_test.gohack/api-codegen/pkg/passthrough/gomod.gohack/api-codegen/pkg/passthrough/integration_test.gohack/api-codegen/pkg/passthrough/loader.gohack/api-codegen/pkg/passthrough/loader_test.gohack/api-codegen/pkg/passthrough/types.gohack/api-codegen/pkg/registry/field_metadata.gohack/api-codegen/pkg/registry/field_metadata.jsonhack/api-codegen/pkg/validation/example_test.gohack/api-codegen/pkg/validation/gated_writemode_test.gohack/api-codegen/pkg/validation/validator.gohack/api-codegen/pkg/validation/validator_test.gohack/tools/go.modhyperfleet-operator/api/go.modhyperfleet-operator/api/v1alpha1/cluster_types.gohyperfleet-operator/api/v1alpha1/configuration.gohyperfleet-operator/api/v1alpha1/hostedclusterspec.passthrough.gohyperfleet-operator/api/v1alpha1/nodepool_types.goplatform-api/internal/codegen/conversion/cluster.goplatform-api/internal/codegen/featuregate/registry.goplatform-api/internal/codegen/featuregate/types.goplatform-api/internal/codegen/registry/field_metadata.goplatform-api/internal/codegen/registry/field_metadata.json
| { | ||
| "fieldPath": "spec.hostedCluster.autoNode", | ||
| "writeMode": "service-set" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Registry drift vs. platform-api/internal/codegen/registry/field_metadata.json on 6 hidden fields.
See consolidated comment below for full detail and both affected files.
Also applies to: 331-334, 340-343, 364-367, 413-416, 417-420
🤖 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/cmd/passthrough-gen/field_metadata.json` around lines 317 -
320, Synchronize the six affected field metadata entries, including
spec.hostedCluster.autoNode and the entries at the other referenced locations,
with the canonical registry so each preserves its hidden-field designation.
Update the metadata definitions to include the matching hidden setting while
retaining their existing fieldPath and writeMode values.
| fieldsMap := make(map[string]serviceSetField) // Use map to deduplicate | ||
|
|
||
| for path, meta := range registry.FieldRegistry { | ||
| if meta.WriteMode == registry.ServiceSet { | ||
| // Infer Go field name from path | ||
| goName := g.pathToGoName(path) | ||
|
|
||
| // Infer type from parsed types | ||
| goType := g.inferTypeFromPath(path) | ||
|
|
||
| jsonTag := g.pathToJSONTag(path) | ||
|
|
||
| // Use JSONTag as key to deduplicate (same field name from different paths) | ||
| // Prefer longer GoType (more specific) | ||
| if existing, exists := fieldsMap[jsonTag]; exists { | ||
| if len(goType) > len(existing.GoType) { | ||
| fieldsMap[jsonTag] = serviceSetField{ | ||
| GoName: goName, | ||
| GoType: goType, | ||
| JSONTag: jsonTag, | ||
| FieldPath: path, | ||
| } | ||
| } | ||
| } else { | ||
| fieldsMap[jsonTag] = serviceSetField{ | ||
| GoName: goName, | ||
| GoType: goType, | ||
| JSONTag: jsonTag, | ||
| FieldPath: path, | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Convert map to slice for sorting | ||
| var fields []serviceSetField | ||
| for _, f := range fieldsMap { | ||
| fields = append(fields, f) | ||
| } | ||
|
|
||
| // Sort for consistent output | ||
| sort.Slice(fields, func(i, j int) bool { | ||
| return fields[i].GoName < fields[j].GoName | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
ServiceSetFields dedup collides across resource namespaces (hostedCluster vs nodePool).
fieldsMap is keyed by jsonTag (line 504), and both pathToJSONTag/pathToGoName derive names from only the last path segment. Registry paths spec.hostedCluster.release/spec.nodePool.release, spec.hostedCluster.platform/spec.nodePool.platform, and spec.hostedCluster.pausedUntil/spec.nodePool.pausedUntil all collide on the same leaf name. The tie-break at Lines 518-526 (prefer the textually-longer GoType) silently drops one resource's mapping, so ServiceSetFields ends up with a single Platform/Release/PausedUntil member typed for only one of HostedCluster/NodePool. generateUnprojectFunction (Lines 926-931) then emits crdSpec.<Field> = enrichment.<Field> for both resources against that single member — producing either a compile error (type mismatch) or, if types happen to coincide, silently wrong enrichment data for the other resource.
Key fieldsMap (and derive the Go field name) from the full field path rather than the bare leaf segment, e.g. qualify by the spec.hostedCluster/spec.nodePool segment so each resource gets a distinct ServiceSetFields member.
🐛 Proposed direction
- fieldsMap := make(map[string]serviceSetField) // Use map to deduplicate
+ fieldsMap := make(map[string]serviceSetField) // keyed by full path, not leaf jsonTag
for path, meta := range registry.FieldRegistry {
if meta.WriteMode == registry.ServiceSet {
- // Infer Go field name from path
- goName := g.pathToGoName(path)
+ // Infer Go field name from full path to avoid collisions between
+ // e.g. spec.hostedCluster.platform and spec.nodePool.platform
+ goName := g.pathToQualifiedGoName(path)
...
- // Use JSONTag as key to deduplicate (same field name from different paths)
- if existing, exists := fieldsMap[jsonTag]; exists {
+ if existing, exists := fieldsMap[path]; exists {Also applies to: 926-931
🤖 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/conversion/generator.go` around lines 504 - 547, Update
the ServiceSetFields collection around fieldsMap to deduplicate using a
resource-qualified key derived from the full field path, including the
spec.hostedCluster or spec.nodePool segment, rather than only the leaf JSON tag.
Derive distinct GoName and JSONTag values from that qualified path so each
resource’s release, platform, and pausedUntil fields remain separate, and ensure
generateUnprojectFunction uses the corresponding qualified members for each
resource.
| func FilterCRDFields(featureSet FeatureSet) []string { | ||
| var includedFields []string | ||
|
|
||
| for fieldPath, meta := range registry.FieldRegistry { | ||
| // Skip hidden fields - they never appear in CRDs | ||
| if meta.Hidden { | ||
| continue | ||
| } | ||
|
|
||
| // If field has no gate, it's always included (GA) | ||
| if meta.FeatureGate == "" { | ||
| includedFields = append(includedFields, fieldPath) | ||
| continue | ||
| } | ||
|
|
||
| // Check if this feature gate is enabled for the feature set | ||
| if IsGateEnabled(meta.FeatureGate, featureSet) { | ||
| includedFields = append(includedFields, fieldPath) | ||
| } | ||
| } | ||
|
|
||
| return includedFields | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Non-deterministic field order from map iteration.
FilterCRDFields builds includedFields by iterating registry.FieldRegistry, a Go map. Map iteration order is randomized per run, so the returned slice order (and therefore any generated/serialized output derived from it) will vary between runs even when the underlying data is unchanged. For a codegen tool whose output is expected to be reproducible (e.g. for verify-* targets diffing regenerated files), this can produce spurious diffs.
🐛 Proposed fix
+import "sort"
+
func FilterCRDFields(featureSet FeatureSet) []string {
var includedFields []string
for fieldPath, meta := range registry.FieldRegistry {
...
}
+ sort.Strings(includedFields)
return includedFields
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func FilterCRDFields(featureSet FeatureSet) []string { | |
| var includedFields []string | |
| for fieldPath, meta := range registry.FieldRegistry { | |
| // Skip hidden fields - they never appear in CRDs | |
| if meta.Hidden { | |
| continue | |
| } | |
| // If field has no gate, it's always included (GA) | |
| if meta.FeatureGate == "" { | |
| includedFields = append(includedFields, fieldPath) | |
| continue | |
| } | |
| // Check if this feature gate is enabled for the feature set | |
| if IsGateEnabled(meta.FeatureGate, featureSet) { | |
| includedFields = append(includedFields, fieldPath) | |
| } | |
| } | |
| return includedFields | |
| } | |
| import "sort" | |
| func FilterCRDFields(featureSet FeatureSet) []string { | |
| var includedFields []string | |
| for fieldPath, meta := range registry.FieldRegistry { | |
| // Skip hidden fields - they never appear in CRDs | |
| if meta.Hidden { | |
| continue | |
| } | |
| // If field has no gate, it's always included (GA) | |
| if meta.FeatureGate == "" { | |
| includedFields = append(includedFields, fieldPath) | |
| continue | |
| } | |
| // Check if this feature gate is enabled for the feature set | |
| if IsGateEnabled(meta.FeatureGate, featureSet) { | |
| includedFields = append(includedFields, fieldPath) | |
| } | |
| } | |
| sort.Strings(includedFields) | |
| return includedFields | |
| } |
🤖 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/featuregate/crd_filter.go` around lines 11 - 33, Make
FilterCRDFields return fields in a deterministic order by sorting the collected
includedFields before returning it. Preserve the existing hidden-field and
feature-gate filtering behavior, and add the required standard-library import
for sorting.
| // collectImports extracts unique imports needed for the generated types | ||
| func (g *Generator) collectImports(typeDefs []*TypeDef) []string { | ||
| importSet := make(map[string]bool) | ||
|
|
||
| // Add source package import if we have a source package | ||
| if g.SourcePackage != "" && g.SourcePackageAlias != "" { | ||
| importSet[fmt.Sprintf(`%s "%s"`, g.SourcePackageAlias, g.SourcePackage)] = true | ||
| } | ||
|
|
||
| for _, typeDef := range typeDefs { | ||
| for _, field := range typeDef.Fields { | ||
| // Strip pointer/slice prefixes repeatedly to handle combined | ||
| // forms like []*configv1.Something, then extract map values. | ||
| typeStr := field.Type | ||
| for strings.HasPrefix(typeStr, "*") || strings.HasPrefix(typeStr, "[]") { | ||
| typeStr = strings.TrimPrefix(typeStr, "*") | ||
| typeStr = strings.TrimPrefix(typeStr, "[]") | ||
| } | ||
| if strings.HasPrefix(typeStr, "map[") { | ||
| // Extract value type from map[K]V | ||
| if idx := strings.LastIndex(typeStr, "]"); idx != -1 && idx+1 < len(typeStr) { | ||
| typeStr = typeStr[idx+1:] | ||
| typeStr = strings.TrimPrefix(typeStr, "*") | ||
| } | ||
| } | ||
|
|
||
| // Extract package from type names like "configv1.URL" | ||
| if strings.Contains(typeStr, ".") { | ||
| // For now, we'll need to manually map these | ||
| // In a real implementation, we'd track imports from the AST | ||
| if strings.HasPrefix(typeStr, "configv1.") { | ||
| importSet[`configv1 "github.com/openshift/api/config/v1"`] = true | ||
| } | ||
| if strings.HasPrefix(typeStr, "corev1.") { | ||
| importSet[`corev1 "k8s.io/api/core/v1"`] = true | ||
| } | ||
| if strings.HasPrefix(typeStr, "metav1.") { | ||
| importSet[`metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"`] = true | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Convert set to sorted slice | ||
| var imports []string | ||
| for imp := range importSet { | ||
| imports = append(imports, imp) | ||
| } | ||
|
|
||
| return imports | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Hardcoded import mapping is fragile and can silently produce non-compiling generated code.
collectImports only recognizes configv1., corev1., and metav1. prefixes via string matching; any other qualified type referenced by a source struct (quite plausible for a large API surface like HostedClusterSpec) will be silently omitted from the generated import (...) block, producing generated Go source that fails to compile. This is acknowledged directly in the code's own comment. Neither generator_test.go nor integration_test.go actually go build/go vet the generated output, so this gap would go undetected until someone consumes the generated file.
Since the AST is already available on Generator (via g.parsedFiles), imports could be derived from the actual *ast.File.Imports declarations of the source files (matching selector qualifiers back to their real import paths) instead of a hardcoded prefix table.
Want me to implement AST-based import resolution (mapping SelectorExpr qualifiers to the source file's actual import declarations) instead of the hardcoded prefix table?
🤖 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/passthrough/generator.go` around lines 106 - 156, Update
Generator.collectImports to derive qualified-type imports from g.parsedFiles and
each source *ast.File.Imports declaration, resolving selector qualifiers to
their actual import paths instead of matching only configv1, corev1, and metav1
prefixes. Preserve the existing source-package import handling and
deduplication, and ensure every referenced external qualifier is included in the
generated imports.
| // CreatorARN is the IAM ARN of the user who created this cluster. | ||
| // Used to bootstrap the initial cluster-admin RBAC mapping. | ||
| // +k8s:openapi-gen=false | ||
| // +hyperfleet:write-mode=service-set | ||
| // +optional | ||
| // +kubebuilder:validation:Pattern=`^arn:aws:` | ||
| CreatorARN string `json:"creatorARN,omitempty"` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Anchor the CreatorARN regex pattern.
^arn:aws: only validates the prefix; anything can follow, so malformed ARNs (or unexpected trailing content) pass validation. As per path instructions, regexes should be anchored (^$).
🛡️ Proposed fix
- // +kubebuilder:validation:Pattern=`^arn:aws:`
+ // +kubebuilder:validation:Pattern=`^arn:aws:iam::\d{12}:(user|role)/[\w+=,.@-]+$`As per path instructions, **/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}: "Normalize Unicode and anchor regexes (^$); watch for ReDoS."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // CreatorARN is the IAM ARN of the user who created this cluster. | |
| // Used to bootstrap the initial cluster-admin RBAC mapping. | |
| // +k8s:openapi-gen=false | |
| // +hyperfleet:write-mode=service-set | |
| // +optional | |
| // +kubebuilder:validation:Pattern=`^arn:aws:` | |
| CreatorARN string `json:"creatorARN,omitempty"` | |
| // CreatorARN is the IAM ARN of the user who created this cluster. | |
| // Used to bootstrap the initial cluster-admin RBAC mapping. | |
| // +k8s:openapi-gen=false | |
| // +hyperfleet:write-mode=service-set | |
| // +optional | |
| // +kubebuilder:validation:Pattern=`^arn:aws:iam::\d{12}:(user|role)/[\w+=,.@-]+$` | |
| CreatorARN string `json:"creatorARN,omitempty"` |
🤖 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/api/v1alpha1/cluster_types.go` around lines 75 - 81,
Update the kubebuilder validation pattern on CreatorARN to anchor both the
beginning and end of the value, requiring the entire string—not just its
prefix—to match the intended AWS ARN pattern. Preserve the existing ARN prefix
validation while rejecting trailing or otherwise unexpected content.
Source: Path instructions
| codegen-verify: build-api-codegen | ||
| cd hyperfleet-operator/api && go build ./... | ||
| cd platform-api && go build ./internal/codegen/... | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run the configuration verifier in this target.
codegen-verify only compiles packages; it never executes verify-configuration, so missing write-mode markers pass this pipeline. Invoke it with $(V1ALPHA1_DIR)/configuration.go.
Proposed fix
codegen-verify: build-api-codegen
+ ./bin/verify-configuration $(V1ALPHA1_DIR)/configuration.go
cd hyperfleet-operator/api && go build ./...
cd platform-api && go build ./internal/codegen/...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| codegen-verify: build-api-codegen | |
| cd hyperfleet-operator/api && go build ./... | |
| cd platform-api && go build ./internal/codegen/... | |
| codegen-verify: build-api-codegen | |
| ./bin/verify-configuration $(V1ALPHA1_DIR)/configuration.go | |
| cd hyperfleet-operator/api && go build ./... | |
| cd platform-api && go build ./internal/codegen/... |
🤖 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 272 - 275, Update the codegen-verify target after the
existing build commands to invoke verify-configuration with
$(V1ALPHA1_DIR)/configuration.go, ensuring configuration markers are validated
as part of verification.
| @@ -0,0 +1,583 @@ | |||
| [ | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Two field_metadata.json registries have drifted on 6 hidden fields. spec.hostedCluster.autoNode, .channel, .configuration, .fips, .operatorConfiguration, and .pausedUntil are marked "hidden": true in the platform-api registry but not in the passthrough-gen embedded registry, even though both are meant to reflect the same source markers on HostedClusterSpec. Since passthrough-gen's copy drives +k8s:openapi-gen=false suppression for generated passthrough types, these 6 platform-managed fields will be visible in the customer-facing OpenAPI schema while the validator still treats them as hidden/service-set-only — an inconsistent contract between the two pipelines.
platform-api/internal/codegen/registry/field_metadata.json#L317-426: source of truth for the 6 fields'hidden: truestate; regenerate/sync the passthrough-gen copy from this (or a shared source) so both stay in lockstep.hack/api-codegen/cmd/passthrough-gen/field_metadata.json#L317-420: add"hidden": truetoautoNode,channel,configuration,fips,operatorConfiguration, andpausedUntilto match the platform-api registry, or better, generate both files from a single canonical registry to prevent future drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@platform-api/internal/codegen/registry/field_metadata.json` at line 1,
Synchronize the passthrough-gen field metadata registry with the platform-api
registry by marking HostedClusterSpec fields autoNode, channel, configuration,
fips, operatorConfiguration, and pausedUntil as hidden. Update the
passthrough-gen copy, or regenerate both registries from a shared canonical
source, while preserving the existing metadata for all other fields.
| { | ||
| "fieldPath": "spec.hostedCluster.autoNode", | ||
| "writeMode": "service-set", | ||
| "hidden": true | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Registry drift vs. hack/api-codegen/cmd/passthrough-gen/field_metadata.json on 6 hidden fields.
See consolidated comment below for full detail and both affected files.
Also applies to: 332-336, 342-346, 367-371, 417-421, 422-426
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@platform-api/internal/codegen/registry/field_metadata.json` around lines 317
- 321, Synchronize the six affected hidden field metadata entries between
platform-api/internal/codegen/registry/field_metadata.json and
hack/api-codegen/cmd/passthrough-gen/field_metadata.json, including the
spec.hostedCluster.autoNode entry and the entries at the referenced ranges.
Ensure each matching fieldPath has identical hidden configuration in both files.
Port the build-time code generators from cdoan1/hyperfleet-api-codegen into the monorepo under hack/api-codegen/. Includes 7 generator commands, 7 library packages, Makefile integration, and codegen integration plan. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Bump go directive to 1.26.5 in hack/api-codegen and hack/tools - Fix conversion generator: conditional imports, OutputDir-derived package names, duplicate helper prevention, go/printer fallback for unsupported AST expressions - Fix CRD variant generator: encoder.Close(), file close error propagation, hidden field exclusion, transparent items/additionalProperties - Fix marker scanner: per-directory type cache, cycle guard for recursive structs - Fix OpenAPI generator: shared resolveTypeSchema helper, pointer/slice stripping for array elements and map values - Fix passthrough generator: repeated pointer/slice prefix stripping - Fix validator: explicit matched flag for feature-gate overrides - Move codegen binaries to repo-root bin/ for consistency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Avoid mutating g.FieldPrefix so each type derives its own prefix
independently when processing multiple types
- Render interface, func, ellipsis, and generic AST expressions via
go/printer instead of silently dropping to interface{}
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add generated passthrough structs (HostedClusterSpecPassthrough, NodePoolSpecPassthrough) with per-field write-mode, visibility, and feature-gate markers to v1alpha1 CRD types. Add envelope fields (DisplayName, DeleteProtection, Tags, etc.) to ClusterSpec and NodePoolSpec. Add runtime libraries for field registry, feature gates, and service-set conversion. Add Makefile codegen pipeline targets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use --source-dir with go list from hyperfleet-operator/api/ (which has the HyperShift dependency) instead of --import-path from repo root. Remove the conflicting zz_generated.passthrough.go after generation since the curated hostedclusterspec.passthrough.go is the authoritative file. Regenerate registry from v1alpha1 markers (75 fields). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The collectImports function iterated a map without sorting, causing non-deterministic import ordering in the generated .raw file across runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ypes The stale deepcopy caused integration test failures — controller-runtime silently dropped new fields during reconciliation, preventing status propagation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Align openshift/hypershift/api and transitive deps with the rest of the monorepo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the embedded registry from passthrough-gen and make --registry a required flag. The Makefile now passes the runtime registry (platform-api/internal/codegen/registry/field_metadata.json) directly, eliminating the stale-copy problem. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
342d72a to
f6cf272
Compare
openshift-ci
Bot
removed
the
needs-rebase
isRootType excluded *Passthrough types, so the scanner never walked their fields into the registry. This meant curating markers on passthrough fields (e.g. flipping openapi-gen to true) had no effect on the generated registry. Treat Passthrough types as scan roots, bringing the registry from 75 to 119 fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
UBI go-toolset defaults GOCACHE to /opt/app-root/src/.cache/go-build which is not writable as user 1001. Create /tmp/gocache and set GOCACHE to point there. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/test on-demand-e2e |
cdoan1
added
the
do-not-merge/hold
2 participants
Description
Add codegen passthrough types to /api/v1alpha1
NOTE: this PR is on top of ROSAENG-62606-codegen-component
object directly.
but no handler or middleware calls into them.
The passthrough structs (HostedClusterSpecPassthrough, NodePoolSpecPassthrough) and the runtime libraries are scaffolding for downstream phases:
Deploying now is safe — the new envelope fields (DisplayName, Tags, etc.) are all optional/omitempty and the CRD will accept them, but no code path reads or acts on them yet.
Type of Change
Testing
make test)Checklist
Summary by CodeRabbit