From 6dce629c7a17423f13ec6ab0e983ab053fadf007 Mon Sep 17 00:00:00 2001 From: Anatolii Stehnii Date: Mon, 7 Sep 2026 17:38:40 +0300 Subject: [PATCH 1/2] Decode versions in order to install key dependencies first --- internal/tools/versions.go | 30 ++++++++++++++++++----- internal/tools/versions_test.go | 43 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/internal/tools/versions.go b/internal/tools/versions.go index f50110ad9..79815f6f3 100644 --- a/internal/tools/versions.go +++ b/internal/tools/versions.go @@ -24,8 +24,6 @@ import ( "gopkg.in/yaml.v3" ) -type versionsYaml map[string]Prerequisite - func GetRequirements() ([]Prerequisite, []string, error) { repoRoot, err := repo.FindRepoRoot() if err != nil { @@ -45,17 +43,37 @@ func GetRequirementsFromDir(dir string) ([]Prerequisite, []string, error) { return nil, nil, fmt.Errorf("Failed to read versions yaml file %s: %w", yamlFile, err) } - var fileParsed versionsYaml + // Decoded via yaml.Node (not a map[string]Prerequisite) to preserve the file's + // declaration order: dependency installation runs in this order, and tools like + // pulumi-datarobot's install command need pulumi already installed, so map + // iteration's randomized order would install prerequisites out of order. + var root yaml.Node - if err = yaml.Unmarshal(data, &fileParsed); err != nil { + if err = yaml.Unmarshal(data, &root); err != nil { return nil, nil, fmt.Errorf("Failed to unmarshal versions yaml file %s: %w", yamlFile, err) } + if len(root.Content) == 0 { + return nil, nil, nil + } + + mapping := root.Content[0] + if mapping.Kind != yaml.MappingNode { + return nil, nil, fmt.Errorf("versions yaml file %s must contain a top-level mapping", yamlFile) + } + var violations []string - versions := make([]Prerequisite, 0, len(fileParsed)) + versions := make([]Prerequisite, 0, len(mapping.Content)/2) + + for i := 0; i+1 < len(mapping.Content); i += 2 { + key := mapping.Content[i].Value + + var version Prerequisite + if err := mapping.Content[i+1].Decode(&version); err != nil { + return nil, nil, fmt.Errorf("Failed to unmarshal entry %q in %s: %w", key, yamlFile, err) + } - for key, version := range fileParsed { version.Key = key violations = append(violations, validatePrerequisite(key, version)...) versions = append(versions, version) diff --git a/internal/tools/versions_test.go b/internal/tools/versions_test.go index 9c67dead2..4b4532892 100644 --- a/internal/tools/versions_test.go +++ b/internal/tools/versions_test.go @@ -130,6 +130,49 @@ tool-b: assert.Len(t, prereqs, 2) } +func TestGetRequirementsFromDir_PreservesDeclarationOrder(t *testing.T) { + // Regression test: entries were previously decoded into a map, which randomizes + // iteration order and could install prerequisites (e.g. a plugin before the tool + // it depends on) out of the order declared in versions.yaml. + const yaml = `tool-z: + name: Tool Z + minimum-version: "1.0.0" + command: "echo z" + url: https://example.com/z + install: + macos: "echo install" + linux: "echo install" +tool-a: + name: Tool A + minimum-version: "1.0.0" + command: "echo a" + url: https://example.com/a + install: + macos: "echo install" + linux: "echo install" +tool-m: + name: Tool M + minimum-version: "1.0.0" + command: "echo m" + url: https://example.com/m + install: + macos: "echo install" + linux: "echo install" +` + + dir := t.TempDir() + + writeVersionsYAML(t, dir, yaml) + + for range 20 { + prereqs, _, err := GetRequirementsFromDir(dir) + + require.NoError(t, err) + require.Len(t, prereqs, 3) + assert.Equal(t, []string{"tool-z", "tool-a", "tool-m"}, []string{prereqs[0].Key, prereqs[1].Key, prereqs[2].Key}) + } +} + func TestGetRequirementsFromDir_EmptyYamlReturnsNoPrereqs(t *testing.T) { dir := t.TempDir() From 649145645fdcae9495f6f713aaed77e7327cf544 Mon Sep 17 00:00:00 2001 From: Anatolii Stehnii Date: Wed, 9 Sep 2026 15:24:53 +0300 Subject: [PATCH 2/2] Duplicate key handling --- internal/tools/versions.go | 9 +++++++++ internal/tools/versions_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/internal/tools/versions.go b/internal/tools/versions.go index 79815f6f3..a87827dfb 100644 --- a/internal/tools/versions.go +++ b/internal/tools/versions.go @@ -65,10 +65,19 @@ func GetRequirementsFromDir(dir string) ([]Prerequisite, []string, error) { var violations []string versions := make([]Prerequisite, 0, len(mapping.Content)/2) + seen := make(map[string]bool, len(mapping.Content)/2) for i := 0; i+1 < len(mapping.Content); i += 2 { key := mapping.Content[i].Value + // yaml.Node traversal doesn't reject duplicate keys the way decoding into a + // map does, so check explicitly to keep that same fail-fast behavior. + if seen[key] { + return nil, nil, fmt.Errorf("versions yaml file %s: duplicate key %q (line %d)", yamlFile, key, mapping.Content[i].Line) + } + + seen[key] = true + var version Prerequisite if err := mapping.Content[i+1].Decode(&version); err != nil { return nil, nil, fmt.Errorf("Failed to unmarshal entry %q in %s: %w", key, yamlFile, err) diff --git a/internal/tools/versions_test.go b/internal/tools/versions_test.go index 4b4532892..264cafaae 100644 --- a/internal/tools/versions_test.go +++ b/internal/tools/versions_test.go @@ -173,6 +173,37 @@ tool-m: } } +func TestGetRequirementsFromDir_DuplicateKeyReturnsError(t *testing.T) { + const yaml = `tool-a: + name: Tool A + minimum-version: "1.0.0" + command: "echo a" + url: https://example.com/a + install: + macos: "echo install" + linux: "echo install" +tool-a: + name: Tool A Again + minimum-version: "2.0.0" + command: "echo a2" + url: https://example.com/a2 + install: + macos: "echo install" + linux: "echo install" +` + + dir := t.TempDir() + + writeVersionsYAML(t, dir, yaml) + + prereqs, _, err := GetRequirementsFromDir(dir) + + require.Error(t, err) + assert.Contains(t, err.Error(), "duplicate key") + assert.Contains(t, err.Error(), "tool-a") + assert.Nil(t, prereqs) +} + func TestGetRequirementsFromDir_EmptyYamlReturnsNoPrereqs(t *testing.T) { dir := t.TempDir()