-
Notifications
You must be signed in to change notification settings - Fork 11
fix: Deployment version refactor rbac #375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis PR updates the authorization logic to shift the focus from release management to deployment version management. Across multiple files, permission checks and resource types have been updated accordingly. The changes affect API endpoints, routers, RBAC utilities, database schemas, SQL queries, and permission enums. No change has been made to the overall request flow beyond the modified permission names and resource identifiers. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant A as API Endpoint
participant RB as RBAC Engine
participant DB as Database
U->>A: Send request (e.g., PATCH update)
A->>RB: Check authorization (DeploymentVersionUpdate)
RB-->>A: Return permission granted/denied
A->>DB: Process update if authorized
DB-->>A: Return result
A-->>U: Send response
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/job-dispatch/src/policies/release-sequencing.ts (1)
54-54: Resource reference updated to deployment_version_idThe SQL query now correctly references
deployment_version_idinstead ofversion_id, aligning with the shift from release management to deployment version management.However, the table alias
release2might be confusing since it references a table now identified asdeploymentVersion. Consider updating the alias for consistency.- inner join ${schema.deploymentVersion} as release2 on rjt2.deployment_version_id = release2.id + inner join ${schema.deploymentVersion} as deploymentVersion2 on rjt2.deployment_version_id = deploymentVersion2.idpackages/api/src/router/environment-approval.ts (1)
1-181: Consider updating function and variable namesWhile the authorization resource types have been properly updated from "release" to "deploymentVersion", function parameters and variable names still use "release" terminology (e.g.,
releaseId).Consider updating these names in a future refactoring to maintain consistency throughout the codebase.
packages/api/src/router/release-metadata-keys.ts (1)
1-64: Consider renaming router and file in a future updateWhile the permission checks have been updated correctly, the router name (
releaseMetadataKeysRouter) and file name (release-metadata-keys.ts) still reference "release".Consider updating these names in a future refactoring to maintain consistency with the new deployment version terminology.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/webservice/src/app/api/v1/releases/[releaseId]/route.ts(1 hunks)apps/webservice/src/app/api/v1/releases/route.ts(1 hunks)packages/api/src/router/deployment-version.ts(7 hunks)packages/api/src/router/environment-approval.ts(4 hunks)packages/api/src/router/job.ts(2 hunks)packages/api/src/router/release-deploy.ts(2 hunks)packages/api/src/router/release-metadata-keys.ts(2 hunks)packages/auth/src/utils/rbac.ts(4 hunks)packages/db/src/schema/rbac.ts(1 hunks)packages/job-dispatch/src/policies/release-sequencing.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.tspackages/job-dispatch/src/policies/release-sequencing.tsapps/webservice/src/app/api/v1/releases/route.tspackages/api/src/router/release-metadata-keys.tspackages/api/src/router/environment-approval.tsapps/webservice/src/app/api/v1/releases/[releaseId]/route.tspackages/api/src/router/release-deploy.tspackages/validators/src/auth/index.tspackages/api/src/router/job.tspackages/auth/src/utils/rbac.tspackages/api/src/router/deployment-version.ts
🧠 Learnings (1)
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`.
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
🔇 Additional comments (23)
packages/db/src/schema/rbac.ts (1)
36-36: Scope type updated to reflect architectural shiftThe change from
"release"to"deploymentVersion"in thescopeTypeenum correctly implements the architectural shift from release management to deployment version management.This change is well aligned with the retrieved learning that the
scopeTypeenum includes various scope types, now including"deploymentVersion".packages/api/src/router/environment-approval.ts (4)
22-22: Authorization resource type updatedResource type has been updated from "release" to "deploymentVersion" while maintaining the same permission. This correctly aligns with the updated scope type defined in the RBAC schema.
68-68: Resource type updated for approve procedureThe approve procedure now correctly authorizes against "deploymentVersion" resource type instead of "release".
130-130: Resource type updated for reject procedureThe reject procedure now correctly authorizes against "deploymentVersion" resource type instead of "release".
164-164: Resource type updated for statusByReleasePolicyId procedureThe statusByReleasePolicyId procedure now correctly authorizes against "deploymentVersion" resource type instead of "release".
packages/api/src/router/release-metadata-keys.ts (2)
14-14: Permission updated to DeploymentVersionGetThe permission check has been correctly updated from
Permission.ReleaseGettoPermission.DeploymentVersionGet, aligning with the shift from release management to deployment version management.
39-39: Permission updated to DeploymentVersionGetThe permission check for the byWorkspace procedure has been correctly updated from
Permission.ReleaseGettoPermission.DeploymentVersionGet, maintaining consistency with the updated permission structure.apps/webservice/src/app/api/v1/releases/route.ts (1)
43-43: LGTM - Permission update aligns with deployment version refactoringThis change correctly updates the permission check from
ReleaseCreatetoDeploymentVersionCreateto align with the shift from release management to deployment version management.apps/webservice/src/app/api/v1/releases/[releaseId]/route.ts (1)
32-33: LGTM - Permission and resource type updates aligned with deployment version focusThe changes correctly update both the permission check (from
ReleaseUpdatetoDeploymentVersionUpdate) and the resource type (from"release"to"deploymentVersion"), maintaining consistency with the deployment version management refactoring.packages/api/src/router/release-deploy.ts (2)
24-27: LGTM - Permission and resource type updates for toEnvironmentThe changes correctly update both the permission check (replacing
ReleaseGetwithDeploymentVersionGet) and the resource type (from"release"to"deploymentVersion"), maintaining consistency with the deployment version management refactoring.
71-74: LGTM - Permission and resource type updates for toResourceThe changes correctly update both the permission check (replacing
ReleaseGetwithDeploymentVersionGet) and the resource type (from"release"to"deploymentVersion"), maintaining consistency with the deployment version management refactoring.packages/auth/src/utils/rbac.ts (3)
91-110: LGTM - Function rename and return type updatesThe function has been correctly renamed from
getReleaseScopestogetDeploymentVersionScopesand the return type values have been updated to reflect deployment version instead of release, maintaining consistency with the deployment version management refactoring.
375-375: LGTM - Updated type in job scopes return arrayThe type in the job scopes return array has been correctly updated from
"release"to"deploymentVersion", maintaining consistency with the deployment version management refactoring.
398-398: LGTM - Updated key in scopeHandlers objectThe key in the
scopeHandlersobject has been correctly updated fromreleasetodeploymentVersion, maintaining consistency with the deployment version management refactoring.packages/validators/src/auth/index.ts (1)
85-88: Permission enum update to reflect deployment version focusThe changes correctly implement the transition from release-based permissions to deployment version-based permissions, aligning with the PR's objective to refocus authorization logic on deployment version management.
packages/api/src/router/job.ts (1)
774-776: LGTM - Properly updated authorization checkThe permission and resource type have been correctly updated to use
Permission.DeploymentVersionGetand"deploymentVersion"respectively, maintaining consistency with the new permission model.packages/api/src/router/deployment-version.ts (7)
52-55: LGTM - Permission correctly updatedThe permission has been properly updated to
Permission.DeploymentVersionList, aligning with the new permission structure.
123-126: LGTM - Authorization check properly updatedBoth the permission and resource type have been correctly updated to
Permission.DeploymentVersionGetand"deploymentVersion"respectively.
173-176: LGTM - Create permission properly updatedThe permission has been correctly updated to
Permission.DeploymentVersionCreatewhile maintaining the same resource type.
242-249: LGTM - Blocked check authorization properly updatedThe permission and resource type have been properly updated to
Permission.DeploymentVersionGetand"deploymentVersion"for the blocked checks.
354-359: LGTM - Status authorization check properly updatedThe permission and resource type are correctly updated for the status retrieval by environment ID.
401-410: LGTM - System directory authorization check properly updatedThe conditional check maintains proper permission structure, using
DeploymentVersionGetfor release ID checks andDeploymentGetfor deployment ID checks.
461-466: LGTM - Latest completion check properly updatedThe permission has been correctly updated while maintaining the same resource type for checking deployment access.
Summary by CodeRabbit
Refactor
New Features
DeploymentVersionCreate,DeploymentVersionGet,DeploymentVersionUpdate, andDeploymentVersionList.