feat: add option to disable deletion protection, CloudSQLBackup resource and fixes by taivox · Pull Request #280 · ekristen/gcp-nuke

taivox · 2026-01-31T15:14:21Z

Improvements

Added option to disable deletion protection via config file
Added CloudSQLBackup resource
Fixed cleanup of IAM permissions for deleted service accounts to avoid orphaned bindings
Fixed DNS policy deletion when attached to a VPC
Fixed Cloud Storage bucket deletion by adding removal of soft-deleted objects, multi-region and dual-region buckets
Updated and improved documentation

…rmissions

ekristen · 2026-01-31T23:58:27Z

Thanks for another PR. The only issue I see is that adding a custom flag to disabling deletion protection. We do this a lot on AWS and it's done by per resource settings to disable vs global. I think I would prefer to keep that behavior as it allows for more granular per resource type vs global.

taivox · 2026-02-01T08:37:05Z

I also added an AWS-like option to disable deletion protection via the config file, for example:

settings:
  CloudSQLInstance:
    DisableDeletionProtection: true
  FilestoreInstance:
    DisableDeletionProtection: true

The --disable-deletion-protection flag was added as a global override to disable deletion protection for all supported resources.
If you prefer, I can remove the global flag and just keep the option to disable it within settings block of the config file.
Let me know what you think.

ekristen · 2026-02-01T16:45:00Z

I've purposely avoided global as much as possibly to prevent unintended actions.

I'm open to the idea of keeping it as a cli option to disable for specific resource types.

I'm open to other counter arguments, definitely want to the tool to be easy enough to use but try and prevent bad accidental usage.

taivox · 2026-02-02T12:28:25Z

Makes sense. Removed the global --disable-deletion-protection flag.
Deletion protection can still be disabled per resource type via config:

settings:
  CloudSQLInstance:
    DisableDeletionProtection: true

taivox · 2026-02-18T18:56:22Z

Can you review this again? @ekristen

ekristen · 2026-02-18T21:10:20Z

Thanks for the bump.

ekristen

@taivox thanks for the submission but it looks like there are a few things that need fixing.

I've dropped several in-line comments.

Additionally, is CloudSQLBackup multi-region logic right? I think it's regional, seeming that this might have been copied from StorageBucket?

CloudSQLBackup is also missing filtering --- no filtering of non-deletable backup states — CloudSQLBackup lists all backups but doesn't filter out FAILED or DELETED states via a Filter() method, probably should also include DELETION_PENDING, and DELETION_FAILED.

taivox · 2026-03-08T15:19:45Z

@taivox thanks for the submission but it looks like there are a few things that need fixing.

I've dropped several in-line comments.

Additionally, is CloudSQLBackup multi-region logic right? I think it's regional, seeming that this might have been copied from StorageBucket?

CloudSQLBackup is also missing filtering --- no filtering of non-deletable backup states — CloudSQLBackup lists all backups but doesn't filter out FAILED or DELETED states via a Filter() method, probably should also include DELETION_PENDING, and DELETION_FAILED.

Thanks for the review.

It think that the CloudSQLBackup logic is correct. Cloud SQL backups can be regional and multi-regional. Link to documentation: https://docs.cloud.google.com/sql/docs/mysql/locations#location-mr

Added other requested fixes as well.

taivox · 2026-03-20T14:03:35Z

Can you review this again? @ekristen

ekristen · 2026-03-20T14:05:05Z

Spinning a test up now and reviewing.

ekristen

HandleWait

You have code on several HandleWait where it is "delete request failed, will retry" -- the tool has retry logic, you adding additional retry logic breaks things unintentionally.

When an error is encountered, you need to return it, so the tool can track properly and retry.

gcp-nuke/libnuke will continue to retry all resources until all resources fail to remove 3 consecutive times, this failure count can be changed, but it's global, so depending on the loop cycle, a resource could be tried 3 or 30 times depending on how many other resources there are and how many times it has to iterate through the loop.

cloud-sql-instance

for some reason on a free tier, trying to disable deletion protection triggers an invalidate operation due to backup configuration and PITR

TRAC[0013] failed to disable deletion protection         error="googleapi: Error 400: Invalid request: The following Operation(s) are not allowed for Cloud SQL Free Trial Instance: [Pitr, Backup Configuration]., invalid" instance=free-trial-first-project

The error is returned, it does seem to try one more time

TRAC[0019] failed to disable deletion protection         error="googleapi: Error 400: Invalid request: The following Operation(s) are not allowed for Cloud SQL Free Trial Instance: [Pitr, Backup Configuration]., invalid" instance=free-trial-first-project```

but then gets stuck in a starting deletion op loop and never completes.

TRAC[0029] failed to start delete, will retry error="googleapi: Error 400: The instance is protected. Please disable the deletion protection and try again. To disable deletion protection, update the instance settings with deletionProtectionEnabled set to false., badRequest" instance=free-trial-first-project


The delete op is getting an error but not returning it properly. 

It should be fixed by my two inline comments.

ekristen · 2026-03-20T14:32:30Z

+			logrus.WithError(err).WithField("instance", *r.Name).Trace("failed to disable deletion protection")
+			return err
+		}
+		return nil


this shouldn't be here.

ekristen · 2026-03-20T14:37:58Z

 		if err != nil {
-			return err
+			logrus.WithError(err).WithField("instance", *r.Name).Trace("failed to start delete, will retry")
+			return liberror.ErrWaitResource(fmt.Sprintf("delete pending: %v", err))


This should just be return err if an err ever hits, it needs to return error. The internal retry logic of gcp-nuke and libnuke takes over and will retry the resource from the start again.

ekristen · 2026-03-20T14:40:34Z

+				return nil
+			}
+			logrus.WithError(err).WithField("cluster", *r.Name).Debug("delete request failed, will retry")
+			return liberror.ErrWaitResource(fmt.Sprintf("delete pending: %v", err))


cannot do this, need to return err, and let the internal systems retry, otherwise we get stuck in an infinite wait.

ekristen · 2026-03-20T14:43:50Z

 	}

 	if r.removeOp.GetError() != nil {
-		return fmt.Errorf("operation failed: %v", r.removeOp.GetError())


this was the right code, unless there is a bonefide error that could occurred that would async fix it self, we have to return an error here.

ekristen · 2026-03-20T14:45:17Z

+			},
+		})
+		if err != nil {
+			logrus.WithError(err).WithField("cluster", *r.Name).Trace("failed to disable deletion protection")


you should return err here.

ekristen · 2026-05-22T14:05:43Z

Following up on this as I'd like to get it merged, apologies for the delay, I'm trying to prioritize alerting to not miss conversations on this stuff going forward.

ekristen

Thanks for this — a lot of genuinely useful work here. The HandleWait refactors are a real improvement (the old cloud-sql-instance path had a synchronous busy poll loop), the soft-deleted object cleanup is correctly guarded, and the storage-bucket multi/dual-region detection via LocationType is a solid fix over the old hardcoded Location == "US". I verified the new APIs against the vendored deps (sqladmin.BackupsService.{ListBackups,DeleteBackup} exist; clusterpb/memorystorepb.DeletionProtectionEnabled are *bool, filestore's is a plain bool).

A few things I'd want addressed before merge — details inline:

Should fix (silent incorrectness on real projects):

CloudSQLBackup lister ignores pagination — only the first page of backups is ever deleted, and the run still reports success.
IAMServiceAccount.removeIAMBindings races on the project IAM policy under parallel deletion — concurrent SetIamPolicy calls collide on the etag, get Warn-logged, and the bindings survive anyway.

Worth considering:
3. Several new HandleWait retry paths (e.g. GKE) reset the op and retry on any failure, so a terminal error like PermissionDenied spins until the wait budget expires instead of surfacing.
4. DNS policy detach swallows the Update error, which makes a later Delete failure confusing.
5. The new IAM-binding removal needs resourcemanager.projects.{get,set}IamPolicy — worth documenting.

Process: the README/docs restructure, the v1.11.0 → v1.12.0 doc version bump, and the go.sum pruning are mixed into a functional PR. Splitting the docs restructure into its own PR would make this easier to review and the version bump may conflict with release tooling.

ekristen · 2026-06-17T15:58:11Z

+	}
+
+	parent := fmt.Sprintf("projects/%s", *opts.Project)
+	backups, err := l.svc.Backups.ListBackups(parent).Context(ctx).Do()


Pagination is not handled. ListBackupsResponse returns a NextPageToken, but this only reads the first page and iterates backups.Backups once. Automated backups accumulate daily, so on any real project every backup beyond the first page is silently left undeleted while the run still reports success.

Consider looping on the page token, e.g.:

parent := fmt.Sprintf("projects/%s", *opts.Project) call := l.svc.Backups.ListBackups(parent).Context(ctx) for { resp, err := call.Do() if err != nil { return nil, err } for _, backup := range resp.Backups { // ... existing handling ... } if resp.NextPageToken == "" { break } call = call.PageToken(resp.NextPageToken) }

(The existing CloudSQLInstance lister is also unpaginated, but it matters far more here since backup counts are unbounded.)

ekristen · 2026-06-17T15:58:11Z

 	})
 }

+func (r *IAMServiceAccount) removeIAMBindings(ctx context.Context) error {


Race on the shared project IAM policy. Each service account does GetIamPolicy → mutate → SetIamPolicy on the same project-level policy, and deletions run in parallel. Concurrent SetIamPolicy calls collide on the etag and fail with 409 ABORTED; that error is only Warn-logged a few lines up and deletion proceeds, so the orphaned serviceAccount:/deleted: bindings this is meant to clean up will frequently survive — defeating the purpose under the parallelism this PR added.

Options: retry on conflict (re-fetch policy + re-apply on ABORTED), serialize this step with a mutex on the lister, or do a single project-policy sweep once rather than per-SA.

ekristen · 2026-06-17T15:58:11Z

+	policy, err := r.svc.Policies.Get(*r.project, *r.Name).Do()
+	if err == nil {
+		policy.Networks = nil
+		_, _ = r.svc.Policies.Update(*r.project, *r.Name, policy).Do()


The detach Update error is discarded (_, _ =). If detaching the networks fails, the following Delete fails with a confusing "still in use" error and no breadcrumb as to why. At minimum Trace/Debug-log the update error so it's diagnosable.

ekristen · 2026-06-17T15:58:11Z

+			"cluster": *r.Name,
+			"error":   r.removeOp.GetError().String(),
+		}).Warn("delete operation failed, will retry")
+		r.removeOp = nil


Resetting r.removeOp = nil and returning ErrWaitResource retries on any operation failure. For a genuinely terminal error (PermissionDenied, InvalidArgument), this spins until libnuke's wait budget expires instead of surfacing the real cause. Consider treating non-retryable gRPC codes as terminal and returning the error directly. (Same pattern applies to the other new HandleWait retry paths.)

taivox added 14 commits January 30, 2026 21:21

fix: detach from networks before deletion to handle deleted VPCs

4483428

fix: add deletion of soft-deleted objects from buckets

6aa69d9

feat: add missing DisableDeletionProtection setting to resources

631ac65

fix: add function to process slow-to-delete resources first

9f0176c

feat: add disable-deletion-protection flag

82823a5

fix: update docs

05a2b33

feat: add cloudsql backup resource

94ec56e

fix: add parallel processing for disabling deletion protection

b661c40

fix: fix removal of cloudsql backups, multi-region buckets and iam pe…

8dce23e

…rmissions

fix: improve cloudsql backup deletion

5016f77

fix: improve cloudsql backup deletion

7e06097

fix: improve dns policy deletion

e4c6229

fix: update documentation

1807aab

fix: update documentation

058dfaa

fix: remove --disable-deletion-protection flag

d925b39

taivox changed the title ~~feat: add disable deletion protection flag, CloudSQLBackup resource and fixes~~ feat: add option to disable deletion protection, CloudSQLBackup resource and fixes Feb 2, 2026

fix: improve retry logic for buckets and gke cluster

2512bdc

ekristen requested changes Feb 19, 2026

View reviewed changes

taivox added 3 commits March 8, 2026 16:53

fix: revert unneeded changes

5fa8f43

fix: add requested fixes

c3401f0

fix: add filter to CloudSQLBackup

bceb207

taivox requested a review from ekristen March 8, 2026 15:20

ekristen reviewed Mar 20, 2026

View reviewed changes

ekristen reviewed Jun 17, 2026

View reviewed changes

Conversation

taivox commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Improvements

Uh oh!

ekristen commented Jan 31, 2026

Uh oh!

taivox commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ekristen commented Feb 1, 2026

Uh oh!

taivox commented Feb 2, 2026

Uh oh!

taivox commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ekristen commented Feb 18, 2026

Uh oh!

ekristen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

taivox commented Mar 8, 2026

Uh oh!

taivox commented Mar 20, 2026

Uh oh!

ekristen commented Mar 20, 2026

Uh oh!

ekristen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

HandleWait

cloud-sql-instance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ekristen commented May 22, 2026

Uh oh!

ekristen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

taivox commented Jan 31, 2026 •

edited

Loading

taivox commented Feb 1, 2026 •

edited

Loading

taivox commented Feb 18, 2026 •

edited

Loading

ekristen left a comment •

edited

Loading