From 5d229c42d5b5078ce7af3f363dca8011084078ca Mon Sep 17 00:00:00 2001 From: Andres Montalban Date: Wed, 17 Nov 2021 18:47:12 -0300 Subject: [PATCH 1/6] feat: Allow configuration of the Kubernetes Secret type to be created --- .../onepassword.com_onepassworditems_crd.yaml | 3 ++ .../onepassword/v1/onepasswordsecret_types.go | 1 + .../deployment/deployment_controller.go | 4 +- .../onepassworditem_controller.go | 4 +- .../kubernetes_secrets_builder.go | 13 +++-- .../kubernetes_secrets_builder_test.go | 50 +++++++++++++++++-- .../kubernetes_secrets_types.go | 17 +++++++ pkg/onepassword/secret_update_handler.go | 2 +- 8 files changed, 81 insertions(+), 13 deletions(-) create mode 100644 pkg/kubernetessecrets/kubernetes_secrets_types.go diff --git a/deploy/crds/onepassword.com_onepassworditems_crd.yaml b/deploy/crds/onepassword.com_onepassworditems_crd.yaml index 3a21918..2a8dc9e 100644 --- a/deploy/crds/onepassword.com_onepassworditems_crd.yaml +++ b/deploy/crds/onepassword.com_onepassworditems_crd.yaml @@ -39,4 +39,7 @@ spec: status: description: OnePasswordItemStatus defines the observed state of OnePasswordItem type: object + type: + description: 'Kubernetes secret type. More info: https://kubernetes.io/docs/concepts/configuration/secret/#secret-types' + type: string type: object diff --git a/pkg/apis/onepassword/v1/onepasswordsecret_types.go b/pkg/apis/onepassword/v1/onepasswordsecret_types.go index 7918539..a2e9ba3 100644 --- a/pkg/apis/onepassword/v1/onepasswordsecret_types.go +++ b/pkg/apis/onepassword/v1/onepasswordsecret_types.go @@ -26,6 +26,7 @@ type OnePasswordItemStatus struct { type OnePasswordItem struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` + Type string `json:"type,omitempty"` Spec OnePasswordItemSpec `json:"spec,omitempty"` Status OnePasswordItemStatus `json:"status,omitempty"` diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 3b81fdf..fc2183d 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -192,6 +192,8 @@ func (r *ReconcileDeployment) HandleApplyingDeployment(namespace string, annotat secretName := annotations[op.NameAnnotation] secretLabels := map[string]string(nil) + secretType := "" + 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 @@ -202,5 +204,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, annotations) + return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, namespace, item, annotations[op.RestartDeploymentsAnnotation], secretLabels, secretType, annotations) } diff --git a/pkg/controller/onepassworditem/onepassworditem_controller.go b/pkg/controller/onepassworditem/onepassworditem_controller.go index dc7fdff..0788f02 100644 --- a/pkg/controller/onepassworditem/onepassworditem_controller.go +++ b/pkg/controller/onepassworditem/onepassworditem_controller.go @@ -3,6 +3,7 @@ package onepassworditem import ( "context" "fmt" + onepasswordv1 "github.com/1Password/onepassword-operator/pkg/apis/onepassword/v1" kubeSecrets "github.com/1Password/onepassword-operator/pkg/kubernetessecrets" "github.com/1Password/onepassword-operator/pkg/onepassword" @@ -145,6 +146,7 @@ func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1 secretName := resource.GetName() labels := resource.Labels annotations := resource.Annotations + secretType := resource.Type autoRestart := annotations[op.RestartDeploymentsAnnotation] item, err := onepassword.GetOnePasswordItemByPath(r.opConnectClient, resource.Spec.ItemPath) @@ -152,5 +154,5 @@ func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1 return fmt.Errorf("Failed to retrieve item: %v", err) } - return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, resource.Namespace, item, autoRestart, labels, annotations) + return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, resource.Namespace, item, autoRestart, labels, secretType, annotations) } diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 48e80fa..e800ad9 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -7,13 +7,14 @@ import ( "regexp" "strings" + "reflect" + "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" kubeValidate "k8s.io/apimachinery/pkg/util/validation" kubernetesClient "sigs.k8s.io/controller-runtime/pkg/client" @@ -29,7 +30,7 @@ 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, secretAnnotations map[string]string) error { +func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart string, labels map[string]string, secretType string, secretAnnotations map[string]string) error { itemVersion := fmt.Sprint(item.Version) @@ -49,7 +50,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa } secretAnnotations[RestartDeploymentsAnnotation] = autoRestart } - secret := BuildKubernetesSecretFromOnePasswordItem(secretName, namespace, secretAnnotations, labels, *item) + secret := BuildKubernetesSecretFromOnePasswordItem(secretName, namespace, secretAnnotations, labels, secretType, *item) currentSecret := &corev1.Secret{} err := kubeClient.Get(context.Background(), types.NamespacedName{Name: secret.Name, Namespace: secret.Namespace}, currentSecret) @@ -60,11 +61,12 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa return err } - if ! reflect.DeepEqual(currentSecret.Annotations, secretAnnotations) || ! reflect.DeepEqual(currentSecret.Labels, labels) { + if !reflect.DeepEqual(currentSecret.Annotations, secretAnnotations) || !reflect.DeepEqual(currentSecret.Labels, labels) || !reflect.DeepEqual(string(currentSecret.Type), secretType) { 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 + currentSecret.Type = KubernetesSecretTypes[secretType] return kubeClient.Update(context.Background(), currentSecret) } @@ -72,7 +74,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa return nil } -func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotations map[string]string, labels map[string]string, item onepassword.Item) *corev1.Secret { +func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotations map[string]string, labels map[string]string, secretType string, item onepassword.Item) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: formatSecretName(name), @@ -81,6 +83,7 @@ func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotation Labels: labels, }, Data: BuildKubernetesSecretData(item.Fields), + Type: KubernetesSecretTypes[secretType], } } diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go index 63c0d0c..e0fd732 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go @@ -35,7 +35,9 @@ func TestCreateKubernetesSecretFromOnePasswordItem(t *testing.T) { secretAnnotations := map[string]string{ "testAnnotation": "exists", } - err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretAnnotations) + secretType := "" + + err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, secretAnnotations) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -66,7 +68,10 @@ func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) { kubeClient := fake.NewFakeClient() secretLabels := map[string]string{} secretAnnotations := map[string]string{} - err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretAnnotations) + secretType := "" + + err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, secretAnnotations) + if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -77,7 +82,7 @@ func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) { newItem.Version = 456 newItem.Vault.ID = "hfnjvi6aymbsnfc2xeeoheizda" newItem.ID = "h46bb3jddvay7nxopfhvlwg35q" - err = CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &newItem, restartDeploymentAnnotation, secretLabels, secretAnnotations) + err = CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &newItem, restartDeploymentAnnotation, secretLabels, secretType, secretAnnotations) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -111,8 +116,9 @@ func TestBuildKubernetesSecretFromOnePasswordItem(t *testing.T) { item := onepassword.Item{} item.Fields = generateFields(5) labels := map[string]string{} + secretType := "" - kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, labels, item) + kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, labels, secretType, item) if kubeSecret.Name != strings.ToLower(name) { t.Errorf("Expected name value: %v but got: %v", name, kubeSecret.Name) } @@ -134,6 +140,7 @@ func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) { } labels := map[string]string{} item := onepassword.Item{} + secretType := "" item.Fields = []*onepassword.ItemField{ { @@ -146,7 +153,7 @@ func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) { }, } - kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, labels, item) + kubeSecret := BuildKubernetesSecretFromOnePasswordItem(name, namespace, annotations, labels, secretType, item) // Assert Secret's meta.name was fixed if kubeSecret.Name != expectedName { @@ -164,6 +171,39 @@ func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) { } } +func TestCreateKubernetesTLSSecretFromOnePasswordItem(t *testing.T) { + secretName := "tls-test-secret-name" + namespace := "test" + + item := onepassword.Item{} + item.Fields = generateFields(5) + item.Version = 123 + item.Vault.ID = "hfnjvi6aymbsnfc2xeeoheizda" + item.ID = "h46bb3jddvay7nxopfhvlwg35q" + + kubeClient := fake.NewFakeClient() + secretLabels := map[string]string{} + secretAnnotations := map[string]string{ + "testAnnotation": "exists", + } + secretType := "kubernetes.io/tls" + + err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, secretAnnotations) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + createdSecret := &corev1.Secret{} + err = kubeClient.Get(context.Background(), types.NamespacedName{Name: secretName, Namespace: namespace}, createdSecret) + + if err != nil { + t.Errorf("Secret was not created: %v", err) + } + + if createdSecret.Type != corev1.SecretTypeTLS { + t.Errorf("Expected secretType to be of tyype corev1.SecretTypeTLS, got %s", string(createdSecret.Type)) + } +} + func compareAnnotationsToItem(annotations map[string]string, item onepassword.Item, t *testing.T) { actualVaultId, actualItemId, err := ParseVaultIdAndItemIdFromPath(annotations[ItemPathAnnotation]) if err != nil { diff --git a/pkg/kubernetessecrets/kubernetes_secrets_types.go b/pkg/kubernetessecrets/kubernetes_secrets_types.go new file mode 100644 index 0000000..16cd8d4 --- /dev/null +++ b/pkg/kubernetessecrets/kubernetes_secrets_types.go @@ -0,0 +1,17 @@ +package kubernetessecrets + +import ( + corev1 "k8s.io/api/core/v1" +) + +// Default to Opaque as this is Kubernetes' default +var KubernetesSecretTypes = map[string]corev1.SecretType{ + "Opaque": corev1.SecretTypeOpaque, + "kubernetes.io/basic-auth": corev1.SecretTypeBasicAuth, + "kubernetes.io/service-account-token": corev1.SecretTypeServiceAccountToken, + "kubernetes.io/dockercfg": corev1.SecretTypeDockercfg, + "kubernetes.io/dockerconfigjson": corev1.SecretTypeDockerConfigJson, + "kubernetes.io/ssh-auth": corev1.SecretTypeSSHAuth, + "kubernetes.io/tls": corev1.SecretTypeTLS, + "bootstrap.kubernetes.io/token": corev1.SecretTypeBootstrapToken, +} diff --git a/pkg/onepassword/secret_update_handler.go b/pkg/onepassword/secret_update_handler.go index ad2e8d6..a261ac0 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, secret.Labels, *item) + updatedSecret := kubeSecrets.BuildKubernetesSecretFromOnePasswordItem(secret.Name, secret.Namespace, secret.Annotations, secret.Labels, string(secret.Type), *item) h.client.Update(context.Background(), updatedSecret) if updatedSecrets[secret.Namespace] == nil { updatedSecrets[secret.Namespace] = make(map[string]*corev1.Secret) From 302653832ea05c8e398086fe5fc9adb3d16f36ee Mon Sep 17 00:00:00 2001 From: Marton Soos Date: Thu, 17 Feb 2022 19:18:33 +0100 Subject: [PATCH 2/6] Account for the fact that the '' type and Opaque are equivalent on secret comparison --- pkg/kubernetessecrets/kubernetes_secrets_builder.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index e800ad9..8ac14aa 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -61,7 +61,16 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa return err } - if !reflect.DeepEqual(currentSecret.Annotations, secretAnnotations) || !reflect.DeepEqual(currentSecret.Labels, labels) || !reflect.DeepEqual(string(currentSecret.Type), secretType) { + currentAnnotations := currentSecret.Annotations + currentLabels := currentSecret.Labels + currentSecretType := string(currentSecret.Type) + if currentSecretType == "" { + currentSecretType = "Opaque" + } + if secretType == "" { + secretType = "Opaque" + } + if !reflect.DeepEqual(currentAnnotations, secretAnnotations) || !reflect.DeepEqual(currentLabels, labels) || !reflect.DeepEqual(currentSecretType, secretType) { log.Info(fmt.Sprintf("Updating Secret %v at namespace '%v'", secret.Name, secret.Namespace)) currentSecret.ObjectMeta.Annotations = secretAnnotations currentSecret.ObjectMeta.Labels = labels From bb7a0c8ca9687cd4e575e1b93fb56a9332e5e619 Mon Sep 17 00:00:00 2001 From: Marton Soos Date: Thu, 17 Feb 2022 19:36:49 +0100 Subject: [PATCH 3/6] Simplify secret type cast and default to Opaque --- .../kubernetes_secrets_builder.go | 12 +++++++----- .../kubernetes_secrets_types.go | 17 ----------------- 2 files changed, 7 insertions(+), 22 deletions(-) delete mode 100644 pkg/kubernetessecrets/kubernetes_secrets_types.go diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 8ac14aa..4548ceb 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -50,6 +50,11 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa } secretAnnotations[RestartDeploymentsAnnotation] = autoRestart } + + // Default to Opaque secrets + if secretType == "" { + secretType = "Opaque" + } secret := BuildKubernetesSecretFromOnePasswordItem(secretName, namespace, secretAnnotations, labels, secretType, *item) currentSecret := &corev1.Secret{} @@ -67,15 +72,12 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa if currentSecretType == "" { currentSecretType = "Opaque" } - if secretType == "" { - secretType = "Opaque" - } if !reflect.DeepEqual(currentAnnotations, secretAnnotations) || !reflect.DeepEqual(currentLabels, labels) || !reflect.DeepEqual(currentSecretType, secretType) { 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 - currentSecret.Type = KubernetesSecretTypes[secretType] + currentSecret.Type = corev1.SecretType(secretType) return kubeClient.Update(context.Background(), currentSecret) } @@ -92,7 +94,7 @@ func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotation Labels: labels, }, Data: BuildKubernetesSecretData(item.Fields), - Type: KubernetesSecretTypes[secretType], + Type: corev1.SecretType(secretType), } } diff --git a/pkg/kubernetessecrets/kubernetes_secrets_types.go b/pkg/kubernetessecrets/kubernetes_secrets_types.go deleted file mode 100644 index 16cd8d4..0000000 --- a/pkg/kubernetessecrets/kubernetes_secrets_types.go +++ /dev/null @@ -1,17 +0,0 @@ -package kubernetessecrets - -import ( - corev1 "k8s.io/api/core/v1" -) - -// Default to Opaque as this is Kubernetes' default -var KubernetesSecretTypes = map[string]corev1.SecretType{ - "Opaque": corev1.SecretTypeOpaque, - "kubernetes.io/basic-auth": corev1.SecretTypeBasicAuth, - "kubernetes.io/service-account-token": corev1.SecretTypeServiceAccountToken, - "kubernetes.io/dockercfg": corev1.SecretTypeDockercfg, - "kubernetes.io/dockerconfigjson": corev1.SecretTypeDockerConfigJson, - "kubernetes.io/ssh-auth": corev1.SecretTypeSSHAuth, - "kubernetes.io/tls": corev1.SecretTypeTLS, - "bootstrap.kubernetes.io/token": corev1.SecretTypeBootstrapToken, -} From f38cf7e1c28f95ce9c4fa0a64a1927a783948104 Mon Sep 17 00:00:00 2001 From: Marton Soos Date: Thu, 17 Feb 2022 21:23:22 +0100 Subject: [PATCH 4/6] Fix tests and add new test --- .../deployment/deployment_controller_test.go | 7 +- .../onepassworditem/onepassworditem_test.go | 69 +++++++++++++++++-- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index 25ae956..ee6ac97 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -279,7 +279,7 @@ 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, }, @@ -292,7 +292,7 @@ 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, }, @@ -329,6 +329,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: "456", }, }, + Type: corev1.SecretTypeOpaque, Data: expectedSecretData, }, expectedError: nil, @@ -340,6 +341,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, + Type: corev1.SecretTypeOpaque, Data: expectedSecretData, }, opItem: map[string]string{ @@ -373,6 +375,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, + Type: corev1.SecretTypeOpaque, Data: expectedSecretData, }, opItem: map[string]string{ diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index 381e336..f6bf8e2 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/pkg/controller/onepassworditem/onepassworditem_test.go @@ -119,7 +119,7 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ - op.VersionAnnotation: fmt.Sprint(version), + op.VersionAnnotation: fmt.Sprint(version), op.ItemPathAnnotation: itemPath, }, }, @@ -131,7 +131,7 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ - op.VersionAnnotation: fmt.Sprint(version), + op.VersionAnnotation: fmt.Sprint(version), op.ItemPathAnnotation: itemPath, }, }, @@ -153,7 +153,7 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ - op.VersionAnnotation: fmt.Sprint(version), + op.VersionAnnotation: fmt.Sprint(version), op.ItemPathAnnotation: itemPath, }, Labels: map[string]string{}, @@ -167,7 +167,7 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ - op.VersionAnnotation: "456", + op.VersionAnnotation: "456", op.ItemPathAnnotation: itemPath, }, Labels: map[string]string{}, @@ -180,11 +180,65 @@ var tests = []testReconcileItem{ Name: name, Namespace: namespace, Annotations: map[string]string{ - op.VersionAnnotation: fmt.Sprint(version), + op.VersionAnnotation: fmt.Sprint(version), op.ItemPathAnnotation: itemPath, }, Labels: map[string]string{}, }, + Type: corev1.SecretTypeOpaque, + Data: expectedSecretData, + }, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + }, + { + testName: "Test Updating Type of Existing Kubernetes Secret using OnePasswordItem", + customResource: &onepasswordv1.OnePasswordItem{ + TypeMeta: metav1.TypeMeta{ + Kind: onePasswordItemKind, + APIVersion: onePasswordItemAPIVersion, + }, + 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, + }, + Type: string(corev1.SecretTypeBasicAuth), + }, + existingSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, + }, + Labels: map[string]string{}, + }, + Type: corev1.SecretTypeOpaque, + Data: expectedSecretData, + }, + expectedError: nil, + expectedResultSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + op.VersionAnnotation: fmt.Sprint(version), + op.ItemPathAnnotation: itemPath, + }, + Labels: map[string]string{}, + }, + Type: corev1.SecretTypeBasicAuth, Data: expectedSecretData, }, opItem: map[string]string{ @@ -206,6 +260,7 @@ var tests = []testReconcileItem{ Spec: onepasswordv1.OnePasswordItemSpec{ ItemPath: itemPath, }, + Type: "custom", }, existingSecret: nil, expectedError: nil, @@ -217,6 +272,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, + Type: corev1.SecretType("custom"), Data: expectedSecretData, }, opItem: map[string]string{ @@ -249,6 +305,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, + Type: corev1.SecretTypeOpaque, Data: expectedSecretData, }, opItem: map[string]string{ @@ -281,6 +338,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, + Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ "password": []byte(password), "username": []byte(username), @@ -322,6 +380,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, + Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ "password": []byte(password), "username": []byte(username), From 285496dc7e869385b336c9974452e9ac6865eda7 Mon Sep 17 00:00:00 2001 From: Marton Soos Date: Fri, 18 Feb 2022 10:27:48 +0100 Subject: [PATCH 5/6] Error when secret type is changed --- .../kubernetes_secrets_builder.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 4548ceb..5845118 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -16,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" kubeValidate "k8s.io/apimachinery/pkg/util/validation" + errs "errors" kubernetesClient "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -51,10 +52,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa secretAnnotations[RestartDeploymentsAnnotation] = autoRestart } - // Default to Opaque secrets - if secretType == "" { - secretType = "Opaque" - } + // "Opaque" and "" secret types are treated the same by Kubernetes. secret := BuildKubernetesSecretFromOnePasswordItem(secretName, namespace, secretAnnotations, labels, secretType, *item) currentSecret := &corev1.Secret{} @@ -69,15 +67,15 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa currentAnnotations := currentSecret.Annotations currentLabels := currentSecret.Labels currentSecretType := string(currentSecret.Type) - if currentSecretType == "" { - currentSecretType = "Opaque" + if !reflect.DeepEqual(currentSecretType, secretType) { + return errs.New("Cannot change secret type. Secret type is immutable") } - if !reflect.DeepEqual(currentAnnotations, secretAnnotations) || !reflect.DeepEqual(currentLabels, labels) || !reflect.DeepEqual(currentSecretType, secretType) { + + if !reflect.DeepEqual(currentAnnotations, secretAnnotations) || !reflect.DeepEqual(currentLabels, 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 - currentSecret.Type = corev1.SecretType(secretType) return kubeClient.Update(context.Background(), currentSecret) } From b16960057a082e89e4113c9a5fc8bf3ad707d202 Mon Sep 17 00:00:00 2001 From: Marton Soos Date: Fri, 18 Feb 2022 10:47:14 +0100 Subject: [PATCH 6/6] Update tests and add new test --- .../deployment/deployment_controller_test.go | 6 +-- .../onepassworditem/onepassworditem_test.go | 51 +++++++++++++++++-- .../kubernetes_secrets_builder.go | 7 ++- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index ee6ac97..bd98231 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -329,7 +329,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: "456", }, }, - Type: corev1.SecretTypeOpaque, + Type: corev1.SecretType(""), Data: expectedSecretData, }, expectedError: nil, @@ -341,7 +341,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, - Type: corev1.SecretTypeOpaque, + Type: corev1.SecretType(""), Data: expectedSecretData, }, opItem: map[string]string{ @@ -375,7 +375,7 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, - Type: corev1.SecretTypeOpaque, + Type: corev1.SecretType(""), Data: expectedSecretData, }, opItem: map[string]string{ diff --git a/pkg/controller/onepassworditem/onepassworditem_test.go b/pkg/controller/onepassworditem/onepassworditem_test.go index f6bf8e2..9add483 100644 --- a/pkg/controller/onepassworditem/onepassworditem_test.go +++ b/pkg/controller/onepassworditem/onepassworditem_test.go @@ -5,6 +5,7 @@ import ( "fmt" "testing" + "github.com/1Password/onepassword-operator/pkg/kubernetessecrets" "github.com/1Password/onepassword-operator/pkg/mocks" op "github.com/1Password/onepassword-operator/pkg/onepassword" @@ -185,7 +186,6 @@ var tests = []testReconcileItem{ }, Labels: map[string]string{}, }, - Type: corev1.SecretTypeOpaque, Data: expectedSecretData, }, opItem: map[string]string{ @@ -224,7 +224,7 @@ var tests = []testReconcileItem{ }, Labels: map[string]string{}, }, - Type: corev1.SecretTypeOpaque, + Type: corev1.SecretTypeBasicAuth, Data: expectedSecretData, }, expectedError: nil, @@ -280,6 +280,50 @@ var tests = []testReconcileItem{ passKey: password, }, }, + { + testName: "Error if secret type is changed", + customResource: &onepasswordv1.OnePasswordItem{ + TypeMeta: metav1.TypeMeta{ + Kind: onePasswordItemKind, + APIVersion: onePasswordItemAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: onepasswordv1.OnePasswordItemSpec{ + ItemPath: itemPath, + }, + Type: "custom", + }, + existingSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + op.VersionAnnotation: fmt.Sprint(version), + }, + }, + Type: corev1.SecretTypeOpaque, + Data: expectedSecretData, + }, + expectedError: kubernetessecrets.ErrCannotUpdateSecretType, + expectedResultSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + op.VersionAnnotation: fmt.Sprint(version), + }, + }, + Type: corev1.SecretTypeOpaque, + Data: expectedSecretData, + }, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + }, { testName: "Secret from 1Password item with invalid K8s labels", customResource: &onepasswordv1.OnePasswordItem{ @@ -305,7 +349,6 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, - Type: corev1.SecretTypeOpaque, Data: expectedSecretData, }, opItem: map[string]string{ @@ -338,7 +381,6 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, - Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ "password": []byte(password), "username": []byte(username), @@ -380,7 +422,6 @@ var tests = []testReconcileItem{ op.VersionAnnotation: fmt.Sprint(version), }, }, - Type: corev1.SecretTypeOpaque, Data: map[string][]byte{ "password": []byte(password), "username": []byte(username), diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 5845118..d3fa890 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -9,6 +9,8 @@ import ( "reflect" + errs "errors" + "github.com/1Password/connect-sdk-go/onepassword" "github.com/1Password/onepassword-operator/pkg/utils" corev1 "k8s.io/api/core/v1" @@ -16,7 +18,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" kubeValidate "k8s.io/apimachinery/pkg/util/validation" - errs "errors" kubernetesClient "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -29,6 +30,8 @@ const restartAnnotation = OnepasswordPrefix + "/last-restarted" const ItemPathAnnotation = OnepasswordPrefix + "/item-path" const RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto-restart" +var ErrCannotUpdateSecretType = errs.New("Cannot change secret type. Secret type is immutable") + var log = logf.Log func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart string, labels map[string]string, secretType string, secretAnnotations map[string]string) error { @@ -68,7 +71,7 @@ func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretNa currentLabels := currentSecret.Labels currentSecretType := string(currentSecret.Type) if !reflect.DeepEqual(currentSecretType, secretType) { - return errs.New("Cannot change secret type. Secret type is immutable") + return ErrCannotUpdateSecretType } if !reflect.DeepEqual(currentAnnotations, secretAnnotations) || !reflect.DeepEqual(currentLabels, labels) {