Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Warning Rate limit exceeded@chronark has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 Walkthrough""" WalkthroughThis change introduces OpenAPI specification support throughout the system. It adds the ability to scrape, store, and retrieve OpenAPI specs for deployed versions, extends the database schema and queries to persist specs, and implements a new gRPC service and protobuf definitions for OpenAPI diffing. Supporting handlers, conversion utilities, and dependency updates are included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CtrlAPI
participant OpenApiService
participant DB
participant DeployedContainer
User->>CtrlAPI: Deploy new version
CtrlAPI->>DeployedContainer: Health check (/v1/liveness)
CtrlAPI->>DeployedContainer: Fetch OpenAPI spec (/openapi.yaml)
DeployedContainer-->>CtrlAPI: Returns OpenAPI YAML
CtrlAPI->>DB: Store OpenAPI spec in versions table
DB-->>CtrlAPI: Ack
User->>OpenApiService: GetOpenApiDiff(old_version_id, new_version_id)
OpenApiService->>DB: Load old version's OpenAPI spec
OpenApiService->>DB: Load new version's OpenAPI spec
OpenApiService->>OpenApiService: Parse specs, diff, check for breaking changes
OpenApiService-->>User: Returns diff summary, breaking changes flag, changelog
Estimated code review effort3 (~45 minutes) Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
chronark
requested review from
Flo4604,
MichaelUnkey,
imeyer,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
go/gen/proto/ctrl/v1/ctrlv1connect/openapi.connect.gois excluded by!**/gen/**go/gen/proto/ctrl/v1/openapi.pb.gois excluded by!**/*.pb.go,!**/gen/**go/go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
go/apps/ctrl/run.go(2 hunks)go/apps/ctrl/services/openapi/convert.go(1 hunks)go/apps/ctrl/services/openapi/get_diff.go(1 hunks)go/apps/ctrl/services/openapi/service.go(1 hunks)go/apps/ctrl/services/openapi/utils.go(1 hunks)go/apps/ctrl/services/version/deploy_workflow.go(2 hunks)go/demo_api/main.go(2 hunks)go/go.mod(3 hunks)go/pkg/db/models_generated.go(1 hunks)go/pkg/db/querier_generated.go(4 hunks)go/pkg/db/queries/version_find_by_id.sql(1 hunks)go/pkg/db/queries/version_insert.sql(2 hunks)go/pkg/db/queries/version_update_openapi_spec.sql(1 hunks)go/pkg/db/schema.sql(1 hunks)go/pkg/db/version_find_by_id.sql_generated.go(3 hunks)go/pkg/db/version_insert.sql_generated.go(6 hunks)go/pkg/db/version_update_openapi_spec.sql_generated.go(1 hunks)go/proto/ctrl/v1/openapi.proto(1 hunks)internal/db/src/schema/versions.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: Flo4604
PR: unkeyed/unkey#3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as `description`) alongside `$ref` in schema objects. Do not flag this as an error in future reviews.
Learnt from: Flo4604
PR: unkeyed/unkey#2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
go/pkg/db/schema.sql (1)
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.359Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
go/pkg/db/queries/version_find_by_id.sql (2)
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.359Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
go/pkg/db/models_generated.go (1)
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.359Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
internal/db/src/schema/versions.ts (2)
Learnt from: chronark
PR: unkeyed/unkey#3161
File: go/pkg/clickhouse/schema/databases/001_verifications/002_raw_key_verifications_v1.sql:31-33
Timestamp: 2025-04-22T14:40:51.459Z
Learning: The ClickHouse table schemas in the codebase mirror the production environment and cannot be modified directly in PRs without careful migration planning.
Learnt from: chronark
PR: unkeyed/unkey#2294
File: apps/api/src/pkg/keys/service.ts:268-271
Timestamp: 2024-10-20T07:05:55.471Z
Learning: In `apps/api/src/pkg/keys/service.ts`, `ratelimitAsync` is a table relation, not a column selection. When querying, ensure that table relations are included appropriately, not as columns.
go/pkg/db/version_find_by_id.sql_generated.go (3)
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.359Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
go/apps/ctrl/services/openapi/service.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/cmd/*/** : Service binary code should be located at `<service>/cmd/<service | command>`.
go/pkg/db/queries/version_insert.sql (2)
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.359Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
go/apps/ctrl/run.go (3)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/cmd/*/** : Service binary code should be located at `<service>/cmd/<service | command>`.
Learnt from: chronark
PR: unkeyed/unkey#3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.212Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.
go/pkg/db/version_insert.sql_generated.go (2)
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.359Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
go/go.mod (3)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: chronark
PR: unkeyed/unkey#3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.212Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.
Learnt from: Flo4604
PR: unkeyed/unkey#3606
File: go/pkg/prometheus/metrics/database.go:29-30
Timestamp: 2025-07-16T10:06:35.397Z
Learning: In Go packages, variables defined in one file within a package (like `latencyBuckets` and `constLabels` in go/pkg/prometheus/metrics/http.go) are accessible from other files in the same package without requiring imports. This is a common pattern for sharing configuration across multiple files within a package.
go/pkg/db/version_update_openapi_spec.sql_generated.go (3)
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.359Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/pkg/db/queries/version_update_openapi_spec.sql (2)
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.359Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
go/pkg/db/querier_generated.go (2)
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.359Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
go/apps/ctrl/services/openapi/get_diff.go (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/proto/ctrl/v1/openapi.proto (1)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
go/apps/ctrl/services/version/deploy_workflow.go (5)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:153-158
Timestamp: 2025-07-16T09:18:45.379Z
Learning: In the go/cmd/cli/commands/deploy/ CLI codebase, ogzhanolguncu prefers to allow deployment to continue even when Docker push fails (around lines 153-158 in deploy.go) because the team is working locally and needs this behavior for local development workflows where registry access might not be available.
Learnt from: Flo4604
PR: unkeyed/unkey#3606
File: go/pkg/db/replica.go:8-11
Timestamp: 2025-07-16T15:38:53.464Z
Learning: For debugging database replica usage in go/pkg/db/replica.go, it's acceptable to mark QueryRowContext operations as "success" even though SQL errors only surface during row.Scan() calls. The timing metrics are the primary concern for debugging replica performance patterns.
Learnt from: chronark
PR: unkeyed/unkey#3420
File: go/pkg/hydra/store/gorm/gorm.go:486-498
Timestamp: 2025-07-02T11:51:58.572Z
Learning: The Hydra package (go/pkg/hydra) is planned to be migrated from GORM to sqlc for database operations, which explains why raw SQL queries are acceptable in the current implementation.
Learnt from: chronark
PR: unkeyed/unkey#3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.212Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.
🧬 Code Graph Analysis (4)
go/apps/ctrl/services/openapi/service.go (2)
go/gen/proto/ctrl/v1/ctrlv1connect/openapi.connect.go (2)
UnimplementedOpenApiServiceHandler(109-109)UnimplementedOpenApiServiceHandler(111-113)internal/db/src/index.ts (1)
Database(13-13)
go/apps/ctrl/run.go (5)
go/gen/proto/ctrl/v1/ctrlv1connect/service.connect.go (1)
NewCtrlServiceHandler(90-105)go/apps/ctrl/services/openapi/service.go (1)
New(15-21)go/apps/ctrl/services/version/service.go (1)
New(17-24)go/gen/proto/ctrl/v1/ctrlv1connect/version.connect.go (1)
NewVersionServiceHandler(113-136)go/gen/proto/ctrl/v1/ctrlv1connect/openapi.connect.go (1)
NewOpenApiServiceHandler(91-106)
go/pkg/db/version_update_openapi_spec.sql_generated.go (1)
go/pkg/hydra/store/db.go (2)
Queries(19-21)DBTX(8-13)
go/apps/ctrl/services/openapi/convert.go (1)
go/gen/proto/ctrl/v1/openapi.pb.go (12)
DiffSummary(160-166)DiffSummary(179-179)DiffSummary(194-196)DiffCounts(272-279)DiffCounts(292-292)DiffCounts(307-309)DiffDetails(212-219)DiffDetails(232-232)DiffDetails(247-249)ChangelogEntry(76-86)ChangelogEntry(99-99)ChangelogEntry(114-116)
🪛 Buf (1.55.1)
go/proto/ctrl/v1/openapi.proto
3-3: Files with package "ctrl.v1" must be within a directory "ctrl/v1" relative to root but were in directory "go/proto/ctrl/v1".
(PACKAGE_DIRECTORY_MATCH)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test Packages / Test
🔇 Additional comments (19)
go/pkg/db/schema.sql (1)
387-387: Schema change looks good.The addition of the
openapi_spectext column to theversionstable is well-designed:
texttype appropriately handles potentially large OpenAPI specifications- Nullable field allows for versions without OpenAPI specs
- Follows established naming conventions and placement within the schema
go/pkg/db/models_generated.go (1)
874-874: Generated model correctly reflects the schema change.The
OpenapiSpecfield addition is properly implemented:
sql.NullStringtype correctly handles the nullable text column- Database tag mapping is accurate
- Field placement and naming follow established conventions
go/pkg/db/queries/version_find_by_id.sql (1)
12-12: Query correctly includes the new field.The addition of
openapi_specto the SELECT statement is properly positioned and maintains consistency with the schema field order.go/pkg/db/queries/version_update_openapi_spec.sql (1)
1-4: Well-implemented update query.The
UpdateVersionOpenApiSpecquery follows best practices:
- Uses parameterized arguments for security
- Focused scope updating only the required field
- Proper sqlc annotations and naming conventions
- Appropriate WHERE clause filtering
internal/db/src/schema/versions.ts (2)
2-2: Import addition is correct.The
textimport is properly added to support the newopenapiSpecfield definition.
35-37: TypeScript schema field properly defined.The
openapiSpecfield implementation is excellent:
- Uses appropriate
texttype for potentially large OpenAPI specifications- Correctly maps to the
openapi_specdatabase column- Nullable field allows for versions without OpenAPI specs
- Descriptive comment enhances code documentation
- Follows established TypeScript naming conventions
go/pkg/db/queries/version_insert.sql (1)
12-12: LGTM! Clean addition of the OpenAPI spec column.The addition of the
openapi_speccolumn to the INSERT query is correctly implemented with proper positioning and consistent parameter binding.Also applies to: 27-27
go/pkg/db/version_find_by_id.sql_generated.go (1)
23-23: LGTM! Generated code correctly includes the OpenAPI spec field.The sqlc-generated code properly adds the
openapi_speccolumn to the SELECT query and maps it to theOpenapiSpecfield in the scanning operation.Also applies to: 43-43, 62-62
go/apps/ctrl/run.go (2)
13-13: LGTM! Proper import for the new OpenAPI service.The import correctly includes the new
openapipackage for the OpenAPI diff functionality.
162-164: LGTM! Clean refactoring with new OpenAPI service registration.The refactoring from storing service instances in variables to directly registering handlers inline is a good simplification. The new OpenAPI service handler registration follows the established pattern and properly integrates the OpenAPI diff functionality.
go/apps/ctrl/services/openapi/service.go (1)
1-21: LGTM! Well-structured OpenAPI service implementation.The service structure follows established patterns with proper dependency injection. The embedded
UnimplementedOpenApiServiceHandlercorrectly provides the gRPC interface, and the constructor properly initializes all fields.go/go.mod (2)
13-13: LGTM! Appropriate dependency updates for OpenAPI functionality.The upgrade of
kin-openapito v0.132.0 and addition ofoasdiffv1.11.4 correctly support the new OpenAPI diffing features.Also applies to: 21-21
51-51: Security check passed for new indirect Go dependenciesAll newly added indirect dependencies were verified against the GitHub Advisory Database at their current versions:
- github.com/oasdiff/oasdiff v1.11.4 (no advisories)
- github.com/tidwall/gjson v1.18.0
• Known HIGH-severity advisories affect versions < 1.9.3, < 1.6.4, < 1.6.5, < 1.6.6
• Current version (1.18.0) is well beyond the first patched releases (≥ 1.9.3)- github.com/wI2L/jsondiff v0.6.1 (no advisories)
- cloud.google.com/go v0.121.0 (no advisories)
No further action required—these versions are free of known vulnerabilities.
go/apps/ctrl/services/openapi/utils.go (1)
10-26: LGTM! Well-implemented database utility function.The implementation correctly handles database queries with proper error wrapping, uses read-only connections appropriately, and provides clear error messages for both database failures and missing OpenAPI specs.
go/pkg/db/version_insert.sql_generated.go (1)
25-25: LGTM! Database schema changes are consistent.The generated code correctly adds the
openapi_speccolumn to the insert operation with proper nullable string handling and consistent parameter ordering.Also applies to: 42-42, 57-57, 75-75, 92-92, 106-106
go/pkg/db/querier_generated.go (1)
454-454: LGTM! Interface updates are consistent with schema changes.The generated interface correctly includes the
openapi_speccolumn in relevant method comments and adds the newUpdateVersionOpenApiSpecmethod with proper signature.Also applies to: 911-911, 928-928, 1285-1290
go/pkg/db/version_update_openapi_spec.sql_generated.go (1)
13-32: LGTM! Generated update method follows proper patterns.The implementation correctly provides a simple UPDATE operation for the
openapi_speccolumn with proper parameter handling and error propagation.go/apps/ctrl/services/openapi/get_diff.go (1)
63-68: Confirm operations sources map usage in backward-compatibility checkPassing an empty
&diff.OperationsSourcesMap{}toCheckBackwardCompatibilityat:
- go/apps/ctrl/services/openapi/get_diff.go:63-68
likely means no operation-level sources are tracked. Please verify in the oasdiff library that either:
- the checker populates missing entries internally when given an empty map, or
- you should supply a fully populated map (for example via a helper like
diff.GetOperationsSourcesMap(...)).Otherwise, operation-level breaking changes may be overlooked.
go/proto/ctrl/v1/openapi.proto (1)
1-46: Well-structured protobuf definitionsThe protobuf definitions are well-organized with clear message structures and appropriate use of optional fields. The static analysis warning about package directory can be safely ignored as it follows Go module conventions.
Flo4604
left a comment
Flo4604
left a comment
There was a problem hiding this comment.
I added a few nits but works as excpected
|
ty, checking |
…ing operationId locally
…ing operationId locally (#3641) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
go/apps/ctrl/services/openapi/convert.go (1)
45-53: The pointer reuse issue still exists and needs to be fixed.This is the same issue that was identified in previous reviews but appears to still be present. Taking the address of the loop variable
operationIddirectly will cause all entries to share the same pointer value.The fix should create a local copy of
operationIdwithin the loop:operationId := change.GetOperationId() -result[i] = &ctrlv1.ChangelogEntry{ - Id: change.GetId(), - Text: change.GetUncolorizedText(localizer), - Level: level, - Operation: change.GetOperation(), - Path: change.GetPath(), - OperationId: &operationId, -} + +var operationIdPtr *string +if operationId != "" { + // Create a local copy to avoid pointer reuse in loop + opId := operationId + operationIdPtr = &opId +} + +result[i] = &ctrlv1.ChangelogEntry{ + Id: change.GetId(), + Text: change.GetUncolorizedText(localizer), + Level: level, + Operation: change.GetOperation(), + Path: change.GetPath(), + OperationId: operationIdPtr, +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
go/apps/ctrl/services/openapi/convert.go(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: Flo4604
PR: unkeyed/unkey#3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as `description`) alongside `$ref` in schema objects. Do not flag this as an error in future reviews.
Learnt from: Flo4604
PR: unkeyed/unkey#2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
go/apps/ctrl/services/openapi/convert.go (4)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's *.go code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Use AIDEV-NOTE:, AIDEV-TODO:, AIDEV-BUSINESS_RULE:, or AIDEV-QUESTION: (all-caps prefix) as anchor comments aimed at AI and developers.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Make sure to add relevant anchor comments whenever a file or piece of code is too complex, very important, confusing, or could have a bug.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Update relevant anchors when modifying associated code.
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Flo4604
PR: unkeyed/unkey#3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as `description`) alongside `$ref` in schema objects. Do not flag this as an error in future reviews.
Learnt from: Flo4604
PR: unkeyed/unkey#2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
go/apps/ctrl/services/openapi/convert.go (4)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's *.go code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Use AIDEV-NOTE:, AIDEV-TODO:, AIDEV-BUSINESS_RULE:, or AIDEV-QUESTION: (all-caps prefix) as anchor comments aimed at AI and developers.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Make sure to add relevant anchor comments whenever a file or piece of code is too complex, very important, confusing, or could have a bug.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Update relevant anchors when modifying associated code.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
go/apps/ctrl/services/openapi/convert.go (1)
9-30: LGTM! Well-structured conversion function.The function correctly handles the conversion from
diff.Summaryto protobuf format with proper safety checks for missing categories.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
go/apps/ctrl/services/openapi/convert.go(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: Flo4604
PR: unkeyed/unkey#3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as `description`) alongside `$ref` in schema objects. Do not flag this as an error in future reviews.
Learnt from: Flo4604
PR: unkeyed/unkey#2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
go/apps/ctrl/services/openapi/convert.go (5)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's *.go code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Use AIDEV-NOTE:, AIDEV-TODO:, AIDEV-BUSINESS_RULE:, or AIDEV-QUESTION: (all-caps prefix) as anchor comments aimed at AI and developers.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Make sure to add relevant anchor comments whenever a file or piece of code is too complex, very important, confusing, or could have a bug.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Update relevant anchors when modifying associated code.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Do not remove AIDEV-*s without explicit human instruction.
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Flo4604
PR: unkeyed/unkey#3421
File: go/apps/api/openapi/openapi.yaml:196-200
Timestamp: 2025-07-03T05:58:10.699Z
Learning: In the Unkey codebase, OpenAPI 3.1 is used, which allows sibling keys (such as `description`) alongside `$ref` in schema objects. Do not flag this as an error in future reviews.
Learnt from: Flo4604
PR: unkeyed/unkey#2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
go/apps/ctrl/services/openapi/convert.go (5)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's *.go code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Use AIDEV-NOTE:, AIDEV-TODO:, AIDEV-BUSINESS_RULE:, or AIDEV-QUESTION: (all-caps prefix) as anchor comments aimed at AI and developers.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Make sure to add relevant anchor comments whenever a file or piece of code is too complex, very important, confusing, or could have a bug.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Update relevant anchors when modifying associated code.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.187Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Do not remove AIDEV-*s without explicit human instruction.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Packages / Test
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
go/apps/ctrl/services/openapi/convert.go (1)
10-31: Well-structured conversion with proper error handling.The
convertSummaryToProtofunction correctly handles missing detail categories and uses explicit type casting. The defensive programming approach with thegetCountshelper function prevents potential nil pointer issues.
do we not want to merge this?
|
|
we do, but your review for some reason wasn't sufficient and I wanted to look into it later |
Ah maybe because some code got changed where i am not code owner? |
|
probably but I haven’t had time to dig in yet and add you |
perkinsjr
left a comment
perkinsjr
left a comment
There was a problem hiding this comment.
Approving to allow you to merge when you want.
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (07/22/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
3 participants
What does this PR do?
This PR adds OpenAPI specification support to the deployment workflow, enabling automatic scraping and storage of OpenAPI specs from deployed containers. It also implements a new OpenAPI service with a diff feature that can compare two API versions to identify breaking changes and modifications.
Key changes:
Type of change
How should this be tested?
/openapi.yamlThe default demo api does this alreadyChecklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainSummary by CodeRabbit
New Features
Database
API
Dependency Updates