feat(cli): Add Plugin Support to the Argo CD CLI

[nitishfy]

Ref: https://github.com/argoproj/argo-cd/pull/19624 Checklist:

• [x] 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.
• [x] 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.
• [x] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• [x] Does this PR require documentation updates?
• [ ] I've updated documentation as required by this PR.
• [x] 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.
• [x] My build is green (troubleshooting builds).
• [x] 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[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]]

:x: Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

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

[codecov[bot]]

Codecov Report

:x: Patch coverage is 73.33333% with 32 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 59.94%. Comparing base (9a738b2) to head (8fafefb). :warning: Report is 487 commits behind head on master.

Files with missing lines	Patch %	Lines
cmd/main.go	27.58%	19 Missing and 2 partials :warning:
cmd/argocd/commands/plugin.go	87.91%	9 Missing and 2 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #20074      +/-   ##
==========================================
+ Coverage   56.08%   59.94%   +3.85%     
==========================================
  Files         343      344       +1     
  Lines       57546    57656     +110     
==========================================
+ Hits        32275    34561    +2286     
+ Misses      22627    20335    -2292     
- Partials     2644     2760     +116     

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

[nitishfy]

Also, I think I'll have to restructure the entire tests for plugin_test.go once the change is accepted.

[nitishfy]

@leoluz I'm personally in the opinion of searching for a plugin, starting with the longest prefix. Why would you want to follow another approach?

[nitishfy]

@leoluz @blakepettersson I'd need your feedback on this PR. Having said that, there are a few things I'd like to mention:

1. We need to decide if we would like to follow the longest executable search performed or not.
2. I can see a lot of code optimization and few testcases can be added. I'm thinking if it would be okay to add them as some separate PR.
3. I'll start working on writing the docs as soon as I get confirmation about if we'd like to have longest executable search operation in code or not.

[blakepettersson]

I can see a lot of code optimization and few testcases can be added. I'm thinking if it would be okay to add them as some separate PR.

What kind of code optimizations and test cases can be done? Why can't they be done in this PR?

[blakepettersson]

Prior art (not sure I agree with this but it at least gives us a common base to discuss) https://kubernetes.io/docs/tasks/extend-kubectl/kubectl-plugins/#flags-and-argument-handling

[nitishfy]

The exact match logic is implemented and i'm working on the docs right now.

[leoluz]

Prior art (not sure I agree with this but it at least gives us a common base to discuss) https://kubernetes.io/docs/tasks/extend-kubectl/kubectl-plugins/#flags-and-argument-handling

@blakepettersson @christianh814 I discussed with Nitish yesterday about it and proposed a simpler approach by searching the plugin by the exact match only. I am not sure if there are strong reasons why kubectl opted to go that route and if that applies to us.

[nitishfy]

Update:

1. All the test cases have been added (including the one to check for relative path)
2. Docs for plugins are added completely.

I'll try to simplify the comments in the codebase but we are all good to move with this PR now.

[nitishfy]

Not to be a Debbie Downer, but we will need some E2E tests as well 😄

I think a set of tests doing what the documentation says would be good. Basically to verify that the documentation is correct, and also to ensure that we generate the correct exit codes.

@blakepettersson The unit tests cover all the scenarios that would fit in an e2e test. If you take a look at the unit tests, we are not testing a specific function, instead the whole test case goes through testing multiple functions in a flow that solve the purpose of an e2e test. Writing an e2e test would only be a redundant task.

[blakepettersson]

@blakepettersson The unit tests cover all the scenarios that would fit in an e2e test. If you take a look at the unit tests, we are not testing a specific function, instead the whole test case goes through testing multiple functions in a flow that solve the purpose of an e2e test. Writing an e2e test would only be a redundant task.

That's all good, but we still haven't:

• verified that the argocd cli itself can be used for plugins
• verified that what the documentation says is correct, from a user perspective (basically the first point, reiterated)
• verified that flags can be passed through / parsed correctly by the plugin handler
• verified that the correct exit codes have been generated, depending on if the plugin handler has been called or not. This is important to verify since people depend on the argo cd cli for scripting tasks, and if we don't generate the correct exit codes, users will have a bad day

... aka all the things that an e2e test would cover. We also want to test the actual functionality without inadvertently depending on internal state of said functions - which is also the point of e2e testing.

[blakepettersson]

@nitishfy my personal opinion is still that we should still have some E2E tests for this (i.e. running tests that exercises the actual CLI).

[nitishfy]

@nitishfy my personal opinion is still that we should still have some E2E tests for this (i.e. running tests that exercises the actual CLI).

If you look at the unit tests that have been written, we're not testing a specific function actually. All it does is callout the similar logic that would go inside an E2E test. I just believe that's redundant testing for the similar thing again.

[blakepettersson]

@nitishfy

I just believe that's redundant testing for the similar thing again.

Well, I disagree with that. We would at the very least still need to check that the CLI parses flags correctly from the command line.

[nitishfy]

All the E2E tests have been added now.

[nitishfy]

cc @agaudreault

[nitishfy]

Update: There's a little issue related to flag parsing that needs to be worked upon. Rn, all the global level flags won't be parsed when you are running your plugin. This is happening because cobra internally doesn't parse the flags when a command is not found. This also means that if a user has set its own flags in a binary that matches with Argo CD global flags, the default value of the Argo CD global flags will be used no matter what (why? because flag parsing never happens by cobra for an unknown command). For normal Argo CD commands, the flag parsing and everything else remains same as this is how it used to be.

What should be a better approach to handle this? Should we add this in a limitation section and tell users about not to use duplicate flags as argocd global flags? Do we even want a user to set it's own flags that overrides argocd global flag values?

> dist/argocd non-existent # default logging format is json for v3.0                   
{"level":"debug","msg":"command does not exist, looking for a plugin...","time":"2025-03-23T15:43:03+05:30"}
{"level":"warning","msg":"error looking for plugin 'argocd-non-existent': exec: \"argocd-non-existent\": executable file not found in $PATH","time":"2025-03-23T15:43:03+05:30"}
{"level":"error","msg":"Error: unknown command \"non-existent\" for \"argocd\"\nRun 'argocd --help' for usage.\n","time":"2025-03-23T15:43:03+05:30"}
              
> dist/argocd non-existent --logformat=text # logformat flag is never parsed
{"level":"debug","msg":"command does not exist, looking for a plugin...","time":"2025-03-23T15:43:06+05:30"}
{"level":"warning","msg":"error looking for plugin 'argocd-non-existent': exec: \"argocd-non-existent\": executable file not found in $PATH","time":"2025-03-23T15:43:06+05:30"}
{"level":"error","msg":"Error: unknown command \"non-existent\" for \"argocd\"\nRun 'argocd --help' for usage.\n","time":"2025-03-23T15:43:06+05:30"}

> dist/argocd login localhost:8080 --logformat=text # this works because login is an existing argocd command
FATA[0000] dial tcp 127.0.0.1:8080: connect: connection refused 

[blakepettersson]

@nitishfy @leoluz

Could we not have a dummy (no-op) command for the error path? We invoke the dummy command that will parse the flags, specifically the flags we would like to still keep (we can use the command.ParseFlags function for this purpose), and then we carry on with pluginHandler.HandleCommandExecutionError as before and pass in whatever flags we want there?

[nitishfy]

1. The plugin doesn't have to authenticate with Argo CD API. Plugin authors can directly invoke the Argo CD API and the current context will be used.

@leoluz I think the plugin is working as expected, I just tested it for this scenario:

 > dist/argocd cluster-list    
Fetching the list of clusters connected to Argo CD...
FATA[0000] rpc error: code = Unauthenticated desc = invalid session: Token is expired 
Error: Failed to fetch cluster list. Ensure you are logged in to the Argo CD CLI.
               
> dist/argocd login localhost:8080 
WARNING: server certificate had error: tls: failed to verify certificate: x509: certificate signed by unknown authority. Proceed insecurely (y/n)? y
Username: admin
Password: 
'admin:login' logged in successfully
Context 'localhost:8080' updated
               
>  dist/argocd cluster-list        
Fetching the list of clusters connected to Argo CD...
SERVER                          NAME        VERSION  STATUS      MESSAGE  PROJECT
https://kubernetes.default.svc  in-cluster  1.31     Successful           
Cluster list retrieved successfully.

argocd-cluster-list content:

 cat /usr/local/bin/argocd-cluster-list 
#!/bin/bash

# Check if the argocd CLI is installed
if ! command -v argocd &> /dev/null; then
    echo "Error: Argo CD CLI (argocd) is not installed. Please install it first."
    exit 1
fi

# Run the `argocd cluster list` command
echo "Fetching the list of clusters connected to Argo CD..."
argocd cluster list

# Check if the command was successful
if [ $? -eq 0 ]; then
    echo "Cluster list retrieved successfully."
else
    echo "Error: Failed to fetch cluster list. Ensure you are logged in to the Argo CD CLI."
fi

This means once you've logged in to the server using the CLI, the current context will be used by the Plugin unless you change the context explicitly.

@agaudreault FYI

>  dist/argocd non-existent                 
WARN[0000] error looking for plugin 'argocd-non-existent': exec: "argocd-non-existent": executable file not found in $PATH 
Error: unknown command "non-existent" for "argocd"
Run 'argocd --help' for usage.

I've explicitly set the log format to text for plugin than json because the text looks actually better than json due to which you see the log level warning appearing as text and not json.

[nitishfy]

Ping @agaudreault