From 1e5a86c4547f7391eebb578a5e9485146de701e6 Mon Sep 17 00:00:00 2001
From: nitishfy <justnitish06@gmail.com>
Date: Tue, 26 Nov 2024 21:40:39 +0530
Subject: [PATCH] handle look for plugin errors

Signed-off-by: nitishfy <justnitish06@gmail.com>
---
 cmd/argocd/commands/plugin.go      | 36 +++++++++++++-------
 cmd/argocd/commands/plugin_test.go | 53 +++++++++++++++++++++++++++---
 cmd/argocd/commands/root.go        | 13 ++++----
 cmd/main.go                        | 10 +++---
 4 files changed, 86 insertions(+), 26 deletions(-)

diff --git a/cmd/argocd/commands/plugin.go b/cmd/argocd/commands/plugin.go
index 2693225928c61..dd37c7a57219e 100644
--- a/cmd/argocd/commands/plugin.go
+++ b/cmd/argocd/commands/plugin.go
@@ -3,11 +3,12 @@ package commands
 import (
 	"errors"
 	"fmt"
-	log "github.com/sirupsen/logrus"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
+
+	log "github.com/sirupsen/logrus"
 )
 
 type ArgoCDCLIOptions struct {
@@ -92,22 +93,35 @@ func HandlePluginCommand(pluginHandler PluginHandler, cmdArgs []string, minArgs
 // LookForPlugin implements PluginHandler. searches for an executable plugin with a given filename
 // and valid prefixes. If the plugin is not found (ErrNotFound), it continues to the next prefix.
 // Unexpected errors (e.g., permission issues) are logged for debugging but do not stop the search.
+// This doesn't care about the plugin execution errors since those errors are handled separately by
+// the execute function.
 func (h *DefaultPluginHandler) LookForPlugin(filename string) (string, bool) {
 	for _, prefix := range h.ValidPrefixes {
-		path, err := exec.LookPath(fmt.Sprintf("%s-%s", prefix, filename))
-		if err == nil {
-			return path, true
-		}
-		log.Warnf("Unexpected error while looking for plugin %s: %v", filename, err)
+		pluginName := fmt.Sprintf("%s-%s", prefix, filename) // Combine prefix and filename
+		path, err := exec.LookPath(pluginName)
+		if err != nil {
+			if errors.Is(err, exec.ErrNotFound) {
+				log.Warnf("Plugin %s not found or not executable. Ensure it exists in the PATH and has executable permissions.", pluginName)
+				continue
+			}
 
-		// Handle specific errors
-		if errors.Is(err, exec.ErrNotFound) {
-			continue // Expected case: Plugin not found, try the next prefix
+			if errors.Is(err, exec.ErrDot) {
+				log.Warnf("Plugin %s cannot be executed because relative paths (e.g., './%s') are not allowed. Ensure the plugin is in a directory listed in the PATH.", pluginName, pluginName)
+				continue
+			}
+
+			if errors.Is(err, exec.ErrWaitDelay) {
+				log.Warnf("Execution of plugin %s is delayed. The system is waiting before launching the process. Please try again later.", pluginName)
+				continue
+			}
+
+			log.Errorf("Unexpected error while looking for plugin %s: %v", pluginName, err)
+			continue
 		}
 
-		// Log unexpected errors for debugging purposes
-		log.Warnf("Unexpected error while looking for plugin %s: %v", filename, err)
+		return path, true
 	}
+
 	return "", false
 }
 
diff --git a/cmd/argocd/commands/plugin_test.go b/cmd/argocd/commands/plugin_test.go
index acf1dc886b85d..bef61f55da523 100644
--- a/cmd/argocd/commands/plugin_test.go
+++ b/cmd/argocd/commands/plugin_test.go
@@ -3,11 +3,14 @@ package commands
 import (
 	"bytes"
 	"fmt"
+	"os"
+	"testing"
+
 	argocdclient "github.com/argoproj/argo-cd/v2/pkg/apiclient"
+
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"os"
-	"testing"
 )
 
 type testPluginHandler struct {
@@ -120,6 +123,12 @@ func Test_ArgoCD_Plugin_Successful_Execution(t *testing.T) {
 
 // Test_CommandIsNeitherNormalCommandNorExistsAsPlugin checks when a command is neither a normal Argo CD CLI command nor Plugin
 func Test_CommandIsNeitherNormalCommandNorExistsAsPlugin(t *testing.T) {
+	buf := new(bytes.Buffer)
+	logrus.SetOutput(buf)
+	defer func() {
+		logrus.SetOutput(os.Stderr)
+	}()
+
 	cmd := NewCommand()
 	cmd.SilenceErrors = true
 	cmd.SilenceUsage = true
@@ -129,6 +138,7 @@ func Test_CommandIsNeitherNormalCommandNorExistsAsPlugin(t *testing.T) {
 		validPrefixes:      []string{"argocd"},
 		executedPluginPath: "",
 	}
+
 	o := ArgoCDCLIOptions{
 		PluginHandler: pluginsHandler,
 		Arguments:     []string{"argocd", "nonexistent"},
@@ -138,6 +148,41 @@ func Test_CommandIsNeitherNormalCommandNorExistsAsPlugin(t *testing.T) {
 	err := cmd.Execute()
 	require.Error(t, err, "unknown command \"nonexistent\" for \"argocd\"")
 
-	pluginError := HandleCommandExecutionError(err, true, o)
-	require.Equal(t, pluginError, err)
+	pluginErr := HandleCommandExecutionError(err, true, o)
+	require.Equal(t, pluginErr, err)
+
+	// check for error logs
+	logOutput := buf.String()
+	assert.Contains(t, logOutput, "Error: unknown command \\\"nonexistent\\\" for \\\"argocd\\\"\\nRun 'argocd --help' for usage.\\n")
+}
+
+func Test_CommandWithInvalidArguments(t *testing.T) {
+	buf := new(bytes.Buffer)
+	logrus.SetOutput(buf)
+	defer func() {
+		logrus.SetOutput(os.Stderr)
+	}()
+
+	cmd := NewCommand()
+	cmd.SilenceErrors = true
+	cmd.SilenceUsage = true
+
+	o := ArgoCDCLIOptions{
+		Arguments: []string{"argocd", "--invalid-flag"},
+	}
+	cmd.SetArgs(o.Arguments[1:])
+
+	err := cmd.Execute()
+	require.Error(t, err, "unknown flag: --invalid-flag")
+
+	pluginErr := HandleCommandExecutionError(err, true, o)
+	require.Equal(t, pluginErr, err)
+
+	// Ensure appropriate error logs
+	logOutput := buf.String()
+	assert.Contains(t, logOutput, "Error: unknown flag: --invalid-flag")
+}
+
+// TODO
+func Test_LookForPluginError(t *testing.T) {
 }
diff --git a/cmd/argocd/commands/root.go b/cmd/argocd/commands/root.go
index 0f1e39c7bed9c..428def28e38b6 100644
--- a/cmd/argocd/commands/root.go
+++ b/cmd/argocd/commands/root.go
@@ -2,9 +2,10 @@ package commands
 
 import (
 	"fmt"
-	"os"
 	"strings"
 
+	log "github.com/sirupsen/logrus"
+
 	"github.com/spf13/cobra"
 	"k8s.io/client-go/tools/clientcmd"
 
@@ -101,9 +102,9 @@ func NewCommand() *cobra.Command {
 // It handles both standard Argo CD commands and plugin commands. We don't require to return
 // error but we are doing it to cover various test scenarios.
 func HandleCommandExecutionError(err error, isArgocdCLI bool, o ArgoCDCLIOptions) error {
+	cli.SetLogFormat("text")
+	cli.SetLogLevel("info")
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "Debug: err type = %T\n", err)
-		fmt.Fprintf(os.Stderr, "Debug: err value = %v\n", err)
 		// If it's an unknown command error, attempt to handle it as a plugin.
 		// Unfortunately, cobra doesn't handle this error, so we need to assume
 		// that error consists of substring "unknown command".
@@ -116,17 +117,17 @@ func HandleCommandExecutionError(err error, isArgocdCLI bool, o ArgoCDCLIOptions
 			// This means the command is neither a normal Argo CD Command nor a plugin.
 			if pluginErr == nil {
 				if PluginPath == "" {
-					fmt.Fprintf(os.Stderr, "Error: %v\nRun 'argocd --help' for usage.\n", err)
+					log.Errorf("Error: %v\nRun 'argocd --help' for usage.\n", err)
 					return err
 				}
 			} else {
 				// If plugin handling fails, report the plugin error and exit
-				fmt.Fprintf(os.Stderr, "Error: %v\n", pluginErr)
+				log.Errorf("Error: %v\n", pluginErr)
 				return pluginErr
 			}
 		} else {
 			// If it's any other error (not an unknown command), report it directly and exit
-			fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+			log.Errorf("Error: %v\n", err)
 			return err
 		}
 	}
diff --git a/cmd/main.go b/cmd/main.go
index 868bf8284264a..395efccaddc00 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -74,11 +74,11 @@ func main() {
 	// silence errors and usages since we'll be printing them manually.
 	// This is because if we execute a plugin, the initial
 	// errors and usage are always going to get printed that we don't want.
-	//if isArgocdCLI {
-	//	command.SilenceErrors = true
-	//	command.SilenceUsage = true
-	//}
+	if isArgocdCLI {
+		command.SilenceErrors = true
+		command.SilenceUsage = true
+	}
 
 	err := command.Execute()
-	err = cli.HandleCommandExecutionError(err, isArgocdCLI, o)
+	_ = cli.HandleCommandExecutionError(err, isArgocdCLI, o)
 }