From 700d98cea987ffd9940cbd9fe50e8a6923b01fc6 Mon Sep 17 00:00:00 2001 From: Thomas De Meyer Date: Fri, 6 Sep 2024 12:38:08 +0200 Subject: [PATCH 1/2] fix: load referenced files before validating config --- .../unreleased/Fixed-20240906-123738.yaml | 3 + internal/config/component.go | 2 +- internal/config/global.go | 4 +- internal/config/parse.go | 60 ++++++-- internal/config/parse_test.go | 144 +++--------------- internal/config/site.go | 8 +- internal/config/testdata/components.yaml | 3 + internal/config/testdata/components.yml | 3 + .../configs/{basic.yaml => basic/main.yaml} | 0 .../{complex.yaml => complex/main.yaml} | 0 .../component_include_func/components.yaml | 3 + .../configs/component_include_func/main.yaml | 21 +++ .../configs/component_ref/components.yaml | 3 + .../testdata/configs/component_ref/main.yaml | 22 +++ internal/config/utils.go | 83 ++-------- internal/config/utils_test.go | 125 +++++---------- internal/updater/updater.go | 14 +- internal/updater/utils.go | 68 +++++++++ internal/updater/utils_test.go | 129 ++++++++++++++++ 19 files changed, 386 insertions(+), 309 deletions(-) create mode 100644 .changes/unreleased/Fixed-20240906-123738.yaml create mode 100644 internal/config/testdata/components.yaml create mode 100644 internal/config/testdata/components.yml rename internal/config/testdata/configs/{basic.yaml => basic/main.yaml} (100%) rename internal/config/testdata/configs/{complex.yaml => complex/main.yaml} (100%) create mode 100644 internal/config/testdata/configs/component_include_func/components.yaml create mode 100644 internal/config/testdata/configs/component_include_func/main.yaml create mode 100644 internal/config/testdata/configs/component_ref/components.yaml create mode 100644 internal/config/testdata/configs/component_ref/main.yaml create mode 100644 internal/updater/utils_test.go diff --git a/.changes/unreleased/Fixed-20240906-123738.yaml b/.changes/unreleased/Fixed-20240906-123738.yaml new file mode 100644 index 00000000..22626f3a --- /dev/null +++ b/.changes/unreleased/Fixed-20240906-123738.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: Load referenced files before validating config +time: 2024-09-06T12:37:38.406897171+02:00 diff --git a/internal/config/component.go b/internal/config/component.go index f453e49d..4ff2ab86 100644 --- a/internal/config/component.go +++ b/internal/config/component.go @@ -31,7 +31,7 @@ func parseComponentsNode(cfg *MachConfig, node *yaml.Node) error { for _, component := range node.Content { for _, plugin := range cfg.Plugins.All() { data := map[string]any{} - nodes := mapYamlNodes(component.Content) + nodes := MapYamlNodes(component.Content) componentName := nodes["name"].Value version := nodes["version"].Value diff --git a/internal/config/global.go b/internal/config/global.go index f9260516..7bc65be4 100644 --- a/internal/config/global.go +++ b/internal/config/global.go @@ -31,7 +31,7 @@ func parseGlobalNode(cfg *MachConfig, globalNode *yaml.Node) error { } } - nodes := mapYamlNodes(globalNode.Content) + nodes := MapYamlNodes(globalNode.Content) for _, plugin := range cfg.Plugins.All() { data := map[string]any{} @@ -53,7 +53,7 @@ func parseGlobalNode(cfg *MachConfig, globalNode *yaml.Node) error { } if node, ok := nodes["terraform_config"]; ok { - children := mapYamlNodes(node.Content) + children := MapYamlNodes(node.Content) // Backwards compat if child, ok := children["aws_remote_state"]; ok { diff --git a/internal/config/parse.go b/internal/config/parse.go index 172f878d..8bdfc7a4 100644 --- a/internal/config/parse.go +++ b/internal/config/parse.go @@ -6,6 +6,7 @@ import ( "fmt" "path" "path/filepath" + "strings" "github.com/mach-composer/mach-composer-cli/internal/state" @@ -106,12 +107,6 @@ func loadConfig(ctx context.Context, filename, cwd string, pr *plugins.PluginRep return nil, err } - componentNode, _, err := LoadRefData(ctx, &raw.Components, cwd) - if err != nil { - return nil, err - } - raw.Components = *componentNode - // Load the plugins raw.plugins = pr if err := loadPlugins(ctx, raw); err != nil { @@ -169,8 +164,7 @@ func loadPlugins(ctx context.Context, raw *rawConfig) error { return nil } -// parseConfig is responsible for parsing a mach composer yaml config file and -// creating the resulting MachConfig struct. +// resolveConfig is responsible for parsing a mach composer yaml config file and creating the resulting MachConfig struct. func resolveConfig(_ context.Context, intermediate *rawConfig) (*MachConfig, error) { if err := intermediate.validate(); err != nil { return nil, err @@ -224,7 +218,7 @@ func resolveVariables(ctx context.Context, rawConfig *rawConfig, cwd string) err // TransformValue the variables per-site to keep track of which site uses which // variable. for _, node := range rawConfig.Sites.Content { - mapping := mapYamlNodes(node.Content) + mapping := MapYamlNodes(node.Content) if idNode, ok := mapping["identifier"]; ok { siteId := idNode.Value if err := vars.InterpolateSiteNode(siteId, node); err != nil { @@ -248,5 +242,53 @@ func loadYamlFile(filename string) (*yaml.Node, error) { if err := yaml.Unmarshal(body, document); err != nil { return nil, err } + + // Resolve $ref and ${include()} references + if err := resolveReferences(document, filepath.Dir(filename)); err != nil { + return nil, err + } + return document, nil } + +func resolveReferences(node *yaml.Node, baseDir string) error { + if node.Kind == yaml.DocumentNode { + for _, contentNode := range node.Content { + if err := resolveReferences(contentNode, baseDir); err != nil { + return err + } + } + } else if node.Kind == yaml.MappingNode { + for i := 0; i < len(node.Content); i += 2 { + keyNode := node.Content[i] + valueNode := node.Content[i+1] + if keyNode.Value == "$ref" && valueNode.Kind == yaml.ScalarNode { + refFilename := filepath.Join(baseDir, valueNode.Value) + refNode, err := loadYamlFile(refFilename) + if err != nil { + return err + } + contentNode := refNode.Content[0] + *node = *contentNode + return nil + } + if err := resolveReferences(valueNode, baseDir); err != nil { + return err + } + } + } else if node.Kind == yaml.SequenceNode { + for _, contentNode := range node.Content { + if err := resolveReferences(contentNode, baseDir); err != nil { + return err + } + } + } else if node.Kind == yaml.ScalarNode && strings.Contains(node.Value, "include") { + refNode, _, err := LoadIncludeDocument(node, baseDir) + if err != nil { + return err + } + *node = *refNode + return nil + } + return nil +} diff --git a/internal/config/parse_test.go b/internal/config/parse_test.go index 23666e84..c4e86598 100644 --- a/internal/config/parse_test.go +++ b/internal/config/parse_test.go @@ -7,14 +7,10 @@ import ( "github.com/mach-composer/mach-composer-cli/internal/config/variable" "testing" + "github.com/mach-composer/mach-composer-cli/internal/plugins" "github.com/rs/zerolog" - "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v3" - - "github.com/mach-composer/mach-composer-cli/internal/plugins" - "github.com/mach-composer/mach-composer-cli/internal/utils" ) func TestMain(m *testing.M) { @@ -28,7 +24,7 @@ var ignoreOpts = []cmp.Option{ } func TestOpenBasic(t *testing.T) { - config, err := Open(context.Background(), "testdata/configs/basic.yaml", &ConfigOptions{ + config, err := Open(context.Background(), "testdata/configs/basic/main.yaml", &ConfigOptions{ Validate: false, NoResolveVars: true, Plugins: plugins.NewPluginRepository(), @@ -45,7 +41,7 @@ func TestOpenBasic(t *testing.T) { } expected := &MachConfig{ - Filename: "basic.yaml", + Filename: "main.yaml", MachComposer: MachComposer{ Version: "1.0.0", Deployment: Deployment{ @@ -83,14 +79,14 @@ func TestOpenBasic(t *testing.T) { assert.True(t, cmp.Equal(config, expected, ignoreOpts...), cmp.Diff(config, expected, ignoreOpts...)) } -func TestParseComplex(t *testing.T) { +func TestOpenComplex(t *testing.T) { pr := plugins.NewPluginRepository() err := pr.Add("my-plugin", plugins.NewPluginV1Adapter(plugins.NewMockPluginV1())) require.NoError(t, err) err = pr.Add("aws", plugins.NewPluginV1Adapter(plugins.NewMockPluginV1())) require.NoError(t, err) - config, err := Open(context.Background(), "testdata/configs/complex.yaml", &ConfigOptions{ + config, err := Open(context.Background(), "testdata/configs/complex/main.yaml", &ConfigOptions{ Validate: false, NoResolveVars: true, Plugins: pr, @@ -109,7 +105,7 @@ func TestParseComplex(t *testing.T) { } expected := &MachConfig{ - Filename: "complex.yaml", + Filename: "main.yaml", MachComposer: MachComposer{ Version: "1.0.0", Plugins: map[string]MachPluginConfig{ @@ -167,122 +163,26 @@ func TestParseComplex(t *testing.T) { assert.True(t, cmp.Equal(config, expected, ignoreOpts...), cmp.Diff(config, expected, ignoreOpts...)) } -func TestParseComponentsNodeInline(t *testing.T) { - var intermediate struct { - Components yaml.Node `yaml:"components"` - } - - data := []byte(utils.TrimIndent(` - components: - - name: your-component - source: "git::https://github.com//.git//terraform" - version: 0.1.0 - `)) - - err := yaml.Unmarshal(data, &intermediate) - require.NoError(t, err) - - cfg := &MachConfig{ - Plugins: plugins.NewPluginRepository(), - Global: GlobalConfig{ - Cloud: "my-cloud", - }, - } - err = cfg.Plugins.Add("my-cloud", plugins.NewPluginV1Adapter(plugins.NewMockPluginV1())) - require.NoError(t, err) - - err = parseComponentsNode(cfg, &intermediate.Components) - require.NoError(t, err) - assert.Len(t, cfg.Components, 1) - assert.Equal(t, "your-component", cfg.Components[0].Name) -} - -func TestParseComponentsNodeRef(t *testing.T) { - utils.FS = afero.NewMemMapFs() - utils.AFS = &afero.Afero{Fs: utils.FS} - - content := utils.TrimIndent(` - components: - - name: your-component - source: "git::https://github.com//.git//terraform" - version: 0.1.0 - `) - - err := utils.AFS.WriteFile("components.yml", []byte(content), 0644) - require.NoError(t, err) - - var intermediate struct { - Components yaml.Node `yaml:"components"` - } - - data := []byte(utils.TrimIndent(` - components: - $ref: components.yml#/components - `)) - - err = yaml.Unmarshal(data, &intermediate) - require.NoError(t, err) - - componentNode, fileName, err := LoadRefData(context.Background(), &intermediate.Components, "") - assert.NoError(t, err) - assert.Equal(t, "components.yml", fileName) - intermediate.Components = *componentNode - - cfg := &MachConfig{ - Plugins: plugins.NewPluginRepository(), - Global: GlobalConfig{ - Cloud: "my-cloud", - }, - } - err = cfg.Plugins.Add("my-cloud", plugins.NewPluginV1Adapter(plugins.NewMockPluginV1())) +func TestOpenComponentIncludeFunc(t *testing.T) { + config, err := Open(context.Background(), "testdata/configs/component_include_func/main.yaml", &ConfigOptions{ + Validate: false, + NoResolveVars: true, + Plugins: plugins.NewPluginRepository(), + }) require.NoError(t, err) - err = parseComponentsNode(cfg, &intermediate.Components) - require.NoError(t, err) - assert.Len(t, cfg.Components, 1) - assert.Equal(t, "your-component", cfg.Components[0].Name) + assert.Len(t, config.Components, 1) + assert.Equal(t, "your-component", config.Components[0].Name) } -func TestParseComponentsNodeInclude(t *testing.T) { - utils.FS = afero.NewMemMapFs() - utils.AFS = &afero.Afero{Fs: utils.FS} - - content := utils.TrimIndent(` - - name: your-component - source: "git::https://github.com//.git//terraform" - version: 0.1.0 - `) - - err := utils.AFS.WriteFile("components.yml", []byte(content), 0644) - require.NoError(t, err) - - var intermediate struct { - Components yaml.Node `yaml:"components"` - } - - data := []byte(utils.TrimIndent(` - components: ${include(components.yml)} - `)) - - err = yaml.Unmarshal(data, &intermediate) - require.NoError(t, err) - - componentNode, fileName, err := LoadRefData(context.Background(), &intermediate.Components, "") - assert.NoError(t, err) - assert.Equal(t, "components.yml", fileName) - intermediate.Components = *componentNode - - cfg := &MachConfig{ - Plugins: plugins.NewPluginRepository(), - Global: GlobalConfig{ - Cloud: "my-cloud", - }, - } - err = cfg.Plugins.Add("my-cloud", plugins.NewPluginV1Adapter(plugins.NewMockPluginV1())) +func TestOpenComponentRef(t *testing.T) { + config, err := Open(context.Background(), "testdata/configs/component_ref/main.yaml", &ConfigOptions{ + Validate: false, + NoResolveVars: true, + Plugins: plugins.NewPluginRepository(), + }) require.NoError(t, err) - err = parseComponentsNode(cfg, &intermediate.Components) - require.NoError(t, err) - assert.Len(t, cfg.Components, 1) - assert.Equal(t, "your-component", cfg.Components[0].Name) + assert.Len(t, config.Components, 1) + assert.Equal(t, "your-component", config.Components[0].Name) } diff --git a/internal/config/site.go b/internal/config/site.go index 71af8621..89511710 100644 --- a/internal/config/site.go +++ b/internal/config/site.go @@ -38,7 +38,7 @@ func parseSitesNode(cfg *MachConfig, sitesNode *yaml.Node) error { } for _, site := range sitesNode.Content { - nodes := mapYamlNodes(site.Content) + nodes := MapYamlNodes(site.Content) siteId := nodes["identifier"].Value for _, plugin := range cfg.Plugins.All() { @@ -89,7 +89,7 @@ func parseSitesNode(cfg *MachConfig, sitesNode *yaml.Node) error { } func parseSiteEndpointNode(cfg *MachConfig, siteId string, node *yaml.Node) error { - nodes := mapYamlNodes(node.Content) + nodes := MapYamlNodes(node.Content) knownTags := []string{"url", "key", "zone", "throttling_rate_limit", "throttling_burst_limit"} for endpointId, endpointNode := range nodes { @@ -125,7 +125,7 @@ func parseSiteEndpointNode(cfg *MachConfig, siteId string, node *yaml.Node) erro continue } - children := mapYamlNodes(endpointNode.Content) + children := MapYamlNodes(endpointNode.Content) if len(pie.Intersect(knownTags, pie.Keys(children))) > 0 { cli.DeprecationWarning(&cli.DeprecationOptions{ @@ -202,7 +202,7 @@ func parseSiteComponentsNode(cfg *MachConfig, siteKey string, node *yaml.Node) e } for _, component := range node.Content { - nodes := mapYamlNodes(component.Content) + nodes := MapYamlNodes(component.Content) componentKey := nodes["name"].Value migrateCommercetools(siteKey, componentKey, nodes) diff --git a/internal/config/testdata/components.yaml b/internal/config/testdata/components.yaml new file mode 100644 index 00000000..a6297296 --- /dev/null +++ b/internal/config/testdata/components.yaml @@ -0,0 +1,3 @@ +- name: your-component + source: "git::https://github.com//.git//terraform" + version: 0.1.0 diff --git a/internal/config/testdata/components.yml b/internal/config/testdata/components.yml new file mode 100644 index 00000000..a6297296 --- /dev/null +++ b/internal/config/testdata/components.yml @@ -0,0 +1,3 @@ +- name: your-component + source: "git::https://github.com//.git//terraform" + version: 0.1.0 diff --git a/internal/config/testdata/configs/basic.yaml b/internal/config/testdata/configs/basic/main.yaml similarity index 100% rename from internal/config/testdata/configs/basic.yaml rename to internal/config/testdata/configs/basic/main.yaml diff --git a/internal/config/testdata/configs/complex.yaml b/internal/config/testdata/configs/complex/main.yaml similarity index 100% rename from internal/config/testdata/configs/complex.yaml rename to internal/config/testdata/configs/complex/main.yaml diff --git a/internal/config/testdata/configs/component_include_func/components.yaml b/internal/config/testdata/configs/component_include_func/components.yaml new file mode 100644 index 00000000..a6297296 --- /dev/null +++ b/internal/config/testdata/configs/component_include_func/components.yaml @@ -0,0 +1,3 @@ +- name: your-component + source: "git::https://github.com//.git//terraform" + version: 0.1.0 diff --git a/internal/config/testdata/configs/component_include_func/main.yaml b/internal/config/testdata/configs/component_include_func/main.yaml new file mode 100644 index 00000000..1c0622e4 --- /dev/null +++ b/internal/config/testdata/configs/component_include_func/main.yaml @@ -0,0 +1,21 @@ +mach_composer: + version: "1.0.0" +plugins: { } +global: + environment: test +sites: + - identifier: my-site + components: + - name: your-component + variables: + FOO_VAR: my-value + BAR_VAR: ${var.foo} + MULTIPLE_VARS: ${var.foo.bar} ${var.bar.foo} + secrets: + MY_SECRET: secretvalue + components: + - name: your-component + source: "git::https://github.com//.git//terraform" + version: 0.1.0 + +components: ${include(components.yaml)} diff --git a/internal/config/testdata/configs/component_ref/components.yaml b/internal/config/testdata/configs/component_ref/components.yaml new file mode 100644 index 00000000..a6297296 --- /dev/null +++ b/internal/config/testdata/configs/component_ref/components.yaml @@ -0,0 +1,3 @@ +- name: your-component + source: "git::https://github.com//.git//terraform" + version: 0.1.0 diff --git a/internal/config/testdata/configs/component_ref/main.yaml b/internal/config/testdata/configs/component_ref/main.yaml new file mode 100644 index 00000000..0d6f0b1c --- /dev/null +++ b/internal/config/testdata/configs/component_ref/main.yaml @@ -0,0 +1,22 @@ +mach_composer: + version: "1.0.0" +plugins: { } +global: + environment: test +sites: + - identifier: my-site + components: + - name: your-component + variables: + FOO_VAR: my-value + BAR_VAR: ${var.foo} + MULTIPLE_VARS: ${var.foo.bar} ${var.bar.foo} + secrets: + MY_SECRET: secretvalue + components: + - name: your-component + source: "git::https://github.com//.git//terraform" + version: 0.1.0 + +components: + $ref: "components.yaml" diff --git a/internal/config/utils.go b/internal/config/utils.go index ad505866..2819cd85 100644 --- a/internal/config/utils.go +++ b/internal/config/utils.go @@ -1,15 +1,11 @@ package config import ( - "context" "fmt" "github.com/mach-composer/mach-composer-cli/internal/cli" - "path" + "gopkg.in/yaml.v3" "path/filepath" "regexp" - "strings" - - "gopkg.in/yaml.v3" "github.com/mach-composer/mach-composer-cli/internal/utils" ) @@ -22,9 +18,9 @@ func nodeAsMap(n *yaml.Node) (map[string]any, error) { return target, nil } -// mapYamlNodes creates a map[key]node from a slice of yaml.Node's. It assumes +// MapYamlNodes creates a map[key]node from a slice of yaml.Node's. It assumes // that the nodes are pairs, e.g. [key, value, key, value] -func mapYamlNodes(nodes []*yaml.Node) map[string]*yaml.Node { +func MapYamlNodes(nodes []*yaml.Node) map[string]*yaml.Node { result := map[string]*yaml.Node{} // Check if there are an even number of nodes as we expect a @@ -41,7 +37,7 @@ func mapYamlNodes(nodes []*yaml.Node) map[string]*yaml.Node { // Check if there is an alias node and resolve it val, ok := result["<<"] if ok { - aliasData := mapYamlNodes(val.Alias.Content) + aliasData := MapYamlNodes(val.Alias.Content) delete(result, "<<") // We only add the aliased data if the key is not already present @@ -56,14 +52,10 @@ func mapYamlNodes(nodes []*yaml.Node) map[string]*yaml.Node { return result } -// LoadRefData will load referenced files and replace the node with the content of these files. It works both with the -// ${include()} syntax and the $ref syntax. -func LoadRefData(_ context.Context, node *yaml.Node, cwd string) (*yaml.Node, string, error) { - switch node.Kind { - case yaml.ScalarNode: - cli.DeprecationWarning(&cli.DeprecationOptions{ - Message: "the '${include()}' syntax is deprecated and will be removed in version 3.0", - Details: ` +func LoadIncludeDocument(node *yaml.Node, cwd string) (*yaml.Node, string, error) { + cli.DeprecationWarning(&cli.DeprecationOptions{ + Message: "the '${include()}' syntax is deprecated and will be removed in version 3.0", + Details: ` For example instead of: components: ${include(components.yml)} @@ -71,65 +63,8 @@ func LoadRefData(_ context.Context, node *yaml.Node, cwd string) (*yaml.Node, st components: $ref: "components.yml" `, - }) - - newNode, filePath, err := loadIncludeDocument(node, cwd) - if err != nil { - return nil, "", err - } - - return newNode, filePath, nil - case yaml.MappingNode: - newNode, filePath, err := loadRefDocument(node, cwd) - if err != nil { - return nil, "", err - } - - return newNode, filePath, nil - default: - return node, "", nil - } -} - -func loadRefDocument(node *yaml.Node, cwd string) (*yaml.Node, string, error) { - data := mapYamlNodes(node.Content) - ref, ok := data["$ref"] - if !ok { - return node, "", nil - } - - parts := strings.SplitN(ref.Value, "#", 2) - fileName := parts[0] - - body, err := utils.AFS.ReadFile(path.Join(cwd, fileName)) - if err != nil { - return nil, "", err - } - result := &yaml.Node{} - if err = yaml.Unmarshal(body, result); err != nil { - return nil, "", err - } - - root := result.Content[0] - - if len(parts) > 1 { - p := strings.TrimPrefix(parts[1], "/") - node := root - for _, p := range strings.Split(p, "/") { - nodes := mapYamlNodes(node.Content) - n, ok := nodes[p] - if !ok { - return nil, "", fmt.Errorf("unable to resolve node %s", parts[1]) - } - node = n - } - root = node - } - - return root, fileName, nil -} + }) -func loadIncludeDocument(node *yaml.Node, cwd string) (*yaml.Node, string, error) { re := regexp.MustCompile(`\$\{include\(([^)]+)\)\}`) data := re.FindStringSubmatch(node.Value) if len(data) != 2 { diff --git a/internal/config/utils_test.go b/internal/config/utils_test.go index be270bf9..bdde99f4 100644 --- a/internal/config/utils_test.go +++ b/internal/config/utils_test.go @@ -1,15 +1,10 @@ package config import ( - "context" "testing" - "github.com/spf13/afero" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" - - "github.com/mach-composer/mach-composer-cli/internal/utils" ) func TestNodeAsMap(t *testing.T) { @@ -106,125 +101,73 @@ func TestMapYamlNodes(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - result := mapYamlNodes(tc.nodes) + result := MapYamlNodes(tc.nodes) assert.Equal(t, tc.expected, result) }) } } -func TestLoadRefData(t *testing.T) { +func TestLoadIncludeDocument(t *testing.T) { tests := []struct { name string node *yaml.Node - refContent string - refFilename string - expected *yaml.Node - wantErr bool + cwd string + expectedErr bool }{ { - name: "mapping node with $ref", + name: "valid include syntax with valid components.yaml", node: &yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - {Value: "$ref"}, - {Value: "ref.yaml"}, - }, + Kind: yaml.ScalarNode, + Value: "${include(components.yaml)}", }, - refContent: "key: value", - refFilename: "ref.yaml", - expected: &yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - {Value: "key"}, - {Value: "value"}, - }, - }, - wantErr: false, + cwd: "testdata", + expectedErr: false, }, { - name: "mapping node with nested $ref", + name: "valid include syntax with valid components.yml", node: &yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - {Value: "$ref"}, - {Value: "ref.yaml"}, - }, - }, - refContent: utils.TrimIndent(` - some-node: - other-node: - key: value - `), - refFilename: "ref.yaml", - expected: &yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - {Value: "key"}, - {Value: "value"}, - }, + Kind: yaml.ScalarNode, + Value: "${include(components.yml)}", }, - wantErr: false, + cwd: "testdata", + expectedErr: false, }, { - name: "mapping node without $ref", + name: "invalid include syntax", node: &yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - {Value: "key"}, - {Value: "value"}, - }, + Kind: yaml.ScalarNode, + Value: "${include(invalid-syntax}", }, - refContent: "", - refFilename: "", - expected: &yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - {Value: "key"}, - {Value: "value"}, - }, + cwd: "testdata", + expectedErr: true, + }, + { + name: "valid include syntax with non-existent file", + node: &yaml.Node{ + Kind: yaml.ScalarNode, + Value: "${include(nonexistent.yml)}", }, - wantErr: false, + cwd: "testdata", + expectedErr: true, }, { - name: "error loading ref document", + name: "valid include syntax with invalid YAML file", node: &yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - {Value: "$ref"}, - {Value: "other.yaml"}, - }, + Kind: yaml.ScalarNode, + Value: "${include(invalid.yml)}", }, - refContent: "", - refFilename: "other.yaml", - expected: nil, - wantErr: true, + cwd: "testdata", + expectedErr: true, }, } - utils.FS = afero.NewMemMapFs() - utils.AFS = &afero.Afero{Fs: utils.FS} - for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - if tc.refContent != "" { - err := utils.AFS.WriteFile("ref.yaml", []byte(tc.refContent), 0644) - require.NoError(t, err) - } - - _, filename, err := LoadRefData(context.Background(), tc.node, "./") - if tc.wantErr { + _, _, err := LoadIncludeDocument(tc.node, tc.cwd) + if tc.expectedErr { assert.Error(t, err) } else { - assert.Equal(t, filename, tc.refFilename) assert.NoError(t, err) - - expectedData := []byte{} - require.NoError(t, tc.expected.Encode(expectedData)) - - resultData := []byte{} - require.NoError(t, tc.node.Encode(resultData)) - - assert.Equal(t, expectedData, resultData) } }) } diff --git a/internal/updater/updater.go b/internal/updater/updater.go index be49bc68..002aa052 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -3,6 +3,7 @@ package updater import ( "context" "fmt" + "github.com/rs/zerolog/log" "path" "path/filepath" "strings" @@ -65,6 +66,7 @@ type Updater struct { // NewUpdater creates an update to update the component versions in a config // file. func NewUpdater(ctx context.Context, filename string, useCloud bool) (*Updater, error) { + //TODO: Switch to using config.loadConfig to load the config file so we have a consistent way of loading the config body, err := utils.AFS.ReadFile(filename) if err != nil { return nil, err @@ -79,7 +81,7 @@ func NewUpdater(ctx context.Context, filename string, useCloud bool) (*Updater, cwd := path.Dir(filename) // Resolve $ref and include() references for the components. - componentNode, componentsFilename, err := config.LoadRefData(ctx, &raw.Components, cwd) + componentNode, componentsFilename, err := loadRefData(ctx, &raw.Components, cwd) if err != nil { return nil, err } @@ -136,7 +138,7 @@ func (u *Updater) UpdateAllComponents(ctx context.Context) error { return err } u.updates = updateSet.updates - fmt.Printf("%d components have updates available\n", len(u.updates)) + log.Info().Msgf("%d components have updates available\n", len(u.updates)) return nil } @@ -145,7 +147,7 @@ func (u *Updater) UpdateAllComponents(ctx context.Context) error { func (u *Updater) UpdateComponent(ctx context.Context, name, version string) error { component := u.config.GetComponent(name) if component == nil { - return fmt.Errorf("No component found with name %s", name) + return fmt.Errorf("no component found with name %s", name) } // If no specific version is defined we auto-detect the last version @@ -157,7 +159,7 @@ func (u *Updater) UpdateComponent(ctx context.Context, name, version string) err Forced: true, }, } - fmt.Printf("Setting component %s to version %s\n", component.Name, version) + log.Info().Msgf("Setting component %s to version %s\n", component.Name, version) return nil } @@ -166,10 +168,10 @@ func (u *Updater) UpdateComponent(ctx context.Context, name, version string) err return err } if updateSet.HasChanges() { - fmt.Printf("Updating component %s to version %s\n", component.Name, updateSet.updates[0].LastVersion) + log.Info().Msgf("Updating component %s to version %s\n", component.Name, updateSet.updates[0].LastVersion) u.updates = updateSet.updates } else { - fmt.Printf("No updates for component %s\n", component.Name) + log.Info().Msgf("No updates for component %s\n", component.Name) } return nil } diff --git a/internal/updater/utils.go b/internal/updater/utils.go index 17fa4cf4..f8e47325 100644 --- a/internal/updater/utils.go +++ b/internal/updater/utils.go @@ -2,6 +2,12 @@ package updater import ( "bufio" + "context" + "fmt" + "github.com/mach-composer/mach-composer-cli/internal/config" + "github.com/mach-composer/mach-composer-cli/internal/utils" + "gopkg.in/yaml.v3" + "path" "strings" ) @@ -13,3 +19,65 @@ func SplitLines(s string) []string { } return lines } + +// LoadRefData will load referenced files and replace the node with the content of these files. It works both with the +// ${include()} syntax and the $ref syntax. +// Deprecated: Only used in the updater logic as we need the file name to update there +func loadRefData(_ context.Context, node *yaml.Node, cwd string) (*yaml.Node, string, error) { + switch node.Kind { + case yaml.ScalarNode: + newNode, filePath, err := config.LoadIncludeDocument(node, cwd) + if err != nil { + return nil, "", err + } + + return newNode, filePath, nil + case yaml.MappingNode: + newNode, filePath, err := loadRefDocument(node, cwd) + if err != nil { + return nil, "", err + } + + return newNode, filePath, nil + default: + return node, "", nil + } +} + +func loadRefDocument(node *yaml.Node, cwd string) (*yaml.Node, string, error) { + data := config.MapYamlNodes(node.Content) + ref, ok := data["$ref"] + if !ok { + return node, "", nil + } + + parts := strings.SplitN(ref.Value, "#", 2) + fileName := parts[0] + + body, err := utils.AFS.ReadFile(path.Join(cwd, fileName)) + if err != nil { + return nil, "", err + } + result := &yaml.Node{} + if err = yaml.Unmarshal(body, result); err != nil { + return nil, "", err + } + + root := result.Content[0] + + if len(parts) > 1 { + p := strings.TrimPrefix(parts[1], "/") + node := root + for _, p := range strings.Split(p, "/") { + nodes := config.MapYamlNodes(node.Content) + n, ok := nodes[p] + if !ok { + return nil, "", fmt.Errorf("unable to resolve node %s", parts[1]) + } + node = n + } + root = node + } + + return root, fileName, nil +} diff --git a/internal/updater/utils_test.go b/internal/updater/utils_test.go new file mode 100644 index 00000000..39de8248 --- /dev/null +++ b/internal/updater/utils_test.go @@ -0,0 +1,129 @@ +package updater + +import ( + "context" + "github.com/mach-composer/mach-composer-cli/internal/utils" + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" + "testing" +) + +func TestLoadRefData(t *testing.T) { + tests := []struct { + name string + node *yaml.Node + refContent string + refFilename string + expected *yaml.Node + wantErr bool + }{ + { + name: "mapping node with $ref", + node: &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Value: "$ref"}, + {Value: "ref.yaml"}, + }, + }, + refContent: "key: value", + refFilename: "ref.yaml", + expected: &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Value: "key"}, + {Value: "value"}, + }, + }, + wantErr: false, + }, + { + name: "mapping node with nested $ref", + node: &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Value: "$ref"}, + {Value: "ref.yaml"}, + }, + }, + refContent: utils.TrimIndent(` + some-node: + other-node: + key: value + `), + refFilename: "ref.yaml", + expected: &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Value: "key"}, + {Value: "value"}, + }, + }, + wantErr: false, + }, + { + name: "mapping node without $ref", + node: &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Value: "key"}, + {Value: "value"}, + }, + }, + refContent: "", + refFilename: "", + expected: &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Value: "key"}, + {Value: "value"}, + }, + }, + wantErr: false, + }, + { + name: "error loading ref document", + node: &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + {Value: "$ref"}, + {Value: "other.yaml"}, + }, + }, + refContent: "", + refFilename: "other.yaml", + expected: nil, + wantErr: true, + }, + } + + utils.FS = afero.NewMemMapFs() + utils.AFS = &afero.Afero{Fs: utils.FS} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if tc.refContent != "" { + err := utils.AFS.WriteFile("ref.yaml", []byte(tc.refContent), 0644) + require.NoError(t, err) + } + + _, filename, err := loadRefData(context.Background(), tc.node, "./") + if tc.wantErr { + assert.Error(t, err) + } else { + assert.Equal(t, filename, tc.refFilename) + assert.NoError(t, err) + + expectedData := []byte{} + require.NoError(t, tc.expected.Encode(expectedData)) + + resultData := []byte{} + require.NoError(t, tc.node.Encode(resultData)) + + assert.Equal(t, expectedData, resultData) + } + }) + } +} From 3a7a7708fe79d6be1e72300229977b1daa27f38a Mon Sep 17 00:00:00 2001 From: Thomas De Meyer Date: Fri, 6 Sep 2024 13:14:06 +0200 Subject: [PATCH 2/2] fix: added some additional alias tests --- internal/config/parse_test.go | 30 +++++++++++++++++++ .../testdata/configs/basic_alias/main.yaml | 19 ++++++++++++ .../component_ref_alias/components.yaml | 6 ++++ .../configs/component_ref_alias/main.yaml | 13 ++++++++ 4 files changed, 68 insertions(+) create mode 100644 internal/config/testdata/configs/basic_alias/main.yaml create mode 100644 internal/config/testdata/configs/component_ref_alias/components.yaml create mode 100644 internal/config/testdata/configs/component_ref_alias/main.yaml diff --git a/internal/config/parse_test.go b/internal/config/parse_test.go index c4e86598..40898d5b 100644 --- a/internal/config/parse_test.go +++ b/internal/config/parse_test.go @@ -186,3 +186,33 @@ func TestOpenComponentRef(t *testing.T) { assert.Len(t, config.Components, 1) assert.Equal(t, "your-component", config.Components[0].Name) } + +func TestAliased(t *testing.T) { + config, err := Open(context.Background(), "testdata/configs/basic_alias/main.yaml", &ConfigOptions{ + Validate: false, + NoResolveVars: true, + Plugins: plugins.NewPluginRepository(), + }) + require.NoError(t, err) + + assert.Len(t, config.Components, 2) + assert.Equal(t, "your-component", config.Components[0].Name) + assert.Equal(t, "0.1.0", config.Components[0].Version) + assert.Equal(t, "your-component-aliased", config.Components[1].Name) + assert.Equal(t, "0.1.0", config.Components[1].Version) +} + +func TestComponentRefAliased(t *testing.T) { + config, err := Open(context.Background(), "testdata/configs/component_ref_alias/main.yaml", &ConfigOptions{ + Validate: false, + NoResolveVars: true, + Plugins: plugins.NewPluginRepository(), + }) + require.NoError(t, err) + + assert.Len(t, config.Components, 2) + assert.Equal(t, "your-component", config.Components[0].Name) + assert.Equal(t, "0.1.0", config.Components[0].Version) + assert.Equal(t, "your-component-aliased", config.Components[1].Name) + assert.Equal(t, "0.1.0", config.Components[1].Version) +} diff --git a/internal/config/testdata/configs/basic_alias/main.yaml b/internal/config/testdata/configs/basic_alias/main.yaml new file mode 100644 index 00000000..4eba0413 --- /dev/null +++ b/internal/config/testdata/configs/basic_alias/main.yaml @@ -0,0 +1,19 @@ +mach_composer: + version: "1.0.0" +plugins: { } +global: + environment: test +sites: + - identifier: my-site + components: + - name: your-component + - name: your-component-aliased + +components: + - &alias + name: your-component + source: "git::https://github.com//.git//terraform" + version: 0.1.0 + - <<: *alias + name: your-component-aliased + diff --git a/internal/config/testdata/configs/component_ref_alias/components.yaml b/internal/config/testdata/configs/component_ref_alias/components.yaml new file mode 100644 index 00000000..6e497dbf --- /dev/null +++ b/internal/config/testdata/configs/component_ref_alias/components.yaml @@ -0,0 +1,6 @@ +- &alias + name: your-component + source: "git::https://github.com//.git//terraform" + version: 0.1.0 +- <<: *alias + name: your-component-aliased diff --git a/internal/config/testdata/configs/component_ref_alias/main.yaml b/internal/config/testdata/configs/component_ref_alias/main.yaml new file mode 100644 index 00000000..5045ab14 --- /dev/null +++ b/internal/config/testdata/configs/component_ref_alias/main.yaml @@ -0,0 +1,13 @@ +mach_composer: + version: "1.0.0" +plugins: { } +global: + environment: test +sites: + - identifier: my-site + components: + - name: your-component + - name: your-component-aliased + +components: + $ref: "components.yaml"