From 8c2e6e92e08d25f4c2f8d50e89d968f1807e16fa Mon Sep 17 00:00:00 2001 From: Alex Shorsher Date: Wed, 25 May 2022 15:46:47 -0400 Subject: [PATCH] Enforce unique plugin names - Plugins names must be globally unique - Better handling of deprecated config in namespace manager - Updated config descriptions Signed-off-by: Alex Shorsher --- docs/reference/config.md | 1 + internal/coremsgs/en_config_descriptions.go | 2 +- internal/coremsgs/en_error_messages.go | 3 +- internal/namespace/config.go | 3 +- internal/namespace/manager.go | 27 ++++--- internal/namespace/manager_test.go | 86 ++++++++++----------- internal/orchestrator/orchestrator.go | 13 +++- internal/orchestrator/orchestrator_test.go | 18 +++++ 8 files changed, 95 insertions(+), 58 deletions(-) diff --git a/docs/reference/config.md b/docs/reference/config.md index e1a8950636..9f1f4ba3bf 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -617,6 +617,7 @@ nav_order: 3 |Key|Description|Type|Default Value| |---|-----------|----|-------------| |description|A description for the namespace|`string`|`` +|mode|The namespace mode|`string`|`` |name|The name of the namespace (must be unique)|`string`|`` |plugins|The list of plugins for this namespace|`string`|`` |remoteName|The namespace name to be sent in plugin calls, if it differs from namespace name|`string`|`` diff --git a/internal/coremsgs/en_config_descriptions.go b/internal/coremsgs/en_config_descriptions.go index ccf60f71f4..4ad572d7e3 100644 --- a/internal/coremsgs/en_config_descriptions.go +++ b/internal/coremsgs/en_config_descriptions.go @@ -254,7 +254,7 @@ var ( ConfigNamespacesPredefined = ffc("config.namespaces.predefined", "A list of namespaces to ensure exists, without requiring a broadcast from the network", "List "+i18n.StringType) ConfigNamespacesPredefinedName = ffc("config.namespaces.predefined[].name", "The name of the namespace (must be unique)", i18n.StringType) ConfigNamespacesPredefinedPlugins = ffc("config.namespaces.predefined[].plugins", "The list of plugins for this namespace", i18n.StringType) - ConfigNamespacesPredefinedMode = ffc("config.namespaces.predefined[].mode", "The namespace mode", i18n.StringType) + ConfigNamespacesPredefinedMode = ffc("config.namespaces.predefined[].mode", "The namespace mode. Valid values: gateway, multiparty", i18n.StringType) ConfigNamespacesPredefinedRemoteName = ffc("config.namespaces.predefined[].remoteName", "The namespace name to be sent in plugin calls, if it differs from namespace name", i18n.StringType) ConfigNamespacesPredefinedDescription = ffc("config.namespaces.predefined[].description", "A description for the namespace", i18n.StringType) diff --git a/internal/coremsgs/en_error_messages.go b/internal/coremsgs/en_error_messages.go index ec6f9d6ced..0a2d19f945 100644 --- a/internal/coremsgs/en_error_messages.go +++ b/internal/coremsgs/en_error_messages.go @@ -234,11 +234,12 @@ var ( MsgInvalidOutputOption = ffe("FF10385", "invalid output option '%s'") MsgInvalidPluginConfiguration = ffe("FF10386", "Invalid %s plugin configuration - name and type are required") MsgReferenceMarkdownMissing = ffe("FF10387", "Reference markdown file missing: '%s'") - MsgFFSystemReservedName = ffe("FF10388", "Invalid namespace configuration - ff_system is a reserved name") + MsgFFSystemReservedName = ffe("FF10388", "Invalid namespace configuration - %s is a reserved name") MsgInvalidNamespaceMode = ffe("FF10389", "Invalid %s namespace configuration - unknown mode") MsgNamespaceUnknownPlugin = ffe("FF10390", "Invalid %s namespace configuration - unknown plugin %s") MsgNamespaceMultipartyConfiguration = ffe("FF10391", "Invalid %s multiparty namespace configuration - database, blockchain, shared storage, and data exchange plugins are required") MsgNamespaceGatewayNoDB = ffe("FF10392", "Invalid %s gateway namespace configuration - a database plugin is required") MsgNamespaceGatewayInvalidPlugins = ffe("FF10393", "Invalid %s gateway namespace configuration - cannot specify dataexchange or shared storage plugins") MsgNamespaceGatewayMultiplePluginType = ffe("FF10394", "Invalid %s namespace configuration - multiple %s plugins provided") + MsgDuplicatePluginName = ffe("FF10395", "Invalid plugin configuration - plugin with name %s already exists") ) diff --git a/internal/namespace/config.go b/internal/namespace/config.go index 794bc9613c..c6b5e7385f 100644 --- a/internal/namespace/config.go +++ b/internal/namespace/config.go @@ -36,10 +36,9 @@ func InitConfig(withDefaults bool) { namespacePredefined.AddKnownKey(coreconfig.NamespaceDescription) namespacePredefined.AddKnownKey(coreconfig.NamespaceRemoteName) namespacePredefined.AddKnownKey(coreconfig.NamespacePlugins) - // do we need to set the default mode? + namespacePredefined.AddKnownKey(coreconfig.NamespaceMode, "multiparty") if withDefaults { namespaceConfig.AddKnownKey(NamespacePredefined+".0."+coreconfig.NamespaceName, "default") namespaceConfig.AddKnownKey(NamespacePredefined+".0."+coreconfig.NamespaceDescription, "Default predefined namespace") - namespaceConfig.AddKnownKey(NamespacePredefined+".0."+coreconfig.NamespaceMode, "multiparty") } } diff --git a/internal/namespace/manager.go b/internal/namespace/manager.go index ae7e48fe11..0bab5fe930 100644 --- a/internal/namespace/manager.go +++ b/internal/namespace/manager.go @@ -145,25 +145,35 @@ func (nm *namespaceManager) validateNamespaceConfig(ctx context.Context, name st return err } - if strings.ToLower(name) == "ff_system" || strings.ToLower(conf.GetString(coreconfig.NamespaceRemoteName)) == "ff_system" { - return i18n.NewError(ctx, coremsgs.MsgFFSystemReservedName) + if strings.ToLower(name) == core.SystemNamespace || strings.ToLower(conf.GetString(coreconfig.NamespaceRemoteName)) == core.SystemNamespace { + return i18n.NewError(ctx, coremsgs.MsgFFSystemReservedName, core.SystemNamespace) } mode := conf.GetString(coreconfig.NamespaceMode) plugins := conf.GetStringSlice(coreconfig.NamespacePlugins) - // If the no plugins are found when querying the config, assume older config file + // If no plugins are found when querying the config, assume older config file if len(plugins) == 0 { - // Still check to ensure at least one bc, dx, ss, and db plugin exist - if len(nm.bcPlugins) == 0 || len(nm.dxPlugins) == 0 || len(nm.ssPlugins) == 0 || len(nm.dbPlugins) == 0 { - return i18n.NewError(ctx, coremsgs.MsgNamespaceMultipartyConfiguration, name) + for plugin := range nm.bcPlugins { + plugins = append(plugins, plugin) + } + + for plugin := range nm.dxPlugins { + plugins = append(plugins, plugin) + } + + for plugin := range nm.ssPlugins { + plugins = append(plugins, plugin) + } + + for plugin := range nm.dbPlugins { + plugins = append(plugins, plugin) } - return nil } switch mode { // Multiparty is the default mode when none is provided - case "multiparty", "": + case "multiparty": if err := nm.validateMultiPartyConfig(ctx, name, plugins); err != nil { return err } @@ -220,7 +230,6 @@ func (nm *namespaceManager) validateMultiPartyConfig(ctx context.Context, name s } if !dbPlugin || !ssPlugin || !dxPlugin || !bcPlugin { - fmt.Printf("plugins: %+v\n", nm) return i18n.NewError(ctx, coremsgs.MsgNamespaceMultipartyConfiguration, name) } diff --git a/internal/namespace/manager_test.go b/internal/namespace/manager_test.go index 9c4cd1dd3e..f6b35a66bb 100644 --- a/internal/namespace/manager_test.go +++ b/internal/namespace/manager_test.go @@ -19,6 +19,7 @@ package namespace import ( "context" "fmt" + "strings" "testing" "github.com/hyperledger/firefly-common/pkg/config" @@ -35,6 +36,7 @@ import ( "github.com/hyperledger/firefly/pkg/dataexchange" "github.com/hyperledger/firefly/pkg/sharedstorage" "github.com/hyperledger/firefly/pkg/tokens" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) @@ -236,23 +238,23 @@ func TestInitNamespacesMultipartyMultipleDB(t *testing.T) { assert.Regexp(t, "FF10394", err) } -func TestInitNamespacesDeprecatedConfig(t *testing.T) { +func TestInitNamespacesDeprecatedConfigMultipleBlockchains(t *testing.T) { nm := newTestNamespaceManager(true) defer nm.cleanup(t) utTestConfig := config.RootSection("default") utTestConfig.AddKnownKey(coreconfig.NamespaceName, "default") utTestConfig.AddKnownKey(coreconfig.NamespaceRemoteName, "default") - utTestConfig.AddKnownKey(coreconfig.NamespaceMode) + utTestConfig.AddKnownKey(coreconfig.NamespaceMode, "multiparty") utTestConfig.AddKnownKey(coreconfig.NamespaceDescription) utTestConfig.AddKnownKey(coreconfig.NamespacePlugins, []string{}) nm.nsConfig = map[string]config.Section{"default": utTestConfig} - nm.mdi.On("GetNamespace", mock.Anything, mock.Anything).Return(nil, nil) - nm.mdi.On("UpsertNamespace", mock.Anything, mock.Anything, true).Return(nil) + // nm.mdi.On("GetNamespace", mock.Anything, mock.Anything).Return(nil, nil) + // nm.mdi.On("UpsertNamespace", mock.Anything, mock.Anything, true).Return(nil) err := nm.initNamespaces(context.Background(), nm.mdi) - assert.NoError(t, err) + assert.Regexp(t, "FF10394", err) } func TestInitNamespacesGatewayMultipleDB(t *testing.T) { @@ -315,26 +317,6 @@ func TestInitNamespacesGatewayWithDX(t *testing.T) { assert.Regexp(t, "FF10393", err) } -func TestInitNamespacesDeprecatedConfigNoPlugins(t *testing.T) { - nm := newTestNamespaceManager(true) - defer nm.cleanup(t) - - utTestConfig := config.RootSection("default") - utTestConfig.AddKnownKey(coreconfig.NamespaceName, "default") - utTestConfig.AddKnownKey(coreconfig.NamespaceRemoteName, "default") - utTestConfig.AddKnownKey(coreconfig.NamespaceMode) - utTestConfig.AddKnownKey(coreconfig.NamespaceDescription) - utTestConfig.AddKnownKey(coreconfig.NamespacePlugins, []string{}) - nm.nsConfig = map[string]config.Section{"default": utTestConfig} - nm.bcPlugins = nil - nm.dbPlugins = nil - nm.ssPlugins = nil - nm.dxPlugins = nil - - err := nm.initNamespaces(context.Background(), nm.mdi) - assert.Regexp(t, "FF10391", err) -} - func TestInitNamespacesGatewayWithSharedStorage(t *testing.T) { nm := newTestNamespaceManager(true) defer nm.cleanup(t) @@ -427,24 +409,42 @@ func TestInitNamespacesDefaultMissing(t *testing.T) { func TestInitNamespacesDupName(t *testing.T) { coreconfig.Reset() - InitConfig(true) - - namespaceConfig.AddKnownKey("predefined.0.name", "ns1") - namespaceConfig.AddKnownKey("predefined.0.remoteName", "ns1") - namespaceConfig.AddKnownKey("predefined.0.mode", "gateway") - namespaceConfig.AddKnownKey("predefined.0.plugins", []string{"ethereum", "postgres", "erc721"}) - - namespaceConfig.AddKnownKey("predefined.1.name", "ns2") - namespaceConfig.AddKnownKey("predefined.1.remoteName", "ns2") - namespaceConfig.AddKnownKey("predefined.1.mode", "gateway") - namespaceConfig.AddKnownKey("predefined.1.plugins", []string{"ethereum", "postgres", "erc721"}) - - namespaceConfig.AddKnownKey("predefined.2.name", "ns2") - namespaceConfig.AddKnownKey("predefined.2.remoteName", "ns2") - namespaceConfig.AddKnownKey("predefined.2.mode", "gateway") - namespaceConfig.AddKnownKey("predefined.2.plugins", []string{"ethereum", "postgres", "erc721"}) - - config.Set(coreconfig.NamespacesDefault, "ns1") + InitConfig(false) + + viper.SetConfigType("yaml") + err := viper.ReadConfig(strings.NewReader(` + namespaces: + default: ns1 + predefined: + - name: ns1 + remoteName: ns1 + mode: gateway + org: + key: 0x123456 + plugins: + - sqlite3 + - ethereum + - erc721 + - name: ns2 + remoteName: ns2 + mode: gateway + org: + key: 0x223456 + plugins: + - sqlite3 + - ethereum + - erc721 + - name: ns2 + remoteName: ns2 + mode: gateway + org: + key: 0x223456 + plugins: + - sqlite3 + - ethereum + - erc721 + `)) + assert.NoError(t, err) nm := newTestNamespaceManager(false) defer nm.cleanup(t) diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 5f2ee9ef27..c378d81b0d 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -176,6 +176,8 @@ type orchestrator struct { sharedDownload shareddownload.Manager txHelper txcommon.Helper namespace namespace.Manager + // Used to detect duplicate plugin names + pluginNames map[string]bool } func NewOrchestrator(withDefaults bool) Orchestrator { @@ -358,6 +360,7 @@ func (or *orchestrator) initDatabasePlugins(ctx context.Context, plugins []datab } name := config.GetString(coreconfig.PluginConfigName) or.databases[name] = plugin + or.databases[name] = plugin if or.database == nil { or.database = plugin @@ -369,9 +372,9 @@ func (or *orchestrator) initDatabasePlugins(ctx context.Context, plugins []datab func (or *orchestrator) validatePluginConfig(ctx context.Context, config config.Section, sectionName string) error { name := config.GetString(coreconfig.PluginConfigName) - dxType := config.GetString(coreconfig.PluginConfigType) + pluginType := config.GetString(coreconfig.PluginConfigType) - if name == "" || dxType == "" { + if name == "" || pluginType == "" { return i18n.NewError(ctx, coremsgs.MsgInvalidPluginConfiguration, sectionName) } @@ -379,6 +382,11 @@ func (or *orchestrator) validatePluginConfig(ctx context.Context, config config. return err } + if _, ok := or.pluginNames[name]; ok { + return i18n.NewError(ctx, coremsgs.MsgDuplicatePluginName, name) + } + or.pluginNames[name] = true + return nil } @@ -444,6 +452,7 @@ func (or *orchestrator) initDataExchange(ctx context.Context) (err error) { } func (or *orchestrator) initPlugins(ctx context.Context) (err error) { + or.pluginNames = make(map[string]bool) if or.metrics == nil { or.metrics = metrics.NewMetricsManager(ctx) } diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index a4a2a5bdf9..c3337d68aa 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -218,6 +218,7 @@ func TestBadDeprecatedDatabaseInitFail(t *testing.T) { func TestDatabaseGetPlugins(t *testing.T) { or := newTestOrchestrator() + or.pluginNames = make(map[string]bool) defer or.cleanup(t) difactory.InitConfig(databaseConfig) config.Set("plugins.database", []fftypes.JSONObject{{}}) @@ -231,6 +232,7 @@ func TestDatabaseGetPlugins(t *testing.T) { func TestDatabaseUnknownPlugin(t *testing.T) { or := newTestOrchestrator() + or.pluginNames = make(map[string]bool) defer or.cleanup(t) difactory.InitConfig(databaseConfig) config.Set("plugins.database", []fftypes.JSONObject{{}}) @@ -244,6 +246,7 @@ func TestDatabaseUnknownPlugin(t *testing.T) { func TestDatabaseGetPluginsNoName(t *testing.T) { or := newTestOrchestrator() + or.pluginNames = make(map[string]bool) defer or.cleanup(t) difactory.InitConfig(databaseConfig) config.Set("plugins.database", []fftypes.JSONObject{{}}) @@ -416,6 +419,7 @@ func TestDeprecatedBlockchainInitFail(t *testing.T) { func TestBlockchainGetPlugins(t *testing.T) { or := newTestOrchestrator() + or.pluginNames = make(map[string]bool) defer or.cleanup(t) bifactory.InitConfig(blockchainConfig) config.Set("plugins.blockchain", []fftypes.JSONObject{{}}) @@ -747,6 +751,19 @@ func TestBadTokensPlugin(t *testing.T) { assert.Error(t, err) } +func TestDuplicatePluginNames(t *testing.T) { + or := newTestOrchestrator() + or.pluginNames = make(map[string]bool) + or.pluginNames["duplicate"] = true + tifactory.InitConfig(tokensConfig) + utConfig := config.RootSection("test") + utConfig.AddKnownKey(coreconfig.PluginConfigName, "duplicate") + utConfig.AddKnownKey(coreconfig.PluginConfigType, "fftokens") + ctx := context.Background() + err := or.validatePluginConfig(ctx, utConfig, "test") + assert.Regexp(t, "FF10395", err) +} + func TestGoodTokensPlugin(t *testing.T) { or := newTestOrchestrator() defer or.cleanup(t) @@ -1098,6 +1115,7 @@ func TestInitDataExchangeGetNodesFail(t *testing.T) { func TestInitDataExchangeWithNodes(t *testing.T) { or := newTestOrchestrator() + or.pluginNames = make(map[string]bool) defer or.cleanup(t) or.database = or.mdi dxfactory.InitConfig(dataexchangeConfig)