From f5542ab1f8347116500641ae41fe6e1e62009e90 Mon Sep 17 00:00:00 2001 From: Bence Csati Date: Sun, 9 Jun 2024 15:25:54 +0200 Subject: [PATCH 1/5] feat: add new annotations to match with the secrets-webhook + backwards compatibility Signed-off-by: Bence Csati --- pkg/reloader/controller.go | 11 +++++--- pkg/reloader/reloader.go | 25 ++++++++++++++--- pkg/reloader/reloader_test.go | 53 +++++++++++++++++++++++++++++------ 3 files changed, 73 insertions(+), 16 deletions(-) diff --git a/pkg/reloader/controller.go b/pkg/reloader/controller.go index 69bbad6..d2923d6 100644 --- a/pkg/reloader/controller.go +++ b/pkg/reloader/controller.go @@ -37,8 +37,11 @@ const ( DaemonSetKind = "DaemonSet" StatefulSetKind = "StatefulSet" - SecretReloadAnnotationName = "alpha.vault.security.banzaicloud.io/reload-on-secret-change" - ReloadCountAnnotationName = "alpha.vault.security.banzaicloud.io/secret-reload-count" + SecretReloadAnnotationName = "secrets-reloader.security.bank-vaults.io/reload-on-secret-change" + ReloadCountAnnotationName = "secrets-reloader.security.bank-vaults.io/secret-reload-count" + + DeprecatedSecretReloadAnnotationName = "alpha.vault.security.banzaicloud.io/reload-on-secret-change" + DeprecatedReloadCountAnnotationName = "alpha.vault.security.banzaicloud.io/secret-reload-count" ) // Controller is the controller implementation for Foo resources @@ -157,7 +160,7 @@ func (c *Controller) handleObject(obj interface{}) { } // Process workload, skip if reload annotation not present - if podTemplateSpec.GetAnnotations()[SecretReloadAnnotationName] != "true" { + if podTemplateSpec.GetAnnotations()[SecretReloadAnnotationName] != "true" || podTemplateSpec.GetAnnotations()[DeprecatedSecretReloadAnnotationName] != "true" { return } c.logger.Debug(fmt.Sprintf("Processing workload: %#v", workloadData)) @@ -204,7 +207,7 @@ func (c *Controller) handleObjectDelete(obj interface{}) { } // Delete workload, skip if reload annotation not present - if podTemplateSpec.GetAnnotations()[SecretReloadAnnotationName] != "true" { + if podTemplateSpec.GetAnnotations()[SecretReloadAnnotationName] != "true" || podTemplateSpec.GetAnnotations()[DeprecatedSecretReloadAnnotationName] != "true" { return } c.logger.Debug(fmt.Sprintf("Deleting workload from store: %#v", workloadData)) diff --git a/pkg/reloader/reloader.go b/pkg/reloader/reloader.go index d2b371d..96c4d02 100644 --- a/pkg/reloader/reloader.go +++ b/pkg/reloader/reloader.go @@ -154,14 +154,31 @@ func (c *Controller) reloadWorkload(workload workload) error { func incrementReloadCountAnnotation(podTemplate *corev1.PodTemplateSpec) { version := "1" + annotationName := ReloadCountAnnotationName + + // If there are no annotations set on the resource, + // .GetAnnotations() will return nil so it needs to be + // initialized so in that case we can add the annotation + annotations := podTemplate.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + + reloadCount, ok := annotations[ReloadCountAnnotationName] + if !ok { + reloadCount, ok = annotations[DeprecatedReloadCountAnnotationName] + if ok { + annotationName = DeprecatedReloadCountAnnotationName + } + } - if reloadCount := podTemplate.GetAnnotations()[ReloadCountAnnotationName]; reloadCount != "" { - count, err := strconv.Atoi(reloadCount) - if err == nil { + if reloadCount != "" { + if count, err := strconv.Atoi(reloadCount); err == nil { count++ version = strconv.Itoa(count) } } - podTemplate.GetAnnotations()[ReloadCountAnnotationName] = version + annotations[annotationName] = version + podTemplate.SetAnnotations(annotations) } diff --git a/pkg/reloader/reloader_test.go b/pkg/reloader/reloader_test.go index 543dcfd..0c2a7a3 100644 --- a/pkg/reloader/reloader_test.go +++ b/pkg/reloader/reloader_test.go @@ -18,19 +18,56 @@ import ( "testing" "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestIncrementReloadCountAnnotation(t *testing.T) { - podTemplate := corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{}, + tests := []struct { + name string + annotation string + annotationValue string + expectedAnnoation string + expectedValue string + }{ + { + name: "empty annotation should use new annotation", + annotation: "", + expectedAnnoation: ReloadCountAnnotationName, + expectedValue: "1", + }, + { + name: "declared annotation should use the same annotation", + annotation: ReloadCountAnnotationName, + annotationValue: "1", + expectedAnnoation: ReloadCountAnnotationName, + expectedValue: "2", + }, + { + name: "deprecated annotation should use the same annotation", + annotation: DeprecatedReloadCountAnnotationName, + annotationValue: "1", + expectedAnnoation: DeprecatedReloadCountAnnotationName, + expectedValue: "2", }, } - incrementReloadCountAnnotation(&podTemplate) - assert.Equal(t, "1", podTemplate.GetAnnotations()[ReloadCountAnnotationName]) - incrementReloadCountAnnotation(&podTemplate) - assert.Equal(t, "2", podTemplate.GetAnnotations()[ReloadCountAnnotationName]) + for _, tt := range tests { + ttp := tt + t.Run(ttp.name, func(t *testing.T) { + podTemplateSpec := &v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + ttp.annotation: ttp.annotationValue, + }, + }, + } + + incrementReloadCountAnnotation(podTemplateSpec) + + annotationValue, ok := podTemplateSpec.Annotations[ttp.expectedAnnoation] + assert.True(t, ok) + assert.Equal(t, ttp.expectedValue, annotationValue) + }) + } } From f0f149509192258d02f0d2eeb55c7599d2c165b5 Mon Sep 17 00:00:00 2001 From: Bence Csati Date: Sun, 9 Jun 2024 15:26:56 +0200 Subject: [PATCH 2/5] docs: add new annotation to docs, examples, test manifests Signed-off-by: Bence Csati --- README.md | 6 +++--- deploy/charts/vault-secrets-reloader/README.md | 2 +- deploy/charts/vault-secrets-reloader/README.md.gotmpl | 2 +- e2e/deploy/workloads/daemonset.yaml | 2 +- e2e/deploy/workloads/deployments.yaml | 8 ++++---- e2e/deploy/workloads/statefulset.yaml | 2 +- examples/reloader-in-bank-vaults-ecosystem.md | 2 +- examples/try-locally.md | 8 ++++---- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 3f2c91d..cf9d476 100644 --- a/README.md +++ b/README.md @@ -15,9 +15,9 @@ Vault Secrets Reloader can periodically check if a secret that is used in watche Upon deployment, the Reloader spawns two “workers”, that run periodically at two different time intervals: -1. The `collector` collects and stores information about the workloads that are opted in via the `alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true"` annotation in their pod template metadata and the Vault secrets they use. +1. The `collector` collects and stores information about the workloads that are opted in via the `secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true"` annotation in their pod template metadata and the Vault secrets they use. -2. The `reloader` iterates on the data collected by the `collector`, polling the configured Vault instance for the current version of the secrets, and if it finds that it differs from the stored one, adds the workloads where the secret is used to a list of workloads that needs reloading. In a following step, it modifies these workloads by incrementing the value of the `alpha.vault.security.banzaicloud.io/secret-reload-count` annotation in their pod template metadata, initiating a new rollout. +2. The `reloader` iterates on the data collected by the `collector`, polling the configured Vault instance for the current version of the secrets, and if it finds that it differs from the stored one, adds the workloads where the secret is used to a list of workloads that needs reloading. In a following step, it modifies these workloads by incrementing the value of the `secrets-reloader.security.bank-vaults.io/secret-reload-count` annotation in their pod template metadata, initiating a new rollout. To get familiarized, check out [how Reloader fits in the Bank-Vaults ecosystem](https://github.com/bank-vaults/vault-secrets-reloader/blob/main/examples/reloader-in-bank-vaults-ecosystem.md), and how can you [give Reloader a spin](https://github.com/bank-vaults/vault-secrets-reloader/blob/main/examples/try-locally.md) on your local machine. @@ -29,7 +29,7 @@ To get familiarized, check out [how Reloader fits in the Bank-Vaults ecosystem]( - It can only check for updated versions of secrets in one specific instance of Hashicorp Vault, no other secret stores are supported yet. -- It can only “reload” Deployments, DaemonSets and StatefulSets that have the `alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true"` annotation set among their `spec.template.metadata.annotations`. +- It can only “reload” Deployments, DaemonSets and StatefulSets that have the `secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true"` annotation set among their `spec.template.metadata.annotations`. - The `collector` can only look for secrets in the workload’s pod template environment variables directly, and in their `secrets-webhook.security.bank-vaults.io/vault-from-path` annotation, in the format the `secrets-webhook` also uses, and are unversioned. diff --git a/deploy/charts/vault-secrets-reloader/README.md b/deploy/charts/vault-secrets-reloader/README.md index 9e4fd3d..3b2c059 100644 --- a/deploy/charts/vault-secrets-reloader/README.md +++ b/deploy/charts/vault-secrets-reloader/README.md @@ -11,7 +11,7 @@ Reloader works in conjunction with the [Secrets Webhook](https://github.com/bank You will need to add the following annotations to the pod template spec of the workloads (i.e. Deployments, DaemonSets and StatefulSets) that you wish to reload: ```yaml -alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" +secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" ``` ## Installing the Chart diff --git a/deploy/charts/vault-secrets-reloader/README.md.gotmpl b/deploy/charts/vault-secrets-reloader/README.md.gotmpl index 90fd82e..ea15a1f 100644 --- a/deploy/charts/vault-secrets-reloader/README.md.gotmpl +++ b/deploy/charts/vault-secrets-reloader/README.md.gotmpl @@ -11,7 +11,7 @@ Reloader works in conjunction with the [Secrets Webhook](https://github.com/bank You will need to add the following annotations to the pod template spec of the workloads (i.e. Deployments, DaemonSets and StatefulSets) that you wish to reload: ```yaml -alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" +secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" ``` ## Installing the Chart diff --git a/e2e/deploy/workloads/daemonset.yaml b/e2e/deploy/workloads/daemonset.yaml index 27a68f1..bf96819 100644 --- a/e2e/deploy/workloads/daemonset.yaml +++ b/e2e/deploy/workloads/daemonset.yaml @@ -15,7 +15,7 @@ spec: secrets-webhook.security.bank-vaults.io/provider: "vault" secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls - alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" + secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" spec: initContainers: - name: init-ubuntu diff --git a/e2e/deploy/workloads/deployments.yaml b/e2e/deploy/workloads/deployments.yaml index 39f4b60..31be499 100644 --- a/e2e/deploy/workloads/deployments.yaml +++ b/e2e/deploy/workloads/deployments.yaml @@ -14,7 +14,7 @@ spec: annotations: secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls - alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" + secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" spec: initContainers: - name: init-ubuntu @@ -100,7 +100,7 @@ spec: annotations: secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls - alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" + secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" spec: containers: - name: alpine @@ -138,7 +138,7 @@ spec: secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls secrets-webhook.security.bank-vaults.io/vault-from-path: "secret/data/accounts/aws" - alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" + secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" spec: containers: - name: alpine @@ -177,7 +177,7 @@ spec: secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls secrets-webhook.security.bank-vaults.io/vault-from-path: "secret/data/dockerrepo#1" - alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" + secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" spec: containers: - name: alpine diff --git a/e2e/deploy/workloads/statefulset.yaml b/e2e/deploy/workloads/statefulset.yaml index 59c26d8..8753112 100644 --- a/e2e/deploy/workloads/statefulset.yaml +++ b/e2e/deploy/workloads/statefulset.yaml @@ -15,7 +15,7 @@ spec: secrets-webhook.security.bank-vaults.io/provider: "vault" secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls - alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" + secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" spec: initContainers: - name: init-ubuntu diff --git a/examples/reloader-in-bank-vaults-ecosystem.md b/examples/reloader-in-bank-vaults-ecosystem.md index 92e54f5..3721ea7 100644 --- a/examples/reloader-in-bank-vaults-ecosystem.md +++ b/examples/reloader-in-bank-vaults-ecosystem.md @@ -6,7 +6,7 @@ This is a high level overview of how the Reloader plays along with other compone ![flowchart](./assets/flowchart.png) -1. The `collector` worker periodically collects unversioned secrets from workloads with the `alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true"` annotation that are in the format for injection by the Webhook. +1. The `collector` worker periodically collects unversioned secrets from workloads with the `secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true"` annotation that are in the format for injection by the Webhook. 2. At its scheduled time, the `reloader` worker checks in Vault if there is a new version of any of the collected secrets since the last sync. If it is the case, it continues to step 3, otherwise the workflow stops. diff --git a/examples/try-locally.md b/examples/try-locally.md index 3cdd36a..a8d1671 100644 --- a/examples/try-locally.md +++ b/examples/try-locally.md @@ -69,7 +69,7 @@ Now that we have the Bank-Vaults ecosystem running in our kind cluster, we can d kubectl apply -f e2e/deploy/workloads ``` -Looking at the manifest of one of the deployments, the only difference from one that is prepared to work with the Bank-Vaults Webhook with all the annotations starting with `secrets-webhook.security.bank-vaults.io` and the env values starting with `vault:` is the presence of the new `alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true"` annotation telling the Reloader to collect secrets and reload it if necessary. +Looking at the manifest of one of the deployments, the only difference from the one that is prepared to work with the Bank-Vaults Webhook with all the annotations starting with `secrets-webhook.security.bank-vaults.io` and the env values starting with `vault:` is the presence of the new `secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true"` annotation telling the Reloader to collect secrets and reload it if necessary. ```yaml apiVersion: apps/v1 @@ -88,7 +88,7 @@ spec: annotations: secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls - alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" + secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" spec: initContainers: - name: init-ubuntu @@ -140,10 +140,10 @@ Now everything is set to try some things out with the Reloader: Also notice that there are two pods with the now changed `MYSQL_PASSWORD` injected into them not being restarted, for the following reasons: - - the pod `reloader-test-deployment-no-reload-xxx` does not have the `alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true"` annotation set + - the pod `reloader-test-deployment-no-reload-xxx` does not have the `secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true"` annotation set - the pod `reloader-test-deployment-fixed-versions-no-reload-xxx` - although it does have the annotation - only uses versioned secrets, so they won't be reloaded for the latest version of the secret. -2. Change two secrets used in a workload, observe the previous pod to be recreated again, also that the pod `reloader-test-daemonset-xxx` only restarted once, although it uses both of these secrets. The number a workload got "reloaded" by the Reloader can be checked on the `alpha.vault.security.banzaicloud.io/secret-reload-count` annotation that is used to trigger a new rollout. +2. Change two secrets used in a workload, observe the previous pod to be recreated again, also that the pod `reloader-test-daemonset-xxx` only restarted once, although it uses both of these secrets. The number a workload got "reloaded" by the Reloader can be checked on the `secrets-reloader.security.bank-vaults.io/secret-reload-count` annotation that is used to trigger a new rollout. ```bash vault kv patch secret/accounts/aws AWS_SECRET_ACCESS_KEY=s3cr3t2 From 0e10325d817258b639441854e346bc8d6b1335a9 Mon Sep 17 00:00:00 2001 From: Bence Csati Date: Sun, 9 Jun 2024 16:52:31 +0200 Subject: [PATCH 3/5] wip Signed-off-by: Bence Csati --- e2e/deploy/workloads/daemonset.yaml | 1 + e2e/deploy/workloads/deployments.yaml | 4 ++++ pkg/reloader/controller.go | 8 ++++++-- pkg/reloader/reloader.go | 25 ++++++++++--------------- pkg/reloader/reloader_test.go | 16 ++++++++++------ 5 files changed, 31 insertions(+), 23 deletions(-) diff --git a/e2e/deploy/workloads/daemonset.yaml b/e2e/deploy/workloads/daemonset.yaml index bf96819..1a6d00e 100644 --- a/e2e/deploy/workloads/daemonset.yaml +++ b/e2e/deploy/workloads/daemonset.yaml @@ -15,6 +15,7 @@ spec: secrets-webhook.security.bank-vaults.io/provider: "vault" secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls + alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" spec: initContainers: diff --git a/e2e/deploy/workloads/deployments.yaml b/e2e/deploy/workloads/deployments.yaml index 31be499..4b83300 100644 --- a/e2e/deploy/workloads/deployments.yaml +++ b/e2e/deploy/workloads/deployments.yaml @@ -12,6 +12,7 @@ spec: labels: app.kubernetes.io/name: reloader-test-deployment-to-be-reloaded annotations: + secrets-webhook.security.bank-vaults.io/provider: "vault" secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" @@ -62,6 +63,7 @@ spec: labels: app.kubernetes.io/name: reloader-test-deployment-no-reload annotations: + secrets-webhook.security.bank-vaults.io/provider: "vault" secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls spec: @@ -98,6 +100,7 @@ spec: labels: app.kubernetes.io/name: reloader-test-deployment-fixed-versions-no-reload annotations: + secrets-webhook.security.bank-vaults.io/provider: "vault" secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" @@ -135,6 +138,7 @@ spec: labels: app.kubernetes.io/name: reloader-test-deployment-annotated-reload annotations: + secrets-webhook.security.bank-vaults.io/provider: "vault" secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls secrets-webhook.security.bank-vaults.io/vault-from-path: "secret/data/accounts/aws" diff --git a/pkg/reloader/controller.go b/pkg/reloader/controller.go index d2923d6..fdb02b5 100644 --- a/pkg/reloader/controller.go +++ b/pkg/reloader/controller.go @@ -160,7 +160,7 @@ func (c *Controller) handleObject(obj interface{}) { } // Process workload, skip if reload annotation not present - if podTemplateSpec.GetAnnotations()[SecretReloadAnnotationName] != "true" || podTemplateSpec.GetAnnotations()[DeprecatedSecretReloadAnnotationName] != "true" { + if shouldProcessWorkload(podTemplateSpec.Annotations) { return } c.logger.Debug(fmt.Sprintf("Processing workload: %#v", workloadData)) @@ -207,9 +207,13 @@ func (c *Controller) handleObjectDelete(obj interface{}) { } // Delete workload, skip if reload annotation not present - if podTemplateSpec.GetAnnotations()[SecretReloadAnnotationName] != "true" || podTemplateSpec.GetAnnotations()[DeprecatedSecretReloadAnnotationName] != "true" { + if shouldProcessWorkload(podTemplateSpec.Annotations) { return } c.logger.Debug(fmt.Sprintf("Deleting workload from store: %#v", workloadData)) c.workloadSecrets.Delete(workloadData) } + +func shouldProcessWorkload(annotations map[string]string) bool { + return annotations[SecretReloadAnnotationName] == "true" || annotations[DeprecatedSecretReloadAnnotationName] == "true" +} diff --git a/pkg/reloader/reloader.go b/pkg/reloader/reloader.go index 96c4d02..588c69f 100644 --- a/pkg/reloader/reloader.go +++ b/pkg/reloader/reloader.go @@ -156,29 +156,24 @@ func incrementReloadCountAnnotation(podTemplate *corev1.PodTemplateSpec) { version := "1" annotationName := ReloadCountAnnotationName - // If there are no annotations set on the resource, - // .GetAnnotations() will return nil so it needs to be - // initialized so in that case we can add the annotation - annotations := podTemplate.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - - reloadCount, ok := annotations[ReloadCountAnnotationName] - if !ok { - reloadCount, ok = annotations[DeprecatedReloadCountAnnotationName] - if ok { + // check for the current annotation + reloadCount, currentExists := podTemplate.GetAnnotations()[ReloadCountAnnotationName] + if !currentExists { + // check for deprecated annotation + if deprecatedReloadCount, deprecatedExists := podTemplate.GetAnnotations()[DeprecatedReloadCountAnnotationName]; deprecatedExists { annotationName = DeprecatedReloadCountAnnotationName + reloadCount = deprecatedReloadCount } } + // parse and increment the reload count if reloadCount != "" { - if count, err := strconv.Atoi(reloadCount); err == nil { + count, err := strconv.Atoi(reloadCount) + if err == nil { count++ version = strconv.Itoa(count) } } - annotations[annotationName] = version - podTemplate.SetAnnotations(annotations) + podTemplate.GetAnnotations()[annotationName] = version } diff --git a/pkg/reloader/reloader_test.go b/pkg/reloader/reloader_test.go index 0c2a7a3..2660b01 100644 --- a/pkg/reloader/reloader_test.go +++ b/pkg/reloader/reloader_test.go @@ -18,7 +18,7 @@ import ( "testing" "github.com/stretchr/testify/assert" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -31,8 +31,9 @@ func TestIncrementReloadCountAnnotation(t *testing.T) { expectedValue string }{ { - name: "empty annotation should use new annotation", + name: "no annotation should use the ReloadCountAnnotationName", annotation: "", + annotationValue: "", expectedAnnoation: ReloadCountAnnotationName, expectedValue: "1", }, @@ -55,11 +56,14 @@ func TestIncrementReloadCountAnnotation(t *testing.T) { for _, tt := range tests { ttp := tt t.Run(ttp.name, func(t *testing.T) { - podTemplateSpec := &v1.PodTemplateSpec{ + annotations := map[string]string{} + if ttp.annotation != "" { + annotations[ttp.annotation] = ttp.annotationValue + } + + podTemplateSpec := &corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - ttp.annotation: ttp.annotationValue, - }, + Annotations: annotations, }, } From 9cef44a7573b2f8cec0f136cebbc997a72d81264 Mon Sep 17 00:00:00 2001 From: Bence Csati Date: Mon, 10 Jun 2024 10:59:23 +0200 Subject: [PATCH 4/5] revert: drop backwards compatibility Signed-off-by: Bence Csati --- e2e/deploy/workloads/daemonset.yaml | 1 - pkg/reloader/controller.go | 11 ++----- pkg/reloader/reloader.go | 16 ++-------- pkg/reloader/reloader_test.go | 45 ++++++++++------------------- 4 files changed, 20 insertions(+), 53 deletions(-) diff --git a/e2e/deploy/workloads/daemonset.yaml b/e2e/deploy/workloads/daemonset.yaml index 1a6d00e..bf96819 100644 --- a/e2e/deploy/workloads/daemonset.yaml +++ b/e2e/deploy/workloads/daemonset.yaml @@ -15,7 +15,6 @@ spec: secrets-webhook.security.bank-vaults.io/provider: "vault" secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls - alpha.vault.security.banzaicloud.io/reload-on-secret-change: "true" secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" spec: initContainers: diff --git a/pkg/reloader/controller.go b/pkg/reloader/controller.go index fdb02b5..291011a 100644 --- a/pkg/reloader/controller.go +++ b/pkg/reloader/controller.go @@ -39,9 +39,6 @@ const ( SecretReloadAnnotationName = "secrets-reloader.security.bank-vaults.io/reload-on-secret-change" ReloadCountAnnotationName = "secrets-reloader.security.bank-vaults.io/secret-reload-count" - - DeprecatedSecretReloadAnnotationName = "alpha.vault.security.banzaicloud.io/reload-on-secret-change" - DeprecatedReloadCountAnnotationName = "alpha.vault.security.banzaicloud.io/secret-reload-count" ) // Controller is the controller implementation for Foo resources @@ -160,7 +157,7 @@ func (c *Controller) handleObject(obj interface{}) { } // Process workload, skip if reload annotation not present - if shouldProcessWorkload(podTemplateSpec.Annotations) { + if podTemplateSpec.GetAnnotations()[SecretReloadAnnotationName] != "true" { return } c.logger.Debug(fmt.Sprintf("Processing workload: %#v", workloadData)) @@ -207,13 +204,9 @@ func (c *Controller) handleObjectDelete(obj interface{}) { } // Delete workload, skip if reload annotation not present - if shouldProcessWorkload(podTemplateSpec.Annotations) { + if podTemplateSpec.GetAnnotations()[SecretReloadAnnotationName] != "true" { return } c.logger.Debug(fmt.Sprintf("Deleting workload from store: %#v", workloadData)) c.workloadSecrets.Delete(workloadData) } - -func shouldProcessWorkload(annotations map[string]string) bool { - return annotations[SecretReloadAnnotationName] == "true" || annotations[DeprecatedSecretReloadAnnotationName] == "true" -} diff --git a/pkg/reloader/reloader.go b/pkg/reloader/reloader.go index 588c69f..d2b371d 100644 --- a/pkg/reloader/reloader.go +++ b/pkg/reloader/reloader.go @@ -154,20 +154,8 @@ func (c *Controller) reloadWorkload(workload workload) error { func incrementReloadCountAnnotation(podTemplate *corev1.PodTemplateSpec) { version := "1" - annotationName := ReloadCountAnnotationName - - // check for the current annotation - reloadCount, currentExists := podTemplate.GetAnnotations()[ReloadCountAnnotationName] - if !currentExists { - // check for deprecated annotation - if deprecatedReloadCount, deprecatedExists := podTemplate.GetAnnotations()[DeprecatedReloadCountAnnotationName]; deprecatedExists { - annotationName = DeprecatedReloadCountAnnotationName - reloadCount = deprecatedReloadCount - } - } - // parse and increment the reload count - if reloadCount != "" { + if reloadCount := podTemplate.GetAnnotations()[ReloadCountAnnotationName]; reloadCount != "" { count, err := strconv.Atoi(reloadCount) if err == nil { count++ @@ -175,5 +163,5 @@ func incrementReloadCountAnnotation(podTemplate *corev1.PodTemplateSpec) { } } - podTemplate.GetAnnotations()[annotationName] = version + podTemplate.GetAnnotations()[ReloadCountAnnotationName] = version } diff --git a/pkg/reloader/reloader_test.go b/pkg/reloader/reloader_test.go index 2660b01..8bce18e 100644 --- a/pkg/reloader/reloader_test.go +++ b/pkg/reloader/reloader_test.go @@ -25,53 +25,40 @@ import ( func TestIncrementReloadCountAnnotation(t *testing.T) { tests := []struct { name string - annotation string - annotationValue string - expectedAnnoation string - expectedValue string + annotations map[string]string + expectedAnnoation map[string]string }{ { - name: "no annotation should use the ReloadCountAnnotationName", - annotation: "", - annotationValue: "", - expectedAnnoation: ReloadCountAnnotationName, - expectedValue: "1", + name: "no annotation should add annotation", + annotations: map[string]string{}, + expectedAnnoation: map[string]string{ + ReloadCountAnnotationName: "1", + }, }, { - name: "declared annotation should use the same annotation", - annotation: ReloadCountAnnotationName, - annotationValue: "1", - expectedAnnoation: ReloadCountAnnotationName, - expectedValue: "2", - }, - { - name: "deprecated annotation should use the same annotation", - annotation: DeprecatedReloadCountAnnotationName, - annotationValue: "1", - expectedAnnoation: DeprecatedReloadCountAnnotationName, - expectedValue: "2", + name: "existing annotation should increment annotation", + annotations: map[string]string{ + ReloadCountAnnotationName: "1", + }, + expectedAnnoation: map[string]string{ + ReloadCountAnnotationName: "2", + }, }, } for _, tt := range tests { ttp := tt t.Run(ttp.name, func(t *testing.T) { - annotations := map[string]string{} - if ttp.annotation != "" { - annotations[ttp.annotation] = ttp.annotationValue - } podTemplateSpec := &corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ - Annotations: annotations, + Annotations: ttp.annotations, }, } incrementReloadCountAnnotation(podTemplateSpec) - annotationValue, ok := podTemplateSpec.Annotations[ttp.expectedAnnoation] - assert.True(t, ok) - assert.Equal(t, ttp.expectedValue, annotationValue) + assert.Equal(t, ttp.expectedAnnoation, podTemplateSpec.Annotations) }) } } From e757412cff716581e8c6a6e3e906295f50d73cf2 Mon Sep 17 00:00:00 2001 From: Bence Csati Date: Tue, 11 Jun 2024 12:58:44 +0200 Subject: [PATCH 5/5] fix: remarks Signed-off-by: Bence Csati --- examples/try-locally.md | 1 + pkg/reloader/reloader_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/examples/try-locally.md b/examples/try-locally.md index a8d1671..8d34fe0 100644 --- a/examples/try-locally.md +++ b/examples/try-locally.md @@ -86,6 +86,7 @@ spec: labels: app.kubernetes.io/name: reloader-test-deployment-to-be-reloaded annotations: + secrets-webhook.security.bank-vaults.io/provider: "vault" secrets-webhook.security.bank-vaults.io/vault-addr: "https://vault:8200" secrets-webhook.security.bank-vaults.io/vault-tls-secret: vault-tls secrets-reloader.security.bank-vaults.io/reload-on-secret-change: "true" diff --git a/pkg/reloader/reloader_test.go b/pkg/reloader/reloader_test.go index 8bce18e..defbcf6 100644 --- a/pkg/reloader/reloader_test.go +++ b/pkg/reloader/reloader_test.go @@ -24,14 +24,14 @@ import ( func TestIncrementReloadCountAnnotation(t *testing.T) { tests := []struct { - name string - annotations map[string]string - expectedAnnoation map[string]string + name string + annotations map[string]string + expectedAnnotations map[string]string }{ { name: "no annotation should add annotation", annotations: map[string]string{}, - expectedAnnoation: map[string]string{ + expectedAnnotations: map[string]string{ ReloadCountAnnotationName: "1", }, }, @@ -40,7 +40,7 @@ func TestIncrementReloadCountAnnotation(t *testing.T) { annotations: map[string]string{ ReloadCountAnnotationName: "1", }, - expectedAnnoation: map[string]string{ + expectedAnnotations: map[string]string{ ReloadCountAnnotationName: "2", }, }, @@ -58,7 +58,7 @@ func TestIncrementReloadCountAnnotation(t *testing.T) { incrementReloadCountAnnotation(podTemplateSpec) - assert.Equal(t, ttp.expectedAnnoation, podTemplateSpec.Annotations) + assert.Equal(t, ttp.expectedAnnotations, podTemplateSpec.Annotations) }) } }