HYPERFLEET-500 - Introduce oapi-codegen #33
Conversation
|
Skipping CI for Draft Pull Request. |
WalkthroughReplaces Docker-based OpenAPI generation with direct oapi-codegen invocation and adds oapi-codegen as a managed bingo tool (.bingo/* changes). Removes Dockerfile.openapi, its dockerignore, and openapitools.json; adds openapi/oapi-codegen.yaml and .bingo module for oapi-codegen. Makefile updated to call $(OAPI_CODEGEN) to generate pkg/api/openapi. Updates go.mod, regenerates OpenAPI types (pointerized Kind, renamed enums, expanded explicit schemas), adapts presenters/handlers, removes embedded spec accessor, and updates tests and Dockerfile to the new generation/runtime flow. Sequence Diagram(s)sequenceDiagram
actor Developer
participant Makefile
participant Bingo as .bingo target
participant OAPI as oapi-codegen
participant Spec as openapi/openapi.yaml
participant Generated as pkg/api/openapi
participant GoBuild as go build / test
Developer->>Makefile: make generate
Makefile->>Bingo: ensure $(OAPI_CODEGEN) target
Bingo->>OAPI: build/install oapi-codegen binary
Makefile->>OAPI: run oapi-codegen with openapi/oapi-codegen.yaml + Spec
OAPI->>Spec: read OpenAPI spec
OAPI-->>Generated: write generated client/models
Makefile->>GoBuild: run go build / go test using Generated
GoBuild-->>Developer: build/test results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
⏺ OpenAPI Code Generation Comparison Overview
Type Style: Strengths:
Weaknesses:
Type Style: Strengths:
Weaknesses:
Type Style: Strengths:
Weaknesses:
Comparison Summary
Recommendation For your use case (types + client only):
|
| JobName *string `json:"job_name,omitempty"` | ||
| JobNamespace *string `json:"job_namespace,omitempty"` | ||
| StartedTime *time.Time `json:"started_time,omitempty"` | ||
| } |
There was a problem hiding this comment.
Is it better to define AdapterMetadata under pkg/api/adapter_status_types.go instead of here?
There was a problem hiding this comment.
I defined a struct in api as you suggest
But, the current openapi metadata struct still has to be defined since is a loose type in the openapi contract
We may want to revisit the definition of metadata in the api-spec project
But that will be better in a separate ticket
![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 (4)
openapi/openapi.yaml (1)
78-105: SearchParams on single-resource endpoint may be unintended.Line 83 adds
SearchParamstogetClusterById, but search/filter parameters are typically only meaningful for list endpoints that return multiple items. For a single-resource GET by ID, the search parameter would have no effect.Consider removing
SearchParamsfrom this endpoint unless there's a specific use case (e.g., field projection).test/helper.go (1)
448-456: OpenapiError method exists but is not being used anywhere in the codebase.The method signature change to accept
[]byteis correct, but the verification reveals no test callsites actually invokehelper.OpenapiError(). Integration tests directly unmarshal response bodies usingjson.Unmarshal(resp.Body(), &errorResponse)instead of calling this helper method. Either this is dead code, or the refactoring to use this method in tests is incomplete and needs to be completed.pkg/api/presenters/cluster_test.go (1)
393-398: Verify the pointer comparison for Kind field.Line 395 compares
result.Kind(a*string) withoriginalReq.Kind(also a*string). Gomega'sEqualmatcher will compare the pointer values, not the underlying strings. Since these are different pointers (one from the original request, one newly allocated inPresentCluster), this assertion may fail unexpectedly.🐛 Proposed fix to dereference the pointer
// Verify data integrity Expect(*result.Id).To(Equal("cluster-roundtrip-123")) - Expect(result.Kind).To(Equal(originalReq.Kind)) + Expect(*result.Kind).To(Equal(*originalReq.Kind)) Expect(result.Name).To(Equal(originalReq.Name))test/integration/clusters_test.go (1)
650-660: Missing nil check onlistResp.JSON200before iterating over.Items.Line 652 assigns
listfromlistResp.JSON200, but there's no nil check before accessinglist.Itemson line 656. This is inconsistent with other tests in this file that perform nil checks.🐛 Proposed fix
listResp, err := client.GetClustersWithResponse(ctx, params, test.WithAuthToken(ctx)) Expect(err).NotTo(HaveOccurred(), "Failed to list clusters with orderBy desc") list := listResp.JSON200 + Expect(list).NotTo(BeNil()) // Find our test clusters in the response
🤖 Fix all issues with AI agents
In @test/integration/clusters_test.go:
- Around line 299-303: The test accesses getResp.JSON200.Spec without ensuring
JSON200 is non-nil; add a nil-check (e.g.,
Expect(getResp.JSON200).NotTo(BeNil()) or if getResp.JSON200 == nil { Fail(...)
}) immediately after calling GetClusterByIdWithResponse and before accessing
Spec so the test fails with a clear message instead of panicking; keep the
existing assertions (Err check and using *resp.JSON201.Id) and then assert
len(getResp.JSON200.Spec) once the nil-check passes.
- Around line 232-235: The test accesses getResp.JSON200.Name without ensuring
JSON200 is non-nil; add an explicit nil assertion before accessing Name by
checking Expect(getResp.JSON200).NotTo(BeNil()) (or an equivalent nil check)
immediately after the err check from client.GetClusterByIdWithResponse, then
assert Expect(getResp.JSON200.Name).To(Equal("duplicate-name-test")); this
ensures the test fails clearly if the response body is absent instead of
panicking.
- Around line 196-204: The test accesses resp.JSON200.Items without checking
that resp.JSON200 is non-nil after calling GetClustersWithResponse; add a
nil-check on resp.JSON200 (and fail the test or use Expect to assert
resp.StatusCode() == 200 or resp.JSON200 != nil) before referencing .Items so
you avoid a nil pointer dereference — update the GetClustersWithResponse
response handling in the test to assert/handle non-200 responses (e.g.,
Expect(resp.JSON200).NotTo(BeNil()) or Expect(resp.StatusCode()).To(Equal(200))
) prior to using resp.JSON200.Items.
🧹 Nitpick comments (7)
pkg/handlers/validation_test.go (2)
118-121: Consider usingopenapi.PtrStringfor consistency.Other tests in this file use
openapi.PtrString()(lines 101, 132, 144), but this loop uses&kind. While functionally correct in Go 1.22+, using the helper would maintain consistency.♻️ Suggested change
for _, kind := range invalidKinds { req := openapi.ClusterCreateRequest{ - Kind: &kind, + Kind: openapi.PtrString(kind), }
128-138: Consider adding a test case fornilKind.This test covers empty string (
""), but there's no test for whenKindisnil(pointer not set). Depending on the validator implementation,nilvs empty string may produce different behavior.📝 Suggested test case
func TestValidateKind_Nil(t *testing.T) { RegisterTestingT(t) req := openapi.ClusterCreateRequest{ Kind: nil, } validator := validateKind(&req, "Kind", "kind", "Cluster") err := validator() Expect(err).ToNot(BeNil()) Expect(err.Reason).To(ContainSubstring("required")) }pkg/handlers/openapi.go (1)
63-65: Consider migrating to structured slog-based logger.
NewOCMLogger(ctx)is deprecated. When convenient, consider switching tologger.With(ctx, ...)for structured, context-aware logging. This applies to line 75 as well. Based on learnings, this is a known deprecation but can be addressed in a follow-up.Makefile (1)
226-243: Generate target successfully migrates to oapi-codegen.The refactored target correctly:
- Depends on the bingo-managed
$(OAPI_CODEGEN)tool- Cleans and recreates the output directory
- Generates types via config file
- Creates helper pointer functions
The inline
helpers.gogeneration is functional. As an optional improvement, consider extracting the helpers to a committed template file to improve readability and make the helpers easier to extend.♻️ Optional: Extract helpers to a template file
Create
openapi/helpers.go.tmpl:package openapi // Ptr returns a pointer to the given value. func Ptr[T any](v T) *T { return &v } // PtrString returns a pointer to the given string. func PtrString(v string) *string { return &v } // PtrInt32 returns a pointer to the given int32. func PtrInt32(v int32) *int32 { return &v }Then simplify the Makefile:
generate: $(OAPI_CODEGEN) rm -rf pkg/api/openapi mkdir -p pkg/api/openapi $(OAPI_CODEGEN) --config openapi/oapi-codegen.yaml openapi/openapi.yaml - @printf '%s\n' \ - 'package openapi' \ - '' \ - '// Ptr returns a pointer to the given value.' \ - 'func Ptr[T any](v T) *T { return &v }' \ - '' \ - '// PtrString returns a pointer to the given string.' \ - 'func PtrString(v string) *string { return &v }' \ - '' \ - '// PtrInt32 returns a pointer to the given int32.' \ - 'func PtrInt32(v int32) *int32 { return &v }' \ - > pkg/api/openapi/helpers.go + cp openapi/helpers.go.tmpl pkg/api/openapi/helpers.gopkg/handlers/node_pool.go (1)
39-41: Consider disabling standalone nodepool creation or requiring owner context.The comment acknowledges that standalone nodepools (without an owner cluster) may not be a supported use case. Passing an empty
ownerIDcould lead to orphaned resources or break referential integrity constraints.The
createdByupdate to"[email protected]"is consistent with other handlers.If standalone nodepool creation isn't supported, consider returning
errors.NotImplemented("standalone nodepool creation")or requiring the owner cluster ID in the request body.pkg/handlers/cluster_nodepools_test.go (1)
133-142: Consider adding missing fields for consistency.The "different cluster" error case returns a minimal NodePool without
Spec,Labels,StatusConditions,CreatedBy, orUpdatedBy. While this works because the presenter isn't invoked on the error path, adding these fields would maintain consistency with the success case mock and prevent issues if the test logic changes.♻️ Proposed consistency fix
mockNodePoolSvc.EXPECT().Get(gomock.Any(), nodePoolID).Return(&api.NodePool{ Meta: api.Meta{ ID: nodePoolID, CreatedTime: now, UpdatedTime: now, }, - Kind: "NodePool", - Name: "test-nodepool", - OwnerID: "different-cluster-789", // Different cluster + Kind: "NodePool", + Name: "test-nodepool", + OwnerID: "different-cluster-789", // Different cluster + Spec: []byte("{}"), + Labels: []byte("{}"), + StatusConditions: []byte("[]"), + CreatedBy: "[email protected]", + UpdatedBy: "[email protected]", }, nil)pkg/api/presenters/adapter_status.go (1)
114-139: Consider extracting the inline metadata struct to reduce duplication.The anonymous struct definition for
openapiMetadatais duplicated verbatim (lines 114-121 and 124-131). If the generated OpenAPI type uses an inline anonymous struct, this is unavoidable. However, if possible, consider defining a local type alias or using direct assignment to reduce the ~30 lines of duplication.♻️ Potential simplification (if OpenAPI schema allows)
If the generated type permits, you could simplify:
// If openapi.AdapterStatus.Metadata accepts *api.AdapterStatusMetadata or similar if metadata != nil { openapiMetadata = &openapi.AdapterStatusMetadata{ Attempt: metadata.Attempt, CompletedTime: metadata.CompletedTime, // ... etc } }Otherwise, the current approach is correct given the anonymous struct constraint.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.bingo/oapi-codegen.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum
📒 Files selected for processing (34)
.bingo/Variables.mk.bingo/oapi-codegen.mod.bingo/variables.envDockerfile.openapiDockerfile.openapi.dockerignoreMakefilego.modopenapi/oapi-codegen.yamlopenapi/openapi.yamlopenapitools.jsonpkg/api/adapter_status_types.gopkg/api/openapi_embed.gopkg/api/presenters/adapter_status.gopkg/api/presenters/adapter_status_test.gopkg/api/presenters/cluster.gopkg/api/presenters/cluster_test.gopkg/api/presenters/node_pool.gopkg/api/presenters/node_pool_test.gopkg/errors/errors.gopkg/handlers/cluster.gopkg/handlers/cluster_nodepools.gopkg/handlers/cluster_nodepools_test.gopkg/handlers/node_pool.gopkg/handlers/openapi.gopkg/handlers/validation_test.gotest/factories/clusters.gotest/factories/node_pools.gotest/helper.gotest/integration/adapter_status_test.gotest/integration/api_contract_test.gotest/integration/clusters_test.gotest/integration/node_pools_test.gotest/integration/search_field_mapping_test.gotest/registration.go
💤 Files with no reviewable changes (4)
- pkg/api/openapi_embed.go
- openapitools.json
- Dockerfile.openapi.dockerignore
- Dockerfile.openapi
🧰 Additional context used
📓 Path-based instructions (9)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Use structured logging via logger.NewOCMLogger(ctx) with context-aware log fields including operation ID, account ID, and transaction ID
Files:
pkg/handlers/node_pool.gotest/registration.gopkg/handlers/cluster.gotest/factories/clusters.gopkg/api/adapter_status_types.gotest/integration/api_contract_test.gopkg/handlers/cluster_nodepools.gotest/helper.gopkg/api/presenters/adapter_status_test.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.gotest/factories/node_pools.gotest/integration/node_pools_test.gopkg/handlers/openapi.gotest/integration/search_field_mapping_test.gopkg/api/presenters/adapter_status.gopkg/errors/errors.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.gopkg/api/presenters/node_pool.gotest/integration/adapter_status_test.gopkg/api/presenters/node_pool_test.gotest/integration/clusters_test.go
pkg/handlers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/handlers/**/*.go: HTTP handlers must follow the pattern: parse request → call service/DAO → handle errors → send response with appropriate HTTP status codes
Database transactions are automatically managed by TransactionMiddleware for all HTTP requests; store session in context and auto-commit on success, rollback on error
Accept resource creation/update/delete requests via REST endpoints, validate, and persist to PostgreSQL
Accept status updates from adapters via POST /{resourceType}/{id}/statuses and store in adapter_statuses table
Serve resource data to Sentinel via GET /{resourceType} endpoints with pagination and filtering support
Health check endpoint at GET /healthcheck returns 200 OK when database is accessible; used by Kubernetes liveness/readiness probes
Prometheus metrics are exposed at GET /metrics endpoint for monitoring; configure metric collection via --label-metrics-inclusion-duration
Files:
pkg/handlers/node_pool.gopkg/handlers/cluster.gopkg/handlers/cluster_nodepools.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.gopkg/handlers/openapi.go
pkg/{dao,handlers}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
List operations must support pagination via ListArgs struct with limit, offset, and search parameters
Files:
pkg/handlers/node_pool.gopkg/handlers/cluster.gopkg/handlers/cluster_nodepools.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.gopkg/handlers/openapi.go
pkg/{handlers,services,dao}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Errors must use structured ServiceError type from pkg/errors/ with HttpCode, Code, and Reason fields; send via errors.SendError(w, r, err)
Files:
pkg/handlers/node_pool.gopkg/handlers/cluster.gopkg/handlers/cluster_nodepools.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.gopkg/handlers/openapi.go
pkg/{handlers,services,db}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use context values for passing request-scoped data (session, operation ID, user info) rather than function parameters or global state
Files:
pkg/handlers/node_pool.gopkg/handlers/cluster.gopkg/handlers/cluster_nodepools.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.gopkg/handlers/openapi.go
openapi/**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
openapi/**/*.yaml: Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Files:
openapi/oapi-codegen.yamlopenapi/openapi.yaml
test/factories/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use test data factories in test/factories/ to provide consistent, reusable test data with builder pattern (e.g., NewClusterBuilder().WithName().Build())
Files:
test/factories/clusters.gotest/factories/node_pools.go
pkg/api/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Files:
pkg/api/adapter_status_types.gopkg/api/presenters/adapter_status_test.gopkg/api/presenters/adapter_status.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.gopkg/api/presenters/node_pool.gopkg/api/presenters/node_pool_test.go
test/integration/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
test/integration/**/*_test.go: Integration tests must use Testcontainers to create isolated PostgreSQL instances for each test suite to ensure no state leakage
All API endpoints must have integration test coverage in test/integration/ including CRUD operations, pagination, search, error cases, and status aggregation
Files:
test/integration/api_contract_test.gotest/integration/node_pools_test.gotest/integration/search_field_mapping_test.gotest/integration/adapter_status_test.gotest/integration/clusters_test.go
🧠 Learnings (27)
📓 Common learnings
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: go.mod:32-36
Timestamp: 2026-01-13T04:19:50.012Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, the team has decided to keep OpenTelemetry core packages (go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/exporters/stdout/stdouttrace) at v1.38.0 rather than upgrading to v1.39.0 in PR #31.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/telemetry/otel.go:10-10
Timestamp: 2026-01-13T10:10:24.211Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository (PR #31), the team has decided to keep the OpenTelemetry semantic conventions package at v1.37.0 (go.opentelemetry.io/otel/semconv/v1.37.0) while using the SDK at v1.38.0 in pkg/telemetry/otel.go.
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Applied to files:
go.mod.bingo/variables.envopenapi/oapi-codegen.yaml.bingo/Variables.mktest/helper.go.bingo/oapi-codegen.modMakefilepkg/handlers/openapi.gopkg/errors/errors.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.gopkg/api/presenters/node_pool_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Applied to files:
go.mod.bingo/variables.envopenapi/oapi-codegen.yamltest/integration/api_contract_test.gotest/helper.go.bingo/oapi-codegen.modMakefilepkg/handlers/openapi.gopkg/api/presenters/adapter_status.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.gopkg/api/presenters/node_pool_test.gotest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Applied to files:
go.modopenapi/oapi-codegen.yaml.bingo/oapi-codegen.modMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Applied to files:
go.mod.bingo/variables.envopenapi/oapi-codegen.yamltest/helper.go.bingo/oapi-codegen.modMakefilepkg/handlers/openapi.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.gopkg/api/presenters/node_pool_test.gotest/integration/clusters_test.go
📚 Learning: 2026-01-08T09:40:10.138Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Applied to files:
go.mod.bingo/variables.envopenapi/oapi-codegen.yaml.bingo/oapi-codegen.modMakefilepkg/handlers/openapi.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.gopkg/api/presenters/node_pool_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/db/**/*.go : Use PostgreSQL 14.2 with GORM ORM for database operations
Applied to files:
go.mod
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to **/*.go : Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Applied to files:
go.mod.bingo/variables.env
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{db/migrations,dao}/**/*.go : NodePool owner_references.id must enforce foreign key relationship to parent cluster to maintain referential integrity
Applied to files:
pkg/handlers/node_pool.gopkg/handlers/cluster_nodepools.gopkg/handlers/cluster_nodepools_test.gotest/factories/node_pools.gotest/integration/node_pools_test.gopkg/api/presenters/node_pool.gopkg/api/presenters/node_pool_test.go
📚 Learning: 2026-01-13T04:16:08.743Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/middleware/masking.go:28-36
Timestamp: 2026-01-13T04:16:08.743Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, NewOCMLogger(ctx) is deprecated. Switch to the slog-based logger and use logger.With(ctx, ...) to produce structured, context-aware logs. Replace occurrences of NewOCMLogger(ctx) with logger.With(ctx, ...). Update imports to use the new logger and adjust any tests to expect structured logging behavior.
Applied to files:
pkg/handlers/node_pool.gotest/registration.gopkg/handlers/cluster.gotest/factories/clusters.gopkg/api/adapter_status_types.gotest/integration/api_contract_test.gopkg/handlers/cluster_nodepools.gotest/helper.gopkg/api/presenters/adapter_status_test.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.gotest/factories/node_pools.gotest/integration/node_pools_test.gopkg/handlers/openapi.gotest/integration/search_field_mapping_test.gopkg/api/presenters/adapter_status.gopkg/errors/errors.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.gopkg/api/presenters/node_pool.gotest/integration/adapter_status_test.gopkg/api/presenters/node_pool_test.gotest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/environments/**/*.go : Environment-based configuration: use OCM_ENV variable to select development, unit_testing, integration_testing, or production configuration
Applied to files:
.bingo/variables.env
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Applied to files:
.bingo/variables.env.bingo/Variables.mkMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Applied to files:
openapi/oapi-codegen.yamlMakefilepkg/handlers/openapi.gopkg/api/presenters/cluster.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/factories/**/*.go : Use test data factories in test/factories/ to provide consistent, reusable test data with builder pattern (e.g., NewClusterBuilder().WithName().Build())
Applied to files:
test/factories/clusters.gotest/factories/node_pools.gopkg/api/presenters/cluster_test.gotest/integration/adapter_status_test.gotest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/handlers/**/*.go : Accept status updates from adapters via POST /{resourceType}/{id}/statuses and store in adapter_statuses table
Applied to files:
pkg/api/adapter_status_types.gopkg/api/presenters/adapter_status_test.gopkg/api/presenters/adapter_status.gotest/integration/adapter_status_test.goopenapi/openapi.yaml
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/services/**/*.go : Calculate aggregate status from adapter conditions and return in status.phase field (Ready, Failed, NotReady)
Applied to files:
pkg/api/adapter_status_types.gotest/integration/api_contract_test.gopkg/api/presenters/adapter_status_test.gotest/integration/search_field_mapping_test.gopkg/api/presenters/adapter_status.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.gopkg/api/presenters/node_pool.gotest/integration/adapter_status_test.gopkg/api/presenters/node_pool_test.goopenapi/openapi.yaml
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/services/**/*.go : Status aggregation: Phase is 'Ready' if all adapters report Ready=True, 'Failed' if any adapter reports Ready=False, 'NotReady' otherwise
Applied to files:
pkg/api/adapter_status_types.gotest/integration/api_contract_test.gopkg/api/presenters/adapter_status_test.gotest/integration/search_field_mapping_test.gopkg/api/presenters/adapter_status.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.gotest/integration/adapter_status_test.gopkg/api/presenters/node_pool_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/integration/**/*_test.go : All API endpoints must have integration test coverage in test/integration/ including CRUD operations, pagination, search, error cases, and status aggregation
Applied to files:
test/integration/api_contract_test.gotest/helper.goMakefiletest/integration/search_field_mapping_test.gotest/integration/adapter_status_test.gotest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,handlers}/**/*.go : List operations must support pagination via ListArgs struct with limit, offset, and search parameters
Applied to files:
.bingo/oapi-codegen.mod
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/serve/**/*.go : API server runs on port 8000, metrics endpoint on 8080, health check on 8083; these are configurable via CLI flags
Applied to files:
pkg/handlers/openapi.gopkg/api/presenters/cluster.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/migrate/**/*.go : Use GORM AutoMigrate at startup via ./bin/hyperfleet-api migrate command; migrations are idempotent and track applied migrations
Applied to files:
pkg/handlers/openapi.gopkg/api/presenters/cluster.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{handlers,services,dao}/**/*.go : Errors must use structured ServiceError type from pkg/errors/ with HttpCode, Code, and Reason fields; send via errors.SendError(w, r, err)
Applied to files:
pkg/handlers/openapi.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,db}/**/*.go : Use polymorphic status tables with owner_type + owner_id to support multiple resource types without creating separate status tables
Applied to files:
pkg/api/presenters/adapter_status.gotest/integration/adapter_status_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/handlers/**/*.go : Health check endpoint at GET /healthcheck returns 200 OK when database is accessible; used by Kubernetes liveness/readiness probes
Applied to files:
pkg/api/presenters/cluster_test.go
📚 Learning: 2026-01-13T10:10:24.211Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/telemetry/otel.go:10-10
Timestamp: 2026-01-13T10:10:24.211Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository (PR #31), the team has decided to keep the OpenTelemetry semantic conventions package at v1.37.0 (go.opentelemetry.io/otel/semconv/v1.37.0) while using the SDK at v1.38.0 in pkg/telemetry/otel.go.
Applied to files:
openapi/openapi.yaml
📚 Learning: 2026-01-13T04:19:50.012Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: go.mod:32-36
Timestamp: 2026-01-13T04:19:50.012Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, the team has decided to keep OpenTelemetry core packages (go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/exporters/stdout/stdouttrace) at v1.38.0 rather than upgrading to v1.39.0 in PR #31.
Applied to files:
openapi/openapi.yaml
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/**/*{auth,middleware}*.go : JWT authentication validates tokens from Red Hat SSO and extracts account ID and username from claims for authorization
Applied to files:
test/integration/clusters_test.go
🧬 Code graph analysis (13)
test/registration.go (1)
test/helper.go (1)
Helper(55-68)
pkg/handlers/cluster.go (1)
pkg/api/presenters/cluster.go (1)
ConvertCluster(14-55)
test/integration/api_contract_test.go (1)
pkg/api/status_types.go (6)
PhaseNotReady(10-10)PhaseReady(11-11)PhaseFailed(12-12)ConditionTrue(20-20)ConditionFalse(21-21)ConditionUnknown(22-22)
pkg/handlers/cluster_nodepools.go (1)
pkg/api/presenters/node_pool.go (1)
ConvertNodePool(13-54)
test/factories/node_pools.go (1)
pkg/api/metadata_types.go (1)
Meta(37-42)
test/integration/node_pools_test.go (1)
test/helper.go (2)
WithAuthToken(308-316)GetAccessTokenFromContext(300-305)
pkg/handlers/openapi.go (1)
pkg/errors/errors.go (1)
GeneralError(196-198)
test/integration/search_field_mapping_test.go (1)
test/helper.go (1)
WithAuthToken(308-316)
pkg/api/presenters/adapter_status.go (3)
pkg/api/status_types.go (1)
ConditionStatus(17-17)pkg/api/adapter_status_types.go (2)
AdapterStatus(11-30)AdapterStatusMetadata(32-39)pkg/api/metadata_types.go (1)
Metadata(28-34)
pkg/api/presenters/cluster_test.go (2)
pkg/errors/errors.go (1)
Href(188-190)pkg/api/presenters/cluster.go (1)
ConvertCluster(14-55)
pkg/api/presenters/cluster.go (3)
pkg/api/cluster_types.go (1)
Cluster(11-34)pkg/api/status_types.go (2)
ConditionStatus(17-17)ResourcePhase(7-7)pkg/errors/errors.go (1)
Href(188-190)
test/integration/adapter_status_test.go (3)
test/helper.go (1)
WithAuthToken(308-316)pkg/api/adapter_status_types.go (1)
AdapterStatus(11-30)pkg/api/resource_id.go (1)
NewID(14-16)
test/integration/clusters_test.go (1)
test/helper.go (2)
WithAuthToken(308-316)GetAccessTokenFromContext(300-305)
🪛 checkmake (0.2.2)
Makefile
[warning] 227-227: Target body for "generate" exceeds allowed length of 5 (15).
(maxbodylength)
🔇 Additional comments (61)
pkg/handlers/validation_test.go (2)
97-106: LGTM!Correctly uses the
openapi.PtrStringhelper to construct the pointer-basedKindfield.
140-150: LGTM!Correctly validates that a wrong kind value produces the expected error message.
pkg/errors/errors.go (1)
165-178: LGTM — correct adaptation to oapi-codegen's inline struct type.The anonymous struct repetition (lines 165-168 and 170-173) is required because oapi-codegen generates
Detailsas a pointer to an anonymous struct slice (*[]struct{...}). Go's type system doesn't allow substituting a locally-defined named type here, so this pattern is the idiomatic approach..bingo/variables.env (1)
11-18: LGTM!The bingo-managed environment variables are correctly updated to include the new
OAPI_CODEGENtool and the golangci-lint version bump. This aligns with the PR's objective to introduce oapi-codegen for OpenAPI code generation.pkg/handlers/openapi.go (1)
22-40: LGTM!The migration from YAML-based OpenAPI loading to using the generated
openapi.GetSwagger()andMarshalJSON()is clean and aligns well with the oapi-codegen approach. Error handling is appropriate with descriptive error messages.openapi/oapi-codegen.yaml (1)
1-17: LGTM!The oapi-codegen configuration is well-structured:
embedded-spec: truecorrectly enables theGetSwagger()function used inpkg/handlers/openapi.go.client: truegenerates theClientWithResponsespattern mentioned in the PR objectives.- The
compatibilityoptions (old-merge-schemas,old-aliasing) ensure smoother migration from the previous generator..bingo/Variables.mk (1)
44-49: LGTM!The
OAPI_CODEGENmake target follows the established bingo pattern consistently with other tool entries. The build command correctly references the oapi-codegen module and outputs to the versioned binary path..bingo/oapi-codegen.mod (1)
1-5: Align Go version across bingo module files to match main go.mod.The
.bingo/oapi-codegen.modspecifies Go 1.25.0, which is inconsistent with the maingo.mod(1.24.0). Additionally,.bingo/mockgen.modalso specifies Go 1.25.0. Update both files to1.24.0to maintain consistency across the project. Per organizational guidelines, the target version should be Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto).⛔ Skipped due to learnings
Learnt from: CR Repo: openshift-hyperfleet/hyperfleet-api PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-08T01:23:20.647Z Learning: Applies to **/*.go : Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)Learnt from: CR Repo: openshift-hyperfleet/hyperfleet-api PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-08T01:23:20.647Z Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progressLearnt from: CR Repo: openshift-hyperfleet/hyperfleet-api PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-08T01:23:20.647Z Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go filesLearnt from: CR Repo: openshift-hyperfleet/hyperfleet-api PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-08T01:23:20.647Z Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deploymentLearnt from: CR Repo: openshift-hyperfleet/hyperfleet-api PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-08T01:23:20.647Z Learning: Applies to pkg/{dao,handlers}/**/*.go : List operations must support pagination via ListArgs struct with limit, offset, and search parametersLearnt from: yasun1 Repo: openshift-hyperfleet/hyperfleet-api PR: 31 File: pkg/errors/errors.go:9-9 Timestamp: 2026-01-08T09:40:10.138Z Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.pkg/api/adapter_status_types.go (1)
32-44: LGTM! New metadata struct aligns with OpenAPI schema.The
AdapterStatusMetadatastruct correctly mirrors themetadataobject defined in the OpenAPI spec forAdapterStatus. All field types match (int32 for attempt, date-time for timestamps, string for duration), and the JSON tags use consistent snake_case naming.pkg/api/presenters/node_pool_test.go (3)
9-12: LGTM! Import and type updates align with oapi-codegen migration.The new
openapi_typesimport fromgit.colasdn.top/oapi-codegen/runtime/typesis correctly used for email-typed fields (CreatedBy,UpdatedBy). This aligns with the OpenAPI spec'sformat: emaildefinition for these fields.
225-231: Status enum assertions correctly updated.The test expectations properly use the new typed enum values (
openapi.Ready,openapi.True) instead of string constants. This provides compile-time type safety and matches the updated OpenAPIResourcePhaseandConditionStatusenum definitions.
395-439: Round-trip test properly validates email type handling.The test correctly verifies that
CreatedBy/UpdatedByfields maintain their semanticopenapi_types.Emailtype through theConvertNodePool→PresentNodePoolcycle. This ensures the presenter layer correctly wraps string values into the typed representation.test/helper.go (2)
245-257: Client creation correctly adapted for oapi-codegen.The
NewApiClient()method properly constructs the server URL and initializes theClientWithResponsesclient. Usinghelper.T.Fatalfon error is appropriate for test setup failures.
288-316: Auth context infrastructure is well-designed.The implementation follows Go best practices:
- Unexported
contextKeyAccessTokentype prevents key collisionsGetAccessTokenFromContextsafely returns empty string for missing valuesWithAuthTokenconditionally injects the header only when a token exists, avoiding emptyAuthorizationheadersopenapi/openapi.yaml (2)
627-680: List schemas with explicit pagination metadata are well-structured.The
AdapterStatusList(and similarlyClusterList,NodePoolList) now explicitly defines pagination fields (kind,page,size,total) as required properties. This provides a consistent structure across all list endpoints and generates clean Go types.
1265-1271: ResourcePhase enum correctly defines lifecycle states.The enum provides the three standard lifecycle phases (
NotReady,Ready,Failed) that align with the status aggregation logic documented in the coding guidelines (Ready if all adapters Ready=True, Failed if any Ready=False, NotReady otherwise).Makefile (1)
254-256: Generate-vendor alias maintains backward compatibility.The simplified alias ensures existing workflows calling
make generate-vendorcontinue to work while eliminating the redundant Docker-based generation path.go.mod (2)
47-47: Transitive dependency updates look correct.The
go-jsonmerge/v2is a transitive dependency of oapi-codegen/runtime, and the bluemonday update from v1.0.23 to v1.0.25 is a minor security-related bump.Also applies to: 89-89
24-24: LGTM - oapi-codegen runtime dependency is current.The
github.com/oapi-codegen/runtime v1.1.2is the latest version and provides the required runtime library for oapi-codegen generated types and client code. This aligns with the PR objective.test/integration/search_field_mapping_test.go (3)
34-44: LGTM - Clean migration to ClientWithResponses pattern.The search parameter construction and response handling follow the new oapi-codegen conventions correctly:
openapi.SearchParams(searchStr)type conversionGetClustersWithResponsewith params and auth token- Proper nil-check on
resp.JSON200before assertionsThis pattern is consistently applied throughout the file.
54-54: Enum naming updated correctly.The change from
openapi.NOT_READYtoopenapi.NotReadyaligns with the standardized enum naming convention in the new generated types.
275-293: NodePool search tests properly mirror Cluster tests.The
GetNodePoolsParamsandGetNodePoolsWithResponseusage correctly parallels the Cluster search implementation, maintaining consistency across resource types.test/integration/adapter_status_test.go (4)
15-24: Good extraction of helper function.The
newAdapterStatusRequesthelper reduces code duplication and ensures consistent construction ofAdapterStatusCreateRequestobjects. SettingObservedTime: time.Now()is appropriate for test scenarios.
44-51: Status enum migration looks correct.The change from string literals
"True"/"False"to typed constantsopenapi.True/openapi.Falseprovides type safety and aligns with the new generated types.
54-60: Response handling correctly uses JSON201 for created resources.The pattern of using
resp.JSON201for POST responses returning created resources andresp.JSON200for GET responses is consistent with HTTP semantics and the oapi-codegen conventions.
375-398: No issue with increment operation on QueryParamsPage.The
pageNum++operation on line 397 is valid.openapi.QueryParamsPagesupports increment operations, as evidenced by its successful use in the pagination loop. No changes needed.test/factories/node_pools.go (1)
34-41: LGTM - Audit fields added to test factory.Adding
CreatedByandUpdatedByfields ensures test data includes user-tracking metadata, which aligns with the broader test infrastructure updates in this PR. The placeholder email"[email protected]"is appropriate for test factories.test/registration.go (1)
11-23: LGTM - Return type updated for new client pattern.The change from
*openapi.APIClientto*openapi.ClientWithResponsesis the foundational change enabling all integration tests to use the new oapi-codegen response-based API pattern. This aligns withhelper.NewApiClient()returning*openapi.ClientWithResponsesas shown in the helper context.pkg/handlers/cluster_nodepools.go (2)
151-152: Owner identifier updated to email format.The change from
"system"to"[email protected]"aligns with the PR-wide standardization of owner/user metadata as email-typed fields. This is consistent with changes inpkg/handlers/cluster.goandpkg/handlers/node_pool.go.
45-49: The string concatenation approach, while not ideal, does not present a SQL injection vulnerability in this context. The search string is parsed by the tree-search-language (TSL) query parser (line 167 of generic.go), which properly extracts values and converts them to parameterized SQL before GORM execution. Additionally, theclusterIDis validated to exist (line 37) before being used in the search filter, and KSUID-generated IDs are constrained to alphanumeric characters (0-9, a-v), preventing quote injection. While a more structured approach would be preferable for clarity, the current implementation is safe through its combination of validation and parameterized query processing.Likely an incorrect or invalid review comment.
test/factories/clusters.go (1)
24-25: LGTM!The addition of
CreatedByandUpdatedByfields with a consistent test email aligns with the API schema changes and mirrors the approach intest/factories/node_pools.go. As per coding guidelines, the factory pattern is correctly maintained for consistent, reusable test data.pkg/handlers/cluster.go (1)
41-42: LGTM!The update to
"[email protected]"is consistent with the PR-wide standardization of owner/user metadata as email-typed fields across all handlers.pkg/handlers/cluster_nodepools_test.go (1)
59-66: LGTM!The mock NodePool now includes all required fields (
Spec,Labels,StatusConditions,CreatedBy,UpdatedBy) needed for the presenter to work correctly. The empty JSON values and test email are appropriate for test fixtures.test/integration/api_contract_test.go (1)
14-40: LGTM!The contract tests correctly verify that domain constants (
api.PhaseNotReady,api.ConditionTrue, etc.) remain synchronized with the renamed OpenAPI constants (openapi.NotReady,openapi.True, etc.). This is a valuable safeguard against drift between domain and generated types after the oapi-codegen migration.pkg/api/presenters/node_pool.go (2)
114-124: LGTM!The ResourceCondition mapping is complete with all required fields properly ordered:
CreatedTime,LastTransitionTime,LastUpdatedTime,Message,ObservedGeneration,Reason,Status, andType. The type cast forStatuscorrectly converts from domain to OpenAPI type.
126-151: LGTM!The NodePool construction comprehensively populates all required fields including metadata, owner references, spec, and status. The use of
toEmailhelper (defined incluster.go) forCreatedBy/UpdatedByaligns with the newopenapi_types.Emailtype requirement.pkg/api/presenters/cluster.go (3)
37-41: LGTM!The Kind defaulting logic follows the same pattern as
ConvertNodePool, ensuring consistency across resource types. Using the pointer value when provided and falling back to "Cluster" is the correct approach.
57-60: LGTM!The
toEmailhelper provides a clean conversion from string toopenapi_types.Email. Since validation occurs at the API boundary, this simple type cast is appropriate for the presenter layer.
126-145: LGTM!The Cluster construction is comprehensive and properly uses the new OpenAPI types:
openapi.PtrStringfor pointer fields,toEmailfor email-typed fields, and direct struct embedding for Status. The field ordering and type casts are correct.pkg/api/presenters/adapter_status.go (2)
43-47: LGTM!The Data handling correctly initializes an empty map when
req.Datais nil, ensuring the marshaled JSON is always valid ({}vsnull). The pointer dereference on line 46 is safe due to the nil check.
152-160: LGTM!The returned AdapterStatus correctly uses pointer types for
DataandMetadataas required by the new OpenAPI schema. Field ordering is clean and the nil-safe timestamp handling ensures zero values are returned rather than nil pointers.pkg/api/presenters/adapter_status_test.go (3)
22-34: LGTM!The test helper correctly constructs the request with the new pointer-based
Dataand inlineMetadatastruct patterns. This aligns with the updated OpenAPI schema.
209-211: LGTM!The test cases correctly use the renamed OpenAPI constants (
openapi.True,openapi.False,openapi.Unknown) and verify bidirectional mapping with domain constants.
426-460: LGTM!The round-trip test (
TestConvertAndPresent_RoundTrip) validates data integrity through the convert/present cycle, including proper handling of pointer-based Data access. This is excellent coverage for the new type patterns.test/integration/node_pools_test.go (3)
61-77: LGTM!The test correctly uses the new
*_WithResponsepattern withresp.StatusCode()andresp.JSON201payload access. The local variable forkindpointer is the correct approach for the updated pointer-based field.
106-128: LGTM!The paging test correctly constructs
GetNodePoolsParamswith typed query parameters (QueryParamsPage,QueryParamsPageSize) and uses the*_WithResponsevariant. The nil check onresp.JSON200ensures safe access.
197-231: LGTM!The test comprehensively covers the happy path (200 OK) and error cases (404 for non-existent nodepool, non-existent cluster, and wrong cluster). The consistent use of
*_WithResponsevariants andStatusCode()assertions is correct.pkg/api/presenters/cluster_test.go (3)
15-27: LGTM!The test helper correctly constructs the request with pointer-based
LabelsandKindfields usingopenapi.PtrString. This aligns with the updated ClusterCreateRequest schema.
216-217: LGTM!The assertions correctly compare
CreatedByandUpdatedByagainstopenapi_types.Emailtyped values, validating thetoEmailhelper conversion inPresentCluster.
226-230: LGTM!The Status phase and condition assertions correctly use the new OpenAPI enum constants (
openapi.Ready,openapi.True). The test validates the complete status structure including nested conditions.test/integration/clusters_test.go (11)
21-51: LGTM!The test correctly adapts to the new
WithResponsepattern, properly checks for nil onresp.JSON200before dereferencing, and correctly usestest.WithAuthToken(ctx)for authentication.
53-86: LGTM!The test properly uses
openapi.PtrString("Cluster")for the pointer field, correctly extracts and validates theJSON201response, and appropriately falls back to raw resty for the malformed JSON test case.
112-125: LGTM!The pagination parameter construction using
openapi.QueryParamsPageandopenapi.QueryParamsPageSizetype aliases is idiomatic for oapi-codegen generated types. The nil check onresp.JSON200is properly placed.
128-149: LGTM!The search parameter construction and response handling are correct and idiomatic.
316-383: LGTM!The schema validation tests properly handle both the typed client and raw resty approaches. The nil checks on
JSON201are in place.
453-507: LGTM!This test correctly uses raw resty for the invalid request, and
resp.Body()from resty works as expected (unlike theHTTPResponse.Bodyfrom the oapi-codegen client).
509-560: LGTM!The default sorting test properly uses the new client pattern and includes appropriate nil checks.
562-616: LGTM!The orderBy parameter construction and response handling are correct.
672-705: LGTM!The test correctly uses raw resty to send invalid JSON payloads that can't be represented with the typed structs.
707-740: LGTM!The test correctly validates that wrong
kindvalues are rejected with a 400 response.
421-434: Response body already consumed byPostClusterWithResponseclient.The
PostClusterWithResponsemethod parses the response body to populate typed response fields likeresp.JSON201orresp.JSON400. Callingio.ReadAll(resp.HTTPResponse.Body)on line 426 attempts to read an already-consumed stream, which returns empty data or EOF.Use the pre-parsed response from the generated client. For error responses on non-2xx status codes, the typed response field (e.g.,
resp.JSON400) should contain the parsed error object, or access the raw body bytes from the response wrapper if available.🐛 Proposed fix
- // Parse error response to verify field-level details - bodyBytes, err := io.ReadAll(resp.HTTPResponse.Body) - if err != nil { - t.Fatalf("failed to read response body: %v", err) - } - var errorResponse openapi.Error - if err := json.Unmarshal(bodyBytes, &errorResponse); err != nil { + if err := json.Unmarshal(resp.Body, &errorResponse); err != nil { t.Fatalf("failed to unmarshal error response body: %v", err) }
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @Dockerfile:
- Around line 4-5: The Dockerfile declares ARGs GIT_SHA and GIT_DIRTY but the
RUN step calls make build which re-computes them; instead pass the ARG values
into make so the build uses the provided values—modify the RUN that invokes make
build to export or forward GIT_SHA and GIT_DIRTY (e.g., pass GIT_SHA="$GIT_SHA"
GIT_DIRTY="$GIT_DIRTY" to make or export them into the environment before
calling make build) so the make process receives the build ARGs.
- Around line 1-2: Update the Dockerfile to use the correct Go toolchain and
FIPS crypto build settings: replace the base image reference FROM golang:1.25
with FROM golang:1.24.9-bookworm to match go.mod/toolchain, change the build
invocation that currently sets CGO_ENABLED=0 to run CGO_ENABLED=1
GOEXPERIMENT=boringcrypto GOOS=linux make build so the binary is built with
BoringCrypto, and remove or rename the stale top comment "OpenAPI generation
stage" to reflect this being a standard builder stage; locate these changes
around the existing FROM line and the command invoking make build in the builder
stage.
In @Makefile:
- Around line 274-280: The clean target currently only removes
pkg/api/openapi/openapi.gen.go but generate produces both openapi.gen.go and
helpers.go; update the Makefile clean target (the "clean" rule) to remove
pkg/api/openapi/helpers.go as well or simply rm -rf the entire pkg/api/openapi
directory (matching the generate target behavior) so helpers.go is cleaned up
too.
🧹 Nitpick comments (1)
Makefile (1)
231-242: Consider removing redundant helper functions.
PtrStringandPtrInt32are functionally redundant sincePtr[T any]handles all types generically. Unless there's a specific need for non-generic helpers (e.g., compatibility with code that can't use generics), these can be removed to reduce surface area.Additionally, consider moving the helpers template to a separate file (e.g.,
openapi/helpers.go.tmpl) and usingcporcatinstead of inlineprintf. This addresses the static analysis warning about target body length and improves maintainability.Simplified helpers.go
@printf '%s\n' \ 'package openapi' \ '' \ '// Ptr returns a pointer to the given value.' \ 'func Ptr[T any](v T) *T { return &v }' \ - '' \ - '// PtrString returns a pointer to the given string.' \ - 'func PtrString(v string) *string { return &v }' \ - '' \ - '// PtrInt32 returns a pointer to the given int32.' \ - 'func PtrInt32(v int32) *int32 { return &v }' \ > pkg/api/openapi/helpers.go
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
DockerfileMakefile
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/telemetry/otel.go:10-10
Timestamp: 2026-01-13T10:10:24.211Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository (PR #31), the team has decided to keep the OpenTelemetry semantic conventions package at v1.37.0 (go.opentelemetry.io/otel/semconv/v1.37.0) while using the SDK at v1.38.0 in pkg/telemetry/otel.go.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: go.mod:32-36
Timestamp: 2026-01-13T04:19:50.012Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, the team has decided to keep OpenTelemetry core packages (go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/exporters/stdout/stdouttrace) at v1.38.0 rather than upgrading to v1.39.0 in PR #31.
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
📚 Learning: 2026-01-08T09:40:10.138Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/serve/**/*.go : API server runs on port 8000, metrics endpoint on 8080, health check on 8083; these are configurable via CLI flags
Applied to files:
Dockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/migrate/**/*.go : Use GORM AutoMigrate at startup via ./bin/hyperfleet-api migrate command; migrations are idempotent and track applied migrations
Applied to files:
Dockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/integration/**/*_test.go : All API endpoints must have integration test coverage in test/integration/ including CRUD operations, pagination, search, error cases, and status aggregation
Applied to files:
Makefile
🪛 checkmake (0.2.2)
Makefile
[warning] 227-227: Target body for "generate" exceeds allowed length of 5 (15).
(maxbodylength)
🔇 Additional comments (4)
Dockerfile (1)
20-42: LGTM!The runtime stage configuration is well-structured: distroless base image for minimal attack surface, correct binary and schema paths, appropriate port exposure, and clear metadata labels.
Makefile (3)
80-82: LGTM!The uppercase naming (
GIT_SHA,GIT_DIRTY) follows Make conventions for variables that may be overridden via environment or command line, and the?=operator correctly allows external overrides while providing sensible defaults.
254-256: LGTM!Good backward compatibility approach—existing CI/CD workflows or developer habits using
make generate-vendorwill continue to work seamlessly.
314-317: LGTM!Build args are correctly propagated to the container build. Once the Dockerfile is updated to export these as environment variables for
make build, the version metadata will be properly embedded.
rh-amarin
force-pushed
the
oapi-codegen
branch
from
e1be283 to
4367834
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @Dockerfile:
- Line 4: Update the Dockerfile base image from "golang:1.25 AS builder" to the
project-required Go version "golang:1.24.9 AS builder" and ensure the build
preserves FIPS-compliant crypto by not forcing CGO off; remove any occurrence of
"CGO_ENABLED=0" in the Dockerfile (or explicitly set "ENV CGO_ENABLED=1" if an
explicit value is required) so the container build honors the Makefile's
CGO_ENABLED default and enables FIPS-compliant crypto.
🧹 Nitpick comments (1)
Makefile (1)
227-243: Consider extracting helper generation to a separate file.The inline
printfforhelpers.goworks but makes the target body long (15 statements). Consider placing the helper source in a template file (e.g.,openapi/helpers.go.tmpl) and copying it during generation, or accept this as-is given it's a one-time setup.Also, the generic
Ptr[T]already covers the functionality ofPtrStringandPtrInt32. You could simplify to just the generic helper unless there are call sites where type inference struggles.♻️ Simplified helpers.go with generic only
@printf '%s\n' \ 'package openapi' \ '' \ '// Ptr returns a pointer to the given value.' \ 'func Ptr[T any](v T) *T { return &v }' \ - '' \ - '// PtrString returns a pointer to the given string.' \ - 'func PtrString(v string) *string { return &v }' \ - '' \ - '// PtrInt32 returns a pointer to the given int32.' \ - 'func PtrInt32(v int32) *int32 { return &v }' \ > pkg/api/openapi/helpers.go
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.dockerignoreDockerfileMakefile
✅ Files skipped from review due to trivial changes (1)
- .dockerignore
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/telemetry/otel.go:10-10
Timestamp: 2026-01-13T10:10:24.211Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository (PR #31), the team has decided to keep the OpenTelemetry semantic conventions package at v1.37.0 (go.opentelemetry.io/otel/semconv/v1.37.0) while using the SDK at v1.38.0 in pkg/telemetry/otel.go.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: go.mod:32-36
Timestamp: 2026-01-13T04:19:50.012Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, the team has decided to keep OpenTelemetry core packages (go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/exporters/stdout/stdouttrace) at v1.38.0 rather than upgrading to v1.39.0 in PR #31.
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
📚 Learning: 2026-01-08T09:40:10.138Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Applied to files:
MakefileDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Applied to files:
MakefileDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Applied to files:
MakefileDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Applied to files:
MakefileDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Applied to files:
MakefileDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Applied to files:
MakefileDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to **/*.go : Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Applied to files:
MakefileDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/environments/**/*.go : Environment-based configuration: use OCM_ENV variable to select development, unit_testing, integration_testing, or production configuration
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/integration/**/*_test.go : All API endpoints must have integration test coverage in test/integration/ including CRUD operations, pagination, search, error cases, and status aggregation
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/serve/**/*.go : API server runs on port 8000, metrics endpoint on 8080, health check on 8083; these are configurable via CLI flags
Applied to files:
Dockerfile
🪛 checkmake (0.2.2)
Makefile
[warning] 228-228: Target body for "generate" exceeds allowed length of 5 (15).
(maxbodylength)
🔇 Additional comments (8)
Makefile (4)
7-8: LGTM!The conditional assignment pattern allows the Dockerfile to override
CGO_ENABLEDfor Alpine-based builds while preserving the FIPS-compliant default for local development. Good documentation in the comment.
81-83: LGTM!Good use of
?=for build metadata variables, enabling both CI and Dockerfile overrides while providing sensible defaults from git.
255-256: LGTM!Good backward compatibility by aliasing
generate-vendortogeneratenow that oapi-codegen handles dependencies natively.
315-319: LGTM!Propagating
GIT_SHAandGIT_DIRTYas build args improves image traceability. TheBASE_IMAGEoverride for dev builds enables Alpine-based images.Also applies to: 343-348
Dockerfile (4)
19-19: Verify FIPS compliance requirement withCGO_ENABLED=0.The Makefile comment states CGO is required for FIPS-compliant crypto (
CGO_ENABLED=1, GOEXPERIMENT=boringcrypto). HardcodingCGO_ENABLED=0disables BoringCrypto and breaks FIPS compliance for production images.If FIPS is required for production:
- Use
CGO_ENABLED=1 GOEXPERIMENT=boringcrypto- Switch to a glibc-based runtime image (e.g.,
gcr.io/distroless/base-debian12or UBI) since static linking with CGO requires musl or glibcIf FIPS is only required for certain deployments, consider parameterizing via build arg.
Based on learnings, the project requires FIPS-compliant crypto. Please confirm whether the container image needs FIPS compliance or if this is acceptable for dev/non-FedRAMP deployments.
1-1: LGTM!The
BASE_IMAGEARG pattern is well-designed — distroless for production security, overridable to Alpine for dev builds via--build-arg BASE_IMAGE=alpine:3.21.Also applies to: 22-22
27-33: LGTM!Binary and OpenAPI schema paths are correctly updated. The
OPENAPI_SCHEMA_PATHenv var enables Helm-based overrides for provider-specific schemas.
40-44: LGTM!Good addition of OCI labels for image metadata. Consider adding
org.opencontainers.image.*labels for broader tooling compatibility in a follow-up.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @Dockerfile:
- Around line 6-7: The Dockerfile declares ARGs GIT_SHA and GIT_DIRTY but never
forwards them into the build, so the Makefile fallback GIT_SHA ?= $(shell git
rev-parse ...) runs inside the container and yields "unknown"; change the RUN
that invokes make (e.g., the make build invocation) to pass the ARG values into
make by appending GIT_SHA="$GIT_SHA" and GIT_DIRTY="$GIT_DIRTY" (and update any
other make invocations similarly), ensuring the Makefile receives the values
instead of attempting to read .git inside the container.
- Line 4: Change the base image from golang:1.25 to golang:1.24 by updating the
FROM line, and enable FIPS-compliant crypto by setting the build environment to
CGO_ENABLED=1 GOEXPERIMENT=boringcrypto where you currently set CGO_ENABLED=0;
locate the FROM instruction and the build environment/ARG or RUN line that
exports CGO_ENABLED and replace those values accordingly to use Go 1.24 and the
boringcrypto experiment.
🧹 Nitpick comments (2)
Makefile (1)
227-244: Consider extracting helpers.go to a separate file.Embedding Go code in a
printfwithin the Makefile works but is fragile—escaping issues and maintenance become harder over time. Consider creating aopenapi/helpers.go.tmplorscripts/helpers.gofile and copying it instead.♻️ Proposed refactor: use a template file
Create
openapi/helpers.go.tmpl:package openapi // Ptr returns a pointer to the given value. func Ptr[T any](v T) *T { return &v } // PtrString returns a pointer to the given string. func PtrString(v string) *string { return &v } // PtrInt32 returns a pointer to the given int32. func PtrInt32(v int32) *int32 { return &v }Then simplify the Makefile:
generate: $(OAPI_CODEGEN) rm -rf pkg/api/openapi mkdir -p pkg/api/openapi $(OAPI_CODEGEN) --config openapi/oapi-codegen.yaml openapi/openapi.yaml - @printf '%s\n' \ - 'package openapi' \ - '' \ - '// Ptr returns a pointer to the given value.' \ - 'func Ptr[T any](v T) *T { return &v }' \ - '' \ - '// PtrString returns a pointer to the given string.' \ - 'func PtrString(v string) *string { return &v }' \ - '' \ - '// PtrInt32 returns a pointer to the given int32.' \ - 'func PtrInt32(v int32) *int32 { return &v }' \ - > pkg/api/openapi/helpers.go + cp openapi/helpers.go.tmpl pkg/api/openapi/helpers.go .PHONY: generate.dockerignore (1)
1-8: Consider addingsecrets/to prevent accidental exposure.The Makefile creates a
secrets/directory with database credentials and service tokens. While this directory may not exist in the repository, it could be created locally and inadvertently included in the Docker build context.♻️ Proposed addition
.git .gitignore .github bin/ docs/ charts/ .env .env.* +secrets/ +*.md +test/
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.dockerignoreDockerfileMakefile
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/telemetry/otel.go:10-10
Timestamp: 2026-01-13T10:10:24.211Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository (PR #31), the team has decided to keep the OpenTelemetry semantic conventions package at v1.37.0 (go.opentelemetry.io/otel/semconv/v1.37.0) while using the SDK at v1.38.0 in pkg/telemetry/otel.go.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: go.mod:32-36
Timestamp: 2026-01-13T04:19:50.012Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, the team has decided to keep OpenTelemetry core packages (go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/exporters/stdout/stdouttrace) at v1.38.0 rather than upgrading to v1.39.0 in PR #31.
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
📚 Learning: 2026-01-08T09:40:10.138Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/serve/**/*.go : API server runs on port 8000, metrics endpoint on 8080, health check on 8083; these are configurable via CLI flags
Applied to files:
Dockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to **/*.go : Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Applied to files:
DockerfileMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/environments/**/*.go : Environment-based configuration: use OCM_ENV variable to select development, unit_testing, integration_testing, or production configuration
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/integration/**/*_test.go : All API endpoints must have integration test coverage in test/integration/ including CRUD operations, pagination, search, error cases, and status aggregation
Applied to files:
Makefile
🪛 checkmake (0.2.2)
Makefile
[warning] 228-228: Target body for "generate" exceeds allowed length of 5 (15).
(maxbodylength)
🔇 Additional comments (7)
Makefile (5)
7-8: LGTM! Good use of conditional assignment.Using
?=forCGO_ENABLEDallows the Dockerfile to override the value (settingCGO_ENABLED=0for Alpine-based builds that don't have glibc), while keeping the FIPS-compliant default of1for local development.
81-83: LGTM! Git metadata extraction is well-handled.The
?=assignment allows CI systems or Dockerfiles to inject these values, and the fallback to "unknown" when git is unavailable is sensible. The|| echopattern correctly handles missing.gitor non-git contexts.
255-256: LGTM! Clean alias for backward compatibility.Aliasing
generate-vendortogeneratemaintains backward compatibility for scripts or CI pipelines that may still call the old target. Based on learnings, the previous workflow used Podman/Docker for generation, so this alias provides a smooth transition.
315-319: LGTM! Build-args properly propagated.The
GIT_SHAandGIT_DIRTYbuild-args are correctly passed to the Dockerfile, enabling consistent version metadata in the built image.
343-348: Verify Alpine compatibility with CGO_ENABLED=0.The dev image uses
alpine:3.21as the base. The Dockerfile setsCGO_ENABLED=0which produces a statically-linked binary—this is correct for Alpine (musl libc). However, the comment on line 7 mentions FIPS compliance requiresCGO_ENABLED=1. Ensure that dev images intentionally skip FIPS compliance.Dockerfile (2)
22-27: LGTM! Multi-stage build with configurable base image.Using
${BASE_IMAGE}allows switching between distroless (production) and Alpine (dev) at build time. The distroless default provides a minimal attack surface for production deployments.
37-38: LGTM! Clean entrypoint configuration.Separating
ENTRYPOINTandCMDallows users to override the command (e.g.,migrate) while keeping the binary path fixed.
| rm -f /local/pkg/api/openapi/go.mod /local/pkg/api/openapi/go.sum && \ | ||
| rm -rf /local/pkg/api/openapi/test | ||
| # OpenAPI generation stage | ||
| FROM golang:1.25 AS builder |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Go programming language release golang:1.25 January 2026
💡 Result:
Go 1.25 was released on August 12, 2025. See the official Go blog announcement and the Go 1.25 release notes for details. [1][2]
Sources:
[1] Go blog — "Go 1.25 is released" (Aug 12, 2025).
[2] Go 1.25 Release Notes (go.dev/doc/go1.25).
🏁 Script executed:
cd /dev/shm && find . -name "Dockerfile" -type f 2>/dev/null | head -5Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 137
🏁 Script executed:
git ls-files | grep -i dockerfileRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 89
🏁 Script executed:
cat -n DockerfileRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 1421
Update Go builder to use 1.24.9 with FIPS-compliant crypto settings.
The Dockerfile uses golang:1.25 and CGO_ENABLED=0, but the project requires Go 1.24.9 with FIPS-compliant crypto enabled. Update line 4 to golang:1.24 and line 19 to CGO_ENABLED=1 GOEXPERIMENT=boringcrypto to comply with project standards.
🤖 Prompt for AI Agents
In @Dockerfile at line 4, Change the base image from golang:1.25 to golang:1.24
by updating the FROM line, and enable FIPS-compliant crypto by setting the build
environment to CGO_ENABLED=1 GOEXPERIMENT=boringcrypto where you currently set
CGO_ENABLED=0; locate the FROM instruction and the build environment/ARG or RUN
line that exports CGO_ENABLED and replace those values accordingly to use Go
1.24 and the boringcrypto experiment.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @Makefile:
- Around line 81-83: GIT_DIRTY fallback produces false positives when git is
unavailable; update the Makefile so GIT_DIRTY is only set when git exists and
we're inside a work tree: check that git is callable and that `git rev-parse
--is-inside-work-tree` (or existence of .git) succeeds before running `git diff
--quiet`; otherwise set GIT_DIRTY to empty. Keep the existing GIT_SHA and
build_version concatenation but ensure GIT_DIRTY is "" (not "-modified") in
non-git environments so build_version becomes just the SHA or "unknown".
- Around line 303-307: The build passes GIT_SHA and GIT_DIRTY as build args but
the Dockerfile's ARG GIT_SHA and ARG GIT_DIRTY are never consumed, so embed them
into the image by referencing those args in the Dockerfile (e.g., after ARG
GIT_SHA / ARG GIT_DIRTY add a LABEL like
org.opencontainers.image.revision="$GIT_SHA" and a label or ENV for dirty state
such as org.example.git.dirty="$GIT_DIRTY"); update the Dockerfile to consume
ARG GIT_SHA and ARG GIT_DIRTY in a LABEL or ENV so the values supplied by the
Makefile build command are persisted in the image metadata.
In @test/integration/clusters_test.go:
- Around line 651-654: The test reads listResp.JSON200 into list without
verifying it's non-nil; update the test after calling
client.GetClustersWithResponse to check that listResp.JSON200 is not nil before
using it (as other tests do). Specifically, after GetClustersWithResponse and
the Expect(err) assertion, add a nil assertion on listResp.JSON200 (or on list
after assignment) before accessing list.Items so the test fails clearly if the
response body is absent.
- Around line 202-205: The test accesses resp.JSON200.Items without ensuring
resp.JSON200 is non-nil; add a nil-check before accessing Items (e.g., assert
that resp.JSON200 is not nil) after calling client.GetClustersWithResponse so
the test fails with a clear message instead of panicking; reference the response
from GetClustersWithResponse, assert resp.JSON200 is not nil, then safely assert
on resp.JSON200.Items.
- Around line 533-538: The test may append a nil pointer when building
createdClusters by dereferencing resp.JSON201; update the logic around where
createdClusters is appended (the code that does createdClusters =
append(createdClusters, *resp.JSON201) after creating clusters) to first assert
or guard that resp != nil and resp.JSON201 != nil (use
Expect(resp).NotTo(BeNil()) and Expect(resp.JSON201).NotTo(BeNil()) or an if
check) and only then append the dereferenced value, mirroring the nil-safety
pattern used elsewhere in this test file.
🧹 Nitpick comments (3)
pkg/util/utils.go (1)
57-59: Consider using the existing genericToPtrfunction instead.
PtrStringduplicates functionality already provided byToPtr[T any]at lines 8-11. For consistency, consider usingutil.ToPtr("someString")throughout the codebase instead of introducing a string-specific helper.That said,
PtrStringis more explicit and avoids type inference, which may be preferable for readability in some contexts.pkg/api/presenters/cluster.go (1)
58-61: Consider documenting the Email type semantics.The
toEmailhelper performs a direct type cast. This works, but consider adding a brief comment noting thatopenapi_types.Emailis a string alias without validation, so email format validation should occur elsewhere (e.g., at API input boundaries).pkg/api/presenters/cluster_test.go (1)
394-396: Pointer comparison may not test value equality correctly.Line 396 compares
result.KindtooriginalReq.Kind, both of which are*string. This compares pointer addresses rather than the underlying string values. Sinceutil.PtrStringcreates a new pointer each time, andPresentClustercreates another new pointer, these will always be different pointers even if the values match.The test currently passes because Gomega's
Equalmatcher dereferences pointers when comparing, but this is implicit behavior that could be confusing. Consider making the comparison explicit.✨ Suggested improvement for clarity
// Verify data integrity Expect(*result.Id).To(Equal("cluster-roundtrip-123")) - Expect(result.Kind).To(Equal(originalReq.Kind)) + Expect(*result.Kind).To(Equal(*originalReq.Kind)) Expect(result.Name).To(Equal(originalReq.Name))
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Makefilepkg/api/presenters/cluster.gopkg/api/presenters/cluster_test.gopkg/api/presenters/kind.gopkg/api/presenters/object_reference.gopkg/api/presenters/path.gopkg/errors/errors.gopkg/handlers/validation_test.gopkg/util/utils.gotest/integration/adapter_status_test.gotest/integration/clusters_test.go
🧰 Additional context used
📓 Path-based instructions (7)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Use structured logging via logger.NewOCMLogger(ctx) with context-aware log fields including operation ID, account ID, and transaction ID
Files:
pkg/util/utils.gopkg/api/presenters/object_reference.gopkg/api/presenters/path.gopkg/api/presenters/cluster.gopkg/errors/errors.gopkg/api/presenters/kind.gotest/integration/adapter_status_test.gopkg/api/presenters/cluster_test.gopkg/handlers/validation_test.gotest/integration/clusters_test.go
pkg/api/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Files:
pkg/api/presenters/object_reference.gopkg/api/presenters/path.gopkg/api/presenters/cluster.gopkg/api/presenters/kind.gopkg/api/presenters/cluster_test.go
test/integration/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
test/integration/**/*_test.go: Integration tests must use Testcontainers to create isolated PostgreSQL instances for each test suite to ensure no state leakage
All API endpoints must have integration test coverage in test/integration/ including CRUD operations, pagination, search, error cases, and status aggregation
Files:
test/integration/adapter_status_test.gotest/integration/clusters_test.go
pkg/handlers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/handlers/**/*.go: HTTP handlers must follow the pattern: parse request → call service/DAO → handle errors → send response with appropriate HTTP status codes
Database transactions are automatically managed by TransactionMiddleware for all HTTP requests; store session in context and auto-commit on success, rollback on error
Accept resource creation/update/delete requests via REST endpoints, validate, and persist to PostgreSQL
Accept status updates from adapters via POST /{resourceType}/{id}/statuses and store in adapter_statuses table
Serve resource data to Sentinel via GET /{resourceType} endpoints with pagination and filtering support
Health check endpoint at GET /healthcheck returns 200 OK when database is accessible; used by Kubernetes liveness/readiness probes
Prometheus metrics are exposed at GET /metrics endpoint for monitoring; configure metric collection via --label-metrics-inclusion-duration
Files:
pkg/handlers/validation_test.go
pkg/{dao,handlers}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
List operations must support pagination via ListArgs struct with limit, offset, and search parameters
Files:
pkg/handlers/validation_test.go
pkg/{handlers,services,dao}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Errors must use structured ServiceError type from pkg/errors/ with HttpCode, Code, and Reason fields; send via errors.SendError(w, r, err)
Files:
pkg/handlers/validation_test.go
pkg/{handlers,services,db}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use context values for passing request-scoped data (session, operation ID, user info) rather than function parameters or global state
Files:
pkg/handlers/validation_test.go
🧠 Learnings (22)
📓 Common learnings
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: go.mod:32-36
Timestamp: 2026-01-13T04:19:50.012Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, the team has decided to keep OpenTelemetry core packages (go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/exporters/stdout/stdouttrace) at v1.38.0 rather than upgrading to v1.39.0 in PR #31.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/telemetry/otel.go:10-10
Timestamp: 2026-01-13T10:10:24.211Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository (PR #31), the team has decided to keep the OpenTelemetry semantic conventions package at v1.37.0 (go.opentelemetry.io/otel/semconv/v1.37.0) while using the SDK at v1.38.0 in pkg/telemetry/otel.go.
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
📚 Learning: 2026-01-13T04:16:08.743Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/middleware/masking.go:28-36
Timestamp: 2026-01-13T04:16:08.743Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, NewOCMLogger(ctx) is deprecated. Switch to the slog-based logger and use logger.With(ctx, ...) to produce structured, context-aware logs. Replace occurrences of NewOCMLogger(ctx) with logger.With(ctx, ...). Update imports to use the new logger and adjust any tests to expect structured logging behavior.
Applied to files:
pkg/util/utils.gopkg/api/presenters/object_reference.gopkg/api/presenters/path.gopkg/api/presenters/cluster.gopkg/errors/errors.gopkg/api/presenters/kind.gotest/integration/adapter_status_test.gopkg/api/presenters/cluster_test.gopkg/handlers/validation_test.gotest/integration/clusters_test.go
📚 Learning: 2026-01-08T09:40:10.138Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Applied to files:
pkg/api/presenters/path.gopkg/errors/errors.gopkg/api/presenters/cluster_test.goMakefiletest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Applied to files:
pkg/api/presenters/path.gopkg/errors/errors.gopkg/api/presenters/kind.gopkg/api/presenters/cluster_test.goMakefiletest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/services/**/*.go : Calculate aggregate status from adapter conditions and return in status.phase field (Ready, Failed, NotReady)
Applied to files:
pkg/api/presenters/cluster.gotest/integration/adapter_status_test.gopkg/api/presenters/cluster_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/services/**/*.go : Status aggregation: Phase is 'Ready' if all adapters report Ready=True, 'Failed' if any adapter reports Ready=False, 'NotReady' otherwise
Applied to files:
pkg/api/presenters/cluster.gotest/integration/adapter_status_test.gopkg/api/presenters/cluster_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{handlers,services,dao}/**/*.go : Errors must use structured ServiceError type from pkg/errors/ with HttpCode, Code, and Reason fields; send via errors.SendError(w, r, err)
Applied to files:
pkg/errors/errors.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Applied to files:
pkg/errors/errors.goMakefiletest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Applied to files:
pkg/errors/errors.gopkg/api/presenters/cluster_test.goMakefiletest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/serve/**/*.go : API server runs on port 8000, metrics endpoint on 8080, health check on 8083; these are configurable via CLI flags
Applied to files:
pkg/errors/errors.gotest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/migrate/**/*.go : Use GORM AutoMigrate at startup via ./bin/hyperfleet-api migrate command; migrations are idempotent and track applied migrations
Applied to files:
pkg/errors/errors.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/handlers/**/*.go : Accept status updates from adapters via POST /{resourceType}/{id}/statuses and store in adapter_statuses table
Applied to files:
test/integration/adapter_status_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/integration/**/*_test.go : All API endpoints must have integration test coverage in test/integration/ including CRUD operations, pagination, search, error cases, and status aggregation
Applied to files:
test/integration/adapter_status_test.gotest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,db}/**/*.go : Use polymorphic status tables with owner_type + owner_id to support multiple resource types without creating separate status tables
Applied to files:
test/integration/adapter_status_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/factories/**/*.go : Use test data factories in test/factories/ to provide consistent, reusable test data with builder pattern (e.g., NewClusterBuilder().WithName().Build())
Applied to files:
pkg/api/presenters/cluster_test.gopkg/handlers/validation_test.gotest/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/handlers/**/*.go : Health check endpoint at GET /healthcheck returns 200 OK when database is accessible; used by Kubernetes liveness/readiness probes
Applied to files:
pkg/api/presenters/cluster_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to **/*.go : Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/environments/**/*.go : Environment-based configuration: use OCM_ENV variable to select development, unit_testing, integration_testing, or production configuration
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Applied to files:
Makefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/**/*{auth,middleware}*.go : JWT authentication validates tokens from Red Hat SSO and extracts account ID and username from claims for authorization
Applied to files:
test/integration/clusters_test.go
🧬 Code graph analysis (7)
pkg/api/presenters/object_reference.go (1)
pkg/util/utils.go (1)
PtrString(59-59)
pkg/api/presenters/path.go (1)
pkg/util/utils.go (1)
PtrString(59-59)
pkg/errors/errors.go (1)
pkg/util/utils.go (1)
PtrString(59-59)
pkg/api/presenters/kind.go (1)
pkg/util/utils.go (1)
PtrString(59-59)
pkg/api/presenters/cluster_test.go (2)
pkg/util/utils.go (1)
PtrString(59-59)pkg/api/presenters/cluster.go (1)
ConvertCluster(15-56)
pkg/handlers/validation_test.go (1)
pkg/util/utils.go (1)
PtrString(59-59)
test/integration/clusters_test.go (2)
test/helper.go (2)
WithAuthToken(308-316)GetAccessTokenFromContext(300-305)pkg/util/utils.go (1)
PtrString(59-59)
🔇 Additional comments (26)
Makefile (6)
7-8: LGTM!The change to
?=assignment allows the Dockerfile to overrideCGO_ENABLEDfor Alpine-based dev images while maintaining the default FIPS-compliant setting. The comment clearly documents this intent.
27-27: LGTM!The
DEV_TAGdefinition correctly uses the deferred$(GIT_SHA)variable. Make's lazy evaluation ensures the forward reference works properly.
227-231: LGTM!The
generatetarget cleanly transitions from Docker-based openapi-generator to the bingo-managedoapi-codegen. The pattern of removing and recreating the output directory ensures no stale artifacts remain. Based on learnings,openapi/openapi.yamlis the source of truth.
243-244: LGTM!Good backward compatibility choice to alias
generate-vendortogeneraterather than removing it entirely. This prevents breaking existing workflows.
266-266: LGTM!The
cleantarget correctly includespkg/api/openapito match the new generation output location.
331-336: LGTM!The
image-devtarget correctly passesBASE_IMAGE=alpine:3.21for lighter dev images, which aligns with theCGO_ENABLED ?=change allowing override for Alpine compatibility.pkg/handlers/validation_test.go (2)
119-127: LGTM - loop variable capture is safe with Go 1.22+.Taking the address of
kindinside the loop (&kind) is safe because Go 1.22+ scopes loop variables per iteration. Since the project uses Go 1.24.9 per coding guidelines, this pattern works correctly.
98-107: LGTM - test correctly adapted to pointer-based Kind field.The test properly uses
util.PtrStringto construct the pointer value for theKindfield, aligning with the new oapi-codegen generated types.pkg/errors/errors.go (2)
185-191: LGTM - helper functions correctly migrated toutil.PtrString.The
CodeStrandHreffunctions maintain the same behavior, simply switching to the newutil.PtrStringhelper.
154-183: TheDetailsfield type assignment is correct. Test code inclusters_test.go:443-445confirms thatopenapi.Error.Detailsis a pointer to a slice with elements containingFieldandErrorfields as pointers to strings. The code correctly creates the anonymous struct slice and assigns its pointer to the field.test/integration/adapter_status_test.go (5)
16-25: LGTM - clean helper function for test data construction.The
newAdapterStatusRequesthelper consolidates adapter status creation with sensible defaults. Usingtime.Now()forObservedTimeis appropriate for integration tests.
38-62: LGTM - test correctly migrated to new API client pattern.The test properly uses the
WithResponseclient methods and validates theJSON201response body for the created status.
219-296: LGTM - thorough idempotency test correctly validates update-not-duplicate behavior.The test properly verifies that posting the same adapter twice updates the existing status rather than creating a duplicate, validating the upsert semantics of the adapter status endpoint.
298-406: LGTM - comprehensive pagination edge case coverage.The test covers important edge cases: empty dataset, page beyond total, single item, and pagination consistency (no duplicates/missing items). This aligns well with integration test guidelines.
376-402: The pagination loop implementation is correct.The
pageNum++operator at line 398 works properly.openapi.QueryParamsPageis a numeric type alias generated by oapi-codegen and supports the increment operator. The pattern is used consistently across multiple integration test files without compilation issues.Likely an incorrect or invalid review comment.
pkg/api/presenters/object_reference.go (1)
8-20: LGTM - presenter correctly migrated toutil.PtrString.The change is a straightforward migration from
openapi.PtrStringtoutil.PtrString, maintaining identical behavior for constructing theObjectReference.Idfield.pkg/api/presenters/kind.go (1)
1-44: LGTM!Clean migration from
openapi.PtrStringtoutil.PtrString. The helper function semantics are preserved, and the change aligns with the broader PR objective of decoupling from the generated openapi package utilities.pkg/api/presenters/path.go (1)
1-50: LGTM!Consistent migration pattern matching
kind.go. Theutil.PtrStringusage inObjectPathis correct.pkg/api/presenters/cluster.go (2)
38-42: Good defensive default for Kind.The fallback to
"Cluster"whenreq.Kindis nil ensures backward compatibility and prevents nil pointer dereference. This aligns well with the pointer-based Kind field in the new OpenAPI types.
127-146: The enum conversions are valid and safe. The domain types (api.ResourcePhaseandapi.ConditionStatus) are string-based enums with constants explicitly defined to match their OpenAPI counterparts. The code comments confirm this (e.g., "matches openapi.NOT_READY"), and comprehensive unit tests inpkg/api/presenters/adapter_status_test.goverify the mappings work correctly. No changes needed.pkg/api/presenters/cluster_test.go (2)
8-12: LGTM!Import additions for
openapi_typesandutilare correct and align with the presenter changes.
160-239: Comprehensive test coverage for PresentCluster.The test thoroughly validates:
- Basic field mapping (Id, Kind, Href, Name, Generation)
- Email type conversion for CreatedBy/UpdatedBy
- Spec and Labels unmarshaling
- Status phase and conditions conversion
- Timestamp preservation
Good coverage of the new OpenAPI type changes.
test/integration/clusters_test.go (4)
22-52: LGTM!The migration to
GetClusterByIdWithResponseis well-implemented with proper:
- Status code assertions via
resp.StatusCode()- Nil checks on
resp.JSON200before field access- Pointer dereferencing for
Id,Kind,Href
104-127: LGTM!Clean migration of paging tests to
GetClustersWithResponsewith properly constructedGetClustersParams. The use of type aliasesopenapi.QueryParamsPageandopenapi.QueryParamsPageSizealigns with the generated client types.
443-450: Good defensive nil handling for error details.The nested nil checks for
errorResponse.Detailsanddetail.Fieldproperly handle optional fields in the error response, preventing panics when iterating over validation error details.
54-87: LGTM!The POST test properly demonstrates:
- Using
PostClusterWithResponsewith typed request body- Extracting response via
JSON201with nil check- Fallback to raw resty request for invalid JSON testing
- Using
test.GetAccessTokenFromContexthelper for auth
| GIT_SHA ?= $(shell git rev-parse --short HEAD 2>/dev/null || echo "unknown") | ||
| GIT_DIRTY ?= $(shell git diff --quiet 2>/dev/null || echo "-modified") | ||
| build_version:=$(GIT_SHA)$(GIT_DIRTY) |
There was a problem hiding this comment.
Minor edge case: GIT_DIRTY may show false positive in non-git environments.
When git is unavailable (e.g., building from a tarball), git diff --quiet fails and triggers the fallback echo "-modified", resulting in unknown-modified version strings. Consider guarding the dirty check:
Suggested fix
-GIT_DIRTY ?= $(shell git diff --quiet 2>/dev/null || echo "-modified")
+GIT_DIRTY ?= $(shell git rev-parse --git-dir >/dev/null 2>&1 && { git diff --quiet 2>/dev/null || echo "-modified"; } || echo "")🤖 Prompt for AI Agents
In @Makefile around lines 81 - 83, GIT_DIRTY fallback produces false positives
when git is unavailable; update the Makefile so GIT_DIRTY is only set when git
exists and we're inside a work tree: check that git is callable and that `git
rev-parse --is-inside-work-tree` (or existence of .git) succeeds before running
`git diff --quiet`; otherwise set GIT_DIRTY to empty. Keep the existing GIT_SHA
and build_version concatenation but ensure GIT_DIRTY is "" (not "-modified") in
non-git environments so build_version becomes just the SHA or "unknown".
| $(container_tool) build \ | ||
| --platform linux/amd64 \ | ||
| --build-arg GIT_SHA=$(GIT_SHA) \ | ||
| --build-arg GIT_DIRTY=$(GIT_DIRTY) \ | ||
| -t $(IMAGE_REGISTRY)/$(IMAGE_NAME):$(IMAGE_TAG) . |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the Dockerfile
find . -name "Dockerfile" -type f | head -20Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Verify Dockerfile accepts GIT_SHA and GIT_DIRTY build arguments
echo "Checking Dockerfile for ARG declarations..."
rg -n "^ARG (GIT_SHA|GIT_DIRTY|BASE_IMAGE)" Dockerfile
echo ""
echo "Checking how these args are used in the build:"
rg -n "GIT_SHA|GIT_DIRTY" DockerfileRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 311
🏁 Script executed:
#!/bin/bash
# Read the full Dockerfile to see actual usage of the build args
cat -n Dockerfile | head -40
echo ""
echo "Searching for variable interpolation of these args:"
rg -n '\$\{?GIT_SHA\}?|\$\{?GIT_DIRTY\}?' DockerfileRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 1257
🏁 Script executed:
#!/bin/bash
# Check if there's more content in Dockerfile
wc -l Dockerfile
echo ""
echo "Get the rest of the file:"
tail -n 20 DockerfileRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 789
GIT_SHA and GIT_DIRTY build arguments are declared but unused in the Dockerfile.
The Dockerfile declares ARG GIT_SHA and ARG GIT_DIRTY (lines 6-7), but these arguments are never referenced anywhere in the build process. They are not embedded in ENV statements, LABEL entries, or used in any RUN commands. This means the version information passed from the Makefile is lost and won't be embedded in the container image. Add usage of these arguments (e.g., in a LABEL or ENV statement) to properly embed version metadata in the built image.
🤖 Prompt for AI Agents
In @Makefile around lines 303 - 307, The build passes GIT_SHA and GIT_DIRTY as
build args but the Dockerfile's ARG GIT_SHA and ARG GIT_DIRTY are never
consumed, so embed them into the image by referencing those args in the
Dockerfile (e.g., after ARG GIT_SHA / ARG GIT_DIRTY add a LABEL like
org.opencontainers.image.revision="$GIT_SHA" and a label or ENV for dirty state
such as org.example.git.dirty="$GIT_DIRTY"); update the Dockerfile to consume
ARG GIT_SHA and ARG GIT_DIRTY in a LABEL or ENV so the values supplied by the
Makefile build command are persisted in the image metadata.
|
|
||
| // List clusters without orderBy parameter - should default to created_time desc | ||
| list, _, err := client.DefaultAPI.GetClusters(ctx).Execute() | ||
| listResp, err := client.GetClustersWithResponse(ctx, nil, test.WithAuthToken(ctx)) | ||
| Expect(err).NotTo(HaveOccurred(), "Failed to list clusters") | ||
| list := listResp.JSON200 | ||
| Expect(list).NotTo(BeNil()) |
There was a problem hiding this comment.
Potential nil dereference when building createdClusters list.
Line 528 appends *resp.JSON201 to createdClusters. While the test expects StatusCreated, if the response unexpectedly contains a nil JSON201, this would panic. Consider adding a nil check for safety, consistent with patterns used elsewhere in this file.
🐛 Suggested fix
resp, err := client.PostClusterWithResponse(ctx, openapi.PostClusterJSONRequestBody(clusterInput), test.WithAuthToken(ctx))
Expect(err).NotTo(HaveOccurred(), "Failed to create cluster %d", i)
+ Expect(resp.JSON201).NotTo(BeNil(), "Expected JSON201 response for cluster %d", i)
createdClusters = append(createdClusters, *resp.JSON201)🤖 Prompt for AI Agents
In @test/integration/clusters_test.go around lines 533 - 538, The test may
append a nil pointer when building createdClusters by dereferencing
resp.JSON201; update the logic around where createdClusters is appended (the
code that does createdClusters = append(createdClusters, *resp.JSON201) after
creating clusters) to first assert or guard that resp != nil and resp.JSON201 !=
nil (use Expect(resp).NotTo(BeNil()) and Expect(resp.JSON201).NotTo(BeNil()) or
an if check) and only then append the dereferenced value, mirroring the
nil-safety pattern used elsewhere in this test file.
6e31497 to
8fba87c
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/api/presenters/cluster_test.go (1)
394-399: Pointer comparison at line 396 may not produce expected results.Comparing
result.Kind(returned fromPresentCluster) directly tooriginalReq.Kind(from test input) compares pointer addresses, not string values. Since these are different allocations, this comparison might fail unexpectedly.🔧 Suggested fix: Dereference pointers for value comparison
// Verify data integrity Expect(*result.Id).To(Equal("cluster-roundtrip-123")) - Expect(result.Kind).To(Equal(originalReq.Kind)) + Expect(*result.Kind).To(Equal(*originalReq.Kind)) Expect(result.Name).To(Equal(originalReq.Name))
🤖 Fix all issues with AI agents
In @Dockerfile:
- Line 19: The Dockerfile currently forces CGO_DISABLED by running the build
with "CGO_ENABLED=0", which prevents enabling boringcrypto for FIPS; update the
build invocation used to produce the binary (the RUN line that executes make
build) to export CGO_ENABLED=1 and set GOEXPERIMENT=boringcrypto so the Go build
can use cgo/boringcrypto for FIPS-compliant crypto, and ensure the Docker base
image is changed to one with glibc (not distroless/static) to support dynamic
linking; alternatively, if FIPS is not required, update the coding guidelines to
reflect CGO_ENABLED=0 as acceptable.
In @openapi/oapi-codegen.yaml:
- Around line 1-17: This file (oapi-codegen.yaml) uses oapi-codegen options like
generate.models and output-options.old-aliasing and must be removed and replaced
by a TypeSpec-based workflow: stop using oapi-codegen.yaml, add a TypeSpec API
spec at openapi/openapi.yaml (the only editable source of truth), configure
codegen to use openapi-generator-cli v7.16.0 in your build/CI scripts (replacing
any oapi-codegen invocation), and update generation settings to produce the
client/server/models equivalent to generate.models and client from the old
config; also remove or migrate any code that depended on oapi-codegen-specific
behaviors (e.g., old-merge-schemas/old-aliasing) so the project builds with the
new generator.
In @test/integration/clusters_test.go:
- Around line 301-303: The test accesses getResp.JSON200.Spec without ensuring
getResp.JSON200 is non-nil; add a nil check after the call to
GetClusterByIdWithResponse (check getResp and getResp.JSON200) and fail the test
or Expect them to be non-nil before asserting len(getResp.JSON200.Spec) == 100.
Specifically, after GetClusterByIdWithResponse and the existing err check, add
Expect(getResp).NotTo(BeNil()) and Expect(getResp.JSON200).NotTo(BeNil()) (or an
if guard that calls Fail/Expect) so accessing getResp.JSON200.Spec is safe.
- Around line 526-528: The test dereferences resp.JSON201 without verifying the
HTTP status or that JSON201 is non-nil, which can panic if the POST fails; after
the client.PostClusterWithResponse call and Err check, assert the response
status is 201 (or use resp.StatusCode/resp.Status) and add
Expect(resp.JSON201).NotTo(BeNil()) before appending, then safely append
*resp.JSON201 to createdClusters; reference resp, resp.JSON201 and
client.PostClusterWithResponse in the changes.
- Around line 233-236: The test accesses getResp.JSON200.Name without ensuring
JSON200 is non-nil; update the assertion after calling
GetClusterByIdWithResponse (in the block using GetClusterByIdWithResponse,
getResp and id1) to first verify getResp.JSON200 is not nil (e.g.,
Expect(getResp.JSON200).NotTo(BeNil())) before asserting on getResp.JSON200.Name
to avoid nil dereference.
- Around line 202-205: After calling GetClustersWithResponse, add a nil check
for resp.JSON200 before accessing Items to avoid a nil pointer dereference;
specifically, in the test after resp, err = client.GetClustersWithResponse(...),
confirm resp != nil and resp.JSON200 != nil (or handle non-200 cases) and only
then assert on len(resp.JSON200.Items) or use
Expect(resp.StatusCode()).To(Equal(200)) prior to inspecting JSON200 so the test
safely handles non-200 responses.
🧹 Nitpick comments (5)
pkg/handlers/validation_test.go (1)
119-127: Consider consistency and adding a nil test case.Two observations:
Style inconsistency: Uses
&kindwhile other test functions useutil.PtrString(). While safe in Go 1.22+, usingutil.PtrString(kind)would be more consistent.Missing edge case: There's no test for
Kind: nil. A nil pointer (field not provided) is semantically different from a pointer to an empty string (field explicitly set to empty), and the validator may need to handle both.Suggested improvements
for _, kind := range invalidKinds { req := openapi.ClusterCreateRequest{ - Kind: &kind, + Kind: util.PtrString(kind), }Consider adding a separate test for nil:
func TestValidateKind_Nil(t *testing.T) { RegisterTestingT(t) req := openapi.ClusterCreateRequest{ Kind: nil, } validator := validateKind(&req, "Kind", "kind", "Cluster") err := validator() Expect(err).ToNot(BeNil()) Expect(err.Reason).To(ContainSubstring("required")) }Dockerfile (1)
6-7: Unused build arguments.
GIT_SHAandGIT_DIRTYare declared but never used in the build. Either remove them or pass them to the build command (e.g., via ldflags for version embedding).♻️ Suggested fix if version embedding is intended
# Build binary -RUN CGO_ENABLED=0 GOOS=linux make build +RUN CGO_ENABLED=0 GOOS=linux GIT_SHA=${GIT_SHA} GIT_DIRTY=${GIT_DIRTY} make buildOr remove the unused ARGs if not needed.
pkg/api/presenters/adapter_status.go (1)
114-139: Consider extracting the inline metadata struct to reduce duplication.The inline struct is defined twice (lines 114-121 and 124-131). While this works, extracting it to a local named type would improve readability.
♻️ Optional refactor to reduce duplication
+ type metadataPayload struct { + Attempt *int32 `json:"attempt,omitempty"` + CompletedTime *time.Time `json:"completed_time,omitempty"` + Duration *string `json:"duration,omitempty"` + JobName *string `json:"job_name,omitempty"` + JobNamespace *string `json:"job_namespace,omitempty"` + StartedTime *time.Time `json:"started_time,omitempty"` + } + - var openapiMetadata *struct { - Attempt *int32 `json:"attempt,omitempty"` - CompletedTime *time.Time `json:"completed_time,omitempty"` - Duration *string `json:"duration,omitempty"` - JobName *string `json:"job_name,omitempty"` - JobNamespace *string `json:"job_namespace,omitempty"` - StartedTime *time.Time `json:"started_time,omitempty"` - } + var openapiMetadata *metadataPayload if metadata != nil { - openapiMetadata = &struct { - Attempt *int32 `json:"attempt,omitempty"` - CompletedTime *time.Time `json:"completed_time,omitempty"` - Duration *string `json:"duration,omitempty"` - JobName *string `json:"job_name,omitempty"` - JobNamespace *string `json:"job_namespace,omitempty"` - StartedTime *time.Time `json:"started_time,omitempty"` - }{ + openapiMetadata = &metadataPayload{ Attempt: metadata.Attempt, CompletedTime: metadata.CompletedTime, Duration: metadata.Duration, JobName: metadata.JobName, JobNamespace: metadata.JobNamespace, StartedTime: metadata.StartedTime, } }pkg/api/presenters/adapter_status_test.go (1)
261-270: Consider extracting repeated inline struct definition to a type alias or helper.The inline struct for
Metadatais defined identically in multiple test locations (lines 25-34, 86-88, and here). While acceptable for tests, a local type alias could reduce duplication and make future changes easier.💡 Optional: Extract metadata struct type
// At package level or in a test helper file type testAdapterStatusMetadata struct { Attempt *int32 `json:"attempt,omitempty"` CompletedTime *time.Time `json:"completed_time,omitempty"` Duration *string `json:"duration,omitempty"` JobName *string `json:"job_name,omitempty"` JobNamespace *string `json:"job_namespace,omitempty"` StartedTime *time.Time `json:"started_time,omitempty"` }test/integration/clusters_test.go (1)
584-587: Consider adding status check for cluster creation in loop.While the response is discarded here, adding a status check would catch creation failures early.
💡 Optional: Add status assertion
_, err := client.PostClusterWithResponse(ctx, openapi.PostClusterJSONRequestBody(clusterInput), test.WithAuthToken(ctx)) Expect(err).NotTo(HaveOccurred(), "Failed to create cluster %s", name) + // Consider: Expect(resp.StatusCode()).To(Equal(http.StatusCreated))
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.bingo/oapi-codegen.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum
📒 Files selected for processing (41)
.bingo/Variables.mk.bingo/oapi-codegen.mod.bingo/variables.env.claude/settings.local.json.dockerignoreDockerfileDockerfile.openapiDockerfile.openapi.dockerignoreMakefilego.modopenapi/oapi-codegen.yamlopenapi/openapi.yamlopenapitools.jsonpkg/api/adapter_status_types.gopkg/api/openapi_embed.gopkg/api/presenters/adapter_status.gopkg/api/presenters/adapter_status_test.gopkg/api/presenters/cluster.gopkg/api/presenters/cluster_test.gopkg/api/presenters/kind.gopkg/api/presenters/node_pool.gopkg/api/presenters/node_pool_test.gopkg/api/presenters/object_reference.gopkg/api/presenters/path.gopkg/errors/errors.gopkg/handlers/cluster.gopkg/handlers/cluster_nodepools.gopkg/handlers/cluster_nodepools_test.gopkg/handlers/node_pool.gopkg/handlers/openapi.gopkg/handlers/validation_test.gopkg/util/utils.gotest/factories/clusters.gotest/factories/node_pools.gotest/helper.gotest/integration/adapter_status_test.gotest/integration/api_contract_test.gotest/integration/clusters_test.gotest/integration/node_pools_test.gotest/integration/search_field_mapping_test.gotest/registration.go
💤 Files with no reviewable changes (4)
- Dockerfile.openapi.dockerignore
- Dockerfile.openapi
- pkg/api/openapi_embed.go
- openapitools.json
✅ Files skipped from review due to trivial changes (1)
- .claude/settings.local.json
🚧 Files skipped from review as they are similar to previous changes (10)
- test/integration/api_contract_test.go
- pkg/api/presenters/object_reference.go
- .bingo/Variables.mk
- .dockerignore
- pkg/api/adapter_status_types.go
- pkg/handlers/openapi.go
- pkg/handlers/node_pool.go
- pkg/util/utils.go
- test/registration.go
- pkg/handlers/cluster_nodepools.go
🧰 Additional context used
📓 Path-based instructions (9)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Use structured logging via logger.NewOCMLogger(ctx) with context-aware log fields including operation ID, account ID, and transaction ID
Files:
pkg/handlers/cluster.gopkg/errors/errors.gotest/factories/clusters.gotest/factories/node_pools.gotest/integration/adapter_status_test.gotest/integration/search_field_mapping_test.gotest/helper.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.gopkg/api/presenters/node_pool_test.gopkg/api/presenters/adapter_status_test.gotest/integration/clusters_test.gotest/integration/node_pools_test.gopkg/api/presenters/path.gopkg/api/presenters/adapter_status.gopkg/api/presenters/kind.gopkg/api/presenters/node_pool.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.go
pkg/handlers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/handlers/**/*.go: HTTP handlers must follow the pattern: parse request → call service/DAO → handle errors → send response with appropriate HTTP status codes
Database transactions are automatically managed by TransactionMiddleware for all HTTP requests; store session in context and auto-commit on success, rollback on error
Accept resource creation/update/delete requests via REST endpoints, validate, and persist to PostgreSQL
Accept status updates from adapters via POST /{resourceType}/{id}/statuses and store in adapter_statuses table
Serve resource data to Sentinel via GET /{resourceType} endpoints with pagination and filtering support
Health check endpoint at GET /healthcheck returns 200 OK when database is accessible; used by Kubernetes liveness/readiness probes
Prometheus metrics are exposed at GET /metrics endpoint for monitoring; configure metric collection via --label-metrics-inclusion-duration
Files:
pkg/handlers/cluster.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.go
pkg/{dao,handlers}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
List operations must support pagination via ListArgs struct with limit, offset, and search parameters
Files:
pkg/handlers/cluster.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.go
pkg/{handlers,services,dao}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Errors must use structured ServiceError type from pkg/errors/ with HttpCode, Code, and Reason fields; send via errors.SendError(w, r, err)
Files:
pkg/handlers/cluster.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.go
pkg/{handlers,services,db}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use context values for passing request-scoped data (session, operation ID, user info) rather than function parameters or global state
Files:
pkg/handlers/cluster.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.go
test/factories/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use test data factories in test/factories/ to provide consistent, reusable test data with builder pattern (e.g., NewClusterBuilder().WithName().Build())
Files:
test/factories/clusters.gotest/factories/node_pools.go
test/integration/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
test/integration/**/*_test.go: Integration tests must use Testcontainers to create isolated PostgreSQL instances for each test suite to ensure no state leakage
All API endpoints must have integration test coverage in test/integration/ including CRUD operations, pagination, search, error cases, and status aggregation
Files:
test/integration/adapter_status_test.gotest/integration/search_field_mapping_test.gotest/integration/clusters_test.gotest/integration/node_pools_test.go
pkg/api/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Files:
pkg/api/presenters/node_pool_test.gopkg/api/presenters/adapter_status_test.gopkg/api/presenters/path.gopkg/api/presenters/adapter_status.gopkg/api/presenters/kind.gopkg/api/presenters/node_pool.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.go
openapi/**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
openapi/**/*.yaml: Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Files:
openapi/oapi-codegen.yamlopenapi/openapi.yaml
🧠 Learnings (27)
📓 Common learnings
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/telemetry/otel.go:10-10
Timestamp: 2026-01-13T10:10:24.211Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository (PR #31), the team has decided to keep the OpenTelemetry semantic conventions package at v1.37.0 (go.opentelemetry.io/otel/semconv/v1.37.0) while using the SDK at v1.38.0 in pkg/telemetry/otel.go.
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: go.mod:32-36
Timestamp: 2026-01-13T04:19:50.012Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, the team has decided to keep OpenTelemetry core packages (go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/exporters/stdout/stdouttrace) at v1.38.0 rather than upgrading to v1.39.0 in PR #31.
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
📚 Learning: 2026-01-13T04:16:08.743Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/middleware/masking.go:28-36
Timestamp: 2026-01-13T04:16:08.743Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, NewOCMLogger(ctx) is deprecated. Switch to the slog-based logger and use logger.With(ctx, ...) to produce structured, context-aware logs. Replace occurrences of NewOCMLogger(ctx) with logger.With(ctx, ...). Update imports to use the new logger and adjust any tests to expect structured logging behavior.
Applied to files:
pkg/handlers/cluster.gopkg/errors/errors.gotest/factories/clusters.gotest/factories/node_pools.gotest/integration/adapter_status_test.gotest/integration/search_field_mapping_test.gotest/helper.gopkg/handlers/validation_test.gopkg/handlers/cluster_nodepools_test.gopkg/api/presenters/node_pool_test.gopkg/api/presenters/adapter_status_test.gotest/integration/clusters_test.gotest/integration/node_pools_test.gopkg/api/presenters/path.gopkg/api/presenters/adapter_status.gopkg/api/presenters/kind.gopkg/api/presenters/node_pool.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{handlers,services,dao}/**/*.go : Errors must use structured ServiceError type from pkg/errors/ with HttpCode, Code, and Reason fields; send via errors.SendError(w, r, err)
Applied to files:
pkg/errors/errors.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/openapi/model_*.go : Generated Go models from openapi-generator-cli include JSON tags and validation metadata; do not manually edit generated model_*.go files
Applied to files:
pkg/errors/errors.go.bingo/variables.envgo.modtest/helper.go.bingo/oapi-codegen.modpkg/api/presenters/node_pool_test.gopkg/api/presenters/path.goopenapi/oapi-codegen.yamlpkg/api/presenters/kind.goMakefilepkg/api/presenters/cluster_test.goDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Use TypeSpec for API specification, which compiles to OpenAPI 3.0.3, then generates Go models via openapi-generator-cli v7.16.0
Applied to files:
pkg/errors/errors.go.bingo/variables.envgo.mod.bingo/oapi-codegen.modpkg/api/presenters/node_pool_test.goopenapi/oapi-codegen.yamlpkg/api/presenters/kind.goMakefilepkg/api/presenters/cluster_test.goDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/api/**/*.go : Embed OpenAPI specification at compile time using Go 1.16+ //go:embed directive for self-contained binary deployment
Applied to files:
pkg/errors/errors.go.bingo/variables.envgo.modtest/helper.go.bingo/oapi-codegen.modpkg/api/presenters/node_pool_test.gotest/integration/clusters_test.gopkg/api/presenters/path.goopenapi/oapi-codegen.yamlpkg/api/presenters/kind.goMakefilepkg/api/presenters/cluster_test.goDockerfile
📚 Learning: 2026-01-08T09:40:10.138Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/errors/errors.go:9-9
Timestamp: 2026-01-08T09:40:10.138Z
Learning: In openshift-hyperfleet/hyperfleet-api, pkg/api/openapi is a generated directory (excluded in .gitignore) created by `make generate` using openapi-generator-cli via Docker; linters that run before code generation will report false-positive import errors for this package.
Applied to files:
pkg/errors/errors.go.bingo/variables.envgo.mod.bingo/oapi-codegen.modpkg/api/presenters/node_pool_test.gotest/integration/clusters_test.gopkg/api/presenters/path.goopenapi/oapi-codegen.yamlMakefilepkg/api/presenters/cluster_test.goDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/serve/**/*.go : API server runs on port 8000, metrics endpoint on 8080, health check on 8083; these are configurable via CLI flags
Applied to files:
pkg/errors/errors.gotest/integration/clusters_test.goDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/migrate/**/*.go : Use GORM AutoMigrate at startup via ./bin/hyperfleet-api migrate command; migrations are idempotent and track applied migrations
Applied to files:
pkg/errors/errors.goDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/factories/**/*.go : Use test data factories in test/factories/ to provide consistent, reusable test data with builder pattern (e.g., NewClusterBuilder().WithName().Build())
Applied to files:
test/factories/clusters.gotest/factories/node_pools.gotest/integration/clusters_test.gopkg/api/presenters/cluster_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to cmd/hyperfleet/environments/**/*.go : Environment-based configuration: use OCM_ENV variable to select development, unit_testing, integration_testing, or production configuration
Applied to files:
.bingo/variables.envMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to **/*.go : Use Go 1.24.9 with FIPS-compliant crypto enabled (CGO_ENABLED=1, GOEXPERIMENT=boringcrypto)
Applied to files:
.bingo/variables.envgo.modMakefileDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Use Podman to run openapi-generator-cli v7.16.0 via make generate to ensure version consistency and avoid environment-specific issues
Applied to files:
.bingo/variables.envMakefileDockerfile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{db/migrations,dao}/**/*.go : NodePool owner_references.id must enforce foreign key relationship to parent cluster to maintain referential integrity
Applied to files:
test/factories/node_pools.gopkg/handlers/cluster_nodepools_test.gopkg/api/presenters/node_pool_test.gotest/integration/node_pools_test.gopkg/api/presenters/node_pool.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/handlers/**/*.go : Accept status updates from adapters via POST /{resourceType}/{id}/statuses and store in adapter_statuses table
Applied to files:
test/integration/adapter_status_test.gopkg/api/presenters/adapter_status_test.gopkg/api/presenters/adapter_status.goopenapi/openapi.yaml
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/services/**/*.go : Calculate aggregate status from adapter conditions and return in status.phase field (Ready, Failed, NotReady)
Applied to files:
test/integration/adapter_status_test.gotest/integration/search_field_mapping_test.gopkg/api/presenters/node_pool_test.gopkg/api/presenters/adapter_status_test.gopkg/api/presenters/adapter_status.gopkg/api/presenters/node_pool.gopkg/api/presenters/cluster_test.goopenapi/openapi.yamlpkg/api/presenters/cluster.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/services/**/*.go : Status aggregation: Phase is 'Ready' if all adapters report Ready=True, 'Failed' if any adapter reports Ready=False, 'NotReady' otherwise
Applied to files:
test/integration/adapter_status_test.gotest/integration/search_field_mapping_test.gopkg/api/presenters/node_pool_test.gopkg/api/presenters/adapter_status_test.gopkg/api/presenters/adapter_status.gopkg/api/presenters/cluster_test.gopkg/api/presenters/cluster.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to test/integration/**/*_test.go : All API endpoints must have integration test coverage in test/integration/ including CRUD operations, pagination, search, error cases, and status aggregation
Applied to files:
test/integration/adapter_status_test.gotest/integration/search_field_mapping_test.gotest/integration/clusters_test.gotest/integration/node_pools_test.gopkg/api/presenters/cluster_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,services}/**/*.go : Increment the generation field on each spec update to enable optimistic concurrency control and track reconciliation progress
Applied to files:
go.mod.bingo/oapi-codegen.modopenapi/oapi-codegen.yamlMakefile
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/db/**/*.go : Use PostgreSQL 14.2 with GORM ORM for database operations
Applied to files:
go.mod
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,handlers}/**/*.go : List operations must support pagination via ListArgs struct with limit, offset, and search parameters
Applied to files:
.bingo/oapi-codegen.mod
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/**/*{auth,middleware}*.go : JWT authentication validates tokens from Red Hat SSO and extracts account ID and username from claims for authorization
Applied to files:
test/integration/clusters_test.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/{dao,db}/**/*.go : Use polymorphic status tables with owner_type + owner_id to support multiple resource types without creating separate status tables
Applied to files:
pkg/api/presenters/adapter_status.go
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to openapi/**/*.yaml : Only edit openapi/openapi.yaml as the source of truth; pkg/api/openapi/api/openapi.yaml is auto-generated and will be overwritten
Applied to files:
openapi/oapi-codegen.yamlMakefileDockerfileopenapi/openapi.yaml
📚 Learning: 2026-01-08T01:23:20.647Z
Learnt from: CR
Repo: openshift-hyperfleet/hyperfleet-api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T01:23:20.647Z
Learning: Applies to pkg/handlers/**/*.go : Health check endpoint at GET /healthcheck returns 200 OK when database is accessible; used by Kubernetes liveness/readiness probes
Applied to files:
pkg/api/presenters/cluster_test.go
📚 Learning: 2026-01-13T10:10:24.211Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: pkg/telemetry/otel.go:10-10
Timestamp: 2026-01-13T10:10:24.211Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository (PR #31), the team has decided to keep the OpenTelemetry semantic conventions package at v1.37.0 (go.opentelemetry.io/otel/semconv/v1.37.0) while using the SDK at v1.38.0 in pkg/telemetry/otel.go.
Applied to files:
openapi/openapi.yaml
📚 Learning: 2026-01-13T04:19:50.012Z
Learnt from: yasun1
Repo: openshift-hyperfleet/hyperfleet-api PR: 31
File: go.mod:32-36
Timestamp: 2026-01-13T04:19:50.012Z
Learning: In the openshift-hyperfleet/hyperfleet-api repository, the team has decided to keep OpenTelemetry core packages (go.opentelemetry.io/otel, go.opentelemetry.io/otel/sdk, go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/exporters/stdout/stdouttrace) at v1.38.0 rather than upgrading to v1.39.0 in PR #31.
Applied to files:
openapi/openapi.yaml
🧬 Code graph analysis (13)
pkg/handlers/cluster.go (1)
pkg/api/presenters/cluster.go (1)
ConvertCluster(15-56)
pkg/errors/errors.go (1)
pkg/util/utils.go (1)
PtrString(59-59)
test/factories/node_pools.go (1)
pkg/api/metadata_types.go (1)
Meta(37-42)
test/integration/search_field_mapping_test.go (1)
test/helper.go (1)
WithAuthToken(328-336)
pkg/handlers/validation_test.go (1)
pkg/util/utils.go (1)
PtrString(59-59)
pkg/api/presenters/node_pool_test.go (1)
pkg/api/presenters/node_pool.go (1)
ConvertNodePool(13-54)
pkg/api/presenters/adapter_status_test.go (2)
pkg/api/metadata_types.go (1)
Metadata(28-34)pkg/api/status_types.go (3)
ConditionTrue(20-20)ConditionFalse(21-21)ConditionUnknown(22-22)
test/integration/clusters_test.go (2)
test/helper.go (2)
WithAuthToken(328-336)GetAccessTokenFromContext(320-325)pkg/util/utils.go (1)
PtrString(59-59)
test/integration/node_pools_test.go (1)
test/helper.go (2)
WithAuthToken(328-336)GetAccessTokenFromContext(320-325)
pkg/api/presenters/path.go (1)
pkg/util/utils.go (1)
PtrString(59-59)
pkg/api/presenters/kind.go (1)
pkg/util/utils.go (1)
PtrString(59-59)
pkg/api/presenters/node_pool.go (2)
pkg/api/status_types.go (2)
ConditionStatus(17-17)ResourcePhase(7-7)pkg/api/node_pool_types.go (1)
NodePool(12-43)
pkg/api/presenters/cluster_test.go (2)
pkg/util/utils.go (1)
PtrString(59-59)pkg/api/presenters/cluster.go (1)
ConvertCluster(15-56)
🔇 Additional comments (77)
pkg/handlers/validation_test.go (3)
8-8: LGTM!Import addition is appropriate for accessing the
util.PtrStringhelper.
98-107: LGTM!Correctly adapts to the pointer-typed
Kindfield using the utility helper.
129-151: LGTM!Both tests correctly adapt to the pointer-typed
Kindfield and maintain proper error message assertions..bingo/variables.env (2)
17-18: LGTM - oapi-codegen tool addition aligns with PR objectives.The addition of
oapi-codegen-v2.5.1to the bingo toolchain is appropriate for replacing the Java-based OpenAPI Generator with a Go-native solution.
11-11: No action required for golangci-lint v2.7.0 upgrade.The repository's
.golangci.ymlalready uses v2 configuration format (declared withversion: 2) and is fully compatible with golangci-lint v2.7.0.test/factories/node_pools.go (1)
34-41: LGTM - Test factory updated with required metadata fields.The addition of
CreatedByandUpdatedByfields with test email values aligns with the OpenAPI type changes introduced by oapi-codegen. This ensures test data includes the new mandatory metadata fields.pkg/errors/errors.go (2)
186-192: LGTM - Consistent migration to util.PtrString.The
CodeStrandHrefhelpers now useutil.PtrStringinstead ofopenapi.PtrString, aligning with the broader migration pattern across presenters.
155-183: The anonymous struct approach is correct and idiomatic. The OpenAPI specification defines thedetailsfield as an array of inline objects without a named type, so using an anonymous struct is the appropriate Go pattern. There is noErrorDetailsInnertype to use instead, and the current implementation is already type-safe and maintainable.pkg/api/presenters/kind.go (2)
6-8: LGTM - Clean migration to util package for pointer helpers.Moving from
openapi.PtrStringtoutil.PtrStringproperly decouples presenter logic from generated code, which improves maintainability when regenerating OpenAPI types.
29-44: LGTM - ObjectKind function unchanged except for pointer helper.The function logic is preserved; only the pointer-wrapping helper source changed. This is consistent with the broader refactoring pattern across presenters.
go.mod (1)
23-23: Version compatibility confirmed.The
oapi-codegen/runtime v1.1.2is appropriate foroapi-codegen v2.5.1. The v2.x generator produces code that uses the separate runtime module, and runtime v1.1.2 is the compatible version for generated imports. The Go version (1.24.0) meets the v2.5.1 requirement of Go >= 1.21.pkg/api/presenters/path.go (1)
7-7: LGTM!Good refactor to use a local
util.PtrStringhelper instead of relying on generated code. This decouples presenters from the OpenAPI generator implementation details and improves maintainability.Also applies to: 34-34
Dockerfile (1)
3-4: Go version inconsistency with project requirements.The Dockerfile uses
golang:1.25, but the coding guidelines specify Go 1.24.9. Ensure this is intentional and update either the Dockerfile or guidelines for consistency.pkg/api/presenters/node_pool_test.go (1)
9-9: LGTM!The test file is properly updated to use oapi-codegen's semantic types (
openapi_types.Email) and the new PascalCase enum values (openapi.Ready,openapi.True,openapi.NotReady). The changes correctly reflect the shift from the old generator's conventions to oapi-codegen's Go-idiomatic output.Also applies to: 212-213, 227-227, 231-231
test/helper.go (3)
265-277: LGTM!Clean implementation of the new oapi-codegen client pattern. The helper correctly constructs the server URL from configuration and uses
T.Fatalffor immediate test failure on client creation errors.
308-336: Well-designed authentication helper pattern.The context-based token storage with type-safe key and the
WithAuthTokenRequestEditorFnfollow oapi-codegen's recommended patterns for request customization.Minor note: On line 329, the inner context parameter
_is correctly ignored since the token comes from the outerctxcaptured in the closure.
584-592: Improved error handling API.Changing
OpenapiErrorto accept[]byteinstead oferroris a better design - it directly handles the response body from failed API calls without requiring the caller to extract the body from an error type first..bingo/oapi-codegen.mod (1)
1-5: oapi-codegen v2.5.1 is the latest stable version — no action needed.The Go version in the bingo module (1.25.0) differs from the project requirement (1.24.9 with FIPS crypto), but this is acceptable since bingo-managed tool modules are isolated containers used only during code generation and don't affect the main project's build or compliance posture.
test/integration/search_field_mapping_test.go (6)
34-44: LGTM! New API client pattern correctly implemented.The transition to
GetClustersWithResponsewithSearchParamstype conversion, status code checking viaresp.StatusCode(), and nil-guarded JSON200 payload access follows the oapi-codegen client pattern correctly.
86-96: LGTM! Time-based search query correctly formatted.RFC3339 formatting for date-time comparison and proper response handling pattern.
136-146: LGTM! Labels query and assertion pattern is correct.Safe nil-check on
item.Labelsbefore dereferencing, and proper use ofHaveKeyWithValuematcher.Also applies to: 155-157
175-184: LGTM! Error response validation is correct.The test correctly expects
errto be nil (HTTP transport succeeded) while checkingresp.StatusCode()for the application-level 400 error.
230-248: LGTM! Combined query test correctly validates AND condition behavior.
275-293: LGTM! NodePool field mapping tests are comprehensive.Tests cover both status.phase and labels mapping for NodePools, maintaining consistency with Cluster test patterns.
Also applies to: 306-316
pkg/api/presenters/adapter_status.go (3)
43-47: LGTM! Pointer dereference for Data is correct.Safe handling of the pointer-based
req.Datawith nil check before dereference.
88-94: LGTM! Condition status conversion is correct.Direct type conversion between
api.ConditionStatusandopenapi.ConditionStatusis valid since both are string-based with matching enum values.
97-103: LGTM! Data handling correctly returns nil-pointer for empty data.When
adapterStatus.Datais empty, returning&data(pointer to nil map) serializes tonullin JSON, which aligns with the optionaldatafield in the OpenAPI spec.Also applies to: 156-156
openapi/openapi.yaml (10)
1-4: LGTM! Version bump reflects schema refactoring.Version 1.0.2 appropriately reflects the significant schema changes in this PR.
22-27: LGTM! SearchParams placement is consistent across all list endpoints.Also applies to: 118-122, 266-270, 335-339, 359-363
427-455: LGTM! AdapterCondition schema is well-defined.Explicit field definitions with clear descriptions and appropriate required fields.
456-548: LGTM! AdapterStatus schema is comprehensive with helpful examples.
549-597: LGTM! AdapterStatusCreateRequest properly requires observed_time.Making
observed_timerequired ensures adapters provide accurate timing information for status reports.
627-651: LGTM! List schemas have consistent pagination structure.All list schemas (AdapterStatusList, ClusterList, NodePoolList) share a uniform pagination pattern with kind, page, size, total, and items.
Also applies to: 819-842, 1132-1155
695-783: LGTM! Cluster schemas correctly separate required fields.Cluster response requires server-generated fields (timestamps, status, generation), while ClusterCreateRequest only requires user-provided fields (name, spec).
Also applies to: 784-818
857-859: LGTM! ClusterStatus.phase directly references ResourcePhase.Clean direct reference instead of allOf inheritance improves code generation.
946-1041: LGTM! NodePool schemas follow consistent patterns with Cluster schemas.Also applies to: 1042-1075, 1076-1131, 1162-1197
1219-1264: LGTM! ResourceCondition includes appropriate API-managed tracking fields.ResourceCondition correctly includes
observed_generation,created_time, andlast_updated_timefor tracking adapter-reported state, distinct from AdapterCondition which is the input format.test/factories/clusters.go (2)
24-25: LGTM! CreatedBy and UpdatedBy fields properly initialized in test factory.Test data factory correctly sets these new required fields with a consistent test email.
64-141: LGTM! Helper factory functions correctly manage test data creation.Functions properly create clusters via service, then use direct DB updates for test-specific field manipulation, and reload to ensure consistency.
pkg/handlers/cluster_nodepools_test.go (2)
59-67: LGTM! Mock NodePool updated with new required fields.Test correctly populates all new required fields (Spec, Labels, StatusConditions, CreatedBy, UpdatedBy) needed for the presenter to generate valid responses.
181-189: LGTM! Test assertions appropriately verify handler behavior.Assertions focus on verifying the handler correctly retrieves and presents the NodePool. Full payload validation is delegated to integration tests.
pkg/handlers/cluster.go (1)
42-42: System user identifier correctly updated to email format.The change from
"system"to"[email protected]"aligns with the OpenAPI spec'sformat: emailconstraint forcreated_byandupdated_byfields.This identifier is used in 3 handler locations (cluster, node_pool, cluster_nodepools). Consider extracting to a package-level constant for maintainability:
const SystemUserEmail = "[email protected]"pkg/api/presenters/node_pool.go (2)
112-124: LGTM! Condition mapping correctly updated for new OpenAPI types.The conversion from domain
ResourceConditiontoopenapi.ResourceConditionnow includes all required fields (LastTransitionTime,ObservedGeneration,Reason,Status,Type) with proper type casting forConditionStatus.
126-151: LGTM! Complete NodePool construction with proper pointer handling.The result construction correctly:
- Uses
toEmailhelper for email-typed fields- Handles pointer fields (
Href,Id,Kind,Labels) appropriately- Constructs
OwnerReferenceswith proper pointer semantics- Maps status fields including
Phasewith type conversiontest/integration/adapter_status_test.go (3)
16-25: LGTM! Clean helper function for test setup.The
newAdapterStatusRequesthelper consolidates test setup and ensures consistentObservedTimeinitialization usingtime.Now().
376-402: LGTM! Pagination consistency test properly validates no duplicates across pages.The test correctly iterates through pages, collecting all adapters and verifying uniqueness. The safety limit at page 10 prevents infinite loops.
27-62: LGTM! Test properly validates cluster status creation with new API patterns.The test correctly uses the new
PostClusterStatusesWithResponsemethod and validates the response viaresp.JSON201and typed status constants (openapi.True).pkg/api/presenters/cluster.go (3)
58-61: LGTM! Simple and effective email type conversion helper.The
toEmailhelper provides a clean abstraction for converting string toopenapi_types.Email. Being unexported is appropriate as it's only used within the presenters package.
38-45: LGTM! Proper default Kind handling for cluster creation.The code correctly defaults to
"Cluster"whenreq.Kindis nil, matching the pattern used inConvertNodePool.
127-146: LGTM! Complete Cluster construction with consistent patterns.The result construction correctly mirrors the
PresentNodePoolpattern, using appropriate type helpers (toEmail,util.PtrString) and pointer semantics for optional fields.Makefile (3)
81-83: LGTM! Robust version info extraction with fallbacks.The
GIT_SHAandGIT_DIRTYvariables handle edge cases gracefully:
GIT_SHAfalls back to "unknown" if git failsGIT_DIRTYonly sets the suffix if there are uncommitted changes
243-245: Good backward compatibility forgenerate-vendortarget.Maintaining this as an alias to
generateensures existing scripts/documentation continue to work while the underlying implementation changes.
227-231: LGTM! Clean migration to oapi-codegen for OpenAPI generation.The generate target now uses the bingo-managed
$(OAPI_CODEGEN)tool (v2.5.1) with proper configuration. This removes the Docker/Java dependency and simplifies the build process. Theopenapi/oapi-codegen.yamlconfiguration file is in place with appropriate settings for code generation.test/integration/node_pools_test.go (5)
61-77: LGTM! NodePool creation test properly updated for new API patterns.The test correctly:
- Uses a local
kindvariable with pointer reference (&kind)- Migrates to
CreateNodePoolWithResponsewithtest.WithAuthToken(ctx)- Validates response via
resp.JSON201andresp.StatusCode()
115-128: LGTM! Paging test correctly uses new typed query parameters.The test properly constructs
GetNodePoolsParamswith typedPageandPageSizeparameters and validates all pagination fields in the response.
140-152: LGTM! Search test properly validates filtering by ID.The test correctly uses
openapi.SearchParamstype and validates that the search returns exactly the expected nodepool.
179-231: LGTM! Comprehensive test coverage for nodepool retrieval by cluster and ID.The test thoroughly validates:
- Successful retrieval (200 OK)
- Non-existent nodepool (404)
- Non-existent cluster (404)
- Cross-cluster access prevention (404)
This ensures proper cluster-scoped authorization.
234-270: LGTM! Validation test correctly verifies empty kind rejection.Using raw
restyto send malformed JSON that the generated client wouldn't allow is the right approach for testing server-side validation. The test properly validates the error response contains the expected message.pkg/api/presenters/adapter_status_test.go (5)
22-44: LGTM - Test data construction correctly adapted to new oapi-codegen types.The changes properly handle:
Dataas*map[string]interface{}(pointer to map)Metadataas an inline anonymous struct pointer matching the generated typeStatusenum updated from uppercase (TRUE) to PascalCase (True)
86-91: Inline struct for metadata unmarshaling is appropriate.The struct matches the generated type shape and properly validates that
JobNameis preserved through the conversion.
299-302: Pointer dereferencing for Data map is correct.The nil check on line 299 guards against nil pointer dereference before accessing map contents.
205-234: Condition status conversion tests correctly updated for new enum values.The test cases align with the domain types (
api.ConditionTrue = "True") frompkg/api/status_types.goand the new oapi-codegen generated enum values.
451-459: Round-trip test correctly verifies data and metadata preservation.The pointer dereferencing and type assertions are appropriate given the preceding nil checks and test setup.
pkg/api/presenters/cluster_test.go (5)
8-12: New imports correctly added for openapi_types and util packages.The imports align with the new type requirements:
openapi_typesforCreatedBy/UpdatedByassertionsutilforPtrStringhelper used inKindfield construction
16-28: Test request construction correctly adapted to pointer-based fields.The changes properly use:
&labelsfor theLabelspointer fieldutil.PtrString("Cluster")for the optionalKindfieldThis matches the relevant code snippet showing
ConvertClusterhandles nilKindwith a default value.
217-218: Email type assertions correctly match the new openapi_types.Email wrapper.The
PresentClusterfunction converts domain string fields toopenapi_types.Email, and tests appropriately wrap expected values.
227-231: Status phase and condition status enum values correctly updated.The new enum values (
openapi.Ready,openapi.True) align with oapi-codegen conventions.
40-49: Domain object assertions correctly use plain strings.The
ConvertClusterfunction returns*api.ClusterwithCreatedBy/UpdatedByas strings (see relevant snippet lines 14-55). The Email type wrapping happens only inPresentCluster.test/integration/clusters_test.go (9)
14-20: Imports correctly updated for new client patterns.The addition of
utilpackage import supports thePtrStringhelper for pointer-basedKindfield construction.
29-52: TestClusterGet correctly migrated to WithResponse pattern.The test properly:
- Uses
GetClusterByIdWithResponsewith status code checks- Extracts typed response via
resp.JSON200- Includes nil check before accessing response fields
- Dereferences pointer fields (
*clusterOutput.Kind)
60-87: TestClusterPost correctly handles creation flow and auth fallback.The test properly uses:
PostClusterWithResponsefor typed responseutil.PtrString("Cluster")for Kind fieldtest.GetAccessTokenFromContext(ctx)for resty requests
113-127: Paging parameters correctly constructed using type aliases.The use of
openapi.QueryParamsPageandopenapi.QueryParamsPageSizetype aliases ensures type safety with the generated client.
138-150: Search test correctly uses SearchParams type alias.The pattern is consistent with other parameterized queries.
339-343: LGTM - Status code check before JSON201 access is appropriate.The pattern of checking
StatusCode() == 201before accessingJSON201is correct and ensures the typed response is populated.
443-450: Error details iteration correctly handles nil pointer.The nil check on
errorResponse.Detailsat line 443 before iterating prevents nil pointer dereference.
674-741: LGTM - Kind validation tests are comprehensive.The tests properly verify that empty and wrong
kindvalues return 400 with appropriate error messages. Using resty allows testing invalid JSON structures that can't be constructed with the typed client.
22-52: Integration test coverage follows coding guidelines.The tests cover CRUD operations, pagination, search, and error cases as required by the coding guidelines for
test/integration/**/*_test.go.
|
|
||
| # Build binary | ||
| RUN CGO_ENABLED=0 GOOS=linux go build -ldflags="-w -s" -o hyperfleet-api ./cmd/hyperfleet-api | ||
| RUN CGO_ENABLED=0 GOOS=linux make build |
There was a problem hiding this comment.
CGO_ENABLED=0 conflicts with FIPS-compliant crypto requirements.
The coding guidelines specify CGO_ENABLED=1, GOEXPERIMENT=boringcrypto for FIPS-compliant crypto. Setting CGO_ENABLED=0 disables cgo, which prevents boringcrypto from being used.
If FIPS compliance is required for production, this needs to be updated. If FIPS is not required, the coding guidelines should be updated for consistency.
🔧 Proposed fix for FIPS compliance
-RUN CGO_ENABLED=0 GOOS=linux make build
+RUN CGO_ENABLED=1 GOOS=linux GOEXPERIMENT=boringcrypto make buildNote: This also requires a base image with glibc (not distroless/static) since CGO requires dynamic linking.
📝 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.
| RUN CGO_ENABLED=0 GOOS=linux make build | |
| RUN CGO_ENABLED=1 GOOS=linux GOEXPERIMENT=boringcrypto make build |
🤖 Prompt for AI Agents
In @Dockerfile at line 19, The Dockerfile currently forces CGO_DISABLED by
running the build with "CGO_ENABLED=0", which prevents enabling boringcrypto for
FIPS; update the build invocation used to produce the binary (the RUN line that
executes make build) to export CGO_ENABLED=1 and set GOEXPERIMENT=boringcrypto
so the Go build can use cgo/boringcrypto for FIPS-compliant crypto, and ensure
the Docker base image is changed to one with glibc (not distroless/static) to
support dynamic linking; alternatively, if FIPS is not required, update the
coding guidelines to reflect CGO_ENABLED=0 as acceptable.
| # oapi-codegen configuration | ||
| # See: https://github.com/oapi-codegen/oapi-codegen | ||
|
|
||
| package: openapi | ||
| output: pkg/api/openapi/openapi.gen.go | ||
| generate: | ||
| models: true | ||
| chi-server: false | ||
| client: true | ||
| embedded-spec: true | ||
| output-options: | ||
| skip-prune: false | ||
| compatibility: | ||
| # Use old allOf merge behavior where schemas are inlined | ||
| old-merge-schemas: true | ||
| # Use old behavior generating type definitions instead of aliases | ||
| old-aliasing: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for existing code generation configurations
git ls-files | grep -E "(codegen|openapi|spec)" | head -20Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 259
🏁 Script executed:
# Check the output directory structure mentioned in the config
find . -type d -name "openapi" 2>/dev/null | head -10Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 88
🏁 Script executed:
# Look for TypeSpec files
fd -e "tsp" -o -name "*spec*" 2>/dev/null | head -20Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 61
🏁 Script executed:
# Check for existing references to openapi-generator or oapi-codegen in build/config files
rg "openapi-generator|oapi-codegen" --type yaml --type toml --type json -lRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 104
🏁 Script executed:
# Check the contents of the .bingo directory and how oapi-codegen is pinned
cat .bingo/oapi-codegen.modRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 242
🏁 Script executed:
# Check Makefile or build scripts for code generation
fd -e "Makefile" -o -e "makefile" -e "*.mk" | xargs grep -l "codegen\|openapi" 2>/dev/nullRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 197
🏁 Script executed:
# Check what currently exists in the pkg/api/openapi directory
find pkg/api/openapi -type f 2>/dev/null | head -20Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 61
🏁 Script executed:
# Check the openapi.yaml file to understand the spec structure
head -50 openapi/openapi.yamlRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 1687
🏁 Script executed:
# Try to find build files more directly
find . -name "Makefile" -o -name "*.mk" 2>/dev/null | head -5Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 111
🏁 Script executed:
# Check if oapi-codegen.yaml is new or modified
git status openapi/oapi-codegen.yaml 2>/dev/null || echo "File status unknown"
git log --oneline openapi/oapi-codegen.yaml 2>/dev/null | head -3 || echo "No git history"Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 170
🏁 Script executed:
# Search for how code generation is invoked in build files
rg "oapi-codegen|openapi-generator" --type go --type sh -A 2 -B 2 2>/dev/null | head -40Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 1140
🏁 Script executed:
# Check README for setup/build instructions
find . -maxdepth 2 -name "README*" -o -name "CONTRIBUTING*" | xargs grep -l "codegen\|generate\|build" 2>/dev/nullRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 109
🏁 Script executed:
# Check the Makefile for code generation setup
cat Makefile | grep -A 10 -B 2 "codegen\|generate\|openapi"Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 2974
🏁 Script executed:
# Look at the README to understand project structure and build process
cat README.md | head -100Repository: openshift-hyperfleet/hyperfleet-api
Length of output: 2910
🏁 Script executed:
# Check .bingo/Variables.mk to see how oapi-codegen is invoked
cat .bingo/Variables.mkRepository: openshift-hyperfleet/hyperfleet-api
Length of output: 3026
Diverges from coding guidelines: Project must migrate from oapi-codegen to openapi-generator-cli v7.16.0 with TypeSpec.
This configuration uses oapi-codegen v2.5.1 and raw OpenAPI 3.0.0, but the coding guidelines specify:
- API specification must use TypeSpec
- Code generation must use openapi-generator-cli v7.16.0
- Only openapi/openapi.yaml is editable as the source of truth
The project has drifted from this prescribed toolchain. The oapi-codegen.yaml configuration should be replaced with TypeSpec sources and openapi-generator-cli v7.16.0 setup as per guidelines.
🤖 Prompt for AI Agents
In @openapi/oapi-codegen.yaml around lines 1 - 17, This file (oapi-codegen.yaml)
uses oapi-codegen options like generate.models and output-options.old-aliasing
and must be removed and replaced by a TypeSpec-based workflow: stop using
oapi-codegen.yaml, add a TypeSpec API spec at openapi/openapi.yaml (the only
editable source of truth), configure codegen to use openapi-generator-cli
v7.16.0 in your build/CI scripts (replacing any oapi-codegen invocation), and
update generation settings to produce the client/server/models equivalent to
generate.models and client from the old config; also remove or migrate any code
that depended on oapi-codegen-specific behaviors (e.g.,
old-merge-schemas/old-aliasing) so the project builds with the new generator.
| resp, err = client.GetClustersWithResponse(ctx, params, test.WithAuthToken(ctx)) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(len(validList.Items)).To(BeNumerically(">=", 0)) | ||
| Expect(len(resp.JSON200.Items)).To(BeNumerically(">=", 0)) | ||
| } |
There was a problem hiding this comment.
Potential nil pointer dereference at line 204.
After GetClustersWithResponse, resp.JSON200 could be nil if the response status is not 200. While err == nil is checked, a non-200 status doesn't set JSON200.
🔧 Suggested fix: Add nil check before accessing JSON200
resp, err = client.GetClustersWithResponse(ctx, params, test.WithAuthToken(ctx))
Expect(err).NotTo(HaveOccurred())
- Expect(len(resp.JSON200.Items)).To(BeNumerically(">=", 0))
+ Expect(resp.JSON200).NotTo(BeNil())
+ Expect(len(resp.JSON200.Items)).To(BeNumerically(">=", 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.
| resp, err = client.GetClustersWithResponse(ctx, params, test.WithAuthToken(ctx)) | |
| Expect(err).NotTo(HaveOccurred()) | |
| Expect(len(validList.Items)).To(BeNumerically(">=", 0)) | |
| Expect(len(resp.JSON200.Items)).To(BeNumerically(">=", 0)) | |
| } | |
| resp, err = client.GetClustersWithResponse(ctx, params, test.WithAuthToken(ctx)) | |
| Expect(err).NotTo(HaveOccurred()) | |
| Expect(resp.JSON200).NotTo(BeNil()) | |
| Expect(len(resp.JSON200.Items)).To(BeNumerically(">=", 0)) | |
| } |
🤖 Prompt for AI Agents
In @test/integration/clusters_test.go around lines 202 - 205, After calling
GetClustersWithResponse, add a nil check for resp.JSON200 before accessing Items
to avoid a nil pointer dereference; specifically, in the test after resp, err =
client.GetClustersWithResponse(...), confirm resp != nil and resp.JSON200 != nil
(or handle non-200 cases) and only then assert on len(resp.JSON200.Items) or use
Expect(resp.StatusCode()).To(Equal(200)) prior to inspecting JSON200 so the test
safely handles non-200 responses.
| getResp, err := client.GetClusterByIdWithResponse(ctx, id1, nil, test.WithAuthToken(ctx)) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(retrieved1.Name).To(Equal("duplicate-name-test")) | ||
| Expect(getResp.JSON200.Name).To(Equal("duplicate-name-test")) | ||
| } |
There was a problem hiding this comment.
Missing nil check before accessing JSON200 at line 235.
For consistency and defensive coding, add a nil check on getResp.JSON200 before accessing .Name.
🔧 Suggested fix: Add nil check
getResp, err := client.GetClusterByIdWithResponse(ctx, id1, nil, test.WithAuthToken(ctx))
Expect(err).NotTo(HaveOccurred())
+ Expect(getResp.JSON200).NotTo(BeNil())
Expect(getResp.JSON200.Name).To(Equal("duplicate-name-test"))📝 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.
| getResp, err := client.GetClusterByIdWithResponse(ctx, id1, nil, test.WithAuthToken(ctx)) | |
| Expect(err).NotTo(HaveOccurred()) | |
| Expect(retrieved1.Name).To(Equal("duplicate-name-test")) | |
| Expect(getResp.JSON200.Name).To(Equal("duplicate-name-test")) | |
| } | |
| getResp, err := client.GetClusterByIdWithResponse(ctx, id1, nil, test.WithAuthToken(ctx)) | |
| Expect(err).NotTo(HaveOccurred()) | |
| Expect(getResp.JSON200).NotTo(BeNil()) | |
| Expect(getResp.JSON200.Name).To(Equal("duplicate-name-test")) | |
| } |
🤖 Prompt for AI Agents
In @test/integration/clusters_test.go around lines 233 - 236, The test accesses
getResp.JSON200.Name without ensuring JSON200 is non-nil; update the assertion
after calling GetClusterByIdWithResponse (in the block using
GetClusterByIdWithResponse, getResp and id1) to first verify getResp.JSON200 is
not nil (e.g., Expect(getResp.JSON200).NotTo(BeNil())) before asserting on
getResp.JSON200.Name to avoid nil dereference.
| getResp, err := client.GetClusterByIdWithResponse(ctx, *resp.JSON201.Id, nil, test.WithAuthToken(ctx)) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(len(retrieved.Spec)).To(Equal(100)) | ||
| Expect(len(getResp.JSON200.Spec)).To(Equal(100)) |
There was a problem hiding this comment.
Missing nil check before accessing JSON200.Spec at line 303.
🔧 Suggested fix: Add nil check
getResp, err := client.GetClusterByIdWithResponse(ctx, *resp.JSON201.Id, nil, test.WithAuthToken(ctx))
Expect(err).NotTo(HaveOccurred())
+ Expect(getResp.JSON200).NotTo(BeNil())
Expect(len(getResp.JSON200.Spec)).To(Equal(100))📝 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.
| getResp, err := client.GetClusterByIdWithResponse(ctx, *resp.JSON201.Id, nil, test.WithAuthToken(ctx)) | |
| Expect(err).NotTo(HaveOccurred()) | |
| Expect(len(retrieved.Spec)).To(Equal(100)) | |
| Expect(len(getResp.JSON200.Spec)).To(Equal(100)) | |
| getResp, err := client.GetClusterByIdWithResponse(ctx, *resp.JSON201.Id, nil, test.WithAuthToken(ctx)) | |
| Expect(err).NotTo(HaveOccurred()) | |
| Expect(getResp.JSON200).NotTo(BeNil()) | |
| Expect(len(getResp.JSON200.Spec)).To(Equal(100)) |
🤖 Prompt for AI Agents
In @test/integration/clusters_test.go around lines 301 - 303, The test accesses
getResp.JSON200.Spec without ensuring getResp.JSON200 is non-nil; add a nil
check after the call to GetClusterByIdWithResponse (check getResp and
getResp.JSON200) and fail the test or Expect them to be non-nil before asserting
len(getResp.JSON200.Spec) == 100. Specifically, after GetClusterByIdWithResponse
and the existing err check, add Expect(getResp).NotTo(BeNil()) and
Expect(getResp.JSON200).NotTo(BeNil()) (or an if guard that calls Fail/Expect)
so accessing getResp.JSON200.Spec is safe.
| resp, err := client.PostClusterWithResponse(ctx, openapi.PostClusterJSONRequestBody(clusterInput), test.WithAuthToken(ctx)) | ||
| Expect(err).NotTo(HaveOccurred(), "Failed to create cluster %d", i) | ||
| createdClusters = append(createdClusters, *cluster) | ||
| createdClusters = append(createdClusters, *resp.JSON201) |
There was a problem hiding this comment.
Missing status and nil check before accessing JSON201.
The loop creates clusters and appends *resp.JSON201. If a POST fails with non-201 status, JSON201 would be nil, causing a panic.
🔧 Suggested fix: Add status and nil check
resp, err := client.PostClusterWithResponse(ctx, openapi.PostClusterJSONRequestBody(clusterInput), test.WithAuthToken(ctx))
Expect(err).NotTo(HaveOccurred(), "Failed to create cluster %d", i)
+ Expect(resp.StatusCode()).To(Equal(http.StatusCreated))
+ Expect(resp.JSON201).NotTo(BeNil())
createdClusters = append(createdClusters, *resp.JSON201)📝 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.
| resp, err := client.PostClusterWithResponse(ctx, openapi.PostClusterJSONRequestBody(clusterInput), test.WithAuthToken(ctx)) | |
| Expect(err).NotTo(HaveOccurred(), "Failed to create cluster %d", i) | |
| createdClusters = append(createdClusters, *cluster) | |
| createdClusters = append(createdClusters, *resp.JSON201) | |
| resp, err := client.PostClusterWithResponse(ctx, openapi.PostClusterJSONRequestBody(clusterInput), test.WithAuthToken(ctx)) | |
| Expect(err).NotTo(HaveOccurred(), "Failed to create cluster %d", i) | |
| Expect(resp.StatusCode()).To(Equal(http.StatusCreated)) | |
| Expect(resp.JSON201).NotTo(BeNil()) | |
| createdClusters = append(createdClusters, *resp.JSON201) |
🤖 Prompt for AI Agents
In @test/integration/clusters_test.go around lines 526 - 528, The test
dereferences resp.JSON201 without verifying the HTTP status or that JSON201 is
non-nil, which can panic if the POST fails; after the
client.PostClusterWithResponse call and Err check, assert the response status is
201 (or use resp.StatusCode/resp.Status) and add
Expect(resp.JSON201).NotTo(BeNil()) before appending, then safely append
*resp.JSON201 to createdClusters; reference resp, resp.JSON201 and
client.PostClusterWithResponse in the changes.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yasun1 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 66df4e6
into
openshift-hyperfleet:main
3 participants
HYPERFLEET-500 - feat: introduce oapi-codegen
This PR introduces oapi-codegen to generate the go types from openapi.yaml schema instead of the Java openapi-generator tool.
This PR depends on the approval of: openshift-hyperfleet/hyperfleet-api-spec#21
The generated openapi.yaml from that PR is already introduced in this PR, with
v1.0.2The PR is organized in 3 commits:
Summary by CodeRabbit
New Features
Improvements
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.