From 1f012004a68ef3b990d047f4310da7c6e15dc26d Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Tue, 1 Mar 2022 11:45:37 +0100 Subject: [PATCH] util: configure tenants vaultAuthNamespace if not set When a tenant provides a configuration that includes the `vaultNamespace` option, the `vaultAuthNamespace` option is still taken from the global configuration. This is not wanted in all cases, as the `vaultAuthNamespace` option defauls to the `vaultNamespace` option which the tenant may want to override as well. The following behaviour is now better defined: 1. no `vaultAuthNamespace` in the global configuration: A tenant can override the `vaultNamespace` option and that will also set the `vaultAuthNamespace` option to the same value. 2. `vaultAuthNamespace` and `vaultNamespace` in the global configuration: When both options are set to different values in the global configuration, the tenant `vaultNamespace` option will not override the global `vaultAuthNamespace` option. The tenant can configure `vaultAuthNamespace` with a different value if required. Signed-off-by: Niels de Vos --- internal/kms/vault_tokens.go | 55 ++++++++++++++++++ internal/kms/vault_tokens_test.go | 93 +++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+) diff --git a/internal/kms/vault_tokens.go b/internal/kms/vault_tokens.go index af604372b..5c7c96e90 100644 --- a/internal/kms/vault_tokens.go +++ b/internal/kms/vault_tokens.go @@ -27,6 +27,7 @@ import ( "github.com/ceph/ceph-csi/internal/util/k8s" "github.com/hashicorp/vault/api" + loss "github.com/libopenstorage/secrets" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -596,9 +597,63 @@ func (vtc *vaultTenantConnection) parseTenantConfig() (map[string]interface{}, e return nil, nil } + vtc.setTenantAuthNamespace(config) + return config, nil } +// setTenantAuthNamespace configures the vaultAuthNamespace for the tenant. +// vaultAuthNamespace defaults to vaultNamespace from the global configuration, +// even if the tenant has vaultNamespace configured. Users expect to have the +// vaultAuthNamespace updated when they configure vaultNamespace, if +// vaultAuthNamespace was not explicitly set in the global configuration. +func (vtc *vaultTenantConnection) setTenantAuthNamespace(tenantConfig map[string]interface{}) { + vaultAuthNamespace, ok := vtc.keyContext[loss.KeyVaultNamespace] + if !ok { + // nothing to do, global connection config does not have the + // vaultAuthNamespace set + return + } + + vaultNamespace, ok := vtc.vaultConfig[api.EnvVaultNamespace] + if !ok { + // nothing to do, global connection config does not have the + // vaultNamespace set, not overriding vaultAuthNamespace with + // vaultNamespace from the tenant + return + } + + if vaultAuthNamespace != vaultNamespace { + // vaultAuthNamespace and vaultNamespace have been configured + // differently in the global connection. Not going to override + // those pre-defined options if the tenantConfig does not have + // them set. + return + } + + // if we reached here, we need to make sure that the vaultAuthNamespace + // gets configured for the tenant, in case the tenant config has + // vaultNamespace set + + _, ok = tenantConfig["vaultAuthNamespace"] + if ok { + // the tenant already has vaultAuthNamespace configured, no + // action needed + return + } + + tenantNamespace, ok := tenantConfig["vaultNamespace"] + if !ok { + // the tenant does not have vaultNamespace configured, no need + // to set vaultAuthNamespace either + return + } + + // the tenant has vaultNamespace configured, use that for + // vaultAuthNamespace as well + tenantConfig["vaultAuthNamespace"] = tenantNamespace +} + // fetchTenantConfig fetches the configuration for the tenant if it exists. func fetchTenantConfig(config map[string]interface{}, tenant string) (map[string]interface{}, bool) { tenantsMap, ok := config["tenants"] diff --git a/internal/kms/vault_tokens_test.go b/internal/kms/vault_tokens_test.go index 63093ebcb..abf9cc85e 100644 --- a/internal/kms/vault_tokens_test.go +++ b/internal/kms/vault_tokens_test.go @@ -235,3 +235,96 @@ func TestVaultTokensKMSRegistered(t *testing.T) { _, ok := kmsManager.providers[kmsTypeVaultTokens] assert.True(t, ok) } + +func TestSetTenantAuthNamespace(t *testing.T) { + t.Parallel() + + vaultNamespace := "tenant" + + t.Run("override vaultAuthNamespace", func(tt *testing.T) { + tt.Parallel() + + kms := &vaultTenantConnection{} + kms.keyContext = map[string]string{ + loss.KeyVaultNamespace: "global", + } + kms.vaultConfig = map[string]interface{}{ + api.EnvVaultNamespace: "global", + } + + config := map[string]interface{}{ + "vaultNamespace": vaultNamespace, + } + + kms.setTenantAuthNamespace(config) + + assert.Equal(tt, vaultNamespace, config["vaultAuthNamespace"]) + }) + + t.Run("inherit vaultAuthNamespace", func(tt *testing.T) { + tt.Parallel() + + vaultAuthNamespace := "configured" + + kms := &vaultTenantConnection{} + kms.keyContext = map[string]string{ + loss.KeyVaultNamespace: vaultAuthNamespace, + } + kms.vaultConfig = map[string]interface{}{ + api.EnvVaultNamespace: "global", + } + + config := map[string]interface{}{ + "vaultNamespace": vaultNamespace, + } + + kms.setTenantAuthNamespace(config) + + // when inheriting from the global config, the config of the + // tenant should not have vaultAuthNamespace configured + assert.Equal(tt, nil, config["vaultAuthNamespace"]) + }) + + t.Run("unset vaultAuthNamespace", func(tt *testing.T) { + tt.Parallel() + + kms := &vaultTenantConnection{} + kms.keyContext = map[string]string{ + // no vaultAuthNamespace configured + } + kms.vaultConfig = map[string]interface{}{ + api.EnvVaultNamespace: "global", + } + + config := map[string]interface{}{ + "vaultNamespace": vaultNamespace, + } + + kms.setTenantAuthNamespace(config) + + // global vaultAuthNamespace is not set, tenant + // vaultAuthNamespace will be configured as vaultNamespace by + // default + assert.Equal(tt, nil, config["vaultAuthNamespace"]) + }) + + t.Run("no vaultNamespace", func(tt *testing.T) { + tt.Parallel() + + kms := &vaultTenantConnection{} + kms.keyContext = map[string]string{ + // no vaultAuthNamespace configured + } + kms.vaultConfig = map[string]interface{}{ + // no vaultNamespace configured + } + + config := map[string]interface{}{ + // no tenant namespaces configured + } + + kms.setTenantAuthNamespace(config) + + assert.Equal(tt, nil, config["vaultAuthNamespace"]) + }) +}