fix: panic in offline linter + handling stdin

The offline linter is working well in ideal scenarios, but given an non-existent file, it panics (I inverted the `info.IsDir()` statement and the one checking for errors)
There's a disconnect between the filepath.Walk in the linter and the one building the offline client which means that there were other issues (such as stdin not working)

To fix that once and for all, I extracted the walk func to a common path (file.WalkManifests); that way, we are sure that changes are always reflected in both locations

I also added some tests for non-existent files as well as offline linting from stdin

cmd/argo/commands/client/conn_test.go

@@ -1,9 +1,11 @@
 package client
 
 import (
+	"context"
 	"os"
 	"testing"
 
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -18,3 +20,29 @@ func TestNamespace(t *testing.T) {
 	defer func() { _ = os.Unsetenv("ARGO_NAMESPACE") }()
 	assert.Equal(t, "my-ns", Namespace())
 }
+
+func TestCreateOfflineClient(t *testing.T) {
+	t.Run("creating an offline client with no files should not fail", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		Offline = true
+		OfflineFiles = []string{}
+		NewAPIClient(context.TODO())
+
+		assert.False(t, fatal, "should have exited")
+	})
+
+	t.Run("creating an offline client with a non-existing file should fail", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		Offline = true
+		OfflineFiles = []string{"non-existing-file"}
+		NewAPIClient(context.TODO())
+
+		assert.True(t, fatal, "should have exited")
+	})
+}

cmd/argo/commands/lint_test.go

@@ -154,4 +154,19 @@ spec:
 
 		assert.False(t, fatal, "should not have exited")
 	})
+
+	t.Run("linting one file from stdin", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		oldStdin := os.Stdin
+		defer func() { os.Stdin = oldStdin }() // Restore original Stdin
+		os.Stdin, err = os.Open(clusterWftmplPath)
+		require.NoError(t, err)
+
+		runLint(context.Background(), []string{workflowPath, wftmplPath, "-"}, true, nil, "pretty", true)
+
+		assert.False(t, fatal, "should not have exited")
+	})
 }

cmd/argo/lint/lint.go

@@ -5,10 +5,9 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"os"
-	"path/filepath"
 	"strings"
 
+	fileutil "github.com/argoproj/argo-workflows/v3/util/file"
 	"github.com/argoproj/pkg/errors"
 	log "github.com/sirupsen/logrus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -121,41 +120,11 @@ func Lint(ctx context.Context, opts *LintOptions) (*LintResults, error) {
 	}
 
 	for _, file := range opts.Files {
-		err := filepath.Walk(file, func(path string, info os.FileInfo, err error) error {
-			var r io.Reader
-			switch {
-			case path == "-":
-				path = "stdin"
-				r = os.Stdin
-			case err != nil:
-				return err
-			case strings.HasPrefix(path, "/dev/") || lintExt[filepath.Ext(path)]:
-				f, err := os.Open(filepath.Clean(path))
-				if err != nil {
-					return err
-				}
-				defer func() {
-					if err := f.Close(); err != nil {
-						log.Fatalf("Error closing file[%s]: %v", path, err)
-					}
-				}()
-				r = f
-			case info.IsDir():
-				return nil // skip
-			default:
-				log.Debugf("ignoring file with unknown extension: %s", path)
-				return nil
-			}
-
-			data, err := ioutil.ReadAll(r)
-			if err != nil {
-				return err
-			}
-
+		err := fileutil.WalkManifests(file, func(path string, data []byte) error {
 			res := lintData(ctx, path, data, opts)
 			results.Results = append(results.Results, res)
 
-			_, err = w.Write([]byte(results.fmtr.Format(res)))
+			_, err := w.Write([]byte(results.fmtr.Format(res)))
 			return err
 		})
 		if err != nil {

pkg/apiclient/offline-client.go

@@ -3,8 +3,6 @@ package apiclient
 import (
 	"context"
 	"fmt"
-	"os"
-	"path/filepath"
 
 	"github.com/argoproj/argo-workflows/v3/pkg/apiclient/clusterworkflowtemplate"
 	"github.com/argoproj/argo-workflows/v3/pkg/apiclient/cronworkflow"
@@ -13,6 +11,7 @@ import (
 	workflowarchivepkg "github.com/argoproj/argo-workflows/v3/pkg/apiclient/workflowarchive"
 	"github.com/argoproj/argo-workflows/v3/pkg/apiclient/workflowtemplate"
 	wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
+	"github.com/argoproj/argo-workflows/v3/util/file"
 	"github.com/argoproj/argo-workflows/v3/workflow/templateresolution"
 
 	"sigs.k8s.io/yaml"
@@ -50,18 +49,7 @@ func newOfflineClient(paths []string) (context.Context, Client, error) {
 	workflowTemplateGetters := offlineWorkflowTemplateGetterMap{}
 
 	for _, basePath := range paths {
-		err := filepath.Walk(basePath, func(path string, info os.FileInfo, err error) error {
-			if info.IsDir() {
-				return nil
-			}
-			if err != nil {
-				return err
-			}
-
-			bytes, err := os.ReadFile(path)
-			if err != nil {
-				return fmt.Errorf("failed to read file %s: %w", path, err)
-			}
+		err := file.WalkManifests(basePath, func(path string, bytes []byte) error {
 			var generic map[string]interface{}
 			if err := yaml.Unmarshal(bytes, &generic); err != nil {
 				return fmt.Errorf("failed to parse YAML from file %s: %w", path, err)

util/file/fileutil.go

@@ -8,14 +8,24 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/klauspost/pgzip"
+	"github.com/sirupsen/logrus"
 	log "github.com/sirupsen/logrus"
 	"k8s.io/utils/env"
 )
 
-var gzipImpl = env.GetString(GZipImplEnvVarKey, PGZIP)
+var (
+	gzipImpl    = env.GetString(GZipImplEnvVarKey, PGZIP)
+	manifestExt = map[string]bool{
+		".yaml": true,
+		".yml":  true,
+		".json": true,
+	}
+)
 
 const (
 	GZipImplEnvVarKey = "GZIP_IMPLEMENTATION"
@@ -119,3 +129,40 @@ func DecompressContent(content []byte) ([]byte, error) {
 	defer close(gzipReader)
 	return ioutil.ReadAll(gzipReader)
 }
+
+// WalkManifests is based on filepath.Walk but will only walk through Kubernetes manifests
+func WalkManifests(root string, fn func(path string, data []byte) error) error {
+	return filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+		var r io.Reader
+		switch {
+		case path == "-":
+			path = "stdin"
+			r = os.Stdin
+		case err != nil:
+			return err
+		case strings.HasPrefix(path, "/dev/") || manifestExt[filepath.Ext(path)]:
+			f, err := os.Open(filepath.Clean(path))
+			if err != nil {
+				return err
+			}
+			defer func() {
+				if err := f.Close(); err != nil {
+					log.Fatalf("Error closing file[%s]: %v", path, err)
+				}
+			}()
+			r = f
+		case info.IsDir():
+			return nil // skip
+		default:
+			logrus.Debugf("ignoring file with unknown extension: %s", path)
+			return nil
+		}
+
+		bytes, err := io.ReadAll(r)
+		if err != nil {
+			return err
+		}
+
+		return fn(path, bytes)
+	})
+}