Skip to content

fix(load_balancer): lift delete protection when deleting the resource - #1480

Open
nikolauspschuetz wants to merge 1 commit into
hetznercloud:mainfrom
nikolauspschuetz:fix/lb-lift-delete-protection-1206
Open

nikolauspschuetz wants to merge 1 commit into
hetznercloud:mainfrom
nikolauspschuetz:fix/lb-lift-delete-protection-1206

Conversation

@nikolauspschuetz

Copy link
Copy Markdown
Contributor

Closes #1206.

Problem

The delete-protection docs state that a protected resource can still be deleted by Terraform — protection only prevents other API consumers (e.g. the Cloud Console) from deleting it. But hcloud_load_balancer failed with load balancer deletion is protected instead of deleting.

As @apricote noted in the issue, only hcloud_zone_rrset currently lifts protection to make its changes; the other resources don't follow the documented behaviour.

Fix

In resourceLoadBalancerDelete, if the Load Balancer has delete protection enabled, disable it (via the existing setProtection helper) before deleting — mirroring hcloud_zone_rrset. Adds TestAccLoadBalancerResource_DeleteProtectionLifted, which creates a Load Balancer with delete_protection = true and never disables it, so the framework's CheckDestroy must delete a still-protected resource (fails without this change).

Scope

I've scoped this to hcloud_load_balancer — the resource in the issue (and the one that bites the hcloud-cloud-controller-manager use case). Your inventory in #1206 lists the same gap for floating_ip, network, primary_ip, server, volume, and zone. Happy to extend this PR to cover the rest in one coordinated change if you'd prefer that over per-resource PRs — just let me know.

Testing

go build ./..., go vet, and gofmt are clean; the new acceptance test compiles and is skipped without TF_ACC. I haven't run it against a live account in this PR — glad for CI / a maintainer to run it, and I can also run it against my own throwaway project if useful.

AI assistance

Implemented with Claude Code; I directed the change (reusing the in-repo setProtection helper and the zone_rrset pattern) and verified build/vet/test-compile locally.

The delete-protection docs state that a protected resource can still be
deleted by Terraform (protection only guards against other API consumers),
but `hcloud_load_balancer` returned "load balancer deletion is protected"
instead of deleting (hetznercloud#1206).

Before deleting, disable delete protection if it is enabled, mirroring the
behaviour already implemented for `hcloud_zone_rrset`. Adds a regression
acceptance test that destroys a still-protected Load Balancer.

Closes hetznercloud#1206

Assisted-by: Claude Code (Anthropic, Opus 4.x)
@nikolauspschuetz
nikolauspschuetz requested a review from a team as a code owner July 8, 2026 04:10
@jooola

jooola commented Jul 14, 2026

Copy link
Copy Markdown
Member

Hey, I've added a comment to #1206, where I find it risky to change the behavior of the delete protection behavior within a minor release.

So this PR alone will not suffice to fix #1206.

@nikolauspschuetz

Copy link
Copy Markdown
Contributor Author

Thanks @jooola — that's fair. Silently changing delete-protection behavior in a
minor release is risky, and a provider-wide setting that lets users opt in while
preserving today's behavior is the safer path.

Happy to implement that. Before I rework this PR: do you have a preferred shape
for the setting — e.g. a delete_protection behavior attribute on the provider
block — and should it default to the current (existing) behavior? I'll align the
change in #1206 to whatever you and @apricote settle on.

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.88%. Comparing base (7a02e66) to head (e279a0c).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
internal/loadbalancer/resource.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1480       +/-   ##
===========================================
- Coverage   73.19%   37.88%   -35.32%     
===========================================
  Files         115      115               
  Lines       12776    12779        +3     
===========================================
- Hits         9352     4841     -4511     
- Misses       2347     7874     +5527     
+ Partials     1077       64     -1013     
Flag Coverage Δ
e2e ?
unit 37.88% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: delete_protection doesnt work as documented

2 participants