fix: Check tracking annotation for being self-referencing

[jannfis]

Fixes #8683

When using the tracking method annotation or annotation+label, the app controller will now consider the resource properties in the annotation (name, namespace, group and kind) to determine whether the resource should be compared against the desired state. If the tracking annotation does not reference the resource it is applied to, the resource will neither affect the application's sync status nor be marked for pruning.

Signed-off-by: jannfis jann@mistrust.net

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• Optional. My organization is added to USERS.md.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).

[codecov]

Codecov Report

Merging #9791 (d3cd413) into master (2a3c692) will increase coverage by 0.01%.
The diff coverage is 78.57%.

@@            Coverage Diff             @@
##           master    #9791      +/-   ##
==========================================
+ Coverage   45.75%   45.77%   +0.01%     
==========================================
  Files         227      227              
  Lines       26961    26982      +21     
==========================================
+ Hits        12337    12352      +15     
- Misses      12943    12949       +6     
  Partials     1681     1681              

Impacted Files	Coverage Δ
util/argo/resource_tracking.go	70.11% <64.28%> (-2.97%)	⬇️
controller/state.go	74.21% <92.85%> (+0.56%)	⬆️
util/settings/settings.go	48.22% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a3c692...d3cd413. Read the comment docs.

[jessesuen]

This is great! One thing that still needs to be done (as a separate PR?), is that I believe that the UI still indicates "needs pruning" on resources which we have no intention of pruning. This is a source of confusion and is misleading to users.

On a similar note, I wonder if we can improve the behavior such that these type of resources don't even have a Sync status? e.g.:

resState.Status = ""

This might solve the confusion behavior in UI.

[jannfis]

@jessesuen Since resources with a non self-referencing tracking annotation will be handled similar to hooks or ignored resources, I believe this change already takes care of that. Such resources won't have a sync status, and thus will neither be marked as pruning.

In below example, there are two additional resources not stored in Git for the Application named guestbook:

• The guestbook-cm ConfigMap
• The guestbook-service Service

Both resources carry a copy of the tracking annotation from the guestbook-ui Service (i.e. argocd.argoproj.io/tracking-id: guestbook:/Service:guestbook/guestbook-ui), to emulate a resource including labels and annotations being copied from a managed resource:

Neither carries the sync status, but Argo CD will assess the resource's health if applicable - same as for resource hooks. I believe this may be the desired behavior that users want to see.

[jessesuen]

Thanks for the clarification. The behavior you described is the correct one. This might have been something that has been improved since last time I looked. Or perhaps IgnoreExtraneous is the one that I am thinking that has confusing behavior. The docs indicate IgnoreExtraneous resources as OutOfSync:

https://argo-cd.readthedocs.io/en/stable/user-guide/compare-options/#ignoring-resources-that-are-extraneous

[jessesuen]

LGTM!

[jannfis]

Actually, let me add an end-to-end test for this :)

[crenshaw-dev]

lgtm, not merging since you're gonna add an e2e test

[jannfis]

Hm, the end-to-end test I added passes locally when run as a single test (i.e. go test -run TestAnnotationTrackingExtraResources).

I suspect the failure is due to the bug reported in #9781 - I had similar issues when writing e2e tests that should leverage tracking annotation feature in another PR.

[crenshaw-dev]

@jannfis would you like to do without the e2e for now and add after #9781 is fixed?

[jannfis]

@crenshaw-dev I'm currently working on a fix for #9781, I'm close to finishing it. Probably that fix should go in first, then I'll rebase this PR so that the end-to-end runs fine.

[jannfis]

Fix PR for this: #9820

[crenshaw-dev]

Cherry-picked onto release-2.4 for 2.4.6.

[mzain]

Hey folks,
is this functionality documented somewhere on the argocd doc site. I couldn't find it.

[crenshaw-dev]

@mzain I think "no." We'd really appreciate a docs PR if you have time!

[mzain]

@crenshaw-dev raised a PR for docs update #16021