Label normalizer now fixes both Secret names and data keys.

Each key in the `data` section of a secret must also be a valid DNS subdomain. The operator needs to "fix" the 1Password item fields before trying to create the secret.

pkg/controller/onepassworditem/onepassworditem_controller.go

@@ -3,9 +3,6 @@ package onepassworditem
 import (
 	"context"
 	"fmt"
-	"regexp"
-	"strings"
-
 	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"
@@ -17,7 +14,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
-	kubeValidate "k8s.io/apimachinery/pkg/util/validation"
 	ctrl "sigs.k8s.io/controller-runtime"
 	kubeClient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -146,7 +142,7 @@ func (r *ReconcileOnePasswordItem) removeOnePasswordFinalizerFromOnePasswordItem
 }
 
 func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1.OnePasswordItem, request reconcile.Request) error {
-	secretName := formatK8sSecretName(resource.GetName())
+	secretName := resource.GetName()
 	autoRestart := resource.Annotations[op.RestartDeploymentsAnnotation]
 
 	item, err := onepassword.GetOnePasswordItemByPath(r.opConnectClient, resource.Spec.ItemPath)
@@ -156,27 +152,3 @@ func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1
 
 	return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, resource.Namespace, item, autoRestart)
 }
-
-// formatK8sSecretName replaces any characters not supported by K8s secret names
-//
-// K8s Secrets must be valid DNS subdomain names (https://kubernetes.io/docs/concepts/configuration/secret/#overview-of-secrets)
-func formatK8sSecretName(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]
-	}
-
-	// only alphanumeric characters allowed at beginning and end of value
-	return strings.Trim(result, "-")
-}

pkg/controller/onepassworditem/onepassworditem_test.go

@@ -211,7 +211,7 @@ var tests = []testReconcileItem{
 		},
 	},
 	{
-		testName: "Secret from 1Password item with invalid K8s secret name",
+		testName: "Secret from 1Password item with invalid K8s labels",
 		customResource: &onepasswordv1.OnePasswordItem{
 			TypeMeta: metav1.TypeMeta{
 				Kind:       onePasswordItemKind,

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"
 
@@ -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
+}