util: make parseTenantConfig() usable for modular KMSs

parseTenantConfig() only allowed configuring a defined set of options,
and KMSs were not able to re-use the implementation. Now, the function
parses the ConfigMap from the Tenants Namespace and returns a map with
options that the KMS supports.

The map that parseTenantConfig() returns can be inspected by the KMS,
and applied to the vaultTenantConnection type by calling parseConfig().

Signed-off-by: Niels de Vos <ndevos@redhat.com>

examples/kms/vault/tenant-sa-admin.yaml

@@ -82,7 +82,7 @@ spec:
             - name: K8S_HOST
               value: https://kubernetes.default.svc.cluster.local
             - name: PLUGIN_ROLE
-              value: csi-kubernetes
+              value: ceph-csi-tenant
             - name: TENANT_SA_NAME
               value: ceph-csi-vault-sa
             - name: TENANT_NAMESPACE

examples/kms/vault/tenant-sa.yaml

@@ -20,3 +20,4 @@ metadata:
   name: ceph-csi-kms-config
 data:
   vaultBackendPath: tenant
+  vaultRole: ceph-csi-tenant

internal/util/vault_sa.go

@@ -98,6 +98,9 @@ func initVaultTenantSA(args KMSInitializerArgs) (EncryptionKMS, error) {
 	}
 
 	kms := &VaultTenantSA{}
+	kms.vaultTenantConnection.init()
+	kms.tenantConfigOptionFilter = isTenantSAConfigOption
+
 	err = kms.initConnection(config)
 	if err != nil {
 		return nil, fmt.Errorf("failed to initialize Vault connection: %w", err)
@@ -107,13 +110,8 @@ func initVaultTenantSA(args KMSInitializerArgs) (EncryptionKMS, error) {
 	kms.ConfigName = vaultTokensDefaultConfigName
 	kms.tenantSAName = vaultTenantSAName
 
-	// TODO: should this be configurable per tenant?
+	// "vaultRole" is configurable per tenant
-	vaultRole := vaultDefaultRole
+	kms.vaultConfig[vault.AuthKubernetesRole] = vaultDefaultRole
-	err = setConfigString(&vaultRole, args.Config, "vaultRole")
-	if errors.Is(err, errConfigOptionInvalid) {
-		return nil, err
-	}
-	kms.vaultConfig[vault.AuthKubernetesRole] = vaultRole
 
 	err = kms.parseConfig(config)
 	if err != nil {
@@ -122,25 +120,9 @@ func initVaultTenantSA(args KMSInitializerArgs) (EncryptionKMS, error) {
 
 	// fetch the configuration for the tenant
 	if args.Tenant != "" {
-		kms.Tenant = args.Tenant
+		err = kms.configureTenant(config, args.Tenant)
-		tenantConfig, found := fetchTenantConfig(config, args.Tenant)
-		if found {
-			// override connection details from the tenant
-			err = kms.parseConfig(tenantConfig)
-			if err != nil {
-				return nil, err
-			}
-		}
-
-		err = kms.parseTenantConfig()
 		if err != nil {
-			return nil, fmt.Errorf("failed to parse config for tenant: %w", err)
+			return nil, err
-		}
-
-		err = kms.setServiceAccountName(config)
-		if err != nil {
-			return nil, fmt.Errorf("failed to set the ServiceAccount name from %s for tenant (%s): %w",
-				kms.ConfigName, kms.Tenant, err)
 		}
 	}
 
@@ -173,6 +155,81 @@ func (kms *VaultTenantSA) Destroy() {
 	kms.vaultTenantConnection.Destroy()
 }
 
+func (kms *VaultTenantSA) configureTenant(config map[string]interface{}, tenant string) error {
+	kms.Tenant = tenant
+	tenantConfig, found := fetchTenantConfig(config, tenant)
+	if found {
+		// override connection details from the tenant
+		err := kms.parseConfig(tenantConfig)
+		if err != nil {
+			return err
+		}
+	}
+
+	// get the ConfigMap from the Tenant and apply the options
+	tenantConfig, err := kms.parseTenantConfig()
+	if err != nil {
+		return fmt.Errorf("failed to parse config for tenant: %w", err)
+	} else if tenantConfig != nil {
+		err = kms.parseConfig(tenantConfig)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// parseConfig calls vaultTenantConnection.parseConfig() and also set
+// additional config options specific to VaultTenantSA. This function is called
+// multiple times, for the different nested configuration layers.
+// parseTenantConfig() calls this as well, with a reduced set of options,
+// filtered by isTenantConfigOption().
+func (kms *VaultTenantSA) parseConfig(config map[string]interface{}) error {
+	err := kms.vaultTenantConnection.parseConfig(config)
+	if err != nil {
+		return err
+	}
+
+	err = kms.setServiceAccountName(config)
+	if err != nil {
+		return fmt.Errorf("failed to set the ServiceAccount name from %s for tenant (%s): %w",
+			kms.ConfigName, kms.Tenant, err)
+	}
+
+	// default vaultRole is set in initVaultTenantSA()
+	var vaultRole string
+	err = setConfigString(&vaultRole, config, "vaultRole")
+	if errors.Is(err, errConfigOptionInvalid) {
+		return err
+	} else if err == nil {
+		kms.vaultConfig[vault.AuthKubernetesRole] = vaultRole
+	}
+
+	return nil
+}
+
+// isTenantSAConfigOption is used by vaultTenantConnection.parseTenantConfig()
+// to filter options that should not be set by the configuration in the tenants
+// ConfigMap. Options that are allowed to be set, will return true, options
+// that are filtered return false.
+func isTenantSAConfigOption(opt string) bool {
+	// standard vaultTenantConnection options are accepted
+	if isTenantConfigOption(opt) {
+		return true
+	}
+
+	// additional options for VaultTenantSA
+	switch opt {
+	case "tenantSAName":
+	case "vaultRole":
+	default:
+		return false
+	}
+
+	return true
+}
+
 // setServiceAccountName stores the name of the ServiceAccount in the
 // configuration if it has been set in the options.
 func (kms *VaultTenantSA) setServiceAccountName(config map[string]interface{}) error {

internal/util/vault_tokens.go

@@ -176,6 +176,12 @@ type vaultTenantConnection struct {
 	Tenant string
 	// ConfigName is the name of the ConfigMap in the Tenants Kubernetes Namespace
 	ConfigName string
+
+	// tenantConfigOptionFilter ise used to filter configuration options
+	// for the KMS that are provided by the ConfigMap in the Tenants
+	// Namespace. It defaults to isTenantConfigOption() as setup by the
+	// init() function.
+	tenantConfigOptionFilter func(string) bool
 }
 
 type VaultTokensKMS struct {
@@ -205,6 +211,7 @@ func initVaultTokensKMS(args KMSInitializerArgs) (EncryptionKMS, error) {
 	}
 
 	kms := &VaultTokensKMS{}
+	kms.vaultTenantConnection.init()
 	err = kms.initConnection(config)
 	if err != nil {
 		return nil, fmt.Errorf("failed to initialize Vault connection: %w", err)
@@ -227,25 +234,9 @@ func initVaultTokensKMS(args KMSInitializerArgs) (EncryptionKMS, error) {
 
 	// fetch the configuration for the tenant
 	if args.Tenant != "" {
-		kms.Tenant = args.Tenant
+		err = kms.configureTenant(config, args.Tenant)
-		tenantConfig, found := fetchTenantConfig(config, args.Tenant)
-		if found {
-			// override connection details from the tenant
-			err = kms.parseConfig(tenantConfig)
-			if err != nil {
-				return nil, err
-			}
-		}
-
-		err = kms.parseTenantConfig()
 		if err != nil {
-			return nil, fmt.Errorf("failed to parse config for tenant: %w", err)
+			return nil, err
-		}
-
-		err = kms.setTokenName(tenantConfig)
-		if err != nil {
-			return nil, fmt.Errorf("failed to set the TokenName from %s for tenant (%s): %w",
-				kms.ConfigName, kms.Tenant, err)
 		}
 	}
 
@@ -269,6 +260,48 @@ func initVaultTokensKMS(args KMSInitializerArgs) (EncryptionKMS, error) {
 	return kms, nil
 }
 
+func (kms *VaultTokensKMS) configureTenant(config map[string]interface{}, tenant string) error {
+	kms.Tenant = tenant
+	tenantConfig, found := fetchTenantConfig(config, tenant)
+	if found {
+		// override connection details from the tenant
+		err := kms.parseConfig(tenantConfig)
+		if err != nil {
+			return err
+		}
+
+		err = kms.setTokenName(tenantConfig)
+		if err != nil {
+			return fmt.Errorf("failed to set the TokenName for tenant (%s): %w",
+				kms.Tenant, err)
+		}
+	}
+
+	// get the ConfigMap from the Tenant and apply the options
+	tenantConfig, err := kms.parseTenantConfig()
+	if err != nil {
+		return fmt.Errorf("failed to parse config for tenant: %w", err)
+	} else if tenantConfig != nil {
+		err = kms.parseConfig(tenantConfig)
+		if err != nil {
+			return fmt.Errorf("failed to parse config (%s) for tenant (%s): %w",
+				kms.ConfigName, kms.Tenant, err)
+		}
+
+		err = kms.setTokenName(tenantConfig)
+		if err != nil {
+			return fmt.Errorf("failed to set the TokenName from %s for tenant (%s): %w",
+				kms.ConfigName, kms.Tenant, err)
+		}
+	}
+
+	return nil
+}
+
+func (vtc *vaultTenantConnection) init() {
+	vtc.tenantConfigOptionFilter = isTenantConfigOption
+}
+
 // parseConfig updates the kms.vaultConfig with the options from config and
 // secrets. This method can be called multiple times, i.e. to override
 // configuration options from tenants.
@@ -495,9 +528,9 @@ func isTenantConfigOption(opt string) bool {
 // parseTenantConfig gets the optional ConfigMap from the Tenants namespace,
 // and applies the allowable options (see isTenantConfigOption) to the KMS
 // configuration.
-func (vtc *vaultTenantConnection) parseTenantConfig() error {
+func (vtc *vaultTenantConnection) parseTenantConfig() (map[string]interface{}, error) {
 	if vtc.Tenant == "" || vtc.ConfigName == "" {
-		return nil
+		return nil, nil
 	}
 
 	// fetch the ConfigMap from the tenants namespace
@@ -506,9 +539,9 @@ func (vtc *vaultTenantConnection) parseTenantConfig() error {
 		vtc.ConfigName, metav1.GetOptions{})
 	if apierrs.IsNotFound(err) {
 		// the tenant did not (re)configure any options
-		return nil
+		return nil, nil
 	} else if err != nil {
-		return fmt.Errorf("failed to get config (%s) for tenant (%s): %w",
+		return nil, fmt.Errorf("failed to get config (%s) for tenant (%s): %w",
 			vtc.ConfigName, vtc.Tenant, err)
 	}
 
@@ -516,23 +549,16 @@ func (vtc *vaultTenantConnection) parseTenantConfig() error {
 	// that a tenant may (re)configure
 	config := make(map[string]interface{})
 	for k, v := range cm.Data {
-		if isTenantConfigOption(k) {
+		if vtc.tenantConfigOptionFilter(k) {
 			config[k] = v
 		} // else: silently ignore the option
 	}
 	if len(config) == 0 {
 		// no options configured by the tenant
-		return nil
+		return nil, nil
 	}
 
-	// apply the configuration options from the tenant
+	return config, nil
-	err = vtc.parseConfig(config)
-	if err != nil {
-		return fmt.Errorf("failed to parse config (%s) for tenant (%s): %w",
-			vtc.ConfigName, vtc.Tenant, err)
-	}
-
-	return nil
 }
 
 // fetchTenantConfig fetches the configuration for the tenant if it exists.