Refactor keystore interface into 3 different interfaces

libbeat/cmd/keystore.go

@@ -130,10 +130,15 @@ func createKeystore(settings instance.Settings, force bool) error {
 		return err
 	}
 
+	wKeystore, ok := store.(keystore.WritableKeystore)
+	if !ok {
+		return fmt.Errorf("the configured keystore is not writable")
+	}
+
 	if store.IsPersisted() == true && force == false {
 		response := terminal.PromptYesNo("A keystore already exists, Overwrite?", false)
 		if response == true {
-			err := store.Create(true)
+			err := wKeystore.Create(true)
 			if err != nil {
 				return fmt.Errorf("error creating the keystore: %s", err)
 			}
@@ -142,7 +147,7 @@ func createKeystore(settings instance.Settings, force bool) error {
 			return nil
 		}
 	} else {
-		err := store.Create(true)
+		err := wKeystore.Create(true)
 		if err != nil {
 			return fmt.Errorf("Error creating the keystore: %s", err)
 		}
@@ -160,14 +165,19 @@ func addKey(store keystore.Keystore, keys []string, force, stdin bool) error {
 		return fmt.Errorf("could not create secret for: %s, you can only provide one key per invocation", keys)
 	}
 
+	wKeystore, ok := store.(keystore.WritableKeystore)
+	if !ok {
+		return fmt.Errorf("the configured keystore is not writable")
+	}
+
 	if store.IsPersisted() == false {
 		if force == false {
 			answer := terminal.PromptYesNo("The keystore does not exist. Do you want to create it?", false)
 			if answer == false {
 				return errors.New("exiting without creating keystore")
 			}
 		}
-		err := store.Create(true)
+		err := wKeystore.Create(true)
 		if err != nil {
 			return fmt.Errorf("could not create keystore, error: %s", err)
 		}
@@ -202,10 +212,10 @@ func addKey(store keystore.Keystore, keys []string, force, stdin bool) error {
 			return fmt.Errorf("could not read value from the input, error: %s", err)
 		}
 	}
-	if err = store.Store(key, keyValue); err != nil {
+	if err = wKeystore.Store(key, keyValue); err != nil {
 		return fmt.Errorf("could not add the key in the keystore, error: %s", err)
 	}
-	if err = store.Save(); err != nil {
+	if err = wKeystore.Save(); err != nil {
 		return fmt.Errorf("fail to save the keystore: %s", err)
 	} else {
 		fmt.Println("Successfully updated the keystore")
@@ -218,6 +228,11 @@ func removeKey(store keystore.Keystore, keys []string) error {
 		return errors.New("you must supply at least one key to remove")
 	}
 
+	wKeystore, ok := store.(keystore.WritableKeystore)
+	if !ok {
+		return fmt.Errorf("the configured keystore is not writable")
+	}
+
 	if store.IsPersisted() == false {
 		return errors.New("the keystore doesn't exist. Use the 'create' command to create one")
 	}
@@ -229,8 +244,8 @@ func removeKey(store keystore.Keystore, keys []string) error {
 			return fmt.Errorf("could not find key '%v' in the keystore", key)
 		}
 
-		store.Delete(key)
-		err = store.Save()
+		wKeystore.Delete(key)
+		err = wKeystore.Save()
 		if err != nil {
 			return fmt.Errorf("could not update the keystore with the changes, key: %s, error: %v", key, err)
 		}
@@ -240,7 +255,11 @@ func removeKey(store keystore.Keystore, keys []string) error {
 }
 
 func list(store keystore.Keystore) error {
-	keys, err := store.List()
+	lKeystore, ok := store.(keystore.ListingKeystore)
+	if !ok {
+		return fmt.Errorf("the configured keystore is not writable")
+	}
+	keys, err := lKeystore.List()
 	if err != nil {
 		return fmt.Errorf("could not read values from the keystore, error: %s", err)
 	}

libbeat/keystore/file_keystore.go

@@ -51,6 +51,12 @@ const (
 // Version of the keystore format, will be added at the beginning of the file.
 var version = []byte("v1")
 
+// Packager defines a keystore that we can read the raw bytes and be packaged in an artifact.
+type Packager interface {
+	Package() ([]byte, error)
+	ConfiguredPath() string
+}
+
 // FileKeystore Allows to store key / secrets pair securely into an encrypted local file.
 type FileKeystore struct {
 	sync.RWMutex
@@ -66,6 +72,27 @@ type serializableSecureString struct {
 	Value []byte `json:"value"`
 }
 
+// Factory Create the right keystore with the configured options.
+func Factory(cfg *common.Config, defaultPath string) (Keystore, error) {
+	config := defaultConfig
+
+	if cfg == nil {
+		cfg = common.NewConfig()
+	}
+	err := cfg.Unpack(&config)
+
+	if err != nil {
+		return nil, fmt.Errorf("could not read keystore configuration, err: %v", err)
+	}
+
+	if config.Path == "" {
+		config.Path = defaultPath
+	}
+
+	keystore, err := NewFileKeystore(config.Path)
+	return keystore, err
+}
+
 // NewFileKeystore returns an new File based keystore or an error, currently users cannot set their
 // own password on the keystore, the default password will be an empty string. When the keystore
 // is initialized the secrets are automatically loaded into memory.

libbeat/keystore/file_keystore_test.go

@@ -37,10 +37,14 @@ func TestCanCreateAKeyStore(t *testing.T) {
 	path := GetTemporaryKeystoreFile()
 	defer os.Remove(path)
 
-	keystore, err := NewFileKeystore(path)
+	keyStore, err := NewFileKeystore(path)
 	assert.NoError(t, err)
-	assert.Nil(t, keystore.Store(keyValue, secretValue))
-	assert.Nil(t, keystore.Save())
+
+	wKeystore, ok := keyStore.(WritableKeystore)
+	assert.True(t, ok)
+
+	assert.Nil(t, wKeystore.Store(keyValue, secretValue))
+	assert.Nil(t, wKeystore.Save())
 }
 
 func TestCanReadAnExistingKeyStoreWithEmptyString(t *testing.T) {
@@ -66,15 +70,18 @@ func TestCanDeleteAKeyFromTheStoreAndPersistChanges(t *testing.T) {
 
 	CreateAnExistingKeystore(path)
 
-	keystore, _ := NewFileKeystore(path)
-	_, err := keystore.Retrieve(keyValue)
+	keyStore, _ := NewFileKeystore(path)
+	_, err := keyStore.Retrieve(keyValue)
 	assert.NoError(t, err)
 
-	keystore.Delete(keyValue)
-	_, err = keystore.Retrieve(keyValue)
+	wKeystore, ok := keyStore.(WritableKeystore)
+	assert.True(t, ok)
+
+	wKeystore.Delete(keyValue)
+	_, err = keyStore.Retrieve(keyValue)
 	assert.Error(t, err)
 
-	_ = keystore.Save()
+	_ = wKeystore.Save()
 	newKeystore, err := NewFileKeystore(path)
 	_, err = newKeystore.Retrieve(keyValue)
 	assert.Error(t, err)
@@ -113,10 +120,14 @@ func TestFilePermissionOnUpdate(t *testing.T) {
 	path := GetTemporaryKeystoreFile()
 	defer os.Remove(path)
 
-	keystore := CreateAnExistingKeystore(path)
-	err := keystore.Store("newkey", []byte("newsecret"))
+	keyStore := CreateAnExistingKeystore(path)
+
+	wKeystore, ok := keyStore.(WritableKeystore)
+	assert.True(t, ok)
+
+	err := wKeystore.Store("newkey", []byte("newsecret"))
 	assert.NoError(t, err)
-	err = keystore.Save()
+	err = wKeystore.Save()
 	assert.NoError(t, err)
 	stats, err := os.Stat(path)
 	assert.NoError(t, err)
@@ -152,9 +163,12 @@ func TestReturnsUsedKeysInTheStore(t *testing.T) {
 	path := GetTemporaryKeystoreFile()
 	defer os.Remove(path)
 
-	keystore := CreateAnExistingKeystore(path)
+	keyStore := CreateAnExistingKeystore(path)
+
+	wKeystore, ok := keyStore.(ListingKeystore)
+	assert.True(t, ok)
 
-	keys, err := keystore.List()
+	keys, err := wKeystore.List()
 
 	assert.NoError(t, err)
 	assert.Equal(t, len(keys), 1)
@@ -165,9 +179,13 @@ func TestCannotDecryptKeyStoreWithWrongPassword(t *testing.T) {
 	path := GetTemporaryKeystoreFile()
 	defer os.Remove(path)
 
-	keystore, err := NewFileKeystoreWithPassword(path, NewSecureString([]byte("password")))
-	keystore.Store("hello", []byte("world"))
-	keystore.Save()
+	keyStore, err := NewFileKeystoreWithPassword(path, NewSecureString([]byte("password")))
+
+	wKeystore, ok := keyStore.(WritableKeystore)
+	assert.True(t, ok)
+
+	wKeystore.Store("hello", []byte("world"))
+	wKeystore.Save()
 
 	_, err = NewFileKeystoreWithPassword(path, NewSecureString([]byte("wrongpassword")))
 	if assert.Error(t, err, "should fail to decrypt the keystore") {
@@ -199,13 +217,16 @@ func TestGetConfig(t *testing.T) {
 	path := GetTemporaryKeystoreFile()
 	defer os.Remove(path)
 
-	keystore := CreateAnExistingKeystore(path)
+	keyStore := CreateAnExistingKeystore(path)
+
+	wKeystore, ok := keyStore.(WritableKeystore)
+	assert.True(t, ok)
 
 	// Add a bit more data of different type
-	keystore.Store("super.nested", []byte("hello"))
-	keystore.Save()
+	wKeystore.Store("super.nested", []byte("hello"))
+	wKeystore.Save()
 
-	cfg, err := keystore.GetConfig()
+	cfg, err := keyStore.GetConfig()
 	assert.NotNil(t, cfg)
 	assert.NoError(t, err)
 
@@ -258,11 +279,14 @@ func createAndReadKeystoreSecret(t *testing.T, password []byte, key string, valu
 	path := GetTemporaryKeystoreFile()
 	defer os.Remove(path)
 
-	keystore, err := NewFileKeystoreWithPassword(path, NewSecureString(password))
+	keyStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password))
 	assert.Nil(t, err)
 
-	keystore.Store(key, value)
-	keystore.Save()
+	wKeystore, ok := keyStore.(WritableKeystore)
+	assert.True(t, ok)
+
+	wKeystore.Store(key, value)
+	wKeystore.Save()
 
 	newStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password))
 	s, _ := newStore.Retrieve(key)
@@ -274,11 +298,14 @@ func createAndReadKeystoreWithPassword(t *testing.T, password []byte) {
 	path := GetTemporaryKeystoreFile()
 	defer os.Remove(path)
 
-	keystore, err := NewFileKeystoreWithPassword(path, NewSecureString(password))
+	keyStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password))
 	assert.NoError(t, err)
 
-	keystore.Store("hello", []byte("world"))
-	keystore.Save()
+	wKeystore, ok := keyStore.(WritableKeystore)
+	assert.True(t, ok)
+
+	wKeystore.Store("hello", []byte("world"))
+	wKeystore.Save()
 
 	newStore, err := NewFileKeystoreWithPassword(path, NewSecureString(password))
 	s, _ := newStore.Retrieve("hello")
@@ -290,14 +317,17 @@ func createAndReadKeystoreWithPassword(t *testing.T, password []byte) {
 // CreateAnExistingKeystore creates a keystore with an existing key
 /// `output.elasticsearch.password` with the value `secret`.
 func CreateAnExistingKeystore(path string) Keystore {
-	keystore, err := NewFileKeystore(path)
+	keyStore, err := NewFileKeystore(path)
 	// Fail fast in the test suite
 	if err != nil {
 		panic(err)
 	}
-	keystore.Store(keyValue, secretValue)
-	keystore.Save()
-	return keystore
+
+	wKeystore, _ := keyStore.(WritableKeystore)
+
+	wKeystore.Store(keyValue, secretValue)
+	wKeystore.Save()
+	return keyStore
 }
 
 // GetTemporaryKeystoreFile create a temporary file on disk to save the keystore.

libbeat/keystore/keystore.go

@@ -19,7 +19,6 @@ package keystore
 
 import (
 	"errors"
-	"fmt"
 
 	"github.com/elastic/beats/v7/libbeat/common"
 	ucfg "github.com/elastic/go-ucfg"
@@ -39,67 +38,33 @@ var (
 // to that concept, so we can deal with tokens that has a limited duration or can be revoked by a
 // remote keystore.
 type Keystore interface {
-	// Store add keys to the keystore, wont be persisted until we save.
-	Store(key string, secret []byte) error
-
 	// Retrieve returns a SecureString instance of the searched key or an error.
 	Retrieve(key string) (*SecureString, error)
 
-	// Delete removes a specific key from the keystore.
-	Delete(key string) error
-
-	// List returns the list of keys in the keystore, return an empty list if none is found.
-	List() ([]string, error)
-
 	// GetConfig returns the key value pair in the config format to be merged with other configuration.
 	GetConfig() (*common.Config, error)
 
-	// Create Allow to create an empty keystore.
-	Create(override bool) error
-
 	// IsPersisted check if the current keystore is persisted.
 	IsPersisted() bool
-
-	// Save persist the changes to the keystore.
-	Save() error
-}
-
-// Packager defines a keystore that we can read the raw bytes and be packaged in an artifact.
-type Packager interface {
-	Package() ([]byte, error)
-	ConfiguredPath() string
 }
 
-// Factory Create the right keystore with the configured options.
-func Factory(cfg *common.Config, defaultPath string) (Keystore, error) {
-	config := defaultConfig
-
-	if cfg == nil {
-		cfg = common.NewConfig()
-	}
-	err := cfg.Unpack(&config)
+type WritableKeystore interface {
+	// Store add keys to the keystore, wont be persisted until we save.
+	Store(key string, secret []byte) error
 
-	if err != nil {
-		return nil, fmt.Errorf("could not read keystore configuration, err: %v", err)
-	}
+	// Delete removes a specific key from the keystore.
+	Delete(key string) error
 
-	if config.Path == "" {
-		config.Path = defaultPath
-	}
+	// Create Allow to create an empty keystore.
+	Create(override bool) error
 
-	keystore, err := NewFileKeystore(config.Path)
-	return keystore, err
+	// Save persist the changes to the keystore.
+	Save() error
 }
 
-// ResolverFromConfig create a resolver from a configuration.
-func ResolverFromConfig(cfg *common.Config, dataPath string) (func(string) (string, parse.Config, error), error) {
-	keystore, err := Factory(cfg, dataPath)
-
-	if err != nil {
-		return nil, err
-	}
-
-	return ResolverWrap(keystore), nil
+type ListingKeystore interface {
+	// List returns the list of keys in the keystore, return an empty list if none is found.
+	List() ([]string, error)
 }
 
 // ResolverWrap wrap a config resolver around an existing keystore.