Do not panic when setting driver config

Use the go, Luke. Return errors instead of throwing a panic.

Signed-off-by: Carolyn Van Slyck <[email protected]>

driver/debug/debug.go

@@ -42,6 +42,7 @@ func (d *Driver) Config() map[string]string {
 }
 
 // SetConfig sets configuration for this driver
-func (d *Driver) SetConfig(settings map[string]string) {
+func (d *Driver) SetConfig(settings map[string]string) error {
 	d.config = settings
+	return nil
 }

driver/docker/docker.go

@@ -19,7 +19,6 @@ import (
 	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/docker/docker/registry"
 	"github.com/mitchellh/copystructure"
-
 	"github.com/pkg/errors"
 
 	"github.com/cnabio/cnab-go/bundle"
@@ -98,16 +97,17 @@ func (d *Driver) Config() map[string]string {
 }
 
 // SetConfig sets Docker driver configuration
-func (d *Driver) SetConfig(settings map[string]string) {
+func (d *Driver) SetConfig(settings map[string]string) error {
 	// Set default and provide feedback on acceptable input values.
 	value, ok := settings["CLEANUP_CONTAINERS"]
 	if !ok {
 		settings["CLEANUP_CONTAINERS"] = "true"
 	} else if value != "true" && value != "false" {
-		fmt.Printf("CLEANUP_CONTAINERS environment variable has unexpected value %q. Supported values are 'true', 'false', or unset.", value)
+		return fmt.Errorf("environment variable CLEANUP_CONTAINERS has unexpected value %q. Supported values are 'true', 'false', or unset", value)
 	}
 
 	d.config = settings
+	return nil
 }
 
 // SetDockerCli makes the driver use an already initialized cli

driver/docker/docker_test.go

@@ -87,6 +87,56 @@ func TestDriver_GetConfigurationOptions(t *testing.T) {
 	})
 }
 
+func TestDriver_SetConfig(t *testing.T) {
+	testcases := []struct {
+		name      string
+		settings  map[string]string
+		wantError string
+	}{
+		{
+			name: "valid settings",
+			settings: map[string]string{
+				"VERBOSE": "true",
+			},
+			wantError: "",
+		},
+		{
+			name: "cleanup containers: true",
+			settings: map[string]string{
+				"CLEANUP_CONTAINERS": "true",
+			},
+			wantError: "",
+		},
+		{
+			name: "cleanup containers: false",
+			settings: map[string]string{
+				"CLEANUP_CONTAINERS": "false",
+			},
+			wantError: "",
+		},
+		{
+			name: "cleanup containers - invalid",
+			settings: map[string]string{
+				"CLEANUP_CONTAINERS": "1",
+			},
+			wantError: "environment variable CLEANUP_CONTAINERS has unexpected value",
+		},
+	}
+
+	for _, tc := range testcases {
+		d := Driver{}
+		err := d.SetConfig(tc.settings)
+
+		if tc.wantError == "" {
+			require.NoError(t, err, "expected SetConfig to succeed")
+			assert.Equal(t, tc.settings, d.config, "expected all of the specified settings to be copied")
+		} else {
+			require.Error(t, err, "expected SetConfig to fail")
+			assert.Contains(t, err.Error(), tc.wantError)
+		}
+	}
+}
+
 func TestDriver_ValidateImageDigest(t *testing.T) {
 	// Mimic the digests created when a bundle is pushed with cnab-to-oci
 	// there is one for the original invocation image and another

driver/driver.go

@@ -103,5 +103,5 @@ type Configurable interface {
 	Config() map[string]string
 	// SetConfig allows setting configuration, where name corresponds to the key in Config, and value is
 	// the value to be set.
-	SetConfig(map[string]string)
+	SetConfig(map[string]string) error
 }

driver/kubernetes/kubernetes.go

@@ -87,7 +87,7 @@ func (k *Driver) Config() map[string]string {
 }
 
 // SetConfig sets Kubernetes driver configuration.
-func (k *Driver) SetConfig(settings map[string]string) {
+func (k *Driver) SetConfig(settings map[string]string) error {
 	k.setDefaults()
 	k.Namespace = settings["KUBE_NAMESPACE"]
 	k.ServiceAccountName = settings["SERVICE_ACCOUNT"]
@@ -101,12 +101,9 @@ func (k *Driver) SetConfig(settings map[string]string) {
 
 	conf, err := clientcmd.BuildConfigFromFlags(settings["MASTER_URL"], kubeconfig)
 	if err != nil {
-		panic(err)
-	}
-	err = k.setClient(conf)
-	if err != nil {
-		panic(err)
+		return errors.Wrapf(err, "error retrieving external kubernetes configuration using configuration:\n%v", settings)
 	}
+	return k.setClient(conf)
 }
 
 func (k *Driver) setDefaults() {
@@ -120,11 +117,11 @@ func (k *Driver) setDefaults() {
 func (k *Driver) setClient(conf *rest.Config) error {
 	coreClient, err := coreclientv1.NewForConfig(conf)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "error creating CoreClient for Kubernetes Driver")
 	}
 	batchClient, err := batchclientv1.NewForConfig(conf)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "error creating BatchClient for Kubernetes Driver")
 	}
 	k.jobs = batchClient.Jobs(k.Namespace)
 	k.secrets = coreClient.Secrets(k.Namespace)

driver/kubernetes/kubernetes_integration_test.go

@@ -7,9 +7,11 @@ import (
 	"os"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/cnabio/cnab-go/bundle"
 	"github.com/cnabio/cnab-go/driver"
-	"github.com/stretchr/testify/assert"
 )
 
 func TestDriver_Run_Integration(t *testing.T) {
@@ -85,3 +87,14 @@ func TestDriver_Run_Integration(t *testing.T) {
 		})
 	}
 }
+
+func TestDriver_SetConfig(t *testing.T) {
+	t.Run("kubeconfig", func(t *testing.T) {
+
+		d := Driver{}
+		err := d.SetConfig(map[string]string{
+			"KUBECONFIG": os.Getenv("KUBECONFIG"),
+		})
+		require.NoError(t, err)
+	})
+}

driver/kubernetes/kubernetes_test.go

@@ -162,3 +162,16 @@ func TestGenerateNameTemplate(t *testing.T) {
 		})
 	}
 }
+
+func TestDriver_SetConfig_Fails(t *testing.T) {
+	t.Run("kubeconfig invalid", func(t *testing.T) {
+
+		d := Driver{}
+		err := d.SetConfig(map[string]string{
+			"KUBECONFIG": "invalid",
+		})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "error retrieving external kubernetes configuration using configuration")
+	})
+
+}