test: refactor E2E examples tests and fix Makefile

[MasonM]

Motivation

This rewrites hack/test-examples.sh in Go to make those tests easier to debug and facilitate testing more complex scenarios, like the VAP added in #14045

This also fixes a Makefile bug that @panicboat reported in #14090 (reply in thread). The expression $(shell [ $PROFILE = plugins ] && echo false || echo true) isn't escaping the $ sign, which causes make to substitute the value of the P variable, if any. The official docs recommend using parentheses:

A dollar sign followed by a character other than a dollar sign, open-parenthesis or open-brace treats that single character as the variable name. Thus, you could reference the variable x with ‘$x’. However, this practice can lead to confusion (e.g., ‘$foo’ refers to the variable f followed by the string oo) so we recommend using parentheses or braces around all variables, even single-letter variables, unless omitting them gives significant readability improvements

Modifications

• Added KubectlApply helper to reduce duplication
• Replaced hack/test-example.sh with test/e2e/examples_test.go. The new tests do strict validation, which caught a bug in examples/webhdfs-input-output-artifacts.yaml:

$ make test-examples
<SNIP>
examples_test.go:28: Error parsing ../../examples/webhdfs-input-output-artifacts.yaml: strict decoding error: unknown field "spec.templates[0].outputs.artifacts[0].overwrite"
=== FAIL: ExamplesSuite/TestExampleWorkflows
FAIL    github.com/argoproj/argo-workflows/v3/test/e2e  1.580s
FAIL
make: *** [Makefile:609: test-examples] Error 1

• Fixed Makefile bug

Verification

Ran E2E tests.

[MasonM]

/retest

[tczhao]

I'm wondering what are you thoughts on not including this?

[MasonM]

Because this is redundant, since we're already checking the image when Given.Workflow() is called:

argo-workflows/test/e2e/fixtures/given.go

Lines 86 to 139 in b2a2c8f

	func (g *Given) checkImages(wf interface{}) {
	g.t.Helper()
	var defaultImage string
	var templates []wfv1.Template
	switch baseTemplate := wf.(type) {
	case *wfv1.Workflow:
	if baseTemplate.Spec.TemplateDefaults != nil && baseTemplate.Spec.TemplateDefaults.Container != nil && baseTemplate.Spec.TemplateDefaults.Container.Image != "" {
	defaultImage = baseTemplate.Spec.TemplateDefaults.Container.Image
	templates = baseTemplate.Spec.Templates
	}
	case *wfv1.WorkflowTemplate:
	if baseTemplate.Spec.TemplateDefaults != nil && baseTemplate.Spec.TemplateDefaults.Container != nil && baseTemplate.Spec.TemplateDefaults.Container.Image != "" {
	defaultImage = baseTemplate.Spec.TemplateDefaults.Container.Image
	templates = baseTemplate.Spec.Templates
	}
	case *wfv1.CronWorkflow:
	if baseTemplate.Spec.WorkflowSpec.TemplateDefaults != nil && baseTemplate.Spec.WorkflowSpec.TemplateDefaults.Container != nil && baseTemplate.Spec.WorkflowSpec.TemplateDefaults.Container.Image != "" {
	defaultImage = baseTemplate.Spec.WorkflowSpec.TemplateDefaults.Container.Image
	templates = baseTemplate.Spec.WorkflowSpec.Templates
	}
	default:
	g.t.Fatalf("Unsupported checkImage workflow type: %s", wf)
	}
	// discouraged
	discouraged := func(image string) bool {
	return image == "python:alpine3.6"
	}
	// Using an arbitrary image will result in slow and flakey tests as we can't really predict when they'll be
	// downloaded or evicted. To keep tests fast and reliable you must use allowed images.
	allowed := func(image string) bool {
	return strings.Contains(image, "argoexec:") || image == "argoproj/argosay:v1" || image == "argoproj/argosay:v2" || discouraged(image)
	}
	for _, t := range templates {
	container := t.Container
	if container != nil {
	var image string
	if container.Image != "" {
	image = container.Image
	} else {
	image = defaultImage
	}
	if !allowed(image) {
	g.t.Fatalf("image not allowed in tests: %s", image)
	}
	// (⎈ |docker-desktop:argo)➜ ~ time docker run --rm argoproj/argosay:v2
	// docker run --rm argoproj/argosay˜:v2 0.21s user 0.10s system 16% cpu 1.912 total
	// docker run --rm argoproj/argosay:v1 0.17s user 0.08s system 31% cpu 0.784 total
	if discouraged(image) {
	_, _ = fmt.Println(color.Ize(color.Yellow, "DISCOURAGED IMAGE: "+g.t.Name()+" is using "+image))
	}
	}
	}
	}

but now that I look again, that code is actually broken: it's only checking the images if the workflow defines TemplateDefaults, since otherwise, templates will be an empty array. I think this was a bug introduced 2 years ago in #10784.

I fixed the bug so that it's properly verifying images again, but that caused a ton of tests to fail, since there were a lot of tests introduced since then that used a banned image. I went through and fixed everything. While I was at it, I changed everything under test/ that was using the deprecated python:alpine3.6 image to use argoproj/argosay:v2 instead, which removes the need for the deprecated image check.

[MasonM]

This one was a little tricky, because the behavior of sh in alpine is actually different than sh in argoproj/argosay:v2, because the former allows cat /upload/testUpload.txt > /upload/testUpload.txt and the latter does not:

 docker run --entrypoint=/bin/sh --rm alpine:3.18.4 -xc 'touch /tmp/foo; cat /tmp/foo > /tmp/foo'; echo $?                                               
+ touch /tmp/foo
+ cat /tmp/foo
0

$ docker run --entrypoint=/bin/sh --rm argoproj/argosay:v2 -xc 'touch /tmp/foo; cat /tmp/foo > /tmp/foo'; echo $?                                         
+ touch /tmp/foo
+ cat /tmp/foo
cat: /tmp/foo: input file is output file
1

I think the intention of this was to copy the input artifact to the output, so I updated it to do so

[MasonM]

/retest

[tczhao]

Looks good!