diff --git a/cmd/argo/commands/lint.go b/cmd/argo/commands/lint.go index c67b9a05f341..f730ae3729a7 100644 --- a/cmd/argo/commands/lint.go +++ b/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) +} diff --git a/cmd/argo/commands/lint_test.go b/cmd/argo/commands/lint_test.go new file mode 100644 index 000000000000..bccf4441a084 --- /dev/null +++ b/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") + }) +} diff --git a/cmd/argo/lint/lint.go b/cmd/argo/lint/lint.go index 35ef9ad1b431..0262a40fee12 100644 --- a/cmd/argo/lint/lint.go +++ b/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) } } diff --git a/pkg/apiclient/offline-client.go b/pkg/apiclient/offline-client.go index 8da6113f3b40..413b34950cca 100644 --- a/pkg/apiclient/offline-client.go +++ b/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{ diff --git a/pkg/apiclient/offline-cluster-workflow-template-service-client.go b/pkg/apiclient/offline-cluster-workflow-template-service-client.go index 35d0bf647fdd..51795f5ed856 100644 --- a/pkg/apiclient/offline-cluster-workflow-template-service-client.go +++ b/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{} diff --git a/pkg/apiclient/offline-cron-workflow-service-client.go b/pkg/apiclient/offline-cron-workflow-service-client.go index c526b96cb3b6..6104a207c89b 100644 --- a/pkg/apiclient/offline-cron-workflow-service-client.go +++ b/pkg/apiclient/offline-cron-workflow-service-client.go @@ -13,13 +13,13 @@ import ( type OfflineCronWorkflowServiceClient struct { clusterWorkflowTemplateGetter templateresolution.ClusterWorkflowTemplateGetter - namespacedWorkflowTemplateGetterMap map[string]templateresolution.WorkflowTemplateNamespacedGetter + namespacedWorkflowTemplateGetterMap offlineWorkflowTemplateGetterMap } var _ cronworkflow.CronWorkflowServiceClient = &OfflineCronWorkflowServiceClient{} func (o OfflineCronWorkflowServiceClient) LintCronWorkflow(ctx context.Context, req *cronworkflow.LintCronWorkflowRequest, _ ...grpc.CallOption) (*v1alpha1.CronWorkflow, error) { - err := validate.ValidateCronWorkflow(o.namespacedWorkflowTemplateGetterMap[req.Namespace], o.clusterWorkflowTemplateGetter, req.CronWorkflow) + err := validate.ValidateCronWorkflow(o.namespacedWorkflowTemplateGetterMap.GetNamespaceGetter(req.Namespace), o.clusterWorkflowTemplateGetter, req.CronWorkflow) if err != nil { return nil, err } diff --git a/pkg/apiclient/offline-workflow-service-client.go b/pkg/apiclient/offline-workflow-service-client.go index c5925e54c6dc..706db86518fb 100644 --- a/pkg/apiclient/offline-workflow-service-client.go +++ b/pkg/apiclient/offline-workflow-service-client.go @@ -13,7 +13,7 @@ import ( type OfflineWorkflowServiceClient struct { clusterWorkflowTemplateGetter templateresolution.ClusterWorkflowTemplateGetter - namespacedWorkflowTemplateGetterMap map[string]templateresolution.WorkflowTemplateNamespacedGetter + namespacedWorkflowTemplateGetterMap offlineWorkflowTemplateGetterMap } var _ workflowpkg.WorkflowServiceClient = &OfflineWorkflowServiceClient{} @@ -71,7 +71,7 @@ func (o OfflineWorkflowServiceClient) SetWorkflow(context.Context, *workflowpkg. } func (o OfflineWorkflowServiceClient) LintWorkflow(_ context.Context, req *workflowpkg.WorkflowLintRequest, _ ...grpc.CallOption) (*wfv1.Workflow, error) { - err := validate.ValidateWorkflow(o.namespacedWorkflowTemplateGetterMap[req.Namespace], o.clusterWorkflowTemplateGetter, req.Workflow, validate.ValidateOpts{Lint: true}) + err := validate.ValidateWorkflow(o.namespacedWorkflowTemplateGetterMap.GetNamespaceGetter(req.Namespace), o.clusterWorkflowTemplateGetter, req.Workflow, validate.ValidateOpts{Lint: true}) if err != nil { return nil, err } diff --git a/pkg/apiclient/offline-workflow-template-service-client.go b/pkg/apiclient/offline-workflow-template-service-client.go index 4cb70c9f29d1..429c232d1094 100644 --- a/pkg/apiclient/offline-workflow-template-service-client.go +++ b/pkg/apiclient/offline-workflow-template-service-client.go @@ -13,7 +13,7 @@ import ( type OfflineWorkflowTemplateServiceClient struct { clusterWorkflowTemplateGetter templateresolution.ClusterWorkflowTemplateGetter - namespacedWorkflowTemplateGetterMap map[string]templateresolution.WorkflowTemplateNamespacedGetter + namespacedWorkflowTemplateGetterMap offlineWorkflowTemplateGetterMap } var _ workflowtemplatepkg.WorkflowTemplateServiceClient = &OfflineWorkflowTemplateServiceClient{} @@ -39,7 +39,7 @@ func (o OfflineWorkflowTemplateServiceClient) DeleteWorkflowTemplate(ctx context } func (o OfflineWorkflowTemplateServiceClient) LintWorkflowTemplate(ctx context.Context, req *workflowtemplatepkg.WorkflowTemplateLintRequest, _ ...grpc.CallOption) (*v1alpha1.WorkflowTemplate, error) { - err := validate.ValidateWorkflowTemplate(o.namespacedWorkflowTemplateGetterMap[req.Namespace], o.clusterWorkflowTemplateGetter, req.Template, validate.ValidateOpts{Lint: true}) + err := validate.ValidateWorkflowTemplate(o.namespacedWorkflowTemplateGetterMap.GetNamespaceGetter(req.Namespace), o.clusterWorkflowTemplateGetter, req.Template, validate.ValidateOpts{Lint: true}) if err != nil { return nil, err }