Fix issue with multiple gkms services

staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/encryptionconfig_test.go

@@ -220,7 +220,7 @@ type testKMSStorage struct {
 	data *map[string]string
 }
 
-func (t *testKMSStorage) Setup() error {
+func (t *testKMSStorage) Setup(_ string) error {
 	_, err := t.GetAllDEKs()
 	if err != nil {
 		cfg := &(map[string]string{})
@@ -271,6 +271,13 @@ func (t *testKMSService) Encrypt(data []byte) (string, error) {
 	return base64.StdEncoding.EncodeToString(data), nil
 }
 
+func (t *testKMSService) GetUniqueID() string {
+	return ""
+}
+
+var _ value.KMSStorage = &testKMSStorage{}
+var _ value.KMSService = &testKMSService{}
+
 func TestEncryptionProviderConfigCorrect(t *testing.T) {
 	// Create a mock kmsFactory
 	kmsFactory := transformhelpers.NewKMSFactoryWithStorageAndGKMS(&testKMSStorage{}, &testKMSService{})

staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/kms/google.go

@@ -27,7 +27,7 @@ import (
 
 const defaultGKMSKeyRing = "google-kubernetes"
 
-type gkmsTransformer struct {
+type gkmsService struct {
 	parentName      string
 	cloudkmsService *cloudkms.Service
 }
@@ -92,14 +92,14 @@ func NewGoogleKMSService(projectID, location, keyRing, cryptoKey string, cloud *
 	}
 	parentName = parentName + "/cryptoKeys/" + cryptoKey
 
-	return &gkmsTransformer{
+	return &gkmsService{
 		parentName:      parentName,
 		cloudkmsService: cloud.CloudkmsService,
 	}, nil
 }
 
 // Decrypt decrypts a base64 representation of encrypted bytes.
-func (t *gkmsTransformer) Decrypt(data string) ([]byte, error) {
+func (t *gkmsService) Decrypt(data string) ([]byte, error) {
 	resp, err := t.cloudkmsService.Projects.Locations.KeyRings.CryptoKeys.
 		Decrypt(t.parentName, &cloudkms.DecryptRequest{
 			Ciphertext: data,
@@ -111,7 +111,7 @@ func (t *gkmsTransformer) Decrypt(data string) ([]byte, error) {
 }
 
 // Encrypt encrypts bytes, and returns base64 representation of the ciphertext.
-func (t *gkmsTransformer) Encrypt(data []byte) (string, error) {
+func (t *gkmsService) Encrypt(data []byte) (string, error) {
 	resp, err := t.cloudkmsService.Projects.Locations.KeyRings.CryptoKeys.
 		Encrypt(t.parentName, &cloudkms.EncryptRequest{
 			Plaintext: base64.StdEncoding.EncodeToString(data),
@@ -121,3 +121,9 @@ func (t *gkmsTransformer) Encrypt(data []byte) (string, error) {
 	}
 	return resp.Ciphertext, nil
 }
+
+func (t *gkmsService) GetUniqueID() string {
+	return "gkms/" + t.parentName
+}
+
+var _ value.KMSService = &gkmsService{}

staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/kms/kms.go

@@ -42,7 +42,7 @@ type kmsTransformer struct {
 // NewKMSTransformer returns a transformer which implements a KEK-DEK based envelope encryption scheme.
 // It uses kmsService to communicate with the KEK store, and storage to communicate with the DEK store.
 func NewKMSTransformer(kmsService value.KMSService, storage value.KMSStorage) (value.Transformer, error) {
-	err := storage.Setup()
+	err := storage.Setup(kmsService.GetUniqueID())
 	if err != nil {
 		return nil, err
 	}

staging/src/k8s.io/apiserver/pkg/storage/value/transformer.go

@@ -35,16 +35,23 @@ type Context interface {
 // KMSStorage allows storing and retreiving Data-Encryption-Keys (DEKs) for implementing envelope encryption
 // based transformers.
 type KMSStorage interface {
-	Setup() error
+	// Setup creates required objects on disk before they get accessed. Object name is passed as argument.
+	Setup(name string) error
+	// GetAllDEKs lists all available DEKs.
 	GetAllDEKs() (map[string]string, error)
+	// StoreNewDEK creates a unique name for the DEK and makes it the primary DEK transactionally.
 	StoreNewDEK(encDEK string) error
 }
 
 // KMSService allows encrypting and decrypting secrets using an external service, which provides a
 // Key-Encryption-Key (KEK). This is needed to implement envelope encryption based transformers.
 type KMSService interface {
+	// Decrypt a given data string using KEK.
 	Decrypt(data string) ([]byte, error)
+	// Encrypte bytes using KEK.
 	Encrypt(data []byte) (string, error)
+	// Get a unique ID for this service for identification and separation of resources in database.
+	GetUniqueID() string
 }
 
 // Transformer allows a value to be transformed before being read from or written to the underlying store. The methods

staging/src/k8s.io/apiserver/pkg/storage/value/transformhelpers/key_store.go

@@ -42,6 +42,7 @@ const (
 
 type keyStore struct {
 	configmaps configmap.Registry
+	name       string
 	lock       sync.RWMutex
 }
 
@@ -56,16 +57,17 @@ func NewKeyStore(config *storagebackend.Config, dekPrefix string) value.KMSStora
 }
 
 // Setup creates the empty configmap on disk if it did not already exist.
-func (p *keyStore) Setup() error {
+func (p *keyStore) Setup(name string) error {
 	ctx := genericapirequest.NewDefaultContext()
+	p.name = dekMapName + "-" + name
 
 	_, err := p.GetAllDEKs()
 	// TODO(sakshams): Should we do this if err is 404, or should we do this as long as err is not nil?
 	if err != nil {
 		// We need to create the configmap
 		cfg := &api.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      dekMapName,
+				Name:      p.name,
 				Namespace: metav1.NamespaceDefault,
 			},
 			Data: map[string]string{},
@@ -82,7 +84,7 @@ func (p *keyStore) Setup() error {
 // GetAllDEKs reads and returns all available DEKs from etcd as a map.
 func (p *keyStore) GetAllDEKs() (map[string]string, error) {
 	ctx := genericapirequest.NewDefaultContext()
-	cfg, err := p.configmaps.GetConfigMap(ctx, dekMapName, &metav1.GetOptions{})
+	cfg, err := p.configmaps.GetConfigMap(ctx, p.name, &metav1.GetOptions{})
 	if err != nil {
 		return nil, err
 	}
@@ -91,13 +93,10 @@ func (p *keyStore) GetAllDEKs() (map[string]string, error) {
 
 // StoreNewDEKs writes the provided DEKs to disk.
 func (p *keyStore) StoreNewDEK(encDEK string) error {
-	// This function is invoked only by Rotate() calls, which are already lock protected.
-	// TODO(sakshams): Investigate if locks are really needed here.
-	p.lock.Lock()
-	defer p.lock.Unlock()
-
 	ctx := genericapirequest.NewDefaultContext()
-	cfg, err := p.configmaps.GetConfigMap(ctx, dekMapName, &metav1.GetOptions{})
+
+	// Obtain existing configmap for sanity check.
+	cfg, err := p.configmaps.GetConfigMap(ctx, p.name, &metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
@@ -125,6 +124,8 @@ func (p *keyStore) StoreNewDEK(encDEK string) error {
 	return err
 }
 
+var _ value.KMSStorage = &keyStore{}
+
 // GenerateName generates a unique new name for a DEK. Exposed for running encryptionconfig_test.
 func GenerateName(existingNames map[string]string) string {
 	name := randutil.String(keyNameLength)

staging/src/k8s.io/apiserver/pkg/storage/value/transformhelpers/kms_factory.go

@@ -24,21 +24,23 @@ import (
 	"k8s.io/apiserver/pkg/storage/value/encrypt/kms"
 )
 
+type gceCloudGetter func() (*kms.GCECloud, error)
+
 // KMSFactory provides shared instances of:
 // 1. Cloud provider in use
-// 2. Google KMS service
+// 2. Google KMS service override (optional)
 // 3. KMS Storage wrapper
 // They are used for cases with multiple transformers.
 type KMSFactory struct {
-	getGCECloud func() (*kms.GCECloud, error)
+	getGCECloud gceCloudGetter
 
-	gkmsService value.KMSService
+	gkmsServiceOverride value.KMSService
 
 	storage value.KMSStorage
 }
 
 // NewKMSFactory creates a key store for KMS data, and returns a KMSFactory instance.
-func NewKMSFactory(storageFactory serverstorage.StorageFactory, getGCECloud func() (*kms.GCECloud, error)) (*KMSFactory, error) {
+func NewKMSFactory(storageFactory serverstorage.StorageFactory, getGCECloud gceCloudGetter) (*KMSFactory, error) {
 	storageConfig, err := storageFactory.NewConfig(api.Resource("configmap"))
 	if err != nil {
 		return nil, err
@@ -53,25 +55,24 @@ func NewKMSFactory(storageFactory serverstorage.StorageFactory, getGCECloud func
 // NewKMSFactoryWithStorageAndGKMS allows creating a mockup KMSFactory for unit tests.
 func NewKMSFactoryWithStorageAndGKMS(storage value.KMSStorage, gkmsService value.KMSService) *KMSFactory {
 	return &KMSFactory{
-		storage:     storage,
-		gkmsService: gkmsService,
+		storage:             storage,
+		gkmsServiceOverride: gkmsService,
 	}
 }
 
 // GetGoogleKMSTransformer creates a Google KMS service which can Encrypt and Decrypt data.
-// Ensures only one instance of the service at any time.
-// TODO(sakshams): Does not handle the case where two different requests for a KMS service are for
-// different projects, locations etc.
+// Creates a new service each time, unless there is an override.
 func (kmsFactory *KMSFactory) GetGoogleKMSTransformer(projectID, location, keyRing, cryptoKey string) (value.Transformer, error) {
-	if kmsFactory.gkmsService == nil {
+	gkmsService := kmsFactory.gkmsServiceOverride
+	if gkmsService == nil {
 		cloud, err := kmsFactory.getGCECloud()
 		if err != nil {
 			return nil, err
 		}
-		kmsFactory.gkmsService, err = kms.NewGoogleKMSService(projectID, location, keyRing, cryptoKey, cloud)
+		gkmsService, err = kms.NewGoogleKMSService(projectID, location, keyRing, cryptoKey, cloud)
 		if err != nil {
 			return nil, err
 		}
 	}
-	return kms.NewKMSTransformer(kmsFactory.gkmsService, kmsFactory.storage)
+	return kms.NewKMSTransformer(gkmsService, kmsFactory.storage)
 }