From bd96d50a9ba90345c49c0b450dea0c704ec8c13a Mon Sep 17 00:00:00 2001 From: mcmarkj Date: Fri, 28 May 2021 16:39:00 +0100 Subject: [PATCH 01/21] Add Labels & Annotations from OPObject to Secret --- .../deployment/deployment_controller.go | 4 ++- .../onepassworditem_controller.go | 6 +++-- .../kubernetes_secrets_builder.go | 25 ++++++++++++++----- .../kubernetes_secrets_builder_test.go | 19 +++++++++++--- pkg/onepassword/secret_update_handler.go | 2 +- 5 files changed, 42 insertions(+), 14 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 93ff956..e4fd363 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -191,6 +191,8 @@ func (r *ReconcileDeployment) HandleApplyingDeployment(namespace string, annotat reqLog := log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) secretName := annotations[op.NameAnnotation] + secretLabels := map[string]string{} + secretAnnotations := map[string]string{} if len(secretName) == 0 { reqLog.Info("No 'item-name' annotation set. 'item-path' and 'item-name' must be set as annotations to add new secret.") return nil @@ -201,5 +203,5 @@ func (r *ReconcileDeployment) HandleApplyingDeployment(namespace string, annotat return fmt.Errorf("Failed to retrieve item: %v", err) } - return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, namespace, item, annotations[op.RestartDeploymentsAnnotation]) + return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, namespace, item, annotations[op.RestartDeploymentsAnnotation], secretLabels, secretAnnotations) } diff --git a/pkg/controller/onepassworditem/onepassworditem_controller.go b/pkg/controller/onepassworditem/onepassworditem_controller.go index 3c808bd..b57721b 100644 --- a/pkg/controller/onepassworditem/onepassworditem_controller.go +++ b/pkg/controller/onepassworditem/onepassworditem_controller.go @@ -144,12 +144,14 @@ func (r *ReconcileOnePasswordItem) removeOnePasswordFinalizerFromOnePasswordItem func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1.OnePasswordItem, request reconcile.Request) error { secretName := resource.GetName() - autoRestart := resource.Annotations[op.RestartDeploymentsAnnotation] + labels := resource.Labels + annotations := resource.Annotations + autoRestart := annotations[op.RestartDeploymentsAnnotation] item, err := onepassword.GetOnePasswordItemByPath(r.opConnectClient, resource.Spec.ItemPath) if err != nil { return fmt.Errorf("Failed to retrieve item: %v", err) } - return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, resource.Namespace, item, autoRestart) + return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, resource.Namespace, item, autoRestart, labels, annotations) } diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 0c658f7..76671ef 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -23,22 +23,34 @@ const RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto-restart" var log = logf.Log -func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart string) error { + +func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart string, labels map[string]string, annotations map[string]string) error { itemVersion := fmt.Sprint(item.Version) - annotations := map[string]string{ + + // Remove OP Annotations if they already exist + delete(annotations,VersionAnnotation) + delete(annotations,ItemPathAnnotation) + + secretAnnotations := map[string]string{ VersionAnnotation: itemVersion, ItemPathAnnotation: fmt.Sprintf("vaults/%v/items/%v", item.Vault.ID, item.ID), } + + // Merge the original annotations map, with our new secretAnnotations map + for k, v := range annotations { + secretAnnotations[k] = v + } + if autoRestart != "" { _, err := utils.StringToBool(autoRestart) if err != nil { log.Error(err, "Error parsing %v annotation on Secret %v. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, secretName) return err } - annotations[RestartDeploymentsAnnotation] = autoRestart + secretAnnotations[RestartDeploymentsAnnotation] = autoRestart } - secret := BuildKubernetesSecretFromOnePasswordItem(secretName, namespace, annotations, *item) + secret := BuildKubernetesSecretFromOnePasswordItem(secretName, namespace, secretAnnotations, labels, *item) currentSecret := &corev1.Secret{} err := kubeClient.Get(context.Background(), types.NamespacedName{Name: secret.Name, Namespace: secret.Namespace}, currentSecret) @@ -51,7 +63,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa if currentSecret.Annotations[VersionAnnotation] != itemVersion { log.Info(fmt.Sprintf("Updating Secret %v at namespace '%v'", secret.Name, secret.Namespace)) - currentSecret.ObjectMeta.Annotations = annotations + currentSecret.ObjectMeta.Annotations = secretAnnotations currentSecret.Data = secret.Data return kubeClient.Update(context.Background(), currentSecret) } @@ -60,12 +72,13 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa return nil } -func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotations map[string]string, item onepassword.Item) *corev1.Secret { +func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotations map[string]string, labels map[string]string, item onepassword.Item) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, Annotations: annotations, + Labels: labels, }, Data: BuildKubernetesSecretData(item.Fields), } diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go index cb331c6..c1913c8 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go @@ -30,7 +30,11 @@ func TestCreateKubernetesSecretFromOnePasswordItem(t *testing.T) { item.ID = "h46bb3jddvay7nxopfhvlwg35q" kubeClient := fake.NewFakeClient() - err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation) + secretLabels := map[string]string{} + secretAnnotations := map[string]string{ + "testAnnotation": "exists", + } + err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretAnnotations) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -42,6 +46,10 @@ func TestCreateKubernetesSecretFromOnePasswordItem(t *testing.T) { } compareFields(item.Fields, createdSecret.Data, t) compareAnnotationsToItem(createdSecret.Annotations, item, t) + + if createdSecret.Annotations["testAnnotation"] != "exists" { + t.Errorf("Expected testAnntion to be merged with existing annotations, but wasn't.") + } } func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) { @@ -55,7 +63,9 @@ func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) { item.ID = "h46bb3jddvay7nxopfhvlwg35q" kubeClient := fake.NewFakeClient() - err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation) + secretLabels := map[string]string{} + secretAnnotations := map[string]string{} + err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretAnnotations) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -66,7 +76,7 @@ func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) { newItem.Version = 456 newItem.Vault.ID = "hfnjvi6aymbsnfc2xeeoheizda" newItem.ID = "h46bb3jddvay7nxopfhvlwg35q" - err = CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &newItem, restartDeploymentAnnotation) + err = CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &newItem, restartDeploymentAnnotation, secretLabels, secretAnnotations) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -99,8 +109,9 @@ func TestBuildKubernetesSecretFromOnePasswordItem(t *testing.T) { } item := onepassword.Item{} item.Fields = generateFields(5) + labels := map[string]string{} - kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, item) + kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, labels, item) if kubeSecret.Name != name { t.Errorf("Expected name value: %v but got: %v", name, kubeSecret.Name) } diff --git a/pkg/onepassword/secret_update_handler.go b/pkg/onepassword/secret_update_handler.go index cb1f659..ad2e8d6 100644 --- a/pkg/onepassword/secret_update_handler.go +++ b/pkg/onepassword/secret_update_handler.go @@ -131,7 +131,7 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]* } log.Info(fmt.Sprintf("Updating kubernetes secret '%v'", secret.GetName())) secret.Annotations[VersionAnnotation] = itemVersion - updatedSecret := kubeSecrets.BuildKubernetesSecretFromOnePasswordItem(secret.Name, secret.Namespace, secret.Annotations, *item) + updatedSecret := kubeSecrets.BuildKubernetesSecretFromOnePasswordItem(secret.Name, secret.Namespace, secret.Annotations, secret.Labels, *item) h.client.Update(context.Background(), updatedSecret) if updatedSecrets[secret.Namespace] == nil { updatedSecrets[secret.Namespace] = make(map[string]*corev1.Secret) From ea2d1f8a09ccce72ab731b8b679aaab1d7bff252 Mon Sep 17 00:00:00 2001 From: mcmarkj Date: Fri, 28 May 2021 18:11:10 +0100 Subject: [PATCH 02/21] Typo --- pkg/kubernetessecrets/kubernetes_secrets_builder_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go index c1913c8..a1ec7e9 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go @@ -48,7 +48,7 @@ func TestCreateKubernetesSecretFromOnePasswordItem(t *testing.T) { compareAnnotationsToItem(createdSecret.Annotations, item, t) if createdSecret.Annotations["testAnnotation"] != "exists" { - t.Errorf("Expected testAnntion to be merged with existing annotations, but wasn't.") + t.Errorf("Expected testAnnotation to be merged with existing annotations, but wasn't.") } } From a428fe74620e9d36267005b7079f38bb32a28101 Mon Sep 17 00:00:00 2001 From: mcmarkj Date: Fri, 28 May 2021 18:15:17 +0100 Subject: [PATCH 03/21] GoFMT --- pkg/kubernetessecrets/kubernetes_secrets_builder.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 76671ef..2929363 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -23,14 +23,13 @@ const RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto-restart" var log = logf.Log - func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart string, labels map[string]string, annotations map[string]string) error { itemVersion := fmt.Sprint(item.Version) // Remove OP Annotations if they already exist - delete(annotations,VersionAnnotation) - delete(annotations,ItemPathAnnotation) + delete(annotations, VersionAnnotation) + delete(annotations, ItemPathAnnotation) secretAnnotations := map[string]string{ VersionAnnotation: itemVersion, @@ -78,7 +77,7 @@ func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotation Name: name, Namespace: namespace, Annotations: annotations, - Labels: labels, + Labels: labels, }, Data: BuildKubernetesSecretData(item.Fields), } From fb1262f1bd133428a09de1c7c88b0288373011fd Mon Sep 17 00:00:00 2001 From: mcmarkj Date: Mon, 7 Jun 2021 21:51:44 +0100 Subject: [PATCH 04/21] PR Feedback' --- .../kubernetes_secrets_builder.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 2929363..280c972 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -23,23 +23,17 @@ const RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto-restart" var log = logf.Log -func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart string, labels map[string]string, annotations map[string]string) error { +func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart string, labels map[string]string, secretAnnotations map[string]string) error { itemVersion := fmt.Sprint(item.Version) - // Remove OP Annotations if they already exist - delete(annotations, VersionAnnotation) - delete(annotations, ItemPathAnnotation) - - secretAnnotations := map[string]string{ - VersionAnnotation: itemVersion, - ItemPathAnnotation: fmt.Sprintf("vaults/%v/items/%v", item.Vault.ID, item.ID), + // If secretAnnotations is nil we create an empty map so we can later assign values for the OP Annotations in the map + if secretAnnotations == nil { + secretAnnotations = map[string]string{} } - // Merge the original annotations map, with our new secretAnnotations map - for k, v := range annotations { - secretAnnotations[k] = v - } + secretAnnotations[VersionAnnotation] = itemVersion + secretAnnotations[ItemPathAnnotation] = fmt.Sprintf("vaults/%v/items/%v", item.Vault.ID, item.ID) if autoRestart != "" { _, err := utils.StringToBool(autoRestart) From 2096f4440fb4e6011f1e1e60927e2bc92113820e Mon Sep 17 00:00:00 2001 From: mcmarkj Date: Tue, 3 Aug 2021 21:32:04 +0100 Subject: [PATCH 05/21] add logic for checking for label or annotation updates --- pkg/kubernetessecrets/kubernetes_secrets_builder.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 280c972..093ede1 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -54,9 +54,10 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa return err } - if currentSecret.Annotations[VersionAnnotation] != itemVersion { + if CompareSecretFieldsWithOnePasswordItem(currentSecret.Annotations, secretAnnotations) || CompareSecretFieldsWithOnePasswordItem(currentSecret.Labels, labels) { log.Info(fmt.Sprintf("Updating Secret %v at namespace '%v'", secret.Name, secret.Namespace)) currentSecret.ObjectMeta.Annotations = secretAnnotations + currentSecret.ObjectMeta.Labels = labels currentSecret.Data = secret.Data return kubeClient.Update(context.Background(), currentSecret) } @@ -86,3 +87,13 @@ func BuildKubernetesSecretData(fields []*onepassword.ItemField) map[string][]byt } return secretData } + +func CompareSecretFieldsWithOnePasswordItem(currentSecretsFields map[string]string, expectedFieldsOnSecret map[string]string) bool{ + for key, value := range expectedFieldsOnSecret { + currentValue, exists := currentSecretsFields[key] + if !exists || currentValue != value { + return true + } + } + return false +} \ No newline at end of file From dff934cbc363b3ce212ebb424268e4052c2690e0 Mon Sep 17 00:00:00 2001 From: mcmarkj Date: Wed, 4 Aug 2021 06:33:56 +0100 Subject: [PATCH 06/21] Fix tests --- 1 | 6 ++++++ pkg/controller/deployment/deployment_controller_test.go | 3 +++ pkg/controller/onepassworditem/onepassworditem_test.go | 2 ++ 3 files changed, 11 insertions(+) create mode 100644 1 diff --git a/1 b/1 new file mode 100644 index 0000000..426b912 --- /dev/null +++ b/1 @@ -0,0 +1,6 @@ +Merge branch 'main' of github.com:1Password/onepassword-operator into pass-labels-and-annotations +# Please enter a commit message to explain why this merge is necessary, +# especially if it merges an updated upstream into a topic branch. +# +# Lines starting with '#' will be ignored, and an empty message aborts +# the commit. diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index d4b99d2..9ab7a55 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -268,6 +268,7 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ + op.VersionAnnotation: fmt.Sprint(version), op.ItemPathAnnotation: itemPath, op.NameAnnotation: name, }, @@ -278,6 +279,7 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ + op.ItemPathAnnotation: itemPath, op.VersionAnnotation: fmt.Sprint(version), }, }, @@ -289,6 +291,7 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ + op.ItemPathAnnotation: itemPath, op.VersionAnnotation: fmt.Sprint(version), }, }, diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index 2639f18..26eb5df 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/pkg/controller/onepassworditem/onepassworditem_test.go @@ -117,6 +117,7 @@ var tests = []testReconcileItem{ Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, }, }, Data: expectedSecretData, @@ -128,6 +129,7 @@ var tests = []testReconcileItem{ Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, }, }, Data: expectedSecretData, From 579b5848da5294c191969e69038b2ed3cb1b511a Mon Sep 17 00:00:00 2001 From: "david.gunter" Date: Thu, 5 Aug 2021 16:34:50 -0700 Subject: [PATCH 07/21] 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 3c808bd..11bec45 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 2639f18..40108d8 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 08/21] 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 11bec45..c5cf899 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 40108d8..2ebb5e0 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 0c658f7..fba54fb 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 cb331c6..43879ce 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 c0037526b0698724c7c8d7c1f3fe8b04a61b2ebb Mon Sep 17 00:00:00 2001 From: mcmarkj Date: Sun, 15 Aug 2021 15:32:18 +0100 Subject: [PATCH 09/21] remove commit file --- 1 | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 1 diff --git a/1 b/1 deleted file mode 100644 index 426b912..0000000 --- a/1 +++ /dev/null @@ -1,6 +0,0 @@ -Merge branch 'main' of github.com:1Password/onepassword-operator into pass-labels-and-annotations -# Please enter a commit message to explain why this merge is necessary, -# especially if it merges an updated upstream into a topic branch. -# -# Lines starting with '#' will be ignored, and an empty message aborts -# the commit. From 8cfe98073e22cb3698b2725bdfd96d45bdef8bb4 Mon Sep 17 00:00:00 2001 From: Eddy Filip Date: Mon, 16 Aug 2021 14:51:44 +0200 Subject: [PATCH 10/21] 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 2ebb5e0..09d5b3b 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 43879ce..3c9484d 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 11/21] 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 a2ec464..ca2e7d3 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. --- From f241d7423d0c5952953469d3562d5397a5f2fdd4 Mon Sep 17 00:00:00 2001 From: mcmarkj Date: Thu, 19 Aug 2021 16:11:29 +0100 Subject: [PATCH 12/21] Use deepequal --- pkg/controller/deployment/deployment_controller.go | 5 ++--- .../deployment/deployment_controller_test.go | 11 +++++++---- .../onepassworditem/onepassworditem_test.go | 9 +++++++++ .../kubernetes_secrets_builder.go | 14 ++------------ 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index e4fd363..3b81fdf 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -191,8 +191,7 @@ func (r *ReconcileDeployment) HandleApplyingDeployment(namespace string, annotat reqLog := log.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) secretName := annotations[op.NameAnnotation] - secretLabels := map[string]string{} - secretAnnotations := map[string]string{} + secretLabels := map[string]string(nil) if len(secretName) == 0 { reqLog.Info("No 'item-name' annotation set. 'item-path' and 'item-name' must be set as annotations to add new secret.") return nil @@ -203,5 +202,5 @@ func (r *ReconcileDeployment) HandleApplyingDeployment(namespace string, annotat return fmt.Errorf("Failed to retrieve item: %v", err) } - return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, namespace, item, annotations[op.RestartDeploymentsAnnotation], secretLabels, secretAnnotations) + return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, namespace, item, annotations[op.RestartDeploymentsAnnotation], secretLabels, annotations) } diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index 9ab7a55..25ae956 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -258,7 +258,7 @@ var tests = []testReconcileItem{ }, }, { - testName: "Test Do not update if OnePassword Item Version has not changed", + testName: "Test Do not update if Annotations have not changed", deploymentResource: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: deploymentKind, @@ -268,10 +268,10 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ - op.VersionAnnotation: fmt.Sprint(version), op.ItemPathAnnotation: itemPath, op.NameAnnotation: name, }, + Labels: map[string]string{}, }, }, existingSecret: &corev1.Secret{ @@ -279,8 +279,9 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ - op.ItemPathAnnotation: itemPath, op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, + op.NameAnnotation: name, }, }, Data: expectedSecretData, @@ -291,9 +292,11 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ - op.ItemPathAnnotation: itemPath, op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, + op.NameAnnotation: name, }, + Labels: map[string]string(nil), }, Data: expectedSecretData, }, diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index 26eb5df..296ef27 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/pkg/controller/onepassworditem/onepassworditem_test.go @@ -149,6 +149,11 @@ var tests = []testReconcileItem{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, + Annotations: map[string]string{ + op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, + }, + Labels: map[string]string{}, }, Spec: onepasswordv1.OnePasswordItemSpec{ ItemPath: itemPath, @@ -160,7 +165,9 @@ var tests = []testReconcileItem{ Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: "456", + op.ItemPathAnnotation: itemPath, }, + Labels: map[string]string{}, }, Data: expectedSecretData, }, @@ -171,7 +178,9 @@ var tests = []testReconcileItem{ Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, }, + Labels: map[string]string{}, }, Data: expectedSecretData, }, diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 093ede1..40f411f 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -3,13 +3,13 @@ package kubernetessecrets import ( "context" "fmt" - "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" + "reflect" kubernetesClient "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -54,7 +54,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa return err } - if CompareSecretFieldsWithOnePasswordItem(currentSecret.Annotations, secretAnnotations) || CompareSecretFieldsWithOnePasswordItem(currentSecret.Labels, labels) { + if ! reflect.DeepEqual(currentSecret.Annotations, secretAnnotations) || ! reflect.DeepEqual(currentSecret.Labels, labels) { log.Info(fmt.Sprintf("Updating Secret %v at namespace '%v'", secret.Name, secret.Namespace)) currentSecret.ObjectMeta.Annotations = secretAnnotations currentSecret.ObjectMeta.Labels = labels @@ -87,13 +87,3 @@ func BuildKubernetesSecretData(fields []*onepassword.ItemField) map[string][]byt } return secretData } - -func CompareSecretFieldsWithOnePasswordItem(currentSecretsFields map[string]string, expectedFieldsOnSecret map[string]string) bool{ - for key, value := range expectedFieldsOnSecret { - currentValue, exists := currentSecretsFields[key] - if !exists || currentValue != value { - return true - } - } - return false -} \ No newline at end of file From 72cad7284ca1c6ba1cda8b4e01e893de0ec2bfb1 Mon Sep 17 00:00:00 2001 From: jillianwilson Date: Fri, 27 Aug 2021 15:21:07 -0300 Subject: [PATCH 13/21] Preparing release --- .VERSION | 2 +- CHANGELOG.md | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.VERSION b/.VERSION index b18d465..e6d5cb8 100644 --- a/.VERSION +++ b/.VERSION @@ -1 +1 @@ -v1.0.1 +1.0.2 \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dbf2a6..79cee80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,14 @@ --- +[//]: # (START/v1.0.2) +# v1.0.2 + +## Fixes + * Name normalizer added to handle non-conforming item names. + +--- + [//]: # (START/v1.0.1) # v1.0.1 From 2c4b4df01a55b88d603702177693640db1e61bec Mon Sep 17 00:00:00 2001 From: Marton Soos Date: Fri, 3 Sep 2021 15:41:46 +0300 Subject: [PATCH 14/21] Do not make secret names lowercase on normalization --- pkg/kubernetessecrets/kubernetes_secrets_builder.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index fba54fb..f2b706b 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -97,11 +97,10 @@ func formatSecretName(value string) string { return createValidSecretName(value) } -var invalidDNS1123Chars = regexp.MustCompile("[^a-z0-9-]+") +var invalidDNS1123Chars = regexp.MustCompile("[^a-zA-Z0-9-]+") func createValidSecretName(value string) string { - result := strings.ToLower(value) - result = invalidDNS1123Chars.ReplaceAllString(result, "-") + result := invalidDNS1123Chars.ReplaceAllString(value, "-") if len(result) > kubeValidate.DNS1123SubdomainMaxLength { result = result[0:kubeValidate.DNS1123SubdomainMaxLength] From e365ebfdfa9234873a366a0718cd4b95b146c281 Mon Sep 17 00:00:00 2001 From: Marton Soos Date: Fri, 3 Sep 2021 15:42:02 +0300 Subject: [PATCH 15/21] Fix tests --- pkg/controller/onepassworditem/onepassworditem_test.go | 6 +++--- pkg/kubernetessecrets/kubernetes_secrets_builder_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index 09d5b3b..db383cb 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/pkg/controller/onepassworditem/onepassworditem_test.go @@ -232,7 +232,7 @@ var tests = []testReconcileItem{ expectedError: nil, expectedResultSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "my-secret-it3m", + Name: "my-sECReT-it3m", Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: fmt.Sprint(version), @@ -264,7 +264,7 @@ var tests = []testReconcileItem{ expectedError: nil, expectedResultSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "my-secret-it3m", + Name: "my-sECReT-it3m", Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: fmt.Sprint(version), @@ -274,7 +274,7 @@ var tests = []testReconcileItem{ "password": []byte(password), "username": []byte(username), "first-host": []byte(firstHost), - "aws-access-key": []byte(awsKey), + "AWS-Access-Key": []byte(awsKey), "ice-cream-type": []byte(iceCream), }, }, diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go index 3c9484d..3ce27ee 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 != strings.ToLower(name) { + if kubeSecret.Name != name { t.Errorf("Expected name value: %v but got: %v", name, kubeSecret.Name) } if kubeSecret.Namespace != namespace { @@ -116,7 +116,7 @@ func TestBuildKubernetesSecretFromOnePasswordItem(t *testing.T) { func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) { name := "inV@l1d k8s secret%name" - expectedName := "inv-l1d-k8s-secret-name" + expectedName := "inV-l1d-k8s-secret-name" namespace := "someNamespace" annotations := map[string]string{ "annotationKey": "annotationValue", From 88728909ff28fca2a6f46da2be64628826aae3ce Mon Sep 17 00:00:00 2001 From: Eddy Filip Date: Wed, 8 Sep 2021 13:49:32 +0300 Subject: [PATCH 16/21] Adjust regex to support `_` and `.` and trim them Now secret names can also contain `_` and `.` and they will be trimmed from start and end of string to be DNS1123 compliant --- pkg/kubernetessecrets/kubernetes_secrets_builder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index f2b706b..49a7522 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -97,7 +97,7 @@ func formatSecretName(value string) string { return createValidSecretName(value) } -var invalidDNS1123Chars = regexp.MustCompile("[^a-zA-Z0-9-]+") +var invalidDNS1123Chars = regexp.MustCompile("[^a-zA-Z0-9-_.]+") func createValidSecretName(value string) string { result := invalidDNS1123Chars.ReplaceAllString(value, "-") @@ -107,5 +107,5 @@ func createValidSecretName(value string) string { } // first and last character MUST be alphanumeric - return strings.Trim(result, "-") + return strings.Trim(result, "-._") } From d80e8dd799d5f769ff72763caaae4aa8ee634276 Mon Sep 17 00:00:00 2001 From: Eddy Filip Date: Wed, 8 Sep 2021 13:58:48 +0300 Subject: [PATCH 17/21] Add tests with names that contain `.` and `_` --- .../onepassworditem/onepassworditem_test.go | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index db383cb..858fe74 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/pkg/controller/onepassworditem/onepassworditem_test.go @@ -286,6 +286,47 @@ var tests = []testReconcileItem{ "😄 ice-cream type": iceCream, }, }, + { + testName: "Secret from 1Password item with `_` and `.`", + 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) { From a45a310611dc7638cdcd77057b7b57a8600f9e12 Mon Sep 17 00:00:00 2001 From: Eddy Filip Date: Wed, 8 Sep 2021 15:36:40 +0300 Subject: [PATCH 18/21] Make secret names DNS1123 Subdomain compiant This is done while ensuring that secret keys are compliant (contain alphanumeric characters, `-`, `_` and `.`) --- .../onepassworditem/onepassworditem_test.go | 18 +++++----- .../kubernetes_secrets_builder.go | 34 +++++++++++++++---- .../kubernetes_secrets_builder_test.go | 10 +++--- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index 858fe74..228cf7f 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/pkg/controller/onepassworditem/onepassworditem_test.go @@ -232,7 +232,7 @@ var tests = []testReconcileItem{ expectedError: nil, expectedResultSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "my-sECReT-it3m", + Name: "my-secret-it3m", Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: fmt.Sprint(version), @@ -264,7 +264,7 @@ var tests = []testReconcileItem{ expectedError: nil, expectedResultSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "my-sECReT-it3m", + Name: "my-secret-it3m", Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: fmt.Sprint(version), @@ -287,7 +287,7 @@ var tests = []testReconcileItem{ }, }, { - testName: "Secret from 1Password item with `_` and `.`", + testName: "Secret from 1Password item with `-`, `_` and `.`", customResource: &onepasswordv1.OnePasswordItem{ TypeMeta: metav1.TypeMeta{ Kind: onePasswordItemKind, @@ -305,18 +305,18 @@ var tests = []testReconcileItem{ expectedError: nil, expectedResultSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "my_sECReT.it3m", + 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), + "password": []byte(password), + "username": []byte(username), + "first-host": []byte(firstHost), + "AWS-Access-Key": []byte(awsKey), + "-_ice_cream.type.": []byte(iceCream), }, }, opItem: map[string]string{ diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 49a7522..571e25b 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -80,16 +80,17 @@ func BuildKubernetesSecretData(fields []*onepassword.ItemField) map[string][]byt secretData := map[string][]byte{} for i := 0; i < len(fields); i++ { if fields[i].Value != "" { - key := formatSecretName(fields[i].Label) + key := formatSecretDataName(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. +// formatSecretName rewrites a value to be a valid Secret name. // -// The Secret meta.name and data keys must be valid DNS subdomain names (https://kubernetes.io/docs/concepts/configuration/secret/#overview-of-secrets) +// 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 @@ -97,15 +98,36 @@ func formatSecretName(value string) string { return createValidSecretName(value) } -var invalidDNS1123Chars = regexp.MustCompile("[^a-zA-Z0-9-_.]+") +// formatSecretDataName rewrites a value to be a valid Secret data key. +// +// The Secret data keys must consist of alphanumeric numbers, `-`, `_` or `.` +// (https://kubernetes.io/docs/concepts/configuration/secret/#overview-of-secrets) +func formatSecretDataName(value string) string { + if errs := kubeValidate.IsConfigMapKey(value); len(errs) == 0 { + return value + } + return createValidSecretDataName(value) +} + +var invalidDNS1123Chars = regexp.MustCompile("[^a-z0-9-.]+") func createValidSecretName(value string) string { - result := invalidDNS1123Chars.ReplaceAllString(value, "-") + 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, "-._") + return strings.Trim(result, "-.") +} + +var invalidDataChars = regexp.MustCompile("[^a-zA-Z0-9-._]+") +var invalidStartEndChars = regexp.MustCompile("(^[^a-zA-Z0-9-._]+|[^a-zA-Z0-9-._]+$)") + +func createValidSecretDataName(value string) string { + result := invalidStartEndChars.ReplaceAllString(value, "") + result = invalidDataChars.ReplaceAllString(result, "-") + return result } diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go index 3ce27ee..ccd86d3 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go @@ -3,13 +3,13 @@ package kubernetessecrets import ( "context" "fmt" - kubeValidate "k8s.io/apimachinery/pkg/util/validation" "strings" "testing" "github.com/1Password/connect-sdk-go/onepassword" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + kubeValidate "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -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 { @@ -116,7 +116,7 @@ func TestBuildKubernetesSecretFromOnePasswordItem(t *testing.T) { func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) { name := "inV@l1d k8s secret%name" - expectedName := "inV-l1d-k8s-secret-name" + expectedName := "inv-l1d-k8s-secret-name" namespace := "someNamespace" annotations := map[string]string{ "annotationKey": "annotationValue", @@ -129,7 +129,7 @@ func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) { Value: "value1", }, { - Label: strings.Repeat("x", kubeValidate.DNS1123SubdomainMaxLength+1), + Label: strings.Repeat("x", kubeValidate.LabelValueMaxLength+1), Value: "name exceeds max length", }, } @@ -205,7 +205,7 @@ func ParseVaultIdAndItemIdFromPath(path string) (string, string, error) { } func validLabel(v string) bool { - if err := kubeValidate.IsDNS1123Subdomain(v); len(err) > 0 { + if err := kubeValidate.IsConfigMapKey(v); len(err) > 0 { return false } return true From 670040477e8e1d7baf1842ac21af7cc6bb8d01f2 Mon Sep 17 00:00:00 2001 From: Eddy Filip Date: Wed, 8 Sep 2021 16:02:08 +0300 Subject: [PATCH 19/21] Add max length for secret key names Max length for secret key names must be DNS1123 compliant (253) --- pkg/kubernetessecrets/kubernetes_secrets_builder.go | 5 +++++ pkg/kubernetessecrets/kubernetes_secrets_builder_test.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 571e25b..f07d86e 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -129,5 +129,10 @@ var invalidStartEndChars = regexp.MustCompile("(^[^a-zA-Z0-9-._]+|[^a-zA-Z0-9-._ func createValidSecretDataName(value string) string { result := invalidStartEndChars.ReplaceAllString(value, "") result = invalidDataChars.ReplaceAllString(result, "-") + + if len(result) > kubeValidate.DNS1123SubdomainMaxLength { + result = result[0:kubeValidate.DNS1123SubdomainMaxLength] + } + return result } diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go index ccd86d3..0d7d22b 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go @@ -129,7 +129,7 @@ func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) { Value: "value1", }, { - Label: strings.Repeat("x", kubeValidate.LabelValueMaxLength+1), + Label: strings.Repeat("x", kubeValidate.DNS1123SubdomainMaxLength+1), Value: "name exceeds max length", }, } From 244771717c0d60c4964e2454097d0f336c435de1 Mon Sep 17 00:00:00 2001 From: "david.gunter" Date: Thu, 23 Sep 2021 11:18:53 -0700 Subject: [PATCH 20/21] Prepare release/1.1.0 --- .VERSION | 2 +- CHANGELOG.md | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.VERSION b/.VERSION index e6d5cb8..1cc5f65 100644 --- a/.VERSION +++ b/.VERSION @@ -1 +1 @@ -1.0.2 \ No newline at end of file +1.1.0 \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 79cee80..a45b92c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,14 @@ --- +[//]: # (START/v1.1.0) +# v1.1.0 + +## Fixes + * Fix normalization for keys in a Secret's `data` section to allow upper- and lower-case alphanumeric characters. {#66} + +--- + [//]: # (START/v1.0.2) # v1.0.2 From 0363ae1e4ebf865946238f0ecc7f3b056c0ca989 Mon Sep 17 00:00:00 2001 From: Claudio Canales Date: Wed, 29 Sep 2021 16:16:45 -0300 Subject: [PATCH 21/21] Removing $ from bash commands Using the copy button is bringing the commands with a $, which is giving the error `-bash: $: command not found` after pasting them to the console. --- README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ca2e7d3..3a4f8fc 100644 --- a/README.md +++ b/README.md @@ -30,14 +30,14 @@ If 1Password Connect is already running, you can skip this step. This guide will Encode the 1password-credentials.json file you generated in the prerequisite steps and save it to a file named op-session: ```bash -$ cat 1password-credentials.json | base64 | \ +cat 1password-credentials.json | base64 | \ tr '/+' '_-' | tr -d '=' | tr -d '\n' > op-session ``` Create a Kubernetes secret from the op-session file: ```bash -$ kubectl create secret generic op-credentials --from-file=1password-credentials.json +kubectl create secret generic op-credentials --from-file=1password-credentials.json ``` Add the following environment variable to the onepassword-connect-operator container in `deploy/operator.yaml`: @@ -53,12 +53,12 @@ Adding this environment variable will have the operator automatically deploy a d "Create a Connect token for the operator and save it as a Kubernetes Secret: ```bash -$ kubectl create secret generic onepassword-token --from-literal=token=" +kubectl create secret generic onepassword-token --from-literal=token=" ``` If you do not have a token for the operator, you can generate a token and save it to kubernetes with the following command: ```bash -$ kubectl create secret generic onepassword-token --from-literal=token=$(op create connect token op-k8s-operator --vault ) +kubectl create secret generic onepassword-token --from-literal=token=$(op create connect token op-k8s-operator --vault ) ``` [More information on generating a token can be found here](https://support.1password.com/secrets-automation/#appendix-issue-additional-access-tokens) @@ -68,13 +68,13 @@ $ kubectl create secret generic onepassword-token --from-literal=token=$(op crea We must create a service account, role, and role binding and Kubernetes. Examples can be found in the `/deploy` folder. ```bash -$ kubectl apply -f deploy/permissions.yaml +kubectl apply -f deploy/permissions.yaml ``` **Create Custom One Password Secret Resource** ```bash -$ kubectl apply -f deploy/crds/onepassword.com_onepassworditems_crd.yaml +kubectl apply -f deploy/crds/onepassword.com_onepassworditems_crd.yaml ``` **Deploying the Operator** @@ -112,13 +112,13 @@ spec: Deploy the OnePasswordItem to Kubernetes: ```bash -$ kubectl apply -f .yaml +kubectl apply -f .yaml ``` To test that the Kubernetes Secret check that the following command returns a secret: ```bash -$ kubectl get secret +kubectl get secret ``` Note: Deleting the `OnePasswordItem` that you've created will automatically delete the created Kubernetes Secret.