From e2fc9e228edceaac3c6c4263a35b010d0b125ce1 Mon Sep 17 00:00:00 2001 From: jillianwilson Date: Wed, 13 Jan 2021 14:02:00 -0400 Subject: [PATCH] Adding configuration for auto rolling restart on deployments - Locked secrets will not trigger rolling restarts of deployments - Configure restart of deployments via operator environment variables, namespace annotations, or deployment annotations - Updating permissions examples to include the ability to list namespaces - Updated readme to reflect additional cofiguration options --- README.md | 38 +- cmd/manager/main.go | 16 +- deploy/operator.yaml | 2 + deploy/operator_multi_namespace_example.yaml | 8 +- deploy/permissions.yaml | 7 +- .../permissions_multi_namespace_example.yaml | 5 +- go.sum | 1 + pkg/onepassword/annotations.go | 13 +- pkg/onepassword/secret_update_handler.go | 103 +++++- pkg/onepassword/secret_update_handler_test.go | 350 ++++++++++++++++-- 10 files changed, 494 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 5355534..6d7f0b0 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ The 1Password Connect Kubernetes Operator provides the ability to integrate Kube The 1Password Connect Kubernetes Operator also allows for Kubernetes Secrets to be composed from a 1Password Item through annotation of an Item Path on a deployment. -The 1Password Connect Kubernetes Operator will continually check for updates from 1Password for any Kubernetes Secret that it has generated. If a Kubernetes Secret is updated, any Deployment using that secret will be automatically restarted. +The 1Password Connect Kubernetes Operator will continually check for updates from 1Password for any Kubernetes Secret that it has generated. If a Kubernetes Secret is updated, any Deployment using that secret can be automatically restarted. ## Setup @@ -85,6 +85,7 @@ To further configure the 1Password Kubernetes Operator the Following Environment - **OP_CONNECT_HOST** (required): Specifies the host name within Kubernetes in which to access the 1Password Connect. - **POLLING_INTERVAL** (default: 600)**:** The number of seconds the 1Password Kubernetes Operator will wait before checking for updates from 1Password Connect. - **MANAGE_CONNECT** (default: false): If set to true, on deployment of the operator, a default configuration of the OnePassword Connect Service will be deployed to the `default` namespace. +- **AUTO_RESTART** (default: false): If set to true, the operator will restart any deployment using a secret from 1Password Connect. This can be overwritten by namespace, deployment, or individual secret. More details on AUTO_RESTART can be found in the ["Configuring Automatic Rolling Restarts of Deployments"](#configuring-automatic-rolling-restarts-of-deployments) section. Apply the deployment file: @@ -135,8 +136,7 @@ Applying this yaml file will create a Kubernetes Secret with the name `{secret_n Note: Deleting the Deployment that you've created will automatically delete the created Kubernetes Secret only if the deployment is still annotated with `onepasswordoperator./item-path` and `onepasswordoperator/item-name` and no other deployment is using the secret. -If a 1Password Item that is linked to a Kubernetes Secret is updated within the `POLLING_INTERVAL` the associated Kubernetes Secret will be updated. Furthermore, any deployments using that secret will be given a rolling restart. - +If a 1Password Item that is linked to a Kubernetes Secret is updated within the POLLING_INTERVAL the associated Kubernetes Secret will be updated. However, if you do not want a specific secret to be updated you can add the tag `onepasswordconnectoperator:ignore_secret` to the item stored in 1Password. While this tag is in place, any updates made to an item will not trigger an update to the associated secret in Kubernetes. --- **NOTE** @@ -144,6 +144,38 @@ If a 1Password Item that is linked to a Kubernetes Secret is updated within the 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. --- + +### Configuring Automatic Rolling Restarts of Deployments + +If a 1Password Item that is linked to a Kubernetes Secret is updated, any deployments configured to `auto_restart` AND are using that secret will be given a rolling restart the next time 1Password Connect is polled for updates. + +There are many levels of granularity on which to configure auto restarts on deployments: at the operator level, per-namespace, or per-deployment. + +**On the operator**: This method allows for managing auto restarts on all deployments within the namespaces watched by operator. Auto restarts can be enabled by setting the environemnt variable `AUTO_RESTART` to true. If the value is not set, the operator will default this value to false. + +**Per Namespace**: This method allows for managing auto restarts on all deployments within a namespace. Auto restarts can by managed by setting the annotation `onepasswordoperator/auto_restart` to either `true` or `false` on the desired namespace. An example of this is shown below: +```yaml +# enabled auto restarts for all deployments within a namespace unless overwritten within a deployment +apiVersion: v1 +kind: Namespace +metadata: + name: "example-namespace" + onepasswordoperator/auto_restart: "true" +``` +If the value is not set, the auto reset settings on the operator will be used. This value can be overwritten by deployment. + +**Per Deployment** +This method allows for managing auto restarts on a given deployment. Auto restarts can by managed by setting the annotation `onepasswordoperator/auto_restart` to either `true` or `false` on the desired deployment. An example of this is shown below: +```yaml +# enabled auto restarts for the deployment +apiVersion: v1 +kind: Deployment +metadata: + name: "example-deployment" + onepasswordoperator/auto_restart: "true" +``` +If the value is not set, the auto reset settings on the namespace will be used. + ## Development ### Creating a Docker image diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 200b76d..66c57a9 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -42,6 +42,7 @@ import ( const envPollingIntervalVariable = "POLLING_INTERVAL" const manageConnect = "MANAGE_CONNECT" +const restartDeploymentsEnvVariable = "AUTO_RESTART" const defaultPollingInterval = 600 // Change below variables to serve metrics on different host or port. @@ -165,7 +166,7 @@ func main() { addMetrics(ctx, cfg) // Setup update secrets task - updatedSecretsPoller := op.NewManager(mgr.GetClient(), opConnectClient) + updatedSecretsPoller := op.NewManager(mgr.GetClient(), opConnectClient, shouldAutoRestartDeployments()) done := make(chan bool) ticker := time.NewTicker(getPollingIntervalForUpdatingSecrets()) go func() { @@ -284,3 +285,16 @@ func shouldManageConnect() bool { } return false } + +func shouldAutoRestartDeployments() bool { + shouldAutoRestartDeployments, found := os.LookupEnv(restartDeploymentsEnvVariable) + if found { + shouldAutoRestartDeploymentsBool, err := strconv.ParseBool(strings.ToLower(shouldAutoRestartDeployments)) + if err != nil { + log.Error(err, "") + os.Exit(1) + } + return shouldAutoRestartDeploymentsBool + } + return false +} diff --git a/deploy/operator.yaml b/deploy/operator.yaml index deb3d73..f00aaf4 100644 --- a/deploy/operator.yaml +++ b/deploy/operator.yaml @@ -35,3 +35,5 @@ spec: secretKeyRef: name: onepassword-token key: token + - name: AUTO_RESTART + value: "false" diff --git a/deploy/operator_multi_namespace_example.yaml b/deploy/operator_multi_namespace_example.yaml index af58fee..5428265 100644 --- a/deploy/operator_multi_namespace_example.yaml +++ b/deploy/operator_multi_namespace_example.yaml @@ -16,9 +16,7 @@ spec: containers: - name: onepassword-connect-operator image: 1password/onepassword-operator - command: - - onepassword-connect-operator - imagePullPolicy: Never + command: ["/manager"] env: - name: WATCH_NAMESPACE value: "default,development" @@ -29,7 +27,7 @@ spec: - name: OPERATOR_NAME value: "onepassword-connect-operator" - name: OP_CONNECT_HOST - value: "http://secret-service:8080" + value: "http://onepassword-connect:8080" - name: POLLING_INTERVAL value: "10" - name: OP_CONNECT_TOKEN @@ -37,3 +35,5 @@ spec: secretKeyRef: name: onepassword-token key: token + - name: AUTO_RESTART + value: "false" diff --git a/deploy/permissions.yaml b/deploy/permissions.yaml index 0f54bee..d90fbd0 100644 --- a/deploy/permissions.yaml +++ b/deploy/permissions.yaml @@ -3,7 +3,7 @@ kind: ServiceAccount metadata: name: onepassword-connect-operator --- -kind: RoleBinding +kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 metadata: name: onepassword-connect-operator-default @@ -13,12 +13,12 @@ subjects: name: onepassword-connect-operator namespace: default roleRef: - kind: Role + kind: ClusterRole name: onepassword-connect-operator apiGroup: rbac.authorization.k8s.io --- apiVersion: rbac.authorization.k8s.io/v1 -kind: Role +kind: ClusterRole metadata: creationTimestamp: null name: onepassword-connect-operator @@ -34,6 +34,7 @@ rules: - events - configmaps - secrets + - namespaces verbs: - create - delete diff --git a/deploy/permissions_multi_namespace_example.yaml b/deploy/permissions_multi_namespace_example.yaml index aaec98a..ae9e081 100644 --- a/deploy/permissions_multi_namespace_example.yaml +++ b/deploy/permissions_multi_namespace_example.yaml @@ -3,7 +3,7 @@ kind: ServiceAccount metadata: name: onepassword-connect-operator --- -kind: RoleBinding +kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 metadata: name: onepassword-connect-operator-default @@ -17,7 +17,7 @@ roleRef: name: onepassword-connect-operator apiGroup: rbac.authorization.k8s.io --- -kind: RoleBinding +kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 metadata: name: onepassword-connect-operator-development @@ -48,6 +48,7 @@ rules: - events - configmaps - secrets + - namespaces verbs: - create - delete diff --git a/go.sum b/go.sum index 58fa4cd..a383254 100644 --- a/go.sum +++ b/go.sum @@ -1447,6 +1447,7 @@ k8s.io/apimachinery v0.18.2/go.mod h1:9SnR/e11v5IbyPCGbvJViimtJ0SwHG4nfZFjU77ftc k8s.io/apimachinery v0.19.2 h1:5Gy9vQpAGTKHPVOh5c4plE274X8D/6cuEiTO2zve7tc= k8s.io/apimachinery v0.19.3 h1:bpIQXlKjB4cB/oNpnNnV+BybGPR7iP5oYpsOTEJ4hgc= k8s.io/apimachinery v0.20.1 h1:LAhz8pKbgR8tUwn7boK+b2HZdt7MiTu2mkYtFMUjTRQ= +k8s.io/apimachinery v0.20.2 h1:hFx6Sbt1oG0n6DZ+g4bFt5f6BoMkOjKWsQFu077M3Vg= k8s.io/apiserver v0.0.0-20190918160949-bfa5e2e684ad/go.mod h1:XPCXEwhjaFN29a8NldXA901ElnKeKLrLtREO9ZhFyhg= k8s.io/apiserver v0.0.0-20191122221311-9d521947b1e1/go.mod h1:RbsZY5zzBIWnz4KbctZsTVjwIuOpTp4Z8oCgFHN4kZQ= k8s.io/apiserver v0.16.7/go.mod h1:/5zSatF30/L9zYfMTl55jzzOnx7r/gGv5a5wtRp8yAw= diff --git a/pkg/onepassword/annotations.go b/pkg/onepassword/annotations.go index 2e24166..b7b5227 100644 --- a/pkg/onepassword/annotations.go +++ b/pkg/onepassword/annotations.go @@ -7,11 +7,12 @@ import ( ) const ( - OnepasswordPrefix = "onepasswordoperator" - ItemPathAnnotation = OnepasswordPrefix + "/item-path" - NameAnnotation = OnepasswordPrefix + "/item-name" - VersionAnnotation = OnepasswordPrefix + "/item-version" - RestartAnnotation = OnepasswordPrefix + "/lastRestarted" + OnepasswordPrefix = "onepasswordoperator" + ItemPathAnnotation = OnepasswordPrefix + "/item-path" + NameAnnotation = OnepasswordPrefix + "/item-name" + VersionAnnotation = OnepasswordPrefix + "/item-version" + RestartAnnotation = OnepasswordPrefix + "/lastRestarted" + RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto_restart" ) func GetAnnotationsForDeployment(deployment *appsv1.Deployment, regex *regexp.Regexp) (map[string]string, bool) { @@ -34,7 +35,7 @@ func GetAnnotationsForDeployment(deployment *appsv1.Deployment, regex *regexp.Re func FilterAnnotations(annotations map[string]string, regex *regexp.Regexp) map[string]string { filteredAnnotations := make(map[string]string) for key, value := range annotations { - if regex.MatchString(key) && key != RestartAnnotation { + if regex.MatchString(key) && key != RestartAnnotation && key != RestartDeploymentsAnnotation { filteredAnnotations[key] = value } } diff --git a/pkg/onepassword/secret_update_handler.go b/pkg/onepassword/secret_update_handler.go index 6fe0100..b7ae112 100644 --- a/pkg/onepassword/secret_update_handler.go +++ b/pkg/onepassword/secret_update_handler.go @@ -3,11 +3,14 @@ package onepassword import ( "context" "fmt" + "strconv" + "strings" "time" kubeSecrets "github.com/1Password/onepassword-operator/pkg/kubernetessecrets" "github.com/1Password/connect-sdk-go/connect" + "github.com/1Password/connect-sdk-go/onepassword" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -15,19 +18,22 @@ import ( ) const envHostVariable = "OP_HOST" +const lockTag = "onepasswordconnectoperator:ignore_secret" var log = logf.Log.WithName("update_op_kubernetes_secrets_task") -func NewManager(kubernetesClient client.Client, opConnectClient connect.Client) *SecretUpdateHandler { +func NewManager(kubernetesClient client.Client, opConnectClient connect.Client, shouldAutoRestartDeploymentsGlobal bool) *SecretUpdateHandler { return &SecretUpdateHandler{ - client: kubernetesClient, - opConnectClient: opConnectClient, + client: kubernetesClient, + opConnectClient: opConnectClient, + shouldAutoRestartDeploymentsGlobal: shouldAutoRestartDeploymentsGlobal, } } type SecretUpdateHandler struct { - client client.Client - opConnectClient connect.Client + client client.Client + opConnectClient connect.Client + shouldAutoRestartDeploymentsGlobal bool } func (h *SecretUpdateHandler) UpdateKubernetesSecretsTask() error { @@ -52,22 +58,33 @@ func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(updatedSecret return err } + if len(deployments.Items) == 0 { + return nil + } + + setForAutoRestartByNamespaceMap, err := h.getIsSetForAutoRestartByNamespaceMap() + if err != nil { + return err + } + for i := 0; i < len(deployments.Items); i++ { deployment := &deployments.Items[i] + if !isDeploymentSetForAutoRestart(deployment, setForAutoRestartByNamespaceMap) { + continue + } updatedSecrets := updatedSecretsByNamespace[deployment.Namespace] secretName := deployment.Annotations[NameAnnotation] - log.Info(fmt.Sprintf("Looking at secret %v for deployment %v", secretName, deployment.Name)) if isUpdatedSecret(secretName, updatedSecrets) || IsDeploymentUsingSecrets(deployment, updatedSecrets) { h.restartDeployment(deployment) } else { - log.Info(fmt.Sprintf("Deployment '%v' is up to date", deployment.GetName())) + log.Info(fmt.Sprintf("Deployment %q at namespace %q is up to date", deployment.GetName(), deployment.Namespace)) } } return nil } func (h *SecretUpdateHandler) restartDeployment(deployment *appsv1.Deployment) { - log.Info(fmt.Sprintf("Deployment '%v' references an updated secret. Restarting", deployment.GetName())) + log.Info(fmt.Sprintf("Deployment %q at namespace %q references an updated secret. Restarting", deployment.GetName(), deployment.Namespace)) deployment.Spec.Template.Annotations = map[string]string{ RestartAnnotation: time.Now().String(), } @@ -102,6 +119,12 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]b itemVersion := fmt.Sprint(item.Version) if currentVersion != itemVersion { + if isItemLockedForForcedRestarts(item) { + log.Info(fmt.Sprintf("Secret '%v' has been updated in 1Password but is set to be ignored. Updates to an ignored secret will not trigger an update to a kubernetes secret or a rolling restart.", secret.GetName())) + secret.Annotations[VersionAnnotation] = itemVersion + h.client.Update(context.Background(), &secret) + continue + } log.Info(fmt.Sprintf("Updating kubernetes secret '%v'", secret.GetName())) secret.Annotations[VersionAnnotation] = itemVersion updatedSecret := kubeSecrets.BuildKubernetesSecretFromOnePasswordItem(secret.Name, secret.Namespace, secret.Annotations, *item) @@ -115,6 +138,16 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets() (map[string]map[string]b return updatedSecrets, nil } +func isItemLockedForForcedRestarts(item *onepassword.Item) bool { + tags := item.Tags + for i := 0; i < len(tags); i++ { + if tags[i] == lockTag { + return true + } + } + return false +} + func isUpdatedSecret(secretName string, updatedSecrets map[string]bool) bool { _, ok := updatedSecrets[secretName] if ok { @@ -122,3 +155,57 @@ func isUpdatedSecret(secretName string, updatedSecrets map[string]bool) bool { } return false } + +func (h *SecretUpdateHandler) getIsSetForAutoRestartByNamespaceMap() (map[string]bool, error) { + namespaces := &corev1.NamespaceList{} + err := h.client.List(context.Background(), namespaces) + if err != nil { + log.Error(err, "Failed to list kubernetes namespaces") + return nil, err + } + + namespacesMap := map[string]bool{} + + for _, namespace := range namespaces.Items { + namespacesMap[namespace.Name] = h.isNamespaceSetToAutoRestart(&namespace) + } + return namespacesMap, nil +} + +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 + if restartDeployment == "" { + return setForAutoRestartByNamespace[deployment.Namespace] + } + + 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 (h *SecretUpdateHandler) isNamespaceSetToAutoRestart(namespace *corev1.Namespace) bool { + restartDeployment := namespace.Annotations[RestartDeploymentsAnnotation] + //If annotation for auto restarts for deployment is not set. Check environment variable set on the operator + if restartDeployment == "" { + return h.shouldAutoRestartDeploymentsGlobal + } + + restartDeploymentBool, err := 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/onepassword/secret_update_handler_test.go b/pkg/onepassword/secret_update_handler_test.go index 8a870e4..6df5b0d 100644 --- a/pkg/onepassword/secret_update_handler_test.go +++ b/pkg/onepassword/secret_update_handler_test.go @@ -34,14 +34,16 @@ const ( ) type testUpdateSecretTask struct { - testName string - existingDeployment *appsv1.Deployment - existingSecret *corev1.Secret - expectedError error - expectedResultSecret *corev1.Secret - expectedEvents []string - opItem map[string]string - expectedRestart bool + testName string + existingDeployment *appsv1.Deployment + existingNamespace *corev1.Namespace + existingSecret *corev1.Secret + expectedError error + expectedResultSecret *corev1.Secret + expectedEvents []string + opItem map[string]string + expectedRestart bool + globalAutoRestartEnabled bool } var ( @@ -52,9 +54,16 @@ var ( itemPath = fmt.Sprintf("vaults/%v/items/%v", vaultId, itemId) ) +var defaultNamespace = &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, +} + var tests = []testUpdateSecretTask{ { - testName: "Test unrelated deployment is not restarted with an updated secret", + testName: "Test unrelated deployment is not restarted with an updated secret", + existingNamespace: defaultNamespace, existingDeployment: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: deploymentKind, @@ -96,10 +105,12 @@ var tests = []testUpdateSecretTask{ userKey: username, passKey: password, }, - expectedRestart: false, + expectedRestart: false, + globalAutoRestartEnabled: true, }, { - testName: "OP item has new version. Secret needs update. Deployment is restarted based on containers", + testName: "OP item has new version. Secret needs update. Deployment is restarted based on containers", + existingNamespace: defaultNamespace, existingDeployment: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: deploymentKind, @@ -160,10 +171,12 @@ var tests = []testUpdateSecretTask{ userKey: username, passKey: password, }, - expectedRestart: true, + expectedRestart: true, + globalAutoRestartEnabled: true, }, { - testName: "OP item has new version. Secret needs update. Deployment is restarted based on annotation", + testName: "OP item has new version. Secret needs update. Deployment is restarted based on annotation", + existingNamespace: defaultNamespace, existingDeployment: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: deploymentKind, @@ -205,10 +218,12 @@ var tests = []testUpdateSecretTask{ userKey: username, passKey: password, }, - expectedRestart: true, + expectedRestart: true, + globalAutoRestartEnabled: true, }, { - testName: "OP item has new version. Secret needs update. Deployment is restarted based on volume", + testName: "OP item has new version. Secret needs update. Deployment is restarted based on volume", + existingNamespace: defaultNamespace, existingDeployment: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: deploymentKind, @@ -262,10 +277,12 @@ var tests = []testUpdateSecretTask{ userKey: username, passKey: password, }, - expectedRestart: true, + expectedRestart: true, + globalAutoRestartEnabled: true, }, { - testName: "No secrets need update. No deployment is restarted", + testName: "No secrets need update. No deployment is restarted", + existingNamespace: defaultNamespace, existingDeployment: &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: deploymentKind, @@ -307,11 +324,298 @@ var tests = []testUpdateSecretTask{ userKey: username, passKey: password, }, - expectedRestart: false, + expectedRestart: false, + globalAutoRestartEnabled: true, + }, + { + testName: `Deployment is not restarted when no auto restart is set to true for all + deployments and is not overwritten by by a namespace or deployment annotation`, + existingNamespace: defaultNamespace, + existingDeployment: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: deploymentKind, + APIVersion: deploymentAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + 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, + }, + }, + Data: expectedSecretData, + }, + expectedError: nil, + expectedResultSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + VersionAnnotation: fmt.Sprint(itemVersion), + ItemPathAnnotation: itemPath, + }, + }, + Data: expectedSecretData, + }, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedRestart: false, + globalAutoRestartEnabled: false, + }, + { + testName: `Deployment autostart true value takes precedence over false global auto restart 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, + }, + }, + Data: expectedSecretData, + }, + expectedError: nil, + expectedResultSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + VersionAnnotation: fmt.Sprint(itemVersion), + ItemPathAnnotation: itemPath, + }, + }, + Data: expectedSecretData, + }, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedRestart: true, + globalAutoRestartEnabled: true, + }, + { + testName: `Deployment autostart false value takes precedence over false global auto restart value, + and true namespace value.`, + existingNamespace: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + Annotations: map[string]string{ + RestartDeploymentsAnnotation: "true", + }, + }, + }, + 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, + }, + }, + Data: expectedSecretData, + }, + expectedError: nil, + expectedResultSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + VersionAnnotation: fmt.Sprint(itemVersion), + ItemPathAnnotation: itemPath, + }, + }, + Data: expectedSecretData, + }, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedRestart: false, + globalAutoRestartEnabled: false, + }, + { + testName: `Namespace autostart true value takes precedence over false global auto restart value`, + existingNamespace: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + Annotations: map[string]string{ + RestartDeploymentsAnnotation: "true", + }, + }, + }, + existingDeployment: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: deploymentKind, + APIVersion: deploymentAPIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + 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, + }, + }, + Data: expectedSecretData, + }, + expectedError: nil, + expectedResultSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + VersionAnnotation: fmt.Sprint(itemVersion), + ItemPathAnnotation: itemPath, + }, + }, + Data: expectedSecretData, + }, + opItem: map[string]string{ + userKey: username, + passKey: password, + }, + expectedRestart: true, + globalAutoRestartEnabled: false, }, } -func TestReconcileDepoyment(t *testing.T) { +func TestUpdateSecretHandler(t *testing.T) { for _, testData := range tests { t.Run(testData.testName, func(t *testing.T) { @@ -322,6 +626,7 @@ func TestReconcileDepoyment(t *testing.T) { // Objects to track in the fake client. objs := []runtime.Object{ testData.existingDeployment, + testData.existingNamespace, } if testData.existingSecret != nil { @@ -342,8 +647,9 @@ func TestReconcileDepoyment(t *testing.T) { return &item, nil } h := &SecretUpdateHandler{ - client: cl, - opConnectClient: opConnectClient, + client: cl, + opConnectClient: opConnectClient, + shouldAutoRestartDeploymentsGlobal: testData.globalAutoRestartEnabled, } err := h.UpdateKubernetesSecretsTask() @@ -377,7 +683,7 @@ func TestReconcileDepoyment(t *testing.T) { _, ok := deployment.Spec.Template.Annotations[RestartAnnotation] if ok { - assert.True(t, testData.expectedRestart) + assert.True(t, testData.expectedRestart, "Expected deployment to restart but it did not") } else { assert.False(t, testData.expectedRestart) }