fix: overrides should not appear in the manifest cache key

[crenshaw-dev]

Currently, git-based parameter overrides appear in the manifest cache key.

That's a problem, because parameter overrides aren't known when the cache get occurs. Overrides shouldn't be in the cache key, because getting parameter overrides is a repo operation, and the entire point of the cache is to avoid repo operations.

So this change makes sure that overrides do not become part of the cache key. Changes to overrides will still bust the cache, because the changes will cause a change in the revision component of the key.

Case for cherry-picking: we have a 2.4 user who's spending a lot of money on git fetches. I think this is a fairly safe change, and it would help them out.

[crenshaw-dev]

The explicit type was redundant.

[codecov]

Codecov Report

Merging #9601 (6647e03) into master (0d0d09f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 6647e03 differs from pull request most recent head 33bb894. Consider uploading reports for the commit 33bb894 to get more accurate results

@@           Coverage Diff           @@
##           master    #9601   +/-   ##
=======================================
  Coverage   45.83%   45.83%           
=======================================
  Files         222      222           
  Lines       26405    26406    +1     
=======================================
+ Hits        12102    12103    +1     
  Misses      12652    12652           
  Partials     1651     1651           

Impacted Files	Coverage Δ
reposerver/repository/repository.go	61.42% <100.00%> (+0.04%)	⬆️

---

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 0d0d09f...33bb894. Read the comment docs.

[leoluz]

LGTM

[crenshaw-dev]

Cherry-picked onto release-2.4 for release with 2.4.3.

[arloliu]

I think the plugin environment modification should invalidate the cache too. (e.g. argocd app set <app_name> --plugin-env ...)
When we change the plugin environments on the fly, the current manifest doesn't be invalidated, and it will cause the app to stay in the synced state, the following app sync operation will not take the new environment variable.

[crenshaw-dev]

@arloliu I'm surprised that plugin env vars aren't breaking the cache. They're part of the ApplicationSource object. Is there an issue already created for the problem you're hitting?

[arloliu]

It's the problem I met these days.
I tested it with a simple CMP shell script and directory app.
the behavior is:

1. argocd app set <app_name> --plugin-env TEST_ENV=test
2. argocd app sync <app_name>

the CMP will receive the TEST_ENV when executing generate procedure in step 2, but the app will not sync with the new manifest even CMP produces a new manifest.(e.g. change replicas number)

And if we use the following steps.

1. argocd app set <app_name> --plugin-env TEST_ENV=test
2. argocd app get <app_name> --hard-refresh
3. argocd app sync <app_name>

The cache will be invalidated in step 2, and step 3 will use the manifest that CMP generated, but the problem is, step 2 also cleans the TEST_ENV before it invokes CMP generate.

BTW, the step 2 invokes CMP generate 3 times if the app's git repo had updated.

[crenshaw-dev]

Can you create an issue with those reprod. steps?

BTW, the step 2 invokes CMP generate 3 times if the app's git repo had updated.

Can you create an additional issue for this? :-)

[arloliu]

sure, I'll create a issue for it.
the cache is a bit weird, it remembers all histories of plugin env.
that's said:
if we never set TEST_ENV plugin env.

1. set TEST_ENV=a it invokes CMP generate
2. set TEST_ENV=b it invokes CMP generate
3. set TEST_ENV=a again, it will not invoke CMP generate
   But it should invoke CMP because `TEST_ENV value already changed.

[crenshaw-dev]

That's exceptionally strange. Definitely something we want to sort out.

[arloliu]

Thanks for your help, I fill the info. and created a issue here:
#9844