From c852f487a5e37cc532a636b0187a4bff4e61b83c Mon Sep 17 00:00:00 2001
From: Niels de Vos <ndevos@redhat.com>
Date: Thu, 28 Oct 2021 12:21:31 +0200
Subject: [PATCH] util: set defaults for Vault config before converting

When using UPPER_CASE formatting for the HashiCorp Vault KMS
configuration, a missing `VAULT_DESTROY_KEYS` will cause the option to
be set to "false". The default for the option is intended for be "true".

This is a difference in behaviour between the `vaultDestroyKeys` and
`VAULT_DESTROY_KEYS` options. Both should use a default of "true" when
the configuration does not set the option explicitly.

By setting the default options in the `standardVault` struct before
unmarshalling the configuration in it, the default values will be
retained for the missing configuration options.

Reported-by: Rachael George <rgeorge@redhat.com>
Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
 internal/kms/vault.go             |  4 ++--
 internal/kms/vault_tokens.go      | 11 ++++++++---
 internal/kms/vault_tokens_test.go | 13 +++++++++++++
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/internal/kms/vault.go b/internal/kms/vault.go
index d1e146538..6a2f2c744 100644
--- a/internal/kms/vault.go
+++ b/internal/kms/vault.go
@@ -43,7 +43,7 @@ const (
 	vaultDefaultRole           = "csi-kubernetes"
 	vaultDefaultNamespace      = ""
 	vaultDefaultPassphrasePath = ""
-	vaultDefaultCAVerify       = "true"
+	vaultDefaultCAVerify       = true
 	vaultDefaultDestroyKeys    = "true"
 )
 
@@ -208,7 +208,7 @@ func (vc *vaultConnection) initConnection(config map[string]interface{}) error {
 		keyContext[loss.KeyVaultNamespace] = vaultNamespace
 	}
 
-	verifyCA := vaultDefaultCAVerify // optional
+	verifyCA := strconv.FormatBool(vaultDefaultCAVerify) // optional
 	err = setConfigString(&verifyCA, config, "vaultCAVerify")
 	if errors.Is(err, errConfigOptionInvalid) {
 		return err
diff --git a/internal/kms/vault_tokens.go b/internal/kms/vault_tokens.go
index 46d7f1a50..d9eee0816 100644
--- a/internal/kms/vault_tokens.go
+++ b/internal/kms/vault_tokens.go
@@ -101,7 +101,6 @@ func (v *vaultTokenConf) convertStdVaultToCSIConfig(s *standardVault) {
 
 	// by default the CA should get verified, only when VaultSkipVerify is
 	// set, verification should be disabled
-	v.VaultCAVerify = vaultDefaultCAVerify
 	verify, err := strconv.ParseBool(s.VaultSkipVerify)
 	if err == nil {
 		v.VaultCAVerify = strconv.FormatBool(!verify)
@@ -124,8 +123,14 @@ func transformConfig(svMap map[string]interface{}) (map[string]interface{}, erro
 		return nil, fmt.Errorf("failed to convert config %T to JSON: %w", svMap, err)
 	}
 
-	// convert the JSON back to a standardVault struct
-	sv := &standardVault{}
+	// convert the JSON back to a standardVault struct, default values are
+	// set in case the configuration does not provide all options
+	sv := &standardVault{
+		VaultDestroyKeys: vaultDefaultDestroyKeys,
+		VaultNamespace:   vaultDefaultNamespace,
+		VaultSkipVerify:  strconv.FormatBool(!vaultDefaultCAVerify),
+	}
+
 	err = json.Unmarshal(data, sv)
 	if err != nil {
 		return nil, fmt.Errorf("failed to Unmarshal the vault configuration: %w", err)
diff --git a/internal/kms/vault_tokens_test.go b/internal/kms/vault_tokens_test.go
index df1e105f0..a398b7e80 100644
--- a/internal/kms/vault_tokens_test.go
+++ b/internal/kms/vault_tokens_test.go
@@ -19,6 +19,7 @@ package kms
 import (
 	"encoding/json"
 	"errors"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -208,6 +209,18 @@ func TestTransformConfig(t *testing.T) {
 	assert.Equal(t, config["vaultCAVerify"], "false")
 }
 
+func TestTransformConfigDefaults(t *testing.T) {
+	t.Parallel()
+	cm := make(map[string]interface{})
+	cm["KMS_PROVIDER"] = kmsTypeVaultTokens
+
+	config, err := transformConfig(cm)
+	require.NoError(t, err)
+	assert.Equal(t, config["encryptionKMSType"], cm["KMS_PROVIDER"])
+	assert.Equal(t, config["vaultDestroyKeys"], vaultDefaultDestroyKeys)
+	assert.Equal(t, config["vaultCAVerify"], strconv.FormatBool(vaultDefaultCAVerify))
+}
+
 func TestVaultTokensKMSRegistered(t *testing.T) {
 	t.Parallel()
 	_, ok := kmsManager.providers[kmsTypeVaultTokens]