Skip to content

feat: add manual apply now action#641

Open
arnaud-dezandee wants to merge 2 commits intopadok-team:mainfrom
arnaud-dezandee:feat/manual-apply
Open

feat: add manual apply now action#641
arnaud-dezandee wants to merge 2 commits intopadok-team:mainfrom
arnaud-dezandee:feat/manual-apply

Conversation

@arnaud-dezandee
Copy link
Copy Markdown
Contributor

@arnaud-dezandee arnaud-dezandee commented Jul 1, 2025

Hi team,

This PR adds a manual "Apply Now" feature, allowing users to manually trigger an apply operation on Terraform layers that have a valid plan but autoApply is disabled.

Changes

  • Added api.terraform.padok.cloud/apply-now annotation to manually trigger applies
  • Added IsApplyScheduled condition check in the controller to handle manual apply requests
  • Modified ApplyNeeded state to distinguish between manual and automatic applies (manual applies bypass autoApply checks)
  • Added new API endpoint POST /layers/:namespace/:layer/apply to trigger manual applies
  • Updated UI with a "Play" icon button to trigger manual applies (shown only when autoApply is false and layer has a valid plan)
  • Added tooltip logic to guide users when manual apply is not available
  • Added unit tests for the manual apply flow

@github-project-automation github-project-automation bot moved this to 📋 Backlog in burrito Jul 1, 2025
@arnaud-dezandee
Copy link
Copy Markdown
Contributor Author

Fix #311

@arnaud-dezandee
Copy link
Copy Markdown
Contributor Author

Screenshot 2025-07-01 at 15 47 27

@michael-todorovic
Copy link
Copy Markdown
Contributor

Screenshot 2025-07-01 at 15 47 27

I'm excited getting this as we don't set auto apply yet :D I'm wondering whether the button should be shown only if state=OutOfSync, wdyt?

@AlanLonguet
Copy link
Copy Markdown
Collaborator

@arnaud-dezandee Thanks for the contribution, LGTM, could you add a test case for the ManualApplyNeeded state ?

@arnaud-dezandee
Copy link
Copy Markdown
Contributor Author

I'm excited getting this as we don't set auto apply yet :D I'm wondering whether the button should be shown only if state=OutOfSync, wdyt?

I've considered it but you could have a TerraformLayer with side effects that are not translated into state, and thus, you may want to apply it multiple times with the same version.

@LucasMrqes
Copy link
Copy Markdown
Collaborator

Hi Arnaud, thanks a lot for the contribution! This is a highly requested feature, and I'm glad you're taking it on.

That said, I have a few concerns regarding the current implementation. At the moment, the behavior of the "Sync" button changes depending on the autoApply setting:

  • When autoApply = false: the Sync button triggers a plan run.
  • When autoApply = true: the Sync button triggers a plan, and since the layer ends up in an ApplyNeeded state after that, an apply run is subsequently triggered.

This leads to the following behavior:

  • With autoApply = false: the buttons behave as expected—"Sync" runs a plan, and "Apply" runs an apply.
  • With autoApply = true: the "Sync" button ends up running a plan + apply flow, while the "Apply" button tries to apply based on the latest plan—which, in this case, should have already been applied automatically.

Suggestions

To make the behavior clearer and more consistent for users, I propose the following:

  • If autoApply = false: rename the "Sync" button to "Plan" in the frontend, for clarity.
  • If autoApply = true: hide the "Apply" button, and keep only the "Sync" button to represent the full plan + apply workflow.

Let me know what you think!
(cc @corrieriluca @AlanLonguet)

@corrieriluca
Copy link
Copy Markdown
Member

I agree with your suggestion @LucasMrqes, we need in the frontend to clarify what "Sync" means.

I suggest the following:

  • If autoApply = false: rename the "Sync" button to "Plan" in the frontend, so we have 2 buttons: "Plan" and "Apply".
  • If autoApply = true: hide the "Apply" button, and keep only the "Sync" button to represent the full plan + apply workflow.
    • Display a popover when the mouse is hover the Sync button to explain Sync is a Plan + Apply.

@corrieriluca
Copy link
Copy Markdown
Member

Reviewing this PR, I think introducing the new state ManualApplyNeeded in the TerraformLayer is a bit overkill 😅

To me, if the condition IsApplyScheduled is true, the layer should end in the ApplyNeeded state 🙂

WDYT @arnaud-dezandee?

@arnaud-dezandee
Copy link
Copy Markdown
Contributor Author

Hi @LucasMrqes @corrieriluca, I've added the autoApply status to the layer response to adjust frontend labels following your suggestions. I've kept the Apply button always visible for the same reason mentioned previously; users may want to "Force apply" to re-run untracked side effects.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 5, 2025

Codecov Report

❌ Patch coverage is 76.47059% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.26%. Comparing base (00f01d0) to head (d031aca).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
internal/server/utils/manual_sync.go 0.00% 7 Missing ⚠️
internal/server/api/layers.go 82.75% 3 Missing and 2 partials ⚠️
internal/controllers/terraformlayer/states.go 73.33% 2 Missing and 2 partials ⚠️
internal/server/api/sync.go 83.33% 2 Missing and 1 partial ⚠️
internal/server/server.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #641      +/-   ##
==========================================
+ Coverage   40.62%   42.26%   +1.64%     
==========================================
  Files          94       94              
  Lines        5539     5612      +73     
==========================================
+ Hits         2250     2372     +122     
+ Misses       3091     3030      -61     
- Partials      198      210      +12     

☔ 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

users may want to "Force apply" to re-run untracked side effects

@arnaud-dezandee I agree. In that case, do you think the "apply" must be done with or without previous plan artifact?

By default for apply actions Burrito creates a TerraformRun that references the last plan artifact produced:

func (r *Reconciler) getRun(layer *configv1alpha1.TerraformLayer, repository *configv1alpha1.TerraformRepository, action Action) configv1alpha1.TerraformRun {
artifact := configv1alpha1.Artifact{}
if action == ApplyAction {
run := strings.Split(layer.Annotations[annotations.LastPlanRun], "/")
artifact.Attempt = run[1]
artifact.Run = run[0]
}

In the Burrito runner pod, the artifact will be used for the "apply" unless the setting spec.remediationStrategy.applyWithoutPlanArtifact is set to True in the repository or layer.

if configv1alpha1.GetApplyWithoutPlanArtifactEnabled(r.Repository, r.Layer) {
log.Infof("applying without reusing plan artifact from previous plan run")
err = r.exec.Apply("")
} else {
err = r.exec.Apply(PlanArtifact)
}

@corrieriluca
Copy link
Copy Markdown
Member

corrieriluca commented Jul 5, 2025

Also, I just thought about an edge case to the Apply Now button. It should not be permitted to run a manual apply run on layers that are generated by the TerraformPullRequest controller as they are temporary layers referencing temporary branches. Only plans should be allowed for these layers.

They can be identified by the label burrito/managed-by (see code) or via OwnerReferences (if the owner's Kind is TerraformPullRequest then block manual apply)

@arnaud-dezandee
Copy link
Copy Markdown
Contributor Author

Also, I just thought about an edge case to the Apply Now button. It should not be permitted to run a manual apply run on layers that are generated by the TerraformPullRequest controller as they are temporary layers referencing temporary branches. Only plans should be allowed for these layers.

They can be identified by the label burrito/managed-by (see code) or via OwnerReferences (if the owner's Kind is TerraformPullRequest then block manual apply)

@corrieriluca Done, I've added a guard to prevent PR layers to be applied

@arnaud-dezandee
Copy link
Copy Markdown
Contributor Author

I agree. In that case, do you think the "apply" must be done with or without previous plan artifact?

By default for apply actions Burrito creates a TerraformRun that references the last plan artifact produced:

We could add a "Hard Apply" option to the manual action. Maybe in another PR
Screenshot 2025-07-07 at 11 24 45

@arnaud-dezandee
Copy link
Copy Markdown
Contributor Author

Hi @corrieriluca,
This PR has been stale for a few months sorry. I've picked it up again and reworked it following all the comments made
If you can take a new look at it 🙏

@corrieriluca corrieriluca linked an issue Feb 27, 2026 that may be closed by this pull request
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.

Feature: Apply now Button

6 participants