HYPERFLEET-1033 - feat: consume OpenAPI schemas from hyperfleet-api-spec Go module#155
Merged
openshift-merge-bot[bot] merged 2 commits intoMay 15, 2026
Conversation
mliptak0
changed the title
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe pull request restructures OpenAPI schema management by introducing Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/api-operator-guide.md (1)
709-733:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve contradictory schema-validation behavior in this section.
Line 709 states validation is always active with an embedded schema, but Lines 731-733 still describe a fallback mode (“start without validation” / non-blocking startup). These statements conflict and can mislead operators during debugging. Please update the behavior bullets to reflect the embedded, always-on model consistently.
🤖 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 `@docs/api-operator-guide.md` around lines 709 - 733, Update the "Behavior" bullets to consistently reflect that the OpenAPI schema is embedded in the binary (from github.com/openshift-hyperfleet/hyperfleet-api-spec) and that schema validation is always active; replace the current dual-mode wording with a single clear statement: specs are validated against the embedded schema and invalid specs return 400 Bad Request, and document that schema load/validation happens at startup and will surface a startup error if the embedded schema is unreadable or invalid (no fallback to “start without validation”). Reference the existing “validation middleware” / “schema validator” text and remove or rework the fallback / non-blocking startup lines so the section is no longer contradictory.
🤖 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 `@CHANGELOG.md`:
- Around line 32-33: Update the two changelog bullets that currently link to PR
`#129` so they correctly reference PR `#155` (and also fix the similar reference at
the other occurrence noted), i.e., replace the
`[`#129`](https://github.com/openshift-hyperfleet/hyperfleet-api/pull/129)` links
in CHANGELOG.md with
`[`#155`](https://github.com/openshift-hyperfleet/hyperfleet-api/pull/155)` so the
entries in the newly added bullets (the OpenAPI embedding and VARIANT/ldflags
notes) point to the actual PR that introduced them.
In `@cmd/hyperfleet-api/server/routes.go`:
- Around line 22-23: Validate the build-injected schemaVariant at startup and
fail fast with a clear error: add a one-time check (e.g., in an init() or server
startup function in routes.go) that compares schemaVariant against the allowed
variants list (e.g., "core", "enterprise", etc.), and if it is not in that set
call log.Fatalf or return an explicit error indicating "invalid schemaVariant:
<value> — allowed values are: [...]"; reference the existing schemaVariant
variable to locate where to insert the check so the server never proceeds to the
"failed to read embedded OpenAPI schema" code path with an invalid variant.
In `@docs/api-operator-guide.md`:
- Around line 709-710: The docs note that the OpenAPI schema is now embedded
from the github.com/openshift-hyperfleet/hyperfleet-api-spec module, but related
architecture docs in the openshift-hyperfleet/architecture repo (e.g.,
generated-code policy, bill of artifacts, ADRs that reference
openapi/openapi.yaml) were not updated; open a follow-up PR in
openshift-hyperfleet/architecture to update those artifacts and textual
references to point to the embedded spec and the hyperfleet-api-spec module,
update any generated-code or bill-of-artifacts policies and ADRs that assert
openapi/openapi.yaml as authoritative, and add a reference in
docs/api-operator-guide.md to the corresponding PR/issue in the architecture
repo for traceability.
In `@openapi/CLAUDE.md`:
- Line 21: Add language identifiers to the fenced code blocks in
openapi/CLAUDE.md: update the block containing "make generate # Extracts
schema from spec module, then runs oapi-codegen" to use ```shell and the block
containing "replace github.com/openshift-hyperfleet/hyperfleet-api-spec =>
/path/to/hyperfleet-api-spec" to use ```go so tooling and rendering correctly
detect the languages.
In `@README.md`:
- Line 11: Update the README section that currently claims "openapi/" is the API
spec source to reflect the module-based workflow: replace references to the
local openapi/ directory with the canonical module source
"github.com/openshift-hyperfleet/hyperfleet-api-spec" and state that local files
in openapi/ are generated artifacts produced by oapi-codegen; ensure the README
mentions the generator change to "oapi-codegen" already present and clarifies
that schemas are sourced from the hyperfleet-api-spec module rather than
maintained locally.
---
Outside diff comments:
In `@docs/api-operator-guide.md`:
- Around line 709-733: Update the "Behavior" bullets to consistently reflect
that the OpenAPI schema is embedded in the binary (from
github.com/openshift-hyperfleet/hyperfleet-api-spec) and that schema validation
is always active; replace the current dual-mode wording with a single clear
statement: specs are validated against the embedded schema and invalid specs
return 400 Bad Request, and document that schema load/validation happens at
startup and will surface a startup error if the embedded schema is unreadable or
invalid (no fallback to “start without validation”). Reference the existing
“validation middleware” / “schema validator” text and remove or rework the
fallback / non-blocking startup lines so the section is no longer contradictory.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: cff11550-c0d1-463b-926a-fc54a682880d
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
.gitignoreAGENTS.mdCHANGELOG.mdCLAUDE.mdDockerfileMakefileREADME.mdcmd/hyperfleet-api/server/routes.godocs/api-operator-guide.mddocs/deployment.mdgo.modopenapi/CLAUDE.mdopenapi/openapi.yamlpkg/config/flags.gopkg/config/loader.gopkg/config/server.gopkg/logger/fields.gopkg/middleware/schema_validation_test.gopkg/validators/schema_validator.gopkg/validators/schema_validator_test.gotest/CLAUDE.mdtest/integration/clusters_test.gotest/integration/integration_test.go
💤 Files with no reviewable changes (7)
- pkg/config/flags.go
- pkg/logger/fields.go
- pkg/config/loader.go
- openapi/openapi.yaml
- Dockerfile
- test/integration/clusters_test.go
- test/integration/integration_test.go
mliptak0
added a commit
to mliptak0/hyperfleet-api
that referenced
this pull request
May 14, 2026
…hift-hyperfleet#155 - Fix CHANGELOG.md PR links: replace openshift-hyperfleet#129 with openshift-hyperfleet#155 on all three bullets - Add init() to routes.go to validate schemaVariant at startup with a clear panic message before reaching the embedded FS read - Fix contradictory Behavior bullets in docs/api-operator-guide.md to reflect the always-on embedded schema model (no fallback mode) - Add shell/go language identifiers to bare fenced code blocks in openapi/CLAUDE.md - Update README.md openapi/ project structure comment to reflect generated artifacts, not source Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
mliptak0
added a commit
to mliptak0/hyperfleet-api
that referenced
this pull request
May 14, 2026
…deduplicate schema docs - Add openapi/README.md as the single detailed source of truth for schema import mechanics, code generation pipeline, validation behaviour, and oapi-codegen configuration - Trim openapi/CLAUDE.md to a short rule-set with a pointer to README.md - Update CLAUDE.md and AGENTS.md references from openapi/CLAUDE.md to openapi/README.md; collapse the two redundant boundary bullets about the spec source into one - Condense docs/api-operator-guide.md §3.6 Schema Validation to a high-level summary and link to openapi/README.md for implementation details — resolves CodeRabbit cross-repo doc-sync comment on PR openshift-hyperfleet#155 Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/api-operator-guide.md`:
- Around line 709-714: Update the earlier sentence that currently reads “when a
schema is configured” to state that schema validation is always active and the
OpenAPI schema is embedded at build time from the
github.com/openshift-hyperfleet/hyperfleet-api-spec module (no external config
required), so the guidance is consistent with the later paragraph describing
always-on validation and exit-on-malformed-schema; ensure the wording aligns
with the “Resource Model” phrasing and replaces any conditional phrasing around
schema configuration in this document.
In `@openapi/README.md`:
- Around line 47-53: The markdown ordered list has fenced code blocks missing
required blank lines in list context (MD031); update the README.md so each
fenced block is separated by a blank line from the surrounding list text — add a
blank line before and after the ```shell block containing "go get
github.com/openshift-hyperfleet/hyperfleet-api-spec@vX.Y.Z" and likewise add a
blank line before the ```shell block containing "make generate-all" so both code
fences comply with markdownlint MD031.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 967ff389-9563-4424-8f48-48ecd7653e55
📒 Files selected for processing (8)
AGENTS.mdCHANGELOG.mdCLAUDE.mdREADME.mdcmd/hyperfleet-api/server/routes.godocs/api-operator-guide.mdopenapi/CLAUDE.mdopenapi/README.md
mliptak0
added a commit
to mliptak0/hyperfleet-api
that referenced
this pull request
May 14, 2026
…deduplicate docs - Add openapi/README.md as the single detailed source of truth for schema import mechanics, code generation pipeline, validation behaviour, and oapi-codegen configuration - Trim openapi/CLAUDE.md to a short rule-set with a pointer to README.md - Update CLAUDE.md and AGENTS.md references from openapi/CLAUDE.md to openapi/README.md; collapse the two redundant boundary bullets about the spec source into one - Condense docs/api-operator-guide.md §3.6 Schema Validation to a high-level summary and link to openapi/README.md for implementation details — resolves CodeRabbit cross-repo doc-sync comment on PR openshift-hyperfleet#155 Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
mliptak0
added a commit
to mliptak0/hyperfleet-api
that referenced
this pull request
May 14, 2026
…deduplicate docs - Add openapi/README.md as the single detailed source of truth for schema import mechanics, code generation pipeline, validation behaviour, and oapi-codegen configuration - Trim openapi/CLAUDE.md to a short rule-set with a pointer to README.md - Update CLAUDE.md and AGENTS.md references from openapi/CLAUDE.md to openapi/README.md; collapse the two redundant boundary bullets about the spec source into one - Condense docs/api-operator-guide.md §3.6 Schema Validation to a high-level summary and link to openapi/README.md for implementation details — resolves CodeRabbit cross-repo doc-sync comment on PR openshift-hyperfleet#155 Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
rh-amarin
reviewed
May 14, 2026
| ) | ||
|
|
||
| // schemaVariant is injected at build time via -ldflags from the Makefile VARIANT variable. | ||
| var schemaVariant = "core" |
Contributor
There was a problem hiding this comment.
We only care about "core" in the API
Could we simplify?
rh-amarin
reviewed
May 14, 2026
| logger.With(ctx, logger.FieldSchemaPath, schemaPath).Info("Schema validation enabled") | ||
| router.Use(middleware.SchemaValidationMiddleware(schemaValidator)) | ||
| } | ||
| schemaData, err := specschemas.FS.ReadFile(schemaVariant + "/openapi.yaml") |
Contributor
There was a problem hiding this comment.
In this case, the validation is not for the "core" schema, but for the partner openapi.yaml schema
How this works:
- Our hyperfleet-api API only cares about clusters/nodepools carrying a
specfield - But what's inside that field is irrelevant for hyperfleet
- But for partners, it is important to validate it
So, how can we allow partners to validate the payload with the full schema of their contract?
- The specify this second openapi.yaml that we use to extract the payloads of the requests and validate against them
- This means using the gcp/openapi.yaml for GCP
- BUT, since we don't know if the GCP team will use our hyperfleet-api-spec repository...
- Don't assume the mechanism for injecting the xxx/openapi.yaml is the same as for or core/openapi.yaml
Long story short... IMO we don't need to touch this function
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/validators/schema_validator_test.go (1)
54-76: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd coverage for
NewSchemaValidator(schemaPath)path behaviorThe refactor removed path-based constructor tests, but
NewSchemaValidator(schemaPath)is still a public entry point and now contains file-read error handling. Please add at least one success case and one missing-file case to prevent regressions in startup-path handling.As per coding guidelines, "
**/*_test.go: Review tests for ... Test names describe the scenario ... Table-driven tests used for multiple cases ...".🤖 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 `@pkg/validators/schema_validator_test.go` around lines 54 - 76, Add tests exercising the public file-path constructor NewSchemaValidator: one success case that writes testSchema to a temp file, calls NewSchemaValidator(tempPath), asserts err==nil, validator and validator.schemas["cluster"/"nodepool"] and their Schema/TypeName are non-nil/expected; and one failure case that calls NewSchemaValidator on a non-existent path and asserts a non-nil error whose message contains the file-load failure substring (e.g., "failed to load OpenAPI schema from file"); implement as two table-driven subtests or two separate tests named like TestNewSchemaValidator_Success and TestNewSchemaValidator_FileNotFound and reuse existing expectations from TestNewSchemaValidatorFromData for schema assertions.
🤖 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.
Outside diff comments:
In `@pkg/validators/schema_validator_test.go`:
- Around line 54-76: Add tests exercising the public file-path constructor
NewSchemaValidator: one success case that writes testSchema to a temp file,
calls NewSchemaValidator(tempPath), asserts err==nil, validator and
validator.schemas["cluster"/"nodepool"] and their Schema/TypeName are
non-nil/expected; and one failure case that calls NewSchemaValidator on a
non-existent path and asserts a non-nil error whose message contains the
file-load failure substring (e.g., "failed to load OpenAPI schema from file");
implement as two table-driven subtests or two separate tests named like
TestNewSchemaValidator_Success and TestNewSchemaValidator_FileNotFound and reuse
existing expectations from TestNewSchemaValidatorFromData for schema assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 782ff394-e237-47ef-ae89-dc0e43788493
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
.gitignoreAGENTS.mdCHANGELOG.mdCLAUDE.mdDockerfileMakefileREADME.mddocs/api-operator-guide.mddocs/deployment.mdgo.modopenapi/CLAUDE.mdopenapi/README.mdopenapi/openapi.yamlpkg/logger/fields.gopkg/middleware/schema_validation_test.gopkg/validators/schema_validator.gopkg/validators/schema_validator_test.gotest/CLAUDE.md
💤 Files with no reviewable changes (2)
- Dockerfile
- openapi/openapi.yaml
rafabene
reviewed
May 14, 2026
kuudori
reviewed
May 14, 2026
| mkdir -p pkg/api/openapi | ||
| mkdir -p pkg/api/openapi openapi | ||
| @rm -f openapi/openapi.yaml | ||
| @cp "$$($(GO) list -m -f '{{.Dir}}' github.com/openshift-hyperfleet/hyperfleet-api-spec)/schemas/core/openapi.yaml" openapi/openapi.yaml |
Contributor
There was a problem hiding this comment.
Noticed you added embed.FS in schemas/schemas.go in the spec repo, but here we're copying via go list -m + cp instead of reading through it. The ticket also mentions hack/extract-schema.go reading from specschemas. Did something change?
Contributor
Author
There was a problem hiding this comment.
Will update the Jira ticket, good point
…pec Go module Replace the committed openapi/openapi.yaml with a Go module dependency on github.com/openshift-hyperfleet/hyperfleet-api-spec. Code generation now extracts the core schema from the module cache via make generate; the file is no longer tracked in git. Runtime partner schema validation is preserved unchanged: operators supply --server-openapi-schema-path (or HYPERFLEET_SERVER_OPENAPI_SCHEMA_PATH) to point at a provider-specific openapi.yaml; validation is optional and non-fatal at startup. - go.mod: add hyperfleet-api-spec module dependency - Makefile: generate target extracts core/openapi.yaml from module cache - .gitignore: exclude openapi/openapi.yaml - routes.go: read schema from operator-supplied file path; warn and skip if missing/invalid (non-fatal) - config: add OpenAPISchemaPath field, --server-openapi-schema-path flag, HYPERFLEET_SERVER_OPENAPI_SCHEMA_PATH env binding (default: openapi/openapi.yaml) - logger: add FieldSchemaPath constant - validators: add NewSchemaValidatorFromData alongside NewSchemaValidator(path) - docs: add openapi/README.md as source of truth; deduplicate operator guide and architecture docs; update ADR, bill-of-artifacts, generated-code policy - tests: restore TestClusterSchemaValidationWithProviderSchema and integration_test.go schema path auto-setup Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rh-amarin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit 40cdfc1
into
openshift-hyperfleet:main
10 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test Plan
make test-allpassesmake lintpassesmake test-helm(if applicable)