From 0824aa0837d5af29dad4c437c646c59720ca408f Mon Sep 17 00:00:00 2001 From: jillianwilson Date: Fri, 26 Feb 2021 10:45:30 -0400 Subject: [PATCH 1/3] Refactoring map of updated secrets to include secret --- ...onepassword.com_v1_onepassworditem_cr.yaml | 1 + .../deployment/deployment_controller.go | 4 ++-- pkg/onepassword/annotations.go | 3 ++- pkg/onepassword/containers.go | 2 +- pkg/onepassword/containers_test.go | 14 ++++++++------ pkg/onepassword/deployments.go | 7 +++++-- pkg/onepassword/deployments_test.go | 19 ++++++++++--------- pkg/onepassword/secret_update_handler.go | 12 ++++++------ pkg/onepassword/secret_update_handler_test.go | 6 +++--- pkg/onepassword/volumes.go | 2 +- pkg/onepassword/volumes_test.go | 14 ++++++++------ 11 files changed, 47 insertions(+), 37 deletions(-) diff --git a/deploy/crds/onepassword.com_v1_onepassworditem_cr.yaml b/deploy/crds/onepassword.com_v1_onepassworditem_cr.yaml index 8afe8fc..d0a94dd 100644 --- a/deploy/crds/onepassword.com_v1_onepassworditem_cr.yaml +++ b/deploy/crds/onepassword.com_v1_onepassworditem_cr.yaml @@ -2,5 +2,6 @@ apiVersion: onepassword.com/v1 kind: OnePasswordItem metadata: name: example + onepasswordoperator/auto_restart: "true" spec: itemPath: "vaults//items/" diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 16f7cdc..d1fc744 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -142,7 +142,7 @@ func (r *ReconcileDeployment) cleanupKubernetesSecretForDeployment(secretName st if len(secretName) == 0 { return nil } - updatedSecrets := map[string]bool{secretName: true} + updatedSecrets := map[string]*corev1.Secret{secretName: kubernetesSecret} multipleDeploymentsUsingSecret, err := r.areMultipleDeploymentsUsingSecret(updatedSecrets, *deletedDeployment) if err != nil { @@ -160,7 +160,7 @@ func (r *ReconcileDeployment) cleanupKubernetesSecretForDeployment(secretName st return nil } -func (r *ReconcileDeployment) areMultipleDeploymentsUsingSecret(updatedSecrets map[string]bool, deletedDeployment appsv1.Deployment) (bool, error) { +func (r *ReconcileDeployment) areMultipleDeploymentsUsingSecret(updatedSecrets map[string]*corev1.Secret, deletedDeployment appsv1.Deployment) (bool, error) { deployments := &appsv1.DeploymentList{} opts := []client.ListOption{ client.InNamespace(deletedDeployment.Namespace), diff --git a/pkg/onepassword/annotations.go b/pkg/onepassword/annotations.go index b7b5227..7c60d8e 100644 --- a/pkg/onepassword/annotations.go +++ b/pkg/onepassword/annotations.go @@ -4,6 +4,7 @@ import ( "regexp" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" ) const ( @@ -42,7 +43,7 @@ func FilterAnnotations(annotations map[string]string, regex *regexp.Regexp) map[ return filteredAnnotations } -func AreAnnotationsUsingSecrets(annotations map[string]string, secrets map[string]bool) bool { +func AreAnnotationsUsingSecrets(annotations map[string]string, secrets map[string]*corev1.Secret) bool { _, ok := secrets[annotations[NameAnnotation]] if ok { return true diff --git a/pkg/onepassword/containers.go b/pkg/onepassword/containers.go index 759c905..2dfed26 100644 --- a/pkg/onepassword/containers.go +++ b/pkg/onepassword/containers.go @@ -2,7 +2,7 @@ package onepassword import corev1 "k8s.io/api/core/v1" -func AreContainersUsingSecrets(containers []corev1.Container, secrets map[string]bool) bool { +func AreContainersUsingSecrets(containers []corev1.Container, secrets map[string]*corev1.Secret) bool { for i := 0; i < len(containers); i++ { envVariables := containers[i].Env for j := 0; j < len(envVariables); j++ { diff --git a/pkg/onepassword/containers_test.go b/pkg/onepassword/containers_test.go index 7836edb..676c517 100644 --- a/pkg/onepassword/containers_test.go +++ b/pkg/onepassword/containers_test.go @@ -2,12 +2,14 @@ package onepassword import ( "testing" + + corev1 "k8s.io/api/core/v1" ) func TestAreContainersUsingSecrets(t *testing.T) { - secretNamesToSearch := map[string]bool{ - "onepassword-database-secret": true, - "onepassword-api-key": true, + secretNamesToSearch := map[string]*corev1.Secret{ + "onepassword-database-secret": &corev1.Secret{}, + "onepassword-api-key": &corev1.Secret{}, } containerSecretNames := []string{ @@ -24,9 +26,9 @@ func TestAreContainersUsingSecrets(t *testing.T) { } func TestAreContainersNotUsingSecrets(t *testing.T) { - secretNamesToSearch := map[string]bool{ - "onepassword-database-secret": true, - "onepassword-api-key": true, + secretNamesToSearch := map[string]*corev1.Secret{ + "onepassword-database-secret": &corev1.Secret{}, + "onepassword-api-key": &corev1.Secret{}, } containerSecretNames := []string{ diff --git a/pkg/onepassword/deployments.go b/pkg/onepassword/deployments.go index f1c1373..e5d5181 100644 --- a/pkg/onepassword/deployments.go +++ b/pkg/onepassword/deployments.go @@ -1,8 +1,11 @@ package onepassword -import appsv1 "k8s.io/api/apps/v1" +import ( + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" +) -func IsDeploymentUsingSecrets(deployment *appsv1.Deployment, secrets map[string]bool) bool { +func IsDeploymentUsingSecrets(deployment *appsv1.Deployment, secrets map[string]*corev1.Secret) bool { volumes := deployment.Spec.Template.Spec.Volumes containers := deployment.Spec.Template.Spec.Containers containers = append(containers, deployment.Spec.Template.Spec.InitContainers...) diff --git a/pkg/onepassword/deployments_test.go b/pkg/onepassword/deployments_test.go index b3b62f8..d7445b1 100644 --- a/pkg/onepassword/deployments_test.go +++ b/pkg/onepassword/deployments_test.go @@ -4,12 +4,13 @@ import ( "testing" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" ) func TestIsDeploymentUsingSecretsUsingVolumes(t *testing.T) { - secretNamesToSearch := map[string]bool{ - "onepassword-database-secret": true, - "onepassword-api-key": true, + secretNamesToSearch := map[string]*corev1.Secret{ + "onepassword-database-secret": &corev1.Secret{}, + "onepassword-api-key": &corev1.Secret{}, } volumeSecretNames := []string{ @@ -26,9 +27,9 @@ func TestIsDeploymentUsingSecretsUsingVolumes(t *testing.T) { } func TestIsDeploymentUsingSecretsUsingContainers(t *testing.T) { - secretNamesToSearch := map[string]bool{ - "onepassword-database-secret": true, - "onepassword-api-key": true, + secretNamesToSearch := map[string]*corev1.Secret{ + "onepassword-database-secret": &corev1.Secret{}, + "onepassword-api-key": &corev1.Secret{}, } containerSecretNames := []string{ @@ -45,9 +46,9 @@ func TestIsDeploymentUsingSecretsUsingContainers(t *testing.T) { } func TestIsDeploymentNotUSingSecrets(t *testing.T) { - secretNamesToSearch := map[string]bool{ - "onepassword-database-secret": true, - "onepassword-api-key": true, + secretNamesToSearch := map[string]*corev1.Secret{ + "onepassword-database-secret": &corev1.Secret{}, + "onepassword-api-key": &corev1.Secret{}, } deployment := &appsv1.Deployment{} diff --git a/pkg/onepassword/secret_update_handler.go b/pkg/onepassword/secret_update_handler.go index b7ae112..bf2b5f8 100644 --- a/pkg/onepassword/secret_update_handler.go +++ b/pkg/onepassword/secret_update_handler.go @@ -45,7 +45,7 @@ func (h *SecretUpdateHandler) UpdateKubernetesSecretsTask() error { return h.restartDeploymentsWithUpdatedSecrets(updatedKubernetesSecrets) } -func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecretsByNamespace map[string]map[string]bool) error { +func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecretsByNamespace map[string]map[string]*corev1.Secret) error { // No secrets to update. Exit if len(updatedSecretsByNamespace) == 0 || updatedSecretsByNamespace == nil { return nil @@ -94,7 +94,7 @@ func (h *SecretUpdateHandler) restartDeployment(deployment *appsv1.Deployment) { } } -func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]bool, error) { +func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]*corev1.Secret, error) { secrets := &corev1.SecretList{} err := h.client.List(context.Background(), secrets) if err != nil { @@ -102,7 +102,7 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]b return nil, err } - updatedSecrets := map[string]map[string]bool{} + updatedSecrets := map[string]map[string]*corev1.Secret{} for i := 0; i < len(secrets.Items); i++ { secret := secrets.Items[i] @@ -130,9 +130,9 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]b updatedSecret := kubeSecrets.BuildKubernetesSecretFromOnePasswordItem(secret.Name, secret.Namespace, secret.Annotations, *item) h.client.Update(context.Background(), updatedSecret) if updatedSecrets[secret.Namespace] == nil { - updatedSecrets[secret.Namespace] = make(map[string]bool) + updatedSecrets[secret.Namespace] = make(map[string]*corev1.Secret) } - updatedSecrets[secret.Namespace][secret.Name] = true + updatedSecrets[secret.Namespace][secret.Name] = &secret } } return updatedSecrets, nil @@ -148,7 +148,7 @@ func isItemLockedForForcedRestarts(item *onepassword.Item) bool { return false } -func isUpdatedSecret(secretName string, updatedSecrets map[string]bool) bool { +func isUpdatedSecret(secretName string, updatedSecrets map[string]*corev1.Secret) bool { _, ok := updatedSecrets[secretName] if ok { return true diff --git a/pkg/onepassword/secret_update_handler_test.go b/pkg/onepassword/secret_update_handler_test.go index 6df5b0d..d2b9934 100644 --- a/pkg/onepassword/secret_update_handler_test.go +++ b/pkg/onepassword/secret_update_handler_test.go @@ -694,12 +694,12 @@ func TestUpdateSecretHandler(t *testing.T) { func TestIsUpdatedSecret(t *testing.T) { secretName := "test-secret" - updatedSecrets := map[string]bool{ - "some_secret": true, + updatedSecrets := map[string]*corev1.Secret{ + "some_secret": &corev1.Secret{}, } assert.False(t, isUpdatedSecret(secretName, updatedSecrets)) - updatedSecrets[secretName] = true + updatedSecrets[secretName] = &corev1.Secret{} assert.True(t, isUpdatedSecret(secretName, updatedSecrets)) } diff --git a/pkg/onepassword/volumes.go b/pkg/onepassword/volumes.go index a75b268..038c15c 100644 --- a/pkg/onepassword/volumes.go +++ b/pkg/onepassword/volumes.go @@ -2,7 +2,7 @@ package onepassword import corev1 "k8s.io/api/core/v1" -func AreVolumesUsingSecrets(volumes []corev1.Volume, secrets map[string]bool) bool { +func AreVolumesUsingSecrets(volumes []corev1.Volume, secrets map[string]*corev1.Secret) bool { for i := 0; i < len(volumes); i++ { if secret := volumes[i].Secret; secret != nil { secretName := secret.SecretName diff --git a/pkg/onepassword/volumes_test.go b/pkg/onepassword/volumes_test.go index d3cce58..00d109c 100644 --- a/pkg/onepassword/volumes_test.go +++ b/pkg/onepassword/volumes_test.go @@ -2,12 +2,14 @@ package onepassword import ( "testing" + + corev1 "k8s.io/api/core/v1" ) func TestAreVolmesUsingSecrets(t *testing.T) { - secretNamesToSearch := map[string]bool{ - "onepassword-database-secret": true, - "onepassword-api-key": true, + secretNamesToSearch := map[string]*corev1.Secret{ + "onepassword-database-secret": &corev1.Secret{}, + "onepassword-api-key": &corev1.Secret{}, } volumeSecretNames := []string{ @@ -24,9 +26,9 @@ func TestAreVolmesUsingSecrets(t *testing.T) { } func TestAreVolumesNotUsingSecrets(t *testing.T) { - secretNamesToSearch := map[string]bool{ - "onepassword-database-secret": true, - "onepassword-api-key": true, + secretNamesToSearch := map[string]*corev1.Secret{ + "onepassword-database-secret": &corev1.Secret{}, + "onepassword-api-key": &corev1.Secret{}, } volumeSecretNames := []string{ From 8635be0cabafa80f93a405ba1c95192ba2471adc Mon Sep 17 00:00:00 2001 From: jillianwilson Date: Mon, 1 Mar 2021 15:58:32 -0400 Subject: [PATCH 2/3] Handle restart annotation on kubernetes secret --- pkg/onepassword/annotations.go | 8 + pkg/onepassword/containers.go | 15 ++ pkg/onepassword/deployments.go | 13 ++ pkg/onepassword/secret_update_handler.go | 34 +++- pkg/onepassword/secret_update_handler_test.go | 146 +++++++++++++++++- pkg/onepassword/volumes.go | 13 ++ 6 files changed, 220 insertions(+), 9 deletions(-) diff --git a/pkg/onepassword/annotations.go b/pkg/onepassword/annotations.go index 7c60d8e..be00f46 100644 --- a/pkg/onepassword/annotations.go +++ b/pkg/onepassword/annotations.go @@ -50,3 +50,11 @@ func AreAnnotationsUsingSecrets(annotations map[string]string, secrets map[strin } return false } + +func AppendAnnotationUpdatedSecret(annotations map[string]string, secrets map[string]*corev1.Secret, updatedDeploymentSecrets map[string]*corev1.Secret) map[string]*corev1.Secret { + secret, ok := secrets[annotations[NameAnnotation]] + if ok { + updatedDeploymentSecrets[secret.Name] = secret + } + return updatedDeploymentSecrets +} diff --git a/pkg/onepassword/containers.go b/pkg/onepassword/containers.go index 2dfed26..1f51bd9 100644 --- a/pkg/onepassword/containers.go +++ b/pkg/onepassword/containers.go @@ -16,3 +16,18 @@ func AreContainersUsingSecrets(containers []corev1.Container, secrets map[string } return false } + +func AppendUpdatedContainerSecrets(containers []corev1.Container, secrets map[string]*corev1.Secret, updatedDeploymentSecrets map[string]*corev1.Secret) map[string]*corev1.Secret { + for i := 0; i < len(containers); i++ { + envVariables := containers[i].Env + for j := 0; j < len(envVariables); j++ { + if envVariables[j].ValueFrom != nil && envVariables[j].ValueFrom.SecretKeyRef != nil { + secret, ok := secrets[envVariables[j].ValueFrom.SecretKeyRef.Name] + if ok { + updatedDeploymentSecrets[secret.Name] = secret + } + } + } + } + return updatedDeploymentSecrets +} diff --git a/pkg/onepassword/deployments.go b/pkg/onepassword/deployments.go index e5d5181..6c97efb 100644 --- a/pkg/onepassword/deployments.go +++ b/pkg/onepassword/deployments.go @@ -11,3 +11,16 @@ func IsDeploymentUsingSecrets(deployment *appsv1.Deployment, secrets map[string] containers = append(containers, deployment.Spec.Template.Spec.InitContainers...) return AreAnnotationsUsingSecrets(deployment.Annotations, secrets) || AreContainersUsingSecrets(containers, secrets) || AreVolumesUsingSecrets(volumes, secrets) } + +func GetUpdatedSecretsForDeployment(deployment *appsv1.Deployment, secrets map[string]*corev1.Secret) map[string]*corev1.Secret { + volumes := deployment.Spec.Template.Spec.Volumes + containers := deployment.Spec.Template.Spec.Containers + containers = append(containers, deployment.Spec.Template.Spec.InitContainers...) + + updatedSecretsForDeployment := map[string]*corev1.Secret{} + AppendAnnotationUpdatedSecret(deployment.Annotations, secrets, updatedSecretsForDeployment) + AppendUpdatedContainerSecrets(containers, secrets, updatedSecretsForDeployment) + AppendUpdatedVolumeSecrets(volumes, secrets, updatedSecretsForDeployment) + + return updatedSecretsForDeployment +} diff --git a/pkg/onepassword/secret_update_handler.go b/pkg/onepassword/secret_update_handler.go index bf2b5f8..c898dda 100644 --- a/pkg/onepassword/secret_update_handler.go +++ b/pkg/onepassword/secret_update_handler.go @@ -69,16 +69,21 @@ func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecret for i := 0; i < len(deployments.Items); i++ { deployment := &deployments.Items[i] - if !isDeploymentSetForAutoRestart(deployment, setForAutoRestartByNamespaceMap) { + updatedSecrets := updatedSecretsByNamespace[deployment.Namespace] + + updatedDeploymentSecrets := GetUpdatedSecretsForDeployment(deployment, updatedSecrets) + if len(updatedDeploymentSecrets) == 0 { continue } - updatedSecrets := updatedSecretsByNamespace[deployment.Namespace] - secretName := deployment.Annotations[NameAnnotation] - if isUpdatedSecret(secretName, updatedSecrets) || IsDeploymentUsingSecrets(deployment, updatedSecrets) { - h.restartDeployment(deployment) - } else { - log.Info(fmt.Sprintf("Deployment %q at namespace %q is up to date", deployment.GetName(), deployment.Namespace)) + for _, secret := range updatedDeploymentSecrets { + if isSecretSetForAutoRestart(secret, deployment, setForAutoRestartByNamespaceMap) { + h.restartDeployment(deployment) + continue + } } + + log.Info(fmt.Sprintf("Deployment %q at namespace %q is up to date", deployment.GetName(), deployment.Namespace)) + } return nil } @@ -172,6 +177,21 @@ func (h *SecretUpdateHandler) getIsSetForAutoRestartByNamespaceMap() (map[string return namespacesMap, nil } +func isSecretSetForAutoRestart(secret *corev1.Secret, deployment *appsv1.Deployment, setForAutoRestartByNamespace map[string]bool) bool { + restartDeployment := secret.Annotations[RestartDeploymentsAnnotation] + //If annotation for auto restarts for deployment is not set. Check for the annotation on its namepsace + if restartDeployment == "" { + return isDeploymentSetForAutoRestart(deployment, setForAutoRestartByNamespace) + } + + restartDeploymentBool, err := stringToBool(restartDeployment) + if err != nil { + log.Error(err, "Error parsing %v annotation on Deployment %v. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, deployment.Name) + return false + } + return restartDeploymentBool +} + func isDeploymentSetForAutoRestart(deployment *appsv1.Deployment, setForAutoRestartByNamespace map[string]bool) bool { restartDeployment := deployment.Annotations[RestartDeploymentsAnnotation] //If annotation for auto restarts for deployment is not set. Check for the annotation on its namepsace diff --git a/pkg/onepassword/secret_update_handler_test.go b/pkg/onepassword/secret_update_handler_test.go index d2b9934..269a179 100644 --- a/pkg/onepassword/secret_update_handler_test.go +++ b/pkg/onepassword/secret_update_handler_test.go @@ -394,6 +394,148 @@ var tests = []testUpdateSecretTask{ expectedRestart: false, globalAutoRestartEnabled: false, }, + { + testName: `Secret autostart true value takes precedence over false deployment value`, + existingNamespace: defaultNamespace, + existingDeployment: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: deploymentKind, + APIVersion: deploymentAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + RestartDeploymentsAnnotation: "false", + }, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: name, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: name, + }, + Key: passKey, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + existingSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + VersionAnnotation: "old version", + ItemPathAnnotation: itemPath, + RestartDeploymentsAnnotation: "true", + }, + }, + Data: expectedSecretData, + }, + expectedError: nil, + expectedResultSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + VersionAnnotation: fmt.Sprint(itemVersion), + ItemPathAnnotation: itemPath, + RestartDeploymentsAnnotation: "true", + }, + }, + Data: expectedSecretData, + }, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedRestart: true, + globalAutoRestartEnabled: false, + }, + { + testName: `Secret autostart true value takes precedence over false deployment value`, + existingNamespace: defaultNamespace, + existingDeployment: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: deploymentKind, + APIVersion: deploymentAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + RestartDeploymentsAnnotation: "true", + }, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: name, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: name, + }, + Key: passKey, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + existingSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + VersionAnnotation: "old version", + ItemPathAnnotation: itemPath, + RestartDeploymentsAnnotation: "false", + }, + }, + Data: expectedSecretData, + }, + expectedError: nil, + expectedResultSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + VersionAnnotation: fmt.Sprint(itemVersion), + ItemPathAnnotation: itemPath, + RestartDeploymentsAnnotation: "false", + }, + }, + Data: expectedSecretData, + }, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedRestart: false, + globalAutoRestartEnabled: true, + }, { testName: `Deployment autostart true value takes precedence over false global auto restart value`, existingNamespace: defaultNamespace, @@ -461,7 +603,7 @@ var tests = []testUpdateSecretTask{ passKey: password, }, expectedRestart: true, - globalAutoRestartEnabled: true, + globalAutoRestartEnabled: false, }, { testName: `Deployment autostart false value takes precedence over false global auto restart value, @@ -685,7 +827,7 @@ func TestUpdateSecretHandler(t *testing.T) { if ok { assert.True(t, testData.expectedRestart, "Expected deployment to restart but it did not") } else { - assert.False(t, testData.expectedRestart) + assert.False(t, testData.expectedRestart, "Deployment was restarted but should not have been.") } }) } diff --git a/pkg/onepassword/volumes.go b/pkg/onepassword/volumes.go index 038c15c..adf2e8b 100644 --- a/pkg/onepassword/volumes.go +++ b/pkg/onepassword/volumes.go @@ -14,3 +14,16 @@ func AreVolumesUsingSecrets(volumes []corev1.Volume, secrets map[string]*corev1. } return false } + +func AppendUpdatedVolumeSecrets(volumes []corev1.Volume, secrets map[string]*corev1.Secret, updatedDeploymentSecrets map[string]*corev1.Secret) map[string]*corev1.Secret { + for i := 0; i < len(volumes); i++ { + if secret := volumes[i].Secret; secret != nil { + secretName := secret.SecretName + secret, ok := secrets[secretName] + if ok { + updatedDeploymentSecrets[secret.Name] = secret + } + } + } + return updatedDeploymentSecrets +} From d98f9172a0a84fbb09bdff022a9712cd91ccf849 Mon Sep 17 00:00:00 2001 From: jillianwilson Date: Tue, 2 Mar 2021 16:59:21 -0400 Subject: [PATCH 3/3] Auto restart one password custom resource will be be added to converted kubernetes secret --- README.md | 12 ++++++++++ ...onepassword.com_v1_onepassworditem_cr.yaml | 1 - .../deployment/deployment_controller.go | 2 +- .../onepassworditem_controller.go | 4 +++- .../kubernetes_secrets_builder.go | 22 ++++++++++++++----- .../kubernetes_secrets_builder_test.go | 12 +++++++--- pkg/onepassword/secret_update_handler.go | 19 +++++----------- pkg/utils/string.go | 13 +++++++++++ 8 files changed, 59 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 6d7f0b0..2710620 100644 --- a/README.md +++ b/README.md @@ -176,6 +176,18 @@ metadata: ``` If the value is not set, the auto reset settings on the namespace will be used. +**Per OnePasswordItem Custom Resource** +This method allows for managing auto restarts on a given OnePasswordItem custom resource. Auto restarts can by managed by setting the annotation `onepasswordoperator/auto_restart` to either `true` or `false` on the desired OnePasswordItem. An example of this is shown below: +```yaml +# enabled auto restarts for the OnePasswordItem +apiVersion: onepassword.com/v1 +kind: OnePasswordItem +metadata: + name: example + onepasswordoperator/auto_restart: "true" +``` +If the value is not set, the auto reset settings on the deployment will be used. + ## Development ### Creating a Docker image diff --git a/deploy/crds/onepassword.com_v1_onepassworditem_cr.yaml b/deploy/crds/onepassword.com_v1_onepassworditem_cr.yaml index d0a94dd..8afe8fc 100644 --- a/deploy/crds/onepassword.com_v1_onepassworditem_cr.yaml +++ b/deploy/crds/onepassword.com_v1_onepassworditem_cr.yaml @@ -2,6 +2,5 @@ apiVersion: onepassword.com/v1 kind: OnePasswordItem metadata: name: example - onepasswordoperator/auto_restart: "true" spec: itemPath: "vaults//items/" diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index d1fc744..500f60d 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -201,5 +201,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) + return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, namespace, item, annotations[op.RestartDeploymentsAnnotation]) } diff --git a/pkg/controller/onepassworditem/onepassworditem_controller.go b/pkg/controller/onepassworditem/onepassworditem_controller.go index cfc8ab7..3c808bd 100644 --- a/pkg/controller/onepassworditem/onepassworditem_controller.go +++ b/pkg/controller/onepassworditem/onepassworditem_controller.go @@ -7,6 +7,7 @@ import ( 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" + op "github.com/1Password/onepassword-operator/pkg/onepassword" "github.com/1Password/onepassword-operator/pkg/utils" "github.com/1Password/connect-sdk-go/connect" @@ -143,11 +144,12 @@ func (r *ReconcileOnePasswordItem) removeOnePasswordFinalizerFromOnePasswordItem func (r *ReconcileOnePasswordItem) HandleOnePasswordItem(resource *onepasswordv1.OnePasswordItem, request reconcile.Request) error { secretName := resource.GetName() + autoRestart := resource.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) + return kubeSecrets.CreateKubernetesSecretFromItem(r.kubeClient, secretName, resource.Namespace, item, autoRestart) } diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder.go b/pkg/kubernetessecrets/kubernetes_secrets_builder.go index 5f250b1..ebcf221 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder.go @@ -5,6 +5,7 @@ import ( "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" @@ -13,21 +14,30 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" ) -const onepasswordPrefix = "onepasswordoperator" -const NameAnnotation = onepasswordPrefix + "/item-name" -const VersionAnnotation = onepasswordPrefix + "/item-version" -const restartAnnotation = onepasswordPrefix + "/lastRestarted" -const ItemPathAnnotation = onepasswordPrefix + "/item-path" +const OnepasswordPrefix = "onepasswordoperator" +const NameAnnotation = OnepasswordPrefix + "/item-name" +const VersionAnnotation = OnepasswordPrefix + "/item-version" +const restartAnnotation = OnepasswordPrefix + "/lastRestarted" +const ItemPathAnnotation = OnepasswordPrefix + "/item-path" +const RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto_restart" var log = logf.Log -func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item) error { +func CreateKubernetesSecretFromItem(kubeClient kubernetesClient.Client, secretName, namespace string, item *onepassword.Item, autoRestart 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 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 + } secret := BuildKubernetesSecretFromOnePasswordItem(secretName, namespace, annotations, *item) currentSecret := &corev1.Secret{} diff --git a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go index 8a706ec..cb331c6 100644 --- a/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go +++ b/pkg/kubernetessecrets/kubernetes_secrets_builder_test.go @@ -13,6 +13,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +const restartDeploymentAnnotation = "false" + type k8s struct { clientset kubernetes.Interface } @@ -28,7 +30,7 @@ func TestCreateKubernetesSecretFromOnePasswordItem(t *testing.T) { item.ID = "h46bb3jddvay7nxopfhvlwg35q" kubeClient := fake.NewFakeClient() - err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item) + err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -53,7 +55,7 @@ func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) { item.ID = "h46bb3jddvay7nxopfhvlwg35q" kubeClient := fake.NewFakeClient() - err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item) + err := CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &item, restartDeploymentAnnotation) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -64,7 +66,7 @@ func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) { newItem.Version = 456 newItem.Vault.ID = "hfnjvi6aymbsnfc2xeeoheizda" newItem.ID = "h46bb3jddvay7nxopfhvlwg35q" - err = CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &newItem) + err = CreateKubernetesSecretFromItem(kubeClient, secretName, namespace, &newItem, restartDeploymentAnnotation) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -125,6 +127,10 @@ func compareAnnotationsToItem(annotations map[string]string, item onepassword.It if annotations[VersionAnnotation] != fmt.Sprint(item.Version) { t.Errorf("Expected annotation version to be %v but was %v", item.Version, annotations[VersionAnnotation]) } + + if annotations[RestartDeploymentsAnnotation] != "false" { + t.Errorf("Expected restart deployments annotation to be %v but was %v", restartDeploymentAnnotation, RestartDeploymentsAnnotation) + } } func compareFields(actualFields []*onepassword.ItemField, secretData map[string][]byte, t *testing.T) { diff --git a/pkg/onepassword/secret_update_handler.go b/pkg/onepassword/secret_update_handler.go index c898dda..7b38a7c 100644 --- a/pkg/onepassword/secret_update_handler.go +++ b/pkg/onepassword/secret_update_handler.go @@ -3,11 +3,10 @@ package onepassword import ( "context" "fmt" - "strconv" - "strings" "time" kubeSecrets "github.com/1Password/onepassword-operator/pkg/kubernetessecrets" + "github.com/1Password/onepassword-operator/pkg/utils" "github.com/1Password/connect-sdk-go/connect" "github.com/1Password/connect-sdk-go/onepassword" @@ -184,9 +183,9 @@ func isSecretSetForAutoRestart(secret *corev1.Secret, deployment *appsv1.Deploym return isDeploymentSetForAutoRestart(deployment, setForAutoRestartByNamespace) } - restartDeploymentBool, err := stringToBool(restartDeployment) + restartDeploymentBool, err := utils.StringToBool(restartDeployment) if err != nil { - log.Error(err, "Error parsing %v annotation on Deployment %v. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, deployment.Name) + log.Error(err, "Error parsing %v annotation on Secret %v. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, secret.Name) return false } return restartDeploymentBool @@ -199,7 +198,7 @@ func isDeploymentSetForAutoRestart(deployment *appsv1.Deployment, setForAutoRest return setForAutoRestartByNamespace[deployment.Namespace] } - restartDeploymentBool, err := stringToBool(restartDeployment) + restartDeploymentBool, err := utils.StringToBool(restartDeployment) if err != nil { log.Error(err, "Error parsing %v annotation on Deployment %v. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, deployment.Name) return false @@ -214,18 +213,10 @@ func (h *SecretUpdateHandler) isNamespaceSetToAutoRestart(namespace *corev1.Name return h.shouldAutoRestartDeploymentsGlobal } - restartDeploymentBool, err := stringToBool(restartDeployment) + restartDeploymentBool, err := utils.StringToBool(restartDeployment) if err != nil { log.Error(err, "Error parsing %v annotation on Namespace %v. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, namespace.Name) return false } return restartDeploymentBool } - -func stringToBool(str string) (bool, error) { - restartDeploymentBool, err := strconv.ParseBool(strings.ToLower(str)) - if err != nil { - return false, err - } - return restartDeploymentBool, nil -} diff --git a/pkg/utils/string.go b/pkg/utils/string.go index 09862e6..ffe2871 100644 --- a/pkg/utils/string.go +++ b/pkg/utils/string.go @@ -1,5 +1,10 @@ package utils +import ( + "strconv" + "strings" +) + func ContainsString(slice []string, s string) bool { for _, item := range slice { if item == s { @@ -18,3 +23,11 @@ func RemoveString(slice []string, s string) (result []string) { } return } + +func StringToBool(str string) (bool, error) { + restartDeploymentBool, err := strconv.ParseBool(strings.ToLower(str)) + if err != nil { + return false, err + } + return restartDeploymentBool, nil +}