HYPERFLEET-1103 - feat: split core and gcp contracts in separate repos#52
Merged
openshift-merge-bot[bot] merged 2 commits intoMay 26, 2026
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR migrates the repo to produce only the core OpenAPI schema: CI and release workflows now build/publish core only, build-schema.sh is simplified to a fixed core build, package.json and Go embedding updated to reference core/openapi.yaml, TypeSpec sources were reorganized to shared/ vs core/ with many GCP-specific models/services removed or emptied, core OpenAPI bumped to 1.0.18 with added PUT status endpoints for clusters and nodepools, and documentation/changelog updated to reflect the core-only workflow. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Linked repositories: Couldn't analyze Linked repositories: Couldn't analyze Comment |
rh-amarin
force-pushed
the
remove-gcp-from-core
branch
4 times, most recently
from
bd81e1a to
9f365a7
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shared/services/statuses.tsp (1)
37-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
@useAuthdecorator creates unauthenticated endpoint.
NodePoolStatusesinterface lacks@useAuth(HyperFleet.BearerAuth)whileClusterStatuses(line 14) has it. This makesgetNodePoolsStatusespublicly accessible without authentication, exposing adapter status data for nodepools.If the intent was to align with cluster-level auth handling,
ClusterStatusesdoes require auth, soNodePoolStatusesshould too.🔒 Proposed fix
`@tag`("NodePool statuses") `@route`("/clusters/{cluster_id}/nodepools/{nodepool_id}/statuses") +@useAuth(HyperFleet.BearerAuth) interface NodePoolStatuses{🤖 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 `@shared/services/statuses.tsp` around lines 37 - 39, The NodePoolStatuses interface is missing the authentication decorator, making its endpoints (e.g., getNodePoolsStatuses) unauthenticated; add the same decorator used by ClusterStatuses — `@useAuth`(HyperFleet.BearerAuth) — directly above the NodePoolStatuses declaration so all routes under NodePoolStatuses require the BearerAuth protection.
🤖 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 141: The changelog link for [Unreleased] was reset to
compare/v1.0.14...HEAD which breaks released-version traceability; update the
[Unreleased] link to compare/v1.0.15...HEAD (or to the last published tag
appropriate for downstream consumers) and restore the missing release note
sections for 1.0.15 through 1.0.17 so historical, authoritative release notes
remain present; locate the `[Unreleased]:
https://github.com/openshift-hyperfleet/hyperfleet-api-spec/compare/v1.0.14...HEAD`
entry and the removed "1.0.15"–"1.0.17" sections in CHANGELOG.md and reinsert
the released entries and their contents verbatim (or merge any upstream changes)
to preserve accurate history.
In `@main.tsp`:
- Around line 33-35: CI fails because the release-version extractor cannot parse
the current `@info` map-style declaration in main.tsp (the `@info`(...) block
containing version: "1.0.18"); either change the `@info` declaration so the
version appears as the extractor-expected inline token (put the version as the
inline/positional string the extractor expects instead of nested inside the map
under the key version) or update the extractor to accept the existing map-shaped
`@info` with the version key; update the `@info` usage and any tests referencing it
(symbol: `@info` and the version field) to ensure the extractor finds the version
string during CI.
In `@schemas/schemas.go`:
- Line 13: You removed "gcp/openapi.yaml" from the embedded filesystem (the
//go:embed declaration that populates FS), breaking callers that do
FS.ReadFile("gcp/openapi.yaml"); restore compatibility by either re-adding
gcp/openapi.yaml to the //go:embed list (e.g., embed both core/openapi.yaml and
gcp/openapi.yaml into FS) or provide a compatibility shim that falls back to the
new core/openapi.yaml path when FS.ReadFile("gcp/openapi.yaml") is called; if
you intend a breaking change instead, update the RELEASING.md and add a clear
migration note in the same PR describing the removal and the new path so
consumers can update their usage of FS.ReadFile("gcp/openapi.yaml").
---
Outside diff comments:
In `@shared/services/statuses.tsp`:
- Around line 37-39: The NodePoolStatuses interface is missing the
authentication decorator, making its endpoints (e.g., getNodePoolsStatuses)
unauthenticated; add the same decorator used by ClusterStatuses —
`@useAuth`(HyperFleet.BearerAuth) — directly above the NodePoolStatuses
declaration so all routes under NodePoolStatuses require the BearerAuth
protection.
🪄 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: c89f0c5f-0286-4c18-aff9-e0f3419b6e46
📒 Files selected for processing (56)
.github/workflows/ci.yml.github/workflows/release.ymlCHANGELOG.mdaliases-core.tspaliases-gcp.tspaliases.tspbuild-schema.shcore/models/cluster/example_cluster.tspcore/models/cluster/example_patch.tspcore/models/cluster/example_post.tspcore/models/cluster/model.tspcore/models/nodepool/example_nodepool.tspcore/models/nodepool/example_patch.tspcore/models/nodepool/example_post.tspcore/models/nodepool/model.tspcore/services/force-delete-internal.tspcore/services/statuses-internal.tspmain.tspmodels-gcp/channel/example_channel.tspmodels-gcp/channel/example_patch.tspmodels-gcp/channel/example_post.tspmodels-gcp/channel/model.tspmodels-gcp/cluster/example_cluster.tspmodels-gcp/cluster/example_patch.tspmodels-gcp/cluster/example_post.tspmodels-gcp/cluster/model.tspmodels-gcp/nodepool/example_nodepool.tspmodels-gcp/nodepool/example_patch.tspmodels-gcp/nodepool/example_post.tspmodels-gcp/nodepool/model.tspmodels-gcp/version/example_patch.tspmodels-gcp/version/example_post.tspmodels-gcp/version/example_version.tspmodels-gcp/version/model.tspmodels/compatibility/README.mdpackage.jsonschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlschemas/schemas.goservices/channels.tspservices/versions.tspshared/models/clusters/model.tspshared/models/common/model.tspshared/models/nodepools/model.tspshared/models/resource/example_patch.tspshared/models/resource/example_post.tspshared/models/resource/example_resource.tspshared/models/resource/model.tspshared/models/statuses/example_adapter_status.tspshared/models/statuses/model.tspshared/services/clusters.tspshared/services/nodepools.tspshared/services/resources.tspshared/services/statuses.tsp
💤 Files with no reviewable changes (25)
- models-gcp/nodepool/model.tsp
- models-gcp/nodepool/example_patch.tsp
- aliases.tsp
- schemas/core/swagger.yaml
- models-gcp/version/model.tsp
- models-gcp/cluster/example_post.tsp
- services/versions.tsp
- schemas/gcp/openapi.yaml
- models-gcp/cluster/model.tsp
- models-gcp/cluster/example_cluster.tsp
- services/channels.tsp
- models-gcp/channel/example_post.tsp
- models-gcp/channel/example_patch.tsp
- models-gcp/channel/model.tsp
- models-gcp/version/example_post.tsp
- models-gcp/nodepool/example_nodepool.tsp
- aliases-core.tsp
- models-gcp/cluster/example_patch.tsp
- models/compatibility/README.md
- models-gcp/channel/example_channel.tsp
- models-gcp/version/example_version.tsp
- schemas/gcp/swagger.yaml
- models-gcp/nodepool/example_post.tsp
- models-gcp/version/example_patch.tsp
- aliases-gcp.tsp
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)
shared/services/statuses.tsp (1)
37-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore auth on
NodePoolStatusesto avoid unintended public access.
@useAuth(HyperFleet.BearerAuth)was removed from this interface, which leavesGET /clusters/{cluster_id}/nodepools/{nodepool_id}/statusesunauthenticated while adjacent status endpoints remain protected.Suggested fix
`@tag`("NodePool statuses") `@route`("/clusters/{cluster_id}/nodepools/{nodepool_id}/statuses") +@useAuth(HyperFleet.BearerAuth) interface NodePoolStatuses{🤖 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 `@shared/services/statuses.tsp` around lines 37 - 40, The NodePoolStatuses interface lost its authentication decorator causing GET /clusters/{cluster_id}/nodepools/{nodepool_id}/statuses to be publicly accessible; restore the auth by re-adding the `@useAuth`(HyperFleet.BearerAuth) decorator above the NodePoolStatuses interface (the same place other status interfaces are protected) so the NodePoolStatuses interface and its route are guarded by the HyperFleet.BearerAuth scheme.
♻️ Duplicate comments (1)
main.tsp (1)
33-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCI blocker: version extraction from
@infostill failingPipeline still fails to parse version from this block (
Failed to extract version from main.tsp). This remains a release-gating issue; please align the@infoformatting with the extractor or update the extractor to parse the current layout.Compatible formatting option
-@info(#{ - version: "1.0.18", +@info(#{ version: "1.0.18", contact: #{🤖 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 33 - 35, The CI fails because the pipeline extractor can't read the version from the `@info` block (the "version" key inside `@info` is laid out in a way the extractor doesn't handle). Fix by either (A) normalizing the `@info` block so the version appears in the extractor's expected format (e.g., move/version key to the exact single-line/quoted format the extractor expects under `@info`) or (B) update the extractor code (the function that reads `@info` / extractVersion or parseInfoBlock) to accept the current #{} nested layout and robustly parse the "version" token; choose one approach and make sure the "version" string under `@info` is discoverable by the extractor.
🤖 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/models/cluster/example_cluster.tsp`:
- Around line 1-3: Add a namespace declaration immediately after the import
block in example_cluster.tsp (e.g., namespace core.models.cluster) so the file
follows the rule that imports come first then namespace; update or create the
namespace line to match the module's logical path and ensure subsequent type
definitions are inside that namespace (find the import block and insert the
namespace declaration directly beneath it, then move/confirm types like any
cluster interfaces or models are declared under that namespace).
In `@core/models/nodepool/example_post.tsp`:
- Around line 1-2: The file contains only imports (import "./model.tsp"; import
"../../../shared/models/nodepools/model.tsp";) but is missing the required
namespace declaration; add a namespace declaration immediately after those
import statements (e.g., add a line like `namespace <AppropriateName>;`) using
the same namespace name convention as other TypeSpec files in this module so
imports remain first and the namespace follows.
In `@package.json`:
- Around line 18-19: package.json currently pins "`@typespec/rest`" and
"`@typespec/versioning`" to ^0.81.0 which conflicts with other `@typespec/`*
packages; update the dependency entries for "`@typespec/rest`" and
"`@typespec/versioning`" to "^1.6.0" in package.json (keeping the same dependency
type), then run your package manager (npm install / yarn install / pnpm install)
to update the lockfile so the project uses the consistent TypeSpec ^1.6.0 set.
In `@shared/models/statuses/example_adapter_status.tsp`:
- Around line 1-2: This file has imports (import "./model.tsp"; import
"../common/model.tsp";) but is missing the required TypeSpec namespace
declaration immediately after them; add a namespace declaration line (e.g.,
namespace <your_namespace>; ) directly following the import statements so the
file follows the rule "imports first, then namespace" and ensure the namespace
name matches your project's naming conventions and other .tsp files.
---
Outside diff comments:
In `@shared/services/statuses.tsp`:
- Around line 37-40: The NodePoolStatuses interface lost its authentication
decorator causing GET /clusters/{cluster_id}/nodepools/{nodepool_id}/statuses to
be publicly accessible; restore the auth by re-adding the
`@useAuth`(HyperFleet.BearerAuth) decorator above the NodePoolStatuses interface
(the same place other status interfaces are protected) so the NodePoolStatuses
interface and its route are guarded by the HyperFleet.BearerAuth scheme.
---
Duplicate comments:
In `@main.tsp`:
- Around line 33-35: The CI fails because the pipeline extractor can't read the
version from the `@info` block (the "version" key inside `@info` is laid out in a
way the extractor doesn't handle). Fix by either (A) normalizing the `@info` block
so the version appears in the extractor's expected format (e.g., move/version
key to the exact single-line/quoted format the extractor expects under `@info`) or
(B) update the extractor code (the function that reads `@info` / extractVersion or
parseInfoBlock) to accept the current #{} nested layout and robustly parse the
"version" token; choose one approach and make sure the "version" string under
`@info` is discoverable by the extractor.
🪄 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: ccd75fa4-64cc-43d8-b6d9-12fac1268546
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (60)
.github/workflows/ci.yml.github/workflows/release.ymlCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdREADME.mdRELEASING.mdaliases-core.tspaliases-gcp.tspaliases.tspbuild-schema.shcore/models/cluster/example_cluster.tspcore/models/cluster/example_patch.tspcore/models/cluster/example_post.tspcore/models/cluster/model.tspcore/models/nodepool/example_nodepool.tspcore/models/nodepool/example_patch.tspcore/models/nodepool/example_post.tspcore/models/nodepool/model.tspcore/services/force-delete-internal.tspcore/services/statuses-internal.tspmain.tspmodels-gcp/channel/example_channel.tspmodels-gcp/channel/example_patch.tspmodels-gcp/channel/example_post.tspmodels-gcp/channel/model.tspmodels-gcp/cluster/example_cluster.tspmodels-gcp/cluster/example_patch.tspmodels-gcp/cluster/example_post.tspmodels-gcp/cluster/model.tspmodels-gcp/nodepool/example_nodepool.tspmodels-gcp/nodepool/example_patch.tspmodels-gcp/nodepool/example_post.tspmodels-gcp/nodepool/model.tspmodels-gcp/version/example_patch.tspmodels-gcp/version/example_post.tspmodels-gcp/version/example_version.tspmodels-gcp/version/model.tspmodels/compatibility/README.mdpackage.jsonschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlschemas/schemas.goservices/channels.tspservices/versions.tspshared/models/clusters/model.tspshared/models/common/model.tspshared/models/nodepools/model.tspshared/models/resource/example_patch.tspshared/models/resource/example_post.tspshared/models/resource/example_resource.tspshared/models/resource/model.tspshared/models/statuses/example_adapter_status.tspshared/models/statuses/model.tspshared/services/clusters.tspshared/services/nodepools.tspshared/services/resources.tspshared/services/statuses.tsp
💤 Files with no reviewable changes (25)
- models-gcp/channel/example_patch.tsp
- schemas/core/swagger.yaml
- models-gcp/nodepool/example_post.tsp
- models-gcp/channel/example_post.tsp
- aliases.tsp
- models/compatibility/README.md
- models-gcp/version/model.tsp
- aliases-core.tsp
- services/versions.tsp
- models-gcp/nodepool/example_patch.tsp
- models-gcp/channel/model.tsp
- models-gcp/nodepool/model.tsp
- models-gcp/version/example_patch.tsp
- models-gcp/cluster/example_cluster.tsp
- models-gcp/cluster/model.tsp
- services/channels.tsp
- models-gcp/channel/example_channel.tsp
- models-gcp/cluster/example_patch.tsp
- aliases-gcp.tsp
- schemas/gcp/openapi.yaml
- models-gcp/nodepool/example_nodepool.tsp
- schemas/gcp/swagger.yaml
- models-gcp/version/example_version.tsp
- models-gcp/cluster/example_post.tsp
- models-gcp/version/example_post.tsp
rh-amarin
force-pushed
the
remove-gcp-from-core
branch
from
9f365a7 to
33d57f0
Compare
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 (4)
.github/workflows/release.yml (1)
39-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTag-based skip logic can permanently block release creation after partial failure.
Using tag existence as the global skip gate means a failed
Create GitHub Releaseafter tag push cannot be recovered by rerun (skip=trueshort-circuits all later steps). Gate on release existence (or separate tag creation from release creation) instead of tag existence alone.Suggested adjustment
- - name: Check if release already exists - id: check_tag + - name: Check if GitHub release already exists + id: check_release env: TAG: ${{ steps.version.outputs.tag }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - git fetch --tags - if git rev-parse "$TAG" >/dev/null 2>&1; then - echo "Tag $TAG already exists — skipping release" + if gh release view "$TAG" >/dev/null 2>&1; then + echo "Release $TAG already exists — skipping release" echo "skip=true" >> "$GITHUB_OUTPUT" else echo "skip=false" >> "$GITHUB_OUTPUT" fi - - name: Build core schema - if: steps.check_tag.outputs.skip == 'false' + - name: Build core schema + if: steps.check_release.outputs.skip == 'false' run: ./build-schema.shAlso applies to: 61-73
🤖 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 @.github/workflows/release.yml around lines 39 - 50, The workflow currently uses the check_tag step (env TAG) to skip when a git tag exists, which blocks reruns if release creation failed; change the gating to check for a GitHub release instead (or split tag push and release creation into two distinct steps) by replacing the tag-existence test in the check_tag logic with a release-existence query against the GitHub API (or add a new step like check_release that calls GET /repos/{owner}/{repo}/releases/tags/{tag}) and use that step's output to set skip; update downstream consumers (e.g., the Create GitHub Release step) to rely on the release-existence output so a missing release can be retried even if the tag already exists.README.md (1)
158-177:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate “Adding a New Service” paths to the shared/core layout.
The section still points contributors to
services/new-service.tspand./services/new-service.tsp, which conflicts with this repo’s split structure and will send new endpoints to the wrong location.Suggested correction
-1. Create a new service file: `services/new-service.tsp` +1. Create a new service file under the correct scope: + - Public/shared endpoint: `shared/services/new-service.tsp` + - Internal/core-only endpoint: `core/services/new-service.tsp` @@ - import "../models/common/model.tsp"; + import "../models/common/model.tsp"; @@ -2. Import the new service in `main.tsp`: +2. Import the new service in `main.tsp` using its scoped path: @@ - import "./services/new-service.tsp"; + import "./shared/services/new-service.tsp"; + // or: import "./core/services/new-service.tsp";As per coding guidelines
**: Focus on major issues impacting performance, readability, maintainability and security. 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 158 - 177, The README's "Adding a New Service" section incorrectly points to services/new-service.tsp and ./services/new-service.tsp which conflicts with the repo's split layout; update the example to create the service under the shared/core layout (e.g., shared/core/services/new-service.tsp) and update the import in main.tsp to the corresponding shared/core path, keeping the same interface name NewService and route("/new-resource") so examples match the repository structure and will place new endpoints in the correct shared/core location.core/services/force-delete-internal.tsp (1)
12-60: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winSplit this service into one-resource-per-file.
ClustersForceDeleteandNodePoolsForceDeleteare defined in the same service file, which violates the service-file boundary rule and makes ownership/maintenance harder.As per coding guidelines
{shared,core}/services/**/*.tsp: Keep TypeSpec service files focused with one resource per service file.🤖 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 `@core/services/force-delete-internal.tsp` around lines 12 - 60, Split the combined service into two TypeSpec service files so each resource has its own service: move the ClustersForceDelete interface (including its `@tag`("Clusters"), `@route`("/clusters") and forceDeleteCluster operation) into a new file and move the NodePoolsForceDelete interface (including `@tag`("NodePools"), `@route`("/clusters/{cluster_id}/nodepools") and forceDeleteNodePool) into a separate file; preserve the same interface names, decorators, request/response types (ForceDeleteRequest, statusCode 204, and the same error union), update any imports or references to these interfaces elsewhere, and ensure each new file follows the project service-file pattern ({shared,core}/services/**/*.tsp) and compiles.shared/services/statuses.tsp (1)
37-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore auth on
NodePoolStatusesGET endpoints.
@useAuth(HyperFleet.BearerAuth)was dropped fromNodePoolStatuses, which makes nodepool status reads unauthenticated in the generated contract.Suggested fix
`@tag`("NodePool statuses") `@route`("/clusters/{cluster_id}/nodepools/{nodepool_id}/statuses") +@useAuth(HyperFleet.BearerAuth) interface NodePoolStatuses{🤖 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 `@shared/services/statuses.tsp` around lines 37 - 40, The NodePoolStatuses interface's routes lost authentication — restore the `@useAuth`(HyperFleet.BearerAuth) decorator on the NodePoolStatuses contract so all GET endpoints under interface NodePoolStatuses (route "/clusters/{cluster_id}/nodepools/{nodepool_id}/statuses") require bearer auth; add the `@useAuth`(HyperFleet.BearerAuth) annotation immediately above the `@tag/`@route declarations for NodePoolStatuses to re-enable authenticated reads.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 10-18: The CHANGELOG entry for [1.0.18] only has "### Changed";
update the section to include the full Keep a Changelog heading set by adding
"### Added", "### Changed", "### Fixed", and "### Removed" subsections under the
[1.0.18] header (populate "Changed" with the existing bullets and add the other
three with either relevant bullets or a single "- None." placeholder), ensuring
the version header "[1.0.18] - 2026-05-26" remains unchanged and the overall
format matches other releases in CHANGELOG.md.
In `@package.json`:
- Around line 18-19: Update the two package entries "`@typespec/rest`" and
"`@typespec/versioning`" in package.json so their versions match the repository
TypeSpec policy; change their version strings from "^0.81.0" to "^1.6.0" (the
same version used by "`@typespec/compiler`", "`@typespec/http`",
"`@typespec/openapi`", and "`@typespec/openapi3`") to prevent dependency-version
drift.
In `@README.md`:
- Around line 94-96: The README table is inaccurate:
`core/services/statuses-internal.tsp` is marked "❌ No (opt-in)" but the current
core schema includes the PUT (write) status operations; update the table row for
`core/services/statuses-internal.tsp` to reflect that PUT (write) is included in
the core build (e.g., change to "✅ Yes (included)") and adjust the descriptive
note accordingly, and then re-validate the change against HyperFleet
architecture standards to ensure wording matches project conventions.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 39-50: The workflow currently uses the check_tag step (env TAG) to
skip when a git tag exists, which blocks reruns if release creation failed;
change the gating to check for a GitHub release instead (or split tag push and
release creation into two distinct steps) by replacing the tag-existence test in
the check_tag logic with a release-existence query against the GitHub API (or
add a new step like check_release that calls GET
/repos/{owner}/{repo}/releases/tags/{tag}) and use that step's output to set
skip; update downstream consumers (e.g., the Create GitHub Release step) to rely
on the release-existence output so a missing release can be retried even if the
tag already exists.
In `@core/services/force-delete-internal.tsp`:
- Around line 12-60: Split the combined service into two TypeSpec service files
so each resource has its own service: move the ClustersForceDelete interface
(including its `@tag`("Clusters"), `@route`("/clusters") and forceDeleteCluster
operation) into a new file and move the NodePoolsForceDelete interface
(including `@tag`("NodePools"), `@route`("/clusters/{cluster_id}/nodepools") and
forceDeleteNodePool) into a separate file; preserve the same interface names,
decorators, request/response types (ForceDeleteRequest, statusCode 204, and the
same error union), update any imports or references to these interfaces
elsewhere, and ensure each new file follows the project service-file pattern
({shared,core}/services/**/*.tsp) and compiles.
In `@README.md`:
- Around line 158-177: The README's "Adding a New Service" section incorrectly
points to services/new-service.tsp and ./services/new-service.tsp which
conflicts with the repo's split layout; update the example to create the service
under the shared/core layout (e.g., shared/core/services/new-service.tsp) and
update the import in main.tsp to the corresponding shared/core path, keeping the
same interface name NewService and route("/new-resource") so examples match the
repository structure and will place new endpoints in the correct shared/core
location.
In `@shared/services/statuses.tsp`:
- Around line 37-40: The NodePoolStatuses interface's routes lost authentication
— restore the `@useAuth`(HyperFleet.BearerAuth) decorator on the NodePoolStatuses
contract so all GET endpoints under interface NodePoolStatuses (route
"/clusters/{cluster_id}/nodepools/{nodepool_id}/statuses") require bearer auth;
add the `@useAuth`(HyperFleet.BearerAuth) annotation immediately above the
`@tag/`@route declarations for NodePoolStatuses to re-enable authenticated reads.
🪄 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: 832e3815-fdaa-474e-9f80-c57df94e33c7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (60)
.github/workflows/ci.yml.github/workflows/release.ymlCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdREADME.mdRELEASING.mdaliases-core.tspaliases-gcp.tspaliases.tspbuild-schema.shcore/models/cluster/example_cluster.tspcore/models/cluster/example_patch.tspcore/models/cluster/example_post.tspcore/models/cluster/model.tspcore/models/nodepool/example_nodepool.tspcore/models/nodepool/example_patch.tspcore/models/nodepool/example_post.tspcore/models/nodepool/model.tspcore/services/force-delete-internal.tspcore/services/statuses-internal.tspmain.tspmodels-gcp/channel/example_channel.tspmodels-gcp/channel/example_patch.tspmodels-gcp/channel/example_post.tspmodels-gcp/channel/model.tspmodels-gcp/cluster/example_cluster.tspmodels-gcp/cluster/example_patch.tspmodels-gcp/cluster/example_post.tspmodels-gcp/cluster/model.tspmodels-gcp/nodepool/example_nodepool.tspmodels-gcp/nodepool/example_patch.tspmodels-gcp/nodepool/example_post.tspmodels-gcp/nodepool/model.tspmodels-gcp/version/example_patch.tspmodels-gcp/version/example_post.tspmodels-gcp/version/example_version.tspmodels-gcp/version/model.tspmodels/compatibility/README.mdpackage.jsonschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlschemas/schemas.goservices/channels.tspservices/versions.tspshared/models/clusters/model.tspshared/models/common/model.tspshared/models/nodepools/model.tspshared/models/resource/example_patch.tspshared/models/resource/example_post.tspshared/models/resource/example_resource.tspshared/models/resource/model.tspshared/models/statuses/example_adapter_status.tspshared/models/statuses/model.tspshared/services/clusters.tspshared/services/nodepools.tspshared/services/resources.tspshared/services/statuses.tsp
💤 Files with no reviewable changes (25)
- models-gcp/version/example_version.tsp
- models-gcp/nodepool/example_nodepool.tsp
- models-gcp/nodepool/example_patch.tsp
- models-gcp/channel/example_post.tsp
- services/channels.tsp
- aliases-gcp.tsp
- models-gcp/channel/example_channel.tsp
- models-gcp/version/example_patch.tsp
- models-gcp/version/example_post.tsp
- models-gcp/nodepool/model.tsp
- models-gcp/channel/model.tsp
- models-gcp/channel/example_patch.tsp
- models-gcp/cluster/example_cluster.tsp
- models-gcp/nodepool/example_post.tsp
- schemas/core/swagger.yaml
- models-gcp/cluster/model.tsp
- models-gcp/cluster/example_patch.tsp
- models/compatibility/README.md
- models-gcp/version/model.tsp
- schemas/gcp/swagger.yaml
- services/versions.tsp
- models-gcp/cluster/example_post.tsp
- schemas/gcp/openapi.yaml
- aliases.tsp
- aliases-core.tsp
rafabene
reviewed
May 26, 2026
rafabene
reviewed
May 26, 2026
…ared - Move GCP-specific models out of this repo (now in hyperfleet-api-spec-gcp) - Restructure sources into core/ (core-specific) and shared/ (cross-provider) - Add generic Resource model and CRUD service (from main reconciliation) - Add force-delete endpoints for clusters, nodepools, and resources (internal) - Add ResourceStatusesInternal for adapter resource status reporting - Simplify build script: single provider (core only), openapi output only - Add npm sub-path exports to enable downstream package imports Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> chore: fix CI/CD workflow path for version extraction main.tsp moved from core/main.tsp to root main.tsp as part of restructuring. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rh-amarin
force-pushed
the
remove-gcp-from-core
branch
from
33d57f0 to
17ab4a9
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)
schemas/core/openapi.yaml (1)
621-659:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd BearerAuth to
getNodePoolsStatusesand regenerate schema.Line 621–659 is missing a
securityblock forGET /clusters/{cluster_id}/nodepools/{nodepool_id}/statuses. CI already reports generated output differs here, so the committed schema is both out-of-sync and currently under-specifies auth for this endpoint.Suggested fix
/api/hyperfleet/v1/clusters/{cluster_id}/nodepools/{nodepool_id}/statuses: @@ get: @@ tags: - NodePool statuses + security: + - BearerAuth: []Then run
./build-schema.shand commit regeneratedschemas/core/openapi.yaml.🤖 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 - 659, The getNodePoolsStatuses operation is missing the security requirement; add a security block under the GET operation for operationId getNodePoolsStatuses that enforces bearer authentication (e.g., security: - bearerAuth: []) for the path GET /clusters/{cluster_id}/nodepools/{nodepool_id}/statuses, then run ./build-schema.sh to regenerate schemas/core/openapi.yaml and commit the updated generated file so the spec and generated output stay in sync.
🤖 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 `@schemas/core/openapi.yaml`:
- Around line 621-659: The getNodePoolsStatuses operation is missing the
security requirement; add a security block under the GET operation for
operationId getNodePoolsStatuses that enforces bearer authentication (e.g.,
security: - bearerAuth: []) for the path GET
/clusters/{cluster_id}/nodepools/{nodepool_id}/statuses, then run
./build-schema.sh to regenerate schemas/core/openapi.yaml and commit the updated
generated file so the spec and generated output stay in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 213d6c9f-4667-424e-b506-63c355384a8e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (60)
.github/workflows/ci.yml.github/workflows/release.ymlCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdREADME.mdRELEASING.mdaliases-core.tspaliases-gcp.tspaliases.tspbuild-schema.shcore/models/cluster/example_cluster.tspcore/models/cluster/example_patch.tspcore/models/cluster/example_post.tspcore/models/cluster/model.tspcore/models/nodepool/example_nodepool.tspcore/models/nodepool/example_patch.tspcore/models/nodepool/example_post.tspcore/models/nodepool/model.tspcore/services/force-delete-internal.tspcore/services/statuses-internal.tspmain.tspmodels-gcp/channel/example_channel.tspmodels-gcp/channel/example_patch.tspmodels-gcp/channel/example_post.tspmodels-gcp/channel/model.tspmodels-gcp/cluster/example_cluster.tspmodels-gcp/cluster/example_patch.tspmodels-gcp/cluster/example_post.tspmodels-gcp/cluster/model.tspmodels-gcp/nodepool/example_nodepool.tspmodels-gcp/nodepool/example_patch.tspmodels-gcp/nodepool/example_post.tspmodels-gcp/nodepool/model.tspmodels-gcp/version/example_patch.tspmodels-gcp/version/example_post.tspmodels-gcp/version/example_version.tspmodels-gcp/version/model.tspmodels/compatibility/README.mdpackage.jsonschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yamlschemas/schemas.goservices/channels.tspservices/versions.tspshared/models/clusters/model.tspshared/models/common/model.tspshared/models/nodepools/model.tspshared/models/resource/example_patch.tspshared/models/resource/example_post.tspshared/models/resource/example_resource.tspshared/models/resource/model.tspshared/models/statuses/example_adapter_status.tspshared/models/statuses/model.tspshared/services/clusters.tspshared/services/nodepools.tspshared/services/resources.tspshared/services/statuses.tsp
💤 Files with no reviewable changes (25)
- models-gcp/cluster/example_post.tsp
- aliases-gcp.tsp
- schemas/core/swagger.yaml
- aliases.tsp
- models-gcp/cluster/model.tsp
- models-gcp/nodepool/model.tsp
- models-gcp/channel/example_patch.tsp
- models-gcp/cluster/example_cluster.tsp
- models-gcp/channel/example_channel.tsp
- models/compatibility/README.md
- schemas/gcp/openapi.yaml
- models-gcp/channel/model.tsp
- models-gcp/channel/example_post.tsp
- models-gcp/nodepool/example_patch.tsp
- models-gcp/version/model.tsp
- models-gcp/version/example_patch.tsp
- models-gcp/cluster/example_patch.tsp
- models-gcp/nodepool/example_post.tsp
- models-gcp/nodepool/example_nodepool.tsp
- services/versions.tsp
- models-gcp/version/example_version.tsp
- models-gcp/version/example_post.tsp
- aliases-core.tsp
- schemas/gcp/swagger.yaml
- services/channels.tsp
…uses GET Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rafabene
reviewed
May 26, 2026
| # Step 3: Compile TypeSpec to generate OpenAPI schema | ||
| echo -e "${YELLOW}Step 3: Compiling TypeSpec...${NC}" | ||
| TEMP_OUTPUT_DIR="tsp-output-${PROVIDER}" | ||
| echo -e "${YELLOW}Step 3: Compiling TypeSpec from core/main.tsp...${NC}" |
Contributor
There was a problem hiding this comment.
Tip
nit — non-blocking suggestion
Category: Bug
The log message says core/main.tsp but the compile command below uses ./main.tsp (root level). Mismatch will confuse folks debugging builds.
Suggested change
| echo -e "${YELLOW}Step 3: Compiling TypeSpec from core/main.tsp...${NC}" | |
| echo -e "${YELLOW}Step 3: Compiling TypeSpec from main.tsp...${NC}" |
rafabene
reviewed
May 26, 2026
Comment on lines
+5
to
+7
| "exports": { | ||
| "./*": "./*" | ||
| }, |
Contributor
There was a problem hiding this comment.
Tip
nit — non-blocking suggestion
Category: Security
The "./*": "./*" wildcard exports everything in the repo — CI workflows, CLAUDE.md, build scripts, etc. If downstream consumers only need shared/ models and maybe core/ types, consider narrowing the export map:
Suggested change
| "exports": { | |
| "./*": "./*" | |
| }, | |
| "./shared/*": "./shared/*", | |
| "./core/*": "./core/*" |
This prevents accidental imports of internal files while still supporting the intended npm consumption pattern.
Contributor
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rafabene 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 78199f6
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.
hyperfleet-api-spec-gcpThe changes convert this to be a npm package that can be imported by a partner to build their own API contract leveraging common structures.
To test the GCP contract
It should generate the
openapi.yamlandswagger.yamlfiles