-
Notifications
You must be signed in to change notification settings - Fork 12
chore: init force deploy #723
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis change introduces a new force deploy feature across the workspace engine. It adds an OpenAPI schema definition for Changes
Sequence Diagram(s)sequenceDiagram
participant Event as Event System
participant Handler as Event Handler
participant Manager as Release Manager
participant Orchestrator as Deployment<br/>Orchestrator
participant Planner as Deployment<br/>Planner
Event->>Handler: ReleaseTargetForceDeployEvent
activate Handler
Handler->>Manager: ForceDeploy(releaseTarget, version)
deactivate Handler
activate Manager
Manager->>Orchestrator: ReconcileTarget(opts with forceDeployVersion)
deactivate Manager
activate Orchestrator
Orchestrator->>Planner: Phase 1: PlanDeployment(WithForceDeployVersion)
deactivate Orchestrator
activate Planner
note over Planner: deployableVersion = forceDeployVersion<br/>(skip policy-based selection)
Planner-->>Orchestrator: deployment plan
deactivate Planner
Orchestrator->>Orchestrator: Phase 2 & 3: Execute & Finalize
Orchestrator-->>Manager: success/error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
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: 2
🧹 Nitpick comments (1)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (1)
82-86: Add exported function documentation for WithForceDeployVersion.The
WithForceDeployVersionfunction is exported and bypasses normal policy-based version selection. Per the coding guidelines, exported functions should be documented. Add a godoc comment explaining:
- The purpose and that it bypasses policy evaluation
- That this is used for explicit force deploy operations (event-driven)
- That the caller is responsible for ensuring the version is valid for the target deployment
Suggested documentation:
// WithForceDeployVersion forces deployment of a specific version, bypassing normal // policy-based version selection. This is used for explicit force deploy operations // triggered via events. The caller is responsible for ensuring the version is valid // for the target deployment. func WithForceDeployVersion(version *oapi.DeploymentVersion) planDeploymentOptions { return func(cfg *planDeploymentConfig) { cfg.forceDeployVersion = version } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
apps/workspace-engine/oapi/openapi.json(1 hunks)apps/workspace-engine/oapi/spec/main.jsonnet(1 hunks)apps/workspace-engine/oapi/spec/schemas/release-targets.jsonnet(1 hunks)apps/workspace-engine/pkg/events/handler/redeploy/redeploy.go(1 hunks)apps/workspace-engine/pkg/oapi/oapi.gen.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go(3 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/manager.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/opts.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/workspace-engine/**/*.go
📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)
apps/workspace-engine/**/*.go: Do not add extraneous inline comments that state the obvious
Do not add comments that simply restate what the code does
Do not add comments for standard Go patterns (e.g., noting WaitGroup or semaphore usage)
Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods
Files:
apps/workspace-engine/pkg/oapi/oapi.gen.goapps/workspace-engine/pkg/events/handler/redeploy/redeploy.goapps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.goapps/workspace-engine/pkg/workspace/releasemanager/opts.goapps/workspace-engine/pkg/workspace/releasemanager/manager.go
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
apps/workspace-engine/oapi/openapi.json
🧠 Learnings (2)
📓 Common learnings
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 655
File: apps/workspace-engine/pkg/engine/workspace/fluent.go:166-171
Timestamp: 2025-08-12T20:49:05.086Z
Learning: The UpdateDeploymentVersions() method with OperationCreate case in apps/workspace-engine/pkg/engine/workspace/fluent.go is specifically designed only for creating new deployment versions, not for handling potential duplicates or existing versions.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
📚 Learning: 2025-08-12T20:49:05.086Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 655
File: apps/workspace-engine/pkg/engine/workspace/fluent.go:166-171
Timestamp: 2025-08-12T20:49:05.086Z
Learning: The UpdateDeploymentVersions() method with OperationCreate case in apps/workspace-engine/pkg/engine/workspace/fluent.go is specifically designed only for creating new deployment versions, not for handling potential duplicates or existing versions.
Applied to files:
apps/workspace-engine/pkg/oapi/oapi.gen.goapps/workspace-engine/pkg/events/handler/redeploy/redeploy.goapps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.goapps/workspace-engine/pkg/workspace/releasemanager/opts.goapps/workspace-engine/pkg/workspace/releasemanager/manager.go
🧬 Code graph analysis (6)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
apps/web/app/routes/ws/deployments/_components/types.ts (3)
ReleaseTarget(73-85)Version(54-54)DeploymentVersion(23-33)
apps/workspace-engine/pkg/events/handler/redeploy/redeploy.go (2)
apps/workspace-engine/pkg/events/handler/handler.go (1)
RawEvent(95-100)apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
ReleaseTargetForceDeployEvent(576-579)ReleaseTarget(569-573)
apps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.go (2)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (1)
WithForceDeployVersion(82-86)apps/workspace-engine/pkg/workspace/releasemanager/opts.go (1)
WithForceDeployVersion(46-50)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
DeploymentVersion(219-230)apps/workspace-engine/pkg/workspace/releasemanager/opts.go (1)
WithForceDeployVersion(46-50)
apps/workspace-engine/pkg/workspace/releasemanager/opts.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
DeploymentVersion(219-230)apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (1)
WithForceDeployVersion(82-86)
apps/workspace-engine/pkg/workspace/releasemanager/manager.go (3)
apps/workspace-engine/pkg/workspace/releasemanager/opts.go (3)
WithSkipEligibilityCheck(28-32)WithTrigger(34-38)WithForceDeployVersion(46-50)apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go (2)
WithTrigger(151-155)TriggerManual(94-94)apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go (1)
WithForceDeployVersion(82-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Lint
- GitHub Check: Typecheck
- GitHub Check: Format
- GitHub Check: workspace-engine-tests
- GitHub Check: tests
- GitHub Check: build (linux/amd64)
🔇 Additional comments (6)
apps/workspace-engine/pkg/workspace/releasemanager/opts.go (1)
15-15: LGTM!The new
forceDeployVersionoption follows the established pattern for other options in this file. The pointer type allows nil to represent "no forced version," which aligns with the optional nature of this feature.Also applies to: 46-50
apps/workspace-engine/oapi/openapi.json (1)
1175-1189: Schema structure is correct.The
ReleaseTargetForceDeployEventschema properly defines the required fields for force deployment. Since this is a generated file, the source Jsonnet definition (reviewed separately) is the primary concern.apps/workspace-engine/pkg/workspace/releasemanager/deployment_orchestrator.go (1)
97-97: LGTM!Correctly passes the forced deployment version option to the planner during Phase 1. The option pattern gracefully handles nil values, so no additional checks are needed here.
apps/workspace-engine/pkg/events/handler/redeploy/redeploy.go (1)
31-38: Verify error handling inconsistency with HandleReleaseTargetDeploy.
HandleReleaseTargetForceDeployreturns errors directly (Line 37), whileHandleReleaseTargetDeploylogs errors and returnsnil(lines 20-25). This difference could be intentional—force deploy failures might warrant hard failures—but should be confirmed.If force deploy should follow the same pattern as redeploy:
- return ws.ReleaseManager().ForceDeploy(ctx, &releaseTargetForceDeployEvent.ReleaseTarget, &releaseTargetForceDeployEvent.Version) + if err := ws.ReleaseManager().ForceDeploy(ctx, &releaseTargetForceDeployEvent.ReleaseTarget, &releaseTargetForceDeployEvent.Version); err != nil { + log.Warn("Failed to force deploy release target", + "releaseTargetKey", releaseTargetForceDeployEvent.ReleaseTarget.Key(), + "error", err.Error()) + return nil + } + + return nilapps/workspace-engine/oapi/spec/schemas/release-targets.jsonnet (1)
1-12: LGTM!The schema definition is clean and follows the established patterns in the codebase. Using
schemaReffor existing types maintains consistency and avoids duplication.apps/workspace-engine/oapi/spec/main.jsonnet (1)
40-41: LGTM!The new schema module is correctly imported and added to the components.schemas composition. The placement at the end is appropriate for this additive change.
| deployableVersion := cfg.forceDeployVersion | ||
|
|
||
| // Step 2: Find first version that passes user-defined policies | ||
| span.AddEvent("Step 2: Finding deployable version") | ||
| deployableVersion := p.findDeployableVersion(ctx, candidateVersions, releaseTarget, planning) | ||
| if deployableVersion == nil { | ||
| span.AddEvent("Step 2: Finding deployable version") | ||
| deployableVersion = p.findDeployableVersion(ctx, candidateVersions, releaseTarget, planning) | ||
| } |
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.
🧩 Analysis chain
Verify validation of forced deployment versions upstream.
The forced version bypasses all policy-based selection (Step 2), which could lead to deploying versions in invalid states or belonging to different deployments if not validated upstream.
Ensure the caller (likely manager.ForceDeploy) validates:
- The forced version belongs to the correct deployment (matching
releaseTarget.DeploymentId) - The version status is deployable (not "building", "failed", or "paused")
- Proper authorization checks are in place since policies are bypassed
Run the following script to check validation in the force deploy flow:
🏁 Script executed:
#!/bin/bash
# Check ForceDeploy implementation and event handler for validation
echo "=== Checking manager.ForceDeploy implementation ==="
ast-grep --pattern $'func ($$$) ForceDeploy($$$) {
$$$
}'
echo ""
echo "=== Checking HandleReleaseTargetForceDeploy for validation ==="
ast-grep --pattern $'func HandleReleaseTargetForceDeploy($$$) {
$$$
}'
echo ""
echo "=== Checking if version.DeploymentId is validated ==="
rg -n 'DeploymentId.*==|version.*Deployment' --type=go -g '**/manager.go' -g '**/redeploy.go'Length of output: 495
🏁 Script executed:
#!/bin/bash
# Extract ForceDeploy implementation
echo "=== ForceDeploy implementation (manager.go:288) ==="
sed -n '288,320p' apps/workspace-engine/pkg/workspace/releasemanager/manager.go
echo ""
echo "=== DeploymentId validation context (versions/manager.go around line 47) ==="
sed -n '40,60p' apps/workspace-engine/pkg/workspace/releasemanager/versions/manager.go
echo ""
echo "=== Check version status validation ==="
rg -n 'Status|state' apps/workspace-engine/pkg/workspace/releasemanager/manager.go -A2 -B2 | head -40Length of output: 3880
🏁 Script executed:
#!/bin/bash
# Check how forced version is handled in planner
echo "=== Checking planner.go for forced version handling ==="
rg -n 'forceDeployVersion|ForceDeployVersion' apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go -B2 -A5
echo ""
echo "=== Check findDeployableVersion in planner ==="
rg -n 'func.*findDeployableVersion' apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go -A20 | head -50
echo ""
echo "=== Check if version is validated before deployment ==="
rg -n 'Status|Deployable' apps/workspace-engine/pkg/workspace/releasemanager/deployment/ --type=go | head -20Length of output: 4849
🏁 Script executed:
#!/bin/bash
# Check what happens after deployableVersion is selected
echo "=== Planner flow after selecting deployableVersion (lines 143-165) ==="
sed -n '143,165p' apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go
echo ""
echo "=== Check orchestrator for version validation ==="
rg -n 'deployableVersion|validat' apps/workspace-engine/pkg/workspace/releasemanager/deployment/orchestrator.go --type=go -B2 -A2 | head -60
echo ""
echo "=== Check if deployment executor validates version status ==="
rg -n 'Status.*Ready|Status.*Building|Status.*Failed' apps/workspace-engine/pkg/workspace/releasemanager/ --type=go -l | head -10Length of output: 1957
Add validation for forced deployment versions to prevent deploying invalid states or wrong deployments.
The forced version (line 136) bypasses all policy validation (Step 2) without checking the version status (could be "Building", "Failed", or "Paused") or confirming it belongs to the correct deployment. Add validation after line 142:
if deployableVersion != nil && deployableVersion.DeploymentId != releaseTarget.DeploymentId {
span.AddEvent("Forced version belongs to different deployment")
span.SetStatus(codes.Error, "invalid_forced_version")
return nil, fmt.Errorf("forced version %s does not belong to deployment %s", deployableVersion.Id, releaseTarget.DeploymentId)
}
if deployableVersion != nil && deployableVersion.Status != oapi.DeploymentVersionStatusReady {
span.AddEvent("Forced version is not ready for deployment", attribute.String("status", string(deployableVersion.Status)))
span.SetStatus(codes.Error, "invalid_version_status")
return nil, fmt.Errorf("forced version %s is in %s state, cannot deploy", deployableVersion.Id, deployableVersion.Status)
}Also add validation in ForceDeploy (manager.go:288) before passing the version to ReconcileTarget, or clearly document that callers are responsible for validation.
🤖 Prompt for AI Agents
In apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner.go
around lines 136-142, the current logic accepts cfg.forceDeployVersion without
validation allowing forced deployments of versions that belong to other
deployments or that are in invalid statuses (e.g., Building, Failed, Paused);
add checks after the existing block to verify deployableVersion != nil then (1)
ensure deployableVersion.DeploymentId equals releaseTarget.DeploymentId and
return a clear error/trace/span event if not, and (2) ensure
deployableVersion.Status == oapi.DeploymentVersionStatusReady and return a clear
error/trace/span event if not; also add equivalent validation in manager.go at
ForceDeploy (around line 288) before calling ReconcileTarget (or document
callers must validate) so forced versions cannot bypass policy/status checks.
| func (m *Manager) ForceDeploy(ctx context.Context, releaseTarget *oapi.ReleaseTarget, version *oapi.DeploymentVersion) error { | ||
| ctx, span := tracer.Start(ctx, "ForceDeploy") | ||
| defer span.End() | ||
|
|
||
| return m.ReconcileTarget(ctx, releaseTarget, | ||
| WithSkipEligibilityCheck(true), | ||
| WithTrigger(trace.TriggerManual), | ||
| WithForceDeployVersion(version)) | ||
| } |
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.
Add validation and documentation.
The ForceDeploy method has several concerns compared to the similar Redeploy method:
-
Missing in-progress job check: Unlike
Redeploy(lines 262-281), this doesn't check if a job is already processing. This could create multiple concurrent jobs for the same release target, leading to race conditions and wasted resources. -
Missing input validation: No nil check on the
versionparameter or verification that the version belongs to the target's deployment. -
Missing documentation: No godoc explaining what "force deploy" means, how it differs from
Redeploy, or when to use it.
Consider applying this pattern:
+// ForceDeploy forces deployment of a specific version for a release target,
+// bypassing policy-based version selection and eligibility checks.
+//
+// Unlike Redeploy which uses the policy-determined version, ForceDeploy deploys
+// the exact version specified, regardless of policies. This is useful for rollbacks
+// or emergency deployments.
+//
+// Returns error if:
+// - version is nil
+// - version doesn't belong to the target's deployment
+// - A job is already in progress for this release target
+// - Planning or execution fails
func (m *Manager) ForceDeploy(ctx context.Context, releaseTarget *oapi.ReleaseTarget, version *oapi.DeploymentVersion) error {
ctx, span := tracer.Start(ctx, "ForceDeploy")
defer span.End()
+ // Validate inputs
+ if version == nil {
+ err := fmt.Errorf("version cannot be nil")
+ span.RecordError(err)
+ span.SetStatus(codes.Error, "invalid input")
+ return err
+ }
+
+ if version.DeploymentId != releaseTarget.DeploymentId {
+ err := fmt.Errorf("version %s does not belong to deployment %s", version.Id, releaseTarget.DeploymentId)
+ span.RecordError(err)
+ span.SetStatus(codes.Error, "version mismatch")
+ return err
+ }
+
+ // Check if there's already a job in progress for this release target
+ inProgressJobs := m.store.Jobs.GetJobsInProcessingStateForReleaseTarget(releaseTarget)
+ if len(inProgressJobs) > 0 {
+ var jobId string
+ var jobStatus oapi.JobStatus
+ for _, job := range inProgressJobs {
+ jobId = job.Id
+ jobStatus = job.Status
+ break
+ }
+
+ err := fmt.Errorf("cannot force deploy: job %s already in progress (status: %s)", jobId, jobStatus)
+ span.RecordError(err)
+ span.SetStatus(codes.Error, "job in progress")
+ log.Warn("ForceDeploy blocked: job already in progress",
+ "releaseTargetKey", releaseTarget.Key(),
+ "jobId", jobId,
+ "jobStatus", jobStatus)
+ return err
+ }
+
return m.ReconcileTarget(ctx, releaseTarget,
WithSkipEligibilityCheck(true),
WithTrigger(trace.TriggerManual),
WithForceDeployVersion(version))
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/workspace-engine/pkg/workspace/releasemanager/manager.go around lines
288 to 296, the ForceDeploy method lacks the in-progress job check, input
validation, and documentation present on Redeploy; add a godoc comment
explaining what ForceDeploy does and how it differs from Redeploy (e.g., forces
a specific deployment version, bypasses eligibility checks), then in the method
body perform the same "is job already processing" check used by Redeploy to
return ErrJobProcessing if a job exists, validate that the version parameter is
non-nil and that the version.DeploymentID (or equivalent) matches the
releaseTarget's deployment ID (return an appropriate error if not), and only
then call ReconcileTarget with WithSkipEligibilityCheck(true),
WithTrigger(trace.TriggerManual), and WithForceDeployVersion(version).
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.