Skip to content

Commit

Permalink
Merge pull request #1628 from estroz/bugfix/v3-config-fields
Browse files Browse the repository at this point in the history 
pkg/model/config: only project versions >= v3 can use plugins config options
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Aug 11, 2020
2 parents 4ebcf31 + b71774a commit 0ef04e0
Show file tree
Hide file tree
Showing 3 changed files with 195 additions and 140 deletions.
246 changes: 145 additions & 101 deletions internal/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,67 +27,69 @@ import (
)

var _ = Describe("Config", func() {
It("should save correctly", func() {
var (
cfg Config
expectedConfigStr string
)

By("saving empty config")
Expect(cfg.Save()).To(HaveOccurred())

By("saving empty config with path")
cfg = Config{
fs: afero.NewMemMapFs(),
path: DefaultPath,
}
Expect(cfg.Save()).ToNot(HaveOccurred())
cfgBytes, err := afero.ReadFile(cfg.fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(string(cfgBytes)).To(Equal(expectedConfigStr))

By("saving config version 2")
cfg = Config{
Config: config.Config{
Version: config.Version2,
Repo: "github.com/example/project",
Domain: "example.com",
},
fs: afero.NewMemMapFs(),
path: DefaultPath,
}
expectedConfigStr = `domain: example.com
var (
cfg Config
expectedConfigStr string
)

Context("with valid keys", func() {
It("should save correctly", func() {

By("saving empty config")
Expect(cfg.Save()).To(HaveOccurred())

By("saving empty config with path")
cfg = Config{
fs: afero.NewMemMapFs(),
path: DefaultPath,
}
Expect(cfg.Save()).To(Succeed())
cfgBytes, err := afero.ReadFile(cfg.fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(string(cfgBytes)).To(Equal(expectedConfigStr))

By("saving config version 2")
cfg = Config{
Config: config.Config{
Version: config.Version2,
Repo: "github.com/example/project",
Domain: "example.com",
},
fs: afero.NewMemMapFs(),
path: DefaultPath,
}
expectedConfigStr = `domain: example.com
repo: github.com/example/project
version: "2"
`
Expect(cfg.Save()).ToNot(HaveOccurred())
cfgBytes, err = afero.ReadFile(cfg.fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(string(cfgBytes)).To(Equal(expectedConfigStr))

By("saving config version 2 with plugin config")
cfg = Config{
Config: config.Config{
Version: config.Version2,
Repo: "github.com/example/project",
Domain: "example.com",
Plugins: config.PluginConfigs{
"plugin-x": map[string]interface{}{
"data-1": "single plugin datum",
},
"plugin-y/v1": map[string]interface{}{
"data-1": "plugin value 1",
"data-2": "plugin value 2",
"data-3": []string{"plugin value 3", "plugin value 4"},
Expect(cfg.Save()).To(Succeed())
cfgBytes, err = afero.ReadFile(cfg.fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(string(cfgBytes)).To(Equal(expectedConfigStr))

By("saving config version 3-alpha with plugin config")
cfg = Config{
Config: config.Config{
Version: config.Version3Alpha,
Repo: "github.com/example/project",
Domain: "example.com",
Plugins: config.PluginConfigs{
"plugin-x": map[string]interface{}{
"data-1": "single plugin datum",
},
"plugin-y/v1": map[string]interface{}{
"data-1": "plugin value 1",
"data-2": "plugin value 2",
"data-3": []string{"plugin value 3", "plugin value 4"},
},
},
},
},
fs: afero.NewMemMapFs(),
path: DefaultPath,
}
expectedConfigStr = `domain: example.com
fs: afero.NewMemMapFs(),
path: DefaultPath,
}
expectedConfigStr = `domain: example.com
repo: github.com/example/project
version: "2"
version: "3-alpha"
plugins:
plugin-x:
data-1: single plugin datum
Expand All @@ -98,39 +100,39 @@ plugins:
- plugin value 3
- plugin value 4
`
Expect(cfg.Save()).ToNot(HaveOccurred())
cfgBytes, err = afero.ReadFile(cfg.fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(string(cfgBytes)).To(Equal(expectedConfigStr))
})
Expect(cfg.Save()).To(Succeed())
cfgBytes, err = afero.ReadFile(cfg.fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(string(cfgBytes)).To(Equal(expectedConfigStr))
})

It("should load correctly", func() {
var (
fs = afero.NewMemMapFs()
configStr string
expectedConfig config.Config
)
It("should load correctly", func() {
var (
fs = afero.NewMemMapFs()
configStr string
expectedConfig config.Config
)

By("loading config version 2")
configStr = `domain: example.com
By("loading config version 2")
configStr = `domain: example.com
repo: github.com/example/project
version: "2"`
expectedConfig = config.Config{
Version: config.Version2,
Repo: "github.com/example/project",
Domain: "example.com",
}
err := afero.WriteFile(fs, DefaultPath, []byte(configStr), os.ModePerm)
Expect(err).ToNot(HaveOccurred())
cfg, err := readFrom(fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(cfg).To(BeEquivalentTo(expectedConfig))

By("loading config version 2 with plugin config")
fs = afero.NewMemMapFs()
configStr = `domain: example.com
expectedConfig = config.Config{
Version: config.Version2,
Repo: "github.com/example/project",
Domain: "example.com",
}
err := afero.WriteFile(fs, DefaultPath, []byte(configStr), os.ModePerm)
Expect(err).ToNot(HaveOccurred())
cfg, err := readFrom(fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(cfg).To(BeEquivalentTo(expectedConfig))

By("loading config version 3-alpha with plugin config")
fs = afero.NewMemMapFs()
configStr = `domain: example.com
repo: github.com/example/project
version: "2"
version: "3-alpha"
plugins:
plugin-x:
data-1: single plugin datum
Expand All @@ -140,25 +142,67 @@ plugins:
data-3:
- "plugin value 3"
- "plugin value 4"`
expectedConfig = config.Config{
Version: config.Version2,
Repo: "github.com/example/project",
Domain: "example.com",
Plugins: config.PluginConfigs{
"plugin-x": map[string]interface{}{
"data-1": "single plugin datum",
expectedConfig = config.Config{
Version: config.Version2,
Repo: "github.com/example/project",
Domain: "example.com",
Plugins: config.PluginConfigs{
"plugin-x": map[string]interface{}{
"data-1": "single plugin datum",
},
"plugin-y/v1": map[string]interface{}{
"data-1": "plugin value 1",
"data-2": "plugin value 2",
"data-3": []interface{}{"plugin value 3", "plugin value 4"},
},
},
"plugin-y/v1": map[string]interface{}{
"data-1": "plugin value 1",
"data-2": "plugin value 2",
"data-3": []interface{}{"plugin value 3", "plugin value 4"},
}
err = afero.WriteFile(fs, DefaultPath, []byte(configStr), os.ModePerm)
Expect(err).ToNot(HaveOccurred())
cfg, err = readFrom(fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(cfg).To(Equal(expectedConfig))
})
})

Context("with invalid keys", func() {
It("should return a save error", func() {
By("saving config version 2 with plugin config")
cfg = Config{
Config: config.Config{
Version: config.Version2,
Repo: "github.com/example/project",
Domain: "example.com",
Plugins: config.PluginConfigs{
"plugin-x": map[string]interface{}{
"data-1": "single plugin datum",
},
},
},
},
}
err = afero.WriteFile(fs, DefaultPath, []byte(configStr), os.ModePerm)
Expect(err).ToNot(HaveOccurred())
cfg, err = readFrom(fs, DefaultPath)
Expect(err).NotTo(HaveOccurred())
Expect(cfg).To(Equal(expectedConfig))
fs: afero.NewMemMapFs(),
path: DefaultPath,
}
Expect(cfg.Save()).NotTo(Succeed())
})

It("should return a load error", func() {
var (
fs afero.Fs
configStr string
err error
)
By("loading config version 2 with plugin config")
fs = afero.NewMemMapFs()
configStr = `domain: example.com
repo: github.com/example/project
version: "3-alpha"
plugins:
plugin-x:
data-1: single plugin datum`
err = afero.WriteFile(fs, DefaultPath, []byte(configStr), os.ModePerm)
Expect(err).ToNot(HaveOccurred())
_, err = readFrom(fs, DefaultPath)
Expect(err).To(HaveOccurred())
})
})
})
29 changes: 15 additions & 14 deletions pkg/model/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ func (c Config) Marshal() ([]byte, error) {
content = []byte{}
}
// Append extra fields to put them at the config's bottom, unless the
// project is v1 which does not support extra fields.
if !cfg.IsV1() && len(c.Plugins) != 0 {
// project version is < v3 which do not support a plugins field.
if cfg.IsV3() && len(c.Plugins) != 0 {
pluginConfigBytes, err := yaml.Marshal(Config{Plugins: c.Plugins})
if err != nil {
return nil, fmt.Errorf("error marshalling project configuration extra fields: %v", err)
Expand All @@ -169,20 +169,21 @@ func (c *Config) Unmarshal(b []byte) error {
if err := yaml.UnmarshalStrict(b, c); err != nil {
return fmt.Errorf("error unmarshalling project configuration: %v", err)
}
// v1 projects do not support extra fields.
if c.IsV1() {
// Project versions < v3 do not support a plugins field.
if !c.IsV3() {
c.Plugins = nil
}
return nil
}

// EncodePluginConfig encodes a config object into c by overwriting the existing
// object stored under key. This method is intended to be used for custom
// configuration objects.
// configuration objects, which were introduced in project version 3-alpha.
// EncodePluginConfig will return an error if used on any project version < v3.
func (c *Config) EncodePluginConfig(key string, configObj interface{}) error {
// Short-circuit v1
if c.IsV1() {
return fmt.Errorf("v1 project configs do not have extra fields")
// Short-circuit project versions < v3.
if !c.IsV3() {
return fmt.Errorf("project versions < v3 do not support extra fields")
}

// Get object's bytes and set them under key in extra fields.
Expand All @@ -201,13 +202,13 @@ func (c *Config) EncodePluginConfig(key string, configObj interface{}) error {
return nil
}

// DecodePluginConfig decodes a plugin config stored in c into configObj. This
// method is intended to be used for custom configuration objects.
// configObj must be a pointer.
// DecodePluginConfig decodes a plugin config stored in c into configObj, which must be a pointer
// This method is intended to be used for custom configuration objects, which were introduced
// in project version 3-alpha. EncodePluginConfig will return an error if used on any project version < v3.
func (c Config) DecodePluginConfig(key string, configObj interface{}) error {
// Short-circuit v1
if c.IsV1() {
return fmt.Errorf("v1 project configs do not have extra fields")
// Short-circuit project versions < v3.
if !c.IsV3() {
return fmt.Errorf("project versions < v3 do not support extra fields")
}
if len(c.Plugins) == 0 {
return nil
Expand Down

0 comments on commit 0ef04e0

Please sign in to comment.