Merge pull request #58 from 1Password/dg/normalize-secret-name

Add secret name normalizer to the operator.

README.md

@@ -144,7 +144,12 @@ If a 1Password Item that is linked to a Kubernetes Secret is updated within the
 ---
 **NOTE**
 
-If multiple 1Password vaults/items have the same `title` when using a title in the access path, the desired action will be performed on the oldest vault/item. Furthermore, titles that include white space characters cannot be used.
+If multiple 1Password vaults/items have the same `title` when using a title in the access path, the desired action will be performed on the oldest vault/item. 
+
+Titles and field names that include white space and other characters that are not a valid [DNS subdomain name](https://kubernetes.io/docs/concepts/configuration/secret/) will create Kubernetes secrets that have titles and fields in the following format:
+ - Invalid characters before the first alphanumeric character and after the last alphanumeric character will be removed
+ - All whitespaces between words will be replaced by `-`
+ - All the letters will be lower-cased.
 
 ---
 

pkg/controller/onepassworditem/onepassworditem_controller.go

@@ -3,7 +3,6 @@ package onepassworditem
 import (
 	"context"
 	"fmt"
-
 	onepasswordv1 "github.com/1Password/onepassword-operator/pkg/apis/onepassword/v1"
 	kubeSecrets "github.com/1Password/onepassword-operator/pkg/kubernetessecrets"
 	"github.com/1Password/onepassword-operator/pkg/onepassword"

pkg/controller/onepassworditem/onepassworditem_test.go

@@ -31,6 +31,9 @@ const (
 	itemId                    = "nwrhuano7bcwddcviubpp4mhfq"
 	username                  = "test-user"
 	password                  = "QmHumKc$mUeEem7caHtbaBaJ"
+	firstHost                 = "http://localhost:8080"
+	awsKey                    = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
+	iceCream                  = "freezing blue 20%"
 	userKey                   = "username"
 	passKey                   = "password"
 	version                   = 123
@@ -210,6 +213,79 @@ var tests = []testReconcileItem{
 			passKey: password,
 		},
 	},
+	{
+		testName: "Secret from 1Password item with invalid K8s labels",
+		customResource: &onepasswordv1.OnePasswordItem{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       onePasswordItemKind,
+				APIVersion: onePasswordItemAPIVersion,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "!my sECReT it3m%",
+				Namespace: namespace,
+			},
+			Spec: onepasswordv1.OnePasswordItemSpec{
+				ItemPath: itemPath,
+			},
+		},
+		existingSecret: nil,
+		expectedError:  nil,
+		expectedResultSecret: &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "my-secret-it3m",
+				Namespace: namespace,
+				Annotations: map[string]string{
+					op.VersionAnnotation: fmt.Sprint(version),
+				},
+			},
+			Data: expectedSecretData,
+		},
+		opItem: map[string]string{
+			userKey: username,
+			passKey: password,
+		},
+	},
+	{
+		testName: "Secret from 1Password item with fields and sections that have invalid K8s labels",
+		customResource: &onepasswordv1.OnePasswordItem{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       onePasswordItemKind,
+				APIVersion: onePasswordItemAPIVersion,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "!my sECReT it3m%",
+				Namespace: namespace,
+			},
+			Spec: onepasswordv1.OnePasswordItemSpec{
+				ItemPath: itemPath,
+			},
+		},
+		existingSecret: nil,
+		expectedError:  nil,
+		expectedResultSecret: &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "my-secret-it3m",
+				Namespace: namespace,
+				Annotations: map[string]string{
+					op.VersionAnnotation: fmt.Sprint(version),
+				},
+			},
+			Data: map[string][]byte{
+				"password":       []byte(password),
+				"username":       []byte(username),
+				"first-host":     []byte(firstHost),
+				"aws-access-key": []byte(awsKey),
+				"ice-cream-type": []byte(iceCream),
+			},
+		},
+		opItem: map[string]string{
+			userKey:            username,
+			passKey:            password,
+			"first host":       firstHost,
+			"AWS Access Key":   awsKey,
+			"😄 ice-cream type": iceCream,
+		},
+	},
 }
 
 func TestReconcileOnePasswordItem(t *testing.T) {
@@ -241,7 +317,10 @@ func TestReconcileOnePasswordItem(t *testing.T) {
 			mocks.GetGetItemFunc = func(uuid string, vaultUUID string) (*onepassword.Item, error) {
 
 				item := onepassword.Item{}
-				item.Fields = generateFields(testData.opItem["username"], testData.opItem["password"])
+				item.Fields = []*onepassword.ItemField{}
+				for k, v := range testData.opItem {
+					item.Fields = append(item.Fields, &onepassword.ItemField{Label: k, Value: v})
+				}
 				item.Version = version
 				item.Vault.ID = vaultUUID
 				item.ID = uuid
@@ -257,8 +336,8 @@ func TestReconcileOnePasswordItem(t *testing.T) {
 			// watched resource .
 			req := reconcile.Request{
 				NamespacedName: types.NamespacedName{
-					Name:      name,
-					Namespace: namespace,
+					Name:      testData.customResource.ObjectMeta.Name,
+					Namespace: testData.customResource.ObjectMeta.Namespace,
 				},
 			}
 			_, err := r.Reconcile(req)

pkg/kubernetessecrets/kubernetes_secrets_builder.go

@@ -4,12 +4,17 @@ import (
 	"context"
 	"fmt"
 
+	"regexp"
+	"strings"
+
 	"github.com/1Password/connect-sdk-go/onepassword"
 	"github.com/1Password/onepassword-operator/pkg/utils"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	kubeValidate "k8s.io/apimachinery/pkg/util/validation"
+
 	kubernetesClient "sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
@@ -63,7 +68,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa
 func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotations map[string]string, item onepassword.Item) *corev1.Secret {
 	return &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        name,
+			Name:        formatSecretName(name),
 			Namespace:   namespace,
 			Annotations: annotations,
 		},
@@ -75,8 +80,33 @@ func BuildKubernetesSecretData(fields []*onepassword.ItemField) map[string][]byt
 	secretData := map[string][]byte{}
 	for i := 0; i < len(fields); i++ {
 		if fields[i].Value != "" {
-			secretData[fields[i].Label] = []byte(fields[i].Value)
+			key := formatSecretName(fields[i].Label)
+			secretData[key] = []byte(fields[i].Value)
 		}
 	}
 	return secretData
 }
+
+// formatSecretName rewrites a value to be a valid Secret name or Secret data key.
+//
+// The Secret meta.name and data keys must be valid DNS subdomain names (https://kubernetes.io/docs/concepts/configuration/secret/#overview-of-secrets)
+func formatSecretName(value string) string {
+	if errs := kubeValidate.IsDNS1123Subdomain(value); len(errs) == 0 {
+		return value
+	}
+	return createValidSecretName(value)
+}
+
+var invalidDNS1123Chars = regexp.MustCompile("[^a-z0-9-]+")
+
+func createValidSecretName(value string) string {
+	result := strings.ToLower(value)
+	result = invalidDNS1123Chars.ReplaceAllString(result, "-")
+
+	if len(result) > kubeValidate.DNS1123SubdomainMaxLength {
+		result = result[0:kubeValidate.DNS1123SubdomainMaxLength]
+	}
+
+	// first and last character MUST be alphanumeric
+	return strings.Trim(result, "-")
+}

pkg/kubernetessecrets/kubernetes_secrets_builder_test.go

@@ -3,6 +3,7 @@ package kubernetessecrets
 import (
 	"context"
 	"fmt"
+	kubeValidate "k8s.io/apimachinery/pkg/util/validation"
 	"strings"
 	"testing"
 
@@ -101,7 +102,7 @@ func TestBuildKubernetesSecretFromOnePasswordItem(t *testing.T) {
 	item.Fields = generateFields(5)
 
 	kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, item)
-	if kubeSecret.Name != name {
+	if kubeSecret.Name != strings.ToLower(name) {
 		t.Errorf("Expected name value: %v but got: %v", name, kubeSecret.Name)
 	}
 	if kubeSecret.Namespace != namespace {
@@ -113,6 +114,44 @@ func TestBuildKubernetesSecretFromOnePasswordItem(t *testing.T) {
 	compareFields(item.Fields, kubeSecret.Data, t)
 }
 
+func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) {
+	name := "inV@l1d k8s secret%name"
+	expectedName := "inv-l1d-k8s-secret-name"
+	namespace := "someNamespace"
+	annotations := map[string]string{
+		"annotationKey": "annotationValue",
+	}
+	item := onepassword.Item{}
+
+	item.Fields = []*onepassword.ItemField{
+		{
+			Label: "label w%th invalid ch!rs-",
+			Value: "value1",
+		},
+		{
+			Label: strings.Repeat("x", kubeValidate.DNS1123SubdomainMaxLength+1),
+			Value: "name exceeds max length",
+		},
+	}
+
+	kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, item)
+
+	// Assert Secret's meta.name was fixed
+	if kubeSecret.Name != expectedName {
+		t.Errorf("Expected name value: %v but got: %v", name, kubeSecret.Name)
+	}
+	if kubeSecret.Namespace != namespace {
+		t.Errorf("Expected namespace value: %v but got: %v", namespace, kubeSecret.Namespace)
+	}
+
+	// assert labels were fixed for each data key
+	for key := range kubeSecret.Data {
+		if !validLabel(key) {
+			t.Errorf("Expected valid kubernetes label, got %s", key)
+		}
+	}
+}
+
 func compareAnnotationsToItem(annotations map[string]string, item onepassword.Item, t *testing.T) {
 	actualVaultId, actualItemId, err := ParseVaultIdAndItemIdFromPath(annotations[ItemPathAnnotation])
 	if err != nil {
@@ -164,3 +203,10 @@ func ParseVaultIdAndItemIdFromPath(path string) (string, string, error) {
 	}
 	return "", "", fmt.Errorf("%q is not an acceptable path for One Password item. Must be of the format: `vaults/{vault_id}/items/{item_id}`", path)
 }
+
+func validLabel(v string) bool {
+	if err := kubeValidate.IsDNS1123Subdomain(v); len(err) > 0 {
+		return false
+	}
+	return true
+}