From d941e5abacff96374a38efe3ff70aae19c33c547 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Tue, 13 Jul 2021 12:59:29 +0200 Subject: [PATCH] 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 --- examples/kms/vault/tenant-sa-admin.yaml | 2 +- examples/kms/vault/tenant-sa.yaml | 1 + internal/util/vault_sa.go | 107 ++++++++++++++++++------ internal/util/vault_tokens.go | 90 +++++++++++++------- 4 files changed, 142 insertions(+), 58 deletions(-) diff --git a/examples/kms/vault/tenant-sa-admin.yaml b/examples/kms/vault/tenant-sa-admin.yaml index b0e2eec4e..5f8fd1473 100644 --- a/examples/kms/vault/tenant-sa-admin.yaml +++ b/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 diff --git a/examples/kms/vault/tenant-sa.yaml b/examples/kms/vault/tenant-sa.yaml index 3445dc9cc..5f6f20891 100644 --- a/examples/kms/vault/tenant-sa.yaml +++ b/examples/kms/vault/tenant-sa.yaml @@ -20,3 +20,4 @@ metadata: name: ceph-csi-kms-config data: vaultBackendPath: tenant + vaultRole: ceph-csi-tenant diff --git a/internal/util/vault_sa.go b/internal/util/vault_sa.go index 04df9ed9b..8ad379a79 100644 --- a/internal/util/vault_sa.go +++ b/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 := vaultDefaultRole - err = setConfigString(&vaultRole, args.Config, "vaultRole") - if errors.Is(err, errConfigOptionInvalid) { - return nil, err - } - kms.vaultConfig[vault.AuthKubernetesRole] = vaultRole + // "vaultRole" is configurable per tenant + kms.vaultConfig[vault.AuthKubernetesRole] = vaultDefaultRole 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 - 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() + err = kms.configureTenant(config, args.Tenant) if err != nil { - return nil, fmt.Errorf("failed to parse config for tenant: %w", 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) + return nil, 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 { diff --git a/internal/util/vault_tokens.go b/internal/util/vault_tokens.go index e3e2a5656..6e3569a6d 100644 --- a/internal/util/vault_tokens.go +++ b/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 - 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() + err = kms.configureTenant(config, args.Tenant) if err != nil { - return nil, fmt.Errorf("failed to parse config for tenant: %w", 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) + return nil, 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 - 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 + return config, nil } // fetchTenantConfig fetches the configuration for the tenant if it exists.