fix(cli): fix tracking annotation diff for non-namespaced resources (#9773)

[maxbrunet]

The UnstructuredToAppInstanceValue() function which SetAppInstance() passes the namespace to says the following:

argo-cd/util/argo/resource_tracking.go

Lines 116 to 120 in b2c5901

	// UnstructuredToAppInstanceValue will build the AppInstanceValue based
	// on the provided unstructured. The given namespace works as a default
	// value if the resource's namespace is not defined. It should be the
	// Application's target destination namespace.
	func UnstructuredToAppInstanceValue(un *unstructured.Unstructured, appName, namespace string) AppInstanceValue {

But when called from the CLI, this namespace was always an empty string:

argo-cd/cmd/argocd/commands/app.go

Line 1122 in b2c5901

err = resourceTracking.SetAppInstance(local, argoSettings.AppLabelKey, appName, "", argoappv1.TrackingMethod(argoSettings.GetTrackingMethod()))

I have tested argocd app diff with --local and --revision, and they do not report the annotation diff anymore. I have not been able to test --server-side-generate due to another bug (related to GRPC-Web in my setup: #12032), but I am confident it'll behave like the others. I have not tested with label as tracking method (help welcome).

Fixes #9773

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).
• The title of the PR conforms to the Toolchain Guide
• 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).
• My new feature complies with the feature status guidelines.
• I have added a brief description of why this PR is necessary and/or what this PR solves.

Please see Contribution FAQs if you have questions about your pull-request.

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.14 ⚠️

Comparison is base (40e26fc) 49.78% compared to head (772e7ca) 49.64%.

❗ Current head 772e7ca differs from pull request most recent head dcdd1f1. Consider uploading reports for the commit dcdd1f1 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #13924      +/-   ##
==========================================
- Coverage   49.78%   49.64%   -0.14%     
==========================================
  Files         261      258       -3     
  Lines       44672    44192     -480     
==========================================
- Hits        22238    21940     -298     
+ Misses      20249    20091     -158     
+ Partials     2185     2161      -24     

Impacted Files	Coverage Δ
cmd/argocd/commands/app.go	21.85% <0.00%> (ø)

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[crenshaw-dev]

I think the PR makes sense to me. Would you mind adding tests?

[maxbrunet]

Would you mind adding tests?

I can try, could you point me where it would be with an example I could base myself on, please?

[crenshaw-dev]

So, I think a unit test would be preferable. But our current examples of unit testing the CLI are very minimal and don't involve mocking a server (as you would have to do in this case). It might be tricky. It would probably involve modifying the New___Cmd function to accept an optional struct full of mock interfaces for server interactions. https://github.com/argoproj/argo-cd/blob/master/cmd/argocd/commands/version_test.go

Probably the easier way to go is to write an e2e test. There are already e2e tests which invoke the diff command, like this one:

argo-cd/test/e2e/app_management_test.go

Line 674 in 495d093

func TestAppWithSecrets(t *testing.T) {

[maxbrunet]

Thank you @crenshaw-dev for your guidance, I have added an E2E test, please let me know what you think.

[crenshaw-dev]

/cherry-pick release-2.7

[crenshaw-dev]

/cherry-pick release-2.8

[gcp-cherry-pick-bot]

Cherry-pick failed with Merge error 103a41992eac24101bcf3b0a2637379249d1e6e4 into temp-cherry-pick-674072-release-2.7