feat: deployment dependencies evaluator#1081
Merged
adityachoudhari26 merged 3 commits intomainfrom Apr 28, 2026
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughA new deployment dependency evaluation feature is introduced in the workspace-engine. The changes add database schema for modeling deployment-to-deployment dependencies, define API types and database queries to fetch dependency relationships, implement a new evaluator that verifies CEL-based version selectors against dependency deployments' currently deployed versions, and integrate this evaluator into the release pipeline alongside policy rule evaluators. Supporting test mocks and helpers are updated across the codebase. Changes
Sequence DiagramsequenceDiagram
participant Evaluator as Deployment Version<br/>Dependency Evaluator
participant Getter as PostgresGetters
participant DB as Database
participant CEL as CEL Evaluator
Evaluator->>Getter: GetDependencies(ctx, deploymentID)
Getter->>DB: Query dependency edges
DB-->>Getter: DependencyEdge[]
Getter-->>Evaluator: DependencyEdge[]
loop For each dependency edge
Evaluator->>CEL: Compile(versionSelector)
CEL-->>Evaluator: Program
Evaluator->>Getter: GetReleaseTargetForDeploymentResource(ctx, depID, resourceID)
Getter->>DB: Query release target
DB-->>Getter: ReleaseTarget
Getter-->>Evaluator: ReleaseTarget
Evaluator->>Getter: GetCurrentVersionForReleaseTarget(ctx, rt)
Getter->>DB: Query current version
DB-->>Getter: DeploymentVersion
Getter-->>Evaluator: DeploymentVersion
Evaluator->>CEL: Evaluate(version)
CEL-->>Evaluator: bool (match result)
alt Version matches selector
Evaluator->>Evaluator: Allow (continue next dependency)
else Version mismatch or error
Evaluator->>Evaluator: Deny (return failure)
end
end
Evaluator-->>Evaluator: Return RuleEvaluation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements deployment-version dependency gating for desired releases (issue #1077) by introducing a new deployment_dependency data model and a non-policy-rule evaluator that blocks desired releases until declared dependency versions satisfy CEL selectors.
Changes:
- Add
deployment_dependencytable + migration/snapshot updates. - Add
deploymentversiondependencyevaluator wired into desired-release policy evaluation as a non-rule evaluator. - Add SQLC queries/getters and update mocks/tests to cover dependency evaluation paths.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/db/src/schema/deployment.ts | Adds Drizzle table definition for deployment_dependency. |
| packages/db/drizzle/meta/_journal.json | Records new migration entry. |
| packages/db/drizzle/meta/0190_snapshot.json | Updates Drizzle snapshot to include deployment_dependency. |
| packages/db/drizzle/0190_handy_luckman.sql | Creates deployment_dependency table and FKs. |
| apps/workspace-engine/test/controllers/harness/mocks.go | Extends harness mocks for new dependency getter methods. |
| apps/workspace-engine/svc/controllers/desiredrelease/reconcile_test.go | Extends reconcile test getter mock to satisfy new interface. |
| apps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval_test.go | Updates evaluator collection tests to account for non-rule evaluators. |
| apps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval.go | Adds non-rule evaluator collection and wires in deployment version dependency evaluator. |
| apps/workspace-engine/svc/controllers/desiredrelease/policyeval/getter.go | Extends policyeval Getter interface with deploymentversiondependency getters. |
| apps/workspace-engine/svc/controllers/desiredrelease/policyeval/getter_postgres.go | Implements new getter methods via a deploymentversiondependency Postgres getter. |
| apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentversiondependency/getter.go | Introduces getter interface and DependencyEdge model for evaluator. |
| apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentversiondependency/getter_postgres.go | Adds Postgres-backed implementations for dependency edges and lookups. |
| apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentversiondependency/deployment_version_dependency.go | Adds the core evaluator enforcing dependency version selectors. |
| apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentversiondependency/deployment_version_dependency_test.go | Adds unit tests covering allow/deny/error cases and CEL behavior. |
| apps/workspace-engine/pkg/oapi/oapi.gen.go | Adds DeploymentDependency schema/type to generated OAPI models. |
| apps/workspace-engine/oapi/spec/schemas/deployments.jsonnet | Adds OpenAPI schema for DeploymentDependency. |
| apps/workspace-engine/oapi/openapi.json | Updates rendered OpenAPI spec with DeploymentDependency. |
| apps/workspace-engine/pkg/db/queries/deployments.sql | Adds SQLC query to list deployment dependencies. |
| apps/workspace-engine/pkg/db/deployments.sql.go | Adds generated SQLC code for dependency query. |
| apps/workspace-engine/pkg/db/queries/computed_resources.sql | Adds SQLC query to get a release target for (deployment, resource) across environments. |
| apps/workspace-engine/pkg/db/computed_resources.sql.go | Adds generated SQLC code for the new release target query. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+75
to
+93
| -- name: GetReleaseTargetForDeploymentResource :one | ||
| -- Returns one release target for a (deployment, resource) pair across any | ||
| -- environment. Used by the deployment-version-dependency evaluator, which | ||
| -- only needs to identify some environment in which the dependency lives. | ||
| SELECT DISTINCT | ||
| cdr.deployment_id, | ||
| cer.environment_id, | ||
| cdr.resource_id | ||
| FROM computed_deployment_resource cdr | ||
| JOIN computed_environment_resource cer | ||
| ON cer.resource_id = cdr.resource_id | ||
| JOIN system_deployment sd | ||
| ON sd.deployment_id = cdr.deployment_id | ||
| JOIN system_environment se | ||
| ON se.environment_id = cer.environment_id | ||
| AND se.system_id = sd.system_id | ||
| WHERE cdr.deployment_id = @deployment_id | ||
| AND cdr.resource_id = @resource_id | ||
| LIMIT 1; |
There was a problem hiding this comment.
GetReleaseTargetForDeploymentResource picks an arbitrary environment via LIMIT 1 over computed tables, then GetCurrentReleaseByReleaseTarget looks for the latest successful release in that specific environment. If the chosen environment has no successful releases (but another environment does), the deployment-version-dependency evaluator will incorrectly deny. Consider changing this query (or adding a dedicated query) to select the environment that has the latest successful release for (deployment_id, resource_id), e.g., by joining release/release_job/job with j.status='successful' and ordering by j.completed_at DESC.
Comment on lines
+56
to
+72
| return results.NewDeniedResult( | ||
| fmt.Sprintf("Deployment dependency: failed to load dependencies: %v", err), | ||
| ).WithDetail("error", err.Error()) | ||
| } | ||
|
|
||
| if len(edges) == 0 { | ||
| return results.NewAllowedResult("Deployment dependency: no dependencies declared") | ||
| } | ||
|
|
||
| for _, edge := range edges { | ||
| if denied := e.evaluateEdge(ctx, scope, edge); denied != nil { | ||
| return denied | ||
| } | ||
| } | ||
|
|
||
| return results.NewAllowedResult("Deployment dependency: all dependencies satisfied") | ||
| } |
There was a problem hiding this comment.
All user-facing result messages in this evaluator start with "Deployment dependency:", which is easily confused with the existing policy-rule evaluator deploymentdependency (and the new rule type is deploymentVersionDependency). Consider updating the message prefix to something unambiguous like "Deployment version dependency:" so UI/logs clearly identify which evaluator produced the result.
Comment on lines
+83
to
+91
| deploymentId: uuid("deployment_id") | ||
| .references(() => deployment.id, { onDelete: "cascade" }) | ||
| .notNull(), | ||
| dependencyDeploymentId: uuid("dependency_deployment_id") | ||
| .references(() => deployment.id, { onDelete: "cascade" }) | ||
| .notNull(), | ||
| versionSelector: text("version_selector").notNull().default("false"), | ||
| }, | ||
| (t) => [primaryKey({ columns: [t.deploymentId, t.dependencyDeploymentId] })], |
There was a problem hiding this comment.
The new deployment_dependency table allows deployment_id == dependency_deployment_id, which would create a self-dependency edge. That can lead to confusing behavior (a deployment gating itself) and makes cycles easier to create. Consider adding a DB-level check constraint (and/or application validation) to prevent self-dependencies.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/workspace-engine/pkg/db/deployments.sql.go (1)
69-97: Consider a stable ordering for dependency rows.The evaluator will see dependencies in whatever order PostgreSQL returns them, so first-failure diagnostics can vary between runs. If that matters downstream, add a deterministic
ORDER BYhere.📌 Possible tweak
const getDeploymentDependenciesByDeploymentID = `-- name: GetDeploymentDependenciesByDeploymentID :many SELECT dependency_deployment_id, version_selector FROM deployment_dependency WHERE deployment_id = $1 +ORDER BY dependency_deployment_id `🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/pkg/db/deployments.sql.go` around lines 69 - 97, The SQL query getDeploymentDependenciesByDeploymentID (used by function GetDeploymentDependenciesByDeploymentID) returns rows in unspecified order; make the result deterministic by adding an ORDER BY to the query (for example ORDER BY dependency_deployment_id [, version_selector]) so dependencies are returned in a stable, reproducible order when scanning into GetDeploymentDependenciesByDeploymentIDRow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentversiondependency/getter_postgres.go`:
- Around line 27-30: The getter currently uses uuid.MustParse(deploymentID)
which can panic; replace it with id, err := uuid.Parse(deploymentID) and handle
the error path instead of panicking (e.g., return a wrapped error or
context-aware error) before calling
p.queries.GetDeploymentDependenciesByDeploymentID(ctx, id); update any
surrounding code in the getter function to propagate the parse error
consistently with other getters/controllers.
In
`@apps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval_test.go`:
- Around line 550-563: The test currently computes baseCount via baseCount :=
len(CollectEvaluators(ctx, getter, rt, nil)) which reuses the code under test;
instead hardcode the expected baseline evaluators (or their count) that must
always be present and assert those explicitly. Update the two subtests ("returns
base set for nil policies" and "skips nil policies") to call CollectEvaluators
but assert that the returned set contains the pinned always-present evaluator(s)
by name/type (e.g., the non-rule evaluator like deploymentVersionDependency or
whatever evaluator identifier is added in CollectEvaluators) and/or equals the
explicit expected length, rather than deriving baseCount from CollectEvaluators
itself; reference CollectEvaluators and the variable baseCount in your changes
so reviewers can locate the test logic to replace.
In `@packages/db/drizzle/0190_handy_luckman.sql`:
- Around line 1-9: Add a workspace_id column and thread it through the PK and
FKs: alter the "deployment_dependency" table (in 0190_handy_luckman.sql) to add
workspace_id uuid NOT NULL, change the primary key constraint
"deployment_dependency_deployment_id_dependency_deployment_id_pk" to include
workspace_id (e.g. PRIMARY KEY (workspace_id, deployment_id,
dependency_deployment_id)), and replace the single-column FKs on deployment_id
and dependency_deployment_id with composite FKs that reference the deployment
table’s (workspace_id, id) pair (or add an explicit FK to the workspace table)
so tenant isolation is enforced; update constraint names accordingly (e.g. the
two FK constraints "deployment_dependency_deployment_id_deployment_id_fk" and
"deployment_dependency_dependency_deployment_id_deployment_id_fk") and ensure ON
DELETE/ON UPDATE behaviors remain consistent.
---
Nitpick comments:
In `@apps/workspace-engine/pkg/db/deployments.sql.go`:
- Around line 69-97: The SQL query getDeploymentDependenciesByDeploymentID (used
by function GetDeploymentDependenciesByDeploymentID) returns rows in unspecified
order; make the result deterministic by adding an ORDER BY to the query (for
example ORDER BY dependency_deployment_id [, version_selector]) so dependencies
are returned in a stable, reproducible order when scanning into
GetDeploymentDependenciesByDeploymentIDRow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa90f95d-4441-4ac8-ace3-8c5e783cf9c2
📒 Files selected for processing (21)
apps/workspace-engine/oapi/openapi.jsonapps/workspace-engine/oapi/spec/schemas/deployments.jsonnetapps/workspace-engine/pkg/db/computed_resources.sql.goapps/workspace-engine/pkg/db/deployments.sql.goapps/workspace-engine/pkg/db/queries/computed_resources.sqlapps/workspace-engine/pkg/db/queries/deployments.sqlapps/workspace-engine/pkg/oapi/oapi.gen.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentversiondependency/deployment_version_dependency.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentversiondependency/deployment_version_dependency_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentversiondependency/getter.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/deploymentversiondependency/getter_postgres.goapps/workspace-engine/svc/controllers/desiredrelease/policyeval/getter.goapps/workspace-engine/svc/controllers/desiredrelease/policyeval/getter_postgres.goapps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval.goapps/workspace-engine/svc/controllers/desiredrelease/policyeval/policyeval_test.goapps/workspace-engine/svc/controllers/desiredrelease/reconcile_test.goapps/workspace-engine/test/controllers/harness/mocks.gopackages/db/drizzle/0190_handy_luckman.sqlpackages/db/drizzle/meta/0190_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/deployment.ts
Comment on lines
+1
to
+9
| CREATE TABLE "deployment_dependency" ( | ||
| "deployment_id" uuid NOT NULL, | ||
| "dependency_deployment_id" uuid NOT NULL, | ||
| "version_selector" text DEFAULT 'false' NOT NULL, | ||
| CONSTRAINT "deployment_dependency_deployment_id_dependency_deployment_id_pk" PRIMARY KEY("deployment_id","dependency_deployment_id") | ||
| ); | ||
| --> statement-breakpoint | ||
| ALTER TABLE "deployment_dependency" ADD CONSTRAINT "deployment_dependency_deployment_id_deployment_id_fk" FOREIGN KEY ("deployment_id") REFERENCES "public"."deployment"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "deployment_dependency" ADD CONSTRAINT "deployment_dependency_dependency_deployment_id_deployment_id_fk" FOREIGN KEY ("dependency_deployment_id") REFERENCES "public"."deployment"("id") ON DELETE cascade ON UPDATE no action; No newline at end of file |
Contributor
There was a problem hiding this comment.
Add workspace isolation to the new dependency table.
This table currently has no workspace_id, so dependency edges can cross tenant boundaries and can’t be scoped by workspace. Please thread workspace scope through the migration and keep the Drizzle schema aligned.
As per coding guidelines, All database tables include workspaceId for multi-tenant isolation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/drizzle/0190_handy_luckman.sql` around lines 1 - 9, Add a
workspace_id column and thread it through the PK and FKs: alter the
"deployment_dependency" table (in 0190_handy_luckman.sql) to add workspace_id
uuid NOT NULL, change the primary key constraint
"deployment_dependency_deployment_id_dependency_deployment_id_pk" to include
workspace_id (e.g. PRIMARY KEY (workspace_id, deployment_id,
dependency_deployment_id)), and replace the single-column FKs on deployment_id
and dependency_deployment_id with composite FKs that reference the deployment
table’s (workspace_id, id) pair (or add an explicit FK to the workspace table)
so tenant isolation is enforced; update constraint names accordingly (e.g. the
two FK constraints "deployment_dependency_deployment_id_deployment_id_fk" and
"deployment_dependency_dependency_deployment_id_deployment_id_fk") and ensure ON
DELETE/ON UPDATE behaviors remain consistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #1077
Summary by CodeRabbit