From c86215b7cfd5bb1bd61fb64c5b9169aebd2fade8 Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Wed, 17 Sep 2025 14:57:40 +0200 Subject: [PATCH 1/9] test: Add artifact plugin unit tests. re: #9812 Signed-off-by: J.P. Zivalich --- .../v1alpha1/artifact_plugins_test.go | 601 ++++++++++++++++++ 1 file changed, 601 insertions(+) create mode 100644 pkg/apis/workflow/v1alpha1/artifact_plugins_test.go diff --git a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go new file mode 100644 index 000000000000..3553ab811a83 --- /dev/null +++ b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go @@ -0,0 +1,601 @@ +package v1alpha1 + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apiv1 "k8s.io/api/core/v1" + "k8s.io/utils/ptr" + + "github.com/argoproj/argo-workflows/v3/util/logging" +) + +func TestArtifactPluginName(t *testing.T) { + pluginName := ArtifactPluginName("my-plugin") + + t.Run("SocketDir", func(t *testing.T) { + expected := "/artifact-plugins/my-plugin" + assert.Equal(t, expected, pluginName.SocketDir()) + }) + + t.Run("SocketPath", func(t *testing.T) { + expected := "/artifact-plugins/my-plugin/socket" + assert.Equal(t, expected, pluginName.SocketPath()) + }) + + t.Run("VolumeMount", func(t *testing.T) { + volumeMount := pluginName.VolumeMount() + expected := apiv1.VolumeMount{ + Name: "artifact-plugin-my-plugin", + MountPath: "/artifact-plugins/my-plugin", + } + assert.Equal(t, expected, volumeMount) + }) + + t.Run("Volume", func(t *testing.T) { + volume := pluginName.Volume() + expected := apiv1.Volume{ + Name: "artifact-plugin-my-plugin", + VolumeSource: apiv1.VolumeSource{ + EmptyDir: &apiv1.EmptyDirVolumeSource{}, + }, + } + assert.Equal(t, expected, volume) + }) + + t.Run("EmptyPluginName", func(t *testing.T) { + emptyPlugin := ArtifactPluginName("") + assert.Equal(t, "/artifact-plugins/", emptyPlugin.SocketDir()) + assert.Equal(t, "/artifact-plugins//socket", emptyPlugin.SocketPath()) + }) + + t.Run("PluginNameWithSpecialChars", func(t *testing.T) { + specialPlugin := ArtifactPluginName("my-plugin-v1.2.3") + assert.Equal(t, "/artifact-plugins/my-plugin-v1.2.3", specialPlugin.SocketDir()) + assert.Equal(t, "artifact-plugin-my-plugin-v1.2.3", specialPlugin.Volume().Name) + }) +} + +func TestPluginArtifact(t *testing.T) { + t.Run("GetKey", func(t *testing.T) { + plugin := &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + } + key, err := plugin.GetKey() + assert.NoError(t, err) + assert.Equal(t, "path/to/artifact", key) + }) + + t.Run("SetKey", func(t *testing.T) { + plugin := &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "old/path", + } + err := plugin.SetKey("new/path/to/artifact") + assert.NoError(t, err) + assert.Equal(t, "new/path/to/artifact", plugin.Key) + }) + + t.Run("HasLocation_Complete", func(t *testing.T) { + plugin := &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + } + assert.True(t, plugin.HasLocation()) + }) + + t.Run("HasLocation_MissingName", func(t *testing.T) { + plugin := &PluginArtifact{ + Name: "", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + } + assert.False(t, plugin.HasLocation()) + }) + + t.Run("HasLocation_MissingConfiguration", func(t *testing.T) { + plugin := &PluginArtifact{ + Name: "test-plugin", + Configuration: "", + Key: "path/to/artifact", + } + assert.False(t, plugin.HasLocation()) + }) + + t.Run("HasLocation_MissingKey", func(t *testing.T) { + plugin := &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "", + } + assert.False(t, plugin.HasLocation()) + }) + + t.Run("HasLocation_Nil", func(t *testing.T) { + var plugin *PluginArtifact + assert.False(t, plugin.HasLocation()) + }) + + t.Run("ConnectionTimeoutSeconds", func(t *testing.T) { + plugin := &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + ConnectionTimeoutSeconds: 30, + } + assert.Equal(t, int32(30), plugin.ConnectionTimeoutSeconds) + assert.True(t, plugin.HasLocation()) + }) +} + +func TestPluginArtifactRepository(t *testing.T) { + t.Run("IntoArtifactLocation_WithKeyFormat", func(t *testing.T) { + repo := &PluginArtifactRepository{ + Name: "my-plugin", + KeyFormat: "custom/{{workflow.name}}/{{pod.name}}/{{artifact.name}}", + Configuration: `{"endpoint": "https://my-storage.com"}`, + } + + location := &ArtifactLocation{} + repo.IntoArtifactLocation(location) + + require.NotNil(t, location.Plugin) + assert.Equal(t, ArtifactPluginName("my-plugin"), location.Plugin.Name) + assert.Equal(t, `{"endpoint": "https://my-storage.com"}`, location.Plugin.Configuration) + assert.Equal(t, "custom/{{workflow.name}}/{{pod.name}}/{{artifact.name}}", location.Plugin.Key) + }) + + t.Run("IntoArtifactLocation_WithoutKeyFormat", func(t *testing.T) { + repo := &PluginArtifactRepository{ + Name: "my-plugin", + Configuration: `{"endpoint": "https://my-storage.com"}`, + } + + location := &ArtifactLocation{} + repo.IntoArtifactLocation(location) + + require.NotNil(t, location.Plugin) + assert.Equal(t, ArtifactPluginName("my-plugin"), location.Plugin.Name) + assert.Equal(t, `{"endpoint": "https://my-storage.com"}`, location.Plugin.Configuration) + assert.Equal(t, DefaultArchivePattern, location.Plugin.Key) + }) + + t.Run("IntoArtifactLocation_EmptyKeyFormat", func(t *testing.T) { + repo := &PluginArtifactRepository{ + Name: "my-plugin", + KeyFormat: "", + Configuration: `{"endpoint": "https://my-storage.com"}`, + } + + location := &ArtifactLocation{} + repo.IntoArtifactLocation(location) + + require.NotNil(t, location.Plugin) + assert.Equal(t, DefaultArchivePattern, location.Plugin.Key) + }) +} + +func TestArtifactLocation_Plugin(t *testing.T) { + t.Run("Get_Plugin", func(t *testing.T) { + location := &ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + }, + } + + artifact, err := location.Get() + assert.NoError(t, err) + assert.IsType(t, &PluginArtifact{}, artifact) + + pluginArtifact := artifact.(*PluginArtifact) + assert.Equal(t, ArtifactPluginName("test-plugin"), pluginArtifact.Name) + assert.Equal(t, `{"bucket": "my-bucket"}`, pluginArtifact.Configuration) + assert.Equal(t, "path/to/artifact", pluginArtifact.Key) + }) + + t.Run("SetType_Plugin", func(t *testing.T) { + location := &ArtifactLocation{} + pluginArtifact := &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + } + + err := location.SetType(pluginArtifact) + assert.NoError(t, err) + assert.NotNil(t, location.Plugin) + // Note: SetType creates a new empty instance, not copying the values + assert.Equal(t, ArtifactPluginName(""), location.Plugin.Name) + }) + + t.Run("HasLocation_Plugin", func(t *testing.T) { + location := &ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + }, + } + assert.True(t, location.HasLocation()) + }) + + t.Run("HasLocation_PluginIncomplete", func(t *testing.T) { + location := &ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "test-plugin", + Configuration: "", + Key: "path/to/artifact", + }, + } + assert.False(t, location.HasLocation()) + }) + + t.Run("GetKey_Plugin", func(t *testing.T) { + location := &ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + }, + } + + key, err := location.GetKey() + assert.NoError(t, err) + assert.Equal(t, "path/to/artifact", key) + }) + + t.Run("SetKey_Plugin", func(t *testing.T) { + location := &ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "old/path", + }, + } + + err := location.SetKey("new/path/to/artifact") + assert.NoError(t, err) + assert.Equal(t, "new/path/to/artifact", location.Plugin.Key) + }) +} + +func TestArtifacts_GetPluginNames(t *testing.T) { + ctx := logging.WithLogger(context.Background(), logging.NewTestLogger(logging.Info, logging.JSON)) + + t.Run("NoPlugins", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "regular-artifact", + ArtifactLocation: ArtifactLocation{ + S3: &S3Artifact{ + S3Bucket: S3Bucket{Bucket: "my-bucket"}, + Key: "path/to/artifact", + }, + }, + }, + } + + pluginNames := artifacts.GetPluginNames(ctx, nil, ExcludeLogs) + assert.Empty(t, pluginNames) + }) + + t.Run("SinglePlugin", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "plugin-artifact", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "my-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + }, + }, + }, + } + + pluginNames := artifacts.GetPluginNames(ctx, nil, ExcludeLogs) + assert.Len(t, pluginNames, 1) + assert.Contains(t, pluginNames, ArtifactPluginName("my-plugin")) + }) + + t.Run("MultiplePlugins", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "plugin-artifact-1", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "plugin-1", + Configuration: `{"bucket": "bucket-1"}`, + Key: "path/to/artifact1", + }, + }, + }, + { + Name: "plugin-artifact-2", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "plugin-2", + Configuration: `{"bucket": "bucket-2"}`, + Key: "path/to/artifact2", + }, + }, + }, + } + + pluginNames := artifacts.GetPluginNames(ctx, nil, ExcludeLogs) + assert.Len(t, pluginNames, 2) + assert.Contains(t, pluginNames, ArtifactPluginName("plugin-1")) + assert.Contains(t, pluginNames, ArtifactPluginName("plugin-2")) + }) + + t.Run("DuplicatePlugins", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "plugin-artifact-1", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "my-plugin", + Configuration: `{"bucket": "bucket-1"}`, + Key: "path/to/artifact1", + }, + }, + }, + { + Name: "plugin-artifact-2", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "my-plugin", + Configuration: `{"bucket": "bucket-2"}`, + Key: "path/to/artifact2", + }, + }, + }, + } + + pluginNames := artifacts.GetPluginNames(ctx, nil, ExcludeLogs) + assert.Len(t, pluginNames, 1) + assert.Contains(t, pluginNames, ArtifactPluginName("my-plugin")) + }) + + t.Run("WithDefaultRepo", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "artifact-without-plugin", + ArtifactLocation: ArtifactLocation{ + // No plugin specified, should use default repo + }, + }, + } + + defaultRepo := &ArtifactRepository{ + Plugin: &PluginArtifactRepository{ + Name: "default-plugin", + Configuration: `{"bucket": "default-bucket"}`, + }, + } + + pluginNames := artifacts.GetPluginNames(ctx, defaultRepo, ExcludeLogs) + assert.Len(t, pluginNames, 1) + assert.Contains(t, pluginNames, ArtifactPluginName("default-plugin")) + }) + + t.Run("IncludeLogs", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "regular-artifact", + ArtifactLocation: ArtifactLocation{ + S3: &S3Artifact{ + S3Bucket: S3Bucket{Bucket: "my-bucket"}, + Key: "path/to/artifact", + }, + }, + }, + } + + defaultRepo := &ArtifactRepository{ + Plugin: &PluginArtifactRepository{ + Name: "log-plugin", + Configuration: `{"bucket": "log-bucket"}`, + }, + ArchiveLogs: ptr.To(true), + } + + pluginNames := artifacts.GetPluginNames(ctx, defaultRepo, IncludeLogs) + assert.Len(t, pluginNames, 1) + assert.Contains(t, pluginNames, ArtifactPluginName("log-plugin")) + }) + + t.Run("ExcludeLogs", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "regular-artifact", + ArtifactLocation: ArtifactLocation{ + S3: &S3Artifact{ + S3Bucket: S3Bucket{Bucket: "my-bucket"}, + Key: "path/to/artifact", + }, + }, + }, + } + + defaultRepo := &ArtifactRepository{ + S3: &S3ArtifactRepository{ + S3Bucket: S3Bucket{Bucket: "log-bucket"}, + }, + ArchiveLogs: ptr.To(true), + } + + pluginNames := artifacts.GetPluginNames(ctx, defaultRepo, ExcludeLogs) + // When ExcludeLogs is used and there are no plugin artifacts, should be empty + assert.Empty(t, pluginNames) + }) + + t.Run("MixedArtifacts", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "s3-artifact", + ArtifactLocation: ArtifactLocation{ + S3: &S3Artifact{ + S3Bucket: S3Bucket{Bucket: "s3-bucket"}, + Key: "path/to/s3-artifact", + }, + }, + }, + { + Name: "plugin-artifact", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "my-plugin", + Configuration: `{"bucket": "plugin-bucket"}`, + Key: "path/to/plugin-artifact", + }, + }, + }, + { + Name: "default-artifact", + ArtifactLocation: ArtifactLocation{ + // No specific location, should use default + }, + }, + } + + defaultRepo := &ArtifactRepository{ + Plugin: &PluginArtifactRepository{ + Name: "default-plugin", + Configuration: `{"bucket": "default-bucket"}`, + }, + } + + pluginNames := artifacts.GetPluginNames(ctx, defaultRepo, ExcludeLogs) + assert.Len(t, pluginNames, 2) + assert.Contains(t, pluginNames, ArtifactPluginName("my-plugin")) + assert.Contains(t, pluginNames, ArtifactPluginName("default-plugin")) + }) +} + +func TestTemplate_PluginType(t *testing.T) { + t.Run("GetType_Plugin", func(t *testing.T) { + tmpl := &Template{ + Plugin: &Plugin{ + Object: Object{ + Value: []byte(`{"my-plugin": {"config": "value"}}`), + }, + }, + } + assert.Equal(t, TemplateTypePlugin, tmpl.GetType()) + }) + + t.Run("GetNodeType_Plugin", func(t *testing.T) { + tmpl := &Template{ + Plugin: &Plugin{ + Object: Object{ + Value: []byte(`{"my-plugin": {"config": "value"}}`), + }, + }, + } + assert.Equal(t, NodeTypePlugin, tmpl.GetNodeType()) + }) + + t.Run("IsLeaf_Plugin", func(t *testing.T) { + tmpl := &Template{ + Plugin: &Plugin{ + Object: Object{ + Value: []byte(`{"my-plugin": {"config": "value"}}`), + }, + }, + } + assert.True(t, tmpl.IsLeaf()) + }) + + t.Run("IsPodType_Plugin", func(t *testing.T) { + tmpl := &Template{ + Plugin: &Plugin{ + Object: Object{ + Value: []byte(`{"my-plugin": {"config": "value"}}`), + }, + }, + } + assert.False(t, tmpl.IsPodType()) + }) + + t.Run("HasOutput_Plugin", func(t *testing.T) { + tmpl := &Template{ + Plugin: &Plugin{ + Object: Object{ + Value: []byte(`{"my-plugin": {"config": "value"}}`), + }, + }, + } + assert.True(t, tmpl.HasOutput()) + }) +} + +func TestNodeStatus_PluginType(t *testing.T) { + t.Run("IsTaskSetNode_Plugin", func(t *testing.T) { + node := &NodeStatus{ + Type: NodeTypePlugin, + } + assert.True(t, node.IsTaskSetNode()) + }) + + t.Run("IsTaskSetNode_HTTP", func(t *testing.T) { + node := &NodeStatus{ + Type: NodeTypeHTTP, + } + assert.True(t, node.IsTaskSetNode()) + }) + + t.Run("IsTaskSetNode_Pod", func(t *testing.T) { + node := &NodeStatus{ + Type: NodeTypePod, + } + assert.False(t, node.IsTaskSetNode()) + }) +} + +func TestArtifactRepository_Plugin_Integration(t *testing.T) { + t.Run("Get_Plugin", func(t *testing.T) { + repo := &ArtifactRepository{ + Plugin: &PluginArtifactRepository{ + Name: "test-plugin", + KeyFormat: "custom/{{workflow.name}}/{{pod.name}}", + Configuration: `{"endpoint": "https://my-storage.com"}`, + }, + } + + repoType := repo.Get() + assert.IsType(t, &PluginArtifactRepository{}, repoType) + + pluginRepo := repoType.(*PluginArtifactRepository) + assert.Equal(t, ArtifactPluginName("test-plugin"), pluginRepo.Name) + assert.Equal(t, "custom/{{workflow.name}}/{{pod.name}}", pluginRepo.KeyFormat) + assert.Equal(t, `{"endpoint": "https://my-storage.com"}`, pluginRepo.Configuration) + }) + + t.Run("ToArtifactLocation_Plugin", func(t *testing.T) { + repo := &ArtifactRepository{ + Plugin: &PluginArtifactRepository{ + Name: "test-plugin", + KeyFormat: "custom/{{workflow.name}}/{{pod.name}}", + Configuration: `{"endpoint": "https://my-storage.com"}`, + }, + ArchiveLogs: ptr.To(true), + } + + location := repo.ToArtifactLocation() + require.NotNil(t, location) + assert.Equal(t, ptr.To(true), location.ArchiveLogs) + require.NotNil(t, location.Plugin) + assert.Equal(t, ArtifactPluginName("test-plugin"), location.Plugin.Name) + assert.Equal(t, "custom/{{workflow.name}}/{{pod.name}}", location.Plugin.Key) + assert.Equal(t, `{"endpoint": "https://my-storage.com"}`, location.Plugin.Configuration) + }) +} From 0c6c734324d3dc3721089eae07731bb3d2d651ec Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Wed, 17 Sep 2025 15:00:29 +0200 Subject: [PATCH 2/9] test multiple plugins Signed-off-by: J.P. Zivalich --- .../v1alpha1/artifact_plugins_test.go | 338 ++++++++++++++++++ 1 file changed, 338 insertions(+) diff --git a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go index 3553ab811a83..ca19f88f9d0f 100644 --- a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go +++ b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go @@ -479,6 +479,344 @@ func TestArtifacts_GetPluginNames(t *testing.T) { assert.Contains(t, pluginNames, ArtifactPluginName("my-plugin")) assert.Contains(t, pluginNames, ArtifactPluginName("default-plugin")) }) + + t.Run("MultiplePluginsWithDefaultRepo", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "plugin-artifact-1", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "plugin-1", + Configuration: `{"bucket": "bucket-1"}`, + Key: "path/to/artifact1", + }, + }, + }, + { + Name: "plugin-artifact-2", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "plugin-2", + Configuration: `{"bucket": "bucket-2"}`, + Key: "path/to/artifact2", + }, + }, + }, + { + Name: "default-artifact", + ArtifactLocation: ArtifactLocation{ + // No specific location, should use default repo + }, + }, + } + + defaultRepo := &ArtifactRepository{ + Plugin: &PluginArtifactRepository{ + Name: "default-plugin", + Configuration: `{"bucket": "default-bucket"}`, + }, + ArchiveLogs: ptr.To(true), + } + + pluginNames := artifacts.GetPluginNames(ctx, defaultRepo, IncludeLogs) + assert.Len(t, pluginNames, 3) + assert.Contains(t, pluginNames, ArtifactPluginName("plugin-1")) + assert.Contains(t, pluginNames, ArtifactPluginName("plugin-2")) + assert.Contains(t, pluginNames, ArtifactPluginName("default-plugin")) + }) + + t.Run("MultiplePluginsWithLogging", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "s3-artifact", + ArtifactLocation: ArtifactLocation{ + S3: &S3Artifact{ + S3Bucket: S3Bucket{Bucket: "s3-bucket"}, + Key: "path/to/s3-artifact", + }, + }, + }, + { + Name: "plugin-artifact-1", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "storage-plugin", + Configuration: `{"endpoint": "https://storage1.com"}`, + Key: "path/to/plugin-artifact1", + }, + }, + }, + { + Name: "plugin-artifact-2", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "backup-plugin", + Configuration: `{"endpoint": "https://backup.com"}`, + Key: "path/to/plugin-artifact2", + }, + }, + }, + } + + defaultRepo := &ArtifactRepository{ + Plugin: &PluginArtifactRepository{ + Name: "log-plugin", + Configuration: `{"endpoint": "https://logs.com"}`, + }, + ArchiveLogs: ptr.To(true), + } + + pluginNames := artifacts.GetPluginNames(ctx, defaultRepo, IncludeLogs) + assert.Len(t, pluginNames, 3) + assert.Contains(t, pluginNames, ArtifactPluginName("storage-plugin")) + assert.Contains(t, pluginNames, ArtifactPluginName("backup-plugin")) + assert.Contains(t, pluginNames, ArtifactPluginName("log-plugin")) + }) + + t.Run("SamePluginMultipleConfigurations", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "plugin-artifact-1", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "my-plugin", + Configuration: `{"bucket": "bucket-1", "region": "us-east-1"}`, + Key: "path/to/artifact1", + }, + }, + }, + { + Name: "plugin-artifact-2", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "my-plugin", + Configuration: `{"bucket": "bucket-2", "region": "us-west-2"}`, + Key: "path/to/artifact2", + }, + }, + }, + { + Name: "plugin-artifact-3", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "other-plugin", + Configuration: `{"endpoint": "https://other.com"}`, + Key: "path/to/artifact3", + }, + }, + }, + } + + pluginNames := artifacts.GetPluginNames(ctx, nil, ExcludeLogs) + // Should only have 2 unique plugin names despite 3 artifacts + assert.Len(t, pluginNames, 2) + assert.Contains(t, pluginNames, ArtifactPluginName("my-plugin")) + assert.Contains(t, pluginNames, ArtifactPluginName("other-plugin")) + }) + + t.Run("ComplexMultiPluginScenario", func(t *testing.T) { + artifacts := Artifacts{ + { + Name: "input-artifact", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "input-plugin", + Configuration: `{"source": "external"}`, + Key: "inputs/data.json", + }, + }, + }, + { + Name: "processing-artifact", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "processing-plugin", + Configuration: `{"temp": true}`, + Key: "temp/processing.dat", + }, + }, + }, + { + Name: "output-artifact", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "output-plugin", + Configuration: `{"destination": "final"}`, + Key: "outputs/result.json", + }, + }, + }, + { + Name: "backup-artifact", + ArtifactLocation: ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "backup-plugin", + Configuration: `{"retention": "30d"}`, + Key: "backups/result-backup.json", + }, + }, + }, + { + Name: "s3-artifact", + ArtifactLocation: ArtifactLocation{ + S3: &S3Artifact{ + S3Bucket: S3Bucket{Bucket: "legacy-bucket"}, + Key: "legacy/data.json", + }, + }, + }, + { + Name: "default-artifact", + ArtifactLocation: ArtifactLocation{ + // Uses default repo + }, + }, + } + + defaultRepo := &ArtifactRepository{ + Plugin: &PluginArtifactRepository{ + Name: "default-plugin", + Configuration: `{"default": true}`, + }, + ArchiveLogs: ptr.To(true), + } + + pluginNames := artifacts.GetPluginNames(ctx, defaultRepo, IncludeLogs) + // Should have 5 unique plugins: input, processing, output, backup, default (for both default artifact and logs) + assert.Len(t, pluginNames, 5) + assert.Contains(t, pluginNames, ArtifactPluginName("input-plugin")) + assert.Contains(t, pluginNames, ArtifactPluginName("processing-plugin")) + assert.Contains(t, pluginNames, ArtifactPluginName("output-plugin")) + assert.Contains(t, pluginNames, ArtifactPluginName("backup-plugin")) + assert.Contains(t, pluginNames, ArtifactPluginName("default-plugin")) + }) +} + +func TestMultiplePluginArtifactRepositories(t *testing.T) { + t.Run("DifferentPluginRepositories", func(t *testing.T) { + repo1 := &PluginArtifactRepository{ + Name: "plugin-1", + KeyFormat: "repo1/{{workflow.name}}/{{pod.name}}", + Configuration: `{"endpoint": "https://repo1.com"}`, + } + + repo2 := &PluginArtifactRepository{ + Name: "plugin-2", + KeyFormat: "repo2/{{workflow.name}}/{{pod.name}}", + Configuration: `{"endpoint": "https://repo2.com"}`, + } + + location1 := &ArtifactLocation{} + repo1.IntoArtifactLocation(location1) + + location2 := &ArtifactLocation{} + repo2.IntoArtifactLocation(location2) + + // Verify both locations are configured correctly + require.NotNil(t, location1.Plugin) + require.NotNil(t, location2.Plugin) + + assert.Equal(t, ArtifactPluginName("plugin-1"), location1.Plugin.Name) + assert.Equal(t, "repo1/{{workflow.name}}/{{pod.name}}", location1.Plugin.Key) + assert.Equal(t, `{"endpoint": "https://repo1.com"}`, location1.Plugin.Configuration) + + assert.Equal(t, ArtifactPluginName("plugin-2"), location2.Plugin.Name) + assert.Equal(t, "repo2/{{workflow.name}}/{{pod.name}}", location2.Plugin.Key) + assert.Equal(t, `{"endpoint": "https://repo2.com"}`, location2.Plugin.Configuration) + }) + + t.Run("SamePluginDifferentConfigurations", func(t *testing.T) { + repo1 := &PluginArtifactRepository{ + Name: "shared-plugin", + KeyFormat: "env1/{{workflow.name}}/{{pod.name}}", + Configuration: `{"environment": "production", "region": "us-east-1"}`, + } + + repo2 := &PluginArtifactRepository{ + Name: "shared-plugin", + KeyFormat: "env2/{{workflow.name}}/{{pod.name}}", + Configuration: `{"environment": "staging", "region": "us-west-2"}`, + } + + location1 := &ArtifactLocation{} + repo1.IntoArtifactLocation(location1) + + location2 := &ArtifactLocation{} + repo2.IntoArtifactLocation(location2) + + // Both should use the same plugin name but different configurations + require.NotNil(t, location1.Plugin) + require.NotNil(t, location2.Plugin) + + assert.Equal(t, ArtifactPluginName("shared-plugin"), location1.Plugin.Name) + assert.Equal(t, ArtifactPluginName("shared-plugin"), location2.Plugin.Name) + + assert.Equal(t, "env1/{{workflow.name}}/{{pod.name}}", location1.Plugin.Key) + assert.Equal(t, "env2/{{workflow.name}}/{{pod.name}}", location2.Plugin.Key) + + assert.Equal(t, `{"environment": "production", "region": "us-east-1"}`, location1.Plugin.Configuration) + assert.Equal(t, `{"environment": "staging", "region": "us-west-2"}`, location2.Plugin.Configuration) + }) +} + +func TestArtifactLocation_MultiplePluginTypes(t *testing.T) { + t.Run("PluginTakesPrecedence", func(t *testing.T) { + // Test that when multiple artifact types are set, the Get() method returns the correct one + // Based on the implementation order in ArtifactLocation.Get() + // Order: Artifactory, Azure, Git, GCS, HDFS, HTTP, OSS, Raw, S3, Plugin + location := &ArtifactLocation{ + S3: &S3Artifact{ + S3Bucket: S3Bucket{Bucket: "s3-bucket"}, + Key: "s3/path", + }, + Plugin: &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "plugin-bucket"}`, + Key: "plugin/path", + }, + } + + artifact, err := location.Get() + assert.NoError(t, err) + // S3 should take precedence over Plugin based on the order in Get() method + assert.IsType(t, &S3Artifact{}, artifact) + + s3Artifact := artifact.(*S3Artifact) + assert.Equal(t, "s3-bucket", s3Artifact.Bucket) + }) + + t.Run("PluginKeyOperations", func(t *testing.T) { + location := &ArtifactLocation{ + Plugin: &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "original/path", + }, + } + + // Test GetKey + key, err := location.GetKey() + assert.NoError(t, err) + assert.Equal(t, "original/path", key) + + // Test SetKey + err = location.SetKey("new/path/to/artifact") + assert.NoError(t, err) + + // Verify the key was updated + newKey, err := location.GetKey() + assert.NoError(t, err) + assert.Equal(t, "new/path/to/artifact", newKey) + assert.Equal(t, "new/path/to/artifact", location.Plugin.Key) + + // Test AppendToKey + err = location.AppendToKey("subdir/file.txt") + assert.NoError(t, err) + + finalKey, err := location.GetKey() + assert.NoError(t, err) + assert.Equal(t, "new/path/to/artifact/subdir/file.txt", finalKey) + }) } func TestTemplate_PluginType(t *testing.T) { From 4dba3a114ecfcd9233d2fec6c3678f99020c6bef Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Thu, 18 Sep 2025 14:17:35 +0200 Subject: [PATCH 3/9] remove unnecessary Signed-off-by: J.P. Zivalich --- .../v1alpha1/artifact_plugins_test.go | 157 ------------------ 1 file changed, 157 deletions(-) diff --git a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go index ca19f88f9d0f..ebfd7592233b 100644 --- a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go +++ b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go @@ -194,11 +194,6 @@ func TestArtifactLocation_Plugin(t *testing.T) { artifact, err := location.Get() assert.NoError(t, err) assert.IsType(t, &PluginArtifact{}, artifact) - - pluginArtifact := artifact.(*PluginArtifact) - assert.Equal(t, ArtifactPluginName("test-plugin"), pluginArtifact.Name) - assert.Equal(t, `{"bucket": "my-bucket"}`, pluginArtifact.Configuration) - assert.Equal(t, "path/to/artifact", pluginArtifact.Key) }) t.Run("SetType_Plugin", func(t *testing.T) { @@ -784,156 +779,4 @@ func TestArtifactLocation_MultiplePluginTypes(t *testing.T) { s3Artifact := artifact.(*S3Artifact) assert.Equal(t, "s3-bucket", s3Artifact.Bucket) }) - - t.Run("PluginKeyOperations", func(t *testing.T) { - location := &ArtifactLocation{ - Plugin: &PluginArtifact{ - Name: "test-plugin", - Configuration: `{"bucket": "my-bucket"}`, - Key: "original/path", - }, - } - - // Test GetKey - key, err := location.GetKey() - assert.NoError(t, err) - assert.Equal(t, "original/path", key) - - // Test SetKey - err = location.SetKey("new/path/to/artifact") - assert.NoError(t, err) - - // Verify the key was updated - newKey, err := location.GetKey() - assert.NoError(t, err) - assert.Equal(t, "new/path/to/artifact", newKey) - assert.Equal(t, "new/path/to/artifact", location.Plugin.Key) - - // Test AppendToKey - err = location.AppendToKey("subdir/file.txt") - assert.NoError(t, err) - - finalKey, err := location.GetKey() - assert.NoError(t, err) - assert.Equal(t, "new/path/to/artifact/subdir/file.txt", finalKey) - }) -} - -func TestTemplate_PluginType(t *testing.T) { - t.Run("GetType_Plugin", func(t *testing.T) { - tmpl := &Template{ - Plugin: &Plugin{ - Object: Object{ - Value: []byte(`{"my-plugin": {"config": "value"}}`), - }, - }, - } - assert.Equal(t, TemplateTypePlugin, tmpl.GetType()) - }) - - t.Run("GetNodeType_Plugin", func(t *testing.T) { - tmpl := &Template{ - Plugin: &Plugin{ - Object: Object{ - Value: []byte(`{"my-plugin": {"config": "value"}}`), - }, - }, - } - assert.Equal(t, NodeTypePlugin, tmpl.GetNodeType()) - }) - - t.Run("IsLeaf_Plugin", func(t *testing.T) { - tmpl := &Template{ - Plugin: &Plugin{ - Object: Object{ - Value: []byte(`{"my-plugin": {"config": "value"}}`), - }, - }, - } - assert.True(t, tmpl.IsLeaf()) - }) - - t.Run("IsPodType_Plugin", func(t *testing.T) { - tmpl := &Template{ - Plugin: &Plugin{ - Object: Object{ - Value: []byte(`{"my-plugin": {"config": "value"}}`), - }, - }, - } - assert.False(t, tmpl.IsPodType()) - }) - - t.Run("HasOutput_Plugin", func(t *testing.T) { - tmpl := &Template{ - Plugin: &Plugin{ - Object: Object{ - Value: []byte(`{"my-plugin": {"config": "value"}}`), - }, - }, - } - assert.True(t, tmpl.HasOutput()) - }) -} - -func TestNodeStatus_PluginType(t *testing.T) { - t.Run("IsTaskSetNode_Plugin", func(t *testing.T) { - node := &NodeStatus{ - Type: NodeTypePlugin, - } - assert.True(t, node.IsTaskSetNode()) - }) - - t.Run("IsTaskSetNode_HTTP", func(t *testing.T) { - node := &NodeStatus{ - Type: NodeTypeHTTP, - } - assert.True(t, node.IsTaskSetNode()) - }) - - t.Run("IsTaskSetNode_Pod", func(t *testing.T) { - node := &NodeStatus{ - Type: NodeTypePod, - } - assert.False(t, node.IsTaskSetNode()) - }) -} - -func TestArtifactRepository_Plugin_Integration(t *testing.T) { - t.Run("Get_Plugin", func(t *testing.T) { - repo := &ArtifactRepository{ - Plugin: &PluginArtifactRepository{ - Name: "test-plugin", - KeyFormat: "custom/{{workflow.name}}/{{pod.name}}", - Configuration: `{"endpoint": "https://my-storage.com"}`, - }, - } - - repoType := repo.Get() - assert.IsType(t, &PluginArtifactRepository{}, repoType) - - pluginRepo := repoType.(*PluginArtifactRepository) - assert.Equal(t, ArtifactPluginName("test-plugin"), pluginRepo.Name) - assert.Equal(t, "custom/{{workflow.name}}/{{pod.name}}", pluginRepo.KeyFormat) - assert.Equal(t, `{"endpoint": "https://my-storage.com"}`, pluginRepo.Configuration) - }) - - t.Run("ToArtifactLocation_Plugin", func(t *testing.T) { - repo := &ArtifactRepository{ - Plugin: &PluginArtifactRepository{ - Name: "test-plugin", - KeyFormat: "custom/{{workflow.name}}/{{pod.name}}", - Configuration: `{"endpoint": "https://my-storage.com"}`, - }, - ArchiveLogs: ptr.To(true), - } - - location := repo.ToArtifactLocation() - require.NotNil(t, location) - assert.Equal(t, ptr.To(true), location.ArchiveLogs) - require.NotNil(t, location.Plugin) - assert.Equal(t, ArtifactPluginName("test-plugin"), location.Plugin.Name) - assert.Equal(t, "custom/{{workflow.name}}/{{pod.name}}", location.Plugin.Key) - assert.Equal(t, `{"endpoint": "https://my-storage.com"}`, location.Plugin.Configuration) - }) } From da7d6f1fc00de4993b4855dc6a10a96bbf7809ac Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Thu, 18 Sep 2025 15:18:28 +0200 Subject: [PATCH 4/9] remove more unneeded Signed-off-by: J.P. Zivalich --- .../v1alpha1/artifact_plugins_test.go | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go index ebfd7592233b..292b9da2969e 100644 --- a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go +++ b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go @@ -232,34 +232,6 @@ func TestArtifactLocation_Plugin(t *testing.T) { } assert.False(t, location.HasLocation()) }) - - t.Run("GetKey_Plugin", func(t *testing.T) { - location := &ArtifactLocation{ - Plugin: &PluginArtifact{ - Name: "test-plugin", - Configuration: `{"bucket": "my-bucket"}`, - Key: "path/to/artifact", - }, - } - - key, err := location.GetKey() - assert.NoError(t, err) - assert.Equal(t, "path/to/artifact", key) - }) - - t.Run("SetKey_Plugin", func(t *testing.T) { - location := &ArtifactLocation{ - Plugin: &PluginArtifact{ - Name: "test-plugin", - Configuration: `{"bucket": "my-bucket"}`, - Key: "old/path", - }, - } - - err := location.SetKey("new/path/to/artifact") - assert.NoError(t, err) - assert.Equal(t, "new/path/to/artifact", location.Plugin.Key) - }) } func TestArtifacts_GetPluginNames(t *testing.T) { From a9f8e47a4826d349af259fb0a1b46226cf3f69a4 Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Thu, 18 Sep 2025 15:33:41 +0200 Subject: [PATCH 5/9] remove unneeded Signed-off-by: J.P. Zivalich --- .../workflow/v1alpha1/artifact_plugins_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go index 292b9da2969e..c0708e4493ed 100644 --- a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go +++ b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go @@ -182,20 +182,6 @@ func TestPluginArtifactRepository(t *testing.T) { } func TestArtifactLocation_Plugin(t *testing.T) { - t.Run("Get_Plugin", func(t *testing.T) { - location := &ArtifactLocation{ - Plugin: &PluginArtifact{ - Name: "test-plugin", - Configuration: `{"bucket": "my-bucket"}`, - Key: "path/to/artifact", - }, - } - - artifact, err := location.Get() - assert.NoError(t, err) - assert.IsType(t, &PluginArtifact{}, artifact) - }) - t.Run("SetType_Plugin", func(t *testing.T) { location := &ArtifactLocation{} pluginArtifact := &PluginArtifact{ From edee9832b071e30435f95c63dd7e6c843ce162e9 Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Thu, 18 Sep 2025 15:34:50 +0200 Subject: [PATCH 6/9] remove more Signed-off-by: J.P. Zivalich --- .../v1alpha1/artifact_plugins_test.go | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go index c0708e4493ed..29311506e6c3 100644 --- a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go +++ b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go @@ -59,28 +59,6 @@ func TestArtifactPluginName(t *testing.T) { } func TestPluginArtifact(t *testing.T) { - t.Run("GetKey", func(t *testing.T) { - plugin := &PluginArtifact{ - Name: "test-plugin", - Configuration: `{"bucket": "my-bucket"}`, - Key: "path/to/artifact", - } - key, err := plugin.GetKey() - assert.NoError(t, err) - assert.Equal(t, "path/to/artifact", key) - }) - - t.Run("SetKey", func(t *testing.T) { - plugin := &PluginArtifact{ - Name: "test-plugin", - Configuration: `{"bucket": "my-bucket"}`, - Key: "old/path", - } - err := plugin.SetKey("new/path/to/artifact") - assert.NoError(t, err) - assert.Equal(t, "new/path/to/artifact", plugin.Key) - }) - t.Run("HasLocation_Complete", func(t *testing.T) { plugin := &PluginArtifact{ Name: "test-plugin", From f693efe56ae8d0aacbc02f905523c0a8d7b5f5d2 Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Thu, 18 Sep 2025 15:44:31 +0200 Subject: [PATCH 7/9] move SetType Signed-off-by: J.P. Zivalich --- .../workflow/v1alpha1/artifact_plugins_test.go | 15 --------------- pkg/apis/workflow/v1alpha1/workflow_types_test.go | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go index 29311506e6c3..1fcbbe164417 100644 --- a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go +++ b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go @@ -160,21 +160,6 @@ func TestPluginArtifactRepository(t *testing.T) { } func TestArtifactLocation_Plugin(t *testing.T) { - t.Run("SetType_Plugin", func(t *testing.T) { - location := &ArtifactLocation{} - pluginArtifact := &PluginArtifact{ - Name: "test-plugin", - Configuration: `{"bucket": "my-bucket"}`, - Key: "path/to/artifact", - } - - err := location.SetType(pluginArtifact) - assert.NoError(t, err) - assert.NotNil(t, location.Plugin) - // Note: SetType creates a new empty instance, not copying the values - assert.Equal(t, ArtifactPluginName(""), location.Plugin.Name) - }) - t.Run("HasLocation_Plugin", func(t *testing.T) { location := &ArtifactLocation{ Plugin: &PluginArtifact{ diff --git a/pkg/apis/workflow/v1alpha1/workflow_types_test.go b/pkg/apis/workflow/v1alpha1/workflow_types_test.go index c46adbe2e4c5..2548034cfed6 100644 --- a/pkg/apis/workflow/v1alpha1/workflow_types_test.go +++ b/pkg/apis/workflow/v1alpha1/workflow_types_test.go @@ -518,6 +518,21 @@ func TestArtifactLocation_SetType(t *testing.T) { require.NoError(t, l.SetType(&AzureArtifact{})) assert.NotNil(t, l.Azure) }) + + t.Run("SetType_Plugin", func(t *testing.T) { + location := &ArtifactLocation{} + pluginArtifact := &PluginArtifact{ + Name: "test-plugin", + Configuration: `{"bucket": "my-bucket"}`, + Key: "path/to/artifact", + } + + err := location.SetType(pluginArtifact) + assert.NoError(t, err) + assert.NotNil(t, location.Plugin) + // Note: SetType creates a new empty instance, not copying the values + assert.Equal(t, ArtifactPluginName(""), location.Plugin.Name) + }) } func TestArtifactLocation_Key(t *testing.T) { From a454bbf557fda8eee686bed89549875382b0779f Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Tue, 30 Sep 2025 10:42:44 +0200 Subject: [PATCH 8/9] update for pr comments Signed-off-by: J.P. Zivalich --- .../workflow/v1alpha1/artifact_plugins_test.go | 9 ++------- .../workflow/v1alpha1/workflow_types_test.go | 18 ++++-------------- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go index 1fcbbe164417..ee7951f9b6a6 100644 --- a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go +++ b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go @@ -692,12 +692,7 @@ func TestArtifactLocation_MultiplePluginTypes(t *testing.T) { }, } - artifact, err := location.Get() - assert.NoError(t, err) - // S3 should take precedence over Plugin based on the order in Get() method - assert.IsType(t, &S3Artifact{}, artifact) - - s3Artifact := artifact.(*S3Artifact) - assert.Equal(t, "s3-bucket", s3Artifact.Bucket) + _, err := location.Get() + assert.Error(t, err) }) } diff --git a/pkg/apis/workflow/v1alpha1/workflow_types_test.go b/pkg/apis/workflow/v1alpha1/workflow_types_test.go index 2548034cfed6..1a032a70d577 100644 --- a/pkg/apis/workflow/v1alpha1/workflow_types_test.go +++ b/pkg/apis/workflow/v1alpha1/workflow_types_test.go @@ -518,20 +518,10 @@ func TestArtifactLocation_SetType(t *testing.T) { require.NoError(t, l.SetType(&AzureArtifact{})) assert.NotNil(t, l.Azure) }) - - t.Run("SetType_Plugin", func(t *testing.T) { - location := &ArtifactLocation{} - pluginArtifact := &PluginArtifact{ - Name: "test-plugin", - Configuration: `{"bucket": "my-bucket"}`, - Key: "path/to/artifact", - } - - err := location.SetType(pluginArtifact) - assert.NoError(t, err) - assert.NotNil(t, location.Plugin) - // Note: SetType creates a new empty instance, not copying the values - assert.Equal(t, ArtifactPluginName(""), location.Plugin.Name) + t.Run("Plugin", func(t *testing.T) { + l := &ArtifactLocation{} + require.NoError(t, l.SetType(&PluginArtifact{})) + assert.NotNil(t, l.Plugin) }) } From 9bbb9728bddf4cb2a5a32b26e3ce16127193be00 Mon Sep 17 00:00:00 2001 From: "J.P. Zivalich" Date: Tue, 30 Sep 2025 10:44:33 +0200 Subject: [PATCH 9/9] remove precedence test Signed-off-by: J.P. Zivalich --- .../v1alpha1/artifact_plugins_test.go | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go index ee7951f9b6a6..6ff03fd43b4e 100644 --- a/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go +++ b/pkg/apis/workflow/v1alpha1/artifact_plugins_test.go @@ -674,25 +674,3 @@ func TestMultiplePluginArtifactRepositories(t *testing.T) { assert.Equal(t, `{"environment": "staging", "region": "us-west-2"}`, location2.Plugin.Configuration) }) } - -func TestArtifactLocation_MultiplePluginTypes(t *testing.T) { - t.Run("PluginTakesPrecedence", func(t *testing.T) { - // Test that when multiple artifact types are set, the Get() method returns the correct one - // Based on the implementation order in ArtifactLocation.Get() - // Order: Artifactory, Azure, Git, GCS, HDFS, HTTP, OSS, Raw, S3, Plugin - location := &ArtifactLocation{ - S3: &S3Artifact{ - S3Bucket: S3Bucket{Bucket: "s3-bucket"}, - Key: "s3/path", - }, - Plugin: &PluginArtifact{ - Name: "test-plugin", - Configuration: `{"bucket": "plugin-bucket"}`, - Key: "plugin/path", - }, - } - - _, err := location.Get() - assert.Error(t, err) - }) -}