From 05540d9201936c459cd34b7295654aeb03f6adf0 Mon Sep 17 00:00:00 2001 From: Tim Bauer <30375389+bimtauer@users.noreply.github.com> Date: Thu, 11 May 2023 16:39:54 +0200 Subject: [PATCH] Use group attribute for KV version and add DB engine support (#539) * Continue supporting config kvversion as default arg Signed-off-by: Tim Bauer * Remove obsolete test Signed-off-by: Tim Bauer * Add deprecation information Signed-off-by: Tim Bauer * Remove comment Signed-off-by: Tim Bauer * Remove comments Signed-off-by: Tim Bauer --------- Signed-off-by: Tim Bauer --- flytepropeller/pkg/webhook/config/config.go | 2 +- flytepropeller/pkg/webhook/utils.go | 38 +++++-- .../pkg/webhook/vault_secret_manager.go | 6 +- .../pkg/webhook/vault_secret_manager_test.go | 106 +++++++++++++----- 4 files changed, 106 insertions(+), 46 deletions(-) diff --git a/flytepropeller/pkg/webhook/config/config.go b/flytepropeller/pkg/webhook/config/config.go index b4a68702b..2f91b953a 100644 --- a/flytepropeller/pkg/webhook/config/config.go +++ b/flytepropeller/pkg/webhook/config/config.go @@ -114,7 +114,7 @@ type GCPSecretManagerConfig struct { type VaultSecretManagerConfig struct { Role string `json:"role" pflag:",Specifies the vault role to use"` - KVVersion KVVersion `json:"kvVersion" pflag:"-,The KV Engine Version. Defaults to 2. Use 1 for unversioned secrets. Refer to - https://www.vaultproject.io/docs/secrets/kv#kv-secrets-engine."` + KVVersion KVVersion `json:"kvVersion" pflag:"-,DEPRECATED! Use the GroupVersion field of the Secret request instead. The KV Engine Version. Defaults to 2. Use 1 for unversioned secrets. Refer to - https://www.vaultproject.io/docs/secrets/kv#kv-secrets-engine."` Annotations map[string]string `json:"annotations" pflag:"-,Annotation to be added to user task pod. The annotation can also be used to override default annotations added by Flyte. Useful to customize Vault integration (https://developer.hashicorp.com/vault/docs/platform/k8s/injector/annotations)"` } diff --git a/flytepropeller/pkg/webhook/utils.go b/flytepropeller/pkg/webhook/utils.go index 9f07ac417..2dd0bb991 100644 --- a/flytepropeller/pkg/webhook/utils.go +++ b/flytepropeller/pkg/webhook/utils.go @@ -123,30 +123,44 @@ func AppendVolume(volumes []corev1.Volume, volume corev1.Volume) []corev1.Volume return append(volumes, volume) } -func CreateVaultAnnotationsForSecret(secret *core.Secret, kvversion config.KVVersion) (map[string]string, error) { +func CreateVaultAnnotationsForSecret(secret *core.Secret, kvversion config.KVVersion) map[string]string { // Creates three grouped annotations "agent-inject-secret", "agent-inject-file" and "agent-inject-template" // for a given secret request and KV engine version. The annotations respectively handle: 1. retrieving the // secret from a vault path specified in secret.Group, 2. storing it in a file named after secret.Group/secret.Key // and 3. creating a template that retrieves only secret.Key from the multiple k:v pairs present in a vault secret. id := string(uuid.NewUUID()) + secretVaultAnnotations := map[string]string{ + fmt.Sprintf("vault.hashicorp.com/agent-inject-secret-%s", id): secret.Group, + fmt.Sprintf("vault.hashicorp.com/agent-inject-file-%s", id): fmt.Sprintf("%s/%s", secret.Group, secret.Key), + } + // Set the consul template language query depending on the KV Secrets Engine version. // Version 1 stores plain k:v pairs under .Data, version 2 supports versioned secrets // and wraps the k:v pairs into an additional subfield. var query string - if kvversion == config.KVVersion1 { + switch secret.GroupVersion { + case "kv1": query = ".Data" - } else if kvversion == config.KVVersion2 { + case "kv2": query = ".Data.data" - } else { - err := fmt.Errorf("unsupported KV Version %v, supported versions are 1 and 2", kvversion) - return nil, err + case "db": + // For the database secrets engine backend we do not want to use the templating + default: + // Deprecated: The config setting KVVersion is deprecated and will be removed in a future release. + // You should instead use the GroupVersion field in the secret definition. + // Support using the legacy KVVersion config if GroupVersion is not set + switch kvversion { + case config.KVVersion1: + query = ".Data" + case config.KVVersion2: + query = ".Data.data" + } } - template := fmt.Sprintf(`{{- with secret "%s" -}}{{ %s.%s }}{{- end -}}`, secret.Group, query, secret.Key) - secretVaultAnnotations := map[string]string{ - fmt.Sprintf("vault.hashicorp.com/agent-inject-secret-%s", id): secret.Group, - fmt.Sprintf("vault.hashicorp.com/agent-inject-file-%s", id): fmt.Sprintf("%s/%s", secret.Group, secret.Key), - fmt.Sprintf("vault.hashicorp.com/agent-inject-template-%s", id): template, + if query != "" { + template := fmt.Sprintf(`{{- with secret "%s" -}}{{ %s.%s }}{{- end -}}`, secret.Group, query, secret.Key) + secretVaultAnnotations[fmt.Sprintf("vault.hashicorp.com/agent-inject-template-%s", id)] = template + } - return secretVaultAnnotations, nil + return secretVaultAnnotations } diff --git a/flytepropeller/pkg/webhook/vault_secret_manager.go b/flytepropeller/pkg/webhook/vault_secret_manager.go index 6ac2281f8..9a337483a 100644 --- a/flytepropeller/pkg/webhook/vault_secret_manager.go +++ b/flytepropeller/pkg/webhook/vault_secret_manager.go @@ -68,11 +68,7 @@ func (i VaultSecretManagerInjector) Inject(ctx context.Context, secret *coreIdl. "vault.hashicorp.com/agent-pre-populate-only": "true", } - secretVaultAnnotations, err := CreateVaultAnnotationsForSecret(secret, i.cfg.KVVersion) - // Creating annotations can break with an unsupported KVVersion - if err != nil { - return p, false, err - } + secretVaultAnnotations := CreateVaultAnnotationsForSecret(secret, i.cfg.KVVersion) p.ObjectMeta.Annotations = utils.UnionMaps(secretVaultAnnotations, commonVaultAnnotations, i.cfg.Annotations, p.ObjectMeta.Annotations) diff --git a/flytepropeller/pkg/webhook/vault_secret_manager_test.go b/flytepropeller/pkg/webhook/vault_secret_manager_test.go index fb4148aa9..52c88c196 100644 --- a/flytepropeller/pkg/webhook/vault_secret_manager_test.go +++ b/flytepropeller/pkg/webhook/vault_secret_manager_test.go @@ -45,7 +45,6 @@ func RetrieveUUID(annotations map[string]string) string { } func ExpectedKVv1(uuid string) *corev1.Pod { - // Injects uuid into expected output for KV v1 secrets expected := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -64,7 +63,6 @@ func ExpectedKVv1(uuid string) *corev1.Pod { } func ExpectedKVv2(uuid string) *corev1.Pod { - // Injects uuid into expected output for KV v2 secrets expected := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -82,8 +80,7 @@ func ExpectedKVv2(uuid string) *corev1.Pod { return expected } -func ExpectedKVv3(uuid string) *corev1.Pod { - // Injects uuid into expected output for KV v2 secrets +func ExpectedExtraAnnotation(uuid string) *corev1.Pod { expected := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -102,8 +99,7 @@ func ExpectedKVv3(uuid string) *corev1.Pod { return expected } -func ExpectedKVv4(uuid string) *corev1.Pod { - // Injects uuid into expected output for KV v2 secrets +func ExpectedExistingRoleAnnotation(uuid string) *corev1.Pod { expected := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -121,8 +117,7 @@ func ExpectedKVv4(uuid string) *corev1.Pod { return expected } -func ExpectedKVv5(uuid string) *corev1.Pod { - // Injects uuid into expected output for KV v2 secrets +func ExpectedConfigAnnotation(uuid string) *corev1.Pod { expected := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -140,6 +135,23 @@ func ExpectedKVv5(uuid string) *corev1.Pod { return expected } +func ExpectedDB(uuid string) *corev1.Pod { + expected := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "vault.hashicorp.com/agent-inject": "true", + "vault.hashicorp.com/secret-volume-path": "/etc/flyte/secrets", + "vault.hashicorp.com/role": "flyte", + "vault.hashicorp.com/agent-pre-populate-only": "true", + fmt.Sprintf("vault.hashicorp.com/agent-inject-secret-%s", uuid): "foo", + fmt.Sprintf("vault.hashicorp.com/agent-inject-file-%s", uuid): "foo/bar", + }, + }, + Spec: PodSpec, + } + return expected +} + func NewInputPod(annotations map[string]string) *corev1.Pod { // Need to create a new Pod for every test since annotations are otherwise appended to original reference object p := &corev1.Pod{ @@ -176,27 +188,35 @@ func TestVaultSecretManagerInjector_Inject(t *testing.T) { wantErr bool }{ { - name: "KVv1 Secret", + name: "KVv1 Secret Group Version argument overwrites config", args: args{ - cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion1}, - secret: inputSecret, - p: NewInputPod(map[string]string{}), + cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion2}, + secret: &coreIdl.Secret{ + Group: "foo", + Key: "bar", + GroupVersion: "kv1", + }, + p: NewInputPod(map[string]string{}), }, want: ExpectedKVv1, wantErr: false, }, { - name: "KVv2 Secret", + name: "KVv2 Secret Group Version argument overwrites config", args: args{ - cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion2}, - secret: inputSecret, - p: NewInputPod(map[string]string{}), + cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion1}, + secret: &coreIdl.Secret{ + Group: "foo", + Key: "bar", + GroupVersion: "kv2", + }, + p: NewInputPod(map[string]string{}), }, want: ExpectedKVv2, wantErr: false, }, { - name: "KVv3 Secret - extra annotations", + name: "Extra annotations from config are added", args: args{ cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion2, Annotations: map[string]string{ "vault.hashicorp.com/auth-config-type": "gce", @@ -204,11 +224,11 @@ func TestVaultSecretManagerInjector_Inject(t *testing.T) { secret: inputSecret, p: NewInputPod(map[string]string{}), }, - want: ExpectedKVv3, + want: ExpectedExtraAnnotation, wantErr: false, }, { - name: "KVv4 Secret - user override annotation", + name: "Already present annotation is not overwritten", args: args{ cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion2, Annotations: map[string]string{}}, secret: inputSecret, @@ -216,11 +236,11 @@ func TestVaultSecretManagerInjector_Inject(t *testing.T) { "vault.hashicorp.com/role": "my-role", }), }, - want: ExpectedKVv4, + want: ExpectedExistingRoleAnnotation, wantErr: false, }, { - name: "KVv5 Secret - system override annotation", + name: "Config annotation overwrites system default annotation", args: args{ cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion2, Annotations: map[string]string{ "vault.hashicorp.com/agent-pre-populate-only": "false", // override vault.hashicorp.com/agent-pre-populate-only @@ -228,18 +248,48 @@ func TestVaultSecretManagerInjector_Inject(t *testing.T) { secret: inputSecret, p: NewInputPod(map[string]string{}), }, - want: ExpectedKVv5, + want: ExpectedConfigAnnotation, wantErr: false, }, { - name: "Unsupported KV version", + name: "DB Secret backend enginge is supported", args: args{ - cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: 3}, - secret: inputSecret, - p: NewInputPod(map[string]string{}), + cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion1}, + secret: &coreIdl.Secret{ + Group: "foo", + Key: "bar", + GroupVersion: "db", + }, + p: NewInputPod(map[string]string{}), }, - want: nil, - wantErr: true, + want: ExpectedDB, + wantErr: false, + }, + { + name: "Legacy config option V1 is still supported", + args: args{ + cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion1}, + secret: &coreIdl.Secret{ + Group: "foo", + Key: "bar", + }, + p: NewInputPod(map[string]string{}), + }, + want: ExpectedKVv1, + wantErr: false, + }, + { + name: "Legacy config option V2 is still supported", + args: args{ + cfg: config.VaultSecretManagerConfig{Role: "flyte", KVVersion: config.KVVersion2}, + secret: &coreIdl.Secret{ + Group: "foo", + Key: "bar", + }, + p: NewInputPod(map[string]string{}), + }, + want: ExpectedKVv2, + wantErr: false, }, }