From f6894909d77ae03648094baa4c0410ceb3a7369b Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Fri, 4 Feb 2022 14:36:19 +0100 Subject: [PATCH] util: use `vaultNamespace` if `vaultAuthNamespace` is not set When a tenant configures `vaultNamespace` in their own ConfigMap, it is not applied to the Vault configuration, unless `vaultAuthNamespace` is set as well. This is unexpected, as the `vaultAuthNamespace` usually is something configured globally, and not per tenant. The `vaultAuthNamespace` is an advanced option, that is often not needed to be configured. Only when tenants have to configure their own `vaultNamespace`, it is possible that they need to use a different `vaultAuthNamespace`. The default for the `vaultAuthNamespace` is now the `vaultNamespace` value from the global configuration. Tenants can still set it to something else in their own ConfigMap if needed. Note that Hashicorp Vault Namespaces are only functional in the Enterprise version of the product. Therefor this can not be tested in the Ceph-CSI e2e with the Open Source version of Vault. Fixes: https://bugzilla.redhat.com/2050056 Reported-by: Rachael George Signed-off-by: Niels de Vos --- internal/kms/vault.go | 6 +++++- internal/kms/vault_tokens_test.go | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/kms/vault.go b/internal/kms/vault.go index 6abe6f0f5..14f9ff974 100644 --- a/internal/kms/vault.go +++ b/internal/kms/vault.go @@ -192,6 +192,11 @@ func (vc *vaultConnection) initConnection(config map[string]interface{}) error { if errors.Is(err, errConfigOptionInvalid) { return err } + // set the option if the value was not invalid + if firstInit || !errors.Is(err, errConfigOptionMissing) { + keyContext[loss.KeyVaultNamespace] = vaultNamespace + } + vaultAuthNamespace := "" err = setConfigString(&vaultAuthNamespace, config, "vaultAuthNamespace") if errors.Is(err, errConfigOptionInvalid) { @@ -205,7 +210,6 @@ func (vc *vaultConnection) initConnection(config map[string]interface{}) error { // set the option if the value was not invalid if firstInit || !errors.Is(err, errConfigOptionMissing) { vaultConfig[api.EnvVaultNamespace] = vaultAuthNamespace - keyContext[loss.KeyVaultNamespace] = vaultNamespace } verifyCA := strconv.FormatBool(vaultDefaultCAVerify) // optional diff --git a/internal/kms/vault_tokens_test.go b/internal/kms/vault_tokens_test.go index a398b7e80..63093ebcb 100644 --- a/internal/kms/vault_tokens_test.go +++ b/internal/kms/vault_tokens_test.go @@ -23,6 +23,8 @@ import ( "strings" "testing" + "github.com/hashicorp/vault/api" + loss "github.com/libopenstorage/secrets" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -41,6 +43,8 @@ func TestParseConfig(t *testing.T) { // fill default options (normally done in initVaultTokensKMS) config["vaultAddress"] = "https://vault.default.cluster.svc" + config["vaultNamespace"] = "default" + config["vaultAuthNamespace"] = "company-sso" config["tenantConfigName"] = vaultTokensDefaultConfigName // parsing with all required options @@ -55,12 +59,17 @@ func TestParseConfig(t *testing.T) { // tenant "bob" uses a different kms.ConfigName bob := make(map[string]interface{}) bob["tenantConfigName"] = "the-config-from-bob" + bob["vaultNamespace"] = "bobs-place" err = vtc.parseConfig(bob) switch { case err != nil: t.Errorf("unexpected error: %s", err) case vtc.ConfigName != "the-config-from-bob": t.Errorf("ConfigName contains unexpected value: %s", vtc.ConfigName) + case vtc.vaultConfig[api.EnvVaultNamespace] != "company-sso": + t.Errorf("EnvVaultNamespace contains unexpected value: %s", vtc.vaultConfig[api.EnvVaultNamespace]) + case vtc.keyContext[loss.KeyVaultNamespace] != "bobs-place": + t.Errorf("KeyVaultNamespace contains unexpected value: %s", vtc.keyContext[loss.KeyVaultNamespace]) } }