Merge pull request #18 from buildkite/pdp-2010-alias-keys

buildkite · Dec 14, 2023 · 943d581 · 943d581
2 parents 6df52d1 + bbb7c4a
commit 943d581
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 21 deletions.
diff --git a/ordered/yaml.go b/ordered/yaml.go
@@ -234,27 +234,37 @@ func rangeYAMLMapImpl(merged map[*yaml.Node]bool, n *yaml.Node, f func(key strin
 // 0xb and 11, to be equivalent, and therefore a duplicate key. JSON requires
 // all keys to be strings.
 func canonicalMapKey(n *yaml.Node) (string, error) {
-	var x any
-	if err := n.Decode(&x); err != nil {
-		return "", err
-	}
-	if x == nil || n.Tag == "!!null" {
-		// Nulls are not valid JSON keys.
-		return "", fmt.Errorf("line %d, col %d: null not supported as a map key", n.Line, n.Column)
-	}
-	switch n.Tag {
-	case "!!bool":
-		// Canonicalise to true or false.
-		return fmt.Sprintf("%t", x), nil
-	case "!!int":
-		// Canonicalise to decimal.
-		return fmt.Sprintf("%d", x), nil
-	case "!!float":
-		// Canonicalise to scientific notation.
-		// Don't handle Inf or NaN specially, as they will be quoted.
-		return fmt.Sprintf("%e", x), nil
+	switch n.Kind {
+	case yaml.AliasNode:
+		return canonicalMapKey(n.Alias)
+
+	case yaml.ScalarNode:
+		var x any
+		if err := n.Decode(&x); err != nil {
+			return "", err
+		}
+		if x == nil || n.Tag == "!!null" {
+			// Nulls are not valid JSON keys.
+			return "", fmt.Errorf("line %d, col %d: null not supported as a map key", n.Line, n.Column)
+		}
+		switch n.Tag {
+		case "!!bool":
+			// Canonicalise to true or false.
+			return fmt.Sprintf("%t", x), nil
+		case "!!int":
+			// Canonicalise to decimal.
+			return fmt.Sprintf("%d", x), nil
+		case "!!float":
+			// Canonicalise to scientific notation.
+			// Don't handle Inf or NaN specially, as they will be quoted.
+			return fmt.Sprintf("%e", x), nil
+		default:
+			// Assume the value is already a suitable key.
+			return n.Value, nil
+		}
+
 	default:
-		// Assume the value is already a suitable key.
-		return n.Value, nil
+		// TODO: Use %v once yaml.Kind has a String method
+		return "", fmt.Errorf("line %d, col %d: cannot use node kind %x as a map key", n.Line, n.Column, n.Kind)
 	}
 }
diff --git a/parser_test.go b/parser_test.go
@@ -248,6 +248,81 @@ steps:
 	}
 }
 
+func TestParserSupportsYAMLAliasesAsKeys(t *testing.T) {
+	const complexYAML = `---
+common_params:
+  # Common versioned attributes
+  - &docker_version "docker#v5.8.0"
+  - &ruby_image "public.ecr.aws/docker/library/ruby:3.2.2"
+
+steps:
+  - label: "Do the thing"
+    command: "whoami"
+    plugins:
+      - *docker_version :
+          image: *ruby_image`
+
+	input := strings.NewReader(complexYAML)
+	got, err := Parse(input)
+	if err != nil {
+		t.Fatalf("Parse(input) error = %v", err)
+	}
+
+	want := &Pipeline{
+		Steps: Steps{
+			&CommandStep{
+				Label:   "Do the thing",
+				Command: "whoami",
+				Plugins: Plugins{
+					{
+						Source: "docker#v5.8.0",
+						Config: map[string]any{
+							"image": "public.ecr.aws/docker/library/ruby:3.2.2",
+						},
+					},
+				},
+			},
+		},
+		RemainingFields: map[string]any{
+			"common_params": []any{
+				"docker#v5.8.0",
+				"public.ecr.aws/docker/library/ruby:3.2.2",
+			},
+		},
+	}
+	if diff := cmp.Diff(got, want, cmp.Comparer(ordered.EqualSA)); diff != "" {
+		t.Errorf("parsed pipeline diff (-got +want):\n%s", diff)
+	}
+
+	gotJSON, err := json.MarshalIndent(got, "", "  ")
+	if err != nil {
+		t.Fatalf(`json.MarshalIndent(got, "", "  ") error = %v`, err)
+	}
+
+	const wantJSON = `{
+  "common_params": [
+    "docker#v5.8.0",
+    "public.ecr.aws/docker/library/ruby:3.2.2"
+  ],
+  "steps": [
+    {
+      "command": "whoami",
+      "label": "Do the thing",
+      "plugins": [
+        {
+          "github.com/buildkite-plugins/docker-buildkite-plugin#v5.8.0": {
+            "image": "public.ecr.aws/docker/library/ruby:3.2.2"
+          }
+        }
+      ]
+    }
+  ]
+}`
+	if diff := cmp.Diff(string(gotJSON), wantJSON); diff != "" {
+		t.Errorf("marshalled JSON diff (-got +want):\n%s", diff)
+	}
+}
+
 func TestParserSupportsDoubleMerge(t *testing.T) {
 	t.Parallel()