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 <ndevos@redhat.com>

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"]

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"])
+	})
+}