Skip to content

feat: Support plan approval#797

Open
fatmcgav wants to merge 2 commits intopadok-team:mainfrom
fatmcgav:feat-support-plan-approval
Open

feat: Support plan approval#797
fatmcgav wants to merge 2 commits intopadok-team:mainfrom
fatmcgav:feat-support-plan-approval

Conversation

@fatmcgav
Copy link
Copy Markdown

@fatmcgav fatmcgav commented Jan 2, 2026

This PR adds support for requiring Terraform plan approval before
applying.

As part of this:

  • Add new PlanApprovalRequired bool to the RemediationStrategy type,
    and add support for repo scoped and layer scoped usage
  • Regenerate CRDs
  • Add new plan-approved runner annotation
  • Update TerraformLayer controller in order to:
    1. Add a new IsPlanApproved condition
    2. Update Reconciler logic to leverage the new
      PlanApprovalRequired config item
    3. Add a new PlanApprovalRequired state to differentiate
    4. Update ApplyNeeded handler to still apply when autoApply is
      disabled but PlanApprovalRequired is enabled.
  • Add new approve POST endpoint to the API and supporting utils
    function

Starts laying some foundations for #751

TODO:

  • Add some docs

This PR adds support for requiring Terraform plan approval before
applying.

As part of this:
* Add new `PlanApprovalRequired` bool to the `RemediationStrategy` type,
  and add support for repo scoped and layer scoped usage
* Regenerate CRDs
* Add new `plan-approved` runner annotation
* Update `TerraformLayer` controller in order to:
  1. Add a new `IsPlanApproved` condition
  2. Update `Reconciler` logic to leverage the new
     `PlanApprovalRequired` config item
  3. Add a new `PlanApprovalRequired` state to differentiate
  4. Update `ApplyNeeded` handler to still apply when `autoApply` is
     disabled but `PlanApprovalRequired` is enabled.
* Add new `approve` POST endpoint to the API and supporting utils
  function

Starts laying some foundations for padok-team#751
Also add supporting testdata fixture.
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 9, 2026

Codecov Report

❌ Patch coverage is 58.82353% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.95%. Comparing base (260dd06) to head (f2db54f).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
internal/server/api/approve.go 0.00% 18 Missing ⚠️
internal/controllers/terraformlayer/conditions.go 82.14% 4 Missing and 1 partial ⚠️
internal/server/utils/approve_plan.go 0.00% 4 Missing ⚠️
internal/controllers/terraformlayer/states.go 89.28% 1 Missing and 2 partials ⚠️
api/v1alpha1/common.go 0.00% 2 Missing ⚠️
internal/server/api/layers.go 0.00% 2 Missing ⚠️
internal/server/server.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #797      +/-   ##
==========================================
+ Coverage   39.79%   39.95%   +0.15%     
==========================================
  Files          94       96       +2     
  Lines        5465     5541      +76     
==========================================
+ Hits         2175     2214      +39     
- Misses       3093     3128      +35     
- Partials      197      199       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@corrieriluca
Copy link
Copy Markdown
Member

Hi @fatmcgav

Thanks for the PR. As a general rule, we prefer discussing implementation details of such contribution in a issue or on Burrito's Discord server (link in README).

That being said, your PR looks complete, with test cases and generated files 😄

Regarding the apply-before-merged feature requested in #751 and your PR:

  • your feature would allow a layer with autoApply: false to be applied if the plan gets approved?
  • what about layers with autoApply: true (on main branch for example)? The plan approval step is skipped?
  • if the layer whose plan to be approved is a temporary PullRequest's layer, if it's applied while not merged, how to not trigger a drift detection plan on the "original" layer on main branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 📋 Backlog

Development

Successfully merging this pull request may close these issues.

2 participants