Skip to content

Commit

Permalink
Do not resolve remote templates in lint (#1787)
Browse files Browse the repository at this point in the history
  • Loading branch information
dtaniwaki authored and sarabala1979 committed Dec 5, 2019
1 parent 20cec1d commit 81c7f5b
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 74 deletions.
17 changes: 17 additions & 0 deletions cmd/argo/commands/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
versioned "github.com/argoproj/argo/pkg/client/clientset/versioned"
"github.com/argoproj/argo/pkg/client/clientset/versioned/typed/workflow/v1alpha1"
"github.com/argoproj/argo/workflow/templateresolution"
"github.com/spf13/cobra"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
Expand All @@ -23,6 +24,7 @@ var (
clientset *kubernetes.Clientset
wfClientset *versioned.Clientset
wfClient v1alpha1.WorkflowInterface
wftmplClient v1alpha1.WorkflowTemplateInterface
jobStatusIconMap map[wfv1.NodePhase]string
noColor bool
namespace string
Expand Down Expand Up @@ -97,6 +99,7 @@ func InitWorkflowClient(ns ...string) v1alpha1.WorkflowInterface {
}
wfClientset = versioned.NewForConfigOrDie(restConfig)
wfClient = wfClientset.ArgoprojV1alpha1().Workflows(namespace)
wftmplClient = wfClientset.ArgoprojV1alpha1().WorkflowTemplates(namespace)
return wfClient
}

Expand All @@ -118,3 +121,17 @@ func ansiFormat(s string, codes ...int) string {
sequence := strings.Join(codeStrs, ";")
return fmt.Sprintf("%s[%sm%s%s[%dm", escape, sequence, s, escape, noFormat)
}

// LazyWorkflowTemplateGetter is a wrapper of v1alpha1.WorkflowTemplateInterface which
// supports lazy initialization.
type LazyWorkflowTemplateGetter struct{}

// Get initializes it just before it's actually used and returns a retrieved workflow template.
func (c LazyWorkflowTemplateGetter) Get(name string) (*wfv1.WorkflowTemplate, error) {
if wftmplClient == nil {
_ = InitWorkflowClient()
}
return templateresolution.WrapWorkflowTemplateInterface(wftmplClient).Get(name)
}

var _ templateresolution.WorkflowTemplateNamespacedGetter = &LazyWorkflowTemplateGetter{}
12 changes: 4 additions & 8 deletions cmd/argo/commands/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,16 @@ func NewLintCommand() *cobra.Command {
os.Exit(1)
}

namespace, _, err := clientConfig.Namespace()
if err != nil {
log.Fatal(err)
}

_ = InitWorkflowClient()
var err error
wftmplGetter := &LazyWorkflowTemplateGetter{}
validateDir := cmdutil.MustIsDir(args[0])
if validateDir {
if len(args) > 1 {
fmt.Printf("Validation of a single directory supported")
os.Exit(1)
}
fmt.Printf("Verifying all workflow manifests in directory: %s\n", args[0])
err = validate.LintWorkflowDir(wfClientset, namespace, args[0], strict)
err = validate.LintWorkflowDir(wftmplGetter, args[0], strict)
} else {
yamlFiles := make([]string, 0)
for _, filePath := range args {
Expand All @@ -48,7 +44,7 @@ func NewLintCommand() *cobra.Command {
yamlFiles = append(yamlFiles, filePath)
}
for _, yamlFile := range yamlFiles {
err = validate.LintWorkflowFile(wfClientset, namespace, yamlFile, strict)
err = validate.LintWorkflowFile(wftmplGetter, yamlFile, strict)
if err != nil {
break
}
Expand Down
16 changes: 16 additions & 0 deletions cmd/argo/commands/template/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package template
import (
"log"

wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
versioned "github.com/argoproj/argo/pkg/client/clientset/versioned"
"github.com/argoproj/argo/pkg/client/clientset/versioned/typed/workflow/v1alpha1"
"github.com/argoproj/argo/workflow/templateresolution"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
Expand Down Expand Up @@ -57,3 +59,17 @@ func InitWorkflowTemplateClient(ns ...string) v1alpha1.WorkflowTemplateInterface
wftmplClient = wfClientset.ArgoprojV1alpha1().WorkflowTemplates(namespace)
return wftmplClient
}

// LazyWorkflowTemplateGetter is a wrapper of v1alpha1.WorkflowTemplateInterface which
// supports lazy initialization.
type LazyWorkflowTemplateGetter struct{}

// Get initializes it just before it's actually used and returns a retrieved workflow template.
func (c LazyWorkflowTemplateGetter) Get(name string) (*wfv1.WorkflowTemplate, error) {
if wftmplClient == nil {
_ = InitWorkflowTemplateClient()
}
return templateresolution.WrapWorkflowTemplateInterface(wftmplClient).Get(name)
}

var _ templateresolution.WorkflowTemplateNamespacedGetter = &LazyWorkflowTemplateGetter{}
4 changes: 3 additions & 1 deletion cmd/argo/commands/template/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/argoproj/argo/workflow/common"
"github.com/argoproj/argo/workflow/templateresolution"
"github.com/argoproj/argo/workflow/util"
"github.com/argoproj/argo/workflow/validate"
)
Expand Down Expand Up @@ -62,7 +63,8 @@ func CreateWorkflowTemplates(filePaths []string, cliOpts *cliCreateOpts) {
}

for _, wftmpl := range workflowTemplates {
err := validate.ValidateWorkflowTemplate(wfClientset, namespace, &wftmpl)
wftmplGetter := templateresolution.WrapWorkflowTemplateInterface(wftmplClient)
err := validate.ValidateWorkflowTemplate(wftmplGetter, &wftmpl)
if err != nil {
log.Fatalf("Failed to create workflow template: %v", err)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/argo/commands/template/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ func NewLintCommand() *cobra.Command {
log.Fatal(err)
}

_ = InitWorkflowTemplateClient()
wftmplGetter := &LazyWorkflowTemplateGetter{}
validateDir := cmdutil.MustIsDir(args[0])
if validateDir {
if len(args) > 1 {
fmt.Printf("Validation of a single directory supported")
os.Exit(1)
}
fmt.Printf("Verifying all workflow template manifests in directory: %s\n", args[0])
err = validate.LintWorkflowTemplateDir(wfClientset, namespace, args[0], strict)
err = validate.LintWorkflowTemplateDir(wftmplGetter, namespace, args[0], strict)
} else {
yamlFiles := make([]string, 0)
for _, filePath := range args {
Expand All @@ -48,7 +48,7 @@ func NewLintCommand() *cobra.Command {
yamlFiles = append(yamlFiles, filePath)
}
for _, yamlFile := range yamlFiles {
err = validate.LintWorkflowTemplateFile(wfClientset, namespace, yamlFile, strict)
err = validate.LintWorkflowTemplateFile(wftmplGetter, namespace, yamlFile, strict)
if err != nil {
break
}
Expand Down
3 changes: 2 additions & 1 deletion workflow/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ func (woc *wfOperationCtx) operate() {
if woc.wf.Status.Phase == "" {
woc.markWorkflowRunning()
validateOpts := validate.ValidateOpts{ContainerRuntimeExecutor: woc.controller.Config.ContainerRuntimeExecutor}
err := validate.ValidateWorkflow(woc.controller.wfclientset, woc.wf.Namespace, woc.wf, validateOpts)
wftmplGetter := templateresolution.WrapWorkflowTemplateInterface(woc.controller.wfclientset.ArgoprojV1alpha1().WorkflowTemplates(woc.wf.Namespace))
err := validate.ValidateWorkflow(wftmplGetter, woc.wf, validateOpts)
if err != nil {
woc.markWorkflowFailed(fmt.Sprintf("invalid spec: %s", err.Error()))
return
Expand Down
7 changes: 6 additions & 1 deletion workflow/templateresolution/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package templateresolution
import (
"github.com/argoproj/argo/errors"
wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/argoproj/argo/pkg/client/clientset/versioned/typed/workflow/v1alpha1"
typed "github.com/argoproj/argo/pkg/client/clientset/versioned/typed/workflow/v1alpha1"
"github.com/argoproj/argo/workflow/common"
"github.com/sirupsen/logrus"
Expand All @@ -19,6 +20,10 @@ type workflowTemplateInterfaceWrapper struct {
clientset typed.WorkflowTemplateInterface
}

func WrapWorkflowTemplateInterface(clientset v1alpha1.WorkflowTemplateInterface) WorkflowTemplateNamespacedGetter {
return &workflowTemplateInterfaceWrapper{clientset: clientset}
}

// Get retrieves the WorkflowTemplate of a given name.
func (wrapper *workflowTemplateInterfaceWrapper) Get(name string) (*wfv1.WorkflowTemplate, error) {
return wrapper.clientset.Get(name, metav1.GetOptions{})
Expand Down Expand Up @@ -55,7 +60,7 @@ func NewContext(wftmplGetter WorkflowTemplateNamespacedGetter, tmplBase wfv1.Tem
// NewContext returns new Context.
func NewContextFromClientset(clientset typed.WorkflowTemplateInterface, tmplBase wfv1.TemplateGetter, storage wfv1.TemplateStorage) *Context {
return &Context{
wftmplGetter: &workflowTemplateInterfaceWrapper{clientset: clientset},
wftmplGetter: WrapWorkflowTemplateInterface(clientset),
tmplBase: tmplBase,
storage: storage,
log: log.WithFields(logrus.Fields{}),
Expand Down
4 changes: 3 additions & 1 deletion workflow/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"github.com/argoproj/argo/util/retry"
unstructutil "github.com/argoproj/argo/util/unstructured"
"github.com/argoproj/argo/workflow/common"
"github.com/argoproj/argo/workflow/templateresolution"
"github.com/argoproj/argo/workflow/validate"
)

Expand Down Expand Up @@ -252,7 +253,8 @@ func SubmitWorkflow(wfIf v1alpha1.WorkflowInterface, wfClientset wfclientset.Int
wf.SetOwnerReferences(append(wf.GetOwnerReferences(), *opts.OwnerReference))
}

err := validate.ValidateWorkflow(wfClientset, namespace, wf, validate.ValidateOpts{})
wftmplGetter := templateresolution.WrapWorkflowTemplateInterface(wfClientset.ArgoprojV1alpha1().WorkflowTemplates(namespace))
err := validate.ValidateWorkflow(wftmplGetter, wf, validate.ValidateOpts{})
if err != nil {
return nil, err
}
Expand Down
18 changes: 9 additions & 9 deletions workflow/validate/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import (
"github.com/argoproj/argo/errors"
"github.com/argoproj/argo/pkg/apis/workflow"
wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
wfclientset "github.com/argoproj/argo/pkg/client/clientset/versioned"
"github.com/argoproj/argo/workflow/common"
"github.com/argoproj/argo/workflow/templateresolution"
)

// LintWorkflowDir validates all workflow manifests in a directory. Ignores non-workflow manifests
func LintWorkflowDir(wfClientset wfclientset.Interface, namespace, dirPath string, strict bool) error {
func LintWorkflowDir(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, dirPath string, strict bool) error {
walkFunc := func(path string, info os.FileInfo, err error) error {
if info == nil || info.IsDir() {
return nil
Expand All @@ -26,14 +26,14 @@ func LintWorkflowDir(wfClientset wfclientset.Interface, namespace, dirPath strin
default:
return nil
}
return LintWorkflowFile(wfClientset, namespace, path, strict)
return LintWorkflowFile(wftmplGetter, path, strict)
}
return filepath.Walk(dirPath, walkFunc)
}

// LintWorkflowFile lints a json file, or multiple workflow manifest in a single yaml file. Ignores
// non-workflow manifests
func LintWorkflowFile(wfClientset wfclientset.Interface, namespace, filePath string, strict bool) error {
func LintWorkflowFile(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, filePath string, strict bool) error {
body, err := ioutil.ReadFile(filePath)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "Can't read from file: %s, err: %v", filePath, err)
Expand Down Expand Up @@ -62,7 +62,7 @@ func LintWorkflowFile(wfClientset wfclientset.Interface, namespace, filePath str
return errors.Errorf(errors.CodeBadRequest, "%s failed to parse: %v", filePath, err)
}
for _, wf := range workflows {
err = ValidateWorkflow(wfClientset, namespace, &wf, ValidateOpts{Lint: true})
err = ValidateWorkflow(wftmplGetter, &wf, ValidateOpts{Lint: true})
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "%s: %s", filePath, err.Error())
}
Expand All @@ -71,7 +71,7 @@ func LintWorkflowFile(wfClientset wfclientset.Interface, namespace, filePath str
}

// LintWorkflowTemplateDir validates all workflow manifests in a directory. Ignores non-workflow template manifests
func LintWorkflowTemplateDir(wfClientset wfclientset.Interface, namespace, dirPath string, strict bool) error {
func LintWorkflowTemplateDir(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, namespace, dirPath string, strict bool) error {
walkFunc := func(path string, info os.FileInfo, err error) error {
if info == nil || info.IsDir() {
return nil
Expand All @@ -82,14 +82,14 @@ func LintWorkflowTemplateDir(wfClientset wfclientset.Interface, namespace, dirPa
default:
return nil
}
return LintWorkflowTemplateFile(wfClientset, namespace, path, strict)
return LintWorkflowTemplateFile(wftmplGetter, namespace, path, strict)
}
return filepath.Walk(dirPath, walkFunc)
}

// LintWorkflowTemplateFile lints a json file, or multiple workflow template manifest in a single yaml file. Ignores
// non-workflow template manifests
func LintWorkflowTemplateFile(wfClientset wfclientset.Interface, namespace, filePath string, strict bool) error {
func LintWorkflowTemplateFile(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, namespace, filePath string, strict bool) error {
body, err := ioutil.ReadFile(filePath)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "Can't read from file: %s, err: %v", filePath, err)
Expand Down Expand Up @@ -118,7 +118,7 @@ func LintWorkflowTemplateFile(wfClientset wfclientset.Interface, namespace, file
return errors.Errorf(errors.CodeBadRequest, "%s failed to parse: %v", filePath, err)
}
for _, wftmpl := range workflowTemplates {
err = ValidateWorkflowTemplate(wfClientset, namespace, &wftmpl)
err = ValidateWorkflowTemplate(wftmplGetter, &wftmpl)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "%s: %s", filePath, err.Error())
}
Expand Down
22 changes: 7 additions & 15 deletions workflow/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (

"github.com/argoproj/argo/errors"
wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
wfclientset "github.com/argoproj/argo/pkg/client/clientset/versioned"
"github.com/argoproj/argo/workflow/artifacts/hdfs"
"github.com/argoproj/argo/workflow/common"
"github.com/argoproj/argo/workflow/templateresolution"
Expand Down Expand Up @@ -48,7 +47,7 @@ type templateValidationCtx struct {
wf *wfv1.Workflow
}

func newTemplateValidationCtx(wfClientset wfclientset.Interface, namespace string, wf *wfv1.Workflow, opts ValidateOpts) *templateValidationCtx {
func newTemplateValidationCtx(wf *wfv1.Workflow, opts ValidateOpts) *templateValidationCtx {
globalParams := make(map[string]string)
globalParams[common.GlobalVarWorkflowName] = placeholderValue
globalParams[common.GlobalVarWorkflowNamespace] = placeholderValue
Expand Down Expand Up @@ -85,13 +84,9 @@ func (args *FakeArguments) GetArtifactByName(name string) *wfv1.Artifact {
var _ wfv1.ArgumentsProvider = &FakeArguments{}

// ValidateWorkflow accepts a workflow and performs validation against it.
func ValidateWorkflow(wfClientset wfclientset.Interface, namespace string, wf *wfv1.Workflow, opts ValidateOpts) error {
if wf.Namespace != "" {
namespace = wf.Namespace
}

ctx := newTemplateValidationCtx(wfClientset, namespace, wf, opts)
tmplCtx := templateresolution.NewContextFromClientset(wfClientset.ArgoprojV1alpha1().WorkflowTemplates(namespace), wf, wf)
func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, wf *wfv1.Workflow, opts ValidateOpts) error {
ctx := newTemplateValidationCtx(wf, opts)
tmplCtx := templateresolution.NewContext(wftmplGetter, wf, wf)

err := validateWorkflowFieldNames(wf.Spec.Templates)
if err != nil {
Expand Down Expand Up @@ -163,12 +158,9 @@ func ValidateWorkflow(wfClientset wfclientset.Interface, namespace string, wf *w
}

// ValidateWorkflow accepts a workflow template and performs validation against it.
func ValidateWorkflowTemplate(wfClientset wfclientset.Interface, namespace string, wftmpl *wfv1.WorkflowTemplate) error {
if wftmpl.Namespace != "" {
namespace = wftmpl.Namespace
}
ctx := newTemplateValidationCtx(wfClientset, namespace, nil, ValidateOpts{})
tmplCtx := templateresolution.NewContextFromClientset(wfClientset.ArgoprojV1alpha1().WorkflowTemplates(namespace), wftmpl, nil)
func ValidateWorkflowTemplate(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, wftmpl *wfv1.WorkflowTemplate) error {
ctx := newTemplateValidationCtx(nil, ValidateOpts{})
tmplCtx := templateresolution.NewContext(wftmplGetter, wftmpl, nil)

// Check if all templates can be resolved.
for _, template := range wftmpl.Spec.Templates {
Expand Down
Loading

0 comments on commit 81c7f5b

Please sign in to comment.