HYPERFLEET-1103 - feat: make statuses core only#54
Merged
openshift-merge-bot[bot] merged 1 commit intoMay 27, 2026
Merged
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR moves resource force-delete and resource-status endpoints into core/services/resources-internal (with imports pointed at shared models), replaces internal cluster/nodepool status PUTs with GET-only interfaces in core/services/statuses-internal, removes the shared statuses service, updates main.tsp to import the new core service, reorders several OpenAPI path entries, and bumps package/OpenAPI versions to 1.0.19. Sequence Diagram(s)sequenceDiagram
participant Client
participant ResourcesService
participant Database
Client->>ResourcesService: POST /resources/{resource_id}/force-delete
ResourcesService->>Database: permanently remove resource (DB-only)
Database-->>ResourcesService: 204 or error
ResourcesService-->>Client: 204 or Error response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
rh-amarin
force-pushed
the
statuses-private
branch
3 times, most recently
from
13f7567 to
d6380dc
Compare
![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 `@core/services/statuses-internal.tsp`:
- Around line 21-33: The getClusterStatuses signature currently returns
Body<AdapterStatusList> | NotFoundResponse | BadRequestResponse and should be
aligned with ResourceStatuses.getResourceStatuses by including the generic Error
union; update the return type of getClusterStatuses (and the sibling list
endpoint method(s) referenced around lines 61-74, e.g., the nodepool/status
method such as getNodepoolStatuses) to return Error | Body<AdapterStatusList> |
NotFoundResponse | BadRequestResponse (matching
ResourceStatuses.getResourceStatuses) so generated clients see a consistent
AdapterStatusList error contract across cluster, nodepool, and resource
endpoints.
In `@main.tsp`:
- Around line 32-33: The `@info` decorator in main.tsp is spread across lines so
the CI extractor cannot read the version; update the `@info`(...) block (the `@info`
decorator) to the CI-parsed format the validator expects by placing the version
payload in the single-line/expected token form (no newlines or non-standard
spacing) so the extractor can reliably parse "version": "1.0.19".
🪄 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: 0967a15f-5556-4669-81ec-b8b181df86f5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
core/services/resources-internal.tspcore/services/statuses-internal.tspmain.tsppackage.jsonschemas/core/openapi.yamlshared/services/statuses.tsp
💤 Files with no reviewable changes (1)
- shared/services/statuses.tsp
rh-amarin
force-pushed
the
statuses-private
branch
2 times, most recently
from
fad6c4c to
f5cd52d
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)
schemas/core/openapi.yaml (1)
621-726:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftStatus write endpoints remain publicly exposed in the released core OpenAPI contract.
.github/workflows/release.ymlcopiesschemas/core/openapi.yamltocore-openapi.yamland publishes it as a GitHub Release asset.schemas/core/openapi.yamlstill includes the status write operations (operationId: putNodePoolStatusesandoperationId: putClusterStatuses), and there are no internal/private markers (e.g.,x-internal/private) in the schema to enforce a “core-only/private” boundary.- To make these endpoints truly core-only, remove them from the public/released OpenAPI or mark them as internal in a way your docs/SDK generation actually honors.
🤖 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/core/openapi.yaml` around lines 621 - 726, The OpenAPI currently exposes write endpoints that should be core-only: remove or mark the two operations with operationId putNodePoolStatuses and putClusterStatuses as internal; specifically either delete the entire PUT operation blocks for those paths or add an explicit vendor extension (e.g., x-internal: true or x-private: true) to each operation and ensure the docs/SDK generator and release step honor that field. Also update the release pipeline step that publishes core OpenAPI (the workflow that copies schemas/core/openapi.yaml to the released artifact) to publish a filtered/public version (or skip publishing when x-internal is present) so status write endpoints are not included in the released contract.
🤖 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 `@package.json`:
- Line 3: The package.json version bump is insufficient for an API-breaking
change; update the "version" field from "1.0.21" to a breaking-major release
(e.g., "2.0.0") and add an explicit migration note describing the status
endpoint/service composition changes to the repo's CHANGELOG.md (or
RELEASE_NOTES/README) so downstream HyperFleet consumers can adapt; also mention
the affected routes and any required adapter/test updates in that note and
ensure any package references or CI/release metadata that depend on the version
string (e.g., release scripts) are updated accordingly.
---
Outside diff comments:
In `@schemas/core/openapi.yaml`:
- Around line 621-726: The OpenAPI currently exposes write endpoints that should
be core-only: remove or mark the two operations with operationId
putNodePoolStatuses and putClusterStatuses as internal; specifically either
delete the entire PUT operation blocks for those paths or add an explicit vendor
extension (e.g., x-internal: true or x-private: true) to each operation and
ensure the docs/SDK generator and release step honor that field. Also update the
release pipeline step that publishes core OpenAPI (the workflow that copies
schemas/core/openapi.yaml to the released artifact) to publish a filtered/public
version (or skip publishing when x-internal is present) so status write
endpoints are not included in the released contract.
🪄 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: 0ec59625-4c5c-40f6-9850-fb638820edc9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
core/services/resources-internal.tspcore/services/statuses-internal.tspmain.tsppackage.jsonschemas/core/openapi.yamlshared/services/statuses.tsp
💤 Files with no reviewable changes (1)
- shared/services/statuses.tsp
rh-amarin
force-pushed
the
statuses-private
branch
from
f5cd52d to
20c44cf
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 `@README.md`:
- Around line 92-95: Update the README table to remove the misleading
external/shared status entry and reflect the core-only status architecture:
delete or replace the `shared/services/statuses.tsp` row, and update the
`core/services/statuses-internal.tsp` row to indicate PUT (write) or appropriate
operations are internal-only (Audience: Internal adapters/private; Included in
Build: ✅ Yes), and add a short note that external customers receive status via
mapped conditions rather than a public statuses endpoint.
🪄 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: 2d83fd41-fb5e-45b7-88d2-c0986a849581
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
README.mdcore/services/resources-internal.tspcore/services/statuses-internal.tspmain.tsppackage.jsonschemas/core/openapi.yamlshared/services/statuses.tsp
💤 Files with no reviewable changes (1)
- shared/services/statuses.tsp
rh-amarin
force-pushed
the
statuses-private
branch
from
20c44cf to
8075b41
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
README.md (1)
92-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPublic/internal status split table is stale and now misleading.
Line 94 still advertises
shared/services/statuses.tspas the external status surface, while current wiring has moved status/resource service composition to core imports. This will misdirect downstream consumers about which contract to use.As per coding guidelines,
**: Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity. 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 `@README.md` around lines 92 - 95, Update the README table to reflect that status/resource service composition is now provided from core imports: replace or change the row referencing shared/services/statuses.tsp so it no longer claims to be the external status surface and instead show core/services/statuses-internal.tsp (or the actual core export symbol) as the authoritative contract for status/resource operations, adjust the Operations/Audience/Included in Build columns accordingly, and validate the changed wording against HyperFleet architecture standards to ensure the contract and opt-in/opt-out build flags are accurately represented.
🤖 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 `@core/services/statuses-internal.tsp`:
- Around line 17-34: The GET-only interface split is not enforced because the
ClusterStatuses/NodePoolStatuses interfaces still expose write methods; remove
the putClusterStatuses and putNodePoolStatuses methods from the GET-focused
interfaces (e.g., ClusterStatuses and the GET NodePoolStatuses) and move them
into a separate PUT-specific interface (or a separate file) so read and write
contracts are distinct; update operationId/route annotations accordingly (e.g.,
keep getClusterStatuses/getNodePoolStatuses in the GET interfaces and place
putClusterStatuses/putNodePoolStatuses in a new interface like
ClusterStatusesWrite or NodePoolStatusesWrite) and adjust any references to
these symbols so generated contracts reflect the GET-only surface.
In `@main.tsp`:
- Around line 32-33: The `@info` block uses the "#{ ... }" interpolation form
which breaks the CI extractor; update the `@info` block in main.tsp to the
extractor-expected plain object layout by replacing the opening "`@info`(#{" with
"`@info`({" and ensure the block closes with "})" (so the version entry remains
version: "1.0.19"). Locate the `@info` symbol in main.tsp and convert the
surrounding braces to a simple object literal format so the extractor can parse
the version.
---
Duplicate comments:
In `@README.md`:
- Around line 92-95: Update the README table to reflect that status/resource
service composition is now provided from core imports: replace or change the row
referencing shared/services/statuses.tsp so it no longer claims to be the
external status surface and instead show core/services/statuses-internal.tsp (or
the actual core export symbol) as the authoritative contract for status/resource
operations, adjust the Operations/Audience/Included in Build columns
accordingly, and validate the changed wording against HyperFleet architecture
standards to ensure the contract and opt-in/opt-out build flags are accurately
represented.
🪄 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: 7c89a1b1-84d3-4061-90f6-95941f9cc254
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
README.mdcore/services/resources-internal.tspcore/services/statuses-internal.tspmain.tsppackage.jsonschemas/core/openapi.yamlshared/services/statuses.tsp
💤 Files with no reviewable changes (1)
- shared/services/statuses.tsp
Contributor
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kuudori 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 cc6a1a9
into
openshift-hyperfleet:main
3 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
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
With the new mapping conditions available, the
/statusendpoint should become private onlyIf data has to be offered to customers it should go through mapped conditions