chore(cmp): refactor and test plugin discovery

[crenshaw-dev]

Plugin discovery logic is complicated and has resulted in difficult to diagnose bugs.

This PR makes a few changes:

1. Deprecate RepositoryResponse.IsDiscoveryEnabled field. No code actually uses that field anymore since we do a pre-flight CMP request to determine whether it has discovery enabled.
2. Break cmpSupports into namedCMPSupports and unnamedCMPSupports. Reading cmpSupports was difficult, because the namedPlugin parameter implied more than the name suggested. Specifically, when namedPlugin == false, we could assume that the app spec did not contain a plugin.name. By using two different functions with two different names, we make it very clear that one is for use when the plugin name is specified, and the other is used when the plugin name is not specified.
3. Make the CMP client constructor mock-able. It requires some code overhead, but discovery is so deeply entangled with CMP server API calls that I think it's worth it.
4. Test the behavior of both unnamedCMPSupports and namedCMPSupports. I'd love to test all the way up to DetectConfigManagementPlugin, but I think that would require more mock logic than I'm comfortable with.
5. Use securejoin instead of our internal path traversal detection logic - it's more robust.

[bunnyshell[bot]]

:exclamation: Preview Environment undeploy from Bunnyshell failed

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

• :rocket: /bns:deploy to redeploy the environment
• :x: /bns:delete to try again to remove the environment

[bunnyshell[bot]]

:white_check_mark: Preview Environment created on Bunnyshell but will not be auto-deployed

See: Environment Details

Available commands (reply to this comment):

• :rocket: /bns:deploy to deploy the environment

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 79.48718% with 16 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@b2091e3). Learn more about missing BASE report. Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
util/app/discovery/discovery.go	76.47%	11 Missing and 5 partials :warning:

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #20217   +/-   ##
=========================================
  Coverage          ?   55.98%           
=========================================
  Files             ?      322           
  Lines             ?    44569           
  Branches          ?        0           
=========================================
  Hits              ?    24951           
  Misses            ?    17047           
  Partials          ?     2571           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[crenshaw-dev]

@CodiumAI-Agent /review

[QodoAI-Agent]

PR Reviewer Guide 🔍

(Review updated until commit https://github.com/argoproj/argo-cd/commit/cf553e6feabb91a4fba8597334d764fd0ec61ab4)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Refactoring The refactoring of the DetectConfigManagementPlugin function to improve modularity and testability is significant. Ensure that the new structure does not introduce any regressions or misunderstandings about how plugins are detected and managed. Logic Change The changes in how discovery is handled (removing isDiscoveryEnabled and using isSupported only) could affect existing functionality. Verify that these changes align with the intended use cases and do not negatively impact existing workflows.

[pradithya]

LGTM

[QodoAI-Agent]

Persistent review updated to latest commit https://github.com/argoproj/argo-cd/commit/cf553e6feabb91a4fba8597334d764fd0ec61ab4