util: move GetID() from EncryptionKMS to VolumeEncryption

There is no need for each EncryptionKMS to implement the same GetID()
function. We have a VolumeEncryption type that is more suitable for
keeping track of the KMS-ID that was used to get the configuration of
the KMS.

This does not change any metadata that is stored anywhere.

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

internal/rbd/encryption.go

@@ -208,7 +208,7 @@ func (ri *rbdImage) configureEncryption(kmsID string, credentials map[string]str
 		return err
 	}
 
-	ri.encryption, err = util.NewVolumeEncryption(kms)
+	ri.encryption, err = util.NewVolumeEncryption(kmsID, kms)
 
 	// if the KMS can not store the DEK itself, we'll store it in the
 	// metadata of the RBD image itself

internal/rbd/rbd_journal.go

@@ -234,7 +234,7 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er
 
 	kmsID := ""
 	if rv.isEncrypted() {
-		kmsID = rv.encryption.KMS.GetID()
+		kmsID = rv.encryption.GetID()
 	}
 
 	j, err := volJournal.Connect(rv.Monitors, rv.RadosNamespace, rv.conn.Creds)
@@ -411,7 +411,7 @@ func reserveVol(ctx context.Context, rbdVol *rbdVolume, rbdSnap *rbdSnapshot, cr
 
 	kmsID := ""
 	if rbdVol.isEncrypted() {
-		kmsID = rbdVol.encryption.KMS.GetID()
+		kmsID = rbdVol.encryption.GetID()
 	}
 
 	j, err := volJournal.Connect(rbdVol.Monitors, rbdVol.RadosNamespace, cr)

internal/util/crypto.go

@@ -57,6 +57,8 @@ type VolumeEncryption struct {
 	// dekStore that will be used, this can be the EncryptionKMS or a
 	// different object implementing the DEKStore interface.
 	dekStore DEKStore
+
+	id string
 }
 
 // NewVolumeEncryption creates a new instance of VolumeEncryption and
@@ -65,8 +67,18 @@ type VolumeEncryption struct {
 // Callers that receive a ErrDEKStoreNeeded error, should use
 // VolumeEncryption.SetDEKStore() to configure an alternative storage for the
 // DEKs.
-func NewVolumeEncryption(kms EncryptionKMS) (*VolumeEncryption, error) {
+func NewVolumeEncryption(id string, kms EncryptionKMS) (*VolumeEncryption, error) {
-	ve := &VolumeEncryption{KMS: kms}
+	kmsID := id
+	if kmsID == "" {
+		// if kmsID is not set, encryption is enabled, and the type is
+		// SecretsKMS
+		kmsID = defaultKMSType
+	}
+
+	ve := &VolumeEncryption{
+		id:  kmsID,
+		KMS: kms,
+	}
 
 	if kms.requiresDEKStore() == DEKStoreIntegrated {
 		dekStore, ok := kms.(DEKStore)
@@ -103,11 +115,14 @@ func (ve *VolumeEncryption) RemoveDEK(volumeID string) error {
 	return ve.dekStore.RemoveDEK(volumeID)
 }
 
+func (ve *VolumeEncryption) GetID() string {
+	return ve.id
+}
+
 // EncryptionKMS provides external Key Management System for encryption
 // passphrases storage.
 type EncryptionKMS interface {
 	Destroy()
-	GetID() string
 
 	// requiresDEKStore returns the DEKStoreType that is needed to be
 	// configure for the KMS. Nothing needs to be done when this function

internal/util/crypto_test.go

@@ -58,11 +58,11 @@ func TestKMSWorkflow(t *testing.T) {
 	kms, err := GetKMS("tenant", defaultKMSType, secrets)
 	assert.NoError(t, err)
 	require.NotNil(t, kms)
-	assert.Equal(t, defaultKMSType, kms.GetID())
 
-	ve, err := NewVolumeEncryption(kms)
+	ve, err := NewVolumeEncryption("", kms)
 	assert.NoError(t, err)
 	require.NotNil(t, ve)
+	assert.Equal(t, defaultKMSType, ve.GetID())
 
 	volumeID := "volume-id"
 

internal/util/kms.go

@@ -76,7 +76,7 @@ func GetKMS(tenant, kmsID string, secrets map[string]string) (EncryptionKMS, err
 			"section: %s", kmsID)
 	}
 
-	return kmsManager.buildKMS(kmsID, tenant, kmsConfig, secrets)
+	return kmsManager.buildKMS(tenant, kmsConfig, secrets)
 }
 
 // getKMSConfiguration reads the configuration file from the filesystem, or if
@@ -181,9 +181,9 @@ func getKMSProvider(config map[string]interface{}) (string, error) {
 // KMSInitializerArgs get passed to KMSInitializerFunc when a new instance of a
 // KMSProvider is initialized.
 type KMSInitializerArgs struct {
-	ID, Tenant string
+	Tenant  string
-	Config     map[string]interface{}
+	Config  map[string]interface{}
-	Secrets    map[string]string
+	Secrets map[string]string
 }
 
 // KMSInitializerFunc gets called when the KMSProvider needs to be
@@ -225,7 +225,10 @@ func RegisterKMSProvider(provider KMSProvider) bool {
 	return true
 }
 
-func (kf *kmsProviderList) buildKMS(kmsID, tenant string, config map[string]interface{}, secrets map[string]string) (EncryptionKMS, error) {
+// buildKMS creates a new KMSProvider instance, based on the configuration that
+// was passed. This uses getKMSProvider() internally to identify the
+// KMSProvider to instantiate.
+func (kf *kmsProviderList) buildKMS(tenant string, config map[string]interface{}, secrets map[string]string) (EncryptionKMS, error) {
 	providerName, err := getKMSProvider(config)
 	if err != nil {
 		return nil, err
@@ -238,7 +241,6 @@ func (kf *kmsProviderList) buildKMS(kmsID, tenant string, config map[string]inte
 	}
 
 	return provider.Initializer(KMSInitializerArgs{
-		ID:      kmsID,
 		Tenant:  tenant,
 		Config:  config,
 		Secrets: secrets,

internal/util/secretskms.go

@@ -58,11 +58,6 @@ func initSecretsKMS(secrets map[string]string) (EncryptionKMS, error) {
 	return SecretsKMS{passphrase: passphraseValue}, nil
 }
 
-// GetID is returning ID representing default KMS `default`.
-func (kms SecretsKMS) GetID() string {
-	return defaultKMSType
-}
-
 // Destroy frees all used resources.
 func (kms SecretsKMS) Destroy() {
 	// nothing to do
@@ -89,8 +84,6 @@ func (kms SecretsKMS) RemoveDEK(key string) error {
 // Data-Encryption-Key (DEK) in the metadata of the volume.
 type SecretsMetadataKMS struct {
 	SecretsKMS
-
-	encryptionKMSID string
 }
 
 var _ = RegisterKMSProvider(KMSProvider{
@@ -114,16 +107,10 @@ func initSecretsMetadataKMS(args KMSInitializerArgs) (EncryptionKMS, error) {
 
 	smKMS := SecretsMetadataKMS{}
 	smKMS.SecretsKMS = sKMS
-	smKMS.encryptionKMSID = args.ID
 
 	return smKMS, nil
 }
 
-// GetID is returning ID representing the SecretsMetadataKMS.
-func (kms SecretsMetadataKMS) GetID() string {
-	return kms.encryptionKMSID
-}
-
 // Destroy frees all used resources.
 func (kms SecretsMetadataKMS) Destroy() {
 	kms.SecretsKMS.Destroy()

internal/util/secretskms_test.go

@@ -42,7 +42,6 @@ func TestGenerateCipher(t *testing.T) {
 
 func TestInitSecretsMetadataKMS(t *testing.T) {
 	args := KMSInitializerArgs{
-		ID:      "secrets-metadata-unit-test",
 		Tenant:  "tenant",
 		Config:  nil,
 		Secrets: map[string]string{},
@@ -59,7 +58,6 @@ func TestInitSecretsMetadataKMS(t *testing.T) {
 	kms, err = initSecretsMetadataKMS(args)
 	assert.NoError(t, err)
 	require.NotNil(t, kms)
-	assert.Equal(t, "secrets-metadata-unit-test", kms.GetID())
 	assert.Equal(t, DEKStoreMetadata, kms.requiresDEKStore())
 }
 
@@ -68,7 +66,6 @@ func TestWorkflowSecretsMetadataKMS(t *testing.T) {
 		encryptionPassphraseKey: "my-passphrase-from-kubernetes",
 	}
 	args := KMSInitializerArgs{
-		ID:      "secrets-metadata-unit-test",
 		Tenant:  "tenant",
 		Config:  nil,
 		Secrets: secrets,

internal/util/vault.go

@@ -71,10 +71,9 @@ Example JSON structure in the KMS config is,
 */
 
 type vaultConnection struct {
-	EncryptionKMSID string
+	secrets     loss.Secrets
-	secrets         loss.Secrets
+	vaultConfig map[string]interface{}
-	vaultConfig     map[string]interface{}
+	keyContext  map[string]string
-	keyContext      map[string]string
 }
 
 type VaultKMS struct {
@@ -114,12 +113,10 @@ func setConfigString(option *string, config map[string]interface{}, key string)
 // vc.connectVault().
 //
 // nolint:gocyclo // iterating through many config options, not complex at all.
-func (vc *vaultConnection) initConnection(kmsID string, config map[string]interface{}) error {
+func (vc *vaultConnection) initConnection(config map[string]interface{}) error {
 	vaultConfig := make(map[string]interface{})
 	keyContext := make(map[string]string)
 
-	vc.EncryptionKMSID = kmsID
-
 	firstInit := (vc.vaultConfig == nil)
 
 	vaultAddress := "" // required
@@ -268,7 +265,7 @@ var _ = RegisterKMSProvider(KMSProvider{
 // InitVaultKMS returns an interface to HashiCorp Vault KMS.
 func initVaultKMS(args KMSInitializerArgs) (EncryptionKMS, error) {
 	kms := &VaultKMS{}
-	err := kms.initConnection(args.ID, args.Config)
+	err := kms.initConnection(args.Config)
 	if err != nil {
 		return nil, fmt.Errorf("failed to initialize Vault connection: %w", err)
 	}
@@ -328,11 +325,6 @@ func initVaultKMS(args KMSInitializerArgs) (EncryptionKMS, error) {
 	return kms, nil
 }
 
-// GetID is returning correlation ID to KMS configuration.
-func (vc *vaultConnection) GetID() string {
-	return vc.EncryptionKMSID
-}
-
 // FetchDEK returns passphrase from Vault. The passphrase is stored in a
 // data.data.passphrase structure.
 func (kms *VaultKMS) FetchDEK(key string) (string, error) {

internal/util/vault_tokens.go

@@ -199,7 +199,7 @@ func initVaultTokensKMS(args KMSInitializerArgs) (EncryptionKMS, error) {
 	}
 
 	kms := &VaultTokensKMS{}
-	err = kms.initConnection(args.ID, args.Config)
+	err = kms.initConnection(args.Config)
 	if err != nil {
 		return nil, fmt.Errorf("failed to initialize Vault connection: %w", err)
 	}
@@ -263,7 +263,7 @@ func initVaultTokensKMS(args KMSInitializerArgs) (EncryptionKMS, error) {
 // secrets. This method can be called multiple times, i.e. to override
 // configuration options from tenants.
 func (kms *VaultTokensKMS) parseConfig(config map[string]interface{}) error {
-	err := kms.initConnection(kms.EncryptionKMSID, config)
+	err := kms.initConnection(config)
 	if err != nil {
 		return err
 	}

internal/util/vault_tokens_test.go

@@ -78,7 +78,6 @@ func TestInitVaultTokensKMS(t *testing.T) {
 	}
 
 	args := KMSInitializerArgs{
-		ID:      "vault-tokens-config",
 		Tenant:  "bob",
 		Config:  make(map[string]interface{}),
 		Secrets: nil,