HYPERFLEET-1082 - feat: partner schema validation fail-fast and Helm support#177
Open
rafabene wants to merge 1 commit into
Open
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR implements OpenAPI schema validation: Helm chart values and templates allow supplying a validation schema via inline content or an existing ConfigMap, the Deployment mounts the schema when enabled, and Helm tests cover enabled/disabled/external/malformed cases. The server loads server.openapi_schema_path at startup using validators.NewSchemaValidator and aborts startup on load errors. The compiled openapi.yaml is copied into the runtime image. Documentation and CLI/env flags were updated to reflect the new behavior. Sequence Diagram(s)sequenceDiagram
participant HelmChart
participant KubernetesAPI
participant Pod
participant Server
participant SchemaValidator
HelmChart->>KubernetesAPI: render & apply Deployment + (optional) ConfigMap
KubernetesAPI->>Pod: create Pod with validation-schema volume (if enabled)
Pod->>Server: container start -> read server.openapi_schema_path
Server->>SchemaValidator: call NewSchemaValidator(path)
SchemaValidator-->>Server: return validator or error (startup proceeds or aborts)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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: 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 `@charts/templates/deployment.yaml`:
- Around line 26-28: The partner-schema checksum currently always hashes the
rendered local template (include (print $.Template.BasePath
"/partner-schema-configmap.yaml") .), which is empty when
.Values.partnerSchema.existingConfigMap is used, so implement a conditional
checksum: if .Values.partnerSchema.existingConfigMap is set, use Helm's lookup
to fetch that external ConfigMap (lookup "v1" "ConfigMap" <namespace> <name>),
serialize its .data (or .binaryData) to a stable string (e.g., toYaml) and
sha256sum that; otherwise keep the existing
include(.../partner-schema-configmap.yaml) | sha256sum path; update the
checksum/partner-schema label logic in charts/templates/deployment.yaml
accordingly, referencing .Values.partnerSchema.existingConfigMap and the
include(...) call.
In `@Makefile`:
- Around line 425-427: The test that verifies existingConfigMap handling is
using the wrong grep pattern ('partner-schema-configmap'), making the assertion
ineffective; update the conditional that inspects $$OUTPUT so it searches for
the actual generated ConfigMap name pattern used by the template (e.g., the
runtime name pattern like 'partner-.*-schema' or 'partner-.*-schema-configmap'
depending on how resources are named) instead of 'partner-schema-configmap', and
keep the logic that fails (exit 1) if a generated ConfigMap matching that real
pattern is found; the change should be made in the Makefile conditional that
currently references 'my-partner-schema' and 'partner-schema-configmap' and uses
$$OUTPUT.
🪄 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: af32342a-ec51-4bca-940b-a733645f87e3
📒 Files selected for processing (10)
Makefilecharts/templates/configmap.yamlcharts/templates/deployment.yamlcharts/templates/partner-schema-configmap.yamlcharts/values.yamlcmd/hyperfleet-api/server/routes.godocs/config.mddocs/deployment.mdpkg/config/flags.gopkg/validators/schema_validator_test.go
rafabene
force-pushed
the
HYPERFLEET-1082-partner-schema-fail-fast
branch
from
1ecf6e7 to
712e875
Compare
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 `@cmd/hyperfleet-api/server/routes.go`:
- Around line 111-117: The startup fails because routes.go calls
validators.NewSchemaValidator using env().Config.Server.OpenAPISchemaPath which
defaults to openapi/openapi.yaml but that file is not present in the runtime
container; fix by ensuring the schema is present at runtime: either update the
Dockerfile to copy generated openapi/openapi.yaml into the final runtime image
(so validators.NewSchemaValidator can read it), or change Helm defaults to
require partnerSchema.enabled=true and ensure the ConfigMap/volume mount
provides the schema path; update deployment/Helm chart so
env().Config.Server.OpenAPISchemaPath points to an existing file or is
overridden at runtime.
In `@Makefile`:
- Around line 430-437: Run the helm template command (the existing `helm
template test-release charts/ ... --set partnerSchema.enabled=true`) but capture
stderr to a temp file instead of discarding it; then assert the command failed
and that the temp stderr contains the specific validation message referencing
`partnerSchema` (and/or `existingConfigMap`/missing content) — e.g., run `helm
template ... 2>err || true`, check exit status is non-zero, then `grep -q
"partnerSchema" err` (or the exact validation text) and fail the test if the
grep does not match; update the Makefile rule that currently discards stderr to
implement this check so only the expected partnerSchema validation error passes.
🪄 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: 0e13d340-2837-4048-800b-8fbad1bb8092
📒 Files selected for processing (10)
Makefilecharts/templates/configmap.yamlcharts/templates/deployment.yamlcharts/templates/partner-schema-configmap.yamlcharts/values.yamlcmd/hyperfleet-api/server/routes.godocs/config.mddocs/deployment.mdpkg/config/flags.gopkg/validators/schema_validator_test.go
rafabene
force-pushed
the
HYPERFLEET-1082-partner-schema-fail-fast
branch
from
712e875 to
62b0e3f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/templates/partner-schema-configmap.yaml`:
- Around line 2-3: The template currently rejects only nil/empty but allows
whitespace-only partnerSchema.content; change the guard to trim the value before
checking (e.g., use the Helm trim and default helpers) so whitespace-only
strings are treated as empty. Specifically, update the conditional that
references .Values.partnerSchema.content to check if not (trim (default ""
.Values.partnerSchema.content)) (keeping the existing fail message and the other
flags partnerSchema.enabled and partnerSchema.existingConfigMap logic intact).
🪄 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: 256da005-bff8-41a7-b560-5fb8614066cc
📒 Files selected for processing (11)
DockerfileMakefilecharts/templates/configmap.yamlcharts/templates/deployment.yamlcharts/templates/partner-schema-configmap.yamlcharts/values.yamlcmd/hyperfleet-api/server/routes.godocs/config.mddocs/deployment.mdpkg/config/flags.gopkg/validators/schema_validator_test.go
rafabene
force-pushed
the
HYPERFLEET-1082-partner-schema-fail-fast
branch
2 times, most recently
from
45c13e8 to
0478be1
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/templates/partner-schema-configmap.yaml`:
- Around line 2-3: The template's validation uses trim on
.Values.partnerSchema.content which can break if that value is explicitly null;
update the check to default partnerSchema.content to an empty string before
trimming (use Helm's default function) so the conditional: if not (trim (default
"" .Values.partnerSchema.content)) correctly triggers the fail when content is
missing while partnerSchema.enabled is true and partnerSchema.existingConfigMap
is not set; keep the existing fail message and only change the expression that
references partnerSchema.content.
In `@charts/values.yaml`:
- Around line 363-401: Add a Helm NOTES.txt template
(charts/templates/NOTES.txt) that explains the new partnerSchema feature and
shows users how to enable it: document the two options (partnerSchema.content
for inline OpenAPI YAML and partnerSchema.existingConfigMap to point to a
pre-created ConfigMap containing openapi.yaml), include example commands for
enabling each option via helm install/upgrade (--set partnerSchema.enabled=true
and either --set partnerSchema.content="|" or --set
partnerSchema.existingConfigMap=NAME), and surface a brief warning that an
invalid schema will prevent the API from starting; reference the partnerSchema
keys (enabled, content, existingConfigMap) so the output maps directly to
values.yaml.
In `@Makefile`:
- Around line 400-402: The test's broad grep for 'partner-schema' can produce
false positives; update the first assertion to search for a ConfigMap-specific
marker (e.g., a component/label string) instead of the generic token. Replace
the grep that matches 'partner-schema' with a check for a unique ConfigMap
identifier such as 'component: partner-schema' or the exact label key/value your
generator uses (for example 'app.kubernetes.io/component=partner-schema'),
leaving the other two checks for 'HYPERFLEET_SERVER_OPENAPI_SCHEMA_PATH' and
'/etc/hyperfleet/partner-schema' unchanged.
🪄 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: f5035c87-b58c-4352-a84b-fe93743be45a
📒 Files selected for processing (11)
DockerfileMakefilecharts/templates/configmap.yamlcharts/templates/deployment.yamlcharts/templates/partner-schema-configmap.yamlcharts/values.yamlcmd/hyperfleet-api/server/routes.godocs/config.mddocs/deployment.mdpkg/config/flags.gopkg/validators/schema_validator_test.go
rafabene
force-pushed
the
HYPERFLEET-1082-partner-schema-fail-fast
branch
from
0478be1 to
1f6fd98
Compare
rh-amarin
reviewed
May 20, 2026
rafabene
force-pushed
the
HYPERFLEET-1082-partner-schema-fail-fast
branch
2 times, most recently
from
f54ecd2 to
d3c009c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openapi/README.md (1)
85-86:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale startup behavior text (still says non-blocking).
Line 85 and Line 103 still document warning-only/non-blocking startup when schema loading fails. That contradicts the current fail-fast behavior and should be updated to “startup fails” semantics.
As per coding guidelines,
docs/deployment.mdstates the API fails to start if the configured schema path is missing, unreadable, or invalid.Also applies to: 103-103
🤖 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 `@openapi/README.md` around lines 85 - 86, Update the README text that currently states "startup remains non-blocking" to reflect the current fail-fast behavior: change any mentions that the API will only log a warning and continue to instead state that startup fails when ClusterSpec or NodePoolSpec is absent, unreadable, or invalid; ensure the wording aligns with docs/deployment.md (which says the API fails to start if the configured schema path is missing/unreadable/invalid) and update both occurrences referencing ClusterSpec and NodePoolSpec so the documentation consistently describes "startup fails" semantics.
🤖 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/deployment.md`:
- Line 179: The docs line incorrectly claims Helm sets the
HYPERFLEET_SERVER_OPENAPI_SCHEMA_PATH environment variable; instead update the
sentence to state that when validationSchema.enabled is true the chart writes
the schema path into the generated config.yaml as server.openapi_schema_path
(using the ternary in charts/templates/configmap.yaml) and does not inject the
HYPERFLEET_SERVER_OPENAPI_SCHEMA_PATH env var; reference
validationSchema.enabled, server.openapi_schema_path and
charts/templates/configmap.yaml in the updated text.
---
Outside diff comments:
In `@openapi/README.md`:
- Around line 85-86: Update the README text that currently states "startup
remains non-blocking" to reflect the current fail-fast behavior: change any
mentions that the API will only log a warning and continue to instead state that
startup fails when ClusterSpec or NodePoolSpec is absent, unreadable, or
invalid; ensure the wording aligns with docs/deployment.md (which says the API
fails to start if the configured schema path is missing/unreadable/invalid) and
update both occurrences referencing ClusterSpec and NodePoolSpec so the
documentation consistently describes "startup fails" semantics.
🪄 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: 2cd6375d-cdad-4036-9e14-3c1de30b688c
📒 Files selected for processing (14)
DockerfileMakefileREADME.mdcharts/templates/NOTES.txtcharts/templates/configmap.yamlcharts/templates/deployment.yamlcharts/templates/validation-schema-configmap.yamlcharts/values.yamlcmd/hyperfleet-api/server/routes.godocs/config.mddocs/deployment.mdopenapi/README.mdpkg/config/flags.gopkg/validators/schema_validator_test.go
…support API now fails to start if the configured OpenAPI schema file is missing or invalid, catching misconfigurations before invalid data enters the database. Added Helm chart support for partner schemas via inline content or existing ConfigMap reference.
rafabene
force-pushed
the
HYPERFLEET-1082-partner-schema-fail-fast
branch
from
d3c009c to
baa213e
Compare
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)
openapi/README.md (1)
85-104:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInconsistent documentation: startup behavior contradicts PR changes.
Lines 85-86 and 99-104 still describe non-blocking startup with warning logs when the schema file is missing or malformed. However, the PR objectives, other documentation files (docs/config.md line 251, docs/deployment.md line 148), and the code changes clearly state that the API now fails to start when the schema is missing or invalid.
Update these sections to reflect the new fail-fast behavior.
📝 Suggested fix for lines 85-86 and 99-104
Replace lines 85-86:
-If `ClusterSpec` or `NodePoolSpec` is absent from the file, the API will fail to load the validator and log a warning (startup remains non-blocking). +If `ClusterSpec` or `NodePoolSpec` is absent from the file, the API will abort startup with an error.Replace lines 99-104:
### Runtime behaviour - Validation runs in HTTP middleware on every `POST` and `PATCH` request, before the service or database layer. - Invalid specs return `400 Bad Request` with field-level error details. -- Startup is **non-blocking**: if the schema file is missing or malformed, the API logs a warning and starts without validation — specs are accepted without field-level checks. +- Startup is **fail-fast**: if the schema file is missing or malformed, the API exits immediately to prevent misconfigured deployments.🤖 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 `@openapi/README.md` around lines 85 - 104, Update the README openapi section to reflect the new fail-fast startup behavior: replace the statement that the API "will fail to load the validator and log a warning (startup remains non-blocking)" and the "Startup is **non-blocking**" paragraph with text that the service will fail to start (exit with error) if the OpenAPI schema (including ClusterSpec/NodePoolSpec) is missing or malformed, and that on startup the server validates the schema and refuses to run until a valid schema path is provided; keep the configuration examples and runtime middleware behavior but change wording to "startup fails" / "service will not start" where those earlier non-blocking assertions appear (refer to the lines mentioning ClusterSpec/NodePoolSpec absence and the "Startup is **non-blocking**" bullet).
🤖 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 `@openapi/README.md`:
- Around line 85-104: Update the README openapi section to reflect the new
fail-fast startup behavior: replace the statement that the API "will fail to
load the validator and log a warning (startup remains non-blocking)" and the
"Startup is **non-blocking**" paragraph with text that the service will fail to
start (exit with error) if the OpenAPI schema (including
ClusterSpec/NodePoolSpec) is missing or malformed, and that on startup the
server validates the schema and refuses to run until a valid schema path is
provided; keep the configuration examples and runtime middleware behavior but
change wording to "startup fails" / "service will not start" where those earlier
non-blocking assertions appear (refer to the lines mentioning
ClusterSpec/NodePoolSpec absence and the "Startup is **non-blocking**" bullet).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b7e8377a-f5af-43a9-8c9e-913f4a4aa4e5
📒 Files selected for processing (14)
DockerfileMakefileREADME.mdcharts/templates/NOTES.txtcharts/templates/configmap.yamlcharts/templates/deployment.yamlcharts/templates/validation-schema-configmap.yamlcharts/values.yamlcmd/hyperfleet-api/server/routes.godocs/config.mddocs/deployment.mdopenapi/README.mdpkg/config/flags.gopkg/validators/schema_validator_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
partnerSchemasection supporting both inline content and existing ConfigMap reference for partner-specific OpenAPI schemasdocs/deployment.mdanddocs/config.mdwith fail-fast behavior and partner schema Helm configurationDetails
Fail-fast schema validation
registerAPIMiddleware()now returns an error whenNewSchemaValidator()fails, propagated viacheck()which callsos.Exit(1)— preventing the app from reachingSetReady(). This ensures misconfigured deployments are caught at startup rather than silently accepting invalid data.Helm partner schema support
Partners can supply a custom schema in two ways:
Inline content:
Existing ConfigMap:
When enabled, the chart creates/references a ConfigMap, mounts it at
/etc/hyperfleet/partner-schema/openapi.yaml, and setsHYPERFLEET_SERVER_OPENAPI_SCHEMA_PATH.Test plan
make test— 920 unit tests passingmake test-integration— 102 integration tests passingmake lint— 0 issuesmake test-helm— all scenarios passing, including: