fix: Resolve issues with offline linter + add tests

argoproj#10059 was tested in very specific cases it seems because a couple things are wrong:
- Linting directories does not work anymore
- Resolving references in a namespace which doesn't show up in the given args, panics

In this PR, I fix those issues but I also add functional tests that checks that all of these cases work from the cmd level
I did not add to the `argo/lint/lint_test.go` tests and instead put the tests earlier down the line because the current tests do not cover the creation and configuration of the offline client

Signed-off-by: Julien Duchesne <julien.duchesne@grafana.com>

cmd/argo/commands/lint.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"strings"
@@ -12,6 +13,8 @@ import (
 	wf "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow"
 )
 
+var allKinds = []string{wf.WorkflowPlural, wf.WorkflowTemplatePlural, wf.CronWorkflowPlural, wf.ClusterWorkflowTemplatePlural}
+
 func NewLintCommand() *cobra.Command {
 	var (
 		strict    bool
@@ -20,8 +23,6 @@ func NewLintCommand() *cobra.Command {
 		offline   bool
 	)
 
-	allKinds := []string{wf.WorkflowPlural, wf.WorkflowTemplatePlural, wf.CronWorkflowPlural, wf.ClusterWorkflowTemplatePlural}
-
 	command := &cobra.Command{
 		Use:   "lint FILE...",
 		Short: "validate files or directories of manifests",
@@ -34,23 +35,12 @@ func NewLintCommand() *cobra.Command {
 
   cat manifests.yaml | argo lint --kinds=workflows,cronworkflows -`,
 		Run: func(cmd *cobra.Command, args []string) {
-			client.Offline = offline
-			client.OfflineFiles = args
-			ctx, apiClient := client.NewAPIClient(cmd.Context())
 			if len(args) == 0 {
 				cmd.HelpFunc()(cmd, args)
 				os.Exit(1)
 			}
-			if len(lintKinds) == 0 || strings.Contains(strings.Join(lintKinds, ","), "all") {
-				lintKinds = allKinds
-			}
-			ops := lint.LintOptions{
-				Files:            args,
-				Strict:           strict,
-				DefaultNamespace: client.Namespace(),
-				Printer:          os.Stdout,
-			}
-			lint.RunLint(ctx, apiClient, lintKinds, output, offline, ops)
+
+			runLint(cmd.Context(), args, offline, lintKinds, output, strict)
 		},
 	}
 
@@ -61,3 +51,20 @@ func NewLintCommand() *cobra.Command {
 
 	return command
 }
+
+func runLint(ctx context.Context, args []string, offline bool, lintKinds []string, output string, strict bool) {
+	client.Offline = offline
+	client.OfflineFiles = args
+	ctx, apiClient := client.NewAPIClient(ctx)
+
+	if len(lintKinds) == 0 || strings.Contains(strings.Join(lintKinds, ","), "all") {
+		lintKinds = allKinds
+	}
+	ops := lint.LintOptions{
+		Files:            args,
+		Strict:           strict,
+		DefaultNamespace: client.Namespace(),
+		Printer:          os.Stdout,
+	}
+	lint.RunLint(ctx, apiClient, lintKinds, output, offline, ops)
+}

cmd/argo/commands/lint_test.go

@@ -0,0 +1,157 @@
+package commands
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_OfflineLint(t *testing.T) {
+	dir := t.TempDir()
+
+	subdir := filepath.Join(dir, "subdir")
+	require.NoError(t, os.Mkdir(subdir, 0755))
+	wftmplPath := filepath.Join(subdir, "wftmpl.yaml")
+	err := os.WriteFile(wftmplPath, []byte(`
+apiVersion: argoproj.io/v1alpha1
+kind: WorkflowTemplate
+metadata:
+  name: hello-world-template-local-arg
+  namespace: test
+spec:
+  templates:
+    - name: hello-world
+      inputs:
+        parameters:
+          - name: msg
+            value: hello world
+      container:
+        image: docker/whalesay
+        command:
+          - cowsay
+        args:
+          - '{{inputs.parameters.msg}}'
+`), 0644)
+	require.NoError(t, err)
+
+	clusterWftmplPath := filepath.Join(subdir, "cluster-workflow-template.yaml")
+	err = os.WriteFile(clusterWftmplPath, []byte(`
+apiVersion: argoproj.io/v1alpha1
+kind: ClusterWorkflowTemplate
+metadata:
+  name: hello-world-cluster
+spec:
+  templates:
+  - name: hello-world
+    inputs:
+      parameters:
+        - name: msg
+          value: hello world
+    container:
+      image: docker/whalesay
+      command:
+        - cowsay
+      args:
+        - '{{inputs.parameters.msg}}'
+`), 0644)
+	require.NoError(t, err)
+
+	workflowPath := filepath.Join(subdir, "workflow.yaml")
+	err = os.WriteFile(workflowPath, []byte(`
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: hello-world-local-arg-
+  namespace: test
+spec:
+  entrypoint: whalesay
+  templates:
+    - name: whalesay
+      steps:
+        - - name: hello-world
+            templateRef:
+              name: hello-world-template-local-arg
+              template: hello-world
+          - name: hello-world-cluster
+            templateRef:
+              name: hello-world-cluster
+              template: hello-world
+              clusterScope: true
+`), 0644)
+	require.NoError(t, err)
+
+	t.Run("linting a workflow missing references", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		runLint(context.Background(), []string{workflowPath}, true, nil, "pretty", true)
+
+		assert.True(t, fatal, "should have exited")
+	})
+
+	t.Run("linting a workflow missing a workflow template ref", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		runLint(context.Background(), []string{workflowPath, clusterWftmplPath}, true, nil, "pretty", true)
+
+		assert.True(t, fatal, "should have exited")
+	})
+
+	t.Run("linting a workflow missing a cluster workflow template ref", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		runLint(context.Background(), []string{workflowPath, wftmplPath}, true, nil, "pretty", true)
+
+		assert.True(t, fatal, "should have exited")
+	})
+
+	t.Run("linting a workflow template on its own", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		runLint(context.Background(), []string{wftmplPath}, true, nil, "pretty", true)
+
+		assert.False(t, fatal, "should not have exited")
+	})
+
+	t.Run("linting a cluster workflow template on its own", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		runLint(context.Background(), []string{clusterWftmplPath}, true, nil, "pretty", true)
+
+		assert.False(t, fatal, "should not have exited")
+	})
+
+	t.Run("linting a workflow and templates", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		runLint(context.Background(), []string{workflowPath, wftmplPath, clusterWftmplPath}, true, nil, "pretty", true)
+
+		assert.False(t, fatal, "should not have exited")
+	})
+
+	t.Run("linting a directory", func(t *testing.T) {
+		defer func() { logrus.StandardLogger().ExitFunc = nil }()
+		var fatal bool
+		logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
+
+		runLint(context.Background(), []string{dir}, true, nil, "pretty", true)
+
+		assert.False(t, fatal, "should not have exited")
+	})
+}

cmd/argo/lint/lint.go

@@ -99,7 +99,7 @@ func RunLint(ctx context.Context, client apiclient.Client, kinds []string, outpu
 	errors.CheckError(err)
 
 	if !res.Success {
-		os.Exit(1)
+		log.StandardLogger().Exit(1)
 	}
 }
 

pkg/apiclient/offline-client.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"path/filepath"
 
 	"github.com/argoproj/argo-workflows/v3/pkg/apiclient/clusterworkflowtemplate"
 	"github.com/argoproj/argo-workflows/v3/pkg/apiclient/cronworkflow"
@@ -17,62 +18,92 @@ import (
 	"sigs.k8s.io/yaml"
 )
 
+type offlineWorkflowTemplateGetterMap map[string]templateresolution.WorkflowTemplateNamespacedGetter
+
+func (m offlineWorkflowTemplateGetterMap) GetNamespaceGetter(namespace string) templateresolution.WorkflowTemplateNamespacedGetter {
+	v := m[namespace]
+	if v == nil {
+		return offlineWorkflowTemplateNamespacedGetter{
+			workflowTemplates: map[string]*wfv1.WorkflowTemplate{},
+			namespace:         namespace,
+		}
+	}
+
+	return m[namespace]
+}
+
 type offlineClient struct {
 	clusterWorkflowTemplateGetter       templateresolution.ClusterWorkflowTemplateGetter
-	namespacedWorkflowTemplateGetterMap map[string]templateresolution.WorkflowTemplateNamespacedGetter
+	namespacedWorkflowTemplateGetterMap offlineWorkflowTemplateGetterMap
 }
 
 var OfflineErr = fmt.Errorf("not supported when you are in offline mode")
 
 var _ Client = &offlineClient{}
 
-func newOfflineClient(files []string) (context.Context, Client, error) {
+// newOfflineClient creates a client that keeps all files (or files recursively contained within a path) given to it in memory.
+// It is useful for linting a set of files without having to connect to a cluster.
+func newOfflineClient(paths []string) (context.Context, Client, error) {
 	clusterWorkflowTemplateGetter := &offlineClusterWorkflowTemplateGetter{
 		clusterWorkflowTemplates: map[string]*wfv1.ClusterWorkflowTemplate{},
 	}
-	workflowTemplateGetters := map[string]templateresolution.WorkflowTemplateNamespacedGetter{}
+	workflowTemplateGetters := offlineWorkflowTemplateGetterMap{}
 
-	for _, file := range files {
-		bytes, err := os.ReadFile(file)
-		if err != nil {
-			return nil, nil, fmt.Errorf("failed to read file %s: %w", file, err)
-		}
-		var generic map[string]interface{}
-		if err := yaml.Unmarshal(bytes, &generic); err != nil {
-			return nil, nil, fmt.Errorf("failed to parse YAML from file %s: %w", file, err)
-		}
-		switch generic["kind"] {
-		case "ClusterWorkflowTemplate":
-			cwftmpl := new(wfv1.ClusterWorkflowTemplate)
-			if err := yaml.Unmarshal(bytes, &cwftmpl); err != nil {
-				return nil, nil, fmt.Errorf("failed to unmarshal file %s as a ClusterWorkflowTemplate: %w", file, err)
+	for _, basePath := range paths {
+		err := filepath.Walk(basePath, func(path string, info os.FileInfo, err error) error {
+			if info.IsDir() {
+				return nil
 			}
-
-			if _, ok := clusterWorkflowTemplateGetter.clusterWorkflowTemplates[cwftmpl.Name]; ok {
-				return nil, nil, fmt.Errorf("duplicate ClusterWorkflowTemplate found: %q", cwftmpl.Name)
+			if err != nil {
+				return err
 			}
-			clusterWorkflowTemplateGetter.clusterWorkflowTemplates[cwftmpl.Name] = cwftmpl
 
-		case "WorkflowTemplate":
-			wftmpl := new(wfv1.WorkflowTemplate)
-			if err := yaml.Unmarshal(bytes, &wftmpl); err != nil {
-				return nil, nil, fmt.Errorf("failed to unmarshal file %s as a WorkflowTemplate: %w", file, err)
+			bytes, err := os.ReadFile(path)
+			if err != nil {
+				return fmt.Errorf("failed to read file %s: %w", path, err)
 			}
-			getter, ok := workflowTemplateGetters[wftmpl.Namespace]
-			if !ok {
-				getter = &offlineWorkflowTemplateNamespacedGetter{
-					namespace:         wftmpl.Namespace,
-					workflowTemplates: map[string]*wfv1.WorkflowTemplate{},
-				}
-				workflowTemplateGetters[wftmpl.Namespace] = getter
+			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)
 			}
+			switch generic["kind"] {
+			case "ClusterWorkflowTemplate":
+				cwftmpl := new(wfv1.ClusterWorkflowTemplate)
+				if err := yaml.Unmarshal(bytes, &cwftmpl); err != nil {
+					return fmt.Errorf("failed to unmarshal file %s as a ClusterWorkflowTemplate: %w", path, err)
+				}
+
+				if _, ok := clusterWorkflowTemplateGetter.clusterWorkflowTemplates[cwftmpl.Name]; ok {
+					return fmt.Errorf("duplicate ClusterWorkflowTemplate found: %q", cwftmpl.Name)
+				}
+				clusterWorkflowTemplateGetter.clusterWorkflowTemplates[cwftmpl.Name] = cwftmpl
+
+			case "WorkflowTemplate":
+				wftmpl := new(wfv1.WorkflowTemplate)
+				if err := yaml.Unmarshal(bytes, &wftmpl); err != nil {
+					return fmt.Errorf("failed to unmarshal file %s as a WorkflowTemplate: %w", path, err)
+				}
+				getter, ok := workflowTemplateGetters[wftmpl.Namespace]
+				if !ok {
+					getter = &offlineWorkflowTemplateNamespacedGetter{
+						namespace:         wftmpl.Namespace,
+						workflowTemplates: map[string]*wfv1.WorkflowTemplate{},
+					}
+					workflowTemplateGetters[wftmpl.Namespace] = getter
+				}
 
-			if _, ok := getter.(*offlineWorkflowTemplateNamespacedGetter).workflowTemplates[wftmpl.Name]; ok {
-				return nil, nil, fmt.Errorf("duplicate WorkflowTemplate found: %q", wftmpl.Name)
+				if _, ok := getter.(*offlineWorkflowTemplateNamespacedGetter).workflowTemplates[wftmpl.Name]; ok {
+					return fmt.Errorf("duplicate WorkflowTemplate found: %q", wftmpl.Name)
+				}
+				getter.(*offlineWorkflowTemplateNamespacedGetter).workflowTemplates[wftmpl.Name] = wftmpl
 			}
-			getter.(*offlineWorkflowTemplateNamespacedGetter).workflowTemplates[wftmpl.Name] = wftmpl
-		}
 
+			return nil
+		})
+
+		if err != nil {
+			return nil, nil, err
+		}
 	}
 
 	return context.Background(), &offlineClient{

pkg/apiclient/offline-cluster-workflow-template-service-client.go

@@ -13,7 +13,7 @@ import (
 
 type OfflineClusterWorkflowTemplateServiceClient struct {
 	clusterWorkflowTemplateGetter       templateresolution.ClusterWorkflowTemplateGetter
-	namespacedWorkflowTemplateGetterMap map[string]templateresolution.WorkflowTemplateNamespacedGetter
+	namespacedWorkflowTemplateGetterMap offlineWorkflowTemplateGetterMap
 }
 
 var _ clusterworkflowtmplpkg.ClusterWorkflowTemplateServiceClient = &OfflineClusterWorkflowTemplateServiceClient{}