Skip to content

Commit

Permalink
fix: Replace expressions with placeholders in resource manifest templ…
Browse files Browse the repository at this point in the history
…ate. Fixes #10924 (#10926)

Signed-off-by: Alexander Crow <acrow@akamai.com>
  • Loading branch information
crowcrow authored Apr 24, 2023
1 parent 2401be8 commit 8dbdc02
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 1 deletion.
33 changes: 33 additions & 0 deletions test/e2e/testdata/workflow-template-with-resource-expr.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
name: workflow-template-with-resource-expr
spec:
entrypoint: whalesay
templates:
- name: whalesay
inputs:
parameters:
- name: intParam
value: '20'
- name: strParam
value: 'foobarbaz'
outputs: {}
metadata: {}
resource:
action: create
setOwnerReference: true
manifest: |
apiVersion: v1
kind: Pod
metadata:
name: foo
spec:
restartPolicy: Never
containers:
- name: 'foo'
image: docker/whalesay
command: [cowsay]
args: ["{{=sprig.replace("bar", "baz", inputs.parameters.strParam)}}"]
ports:
- containerPort: {{=asInt(inputs.parameters.intParam)}}
13 changes: 13 additions & 0 deletions test/e2e/workflow_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,19 @@ func (s *WorkflowTemplateSuite) TestSubmitWorkflowTemplateWorkflowMetadataSubsti
})
}

func (s *WorkflowTemplateSuite) TestSubmitWorkflowTemplateResourceUnquotedExpressions() {
s.Given().
WorkflowTemplate("@testdata/workflow-template-with-resource-expr.yaml").
When().
CreateWorkflowTemplates().
SubmitWorkflowsFromWorkflowTemplates().
WaitForWorkflow().
Then().
ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) {
assert.Equal(t, status.Phase, v1alpha1.WorkflowSucceeded)
})
}

func (s *WorkflowTemplateSuite) TestWorkflowTemplateInvalidOnExit() {
s.Given().
WorkflowTemplate("@testdata/workflow-template-invalid-onexit.yaml").
Expand Down
24 changes: 23 additions & 1 deletion workflow/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,22 @@ func (args *FakeArguments) GetArtifactByName(name string) *wfv1.Artifact {

var _ wfv1.ArgumentsProvider = &FakeArguments{}

func SubstituteResourceManifestExpressions(manifest string) string {
var substitutions = make(map[string]string)
pattern, _ := regexp.Compile(`{{\s*=\s*(.+?)\s*}}`)
for _, match := range pattern.FindAllStringSubmatch(manifest, -1) {
substitutions[string(match[1])] = placeholderGenerator.NextPlaceholder()
}

// since we don't need to resolve/evaluate here we can do just a simple replacement
for old, new := range substitutions {
rmatch, _ := regexp.Compile(`{{\s*=\s*` + regexp.QuoteMeta(old) + `\s*}}`)
manifest = rmatch.ReplaceAllString(manifest, new)
}

return manifest
}

// ValidateWorkflow accepts a workflow and performs validation against it.
func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, cwftmplGetter templateresolution.ClusterWorkflowTemplateGetter, wf *wfv1.Workflow, opts ValidateOpts) error {
ctx := newTemplateValidationCtx(wf, opts)
Expand Down Expand Up @@ -375,6 +391,7 @@ func (ctx *templateValidationCtx) validateInitContainers(containers []wfv1.UserC
}

func (ctx *templateValidationCtx) validateTemplate(tmpl *wfv1.Template, tmplCtx *templateresolution.Context, args wfv1.ArgumentsProvider, workflowTemplateValidation bool) error {

if err := validateTemplateType(tmpl); err != nil {
return err
}
Expand Down Expand Up @@ -750,7 +767,12 @@ func (ctx *templateValidationCtx) validateLeaf(scope map[string]interface{}, tmp
if tmpl.Resource.Manifest != "" && !placeholderGenerator.IsPlaceholder(tmpl.Resource.Manifest) {
// Try to unmarshal the given manifest, just ensuring it's a valid YAML.
var obj interface{}
err := yaml.Unmarshal([]byte(tmpl.Resource.Manifest), &obj)

// Unmarshalling will fail if we have unquoted expressions which is sometimes a false positive,
// so for the sake of template validation we will just replace expressions with placeholders
// and the 'real' validation will be performed at a later stage once the manifest is applied
replaced := SubstituteResourceManifestExpressions(tmpl.Resource.Manifest)
err := yaml.Unmarshal([]byte(replaced), &obj)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.resource.manifest must be a valid yaml", tmpl.Name)
}
Expand Down
69 changes: 69 additions & 0 deletions workflow/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package validate

import (
"context"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -2716,6 +2717,74 @@ func TestWorkflowTemplateWithEnumValueWithoutValue(t *testing.T) {
assert.EqualError(t, err, "spec.arguments.message.value is required")
}

var resourceManifestWithExpressions = `
apiVersion: v1
kind: Pod
metadata:
name: foo
spec:
restartPolicy: Never
containers:
- name: 'foo'
image: docker/whalesay
command: [cowsay]
args: ["{{ = asInt(inputs.parameters.intParam) }}"]
ports:
- containerPort: {{=asInt(inputs.parameters.intParam)}}
`

func TestSubstituteResourceManifestExpressions(t *testing.T) {
replaced := SubstituteResourceManifestExpressions(resourceManifestWithExpressions)
assert.NotEqual(t, resourceManifestWithExpressions, replaced)

// despite spacing in the expr itself we should have only 1 placeholder here
patt, _ := regexp.Compile(`placeholder\-\d+`)
matches := patt.FindAllString(replaced, -1)
assert.Exactly(t, 2, len(matches))
assert.Equal(t, matches[0], matches[1])
}

var validWorkflowTemplateWithResourceManifest = `
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
name: workflow-template-with-resource-expr
spec:
entrypoint: whalesay
templates:
- name: whalesay
inputs:
parameters:
- name: intParam
value: '20'
- name: strParam
value: 'foobarbaz'
outputs: {}
metadata: {}
resource:
action: create
setOwnerReference: true
manifest: |
apiVersion: v1
kind: Pod
metadata:
name: foo
spec:
restartPolicy: Never
containers:
- name: 'foo'
image: docker/whalesay
command: [cowsay]
args: ["{{=sprig.replace("bar", "baz", inputs.parameters.strParam)}}"]
ports:
- containerPort: {{=asInt(inputs.parameters.intParam)}}
`

func TestWorkflowTemplateWithResourceManifest(t *testing.T) {
err := validate(validWorkflowTemplateWithResourceManifest)
assert.NoError(t, err)
}

var validActiveDeadlineSecondsArgoVariable = `
apiVersion: argoproj.io/v1alpha1
kind: Workflow
Expand Down

0 comments on commit 8dbdc02

Please sign in to comment.