[FEATURE/FIX] Viper Configuration and JSON Schema (#472)

Author: iwilltry42

.golangci.yml

@@ -0,0 +1,3 @@
+linters-settings:
+  errcheck:
+    check-blank: false # to keep `_ = viper.BindPFlag(...)` from throwing errors

CHANGELOG.md

@@ -1,5 +1,34 @@
 # Changelog
 
+## v4.1.0
+
+### Important
+
+- Using Viper brings us lots of nice features, but also one problem:
+  - We had to switch StringArray flags to StringSlice flags, which
+    - allow to use multiple flag values comma-separated in a single flag, but also
+    - split flag values that contain a comma into separate parts (and we cannot handle issues that arise due to this)
+      - so if you rely on commas in your flag values (e.g. for `--env X=a,b,c`), please consider filing an issue or supporting <https://github.com/spf13/viper/issues/246> and <https://github.com/spf13/viper/pull/398>
+      - `--env X=a,b,c` would be treated the same as `--env X=a`, `--env b`, `--env c`
+
+### Features & Enhancements
+
+- use [viper](https://github.com/spf13/viper) for configuration management
+  - takes over the job of properly fetching and merging config options from
+    - CLI arguments/flags
+    - environment variables
+    - config file
+  - this also fixes some issues with using the config file (like cobra defaults overriding config file values)
+- add JSON-Schema validation for the `Simple` config file schema
+- new config version `k3d.io/v1alpha2` (some naming changes)
+  - `exposeAPI` -> `kubeAPI`
+  - `options.k3d.noRollback` -> `options.k3d.disableRollback`
+  - `options.k3d.prepDisableHostIPInjection` -> `options.k3d.disableHostIPInjection`
+
+### Misc
+
+- tests/e2e: add config override test
+
 ## v4.0.0
 
 ### Breaking Changes

cmd/cluster/clusterCreate.go

@@ -25,7 +25,7 @@ import (
 	"fmt"
 	"os"
 
-	config "github.com/rancher/k3d/v4/pkg/config/v1alpha1"
+	config "github.com/rancher/k3d/v4/pkg/config/v1alpha2"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )

cmd/config/configInit.go

@@ -94,7 +94,6 @@ func Execute() {
 }
 
 func init() {
-	cobra.OnInitialize(initLogging, initRuntime)
 
 	rootCmd.PersistentFlags().BoolVar(&flags.debugLogging, "verbose", false, "Enable verbose output (debug logging)")
 	rootCmd.PersistentFlags().BoolVar(&flags.traceLogging, "trace", false, "Enable super verbose output (trace logging)")
@@ -119,6 +118,9 @@ func init() {
 			printVersion()
 		},
 	})
+
+	// Init
+	cobra.OnInitialize(initLogging, initRuntime)
 }
 
 // initLogging initializes the logger

cmd/root.go

@@ -41,8 +41,7 @@ func ParsePortExposureSpec(exposedPortSpec, internalPort string) (*k3d.ExposureO
 	match := apiPortRegexp.FindStringSubmatch(exposedPortSpec)
 
 	if len(match) == 0 {
-		log.Errorln("Failed to parse Port Exposure specification")
-		return nil, fmt.Errorf("Port Exposure Spec format error: Must be [(HostIP|HostName):]HostPort")
+		return nil, fmt.Errorf("Failed to parse Port Exposure specification '%s': Format must be [(HostIP|HostName):]HostPort", exposedPortSpec)
 	}
 
 	submatches := util.MapSubexpNames(apiPortRegexp.SubexpNames(), match)

cmd/util/ports.go

@@ -29,7 +29,7 @@ This file can also be used for providing additional information necessary for ac
 If you're using a `SimpleConfig` file to configure your k3d cluster, you may as well embed the registries.yaml in there directly:
 
 ```yaml
-apiVersion: k3d.io/v1alpha1
+apiVersion: k3d.io/v1alpha2
 kind: Simple
 name: test
 servers: 1

docs/usage/guides/registries.md

@@ -39,11 +39,14 @@ require (
 	github.com/spf13/viper v1.7.1
 	github.com/stretchr/testify v1.6.1 // indirect
 	github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 // indirect
+	github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect
+	github.com/xeipuuv/gojsonschema v1.2.0
 	golang.org/x/sync v0.0.0-20190423024810-112230192c58
 	google.golang.org/grpc v1.34.0 // indirect
 	gopkg.in/ini.v1 v1.58.0 // indirect
-	gopkg.in/yaml.v2 v2.3.0
+	gopkg.in/yaml.v2 v2.4.0
 	gotest.tools v2.2.0+incompatible
 	gotest.tools/v3 v3.0.3 // indirect
 	k8s.io/client-go v0.17.0
+	sigs.k8s.io/yaml v1.2.0
 )

go.mod

@@ -448,6 +448,7 @@ github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40T
 github.com/quasilyte/go-consistent v0.0.0-20190521200055-c6f3937de18c/go.mod h1:5STLWrekHfjyYwxBRVRXNOSewLJ3PWfDJd1VyTS21fI=
 github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
 github.com/rogpeppe/go-internal v1.1.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
+github.com/rogpeppe/go-internal v1.3.0 h1:RR9dF3JtopPvtkroDZuVD7qquD0bnHlKSqaQhgwt8yk=
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
@@ -528,9 +529,16 @@ github.com/valyala/quicktemplate v1.1.1/go.mod h1:EH+4AkTd43SvgIbQHYu59/cJyxDoOV
 github.com/valyala/tcplisten v0.0.0-20161114210144-ceec8f93295a/go.mod h1:v3UYOV9WzVtRmSR+PDvWpU/qWl4Wa5LApYYX4ZtKbio=
 github.com/willf/bitset v1.1.11-0.20200630133818-d5bec3311243 h1:R43TdZy32XXSXjJn7M/HhALJ9imq6ztLnChfYJpVDnM=
 github.com/willf/bitset v1.1.11-0.20200630133818-d5bec3311243/go.mod h1:RjeCKbqT1RxIR/KWY6phxZiaY1IyutSBfGjNPySAYV4=
+github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c=
 github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU=
+github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb h1:zGWFAtiMcyryUHoUjUJX0/lt1H2+i2Ka2n+D3DImSNo=
+github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU=
+github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0=
 github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ=
+github.com/xeipuuv/gojsonschema v0.0.0-20180618132009-1d523034197f h1:mvXjJIHRZyhNuGassLTcXTwjiWq7NmjdavZsUnmFybQ=
 github.com/xeipuuv/gojsonschema v0.0.0-20180618132009-1d523034197f/go.mod h1:5yf86TLmAcydyeJq5YvxkGPE2fm/u4myDekKRoLuqhs=
+github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74=
+github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/yvasiyarov/go-metrics v0.0.0-20140926110328-57bccd1ccd43/go.mod h1:aX5oPXxHm3bOH+xeAttToC8pqch2ScQN/JoXYupl6xs=
 github.com/yvasiyarov/gorelic v0.0.0-20141212073537-a9bba5b9ab50/go.mod h1:NUSPSUX/bi6SeDMUh6brw0nXpxHnc96TguQh0+r/ssA=
@@ -750,6 +758,8 @@ gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU=
 gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
+gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=
@@ -785,4 +795,6 @@ rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8
 sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI=
 sigs.k8s.io/yaml v1.1.0 h1:4A07+ZFc2wgJwo8YNlQpr1rVlgUDlxXHhPJciaPY5gs=
 sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
+sigs.k8s.io/yaml v1.2.0 h1:kr/MCeFWJWTwyaHoR9c8EjH9OumOmoF9YGiZd7lFm/Q=
+sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc=
 sourcegraph.com/sqs/pbtypes v0.0.0-20180604144634-d3ebe8f20ae4/go.mod h1:ketZ/q3QxT9HOBeFhu6RdvsftgpsbFHBF5Cas6cDKZ0=

go.sum

@@ -35,7 +35,7 @@ import (
 	"github.com/docker/go-connections/nat"
 	"github.com/imdario/mergo"
 	"github.com/rancher/k3d/v4/pkg/actions"
-	config "github.com/rancher/k3d/v4/pkg/config/v1alpha1"
+	config "github.com/rancher/k3d/v4/pkg/config/v1alpha2"
 	k3drt "github.com/rancher/k3d/v4/pkg/runtimes"
 	"github.com/rancher/k3d/v4/pkg/runtimes/docker"
 	runtimeErr "github.com/rancher/k3d/v4/pkg/runtimes/errors"

pkg/client/cluster.go

@@ -29,36 +29,33 @@ import (
 
 	"github.com/spf13/viper"
 
-	conf "github.com/rancher/k3d/v4/pkg/config/v1alpha1"
-	k3d "github.com/rancher/k3d/v4/pkg/types"
+	conf "github.com/rancher/k3d/v4/pkg/config/v1alpha2"
 )
 
-func ReadConfig(file string) (conf.Config, error) {
-	cfgViper := viper.New()
+func FromViperSimple(config *viper.Viper) (conf.SimpleConfig, error) {
 
-	cfgViper.SetConfigFile(file)
+	var cfg conf.SimpleConfig
 
-	cfgViper.SetConfigType("yaml")
-	cfgViper.SetEnvPrefix(k3d.DefaultObjectNamePrefix)
-	cfgViper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
-	cfgViper.AutomaticEnv()
+	// determine config kind
+	if config.GetString("kind") != "" && strings.ToLower(config.GetString("kind")) != "simple" {
+		return cfg, fmt.Errorf("Wrong `kind` '%s' != 'simple' in config file", config.GetString("kind"))
+	}
 
-	// try to read config into memory (viper map structure)
-	if err := cfgViper.ReadInConfig(); err != nil {
-		if _, ok := err.(viper.ConfigFileNotFoundError); ok {
-			log.Errorln("No config file found!")
+	if err := config.Unmarshal(&cfg); err != nil {
+		log.Errorln("Failed to unmarshal File config")
 
-			return nil, err
-		}
-		// config file found but some other error happened
-		log.Debugf("Failed to read config file: %+v", cfgViper.ConfigFileUsed())
-		return nil, err
+		return cfg, err
 	}
 
+	return cfg, nil
+}
+
+func FromViper(config *viper.Viper) (conf.Config, error) {
+
 	var cfg conf.Config
 
 	// determine config kind
-	switch strings.ToLower(cfgViper.GetString("kind")) {
+	switch strings.ToLower(config.GetString("kind")) {
 	case "simple":
 		cfg = conf.SimpleConfig{}
 	case "cluster":
@@ -68,16 +65,14 @@ func ReadConfig(file string) (conf.Config, error) {
 	case "":
 		return nil, fmt.Errorf("Missing `kind` in config file")
 	default:
-		return nil, fmt.Errorf("Unknown `kind` '%s' in config file", cfgViper.GetString("kind"))
+		return nil, fmt.Errorf("Unknown `kind` '%s' in config file", config.GetString("kind"))
 	}
 
-	if err := cfgViper.Unmarshal(&cfg); err != nil {
+	if err := config.Unmarshal(&cfg); err != nil {
 		log.Errorln("Failed to unmarshal File config")
 
 		return nil, err
 	}
 
-	log.Infof("Using Config: %s", cfgViper.ConfigFileUsed())
-
 	return cfg, nil
 }

pkg/config/config.go

@@ -26,7 +26,8 @@ import (
 	"time"
 
 	"github.com/go-test/deep"
-	conf "github.com/rancher/k3d/v4/pkg/config/v1alpha1"
+	conf "github.com/rancher/k3d/v4/pkg/config/v1alpha2"
+	"github.com/spf13/viper"
 
 	k3d "github.com/rancher/k3d/v4/pkg/types"
 )
@@ -39,7 +40,7 @@ func TestReadSimpleConfig(t *testing.T) {
 
 	expectedConfig := conf.SimpleConfig{
 		TypeMeta: conf.TypeMeta{
-			APIVersion: "k3d.io/v1alpha1",
+			APIVersion: "k3d.io/v1alpha2",
 			Kind:       "Simple",
 		},
 		Name:      "test",
@@ -94,20 +95,27 @@ func TestReadSimpleConfig(t *testing.T) {
 
 	cfgFile := "./test_assets/config_test_simple.yaml"
 
-	cfg, err := ReadConfig(cfgFile)
-	if err != nil {
+	config := viper.New()
+	config.SetConfigFile(cfgFile)
+
+	// try to read config into memory (viper map structure)
+	if err := config.ReadInConfig(); err != nil {
+		if _, ok := err.(viper.ConfigFileNotFoundError); ok {
+			t.Error(err)
+		}
+		// config file found but some other error happened
 		t.Error(err)
 	}
 
-	simpleCfg, ok := cfg.(conf.SimpleConfig)
-	if !ok {
-		t.Error("Config is not of type SimpleConfig")
+	cfg, err := FromViperSimple(config)
+	if err != nil {
+		t.Error(err)
 	}
 
-	t.Logf("\n========== Read Config ==========\n%+v\n=================================\n", simpleCfg)
+	t.Logf("\n========== Read Config %s ==========\n%+v\n=================================\n", config.ConfigFileUsed(), cfg)
 
-	if diff := deep.Equal(simpleCfg, expectedConfig); diff != nil {
-		t.Errorf("Actual representation\n%+v\ndoes not match expected representation\n%+v\nDiff:\n%+v", simpleCfg, expectedConfig, diff)
+	if diff := deep.Equal(cfg, expectedConfig); diff != nil {
+		t.Errorf("Actual representation\n%+v\ndoes not match expected representation\n%+v\nDiff:\n%+v", cfg, expectedConfig, diff)
 	}
 
 }
@@ -116,7 +124,7 @@ func TestReadClusterConfig(t *testing.T) {
 
 	expectedConfig := conf.ClusterConfig{
 		TypeMeta: conf.TypeMeta{
-			APIVersion: "k3d.io/v1alpha1",
+			APIVersion: "k3d.io/v1alpha2",
 			Kind:       "Cluster",
 		},
 		Cluster: k3d.Cluster{
@@ -132,9 +140,21 @@ func TestReadClusterConfig(t *testing.T) {
 
 	cfgFile := "./test_assets/config_test_cluster.yaml"
 
-	readConfig, err := ReadConfig(cfgFile)
+	config := viper.New()
+	config.SetConfigFile(cfgFile)
+
+	// try to read config into memory (viper map structure)
+	if err := config.ReadInConfig(); err != nil {
+		if _, ok := err.(viper.ConfigFileNotFoundError); ok {
+			t.Error(err)
+		}
+		// config file found but some other error happened
+		t.Error(err)
+	}
+
+	readConfig, err := FromViper(config)
 	if err != nil {
-		t.Fatal(err)
+		t.Error(err)
 	}
 
 	t.Logf("\n========== Read Config ==========\n%+v\n=================================\n", readConfig)
@@ -149,7 +169,7 @@ func TestReadClusterListConfig(t *testing.T) {
 
 	expectedConfig := conf.ClusterListConfig{
 		TypeMeta: conf.TypeMeta{
-			APIVersion: "k3d.io/v1alpha1",
+			APIVersion: "k3d.io/v1alpha2",
 			Kind:       "ClusterList",
 		},
 		Clusters: []k3d.Cluster{
@@ -176,9 +196,21 @@ func TestReadClusterListConfig(t *testing.T) {
 
 	cfgFile := "./test_assets/config_test_cluster_list.yaml"
 
-	readConfig, err := ReadConfig(cfgFile)
+	config := viper.New()
+	config.SetConfigFile(cfgFile)
+
+	// try to read config into memory (viper map structure)
+	if err := config.ReadInConfig(); err != nil {
+		if _, ok := err.(viper.ConfigFileNotFoundError); ok {
+			t.Error(err)
+		}
+		// config file found but some other error happened
+		t.Error(err)
+	}
+
+	readConfig, err := FromViper(config)
 	if err != nil {
-		t.Fatal(err)
+		t.Error(err)
 	}
 
 	t.Logf("\n========== Read Config ==========\n%+v\n=================================\n", readConfig)
@@ -193,7 +225,19 @@ func TestReadUnknownConfig(t *testing.T) {
 
 	cfgFile := "./test_assets/config_test_unknown.yaml"
 
-	_, err := ReadConfig(cfgFile)
+	config := viper.New()
+	config.SetConfigFile(cfgFile)
+
+	// try to read config into memory (viper map structure)
+	if err := config.ReadInConfig(); err != nil {
+		if _, ok := err.(viper.ConfigFileNotFoundError); ok {
+			t.Error(err)
+		}
+		// config file found but some other error happened
+		t.Error(err)
+	}
+
+	_, err := FromViperSimple(config)
 	if err == nil {
 		t.Fail()
 	}