Add defaults and validation to configmap

Author: Priya Wadhwa

README.md

@@ -13,18 +13,18 @@ See [DEVELOPMENT.md](DEVELOPMENT.md) for a guide on how to build and deploy your
 ### Configuration
 
 Chains uses a `ConfigMap` called `chains-config` in the `tekton-chains` namespace for configuration.
-Supported keys include:
-
-| Key | Description | Supported Values |
-| --- | --- | --- |
-| `artifacts.taskrun.format` | The format to store `TaskRun` payloads in. | `tekton` |
-| `artifacts.taskrun.storage` | The storage backend to store `TaskRun` signatures in. | `tekton`, `oci`, `gcs`, `docdb` |
-| `artifacts.taskrun.signer` | The signature backend to sign `Taskrun` payloads with. | `pgp`, `x509`, `kms` |
-| `artifacts.oci.format` | The format to store `OCI` payloads in. | `tekton`, `simplesigning` |
-| `artifacts.oci.storage` | The storage backend to store `OCI` signatures in. | `tekton`, `oci`, `gcs`, `docdb` |
-| `artifacts.oci.signer` | The signature backend to sign `OCI` payloads with. | `pgp`, `x509`, `kms` |
-| `signers.kms.kmsref` | The URI reference to a KMS service to use in `KMS` signers. | `gcpkms://projects/<project>/locations/<location>/keyRings/<keyring>/cryptoKeys/<key>`|
-| `storage.docdb.url` | The go-cloud URI reference to a docstore collection | `firestore://projects/<project>/databases/(default)/documents/<collection>?name_field=name`|
+Supported keys include (scroll right for defaults):
+
+| Key | Description | Supported Values | Default |
+| --- | --- | --- | --- |
+| `artifacts.taskrun.format` | The format to store `TaskRun` payloads in. | `tekton` | `tekton` |
+| `artifacts.taskrun.storage` | The storage backend to store `TaskRun` signatures in. | `tekton`, `oci`, `gcs`, `docdb` | `tekton` |
+| `artifacts.taskrun.signer` | The signature backend to sign `Taskrun` payloads with. | `pgp`, `x509`, `kms` | `x509` |
+| `artifacts.oci.format` | The format to store `OCI` payloads in. | `tekton`, `simplesigning` | `simplesigning` |
+| `artifacts.oci.storage` | The storage backend to store `OCI` signatures in. | `tekton`, `oci`, `gcs`, `docdb` | `oci` |
+| `artifacts.oci.signer` | The signature backend to sign `OCI` payloads with. | `pgp`, `x509`, `kms` | `x509` |
+| `signers.kms.kmsref` | The URI reference to a KMS service to use in `KMS` signers. | `gcpkms://projects/<project>/locations/<location>/keyRings/<keyring>/cryptoKeys/<key>`| |
+| `storage.docdb.url` | The go-cloud URI reference to a docstore collection | `firestore://projects/<project>/databases/(default)/documents/<collection>?name_field=name`| |
 
 ### Overview
 

pkg/config/store.go

@@ -95,33 +95,74 @@ const (
 	chainsConfig = "chains-config"
 )
 
+var defaults = map[string]string{
+	taskrunFormatKey:  "tekton",
+	taskrunStorageKey: "tekton",
+	taskrunSignerKey:  "x509",
+	ociFormatKey:      "simplesigning",
+	ociStorageKey:     "oci",
+	ociSignerKey:      "x509",
+}
+
+var supportedValues = map[string][]string{
+	taskrunFormatKey:  {"tekton"},
+	taskrunStorageKey: {"tekton", "oci", "gcs", "docdb"},
+	taskrunSignerKey:  {"pgp", "x509", "kms"},
+	ociFormatKey:      {"tekton", "simplesigning"},
+	ociStorageKey:     {"tekton", "oci", "gcs", "docdb"},
+	ociSignerKey:      {"pgp", "x509", "kms"},
+}
+
 func parse(data map[string]string) Config {
 	cfg := Config{}
 
 	// Artifact-specific configs
 
 	// TaskRuns
-	cfg.Artifacts.TaskRuns.Format = data[taskrunFormatKey]
-	cfg.Artifacts.TaskRuns.StorageBackend = data[taskrunStorageKey]
-	cfg.Artifacts.TaskRuns.Signer = data[taskrunSignerKey]
+	cfg.Artifacts.TaskRuns.Format = valueOrDefault(taskrunFormatKey, data)
+	cfg.Artifacts.TaskRuns.StorageBackend = valueOrDefault(taskrunStorageKey, data)
+	cfg.Artifacts.TaskRuns.Signer = valueOrDefault(taskrunSignerKey, data)
 
 	// OCI
-	cfg.Artifacts.OCI.Format = data[ociFormatKey]
-	cfg.Artifacts.OCI.StorageBackend = data[ociStorageKey]
-	cfg.Artifacts.OCI.Signer = data[ociSignerKey]
+	cfg.Artifacts.OCI.Format = valueOrDefault(ociFormatKey, data)
+	cfg.Artifacts.OCI.StorageBackend = valueOrDefault(ociStorageKey, data)
+	cfg.Artifacts.OCI.Signer = valueOrDefault(ociSignerKey, data)
 
 	// Storage level configs
 
-	cfg.Storage.GCS.Bucket = data[gcsBucketKey]
-	cfg.Storage.OCI.Repository = data[ociRepositoryKey]
-	cfg.Storage.OCI.Insecure = (data[ociRepositoryInsecureKey] == "true")
-	cfg.Storage.DocDB.URL = data[docDBUrlKey]
+	cfg.Storage.GCS.Bucket = valueOrDefault(gcsBucketKey, data)
+	cfg.Storage.OCI.Repository = valueOrDefault(ociRepositoryKey, data)
+	cfg.Storage.OCI.Insecure = (valueOrDefault(ociRepositoryInsecureKey, data) == "true")
+	cfg.Storage.DocDB.URL = valueOrDefault(docDBUrlKey, data)
 
-	cfg.Signers.KMS.KMSRef = data[kmsSignerKMSRef]
+	cfg.Signers.KMS.KMSRef = valueOrDefault(kmsSignerKMSRef, data)
 
 	return cfg
 }
 
+func valueOrDefault(key string, data map[string]string) string {
+	if v, ok := data[key]; ok {
+		if validate(key, v) {
+			return v
+		}
+	}
+	return defaults[key]
+}
+
+func validate(key, value string) bool {
+	vals, ok := supportedValues[key]
+	// if it doesn't exist in supportedValues, we don't validate
+	if !ok {
+		return true
+	}
+	for _, v := range vals {
+		if v == value {
+			return true
+		}
+	}
+	return false
+}
+
 type ConfigStore struct {
 	name   string
 	config atomic.Value

pkg/config/store_test.go

@@ -111,3 +111,42 @@ func Test_parse(t *testing.T) {
 		})
 	}
 }
+
+func TestValueOrDefault(t *testing.T) {
+	tests := []struct {
+		description string
+		key         string
+		value       string
+		expected    string
+	}{
+		{
+			description: "valid key set to default",
+			key:         ociFormatKey,
+			value:       "simplesigning",
+			expected:    "simplesigning",
+		}, {
+			description: "valid key not set to default",
+			key:         ociFormatKey,
+			value:       "tekton",
+			expected:    "tekton",
+		}, {
+			description: "invalid value with default",
+			key:         ociFormatKey,
+			value:       "invalid",
+			expected:    "simplesigning",
+		}, {
+			description: "key with no default",
+			key:         gcsBucketKey,
+			value:       "bucket",
+			expected:    "bucket",
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			got := valueOrDefault(test.key, map[string]string{test.key: test.value})
+			if got != test.expected {
+				t.Fatalf("got (%s) is not expected (%s)", got, test.expected)
+			}
+		})
+	}
+}

test/clients.go

@@ -121,7 +121,8 @@ func createNamespace(ctx context.Context, t *testing.T, namespace string, kubeCl
 	t.Logf("Create namespace %s to deploy to", namespace)
 	if _, err := kubeClient.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: namespace,
+			Name:   namespace,
+			Labels: map[string]string{"chains": "integration-testing"},
 		},
 	}, metav1.CreateOptions{}); err != nil {
 		t.Fatalf("Failed to create namespace %s for tests: %s", namespace, err)

test/e2e_test.go

@@ -60,12 +60,10 @@ func TestTektonStorage(t *testing.T) {
 
 	// Setup the right config.
 	resetConfig := setConfigMap(ctx, t, c, map[string]string{
-		"artifacts.taskrun.storage": "tekton",
-		"artifacts.taskrun.signer":  "pgp",
-		"artifacts.taskrun.format":  "tekton",
-		"artifacts.oci.format":      "tekton",
-		"artifacts.oci.storage":     "tekton",
-		"artifacts.oci.signer":      "pgp",
+		"artifacts.taskrun.signer": "pgp",
+		"artifacts.oci.format":     "tekton",
+		"artifacts.oci.storage":    "tekton",
+		"artifacts.oci.signer":     "pgp",
 	})
 	defer resetConfig()
 
@@ -119,10 +117,7 @@ func TestOCISigning(t *testing.T) {
 			defer cleanup()
 
 			// Setup the right config.
-			resetConfig := setConfigMap(ctx, t, c, map[string]string{
-				"artifacts.oci.format":  "simplesigning",
-				"artifacts.oci.storage": "tekton",
-				"artifacts.oci.signer":  "x509"})
+			resetConfig := setConfigMap(ctx, t, c, map[string]string{"artifacts.oci.storage": "tekton"})
 
 			defer resetConfig()
 
@@ -213,9 +208,6 @@ func TestOCIStorage(t *testing.T) {
 	defer cleanup()
 
 	resetConfig := setConfigMap(ctx, t, c, map[string]string{
-		"artifacts.taskrun.storage":       "oci",
-		"artifacts.taskrun.signer":        "x509",
-		"artifacts.taskrun.format":        "simplesigning",
 		"storage.oci.repository.insecure": "true",
 	})
 	defer resetConfig()