feat: transmit manifest-generate-path resources to the cmp-server for plugin-based applications

[jsolana]

Closes #17951

Transmit only the necessary information to the cmp-server to improve the performance. More info here and here

We are gonna transmit manifest-generate-paths annotation.

This value is not propagated right now . Because this annotation only affects manifests generation maybe make sense to modify ManifestRequest:

repository.proto

// ManifestRequest is a query for manifest generation.
message ManifestRequest {
    github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.Repository repo = 1;
    // revision, potentially un-resolved
    string revision = 2;
    bool noCache = 3;
    string appLabelKey = 4;
    // Name of the application for which the request is triggered
    string appName = 5;
    string namespace = 8;
    github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.ApplicationSource applicationSource = 10;
    repeated github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.Repository repos = 11;
    // Deprecated: use sidecar plugins instead.
    repeated github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.ConfigManagementPlugin plugins = 12;
    github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.KustomizeOptions kustomizeOptions = 13;
    string kubeVersion = 14;
    repeated string apiVersions = 15;
    // Request to verify the signature when generating the manifests (only for Git repositories)
    bool verifySignature = 16;
    repeated github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.RepoCreds helmRepoCreds = 17;
    bool noRevisionCache = 18;
    string trackingMethod = 19;
    map<string, bool> enabledSourceTypes = 20;
    github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.HelmOptions helmOptions = 21;
    bool hasMultipleSources = 22;
    map<string, github.com.argoproj.argo_cd.v2.pkg.apis.application.v1alpha1.RefTarget> refSources = 23;
    // This is used to surface "source not permitted" errors for Helm repositories
    repeated string projectSourceRepos = 24;
    // This is used to surface "source not permitted" errors for Helm repositories
    string projectName = 25;
    // argocd.argoproj.io/manifest-generate-paths annotation value of the Application to allow optimize which resources propagated to cmpserver
    string AnnotationManifestGeneratePaths = 26;
}

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.
• 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.
• Optional. My organization is added to USERS.md.
• Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

[bunnyshell]

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

• 🚀 /bns:deploy to deploy the environment

[bunnyshell]

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

• 🚀 /bns:deploy to deploy the environment

[jsolana]

Hi @todaywasawesome!
It is the same code as #18054 (previous the disaster 😅 )

[codecov]

Codecov Report

Attention: Patch coverage is 84.81013% with 24 lines in your changes missing coverage. Please review.

Project coverage is 55.87%. Comparing base (0710ff9) to head (eb9121a).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
cmd/argocd/commands/app.go	0.00%	12 Missing ⚠️
reposerver/repository/utils.go	85.36%	4 Missing and 2 partials ⚠️
reposerver/repository/repository.go	76.92%	2 Missing and 1 partial ⚠️
pkg/apis/application/v1alpha1/types.go	80.00%	1 Missing ⚠️
util/argo/argo.go	95.00%	0 Missing and 1 partial ⚠️
util/cmp/stream.go	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #19209      +/-   ##
==========================================
+ Coverage   55.81%   55.87%   +0.05%     
==========================================
  Files         320      321       +1     
  Lines       44431    44490      +59     
==========================================
+ Hits        24800    24859      +59     
+ Misses      17066    17060       -6     
- Partials     2565     2571       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jsolana]

cc: @crenshaw-dev , cause you were also related to this issue 😄

[crenshaw-dev]

Since this is breaking behavior, should we allow the CMP author to opt in via the CMP config?

[jsolana]

Since this is breaking behavior, should we allow the CMP author to opt in via the CMP config?

Sounds cool! We could define a "EnableUseGeneratePathsAnnotation". wdyt?

Which approach do you prefer?

• Define it as an environment variable?
  - Define it in the PluginConfigSpec? <-- In this case it requires to update config-magament-plugins.md Ignore this, cause PluginConfigSpec is the configuration of the cmp server side and we are talking about modify the Repo server behavior.

Then the better approach is to define a new environment variable for Repo Server and document it
Thanks!

[jsolana]

New environment variable added! Let me know your thoughts (Im really terrible naming things 😅 )
cc: @crenshaw-dev

[jsolana]

Any news?

[momilo]

It would be fabulous to have this merged, together with depth flag and sparse checkout - we have the whole team of engineers excitedly following these PRs, in hope that they resolve our major problems with argocd (as otherwise we will probably need to migrate to something more appropriate for monorepos).

Please do let me know if there is anything I could do to help progress these PR - happy to help. 🙏

[crenshaw-dev]

@momilo if you have engineering resources to put behind those features, there are some challenges to overcome: #16064 (comment)

[jsolana]

For visibility, after discuss with @crenshaw-dev, next steps:

• Improve log and doc about how root path is calculated (adding a note under multiple paths scenario here)
• Update GetApplicationRootPath implementation: app path should be fed into the common path algorithm. In other words, for this example:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: guestbook
  annotations:
    argocd.argoproj.io/manifest-generate-paths: ./manifests
spec:
  source:
    repoURL: https://github.com/argoproj/argocd-example-apps.git
    targetRevision: HEAD
    path: my-application
# ...

The common root path calculated must be my-application instead to resolve the common root path as my-application/manifests

[jsolana]

Hi mates! Here is the diagram describing how GetApplicationRootPath calculate the root path

graph TD
    A[GetApplicationRootPath] --> B{Is argocd.argoproj.io/manifest-generate-paths empty?}
    B -->|Yes, for backward compatibility| C[Return repoPath]
    B -->|No| D[Set common root path to appPath]
    D --> E[Iterate over each path in argocd.argoproj.io/manifest-generate-paths]
    E --> F{disjoint path and common root path in some point?}
    F -->|Yes, use deepest common path| G[Update common root path]
    F -->|No| I{non disjoint and path is higher than current common root path?}
    I -->|Yes, use path| G
    G -->H[return common root path]

Loading

[jsolana]

Let me know if everything is ok to move the summary of the whole conversation to the issue for better discoverability of the decisions taken

[jsolana]

Hi! @crenshaw-dev, do you think it is ok to merge it or miss anything? Thanks!

[jsoref]

This is not a chore. It's a feat.

[jsolana]

Let me know if it is ok now

Thanks!

[jsolana]

Thanks @crenshaw-dev really nice names now!! (among other tweaks) 🙇

[crenshaw-dev]

Thanks, @jsolana!