diff --git a/internal/tools/versions.go b/internal/tools/versions.go index f50110ad9..a87827dfb 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,46 @@ 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) + 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) + } - 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..264cafaae 100644 --- a/internal/tools/versions_test.go +++ b/internal/tools/versions_test.go @@ -130,6 +130,80 @@ 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_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()