Skip to content

[Backport release-1.6] fix(api): preserve foreign HelmRelease metadata on Application update - #4577

Closed
github-actions[bot] wants to merge 1 commit into
release-1.6from
backport-2914-to-release-1.6
Closed

github-actions[bot] wants to merge 1 commit into
release-1.6from
backport-2914-to-release-1.6

Conversation

@github-actions

Copy link
Copy Markdown

Description

Backport of #2914 to release-1.6.

@github-actions

Copy link
Copy Markdown
Author

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-2914-to-release-1.6
git worktree add --checkout .worktree/backport-2914-to-release-1.6 backport-2914-to-release-1.6
cd .worktree/backport-2914-to-release-1.6
git reset --hard HEAD^
git cherry-pick -x a18cbc581c4829b657ebef0095f4ef0afc4cfc12 6d541c63eb658983175cda34d24c762fea249fd0 31948c88383e98314b2de3ff4d9329badcd69610
git push --force-with-lease

@coderabbitai

coderabbitai Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: af8367e7-05db-4953-b878-f89f831b85e3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Sep 30, 2026
## What this PR does

An application deleted with foreground or orphan propagation (`kubectl
delete --cascade=foreground`, or any client that sends that
`propagationPolicy`) could stay in deletion forever. Helm uninstalled
the release and helm-controller removed `finalizers.fluxcd.io`, but the
HelmRelease kept `foregroundDeletion` in `metadata.finalizers`, so the
HelmRelease and its Application stayed terminating. We hit this on
v1.6.4 with a Bucket and an OpenBAO application.

An Application and its HelmRelease share one UID, so the garbage
collector keeps a single graph node for both and may finalize it through
the Application endpoint. The API never returned finalizers on an
Application, and every update restored the HelmRelease finalizers from
the live object.
[`removeFinalizer`](https://github.com/kubernetes/kubernetes/blob/v1.33.1/pkg/controller/garbagecollector/operations.go#L85-L123)
reads the owner, does not find the finalizer there and returns as if it
had already been removed, so nothing ever took it off the HelmRelease.

This PR:

- returns `foregroundDeletion` and `orphan` in `metadata.finalizers` of
an Application, and only those two;
- carries an add or a remove of either of them from an Application
update to the HelmRelease, as a change against the finalizers the update
started from, so the result is the same when helm-controller or the
garbage collector changes the list during the conflict retry;
- keeps controller finalizers such as `finalizers.fluxcd.io` owned by
the live object, so `helm uninstall` still runs before the release goes
away;
- copies the deletion timestamp and grace period from the live object
into the rebuilt HelmRelease, so an update of a terminating release
stays valid.

`rest_garbage_collection_test.go` covers both finalizers with Helm
finishing first and with the garbage collector finishing first,
concurrent finalizer changes during the conflict retry, controller
finalizers surviving an update, the cached HelmRelease not being mutated
by the conversion, and an `UpdatedObject` that edits the object in
place. All five tests fail on the current `main` without this change.
`go test -race ./pkg/registry/...` passes. The tests use the
controller-runtime fake client, so this has not been run against a live
garbage collector with a patched cozystack-api yet.

The same bug is in release-1.6. There the Update path has no finalizer
carry-over at all, because #2914 is not backported yet, and its
automatic backport #4577 currently carries conflict markers. A 1.6
backport of this fix should go on top of a working backport of #2914.

### Screenshots

Not a UI change.

### Downstream repositories

Walked the trigger map against the diff: it only changes how
cozystack-api handles `metadata.finalizers` of Applications in
`pkg/registry/apps/application`. No package, values, schema, CRD,
`hack/` file or convention is touched.

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
fix(api): an application deleted with foreground or orphan propagation no longer leaves its HelmRelease stuck with the foregroundDeletion or orphan finalizer
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Application updates now preserve Kubernetes deletion behavior,
including foreground deletion and orphaning, even when concurrent
changes occur.
* Helm-managed protection metadata is retained during updates, and
deletion timing details remain intact.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@myasnikovdaniil

Copy link
Copy Markdown
Contributor

Superseded by #4919, it carries both #2914 and #3956.

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.

2 participants