WIP: Add Plugin Support to the Argo CD CLI #20074
Draft
+180
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: nitishfy <justnitish06@gmail.com>
✅ Preview Environment deployed on Bunnyshell
See: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
✅ Preview Environment created on Bunnyshell but will not be auto-deployedSee: Environment Details Available commands (reply to this comment):
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #20074 +/- ##
==========================================
- Coverage 55.82% 55.72% -0.10%
==========================================
Files 320 320
Lines 44431 44514 +83
==========================================
+ Hits 24805 24807 +2
- Misses 17060 17143 +83
+ Partials 2566 2564 -2 ☔ View full report in Codecov by Sentry. |
leoluz
requested changes
Sep 24, 2024
Comment on lines
+27
to
+86
| type ArgoCDCLIOptions struct { | ||
| PluginHandler PluginHandler | ||
| Arguments []string | ||
| } | ||
|
|
||
| // PluginHandler parses command line arguments | ||
| // and performs executable filename lookups to search | ||
| // for valid plugin files, and execute found plugins. | ||
| type PluginHandler interface { | ||
| // LookForPlugin will iterate over a list of given prefixes | ||
| // in order to recognize valid plugin filenames. | ||
| // The first filepath to match a prefix is returned. | ||
| LookForPlugin(filename string) (string, bool) | ||
| // ExecutePlugin receives an executable's filepath, a slice | ||
| // of arguments, and a slice of environment variables | ||
| // to relay to the executable. | ||
| ExecutePlugin(executablePath string, cmdArgs, environment []string) error | ||
| } | ||
|
|
||
| // DefaultPluginHandler implements the PluginHandler interface | ||
| type DefaultPluginHandler struct { | ||
| ValidPrefixes []string | ||
| } | ||
|
|
||
| func NewDefaultPluginHandler(validPrefixes []string) *DefaultPluginHandler { | ||
| return &DefaultPluginHandler{ | ||
| ValidPrefixes: validPrefixes, | ||
| } | ||
| } | ||
|
|
||
| // LookForPlugin implements PluginHandler | ||
| func (h *DefaultPluginHandler) LookForPlugin(filename string) (string, bool) { | ||
| for _, prefix := range h.ValidPrefixes { | ||
| path, err := exec.LookPath(fmt.Sprintf("%s-%s", prefix, filename)) | ||
| if shouldSkipOnLookPathErr(err) || len(path) == 0 { | ||
| continue | ||
| } | ||
| return path, true | ||
| } | ||
| return "", false | ||
| } | ||
|
|
||
| // ExecutePlugin implements PluginHandler | ||
| func (h *DefaultPluginHandler) ExecutePlugin(executablePath string, cmdArgs, environment []string) error { | ||
| // Windows does not support exec syscall. | ||
| if runtime.GOOS == "windows" { | ||
| cmd := Command(executablePath, cmdArgs...) | ||
| cmd.Stdout = os.Stdout | ||
| cmd.Stderr = os.Stderr | ||
| cmd.Stdin = os.Stdin | ||
| cmd.Env = environment | ||
| err := cmd.Run() | ||
| if err == nil { | ||
| os.Exit(0) | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| return syscall.Exec(executablePath, append([]string{executablePath}, cmdArgs...), environment) | ||
| } |
There was a problem hiding this comment.
Please extract this code in a dedicated file with proper unit tests.
Suggestion: util/cli/plugin.go
| return syscall.Exec(executablePath, append([]string{executablePath}, cmdArgs...), environment) | ||
| } | ||
|
|
||
| func Command(name string, arg ...string) *exec.Cmd { |
There was a problem hiding this comment.
Please add docs to this command explaining how it works.
leoluz
requested changes
Sep 24, 2024
|
|
||
| // ExecutePlugin implements PluginHandler | ||
| func (h *DefaultPluginHandler) ExecutePlugin(executablePath string, cmdArgs, environment []string) error { | ||
| // Windows does not support exec syscall. |
There was a problem hiding this comment.
Windows doesn't support syscall but all OSs support cmd.Run().
Why not having the plugin execution handled by a Command in all OSs and simplifying this code?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref: #19624
Checklist: