config: allow addition of arbitrary extra fields to Config

internal/config/config.go

@@ -19,11 +19,11 @@ package config
 import (
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"os"
 
+	"github.com/spf13/afero"
+
 	"sigs.k8s.io/kubebuilder/pkg/model/config"
-	"sigs.k8s.io/yaml"
 )
 
 const (
@@ -34,9 +34,9 @@ const (
 	DefaultVersion = config.Version2
 )
 
-func exists(path string) (bool, error) {
+func exists(fs afero.Fs, path string) (bool, error) {
 	// Look up the file
-	_, err := os.Stat(path)
+	_, err := fs.Stat(path)
 
 	// If we could find it the file exists
 	if err == nil || os.IsExist(err) {
@@ -50,15 +50,15 @@ func exists(path string) (bool, error) {
 	return false, err
 }
 
-func readFrom(path string) (c config.Config, err error) {
+func readFrom(fs afero.Fs, path string) (c config.Config, err error) {
 	// Read the file
-	in, err := ioutil.ReadFile(path) //nolint:gosec
+	in, err := afero.ReadFile(fs, path) //nolint:gosec
 	if err != nil {
 		return
 	}
 
 	// Unmarshal the file content
-	if err = yaml.Unmarshal(in, &c); err != nil {
+	if err = config.Unmarshal(in, &c); err != nil {
 		return
 	}
 
@@ -77,8 +77,7 @@ func Read() (*config.Config, error) {
 
 // ReadFrom obtains the configuration from the provided path but doesn't allow to persist changes
 func ReadFrom(path string) (*config.Config, error) {
-	c, err := readFrom(path)
-
+	c, err := readFrom(afero.NewOsFs(), path)
 	return &c, err
 }
 
@@ -92,6 +91,8 @@ type Config struct {
 	path string
 	// mustNotExist requires the file not to exist when saving it
 	mustNotExist bool
+	// fs is for testing.
+	fs afero.Fs
 }
 
 // New creates a new configuration that will be stored at the provided path
@@ -102,6 +103,7 @@ func New(path string) *Config {
 		},
 		path:         path,
 		mustNotExist: true,
+		fs:           afero.NewOsFs(),
 	}
 }
 
@@ -122,13 +124,16 @@ func LoadInitialized() (*Config, error) {
 
 // LoadFrom obtains the configuration from the provided path allowing to persist changes (Save method)
 func LoadFrom(path string) (*Config, error) {
-	c, err := readFrom(path)
-
-	return &Config{Config: c, path: path}, err
+	fs := afero.NewOsFs()
+	c, err := readFrom(fs, path)
+	return &Config{Config: c, path: path, fs: fs}, err
 }
 
 // Save saves the configuration information
 func (c Config) Save() error {
+	if c.fs == nil {
+		c.fs = afero.NewOsFs()
+	}
 	// If path is unset, it was created directly with `Config{}`
 	if c.path == "" {
 		return saveError{errors.New("no information where it should be stored, " +
@@ -138,7 +143,7 @@ func (c Config) Save() error {
 	// If it is a new configuration, the path should not exist yet
 	if c.mustNotExist {
 		// Lets check that the file doesn't exist
-		alreadyExists, err := exists(c.path)
+		alreadyExists, err := exists(c.fs, c.path)
 		if err != nil {
 			return saveError{err}
 		}
@@ -148,13 +153,13 @@ func (c Config) Save() error {
 	}
 
 	// Marshall into YAML
-	content, err := yaml.Marshal(c)
+	content, err := c.Marshal()
 	if err != nil {
-		return saveError{fmt.Errorf("error marshalling project configuration: %v", err)}
+		return saveError{err}
 	}
 
 	// Write the marshalled configuration
-	err = ioutil.WriteFile(c.path, content, 0600)
+	err = afero.WriteFile(c.fs, c.path, content, 0600)
 	if err != nil {
 		return saveError{fmt.Errorf("failed to save configuration to %s: %v", c.path, err)}
 	}

internal/config/config_test.go

@@ -0,0 +1,192 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package config
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/spf13/afero"
+
+	"sigs.k8s.io/kubebuilder/pkg/model/config"
+)
+
+func TestSaveReadFrom(t *testing.T) {
+	cases := []struct {
+		description  string
+		config       Config
+		expConfig    config.Config
+		expConfigStr string
+		wantSaveErr  bool
+	}{
+		{
+			description:  "empty config",
+			config:       Config{},
+			expConfigStr: "",
+			wantSaveErr:  true,
+		},
+		{
+			description:  "empty config with path",
+			config:       Config{path: DefaultPath},
+			expConfig:    config.Config{Version: config.Version1},
+			expConfigStr: "",
+		},
+		{
+			description: "config version 1",
+			config: Config{
+				Config: config.Config{
+					Version: config.Version1,
+					Repo:    "github.com/example/project",
+					Domain:  "example.com",
+				},
+				path: DefaultPath,
+			},
+			expConfig: config.Config{
+				Version: config.Version1,
+				Repo:    "github.com/example/project",
+				Domain:  "example.com",
+			},
+			expConfigStr: `domain: example.com
+repo: github.com/example/project
+version: "1"`,
+		},
+		{
+			description: "config version 1 with extra fields",
+			config: Config{
+				Config: config.Config{
+					Version: config.Version1,
+					Repo:    "github.com/example/project",
+					Domain:  "example.com",
+					ExtraFields: map[string]interface{}{
+						"plugin-x": "single plugin datum",
+					},
+				},
+				path: DefaultPath,
+			},
+			expConfig: config.Config{
+				Version: config.Version1,
+				Repo:    "github.com/example/project",
+				Domain:  "example.com",
+			},
+			expConfigStr: `domain: example.com
+repo: github.com/example/project
+version: "1"`,
+		},
+		{
+			description: "config version 2 without extra fields",
+			config: Config{
+				Config: config.Config{
+					Version: config.Version2,
+					Repo:    "github.com/example/project",
+					Domain:  "example.com",
+				},
+				path: DefaultPath,
+			},
+			expConfig: config.Config{
+				Version: config.Version2,
+				Repo:    "github.com/example/project",
+				Domain:  "example.com",
+			},
+			expConfigStr: `domain: example.com
+repo: github.com/example/project
+version: "2"`,
+		},
+		{
+			description: "config version 2 with extra fields",
+			config: Config{
+				Config: config.Config{
+					Version: config.Version2,
+					Repo:    "github.com/example/project",
+					Domain:  "example.com",
+					ExtraFields: map[string]interface{}{
+						"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"},
+						},
+					},
+				},
+				path: DefaultPath,
+			},
+			expConfig: config.Config{
+				Version: config.Version2,
+				Repo:    "github.com/example/project",
+				Domain:  "example.com",
+				ExtraFields: map[string]interface{}{
+					"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"},
+					},
+				},
+			},
+			expConfigStr: `domain: example.com
+repo: github.com/example/project
+version: "2"
+plugins:
+  plugin-x:
+    data-1: single plugin datum
+  plugin-y/v1:
+    data-1: plugin value 1
+    data-2: plugin value 2
+    data-3:
+    - plugin value 3
+    - plugin value 4`,
+		},
+	}
+
+	for _, c := range cases {
+		// Setup
+		c.config.fs = afero.NewMemMapFs()
+
+		// Test Save
+		err := c.config.Save()
+		if err != nil {
+			if !c.wantSaveErr {
+				t.Errorf("%s: expected Save to succeed, got error: %s", c.description, err)
+			}
+			continue
+		} else if c.wantSaveErr {
+			t.Errorf("%s: expected Save to fail, got no error", c.description)
+			continue
+		}
+		configBytes, err := afero.ReadFile(c.config.fs, c.config.path)
+		if err != nil {
+			t.Fatalf("%s: %s", c.description, err)
+		}
+		if c.expConfigStr != strings.TrimSpace(string(configBytes)) {
+			t.Errorf("%s: compare saved configs\nexpected:\n%s\n\nreturned:\n%s",
+				c.description, c.expConfigStr, string(configBytes))
+		}
+
+		// Test readFrom
+		cfg, err := readFrom(c.config.fs, c.config.path)
+		if err != nil {
+			t.Fatalf("%s: %s", c.description, err)
+		}
+		if !reflect.DeepEqual(c.expConfig, cfg) {
+			t.Errorf("%s: compare read configs\nexpected:\n%#v\n\nreturned:\n%#v", c.description, c.expConfig, cfg)
+		}
+	}
+}