HYPERFLEET-1083 feat: add generic Resource type and Channel/Version spec schemas#51
Open
kuudori wants to merge 1 commit into
Open
HYPERFLEET-1083 feat: add generic Resource type and Channel/Version spec schemas#51kuudori wants to merge 1 commit into
kuudori wants to merge 1 commit into
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 CodeRabbitRelease Notes
WalkthroughAdds a polymorphic Resource TypeSpec model with create/patch/list shapes and examples; introduces GCP Channel and Version model families and example fixtures; adds TypeSpec service interfaces for Resources, Channels, Versions, and internal adapter-status upserts; imports the resources service into main.tsp and bumps the API version; and updates core and GCP OpenAPI/Swagger specs with new paths/schemas for Resources, Channels, Versions, ConditionRequest, and ForceDeleteRequest. CHANGELOG updated for v1.0.16. Sequence Diagram(s)sequenceDiagram
participant Client
participant HyperFleetAPI
participant ResourcesService
participant Database
participant AdapterService
Client->>HyperFleetAPI: HTTP GET/POST /api/hyperfleet/v1/resources
HyperFleetAPI->>ResourcesService: invoke getResources/postResource
ResourcesService->>Database: list/create/patch/delete Resource
Client->>HyperFleetAPI: HTTP GET /api/hyperfleet/v1/resources/{id}/statuses
HyperFleetAPI->>ResourcesService: invoke getResourceStatuses(resource_id)
ResourcesService->>AdapterService: fetch AdapterStatusList for resource_id
AdapterService-->>ResourcesService: AdapterStatusList
ResourcesService-->>HyperFleetAPI: return Resource / ResourceList / AdapterStatusList
HyperFleetAPI-->>Client: HTTP response (200/201/202/204/etc)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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: 5
🤖 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 `@main.tsp`:
- Around line 24-25: Restore the CI-parsable `@info` decorator by reverting the
current multi-line object to the single-line `@info.version` format expected by
the extractor: replace the current `@info`(# { version: "1.1.0", ... }) style with
the exact decorator syntax the CI expects (e.g., `@info`(version: "1.1.0") or the
single-line `@info.version` declaration used elsewhere), ensuring the symbol `@info`
and the version value "1.1.0" are emitted in the expected parser-friendly form
so the extractor can read the version again.
In `@models/resource/example_resource.tsp`:
- Around line 5-14: The example uses an adapter-managed kind ("Cluster") while
showing an empty status.conditions array; change the resource kind to a
non-adapter type (e.g., "Channel" or "Version") so the example matches the
contract that ResourceStatus.conditions is empty for non-adapter
resources—update the kind field (and any related name/href/id if you keep
conventions) and leave status.conditions as #[] to reflect the correct example
for non-adapter resources.
In `@models/resource/model.tsp`:
- Around line 21-38: The Resource model currently uses a non-discriminated shape
(model Resource with fields kind: string and spec: Record<unknown>), which
collapses all variants and prevents codegen from exposing kind-specific shapes;
update model Resource to be a discriminated contract keyed on the kind field
(i.e., make Resource a polymorphic/discriminated union where each variant is a
distinct subtype identified by kind and carries a strongly-typed spec that will
be validated at runtime against its schema), adjust the Resource definition and
any related factories/parsers that reference Resource.spec to validate/serialize
per-variant specs, and apply the same discriminated-union pattern to the related
models in the other block mentioned (the section covering lines 58-85) so
OpenAPI/codegen can emit concrete Resource/Channel/Version shapes.
In `@schemas/gcp/openapi.yaml`:
- Around line 804-815: The getResourceStatuses operation (operationId
getResourceStatuses) currently only accepts path param resource_id but should
accept the standard list query parameters used by other status-list endpoints;
update the OpenAPI fragment for this path to add query parameters search, page,
pageSize, orderBy and order (with appropriate types: search string, page and
pageSize integer, order enum/string, orderBy string) and ensure the response
schema remains AdapterStatusList so the endpoint supports
pagination/sorting/filtering like other resource status endpoints.
In `@services/resources.tsp`:
- Around line 96-105: getResourceStatuses is missing the shared list query
parameters so the generated /resources/{resource_id}/statuses contract lacks
pagination and filtering; update the getResourceStatuses operation signature to
accept the standard pagination/search query params used by other list endpoints
(the shared list params type or explicit query params such as page,
per_page/limit, q/search, sort) so the contract supports filtering and
paging—modify the getResourceStatuses method declaration to add those query
parameters (keeping resource_id path param and returning AdapterStatusList |
Error | NotFoundResponse | BadRequestResponse).
🪄 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: 610e79c6-450b-4d6a-8d8e-cac1140d88cb
📒 Files selected for processing (14)
CHANGELOG.mdaliases-gcp.tspmain.tspmodels-gcp/channel/model.tspmodels-gcp/version/model.tspmodels/resource/example_patch.tspmodels/resource/example_post.tspmodels/resource/example_resource.tspmodels/resource/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlservices/resources.tsp
rh-amarin
reviewed
May 20, 2026
| namespace HyperFleet; | ||
|
|
||
| @tag("Resources") | ||
| @route("/resources") |
Collaborator
There was a problem hiding this comment.
I think the /force-delete endpoint missing
Collaborator
There was a problem hiding this comment.
and the PUT for /statuses will be required also
rh-amarin
reviewed
May 20, 2026
| import "../common/model.tsp"; | ||
|
|
||
| const exampleResource: Resource = #{ | ||
| kind: "Cluster", |
Collaborator
There was a problem hiding this comment.
I have a doubt if we should show the example with Cluster or something more generic.
Maybe not Resource as it can be confusing, but maybe MyResource ?
rh-amarin
reviewed
May 20, 2026
Collaborator
There was a problem hiding this comment.
In this file we are missing :
./mosels-gcp/services/channels.tsp./mosels-gcp/services/versions.tsp
which should define the operations
rh-amarin
reviewed
May 20, 2026
| @service(#{ title: "HyperFleet API" }) | ||
| @info(#{ | ||
| version: "1.0.15", | ||
| version: "1.1.0", |
Collaborator
There was a problem hiding this comment.
Why 1.1.0 and not 1.0.16 ?
We are not keeping yes semantic versioning, so I would go with the easiest for now
Contributor
Author
There was a problem hiding this comment.
Alright, just thought that's a big change 😮
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemas/gcp/openapi.yaml (1)
1959-2204:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe GCP
Resourcecontract is still too opaque for the new kinds this PR adds.
specstays as an unconstrained object, and the example on Lines 2039-2049 showskind: Clusterwithspec: {}, which contradicts this file’s own required GCPClusterSpec. This file also never publishesChannelSpec/VersionSpec, so clients cannot discover or generate the new GCP resource shapes this PR is supposed to add.🤖 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 `@schemas/gcp/openapi.yaml` around lines 1959 - 2204, The Resource schema is too generic because spec is typed as an unconstrained object and the example incorrectly shows an empty spec for kind: Cluster; update the Resource and ResourceCreateRequest schemas so spec references the concrete spec schemas (e.g., use $ref or oneOf with a discriminator mapping to ClusterSpec, ChannelSpec, VersionSpec) instead of additionalProperties: {} and add/publish the missing ChannelSpec and VersionSpec component schemas; also fix the Resource example (and ResourceCreateRequest example) to include a valid ClusterSpec payload per ClusterSpec, and ensure ResourcePatchRequest and any examples reflect the concrete spec shapes so clients can discover and generate the new GCP resource types.
♻️ Duplicate comments (2)
models/resource/model.tsp (1)
27-39:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUse a discriminated
kindcontract with typedspecvariants.
kind: string+spec: Record<unknown>keeps the API surface opaque, soChannelSpec/VersionSpec(and other kind-specific schemas) are not enforced in request/response contracts. This is a major maintainability and integration risk for downstream consumers expecting stable, typed resource shapes.As per coding guidelines,
Validate changes against HyperFleet architecture standards from the linked architecture repository.Also applies to: 61-69, 82-82
🤖 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 `@models/resource/model.tsp` around lines 27 - 39, Replace the open-ended fields in the Resource model by turning "kind: string" and "spec: Record<unknown>" into a discriminated union that maps each concrete kind to its typed spec (e.g., use a union type such as Resource = ChannelResource | VersionResource ... where each variant has kind: "Channel" | "Version" and spec: ChannelSpec | VersionSpec respectively); update the model symbols "kind" and "spec" and the validation decorators (`@minLength/`@maxLength) to apply on the discriminant and ensure runtime/schema validation uses the corresponding spec schema for each variant (refer to ChannelSpec and VersionSpec types when creating the union) so requests/responses are strongly typed and validated against the HyperFleet architecture standards.models/resource/example_resource.tsp (1)
5-14:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign example kind with empty
status.conditions.This example uses
kind: "Cluster"while showingconditions: #[]; that conflicts with the documented contract where adapter-managed kinds (like Cluster) have populated conditions. Use a non-adapter kind (e.g.,Channel/Version) for this empty-conditions example.As per coding guidelines,
Validate changes against HyperFleet architecture standards from the linked architecture repository.🤖 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 `@models/resource/example_resource.tsp` around lines 5 - 14, The example resource uses an adapter-managed kind "Cluster" while showing an empty status.conditions array; change the kind value (e.g., set kind to "Channel" or "Version") so the example matches a non-adapter kind that may legitimately have status.conditions: #[]; update the kind field in the example and re-run the HyperFleet architecture validation as per guidelines to ensure the example complies with architecture standards.
🤖 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 `@schemas/core/openapi.yaml`:
- Around line 924-955: The getResourceStatuses operation (operationId:
getResourceStatuses) for path
/api/hyperfleet/v1/resources/{resource_id}/statuses is missing the standard
collection query parameters; add query parameters named search, page, pageSize,
orderBy and order (each with in: query and appropriate schemas: search:string,
page:integer, pageSize:integer, orderBy:string, order:string enum
['asc','desc']) so generated clients can filter, paginate and sort the
AdapterStatusList response; ensure these parameters are declared alongside the
existing path param resource_id and reflected in the operation's parameters
array.
In `@schemas/core/swagger.yaml`:
- Around line 1147-1176: The getResourceStatuses operation at
'/api/hyperfleet/v1/resources/{resource_id}/statuses' (operationId:
getResourceStatuses) returns an AdapterStatusList but only declares the path
param resource_id; add the same query parameters used by the other status-list
endpoints (pagination, filtering and sorting) so generated clients can supply
paging/filter controls. Update the operation to include query parameters (e.g.
page, page_size/limit, sort, and any status/filter fields used elsewhere) with
matching types/descriptions and required flags, mirroring the parameter
definitions used by the other status endpoints and ensuring the response schema
still references AdapterStatusList.
In `@schemas/gcp/swagger.yaml`:
- Around line 2382-2383: ResourceList.total is declared with format: int64 while
other list models (ClusterList.total, NodePoolList.total,
AdapterStatusList.total) use format: int32; update the ResourceList.total
declaration to use format: int32 so the pagination type is consistent across
models and prevents client generation/type mismatches.
- Around line 1008-1037: The GET operation getResourceStatuses for path
'/api/hyperfleet/v1/resources/{resource_id}/statuses' is missing the standard
pagination/query parameters; add the same parameters used by the other /statuses
endpoints (search, page, pageSize, orderBy, order) to this operation so the
response can be paginated uniformly (match types and descriptions used by
nodepool/cluster statuses and ensure they appear in the parameters array for
getResourceStatuses while leaving the existing path parameter resource_id and
responses untouched).
---
Outside diff comments:
In `@schemas/gcp/openapi.yaml`:
- Around line 1959-2204: The Resource schema is too generic because spec is
typed as an unconstrained object and the example incorrectly shows an empty spec
for kind: Cluster; update the Resource and ResourceCreateRequest schemas so spec
references the concrete spec schemas (e.g., use $ref or oneOf with a
discriminator mapping to ClusterSpec, ChannelSpec, VersionSpec) instead of
additionalProperties: {} and add/publish the missing ChannelSpec and VersionSpec
component schemas; also fix the Resource example (and ResourceCreateRequest
example) to include a valid ClusterSpec payload per ClusterSpec, and ensure
ResourcePatchRequest and any examples reflect the concrete spec shapes so
clients can discover and generate the new GCP resource types.
---
Duplicate comments:
In `@models/resource/example_resource.tsp`:
- Around line 5-14: The example resource uses an adapter-managed kind "Cluster"
while showing an empty status.conditions array; change the kind value (e.g., set
kind to "Channel" or "Version") so the example matches a non-adapter kind that
may legitimately have status.conditions: #[]; update the kind field in the
example and re-run the HyperFleet architecture validation as per guidelines to
ensure the example complies with architecture standards.
In `@models/resource/model.tsp`:
- Around line 27-39: Replace the open-ended fields in the Resource model by
turning "kind: string" and "spec: Record<unknown>" into a discriminated union
that maps each concrete kind to its typed spec (e.g., use a union type such as
Resource = ChannelResource | VersionResource ... where each variant has kind:
"Channel" | "Version" and spec: ChannelSpec | VersionSpec respectively); update
the model symbols "kind" and "spec" and the validation decorators
(`@minLength/`@maxLength) to apply on the discriminant and ensure runtime/schema
validation uses the corresponding spec schema for each variant (refer to
ChannelSpec and VersionSpec types when creating the union) so requests/responses
are strongly typed and validated against the HyperFleet architecture standards.
🪄 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: e87483ae-1360-4c84-a45b-5d23bf8f2e35
📒 Files selected for processing (14)
CHANGELOG.mdaliases-gcp.tspmain.tspmodels-gcp/channel/model.tspmodels-gcp/version/model.tspmodels/resource/example_patch.tspmodels/resource/example_post.tspmodels/resource/example_resource.tspmodels/resource/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlservices/resources.tsp
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 (2)
schemas/gcp/swagger.yaml (2)
69-101:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftKeep
/channelsand/versionstyped instead of collapsing them toResource*.These operations now expose
Resource,ResourceCreateRequest, andResourcePatchRequest, but those schemas makespeca free-form object andname/kindgeneric. That drops the published Channel/Version contract, so codegen and request validation can no longer enforce kind-specific fields or constraints.Please keep the generic resource model for the top-level
/resourcesAPI, but use Channel/Version-specific wrappers here (orallOfoverlays that pinkindand the concretespecschema).Also applies to: 204-209, 274-310, 412-417, 2735-2740, 2853-2856, 2904-2906
🤖 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 `@schemas/gcp/swagger.yaml` around lines 69 - 101, The /channels and /versions endpoints (e.g., operationId getChannels and the POST that references ResourceCreateRequest) must not use the generic Resource/ResourceCreateRequest/ResourcePatchRequest directly; instead create Channel/Version-specific request and response wrappers (or use allOf overlays) that pin "kind" to "Channel"/"Version" and replace the free-form "spec" with the concrete Channel/Version spec schema; update the path entries so GET/POST/PUT/PATCH responses and body parameters reference these new Channel* / Version* schemas (or allOf compositions) rather than Resource*, ensuring codegen and validation can enforce kind-specific fields and constraints.
2640-2651:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't allow empty
referencesentries in create/patch payloads.
referencesreusesObjectReference, butObjectReferencehas no required fields. That makes{}a valid reference entry, so the contract cannot guarantee a resolvable association and the new reference-based delete restrictions become unenforceable at the schema level.Use a request-side reference schema with required identifiers instead of reusing the response-side object as-is.
Also applies to: 2846-2852, 2898-2903
🤖 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 `@schemas/gcp/swagger.yaml` around lines 2640 - 2651, The ObjectReference schema is used for response objects but has no required fields, which allows empty `{}` entries in request payloads for `references`; create a new request-side schema (e.g., ObjectReferenceRequest or ReferenceCreate) that requires the identifier fields needed to resolve associations (such as `id` and/or `href` depending on your resolution logic), update the create/patch operation request bodies to use this new schema for `references` instead of `ObjectReference`, and ensure the new schema is referenced for all affected places (including the other occurrences noted around the 2846-2852 and 2898-2903 ranges) so empty reference objects are rejected by the schema.
♻️ Duplicate comments (1)
models/resource/model.tsp (1)
21-56:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe past review concern remains unaddressed: Resource must be a discriminated union.
The current model uses
kind: string+spec: Record<unknown>, which prevents the generated OpenAPI/Swagger from exposing theChannelSpecandVersionSpecschemas introduced in this PR. This forces all API consumers (including the linked hyperfleet-api, sentinel, and adapter repositories) to work with untypedspecblobs and lose compile-time validation.The PR objectives state adding "Channel/Version spec schemas," but without discriminated-union polymorphism, those schemas remain invisible to codegen and clients.
As per coding guidelines, validate changes against HyperFleet architecture standards—the current opaque-spec design contradicts the architecture's intent to provide strongly-typed resource variants.
🤖 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 `@models/resource/model.tsp` around lines 21 - 56, The Resource model must be converted from a single opaque record into a discriminated union so the OpenAPI generator exposes ChannelSpec/VersionSpec schemas; change the Resource declaration to a union of concrete variants (e.g., ChannelResource, VersionResource, etc.) that each include the discriminator field kind with a literal value and a strongly-typed spec property (e.g., spec: ChannelSpec, spec: VersionSpec), update any usages that construct or validate Resource to handle the union variants, and ensure the OpenAPI generation picks up the discriminator by keeping the kind field as the union discriminator and removing the generic spec: Record<unknown> entry from the top-level Resource model.
🤖 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 `@main.tsp`:
- Around line 24-25: The `@info` decorator is using a multi-line object literal
that the CI version extractor can't parse; change the `@info` decorator (the
symbol "`@info`" and its "version" field) back to the compact format the extractor
expects by collapsing the object to a single-line/simple decorator containing
only the version value so the extractor can read "version" correctly (e.g.,
replace the multi-line block that includes version: "1.0.16" with a single-line
decorator form).
In `@schemas/core/openapi.yaml`:
- Around line 1968-1972: Update the OpenAPI documentation for the generation
field so it declares that generation is incremented on spec, label, or
references changes (not just spec or label); specifically modify the description
of the generation property and any related schema comments to mention
"references" as a trigger, and ensure any linked schema types like
ResourcePatchRequest are referenced in the description; also review and update
any validation or reconciliation logic that consumes generation (e.g.,
reconciliation/generation comparison code that assumes only spec/label changes)
to treat reference-only edits as generation-changing so downstream processors
will requeue and aggregate status correctly.
In `@schemas/gcp/swagger.yaml`:
- Around line 1023-1086: The GET operation getNodePoolsStatuses is missing a
security block and is unintentionally public; add the same security requirement
used by other protected endpoints by adding a security entry that requires
BearerAuth (e.g., "security: - BearerAuth: []") under the
'/api/hyperfleet/v1/clusters/{cluster_id}/nodepools/{nodepool_id}/statuses' get
operation (operationId getNodePoolsStatuses) so the endpoint enforces bearer
token authentication.
In `@services/channels.tsp`:
- Around line 16-70: The routes under the Channels interface are still using
generic Resource types, so replace the generic types with channel-specific
schemas: update getChannels/getChannelById return types to use
ChannelResource/Channel (or ResourceList<ChannelResource> as appropriate),
change postChannel body type from ResourceCreateRequest to ChannelCreateRequest
and its response resource to ChannelResource, and change patchChannelById body
from ResourcePatchRequest to ChannelPatchRequest (and return type to
ChannelResource/Channel); keep the existing route decorators and operationId
names (getChannels, getChannelById, postChannel, patchChannelById) but bind them
to ChannelSpec/Channel-specific request and response types so the contract
enforces channel kind and surfaces channel fields. Ensure
NotFoundResponse/BadRequestResponse/ConflictResponse usage remains compatible
with the new channel-specific types.
In `@services/versions.tsp`:
- Around line 15-94: The interface Versions methods currently use generic
Resource types which allow any kind/spec; update all method signatures to use
version-specific request/response types so the API contract enforces
kind=Version and surfaces VersionSpec (e.g., replace ResourceList with
VersionList or ResourceList<Version> in getVersionsByChannelId,
ResourceCreateRequest with VersionCreateRequest in postVersion, Resource with
Version in getVersionById and deleteVersionById responses, and
ResourcePatchRequest with VersionPatchRequest in patchVersionById), keeping
existing statusCode shapes and error responses intact and ensuring the
path/operationId/method names (getVersionsByChannelId, postVersion,
getVersionById, patchVersionById, deleteVersionById) are updated to reference
these new Version-specific types.
---
Outside diff comments:
In `@schemas/gcp/swagger.yaml`:
- Around line 69-101: The /channels and /versions endpoints (e.g., operationId
getChannels and the POST that references ResourceCreateRequest) must not use the
generic Resource/ResourceCreateRequest/ResourcePatchRequest directly; instead
create Channel/Version-specific request and response wrappers (or use allOf
overlays) that pin "kind" to "Channel"/"Version" and replace the free-form
"spec" with the concrete Channel/Version spec schema; update the path entries so
GET/POST/PUT/PATCH responses and body parameters reference these new Channel* /
Version* schemas (or allOf compositions) rather than Resource*, ensuring codegen
and validation can enforce kind-specific fields and constraints.
- Around line 2640-2651: The ObjectReference schema is used for response objects
but has no required fields, which allows empty `{}` entries in request payloads
for `references`; create a new request-side schema (e.g., ObjectReferenceRequest
or ReferenceCreate) that requires the identifier fields needed to resolve
associations (such as `id` and/or `href` depending on your resolution logic),
update the create/patch operation request bodies to use this new schema for
`references` instead of `ObjectReference`, and ensure the new schema is
referenced for all affected places (including the other occurrences noted around
the 2846-2852 and 2898-2903 ranges) so empty reference objects are rejected by
the schema.
---
Duplicate comments:
In `@models/resource/model.tsp`:
- Around line 21-56: The Resource model must be converted from a single opaque
record into a discriminated union so the OpenAPI generator exposes
ChannelSpec/VersionSpec schemas; change the Resource declaration to a union of
concrete variants (e.g., ChannelResource, VersionResource, etc.) that each
include the discriminator field kind with a literal value and a strongly-typed
spec property (e.g., spec: ChannelSpec, spec: VersionSpec), update any usages
that construct or validate Resource to handle the union variants, and ensure the
OpenAPI generation picks up the discriminator by keeping the kind field as the
union discriminator and removing the generic spec: Record<unknown> entry from
the top-level Resource model.
🪄 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: b454e2e9-0d12-4d5b-a8c8-d71d26af2fe2
📒 Files selected for processing (16)
CHANGELOG.mdaliases-gcp.tspmain.tspmodels-gcp/channel/model.tspmodels-gcp/version/model.tspmodels/resource/example_patch.tspmodels/resource/example_post.tspmodels/resource/example_resource.tspmodels/resource/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlservices/channels.tspservices/resources.tspservices/versions.tsp
rh-amarin
reviewed
May 20, 2026
| @route("") | ||
| @summary("List channels") | ||
| @operationId("getChannels") | ||
| getChannels(...QueryParams): Body<ResourceList> |
Collaborator
There was a problem hiding this comment.
This should return a Body<ChannelList> and similarly for other methods and for Versions
In the current version, no ChannelSpec is produced in the gcp/openapi.yaml so it can not be used later in the API to validate the requests.
Take the example of Clusters, which render in the openapi.yaml a schema for ClusterSpec that is used in the api in schema_validation.go for checking against the POST/PATCH requests
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemas/gcp/swagger.yaml (1)
90-95:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftConstrain
kindon kind-specific create routes.
POST /channelsandPOST /channels/{channel_id}/versionsnow accept the fully genericResourceCreateRequest, so the schema allows contradictory payloads likekind: Clusteron a channel route orkind: Channelon a version route. That weakens the public contract and generated clients will not catch invalid combinations.Illustrative fix
- in: body name: body required: true schema: - $ref: '`#/definitions/ResourceCreateRequest`' + allOf: + - $ref: '`#/definitions/ResourceCreateRequest`' + - type: object + properties: + kind: + enum: + - ChannelApply the same pattern to the version route with
Version.Also applies to: 299-303
🤖 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 `@schemas/gcp/swagger.yaml` around lines 90 - 95, Replace the generic ResourceCreateRequest on kind-specific POST routes with kind-constrained request schemas: update the POST /channels body schema to reference the ChannelCreateRequest definition (which must fix "kind" to "Channel") and update the POST /channels/{channel_id}/versions body schema to reference VersionCreateRequest (which must fix "kind" to "Version"); also apply the same change to the other occurrence noted (lines ~299-303) so generated clients and validation enforce the correct kind for each route.
♻️ Duplicate comments (1)
main.tsp (1)
24-25:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRestore the extractor-friendly
@infolayout.
CI / validateis still failing withFailed to extract version from main.tsp, so this decorator shape is not parseable by the current version extractor.Suggested fix
-@info(#{ - version: "1.0.16", +@info(#{ version: "1.0.16", contact: #{ name: "HyperFleet Team", url: "https://github.com/openshift-hyperfleet", }, license: #{ name: "Apache 2.0", url: "https://www.apache.org/licenses/LICENSE-2.0", }, })🤖 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 `@main.tsp` around lines 24 - 25, The `@info` decorator currently uses a nonstandard "#{...}" map shape which the version extractor cannot parse; update the `@info` usage to the extractor-friendly layout by replacing the `@info`(#{ version: "1.0.16", ... }) form with the simple key/value decorator form that the extractor expects (i.e., use `@info`(version: "1.0.16", ...) or the project's canonical multi-line `@info` layout), ensuring the "version" key remains present so the extractor can find it.
🤖 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`:
- Line 5: Update the Keep a Changelog URL to reference the official Keep a
Changelog format version (e.g., replace the current
"https://keepachangelog.com/en/1.0.16/" with
"https://keepachangelog.com/en/1.0.0/" or
"https://keepachangelog.com/en/1.1.0/"); locate the link text in the CHANGELOG
header line that currently contains "keepachangelog.com/en/1.0.16" and change
the version segment to the correct format version so the link points to the Keep
a Changelog project's documentation rather than this project's version.
In `@schemas/gcp/swagger.yaml`:
- Around line 2908-2927: The ResourceStatus schema currently only exposes
conditions (ResourceStatus with properties.conditions referencing
ResourceCondition) which drops manifest-level status (resource.status.manifests)
consumed by hyperfleet-adapter/sentinel; update the ResourceStatus definition to
include a manifests property (e.g., properties.manifests as an array/object
matching the existing manifest status shape used by consumers) alongside
conditions, adjust required if manifests must be present (or leave optional) and
ensure the manifest item schema matches the existing contract so clients reading
resource.status.manifests continue to work.
In `@services/versions.tsp`:
- Around line 81-93: The deleteVersionById route signature is missing a
ConflictResponse branch for 409s; update the deleteVersionById method (the
`@route` "/channels/{channel_id}/versions/{version_id}" `@delete` operationId
"deleteVersionById") to include ConflictResponse in its return union alongside
NotFoundResponse, Error, and BadRequestResponse, so
deletion-blocked-by-reference/restrict errors surface as 409, then regenerate
the GCP spec so the 409 appears in generated clients.
---
Outside diff comments:
In `@schemas/gcp/swagger.yaml`:
- Around line 90-95: Replace the generic ResourceCreateRequest on kind-specific
POST routes with kind-constrained request schemas: update the POST /channels
body schema to reference the ChannelCreateRequest definition (which must fix
"kind" to "Channel") and update the POST /channels/{channel_id}/versions body
schema to reference VersionCreateRequest (which must fix "kind" to "Version");
also apply the same change to the other occurrence noted (lines ~299-303) so
generated clients and validation enforce the correct kind for each route.
---
Duplicate comments:
In `@main.tsp`:
- Around line 24-25: The `@info` decorator currently uses a nonstandard "#{...}"
map shape which the version extractor cannot parse; update the `@info` usage to
the extractor-friendly layout by replacing the `@info`(#{ version: "1.0.16", ...
}) form with the simple key/value decorator form that the extractor expects
(i.e., use `@info`(version: "1.0.16", ...) or the project's canonical multi-line
`@info` layout), ensuring the "version" key remains present so the extractor can
find it.
🪄 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: b1acb90e-239e-4dcf-acaa-6e76bf9e2f15
📒 Files selected for processing (18)
CHANGELOG.mdaliases-gcp.tspaliases.tspmain.tspmodels-gcp/channel/model.tspmodels-gcp/version/model.tspmodels/resource/example_patch.tspmodels/resource/example_post.tspmodels/resource/example_resource.tspmodels/resource/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlservices/channels.tspservices/resources.tspservices/statuses-internal.tspservices/versions.tsp
| All notable changes to the HyperFleet API specification will be documented in this file. | ||
|
|
||
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), | ||
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.16/), |
There was a problem hiding this comment.
Revert the Keep a Changelog link to the correct version.
The Keep a Changelog link was changed from version 1.0.0 to 1.0.16, but 1.0.16 refers to this project's version, not the Keep a Changelog format version. The URL should reference the Keep a Changelog project's own versioning (either 1.0.0 or the current 1.1.0), not this project's version.
🔗 Proposed fix
-The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.16/),
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),📝 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.
Suggested change
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.16/), | |
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), |
🤖 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 `@CHANGELOG.md` at line 5, Update the Keep a Changelog URL to reference the
official Keep a Changelog format version (e.g., replace the current
"https://keepachangelog.com/en/1.0.16/" with
"https://keepachangelog.com/en/1.0.0/" or
"https://keepachangelog.com/en/1.1.0/"); locate the link text in the CHANGELOG
header line that currently contains "keepachangelog.com/en/1.0.16" and change
the version segment to the correct format version so the link points to the Keep
a Changelog project's documentation rather than this project's version.
Comment on lines
+2908
to
+2927
| ResourceStatus: | ||
| description: >- | ||
| Status computed from adapter condition reports. | ||
|
|
||
|
|
||
| For adapter-managed resources (Cluster, NodePool), conditions are | ||
| populated | ||
|
|
||
| by the status aggregation pipeline. For non-adapter resources (Channel, | ||
| Version), | ||
|
|
||
| conditions array is empty. | ||
| properties: | ||
| conditions: | ||
| items: | ||
| $ref: '#/definitions/ResourceCondition' | ||
| type: array | ||
| required: | ||
| - conditions | ||
| type: object |
There was a problem hiding this comment.
ResourceStatus drops manifest-level status data.
This schema now exposes only conditions, but linked consumers in openshift-hyperfleet/hyperfleet-adapter and openshift-hyperfleet/hyperfleet-sentinel still read manifest status from resource.status.manifests. Publishing this as the generic Resource contract will make those clients lose that field unless the break is coordinated across repos.
As per coding guidelines, "Validate changes against HyperFleet architecture standards from the linked architecture repository."
🤖 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 `@schemas/gcp/swagger.yaml` around lines 2908 - 2927, The ResourceStatus schema
currently only exposes conditions (ResourceStatus with properties.conditions
referencing ResourceCondition) which drops manifest-level status
(resource.status.manifests) consumed by hyperfleet-adapter/sentinel; update the
ResourceStatus definition to include a manifests property (e.g.,
properties.manifests as an array/object matching the existing manifest status
shape used by consumers) alongside conditions, adjust required if manifests must
be present (or leave optional) and ensure the manifest item schema matches the
existing contract so clients reading resource.status.manifests continue to work.
Comment on lines
+81
to
+93
| @route("/channels/{channel_id}/versions/{version_id}") | ||
| @delete | ||
| @summary("Request version deletion") | ||
| @operationId("deleteVersionById") | ||
| deleteVersionById( | ||
| @path channel_id: string, | ||
| @path version_id: string, | ||
| ): { | ||
| @statusCode statusCode: 202; | ||
| @body resource: Resource; | ||
| } | NotFoundResponse | ||
| | Error | ||
| | BadRequestResponse; |
There was a problem hiding this comment.
Add ConflictResponse to version deletion.
The generic DELETE /resources/{resource_id} contract already models 409 when deletion is blocked by references or restrict rules, but DELETE /channels/{channel_id}/versions/{version_id} drops that branch. Since versions are still Resources, clients generated from the nested route cannot distinguish a real delete conflict from an unexpected error. Regenerate the GCP spec after fixing this signature so the 409 is exposed there too.
Suggested fix
deleteVersionById(
`@path` channel_id: string,
`@path` version_id: string,
): {
`@statusCode` statusCode: 202;
`@body` resource: Resource;
} | NotFoundResponse
+ | ConflictResponse
| Error
| BadRequestResponse;📝 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.
Suggested change
| @route("/channels/{channel_id}/versions/{version_id}") | |
| @delete | |
| @summary("Request version deletion") | |
| @operationId("deleteVersionById") | |
| deleteVersionById( | |
| @path channel_id: string, | |
| @path version_id: string, | |
| ): { | |
| @statusCode statusCode: 202; | |
| @body resource: Resource; | |
| } | NotFoundResponse | |
| | Error | |
| | BadRequestResponse; | |
| `@route`("/channels/{channel_id}/versions/{version_id}") | |
| `@delete` | |
| `@summary`("Request version deletion") | |
| `@operationId`("deleteVersionById") | |
| deleteVersionById( | |
| `@path` channel_id: string, | |
| `@path` version_id: string, | |
| ): { | |
| `@statusCode` statusCode: 202; | |
| `@body` resource: Resource; | |
| } | NotFoundResponse | |
| | ConflictResponse | |
| | Error | |
| | BadRequestResponse; |
🤖 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 `@services/versions.tsp` around lines 81 - 93, The deleteVersionById route
signature is missing a ConflictResponse branch for 409s; update the
deleteVersionById method (the `@route`
"/channels/{channel_id}/versions/{version_id}" `@delete` operationId
"deleteVersionById") to include ConflictResponse in its return union alongside
NotFoundResponse, Error, and BadRequestResponse, so
deletion-blocked-by-reference/restrict errors surface as 409, then regenerate
the GCP spec so the 409 appears in generated clients.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
CHANGELOG.md (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the correct Keep a Changelog spec URL in Line 5.
The link currently points to
.../1.0.16/(project version), not a Keep a Changelog format version.Suggested fix
-The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.16/), +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),🤖 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 `@CHANGELOG.md` at line 5, Update the Keep a Changelog URL on the line that reads "The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.16/)," to point to the canonical spec release (for example replace 1.0.16 with 1.0.0 so it becomes https://keepachangelog.com/en/1.0.0/) so the link references the Keep a Changelog format spec rather than the project version.services/versions.tsp (1)
66-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExpose
409 ConflictindeleteVersionById.Line 76’s return union omits
ConflictResponse. Delete flows can hit restrict/reference conflicts, and this endpoint currently hides that path behind generic errors.Suggested fix
): { `@statusCode` statusCode: 202; `@body` version: Version; } | NotFoundResponse + | ConflictResponse | Error | BadRequestResponse;As per coding guidelines,
Validate changes against HyperFleet architecture standards from the linked architecture repository.🤖 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 `@services/versions.tsp` around lines 66 - 78, The deleteVersionById API declaration omits ConflictResponse from its return union causing 409 conflict cases to be hidden; update the deleteVersionById method signature to include ConflictResponse in the union of possible returns (and ensure any specific 409 statusCode/response metadata for deleteVersionById is present/consistent), referencing the deleteVersionById operation and the ConflictResponse type so callers can observe and handle 409 Conflict scenarios per HyperFleet standards.schemas/gcp/openapi.yaml (1)
334-368:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing operation description for
deleteVersionById.The static analysis tool flags this operation as missing a description. For consistency with
deleteChannelById(which documents RESTRICT policy) anddeleteResourceById, add a description explaining the deletion behavior.Suggested fix
delete: operationId: deleteVersionById summary: Request version deletion + description: Marks the version for deletion by setting deleted_time. parameters:🤖 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 `@schemas/gcp/openapi.yaml` around lines 334 - 368, The delete operation deleteVersionById is missing an operation-level description; add a concise description explaining the deletion semantics (e.g., that the request schedules deletion and follows the service's delete policy such as RESTRICT or soft-delete behavior) matching the style of deleteChannelById and deleteResourceById so static analysis passes and consumers understand the behavior. Update the OpenAPI entry for operationId deleteVersionById to include a "description" field that documents the behavior and any policy constraints or returned status (e.g., 202 accepted when deletion is scheduled).
🤖 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 `@models-gcp/channel/model.tsp`:
- Around line 27-34: Add the missing status mixin/field to the Channel model so
Channel responses match the generic Resource shape; update model Channel (which
currently spreads ...ChannelBase and ...APIMetadata and declares generation) to
also include the resource status (e.g., add the existing ...ResourceStatus mixin
or a status: ResourceStatus field used elsewhere) so status.conditions is
present (empty when not aggregated) and run validation against the HyperFleet
architecture standards.
In `@models-gcp/version/model.tsp`:
- Around line 37-46: The Version model is missing the status field required by
the generic resource contract; update the model Version to include a status:
ResourceStatus field (alongside ...VersionBase and ...APIMetadata) so
/channels/{channel_id}/versions returns a ResourceStatus (ensure non-adapter
resources produce an empty conditions array by default), and run the HyperFleet
architecture validation for the change to confirm compliance.
In `@models/resource/example_resource.tsp`:
- Line 7: Update the example href in the resource example so it uses the generic
Resource API route instead of a kind-specific path: replace the current href
value (the href key in models/resource/example_resource.tsp) that points to
"/api/hyperfleet/v1/myresources/{id}" with the generic
"/api/hyperfleet/v1/resources/{id}" (keeping the same UUID), so the example
matches the new /resources/{id} contract.
In `@schemas/gcp/openapi.yaml`:
- Around line 2732-2734: ResourceList.total currently declares format: int64
while other list schemas (ChannelList.total, ClusterList.total,
NodePoolList.total, AdapterStatusList.total, VersionList.total) use format:
int32; update ResourceList.total to use format: int32 so it matches the
established pattern and avoids client codegen inconsistencies—locate the
ResourceList schema and change its total property's format from int64 to int32.
- Around line 291-333: The patchVersionById operation (operationId:
patchVersionById) is missing a description; add a brief descriptive
"description" field under the operation that explains what the endpoint does
(e.g., "Partially update a Version by its ID within a channel"), include any
important behaviors or constraints (such as which fields can be patched or
possible side effects), and keep the style consistent with other PATCH
operations in the spec.
---
Duplicate comments:
In `@CHANGELOG.md`:
- Line 5: Update the Keep a Changelog URL on the line that reads "The format is
based on [Keep a Changelog](https://keepachangelog.com/en/1.0.16/)," to point to
the canonical spec release (for example replace 1.0.16 with 1.0.0 so it becomes
https://keepachangelog.com/en/1.0.0/) so the link references the Keep a
Changelog format spec rather than the project version.
In `@schemas/gcp/openapi.yaml`:
- Around line 334-368: The delete operation deleteVersionById is missing an
operation-level description; add a concise description explaining the deletion
semantics (e.g., that the request schedules deletion and follows the service's
delete policy such as RESTRICT or soft-delete behavior) matching the style of
deleteChannelById and deleteResourceById so static analysis passes and consumers
understand the behavior. Update the OpenAPI entry for operationId
deleteVersionById to include a "description" field that documents the behavior
and any policy constraints or returned status (e.g., 202 accepted when deletion
is scheduled).
In `@services/versions.tsp`:
- Around line 66-78: The deleteVersionById API declaration omits
ConflictResponse from its return union causing 409 conflict cases to be hidden;
update the deleteVersionById method signature to include ConflictResponse in the
union of possible returns (and ensure any specific 409 statusCode/response
metadata for deleteVersionById is present/consistent), referencing the
deleteVersionById operation and the ConflictResponse type so callers can observe
and handle 409 Conflict scenarios per HyperFleet standards.
🪄 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: 898c398b-9db6-48cd-856c-d24667d1b5a9
📒 Files selected for processing (24)
CHANGELOG.mdaliases-gcp.tspaliases.tspmain.tspmodels-gcp/channel/example_channel.tspmodels-gcp/channel/example_patch.tspmodels-gcp/channel/example_post.tspmodels-gcp/channel/model.tspmodels-gcp/version/example_patch.tspmodels-gcp/version/example_post.tspmodels-gcp/version/example_version.tspmodels-gcp/version/model.tspmodels/resource/example_patch.tspmodels/resource/example_post.tspmodels/resource/example_resource.tspmodels/resource/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlservices/channels.tspservices/resources.tspservices/statuses-internal.tspservices/versions.tsp
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
schemas/gcp/openapi.yaml (1)
3107-3121:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftRestore
ResourceStatus.manifestsor coordinate the breaking change.This schema now publishes only
conditions, but linked consumers inopenshift-hyperfleet/hyperfleet-adapter(internal/manifest/generation.go) andopenshift-hyperfleet/hyperfleet-sentinel(internal/client/client.go) still readresource.status.manifests. Merging this contract as-is will break those clients unless the cross-repo migration is coordinated first. As per coding guidelines, "Validate changes against HyperFleet architecture standards from the linked architecture repository."🤖 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 `@schemas/gcp/openapi.yaml` around lines 3107 - 3121, The change removed ResourceStatus.manifests from the ResourceStatus schema causing downstream breakage; either restore the manifests property on ResourceStatus (e.g., re-add a manifests array property with the appropriate item schema/ref used elsewhere) so existing consumers like internal/manifest/generation.go and internal/client/client.go continue to read resource.status.manifests, or, if you intend a breaking API change, coordinate and version it: add a clear migration note in the OpenAPI change, update callers in openshift-hyperfleet/hyperfleet-adapter and openshift-hyperfleet/hyperfleet-sentinel to stop reading manifests (or read a new field), and bump the API version so consumers can migrate safely.
🤖 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 `@services/statuses-internal.tsp`:
- Around line 66-83: Add a compatibility validation step and gate the release on
regenerated client/schema checks for the new /resources/{resource_id}/statuses
endpoint: update the integration checklist to run a schema/client regeneration
and compatibility test against the downstream repos that consume
Resource/ResourceStatus contracts (openshift-hyperfleet/hyperfleet-api,
-sentinel, -adapter), ensure the generated client types for the
ResourceStatusesInternal interface and putResourceStatuses operation (and
related types like
AdapterStatusCreateRequest/AdapterStatus/ResourceStatus/Resource) match existing
consumers, and block merging/releasing until the regenerated clients pass
automated compatibility tests in CI.
---
Duplicate comments:
In `@schemas/gcp/openapi.yaml`:
- Around line 3107-3121: The change removed ResourceStatus.manifests from the
ResourceStatus schema causing downstream breakage; either restore the manifests
property on ResourceStatus (e.g., re-add a manifests array property with the
appropriate item schema/ref used elsewhere) so existing consumers like
internal/manifest/generation.go and internal/client/client.go continue to read
resource.status.manifests, or, if you intend a breaking API change, coordinate
and version it: add a clear migration note in the OpenAPI change, update callers
in openshift-hyperfleet/hyperfleet-adapter and
openshift-hyperfleet/hyperfleet-sentinel to stop reading manifests (or read a
new field), and bump the API version so consumers can migrate safely.
🪄 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: f752b2f1-0930-47f5-b51a-e144df33093a
📒 Files selected for processing (25)
CHANGELOG.mdaliases-gcp.tspaliases.tspmain.tspmodels-gcp/channel/example_channel.tspmodels-gcp/channel/example_patch.tspmodels-gcp/channel/example_post.tspmodels-gcp/channel/model.tspmodels-gcp/version/example_patch.tspmodels-gcp/version/example_post.tspmodels-gcp/version/example_version.tspmodels-gcp/version/model.tspmodels/resource/example_patch.tspmodels/resource/example_post.tspmodels/resource/example_resource.tspmodels/resource/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlservices/channels.tspservices/resources.tspservices/statuses-internal-gcp.tspservices/statuses-internal.tspservices/versions.tsp
Comment on lines
+66
to
+83
| @tag("Resource statuses") | ||
| @route("/resources/{resource_id}/statuses") | ||
| @useAuth(HyperFleet.BearerAuth) | ||
| interface ResourceStatusesInternal { | ||
| @route("") | ||
| @put | ||
| @summary("Adapter creates or updates resource status") | ||
| @operationId("putResourceStatuses") | ||
| @doc("Adapters call this endpoint to report status for a resource after each evaluation. The adapter's status entry is created if it doesn't exist, or updated if it does (upserted by adapter name).") | ||
| putResourceStatuses( | ||
| @path resource_id: string, | ||
| @body body: AdapterStatusCreateRequest, | ||
| ): | ||
| | (CreatedResponse & AdapterStatus) | ||
| | BadRequestResponse | ||
| | NotFoundResponse | ||
| | ConflictResponse; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Validate downstream consumer compatibility for new resource-status endpoint.
Given linked repos (openshift-hyperfleet/hyperfleet-api, openshift-hyperfleet/hyperfleet-sentinel, openshift-hyperfleet/hyperfleet-adapter) consume Resource/ResourceStatus contracts, gate release on regenerated client/schema compatibility checks for /resources/{resource_id}/statuses to avoid integration 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 `@services/statuses-internal.tsp` around lines 66 - 83, Add a compatibility
validation step and gate the release on regenerated client/schema checks for the
new /resources/{resource_id}/statuses endpoint: update the integration checklist
to run a schema/client regeneration and compatibility test against the
downstream repos that consume Resource/ResourceStatus contracts
(openshift-hyperfleet/hyperfleet-api, -sentinel, -adapter), ensure the generated
client types for the ResourceStatusesInternal interface and putResourceStatuses
operation (and related types like
AdapterStatusCreateRequest/AdapterStatus/ResourceStatus/Resource) match existing
consumers, and block merging/releasing until the regenerated clients pass
automated compatibility tests in CI.
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
Test Plan
make test-allpassesmake lintpassesmake test-helm(if applicable)