fix: Update rbac for deployment version channel#378
Conversation
WalkthroughThis pull request updates several permission checks, renaming and refactoring references from “release channels” to “deployment version channels.” The changes span API routes, RBAC utility functions, database schemas, and validators. The overall control flow and response handling remain intact, with adjustments made to ensure that permission validations are now correctly scoped to operations on deployment version channels. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Auth
participant DB
Client->>API: Send request (CREATE/DELETE/UPDATE)
API->>Auth: Check DeploymentVersionChannel permission
Auth-->>API: Return permission result
API->>DB: Execute operation if authorized
DB-->>API: Return operation result
API-->>Client: Send response
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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:
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
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/api/src/router/deployment.ts (1)
30-152: 💡 Verification agent🧩 Analysis chain
Consider updating variable and method naming in releaseChannelRouter
While all the permissions and scope types have been correctly updated, the router is still named
releaseChannelRouterand uses variables likechannels,rc, etc. that refer to release channels. Consider updating these names for consistency with the terminology change.Check for other instances of "release channel" terminology that might need updating:
🏁 Script executed:
#!/bin/bash # Search for variable names, comments, and other instances that might still use "release" terminology # Look for variable names containing "release" rg -i '\b(const|let|var)\s+release[a-zA-Z]*\b' --type ts # Look for method/function names containing "release" rg -i '\bfunction\s+release[a-zA-Z]*\b|\b[a-zA-Z]+\.release[a-zA-Z]*\b' --type ts # Look for comments containing "release channel" rg -i '//.*release channel|/\*.*release channel.*\*/' --type tsLength of output: 72574
Action Required: Update Naming in Deployment Router
In the file
packages/api/src/router/deployment.ts(lines 30–152), the router and variable names still use the legacy "release" terminology (e.g.,releaseChannelRouter,channels, andrc). Although the permissions and scope types have been updated to reflect deployment concepts, these naming inconsistencies could lead to confusion. To align with the updated terminology, please consider:
- Renaming
releaseChannelRouter(and any related endpoint identifiers) to something likedeploymentVersionChannelRouter.- Updating variable names (e.g., changing
channelstodeploymentChannelsand replacing the ambiguousrcwith a more descriptive name) for clarity and consistency.- Reviewing related files for any other lingering "release" nomenclature that should be updated to match the deployment terminology.
🧹 Nitpick comments (3)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/release-channels/name/[name]/route.ts (1)
10-45: Consider updating route path and response messages for naming consistencyWhile the permission has been updated to use "DeploymentVersionChannel", the route path still uses "release-channels" and the response messages still mention "Release channel". Consider updating these for complete naming consistency.
apps/webservice/src/app/api/v1/release-channels/route.ts (2)
25-40: Consider updating variable naming for consistencyWhile the permission has been updated to use "DeploymentVersionChannel", the variable names still use "releaseChannel". For better code clarity and consistency, consider updating variable names to match the new terminology.
- const releaseChannel = await db + const deploymentVersionChannel = await db .select() .from(SCHEMA.deploymentVersionChannel) .where( and( eq(SCHEMA.deploymentVersionChannel.deploymentId, body.deploymentId), eq(SCHEMA.deploymentVersionChannel.name, body.name), ), ) .then(takeFirstOrNull); - if (releaseChannel) + if (deploymentVersionChannel) return NextResponse.json( - { error: "Release channel already exists", id: releaseChannel.id }, + { error: "Deployment version channel already exists", id: deploymentVersionChannel.id }, { status: 409 }, );
42-48: Update variable naming in response handlingFor consistency with the permission updates, consider renaming the variable in the response handling as well.
return db .insert(SCHEMA.deploymentVersionChannel) .values(body) .returning() .then(takeFirst) - .then((releaseChannel) => NextResponse.json(releaseChannel)) + .then((deploymentVersionChannel) => NextResponse.json(deploymentVersionChannel)) .catch((error) => NextResponse.json({ error }, { status: 500 }));
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/webservice/src/app/api/v1/deployments/[deploymentId]/release-channels/name/[name]/route.ts(1 hunks)apps/webservice/src/app/api/v1/release-channels/route.ts(1 hunks)packages/api/src/router/deployment.ts(5 hunks)packages/auth/src/utils/rbac.ts(3 hunks)packages/db/drizzle/0077_lowly_sauron.sql(1 hunks)packages/db/drizzle/meta/_journal.json(1 hunks)packages/db/src/schema/rbac.ts(1 hunks)packages/validators/src/auth/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict en...
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
packages/db/src/schema/rbac.tsapps/webservice/src/app/api/v1/deployments/[deploymentId]/release-channels/name/[name]/route.tsapps/webservice/src/app/api/v1/release-channels/route.tspackages/validators/src/auth/index.tspackages/auth/src/utils/rbac.tspackages/api/src/router/deployment.ts
🧠 Learnings (2)
packages/db/src/schema/rbac.ts (1)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#181
File: packages/auth/src/utils/rbac.ts:102-118
Timestamp: 2025-03-12T21:38:05.696Z
Learning: The `releaseChannel` scope type is included in the `scopeType` enum in `packages/db/src/schema/rbac.ts`.
packages/auth/src/utils/rbac.ts (1)
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#181
File: packages/auth/src/utils/rbac.ts:102-118
Timestamp: 2025-03-12T21:38:05.696Z
Learning: The `releaseChannel` scope type is included in the `scopeType` enum in `packages/db/src/schema/rbac.ts`.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: Lint
🔇 Additional comments (13)
packages/db/drizzle/0077_lowly_sauron.sql (1)
1-2: LGTM: Appropriate scope type additionsThe addition of 'deploymentVersion' and 'deploymentVersionChannel' to the scope_type enum is consistent with the PR objective of updating RBAC for deployment version channels.
packages/db/drizzle/meta/_journal.json (1)
543-550: LGTM: Proper migration journal entryThe new journal entry for "0077_lowly_sauron" correctly tracks the migration that adds the new scope types.
packages/db/src/schema/rbac.ts (1)
37-37: LGTM: Consistent naming updateThe renamed scope type from "releaseChannel" to "deploymentVersionChannel" maintains consistency with the database migration and aligns with the broader terminology updates in the PR.
apps/webservice/src/app/api/v1/deployments/[deploymentId]/release-channels/name/[name]/route.ts (1)
15-15: LGTM: Permission updated to match new naming conventionThe permission check has been correctly updated from ReleaseChannelDelete to DeploymentVersionChannelDelete.
apps/webservice/src/app/api/v1/release-channels/route.ts (1)
19-19: Permission change looks goodThe permission has been properly updated from
ReleaseChannelCreatetoDeploymentVersionChannelCreateto align with the new naming convention.packages/auth/src/utils/rbac.ts (2)
112-134: Function rename and type updates look goodThe function has been properly renamed from
getReleaseChannelScopestogetDeploymentVersionChannelScopesand the return type has been updated to use "deploymentVersionChannel" instead of "releaseChannel". This aligns with the broader refactoring to use consistent terminology.
402-402: Handler map update is correctThe
scopeHandlersmap has been correctly updated to usedeploymentVersionChannelas the key instead ofreleaseChannel, pointing to the renamed function.packages/validators/src/auth/index.ts (1)
90-94: Permission enum updates are completeThe permissions for release channels have been properly replaced with equivalent permissions for deployment version channels. This ensures consistent terminology throughout the codebase and properly reflects the actual entities being operated on.
packages/api/src/router/deployment.ts (5)
35-36: Permission update for create operation looks goodThe permission check has been properly updated from
ReleaseChannelCreatetoDeploymentVersionChannelCreate.
54-55: Permission and scope type updates for update operation are correctBoth the permission (
DeploymentVersionChannelUpdate) and scope type (deploymentVersionChannel) have been updated correctly for the update operation.
70-71: Permission and scope type updates for delete operation are correctBoth the permission (
DeploymentVersionChannelDelete) and scope type (deploymentVersionChannel) have been updated correctly for the delete operation.
85-86: Permission update for list operation looks goodThe permission check has been properly updated from
ReleaseChannelListtoDeploymentVersionChannelList.
118-119: Permission and scope type updates for get operation are correctBoth the permission (
DeploymentVersionChannelGet) and scope type (deploymentVersionChannel) have been updated correctly for the get operation.
Summary by CodeRabbit
Refactor
Chores