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 +}