chore: add reconcile hook for env progression dependencies by adityachoudhari26 · Pull Request #736 · ctrlplanedev/ctrlplane

adityachoudhari26 · 2026-01-14T01:09:20Z

Summary by CodeRabbit

New Features
- Policy-driven environment progression now automates deployment advancement across environments. Deployments automatically progress to subsequent environments when specified policy conditions are satisfied.
- Enables staged and gradual rollout workflows for safer production deployments.
Tests
- Added comprehensive end-to-end test coverage for environment progression scenarios and rollout workflows.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-14T01:09:34Z

📝 Walkthrough

Walkthrough

The pull request introduces environment progression functionality by adding an action that evaluates policies restricting target progression between environments, collects matching release targets, and reconciles them through a provided function. The workspace engine is updated to integrate this action with a unified reconciliation pattern, and end-to-end tests validate the policy-driven progression behavior.

Changes

Cohort / File(s)	Summary
Core Action Implementation `apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environment_progression_action.go`	New EnvironmentProgressionAction type with Execute method that: evaluates environment progression policies against current environment, collects matching release targets via GetForPolicy, and reconciles each target. Includes helper methods for environment lookup, policy filtering, target aggregation, and error-aware reconciliation. Total 122 lines added.
Release Target Query Support `apps/workspace-engine/pkg/workspace/store/release_targets.go`	New GetForPolicy method that iterates all release targets, resolves associated environment/deployment/resource, and evaluates policy match. Returns map of matching targets keyed by target.Key(), skipping targets with resolution errors.
Workspace Integration `apps/workspace-engine/pkg/workspace/workspace.go`	Added environmentprogression import and introduced reconcileFn closure for forwarding target reconciliation to release manager. Refactored action registration chain to use unified reconcileFn across deployment dependency and environment progression actions.
End-to-End Tests `apps/workspace-engine/test/e2e/engine_policy_environment_progression_test.go`	Two test cases validating progression flow: TestEngine_PolicyEnvironmentProgression_TriggersGradualRollout and TestEngine_PolicyEnvironmentProgression_TriggersGradualRolloutStart. Both verify that completing QA environment triggers PROD job creation via policy-driven progression.

Sequence Diagram

sequenceDiagram
    participant E as Execute
    participant S as Store
    participant P as Policy Evaluator
    participant R as Release Manager

    E->>S: getEnvironment(envId)
    S-->>E: environment
    
    E->>P: getProgressionDependentPolicies(environment)
    P-->>E: policies[]
    
    loop For each policy
        E->>S: GetForPolicy(policy)
        S-->>E: releaseTargets{}
    end
    
    E->>E: Aggregate unique targets
    
    loop For each target
        E->>R: reconcileFn(target)
        R-->>E: result/error
    end
    
    E-->>E: Return aggregated errors

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

chore: dependencies trigger downstream target reconciles #731: Introduces similar ReconcileFn pattern for another policy-driven action and uses identical workspace.go integration approach with TriggerJobSuccess.
chore: grad rollouts rule go #706: Implements gradual rollout rule and evaluator that interacts with this PR's release target selection and reconciliation code.
policy view #682: Adds complementary release-target/policy relationship query (GetPolicies) to the store, opposite direction from GetForPolicy.

Poem

🐰 Through progression's tunnel, policies light the way,
QA to PROD, each target finds its day,
With reconcileFn magic and targets in hand,
Environment hops bloom across the land! 🌱✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly summarizes the main change: adding a reconciliation hook mechanism for environment progression dependencies, which is the core focus of all modified files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧹 Recent nitpick comments

apps/workspace-engine/pkg/workspace/store/release_targets.go (1)

189-214: Consider removing unused error return or adding error paths.

The method signature returns an error, but no code path ever returns a non-nil error. This differs from GetPolicies (lines 127-158) which returns errors for missing entities. Either:

Remove the error return since it's unused, or

Add error logging/aggregation for skipped targets to aid debugging stale references

The silent skip behavior is acceptable for resilience, but the unused error return is misleading.

apps/workspace-engine/test/e2e/engine_policy_environment_progression_test.go (1)

15-15: Misleading test name: no gradual rollout policy is configured.

This test only configures an environment progression policy (no WithRuleGradualRollout). Consider renaming to TestEngine_PolicyEnvironmentProgression_TriggersProdReconciliation or similar to accurately reflect what's being tested.

The second test (TriggersGradualRolloutStart) correctly tests with both policies.

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 46c00fd and 3d922a6.

📒 Files selected for processing (4)

apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environment_progression_action.go
apps/workspace-engine/pkg/workspace/store/release_targets.go
apps/workspace-engine/pkg/workspace/workspace.go
apps/workspace-engine/test/e2e/engine_policy_environment_progression_test.go

🧰 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/workspace/store/release_targets.go
apps/workspace-engine/test/e2e/engine_policy_environment_progression_test.go
apps/workspace-engine/pkg/workspace/workspace.go
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environment_progression_action.go

apps/workspace-engine/**/*_test.go

📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)

Follow the existing test structure used in *_test.go files

Files:

apps/workspace-engine/test/e2e/engine_policy_environment_progression_test.go

🧠 Learnings (8)

📓 Common learnings

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.