refactor(api)!: drop the removed permission namespace and name fields - #1897
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughPermission API tests no longer expect deprecated ChangesPermission response cleanup
Proton dependency pin
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to This change removes deprecated permission fields from the API contract, so it should be published with a minor version bump and release-note entry. The code is otherwise mergeable, with explicit owner follow-up needed for release handling. Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
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. Comment |
Coverage Report for CI Build 32819377341Coverage decreased (-0.002%) to 49.129%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
23a9a84 to
444cd87
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f9c61842-ace1-4a8a-8307-91188d5cc6ba
📒 Files selected for processing (2)
Makefileinternal/api/v1beta1connect/permission.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/api/v1beta1connect/permission.go
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.
Bump the proton pin to pull in the proto change that deletes the deprecated namespace and name fields from the Permission message and PermissionRequestBody (their numbers and names are now reserved), and regenerate the vendored protos. Responses carry the permission identity only in key, so transformPermissionToPB no longer populates the two fields and the handler tests assert key-only payloads. The pin bump also picks up the CheckOrganizationDelete RPC definition added on proton main; the handler embeds the generated unimplemented fallback, so that endpoint answers unimplemented until its implementation lands. Part of #1782. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
8321fdf to
3913ddd
Compare
Summary
Final step of #1782 for the permission APIs. The deprecated
namespace/namefields are deleted from thePermissionmessage andPermissionRequestBodyin proton (raystack/proton#500, numbers and names reserved), sokey(service.resource.verb) is the only way a permission's identity travels on the wire in either direction. This lands the regenerated protos and stops populating the two response fields.Depends on raystack/proton#500. The
PROTON_COMMITpin currently points at that PR's head commit; I'll move it to the proton main commit once it merges.Why this is safe for clients
InvalidArgumenttoday; proto3 ignores the unknown bytes either way.keyround-trips losslessly since fix(permission): make the response key lossless and migrate its readers #1887 and every in-repo reader (reconciler, CLI, e2e) uses it. A client still readingnamespace/namewould now see them empty — this is the breaking part, so it should ride a minor version bump and be called out in the release notes.Changes
Makefile: bumpPROTON_COMMIT; the regen also picks up theCheckOrganizationDeleteRPC definition recently added on proton main — the handler embeds the generated unimplemented fallback, so that endpoint answers unimplemented until its implementation (feat(deleter): clear or report org delete blockers up front #1857) lands.proto/: regenerated.internal/api/v1beta1connect/permission.go:transformPermissionToPBno longer setsName/Namespace; the key round-trip guard stays.internal/api/v1beta1connect/permission_test.go: expectations are key-only; the two "deprecated fields sent without key" cases are gone since the fields no longer exist.Test Plan
go build ./...andgo vet ./...(includingregression,e2etags) passgo test ./internal/api/v1beta1connect/...passesgolangci-lint run internal/api/v1beta1connect/...— 0 issuesgo test ./...— only the Docker-backed postgres/e2e suites fail locally (Docker not running); CI runs them🤖 Generated with Claude Code