Chore: Refactor secrets plugin unit tests code (#54231)

* refactor test setup to return struct rather than many values

* changes to code style from review

* apply diff from Leandro for logging
This commit is contained in:
Michael Mandrus 2022-08-25 15:15:58 -04:00 committed by GitHub
parent 03e746d9df
commit 8deababa50
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 62 additions and 43 deletions

View File

@ -3,7 +3,6 @@ package kvstore
import (
"context"
"errors"
"fmt"
"sync"
"testing"
@ -15,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/ini.v1"
)
@ -22,44 +22,47 @@ import (
// Set fatal flag to true, then simulate a plugin start failure
// Should result in an error from the secret store provider
func TestFatalPluginErr_PluginFailsToStartWithFatalFlagSet(t *testing.T) {
svc, mgr, _, _, err := setupFatalCrashTest(t, true, true, false)
_ = fmt.Sprint(mgr) // this is here to satisfy the linter
require.Error(t, err)
require.Nil(t, svc)
p, err := setupFatalCrashTest(t, true, true, false)
assert.Error(t, err)
assert.Equal(t, "mocked failed to start", err.Error())
assert.Nil(t, p.secretsKVStore)
}
// Set fatal flag to false, then simulate a plugin start failure
// Should result in the secret store provider returning the sql impl
func TestFatalPluginErr_PluginFailsToStartWithFatalFlagNotSet(t *testing.T) {
svc, mgr, _, _, err := setupFatalCrashTest(t, true, false, false)
_ = fmt.Sprint(mgr) // this is here to satisfy the linter
require.NoError(t, err)
require.IsType(t, &CachedKVStore{}, svc)
cachedKv, _ := svc.(*CachedKVStore)
require.IsType(t, &secretsKVStoreSQL{}, cachedKv.GetUnwrappedStore())
p, err := setupFatalCrashTest(t, true, false, false)
assert.NoError(t, err)
require.IsType(t, &CachedKVStore{}, p.secretsKVStore)
cachedKv, _ := p.secretsKVStore.(*CachedKVStore)
assert.IsType(t, &secretsKVStoreSQL{}, cachedKv.GetUnwrappedStore())
}
// With fatal flag not set, store a secret in the plugin while backwards compatibility is disabled
// Should result in the fatal flag going from unset -> set to true
func TestFatalPluginErr_FatalFlagGetsSetWithBackwardsCompatDisabled(t *testing.T) {
svc, _, kvstore, _, err := setupFatalCrashTest(t, false, false, true)
require.NoError(t, err)
require.NotNil(t, svc)
err = svc.Set(context.Background(), 0, "datasource", "postgres", "my secret")
require.NoError(t, err)
isFatal, err := isPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(kvstore))
require.NoError(t, err)
require.True(t, isFatal)
p, err := setupFatalCrashTest(t, false, false, true)
assert.NoError(t, err)
require.NotNil(t, p.secretsKVStore)
err = p.secretsKVStore.Set(context.Background(), 0, "datasource", "postgres", "my secret")
assert.NoError(t, err)
isFatal, err := isPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(p.kvstore))
assert.NoError(t, err)
assert.True(t, isFatal)
}
// With fatal flag set, retrieve a secret from the plugin while backwards compatibility is enabled
// Should result in the fatal flag going from set to true -> unset
func TestFatalPluginErr_FatalFlagGetsUnSetWithBackwardsCompatEnabled(t *testing.T) {
svc, mgr, kvstore, _, err := setupFatalCrashTest(t, false, true, false)
require.NoError(t, err)
require.NotNil(t, svc)
p, err := setupFatalCrashTest(t, false, true, false)
assert.NoError(t, err)
require.NotNil(t, p.secretsKVStore)
// setup - store secret and manually bypassing the remote plugin impl
_, err = mgr.SecretsManager().SecretsManager.SetSecret(context.Background(), &secretsmanagerplugin.SetSecretRequest{
_, err = p.pluginManager.SecretsManager().SecretsManager.SetSecret(context.Background(), &secretsmanagerplugin.SetSecretRequest{
KeyDescriptor: &secretsmanagerplugin.Key{
OrgId: 0,
Namespace: "postgres",
@ -67,31 +70,35 @@ func TestFatalPluginErr_FatalFlagGetsUnSetWithBackwardsCompatEnabled(t *testing.
},
Value: "bogus",
})
require.NoError(t, err)
assert.NoError(t, err)
// retrieve the secret and check values
val, exists, err := svc.Get(context.Background(), 0, "postgres", "datasource")
require.NoError(t, err)
require.NotNil(t, val)
require.True(t, exists)
isFatal, err := isPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(kvstore))
require.NoError(t, err)
require.False(t, isFatal)
val, exists, err := p.secretsKVStore.Get(context.Background(), 0, "postgres", "datasource")
assert.NoError(t, err)
assert.NotNil(t, val)
assert.True(t, exists)
isFatal, err := isPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(p.kvstore))
assert.NoError(t, err)
assert.False(t, isFatal)
}
// With fatal flag unset, do a migration with backwards compatibility disabled. When unified secrets are deleted, return an error on the first deletion
// Should result in the fatal flag remaining unset
func TestFatalPluginErr_MigrationTestWithErrorDeletingUnifiedSecrets(t *testing.T) {
svc, _, kvstore, _, err := setupFatalCrashTest(t, false, false, true)
require.NoError(t, err)
p, err := setupFatalCrashTest(t, false, false, true)
assert.NoError(t, err)
migration := setupTestMigratorServiceWithDeletionError(t, svc, &mockstore.SQLStoreMock{
migration := setupTestMigratorServiceWithDeletionError(t, p.secretsKVStore, &mockstore.SQLStoreMock{
ExpectedError: errors.New("random error"),
}, kvstore)
}, p.kvstore)
err = migration.Migrate(context.Background())
require.Error(t, err)
isFatal, err := isPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(kvstore))
require.NoError(t, err)
require.False(t, isFatal)
assert.Error(t, err)
assert.Equal(t, "mocked del error", err.Error())
isFatal, err := isPluginStartupErrorFatal(context.Background(), GetNamespacedKVStore(p.kvstore))
assert.NoError(t, err)
assert.False(t, isFatal)
}
func setupFatalCrashTest(
@ -99,7 +106,7 @@ func setupFatalCrashTest(
shouldFailOnStart bool,
isPluginErrorFatal bool,
isBackwardsCompatDisabled bool,
) (SecretsKVStore, plugins.SecretsPluginManager, kvstore.KVStore, *sqlstore.SQLStore, error) {
) (fatalCrashTestFields, error) {
t.Helper()
fatalFlagOnce = sync.Once{}
startupOnce = sync.Once{}
@ -116,7 +123,19 @@ func setupFatalCrashTest(
t.Cleanup(func() {
fatalFlagOnce = sync.Once{}
})
return svc, manager, kvstore, sqlStore, err
return fatalCrashTestFields{
secretsKVStore: svc,
pluginManager: manager,
kvstore: kvstore,
sqlStore: sqlStore,
}, err
}
type fatalCrashTestFields struct {
secretsKVStore SecretsKVStore
pluginManager plugins.SecretsPluginManager
kvstore kvstore.KVStore
sqlStore *sqlstore.SQLStore
}
func setupTestMigratorServiceWithDeletionError(

View File

@ -59,7 +59,7 @@ func (f *FakeSecretsKVStore) Set(ctx context.Context, orgId int64, namespace str
func (f *FakeSecretsKVStore) Del(ctx context.Context, orgId int64, namespace string, typ string) error {
if f.delError {
return errors.New("bogus")
return errors.New("mocked del error")
}
delete(f.store, buildKey(orgId, namespace, typ))
return nil
@ -239,7 +239,7 @@ type fakePluginClient struct {
func (pc *fakePluginClient) Start(_ context.Context) error {
if pc.shouldFailOnStart {
return errors.New("failed to start")
return errors.New("mocked failed to start")
}
return nil
}