Skip to content

feat: add destroy functionality with auto-delete option for TerraformLayer#770

Open
nerdeveloper wants to merge 3 commits into
padok-team:mainfrom
nerdeveloper:feature/add-destroy-button
Open

feat: add destroy functionality with auto-delete option for TerraformLayer#770
nerdeveloper wants to merge 3 commits into
padok-team:mainfrom
nerdeveloper:feature/add-destroy-button

Conversation

@nerdeveloper

@nerdeveloper nerdeveloper commented Nov 29, 2025

Copy link
Copy Markdown
Contributor

Summary

This PR adds destroy functionality to Burrito, similar to ArgoCD's delete behavior where deleting an Application can also delete the underlying Kubernetes resources. With the deleteAfterDestroy option enabled, when infrastructure is destroyed, the TerraformLayer resource is automatically cleaned up - keeping the Burrito state in sync with the actual infrastructure state.

Features

  • Add destroy-now annotation (api.terraform.padok.cloud/destroy-now) to trigger infrastructure destruction
  • Add deleteAfterDestroy option in RemediationStrategy to auto-delete TerraformLayer after successful destroy (similar to ArgoCD's cascade delete)
  • Add autoDestroy option to control whether destroy apply runs automatically
  • Add new layer states: DestroyNeeded and DestroyApplyNeeded
  • Add runner actions: plan-destroy and apply-destroy
  • Update UI to show "Deleted" state with purple badge when infrastructure is destroyed

Test plan

  • Create TerraformLayer with deleteAfterDestroy: true
  • Verify plan and apply run successfully
  • Trigger destroy with api.terraform.padok.cloud/destroy-now annotation
  • Verify plan-destroy and apply-destroy runs complete
  • Verify layer auto-deletes after successful destroy
  • Verify "Deleted" state shows in UI with purple badge

@github-project-automation github-project-automation Bot moved this to 📋 Backlog in Burrito Nov 29, 2025
@nerdeveloper

Copy link
Copy Markdown
Contributor Author

please @AlanLonguet and @corrieriluca PTAL and review, lmk what you think

@bradenwright

Copy link
Copy Markdown

Thanks @nerdeveloper obviously we've talked but this will be great for us, b/c we manage TerraformLayers via ArgoCD, so right now when they get deleted they leave resources around. This will make it so TerraformLayers can clean up after themselves! This will be fantastic!

@nerdeveloper nerdeveloper force-pushed the feature/add-destroy-button branch from 8393988 to 6eab121 Compare December 1, 2025 01:14
@codecov

codecov Bot commented Dec 1, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 11.90476% with 222 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.55%. Comparing base (7dbe3f2) to head (6eab121).

Files with missing lines Patch % Lines
internal/runner/actions.go 3.17% 60 Missing and 1 partial ⚠️
internal/controllers/terraformlayer/states.go 6.12% 46 Missing ⚠️
internal/controllers/terraformlayer/conditions.go 46.66% 23 Missing and 1 partial ⚠️
internal/server/api/destroy.go 0.00% 21 Missing ⚠️
internal/controllers/terraformlayer/controller.go 13.04% 19 Missing and 1 partial ⚠️
internal/server/auth/auth.go 0.00% 13 Missing ⚠️
internal/runner/tools/terragrunt/terragrunt.go 0.00% 11 Missing ⚠️
internal/controllers/terraformrun/pod.go 0.00% 10 Missing ⚠️
internal/runner/tools/base/base.go 0.00% 7 Missing ⚠️
api/v1alpha1/common.go 0.00% 4 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #770      +/-   ##
==========================================
- Coverage   39.79%   38.55%   -1.25%     
==========================================
  Files          94       95       +1     
  Lines        5465     5711     +246     
==========================================
+ Hits         2175     2202      +27     
- Misses       3093     3309     +216     
- Partials      197      200       +3     

☔ 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 @nerdeveloper @bradenwright

Thanks for the PR. This is an important feature and we understand your use-case. However, after internal discussion there are a few points we'd like to discuss with you:

  • We’d prefer an implementation based on Kubernetes finalizers. Finalizers are the controller pattern designed for resource cleanup (here is an example from KubeBuilder's doc) and fit better with controller lifecycle guarantees than removing resources with the API and with an additional DestroyNeeded state on Layers.

  • Because Burrito does not yet implement any RBAC, we prefer to see this feature gated behind a feature flag in the controller’s global configuration to reduce the risk of incidents. The behavior should be opt-in (disabled by default) and clearly documented so operators can enable it deliberately.

  • For a large change like this, we’d also prefer an issue opened first so we can discuss design, edge cases and rollout strategy on GitHub or on Discord before proceeding with a full implementation. Could you open an issue describing the proposed design and link it here? We’ll review and iterate there so the implementation can go forward with consensus.

Happy to help with the design and with wording for the feature flag and finalizer approach 😄

…Layer

This PR adds destroy functionality to Burrito, similar to ArgoCD's delete
behavior where deleting an Application can also delete the underlying
Kubernetes resources.

Features:
- Add destroy-now annotation to trigger infrastructure destruction
- Add deleteAfterDestroy option in RemediationStrategy to auto-delete
  TerraformLayer after successful destroy (similar to ArgoCD cascade delete)
- Add autoDestroy option to control automatic destroy apply
- Add new layer states: DestroyNeeded and DestroyApplyNeeded
- Add runner actions: plan-destroy and apply-destroy
- Update UI to show "Deleted" state with purple badge
…st-forward bug

The go-git library's Pull() function has a known issue (padok-team#358) where it
returns 'non-fast-forward update' errors even when a fast-forward merge
is possible. This happens when local branches don't have proper upstream
tracking configured.

This commit replaces Pull() with an explicit Fetch() + Hard Reset approach:
1. Fetch latest changes from remote with Force=true
2. Get the remote reference for the target branch
3. Hard reset the worktree to the remote ref
4. Update the local branch reference

This approach:
- Avoids the go-git Pull() bug entirely
- Is more explicit about what we want (always match remote)
- Handles force-push scenarios correctly
- Works regardless of tracking configuration

Fixes: go-git/go-git#358
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 📋 Backlog

Development

Successfully merging this pull request may close these issues.

3 participants