Skip to content

Commit

Permalink
415 parsecomponentsnode invalid memory address or nil pointer derefer…
Browse files Browse the repository at this point in the history
…ence (#442)

<!--

Describe in detail the changes you are proposing, and the rationale.

-->

<!--

Link all GitHub issues fixed by this PR, and add references to prior
related PRs.

-->

Fixes #

### NEW FEATURES | UPGRADE NOTES | ENHANCEMENTS | BUG FIXES |
EXPERIMENTS

<!--

Write a short description of your changes. Examples:

- Fixed a bug
- Added a new feature
- Updated documentation

--> 

-
  • Loading branch information
demeyerthom authored Sep 10, 2024
2 parents 5dd076f + 1ff9f98 commit 97a31f4
Show file tree
Hide file tree
Showing 22 changed files with 447 additions and 302 deletions.
3 changes: 3 additions & 0 deletions .changes/unreleased/Fixed-20240906-123738.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: Fixed
body: Load referenced files before validating config
time: 2024-09-06T12:37:38.406897171+02:00
2 changes: 1 addition & 1 deletion internal/config/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions internal/config/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand All @@ -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 {
Expand Down
60 changes: 51 additions & 9 deletions internal/config/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"path"
"path/filepath"
"strings"

"github.com/mach-composer/mach-composer-cli/internal/state"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
160 changes: 45 additions & 115 deletions internal/config/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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(),
Expand All @@ -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{
Expand Down Expand Up @@ -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,
Expand All @@ -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{
Expand Down Expand Up @@ -167,122 +163,56 @@ 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/<username>/<your-component>.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()))
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 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/<username>/<your-component>.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 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)
}

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/<username>/<your-component>.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)
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)

componentNode, fileName, err := LoadRefData(context.Background(), &intermediate.Components, "")
assert.NoError(t, err)
assert.Equal(t, "components.yml", fileName)
intermediate.Components = *componentNode
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)
}

cfg := &MachConfig{
Plugins: plugins.NewPluginRepository(),
Global: GlobalConfig{
Cloud: "my-cloud",
},
}
err = cfg.Plugins.Add("my-cloud", plugins.NewPluginV1Adapter(plugins.NewMockPluginV1()))
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)

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, 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)
}
8 changes: 4 additions & 4 deletions internal/config/site.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions internal/config/testdata/components.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- name: your-component
source: "git::https://github.com/<username>/<your-component>.git//terraform"
version: 0.1.0
3 changes: 3 additions & 0 deletions internal/config/testdata/components.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- name: your-component
source: "git::https://github.com/<username>/<your-component>.git//terraform"
version: 0.1.0
File renamed without changes.
Loading

0 comments on commit 97a31f4

Please sign in to comment.