From 167164dd9154180fa55de0e573ac31f20b3a0a48 Mon Sep 17 00:00:00 2001 From: Evan Louie Date: Thu, 1 Aug 2019 14:42:04 -0700 Subject: [PATCH 1/2] Added `fab unset` command - `unset` allows users to remove keys from a target configuration; the opposite of `set`. --- cmd/set.go | 4 +- cmd/set_test.go | 6 ++ cmd/unset.go | 86 ++++++++++++++++++++++++++ cmd/unset_test.go | 70 +++++++++++++++++++++ core/componentConfig.go | 64 ++++++++++++++++++- core/componentConfig_test.go | 12 ++-- test/fixtures/set/config/test.yaml | 2 + test/fixtures/unset/config/common.yaml | 11 ++++ 8 files changed, 247 insertions(+), 8 deletions(-) create mode 100644 cmd/unset.go create mode 100644 cmd/unset_test.go create mode 100644 test/fixtures/unset/config/common.yaml diff --git a/cmd/set.go b/cmd/set.go index c48a3d4..3dbccec 100644 --- a/cmd/set.go +++ b/cmd/set.go @@ -132,7 +132,9 @@ func Set(environment string, subcomponent string, pathValuePairStrings []string, } } - componentConfig.SetConfig(subcomponentPath, pathValue.Path, pathValue.Value) + if err = componentConfig.SetConfig(subcomponentPath, pathValue.Path, pathValue.Value); err != nil { + return err + } } return componentConfig.Write(environment) diff --git a/cmd/set_test.go b/cmd/set_test.go index 1119af3..3716eb1 100644 --- a/cmd/set_test.go +++ b/cmd/set_test.go @@ -68,6 +68,12 @@ func TestSetValue(t *testing.T) { err = Set("test", "", []string{"newfoo=faa"}, noNewConfigKeys, "") assert.NotNil(t, err) + // Setting value where one of the keys is a non-map should return an error + err = Set("test", "", []string{"a.b=foobar"}, false, "") + assert.Nil(t, err) + err = Set("test", "", []string{"a.b.c=foobar"}, false, "") + assert.NotNil(t, err) + //////////////////////////////////////////////////////////////////////////////// // Start Set from yaml file //////////////////////////////////////////////////////////////////////////////// diff --git a/cmd/unset.go b/cmd/unset.go new file mode 100644 index 0000000..fbae67f --- /dev/null +++ b/cmd/unset.go @@ -0,0 +1,86 @@ +package cmd + +import ( + "errors" + "strings" + + "github.com/microsoft/fabrikate/core" + "github.com/spf13/cobra" +) + +func unset(keys []string, environment, subcomponent string) (err error) { + // Load config + componentConfig := core.NewComponentConfig(".") + + // Split component path delimited on "." + subcomponentPath := []string{} + if len(subcomponent) > 0 { + subcomponentPath = strings.Split(subcomponent, ".") + } + + // Split key paths delimited on "." + keyPaths := [][]string{} + for _, keyString := range keys { + keyParts := strings.Split(keyString, ".") + keyPaths = append(keyPaths, keyParts) + } + + // Load target env config + if err := componentConfig.Load(environment); err != nil { + return err + } + + // Remove all keys form the config + for _, keyPath := range keyPaths { + if err = componentConfig.UnsetConfig(subcomponentPath, keyPath); err != nil { + return err + } + } + + // Write out the config + return componentConfig.Write(environment) +} + +type unsetCmdOpts struct { + subcomponent string + environment string +} + +func newUnsetCmd() *cobra.Command { + opts := &unsetCmdOpts{} + + cmd := &cobra.Command{ + Use: "unset [--subcomponent subcomponent] ...", + Short: "Unsets a config value for a component for a particular config environment in the Fabrikate definition; deleting the key form the config.", + Long: `Unsets a config value for a component for a particular config environment in the Fabrikate definition; deleting the key from the config. +eg. +$ fab unset --environment prod data.replicas username + +Unsets the key of 'data.replicas' and 'username' in the 'prod' config for the current component. + +$ fab unset --subcomponent "myapp" endpoint + +Unsets the key of 'endpoint' in the 'common' config (the default) for subcomponent 'myapp'. + +$ fab unset --subcomponent "myapp.mysubcomponent" data.replicas + +Unsets the subkey "replicas" in the key 'data' in the 'common' config (the default) for the subcomponent 'mysubcomponent' of the subcomponent 'myapp'. +`, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) < 1 && inputFile == "" { + return errors.New("'unset' takes a config path as the first parameter and one or more keys to remove thereafter") + } + + return unset(args, opts.environment, opts.subcomponent) + }, + } + + cmd.PersistentFlags().StringVar(&opts.environment, "environment", "common", "Environment this configuration should apply to") + cmd.PersistentFlags().StringVar(&opts.subcomponent, "subcomponent", "", "Subcomponent this configuration should apply to") + + return cmd +} + +func init() { + rootCmd.AddCommand(newUnsetCmd()) +} diff --git a/cmd/unset_test.go b/cmd/unset_test.go new file mode 100644 index 0000000..bb01457 --- /dev/null +++ b/cmd/unset_test.go @@ -0,0 +1,70 @@ +package cmd + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/timfpark/yaml" +) + +func TestUnset(t *testing.T) { + // This test changes the cwd. Must change back so any tests following don't break + cwd, err := os.Getwd() + assert.Nil(t, err) + defer func() { + _ = os.Chdir(cwd) + }() + + err = os.Chdir("../test/fixtures/unset") + _ = os.RemoveAll("config") + assert.Nil(t, err) + + // Set config for two nested subcomponents + err = Set("common", "a", []string{"x.y.z=abc", "foo=foo"}, false, "") + err = Set("common", "b", []string{"xyz=abc", "foo.bar=baz"}, false, "") + assert.Nil(t, err) + + // Remove the config for x.y.z; x.y should be an empty map afterwards + err = unset([]string{"x.y.z"}, "common", "a") + assert.Nil(t, err) + + // Removing the a non existant key should return an error + err = unset([]string{"x.y.i.do.not.exist"}, "common", "a") + assert.NotNil(t, err) + + // Unsetting a key path which contains a key to a non-map should return an error + err = unset([]string{"foo.bar.baz"}, "common", "b") + assert.NotNil(t, err) + + // Read the config yaml + commonConfig := map[string]interface{}{} + commonBytes, err := ioutil.ReadFile("config/common.yaml") + assert.Nil(t, err) + yaml.Unmarshal(commonBytes, &commonConfig) + + assert.EqualValues( + t, + map[string]interface{}{ + "subcomponents": map[string]interface{}{ + "a": map[string]interface{}{ + "config": map[string]interface{}{ + "x": map[string]interface{}{ + "y": map[string]interface{}{}, + }, + "foo": "foo", + }, + }, + "b": map[string]interface{}{ + "config": map[string]interface{}{ + "xyz": "abc", + "foo": map[string]interface{}{ + "bar": "baz", + }, + }, + }, + }, + }, + commonConfig) +} diff --git a/core/componentConfig.go b/core/componentConfig.go index 2cabca0..3a58d55 100644 --- a/core/componentConfig.go +++ b/core/componentConfig.go @@ -101,18 +101,28 @@ func (cc *ComponentConfig) HasComponentConfig(path []string) bool { // SetComponentConfig sets the `value` of the given configuration setting. // The configuration setting is indicated via a configuration `path`. -func (cc *ComponentConfig) SetComponentConfig(path []string, value string) { +// If any of the map keys specified within the path lead to non maps (besides +// the last one), it will overwrite the value with an empty map. +func (cc *ComponentConfig) SetComponentConfig(path []string, value string) (err error) { configLevel := cc.Config createdNewConfig := false for levelIndex, pathPart := range path { // if this key is not the final one, we need to decend in the config. if levelIndex < len(path)-1 { + // If key does not exist, create an empty map if _, ok := configLevel[pathPart]; !ok { createdNewConfig = true configLevel[pathPart] = map[string]interface{}{} } + // If the key leads to a non-map value, return an error + if _, isAMap := configLevel[pathPart].(map[string]interface{}); !isAMap { + currentPath := strings.Join(path[:levelIndex+1], ".") + fullPath := strings.Join(path, ".") + return fmt.Errorf("Config path '%s' points to a non-map value; cannot set '%s' to '%s'", currentPath, fullPath, value) + } + configLevel = configLevel[pathPart].(map[string]interface{}) } else { if createdNewConfig { @@ -121,6 +131,46 @@ func (cc *ComponentConfig) SetComponentConfig(path []string, value string) { configLevel[pathPart] = value } } + + return err +} + +// UnsetComponentConfig unsets a key from a component config (deleteing the key +// from the map). If any of the keys provided in `keyPath` are not found, this +// is treated as a noop. +func (cc *ComponentConfig) UnsetComponentConfig(keyPath []string) (err error) { + configLevel := cc.Config + + // iterate down the config tree until reaching the second to last level; so we can delete the last + for pathIndex, key := range keyPath[:len(keyPath)-1] { + // Return an error if any keys don't exist + if _, exists := configLevel[key]; !exists { + currentKeyPath := strings.Join(keyPath[:pathIndex+1], ".") + targetKeyPath := strings.Join(keyPath, ".") + return fmt.Errorf("Config key path '%s' not found. Unable to remove config entry '%s'", currentKeyPath, targetKeyPath) + } + + // Return early/gracefully if keys specify non-maps + ok := true + configLevel, ok = configLevel[key].(map[string]interface{}) + if !ok { + currentKeyPath := strings.Join(keyPath[:pathIndex+1], ".") + targetKeyPath := strings.Join(keyPath, ".") + return fmt.Errorf("Config key path '%s' points to a non-map entry. Unable to remove config entry '%s'", currentKeyPath, targetKeyPath) + } + } + + // Check to see if the last key exists; if so delete, else return an error + lastKey := keyPath[len(keyPath)-1] + if _, exists := configLevel[lastKey]; exists { + // Remove the last key + delete(configLevel, keyPath[len(keyPath)-1]) + } else { + targetKeyPath := strings.Join(keyPath, ".") + return fmt.Errorf("Target key '%s' does not exist in config; unable to remove key '%s'", lastKey, targetKeyPath) + } + + return err } // GetSubcomponentConfig returns the subcomponent config of the given component. @@ -167,9 +217,17 @@ func (cc *ComponentConfig) HasSubcomponentConfig(subcomponentPath []string) bool } // SetConfig sets or creates the configuration `value` for the given `subcomponentPath`. -func (cc *ComponentConfig) SetConfig(subcomponentPath []string, path []string, value string) { +func (cc *ComponentConfig) SetConfig(subcomponentPath []string, path []string, value string) (err error) { + subcomponentConfig := cc.GetSubcomponentConfig(subcomponentPath) + err = subcomponentConfig.SetComponentConfig(path, value) + + return err +} + +// UnsetConfig removes a key from a the target subcomponent config +func (cc *ComponentConfig) UnsetConfig(subcomponentPath []string, path []string) error { subcomponentConfig := cc.GetSubcomponentConfig(subcomponentPath) - subcomponentConfig.SetComponentConfig(path, value) + return subcomponentConfig.UnsetComponentConfig(path) } // MergeNamespaces merges the namespaces between the componentConfig passed and this diff --git a/core/componentConfig_test.go b/core/componentConfig_test.go index 14121cb..f475ac3 100644 --- a/core/componentConfig_test.go +++ b/core/componentConfig_test.go @@ -56,19 +56,23 @@ func TestSet(t *testing.T) { assert.Nil(t, err) // can override a config value successfully - config.SetConfig([]string{}, []string{"foo"}, "fee") + err = config.SetConfig([]string{}, []string{"foo"}, "fee") + assert.Nil(t, err) assert.Equal(t, "fee", config.Config["foo"]) // can create a value successfully - config.SetConfig([]string{}, []string{"new"}, "value") + err = config.SetConfig([]string{}, []string{"new"}, "value") + assert.Nil(t, err) assert.Equal(t, "value", config.Config["new"]) // can override a subcomponent value successfully - config.SetConfig([]string{"myapp"}, []string{"zoo"}, "zee") + err = config.SetConfig([]string{"myapp"}, []string{"zoo"}, "zee") + assert.Nil(t, err) assert.Equal(t, "zee", config.Subcomponents["myapp"].Config["zoo"]) // can create a new deeper subcomponent config level successfully - config.SetConfig([]string{"myapp"}, []string{"data", "storageClass"}, "fast") + err = config.SetConfig([]string{"myapp"}, []string{"data", "storageClass"}, "fast") + assert.Nil(t, err) dataMap := config.Subcomponents["myapp"].Config["data"].(map[string]interface{}) assert.Equal(t, "fast", dataMap["storageClass"]) } diff --git a/test/fixtures/set/config/test.yaml b/test/fixtures/set/config/test.yaml index 470c914..3fe77aa 100644 --- a/test/fixtures/set/config/test.yaml +++ b/test/fixtures/set/config/test.yaml @@ -1,4 +1,6 @@ config: + a: + b: foobar fod: rad foo: faa subcomponents: diff --git a/test/fixtures/unset/config/common.yaml b/test/fixtures/unset/config/common.yaml new file mode 100644 index 0000000..8f37dca --- /dev/null +++ b/test/fixtures/unset/config/common.yaml @@ -0,0 +1,11 @@ +subcomponents: + a: + config: + foo: foo + x: + "y": {} + b: + config: + foo: + bar: baz + xyz: abc From d90a045b25653a3c826dc0ae39271ad6b0d31e98 Mon Sep 17 00:00:00 2001 From: Evan Louie Date: Thu, 8 Aug 2019 10:44:58 -0700 Subject: [PATCH 2/2] stash --- cmd/remove.go | 3 +-- cmd/unset.go | 31 ++++++++++++++++++++++--------- core/componentConfig.go | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/cmd/remove.go b/cmd/remove.go index 0063072..79a6b43 100644 --- a/cmd/remove.go +++ b/cmd/remove.go @@ -33,8 +33,7 @@ func Remove(subcomponent core.Component) (err error) { } } - err = component.RemoveSubcomponent(subcomponent) - if err != nil { + if err = component.RemoveSubcomponent(subcomponent); err != nil { return err } diff --git a/cmd/unset.go b/cmd/unset.go index fbae67f..d0e5513 100644 --- a/cmd/unset.go +++ b/cmd/unset.go @@ -4,11 +4,13 @@ import ( "errors" "strings" + "github.com/kyokomi/emoji" "github.com/microsoft/fabrikate/core" + log "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) -func unset(keys []string, environment, subcomponent string) (err error) { +func unset(keys []string, environment, subcomponent string, removeSubcomponent bool) (err error) { // Load config componentConfig := core.NewComponentConfig(".") @@ -30,11 +32,19 @@ func unset(keys []string, environment, subcomponent string) (err error) { return err } - // Remove all keys form the config - for _, keyPath := range keyPaths { - if err = componentConfig.UnsetConfig(subcomponentPath, keyPath); err != nil { + // Remove the entire target component config if specified + if removeSubcomponent { + log.Info(emoji.Sprintf("")) + if err = componentConfig.RemoveComponentConfig(subcomponentPath); err != nil { return err } + } else { + // Remove all keys from the config + for _, keyPath := range keyPaths { + if err = componentConfig.UnsetConfig(subcomponentPath, keyPath); err != nil { + return err + } + } } // Write out the config @@ -42,8 +52,9 @@ func unset(keys []string, environment, subcomponent string) (err error) { } type unsetCmdOpts struct { - subcomponent string - environment string + subcomponent string + environment string + removeSubcomponent bool } func newUnsetCmd() *cobra.Command { @@ -71,12 +82,14 @@ Unsets the subkey "replicas" in the key 'data' in the 'common' config (the defau return errors.New("'unset' takes a config path as the first parameter and one or more keys to remove thereafter") } - return unset(args, opts.environment, opts.subcomponent) + removeComponent := cmd.Flag("remove-component").Value.String() == "true" + return unset(args, opts.environment, opts.subcomponent, removeComponent) }, } - cmd.PersistentFlags().StringVar(&opts.environment, "environment", "common", "Environment this configuration should apply to") - cmd.PersistentFlags().StringVar(&opts.subcomponent, "subcomponent", "", "Subcomponent this configuration should apply to") + cmd.PersistentFlags().StringVar(&opts.environment, "environment", "common", "Environment this configuration should be removed from") + cmd.PersistentFlags().StringVar(&opts.subcomponent, "subcomponent", "", "Subcomponent this configuration should be removed from") + cmd.PersistentFlags().Bool("remove-component", false, "Remove the component config specified in --subcomponent entirely") return cmd } diff --git a/core/componentConfig.go b/core/componentConfig.go index 3a58d55..27bc3bb 100644 --- a/core/componentConfig.go +++ b/core/componentConfig.go @@ -150,7 +150,7 @@ func (cc *ComponentConfig) UnsetComponentConfig(keyPath []string) (err error) { return fmt.Errorf("Config key path '%s' not found. Unable to remove config entry '%s'", currentKeyPath, targetKeyPath) } - // Return early/gracefully if keys specify non-maps + // Return an if any keys in key path specify non-maps ok := true configLevel, ok = configLevel[key].(map[string]interface{}) if !ok { @@ -195,6 +195,38 @@ func (cc *ComponentConfig) GetSubcomponentConfig(subcomponentPath []string) (sub return subcomponentConfig } +// RemoveComponentConfig removes the subcomponent specified in subcomponentPath. +// Going down the component config tree start at cc until reaching the final +// component. Returns an error if any component in the path is not found. +func (cc *ComponentConfig) RemoveComponentConfig(subcomponentPath []string) (err error) { + subcomponentConfig := *cc + + // Iterate through config tree until subcomponentConfig == second to last component + for componentIndex, subcomponentName := range subcomponentPath[:len(subcomponentPath)] { + // Return an error if the subcomponent does not exist + if _, ok := subcomponentConfig.Subcomponents[subcomponentName]; !ok { + currentComponentPath := subcomponentPath[:componentIndex+1] + targetComponentPath := strings.Join(subcomponentPath, ".") + return fmt.Errorf("Component configuration for '%s' not found in config path '%s'; unable to delete component configuration '%s'", subcomponentName, currentComponentPath, targetComponentPath) + } + + subcomponentConfig = subcomponentConfig.Subcomponents[subcomponentName] + } + + // If second to last config doesn't contain target component config, return error + lastComponentName := subcomponentPath[len(subcomponentPath)+1] + if _, ok := subcomponentConfig.Subcomponents[lastComponentName]; !ok { + currentComponentPath := subcomponentPath[:len(subcomponentPath)] + targetComponentPath := strings.Join(subcomponentPath, ".") + return fmt.Errorf("Component configuration for '%s' not found in config path '%s'; unable to delete component configuration '%s'", lastComponentName, currentComponentPath, targetComponentPath) + } + + // Delete the target component config + delete(subcomponentConfig.Subcomponents, lastComponentName) + + return err +} + // HasSubcomponentConfig checks if a component contains the given subcomponents of the `subcomponentPath` // // Returns true if it contains the subcomponent, otherwise it returns false