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 diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 93ff956..3b81fdf 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -191,6 +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(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 @@ -201,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]) + 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 0d5e427..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 or VaultPath has not changed", + testName: "Test Do not update if Annotations have not changed", deploymentResource: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: deploymentKind, @@ -271,6 +271,7 @@ var tests = []testReconcileItem{ op.ItemPathAnnotation: itemPath, op.NameAnnotation: name, }, + Labels: map[string]string{}, }, }, existingSecret: &corev1.Secret{ @@ -278,8 +279,9 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ - op.VersionAnnotation: fmt.Sprint(version), + op.VersionAnnotation: fmt.Sprint(version), op.ItemPathAnnotation: itemPath, + op.NameAnnotation: name, }, }, Data: expectedSecretData, @@ -291,7 +293,10 @@ var tests = []testReconcileItem{ Namespace: namespace, Annotations: map[string]string{ 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_controller.go b/pkg/controller/onepassworditem/onepassworditem_controller.go index c5cf899..dc7fdff 100644 --- a/pkg/controller/onepassworditem/onepassworditem_controller.go +++ b/pkg/controller/onepassworditem/onepassworditem_controller.go @@ -143,12 +143,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/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index 1c3db6f..85915a2 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/pkg/controller/onepassworditem/onepassworditem_test.go @@ -132,6 +132,7 @@ var tests = []testReconcileItem{ Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, }, }, Data: expectedSecretData, @@ -151,6 +152,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, @@ -162,7 +168,9 @@ var tests = []testReconcileItem{ Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: "456", + op.ItemPathAnnotation: itemPath, }, + Labels: map[string]string{}, }, Data: expectedSecretData, }, @@ -173,7 +181,9 @@ var tests = []testReconcileItem{ Namespace: namespace, Annotations: map[string]string{ op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, }, + Labels: map[string]string{}, }, Data: expectedSecretData, }, @@ -275,7 +285,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), }, }, @@ -287,6 +297,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) { diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 356db09..48e80fa 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "reflect" kubeValidate "k8s.io/apimachinery/pkg/util/validation" kubernetesClient "sigs.k8s.io/controller-runtime/pkg/client" @@ -28,22 +29,27 @@ 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, secretAnnotations map[string]string) error { itemVersion := fmt.Sprint(item.Version) - annotations := 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{} } + + secretAnnotations[VersionAnnotation] = itemVersion + secretAnnotations[ItemPathAnnotation] = fmt.Sprintf("vaults/%v/items/%v", item.Vault.ID, item.ID) + 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) @@ -54,9 +60,10 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa return err } - if currentSecret.Annotations[VersionAnnotation] != itemVersion || currentSecret.Annotations[ItemPathAnnotation] != annotations[ItemPathAnnotation]{ + 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 = annotations + currentSecret.ObjectMeta.Annotations = secretAnnotations + currentSecret.ObjectMeta.Labels = labels currentSecret.Data = secret.Data return kubeClient.Update(context.Background(), currentSecret) } @@ -65,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: formatSecretName(name), Namespace: namespace, Annotations: annotations, + Labels: labels, }, Data: BuildKubernetesSecretData(item.Fields), } @@ -80,16 +88,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,7 +106,18 @@ func formatSecretName(value string) string { return createValidSecretName(value) } -var invalidDNS1123Chars = regexp.MustCompile("[^a-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 := strings.ToLower(value) @@ -108,5 +128,19 @@ func createValidSecretName(value string) string { } // 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, "-") + + 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 3c9484d..63c0d0c 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" ) @@ -31,7 +31,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) } @@ -43,6 +47,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 testAnnotation to be merged with existing annotations, but wasn't.") + } } func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) { @@ -56,7 +64,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) } @@ -67,7 +77,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) } @@ -100,8 +110,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 != strings.ToLower(name) { t.Errorf("Expected name value: %v but got: %v", name, kubeSecret.Name) } @@ -121,6 +132,7 @@ func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) { annotations := map[string]string{ "annotationKey": "annotationValue", } + labels := map[string]string{} item := onepassword.Item{} item.Fields = []*onepassword.ItemField{ @@ -134,7 +146,7 @@ func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) { }, } - kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, item) + kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, labels, item) // Assert Secret's meta.name was fixed if kubeSecret.Name != expectedName { @@ -205,7 +217,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 diff --git a/pkg/onepassword/secret_update_handler.go b/pkg/onepassword/secret_update_handler.go index 088b0cb..a51ac52 100644 --- a/pkg/onepassword/secret_update_handler.go +++ b/pkg/onepassword/secret_update_handler.go @@ -138,7 +138,7 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]* log.Info(fmt.Sprintf("Updating kubernetes secret '%v'", secret.GetName())) secret.Annotations[VersionAnnotation] = itemVersion secret.Annotations[ItemPathAnnotation] = itemPathString - updatedSecret := kubeSecrets.BuildKubernetesSecretFromOnePasswordItem(secret.Name, secret.Namespace, secret.Annotations, *item) + updatedSecret := kubeSecrets.BuildKubernetesSecretFromOnePasswordItem(secret.Name, secret.Namespace, secret.Annotations, secret.Labels, *item) log.Info(fmt.Sprintf("New secret path: %v and version: %v", updatedSecret.Annotations[ItemPathAnnotation], updatedSecret.Annotations[VersionAnnotation])) h.client.Update(context.Background(), updatedSecret) if updatedSecrets[secret.Namespace] == nil {