Merge pull request #151 from buildkite-plugins/SUP-6058-artifact-paths-field-support

Support both 'artifacts' and 'artifact_paths' fields

Author: mcncl

CHANGELOG.md

@@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Added
+* Accept both `artifact_paths` (preferred) and `artifacts` (alternative) field names in plugin configuration for backward compatibility (#120)
+
+### Fixed
+* Fix `artifact_paths` field being ignored when specified in plugin configuration. Both `artifact_paths` and `artifacts` now work correctly (#120)
+* Fix typo in test case: "artifiact" → "artifact"
+* Fix BATS test failures caused by unbound `BUILDKITE_PLUGINS_PATH` variable in hooks/command
+
 ## [v1.6.0]
 
 ### Added

README.md

@@ -449,6 +449,26 @@ steps:
           wait: true
 ```
 
+**Note:** This plugin accepts both `artifact_paths` and `artifacts` field names for backward compatibility:
+
+```yaml
+# Preferred - use "artifact_paths":
+- path: "app/"
+  config:
+    command: "npm test"
+    artifact_paths:
+      - "logs/**/*"
+
+# Also supported - "artifacts":
+- path: "app/"
+  config:
+    command: "npm test"
+    artifacts:
+      - "logs/**/*"
+```
+
+Both field names are supported by Buildkite. The generated pipeline YAML will use `artifact_paths`.
+
 ### `binary_folder` (optional)
 
 Default: `BUILDKITE_PLUGINS_PATH`

hooks/command

@@ -17,7 +17,7 @@ err() {
   exit 1
 }
 
-executable_dir="${BUILDKITE_PLUGIN_MONOREPO_DIFF_BINARY_FOLDER:-${BUILDKITE_PLUGINS_PATH}}"
+executable_dir="${BUILDKITE_PLUGIN_MONOREPO_DIFF_BINARY_FOLDER:-${BUILDKITE_PLUGINS_PATH:-}}"
 
 # Default to current directory if not set
 if [[ -z "${executable_dir}" ||  ${BUILDKITE_PLUGIN_MONOREPO_DIFF_BUILDKITE_PLUGIN_TEST_MODE:-false} == "true" ]]; then

plugin.go

@@ -76,27 +76,66 @@ type StepNotify struct {
 
 // Step is buildkite pipeline definition
 type Step struct {
-	Group     string                   `yaml:"group,omitempty"`
-	Trigger   string                   `yaml:"trigger,omitempty"`
-	Label     string                   `yaml:"label,omitempty"`
-	Branches  string                   `yaml:"branches,omitempty"`
-	Condition string                   `json:"if,omitempty" yaml:"if,omitempty"`
-	Build     Build                    `yaml:"build,omitempty"`
-	Command   interface{}              `yaml:"command,omitempty"`
-	Commands  interface{}              `yaml:"commands,omitempty"`
-	Agents    Agent                    `yaml:"agents,omitempty"`
-	Artifacts []string                 `yaml:"artifacts,omitempty"`
-	RawEnv    interface{}              `json:"env" yaml:",omitempty"`
-	Plugins   []map[string]interface{} `json:"plugins,omitempty" yaml:"plugins,omitempty"`
-	Env       map[string]string        `yaml:"env,omitempty"`
-	Async     bool                     `yaml:"async,omitempty"`
-	SoftFail  interface{}              `json:"soft_fail" yaml:"soft_fail,omitempty"`
-	RawNotify []map[string]interface{} `json:"notify" yaml:",omitempty"`
-	Notify    []StepNotify             `yaml:"notify,omitempty"`
-	DependsOn interface{}              `json:"depends_on" yaml:"depends_on,omitempty"`
-	Key       string                   `yaml:"key,omitempty"`
-	Secrets   interface{}              `json:"secrets,omitempty" yaml:"secrets,omitempty"`
-	Steps     []Step                   `yaml:"steps,omitempty"`
+	Group         string                   `yaml:"group,omitempty"`
+	Trigger       string                   `yaml:"trigger,omitempty"`
+	Label         string                   `yaml:"label,omitempty"`
+	Branches      string                   `yaml:"branches,omitempty"`
+	Condition     string                   `json:"if,omitempty" yaml:"if,omitempty"`
+	Build         Build                    `yaml:"build,omitempty"`
+	Command       interface{}              `yaml:"command,omitempty"`
+	Commands      interface{}              `yaml:"commands,omitempty"`
+	Agents        Agent                    `yaml:"agents,omitempty"`
+	ArtifactPaths []string                 `json:"artifact_paths" yaml:"artifact_paths,omitempty"`
+	RawEnv        interface{}              `json:"env" yaml:",omitempty"`
+	Plugins       []map[string]interface{} `json:"plugins,omitempty" yaml:"plugins,omitempty"`
+	Env           map[string]string        `yaml:"env,omitempty"`
+	Async         bool                     `yaml:"async,omitempty"`
+	SoftFail      interface{}              `json:"soft_fail" yaml:"soft_fail,omitempty"`
+	RawNotify     []map[string]interface{} `json:"notify" yaml:",omitempty"`
+	Notify        []StepNotify             `yaml:"notify,omitempty"`
+	DependsOn     interface{}              `json:"depends_on" yaml:"depends_on,omitempty"`
+	Key           string                   `yaml:"key,omitempty"`
+	Secrets       interface{}              `json:"secrets,omitempty" yaml:"secrets,omitempty"`
+	Steps         []Step                   `yaml:"steps,omitempty"`
+}
+
+// UnmarshalJSON handles both "artifacts" and "artifact_paths" field names for backward compatibility
+// Both fields are supported by the Buildkite API; "artifact_paths" is preferred per documentation
+func (step *Step) UnmarshalJSON(data []byte) error {
+	// Check which fields are present without full unmarshaling
+	var fieldCheck map[string]json.RawMessage
+	if err := json.Unmarshal(data, &fieldCheck); err != nil {
+		return err
+	}
+
+	_, hasArtifacts := fieldCheck["artifacts"]
+	_, hasArtifactPaths := fieldCheck["artifact_paths"]
+
+	// Validate that both fields are not specified
+	if hasArtifacts && hasArtifactPaths {
+		return errors.New("cannot specify both 'artifacts' and 'artifact_paths'; please use 'artifact_paths'")
+	}
+
+	// Use a type alias to avoid infinite recursion
+	type stepAlias Step
+
+	// Unmarshal the main struct (this will populate artifact_paths if present)
+	if err := json.Unmarshal(data, (*stepAlias)(step)); err != nil {
+		return err
+	}
+
+	// If only "artifacts" was specified, manually extract and use it
+	if hasArtifacts && !hasArtifactPaths {
+		var temp struct {
+			Artifacts []string `json:"artifacts"`
+		}
+		if err := json.Unmarshal(data, &temp); err != nil {
+			return err
+		}
+		step.ArtifactPaths = temp.Artifacts
+	}
+
+	return nil
 }
 
 // Agent is Buildkite agent definition
@@ -124,7 +163,9 @@ func (plugin *Plugin) UnmarshalJSON(data []byte) error {
 		Interpolation: true,
 	}
 
-	_ = json.Unmarshal(data, def)
+	if err := json.Unmarshal(data, def); err != nil {
+		return err
+	}
 
 	*plugin = Plugin(*def)
 
@@ -235,7 +276,7 @@ func initializePlugin(data string) (Plugin, error) {
 
 				if err := json.Unmarshal(pluginConfig, &plugin); err != nil {
 					log.Debug(err)
-					return Plugin{}, errors.New("failed to parse plugin configuration")
+					return Plugin{}, err
 				}
 
 				return plugin, nil

plugin.yml

@@ -94,6 +94,10 @@ configuration:
                 type: string
             artifacts:
               type: array
+              description: Artifact paths to upload (alternative field name)
+            artifact_paths:
+              type: array
+              description: Artifact paths to upload (preferred field name)
             env:
               type: array
     wait:

plugin_test.go

@@ -154,7 +154,7 @@ func TestPluginShouldUnmarshallCorrectly(t *testing.T) {
 							"queue": "queue-1",
 							"database": "postgres"
 						},
-						"artifacts": [ "artifiact-1" ],
+						"artifacts": [ "artifact-1" ],
 						"soft_fail": [{
 							"exit_status": 127
 						}]
@@ -266,9 +266,9 @@ func TestPluginShouldUnmarshallCorrectly(t *testing.T) {
 							"env3": "env-3",
 						},
 					},
-					Async:     true,
-					Agents:    map[string]string{"queue": "queue-1", "database": "postgres"},
-					Artifacts: []string{"artifiact-1"},
+					Async:         true,
+					Agents:        map[string]string{"queue": "queue-1", "database": "postgres"},
+					ArtifactPaths: []string{"artifact-1"},
 					SoftFail: []interface{}{map[string]interface{}{
 						"exit_status": float64(127),
 					}},
@@ -1221,3 +1221,65 @@ func TestPluginShouldPreserveSecretsInNestedSteps(t *testing.T) {
 	assert.True(t, ok, "second step secrets should be an array")
 	assert.Equal(t, []interface{}{"PROD_DB_HOST", "PROD_DB_PASS"}, secretsArray)
 }
+
+func TestPluginAcceptsArtifactPathsFieldName(t *testing.T) {
+	// Test that artifact_paths field name works
+	param := `[{
+		"github.com/buildkite-plugins/monorepo-diff-buildkite-plugin#commit": {
+			"watch": [
+				{
+					"path": "service/**/*",
+					"config": {
+						"command": "echo test",
+						"artifact_paths": ["logs/**/*", "coverage/**/*"]
+					}
+				}
+			]
+		}
+	}]`
+
+	got, err := initializePlugin(param)
+	assert.NoError(t, err)
+
+	expected := Plugin{
+		Diff:          "git diff --name-only HEAD~1",
+		Wait:          false,
+		LogLevel:      "info",
+		Interpolation: true,
+		Watch: []WatchConfig{
+			{
+				Paths: []string{"service/**/*"},
+				Step: Step{
+					Command:       "echo test",
+					ArtifactPaths: []string{"logs/**/*", "coverage/**/*"},
+				},
+			},
+		},
+	}
+
+	if diff := cmp.Diff(expected, got); diff != "" {
+		t.Fatalf("plugin diff (-want +got):\n%s", diff)
+	}
+}
+
+func TestPluginRejectsBothArtifactsFields(t *testing.T) {
+	// Test that specifying both fields returns an error
+	param := `[{
+		"github.com/buildkite-plugins/monorepo-diff-buildkite-plugin#commit": {
+			"watch": [
+				{
+					"path": "service/**/*",
+					"config": {
+						"command": "echo test",
+						"artifacts": ["old.log"],
+						"artifact_paths": ["new.log"]
+					}
+				}
+			]
+		}
+	}]`
+
+	_, err := initializePlugin(param)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "cannot specify both 'artifacts' and 'artifact_paths'")
+}