refactor(api)!: drop the removed permission namespace and name fields - #1897
refactor(api)!: drop the removed permission namespace and name fields#1897whoAbhishekSah wants to merge 2 commits into
Conversation
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>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 37 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe permission API no longer returns deprecated ChangesPermission response cleanup
Proton dependency pin
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to The PR removes deprecated permission response fields as an intentional compatibility change. Merge is reasonable with explicit owner awareness that the required minor version bump and release-note entry must be verified. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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 32348861598Coverage decreased (-0.002%) to 48.802%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
23a9a84 to
444cd87
Compare
2 participants
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