From 579b5848da5294c191969e69038b2ed3cb1b511a Mon Sep 17 00:00:00 2001 From: "david.gunter" Date: Thu, 5 Aug 2021 16:34:50 -0700 Subject: [PATCH 1/4] Add secret name normalizer to the operator. The operator will now reformat 1Password item names to become valid names K8s Secret objects. Secret names must be a valid DNS subdomain name. See more: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names --- .../onepassworditem_controller.go | 29 ++++++++++++++- .../onepassworditem/onepassworditem_test.go | 36 +++++++++++++++++-- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/pkg/controller/onepassworditem/onepassworditem_controller.go b/pkg/controller/onepassworditem/onepassworditem_controller.go index 3c808bd3..11bec45c 100644 --- a/pkg/controller/onepassworditem/onepassworditem_controller.go +++ b/pkg/controller/onepassworditem/onepassworditem_controller.go @@ -3,6 +3,8 @@ 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" @@ -15,6 +17,7 @@ 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" @@ -143,7 +146,7 @@ func (r *ReconcileOnePasswordItem) removeOnePasswordFinalizerFromOnePasswordItem } func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1.OnePasswordItem, request reconcile.Request) error { - secretName := resource.GetName() + secretName := formatK8sSecretName(resource.GetName()) autoRestart := resource.Annotations[op.RestartDeploymentsAnnotation] item, err := onepassword.GetOnePasswordItemByPath(r.opConnectClient, resource.Spec.ItemPath) @@ -153,3 +156,27 @@ 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, "-") +} diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index 2639f18e..40108d81 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/pkg/controller/onepassworditem/onepassworditem_test.go @@ -210,6 +210,38 @@ var tests = []testReconcileItem{ passKey: password, }, }, + { + testName: "Secret from 1Password item with invalid K8s secret name", + 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, + }, + }, } func TestReconcileOnePasswordItem(t *testing.T) { @@ -257,8 +289,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) From 96b42e7c52a6434d602e9910405e1bcaf0964273 Mon Sep 17 00:00:00 2001 From: "david.gunter" Date: Fri, 6 Aug 2021 13:18:21 -0700 Subject: [PATCH 2/4] 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. --- .../onepassworditem_controller.go | 30 +----------- .../onepassworditem/onepassworditem_test.go | 2 +- .../kubernetes_secrets_builder.go | 34 +++++++++++++- .../kubernetes_secrets_builder_test.go | 46 +++++++++++++++++++ 4 files changed, 80 insertions(+), 32 deletions(-) diff --git a/pkg/controller/onepassworditem/onepassworditem_controller.go b/pkg/controller/onepassworditem/onepassworditem_controller.go index 11bec45c..c5cf8996 100644 --- a/pkg/controller/onepassworditem/onepassworditem_controller.go +++ b/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, "-") -} diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index 40108d81..2ebb5e02 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/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, diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 0c658f7e..fba54fba 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/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, "-") +} diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go index cb331c65..43879ce3 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go +++ b/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 +} From 8cfe98073e22cb3698b2725bdfd96d45bdef8bb4 Mon Sep 17 00:00:00 2001 From: Eddy Filip Date: Mon, 16 Aug 2021 14:51:44 +0200 Subject: [PATCH 3/4] Improve testing Fix previous tests and add test for items with field names that are not valid DNS subdomain names. --- .../onepassworditem/onepassworditem_test.go | 49 ++++++++++++++++++- .../kubernetes_secrets_builder_test.go | 2 +- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index 2ebb5e02..09d5b3b9 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/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 @@ -242,6 +245,47 @@ var tests = []testReconcileItem{ 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) { @@ -273,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 diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go index 43879ce3..3c9484d0 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go @@ -102,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 { From 753cc5e9a396f7bb9af45fd5250685d5db13929e Mon Sep 17 00:00:00 2001 From: Eddy Filip Date: Mon, 16 Aug 2021 15:24:04 +0200 Subject: [PATCH 4/4] Update note in README The note now explains how the item title and fields are made into DNS subdomain-compliant names for creating Kubernetes secrets. --- README.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a2ec464c..ca2e7d3d 100644 --- a/README.md +++ b/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. ---