Conversation
Signed-off-by: Alan Clucas <alan@clucas.org>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes CRD validation patterns to allow template expressions in artifact names and adds comprehensive e2e test coverage for parameterized global artifacts in exit handlers. The changes address issue #14991 by relaxing the regex pattern to permit curly braces and dots, which are essential for template parameter substitution.
Key Changes
- Updated artifact name validation pattern from
^[-a-zA-Z0-9_]+$to^[-a-zA-Z0-9_{}.]+$to support template expressions like{{inputs.parameters.variable}} - Added e2e test that verifies parameterized artifact names work correctly in exit handlers with global artifacts
- Ensured consistency across all CRD definitions, protobuf schemas, and documentation
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/testdata/exit-handler-parameterized-global-artifacts.yaml | New test workflow demonstrating parameterized artifact names in exit handlers with withParam iteration |
| test/e2e/artifacts_test.go | New test function validating that parameterized global artifacts are correctly resolved and accessible in exit handlers |
| pkg/apis/workflow/v1alpha1/workflow_types.go | Updated Artifact.Name validation pattern to allow curly braces and dots |
| pkg/apis/workflow/v1alpha1/generated.proto | Updated protobuf comment to reflect new validation pattern |
| pkg/plugins/executor/swagger.yml | Updated swagger schema validation pattern for artifact names |
| docs/executor_swagger.md | Updated documentation to reflect new artifact name validation rules |
| manifests/quick-start-*.yaml | Updated CRD validation patterns in quick-start manifests (postgres, mysql, minimal) |
| manifests/base/crds/minimal/*.yaml | Updated CRD validation patterns in minimal CRD definitions |
| manifests/base/crds/full/*.yaml | Updated CRD validation patterns in full CRD definitions across all workflow resource types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MasonM
approved these changes
Nov 21, 2025
guanguxiansheng
pushed a commit
to guanguxiansheng/argo-workflows
that referenced
this pull request
Dec 15, 2025
Signed-off-by: Alan Clucas <alan@clucas.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds test from #11610 for #14991
Fixes CRDs from #15028
Motivation
In testing #14991 I discovered I'd broken the test case from #11610 with #15028
Rather than try to push the test into #14991 I felt it easier to do that myself in this separate PR
Modifications
Allow templates in artifact names, CEL validation was strictly for literals not templates in that.
Add #11610's initial test case as an e2e test with minor modifications to make it work.
Verification
This adds a new test case which would fail without the CRD change enclosed and without #14991
Documentation
Non required