From f8e1afc738444e6d1251fbe4a22ffcf71f1f76d3 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 23 Nov 2022 16:29:44 +0100 Subject: [PATCH 1/3] Find overrides of variables with names in their dots --- internal/packages/packages.go | 27 +++++++++++++++++++ internal/testrunner/runners/system/runner.go | 22 ++++++++++++++- .../testrunner/runners/system/test_config.go | 21 ++++++++++----- 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/internal/packages/packages.go b/internal/packages/packages.go index f56d8dcf24..002be39079 100644 --- a/internal/packages/packages.go +++ b/internal/packages/packages.go @@ -12,6 +12,7 @@ import ( "github.com/pkg/errors" + "github.com/elastic/elastic-package/internal/common" "github.com/elastic/go-ucfg" "github.com/elastic/go-ucfg/yaml" ) @@ -34,6 +35,7 @@ const ( // VarValue represents a variable value as defined in a package or data stream // manifest file. type VarValue struct { + dict map[string]interface{} scalar interface{} list []interface{} } @@ -42,6 +44,8 @@ type VarValue struct { // manifest file into a VarValue. func (vv *VarValue) Unpack(value interface{}) error { switch u := value.(type) { + case map[string]interface{}: + vv.dict = u case []interface{}: vv.list = u default: @@ -57,10 +61,33 @@ func (vv VarValue) MarshalJSON() ([]byte, error) { return json.Marshal(vv.scalar) } else if vv.list != nil { return json.Marshal(vv.list) + } else if vv.dict != nil { + return json.Marshal(vv.dict) } return []byte("null"), nil } +// GetChildValue helps accessing subvalues when the value is a dictionary. +// This is generally a workaround for ucfg not keeping the structure of +// the read configuration (https://github.com/elastic/go-ucfg/issues/124). +// For variables we want their full names. +func (vv *VarValue) GetChildValue(name string) (VarValue, bool) { + if vv.dict == nil { + return VarValue{}, false + } + + ms := common.MapStr(vv.dict) + value, err := ms.GetValue(name) + if err != nil { + return VarValue{}, false + } + + var varValue VarValue + // Ignoring error as this cannot fail. + _ = varValue.Unpack(value) + return varValue, true +} + // Variable is an instance of configuration variable (named, typed). type Variable struct { Name string `config:"name" json:"name" yaml:"name"` diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 9654b6c2e8..07faafe1d2 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -644,7 +644,7 @@ func setKibanaVariables(definitions []packages.Variable, values map[string]packa for _, definition := range definitions { val := definition.Default - value, exists := values[definition.Name] + value, exists := getConfigValue(values, definition.Name) if exists { val = value } @@ -657,6 +657,26 @@ func setKibanaVariables(definitions []packages.Variable, values map[string]packa return vars } +func getConfigValue(values map[string]packages.VarValue, name string) (packages.VarValue, bool) { + value, found := values[name] + if found { + return value, true + } + + // Workaround when the value has been expanded. + root, leaf, found := strings.Cut(name, ".") + if !found { + return packages.VarValue{}, false + } + + parent, found := values[root] + if !found { + return packages.VarValue{}, false + } + + return parent.GetChildValue(leaf) +} + // getDataStreamIndex returns the index of the data stream whose input name // matches. Otherwise it returns the 0. func getDataStreamIndex(inputName string, ds packages.DataStreamManifest) int { diff --git a/internal/testrunner/runners/system/test_config.go b/internal/testrunner/runners/system/test_config.go index 59203f34b3..7869c377d0 100644 --- a/internal/testrunner/runners/system/test_config.go +++ b/internal/testrunner/runners/system/test_config.go @@ -78,20 +78,29 @@ func newConfig(configFilePath string, ctxt servicedeployer.ServiceContext, servi return nil, errors.Wrapf(err, "could not apply context to test configuration file: %s", configFilePath) } - var c testConfig - cfg, err := yaml.NewConfig(data, ucfg.PathSep(".")) + c, err := parseConfig(configFilePath, data) if err != nil { - return nil, errors.Wrapf(err, "unable to load system test configuration file: %s", configFilePath) - } - if err := cfg.Unpack(&c); err != nil { - return nil, errors.Wrapf(err, "unable to unpack system test configuration file: %s", configFilePath) + return nil, err } + // Save path c.Path = configFilePath c.ServiceVariantName = serviceVariantName return &c, nil } +func parseConfig(configFilePath string, data []byte) (testConfig, error) { + var c testConfig + cfg, err := yaml.NewConfig(data, ucfg.PathSep(".")) + if err != nil { + return c, errors.Wrapf(err, "unable to load system test configuration file: %s", configFilePath) + } + if err := cfg.Unpack(&c); err != nil { + return c, errors.Wrapf(err, "unable to unpack system test configuration file: %s", configFilePath) + } + return c, nil +} + func listConfigFiles(systemTestFolderPath string) (files []string, err error) { fHandle, err := os.Open(systemTestFolderPath) if err != nil { From 9774f1770f31bcbc173d030c29307d2696436075 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 23 Nov 2022 17:22:23 +0100 Subject: [PATCH 2/3] Fix format --- internal/packages/packages.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/packages/packages.go b/internal/packages/packages.go index 002be39079..747c67be70 100644 --- a/internal/packages/packages.go +++ b/internal/packages/packages.go @@ -12,9 +12,10 @@ import ( "github.com/pkg/errors" - "github.com/elastic/elastic-package/internal/common" "github.com/elastic/go-ucfg" "github.com/elastic/go-ucfg/yaml" + + "github.com/elastic/elastic-package/internal/common" ) const ( From 2571ea9840328085df3dc4dd6b3967c27efa3fb9 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 23 Nov 2022 17:26:07 +0100 Subject: [PATCH 3/3] Add tests --- .../runners/system/test_config_test.go | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 internal/testrunner/runners/system/test_config_test.go diff --git a/internal/testrunner/runners/system/test_config_test.go b/internal/testrunner/runners/system/test_config_test.go new file mode 100644 index 0000000000..8e67a66d24 --- /dev/null +++ b/internal/testrunner/runners/system/test_config_test.go @@ -0,0 +1,29 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package system + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetExpandedConfigValue(t *testing.T) { + const configData = ` +vars: + jmx.mappings: | + - mbean: 'java.lang:type=Runtime' + attributes: + - attr: Uptime + field: uptime +` + + config, err := parseConfig("test", []byte(configData)) + require.NoError(t, err) + + _, found := getConfigValue(config.Vars, "jmx.mappings") + assert.True(t, found) +}