From 310dca55dd845f0c0c2fced7b1c3d7d655f37fa1 Mon Sep 17 00:00:00 2001 From: Gus Eggert Date: Fri, 3 Jun 2022 17:08:10 -0400 Subject: [PATCH] feat: add fx options plugin This adds a plugin interface that lets the plugin modify the fx options that are passed to fx when the app is initialized. This means plugins can inject their own implementations of IPFS interfaces. This enables granular customization of go-ipfs behavior by plugins, such as: - Bitswap with custom filters (e.g. for CID blocking) Custom interface - implementations such as Pinner or DAGService - Dynamic configuration of libp2p ... One downside of this is that we're exposing the entire dependency graph, init hooks, initialization, etc. to users, so this comes with a caveat that we reserve the right to make breaking changes to the graph structure and initialization logic (although this historically happens rarely). If these things are changed, we should mention them in release notes and changelogs though, since they could impact users of this plugin interface. I'm not particularly fond of DI frameworks (and neither are some of the folks work on/near go-ipfs), but it seems unlikely that somebody will rewrite the dependency wiring and lifecycle hooks of go-ipfs, and add dynamic extension points, so this seems like a palatable compromise. There are also problems that we should clean up in how model the go-ipfs app in fx, such as: - We make extensive use of nested fx.Options, which fx itself discourages because it "limits the user's ability to customize their application". It should be easy to flatten these out into a single []fx.Option slice. - We pass around a list of opaque libp2p opts, which makes it hard to customize after-the-fact...we should consider naming each of these opts and providing them to fx as proper dependencies, so that they can be explicitly overridden. - We call fx.Invoke() in some places with anonymous functions. We should instead only pass exported functions to fx.Invoke(), so that they have exported names, which would make it easier to remove/augment the invocations that happen when the app is initialized. These aren't blocking issues, they just make it harder and more brittle to customize go-ipfs with this plugin. --- core/builder.go | 46 +++++++++++++++++++++++++++++--- go.mod | 2 +- go.sum | 3 ++- plugin/fx.go | 20 ++++++++++++++ plugin/loader/loader.go | 13 ++++++++- plugin/loader/preload.go | 2 ++ plugin/loader/preload_list | 1 + plugin/plugins/fxtest/fxtest.go | 44 ++++++++++++++++++++++++++++++ test/sharness/t0280-plugin-fx.sh | 19 +++++++++++++ 9 files changed, 143 insertions(+), 7 deletions(-) create mode 100644 plugin/fx.go create mode 100644 plugin/plugins/fxtest/fxtest.go create mode 100755 test/sharness/t0280-plugin-fx.sh diff --git a/core/builder.go b/core/builder.go index 2b84c6d83..af6ee4c7b 100644 --- a/core/builder.go +++ b/core/builder.go @@ -15,6 +15,35 @@ import ( "go.uber.org/fx" ) +// FXNodeInfo contains information useful for adding fx options. +type FXNodeInfo struct { + FXOptions []fx.Option +} + +// fxOptFunc takes in some info about the IPFS node and returns the full set of fx opts to use. +type fxOptFunc func(FXNodeInfo) ([]fx.Option, error) + +var fxOptionFuncs []fxOptFunc + +// RegisterFXOptionFunc registers a function that is run before the fx app is initialized. +// Functions are invoked in the order they are registered, +// and the resulting options are passed into the next function's FXNodeInfo. +// +// Note that these are applied globally, by all invocations of NewNode. +// There are multiple places in Kubo that construct nodes, such as: +// - Repo initialization +// - Daemon initialization +// - When running migrations +// - etc. +// +// If your fx options are doing anything sophisticated, you should keep this in mind. +// +// For example, if you plug in a blockservice that disallows non-allowlisted CIDs, +// this may break migrations that fetch migration code over IPFS. +func RegisterFXOptionFunc(optFunc fxOptFunc) { + fxOptionFuncs = append(fxOptionFuncs, optFunc) +} + // from https://stackoverflow.com/a/59348871 type valueContext struct { context.Context @@ -41,12 +70,21 @@ func NewNode(ctx context.Context, cfg *BuildCfg) (*IpfsNode, error) { ctx: ctx, } - app := fx.New( + opts := []fx.Option{ node.IPFS(ctx, cfg), - fx.NopLogger, - fx.Extract(n), - ) + } + for _, optFunc := range fxOptionFuncs { + var err error + opts, err = optFunc(FXNodeInfo{FXOptions: opts}) + if err != nil { + cancel() + return nil, fmt.Errorf("building fx opts: %w", err) + } + } + opts = append(opts, fx.Extract(n)) + + app := fx.New(opts...) var once sync.Once var stopErr error diff --git a/go.mod b/go.mod index 7356c5433..07b2d0a50 100644 --- a/go.mod +++ b/go.mod @@ -113,7 +113,7 @@ require ( go.uber.org/zap v1.21.0 golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e golang.org/x/sync v0.0.0-20210220032951-036812b2e83c - golang.org/x/sys v0.0.0-20220517195934-5e4e11fc645e + golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a ) require ( diff --git a/go.sum b/go.sum index 02188c7b6..17c1d72b5 100644 --- a/go.sum +++ b/go.sum @@ -1925,8 +1925,9 @@ golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220422013727-9388b58f7150/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220517195934-5e4e11fc645e h1:w36l2Uw3dRan1K3TyXriXvY+6T56GNmlKGcqiQUJDfM= golang.org/x/sys v0.0.0-20220517195934-5e4e11fc645e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a h1:dGzPydgVsqGcTRVwiLJ1jVbufYwmzD3LfVPLKsKg+0k= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY= diff --git a/plugin/fx.go b/plugin/fx.go new file mode 100644 index 000000000..905889428 --- /dev/null +++ b/plugin/fx.go @@ -0,0 +1,20 @@ +package plugin + +import ( + "github.com/ipfs/kubo/core" + "go.uber.org/fx" +) + +// PluginFx can be used to customize the fx options passed to the go-ipfs app when it is initialized. +// +// This is invasive and depends on internal details such as the structure of the dependency graph, +// so breaking changes might occur between releases. +// So it's recommended to keep this as simple as possible, and stick to overriding interfaces +// with fx.Replace() or fx.Decorate(). +// +// The returned options become the complete array of options passed to fx. +// Generally you'll want to append additional options to NodeInfo.FXOptions and return that. +type PluginFx interface { + Plugin + Options(core.FXNodeInfo) ([]fx.Option, error) +} diff --git a/plugin/loader/loader.go b/plugin/loader/loader.go index 116c3dfce..bb9c15bef 100644 --- a/plugin/loader/loader.go +++ b/plugin/loader/loader.go @@ -241,7 +241,6 @@ func (loader *PluginLoader) Inject() error { for _, pl := range loader.plugins { if pl, ok := pl.(plugin.PluginIPLD); ok { - err := injectIPLDPlugin(pl) if err != nil { loader.state = loaderFailed @@ -262,6 +261,13 @@ func (loader *PluginLoader) Inject() error { return err } } + if pl, ok := pl.(plugin.PluginFx); ok { + err := injectFxPlugin(pl) + if err != nil { + loader.state = loaderFailed + return err + } + } } return loader.transition(loaderInjecting, loaderInjected) @@ -347,3 +353,8 @@ func injectTracerPlugin(pl plugin.PluginTracer) error { opentracing.SetGlobalTracer(tracer) return nil } + +func injectFxPlugin(pl plugin.PluginFx) error { + core.RegisterFXOptionFunc(pl.Options) + return nil +} diff --git a/plugin/loader/preload.go b/plugin/loader/preload.go index 6b1607dc5..430486211 100644 --- a/plugin/loader/preload.go +++ b/plugin/loader/preload.go @@ -4,6 +4,7 @@ import ( pluginbadgerds "github.com/ipfs/kubo/plugin/plugins/badgerds" pluginiplddagjose "github.com/ipfs/kubo/plugin/plugins/dagjose" pluginflatfs "github.com/ipfs/kubo/plugin/plugins/flatfs" + pluginfxtest "github.com/ipfs/kubo/plugin/plugins/fxtest" pluginipldgit "github.com/ipfs/kubo/plugin/plugins/git" pluginlevelds "github.com/ipfs/kubo/plugin/plugins/levelds" pluginpeerlog "github.com/ipfs/kubo/plugin/plugins/peerlog" @@ -20,4 +21,5 @@ func init() { Preload(pluginflatfs.Plugins...) Preload(pluginlevelds.Plugins...) Preload(pluginpeerlog.Plugins...) + Preload(pluginfxtest.Plugins...) } diff --git a/plugin/loader/preload_list b/plugin/loader/preload_list index 048f4fd28..c18ea80cc 100644 --- a/plugin/loader/preload_list +++ b/plugin/loader/preload_list @@ -10,3 +10,4 @@ badgerds github.com/ipfs/kubo/plugin/plugins/badgerds * flatfs github.com/ipfs/kubo/plugin/plugins/flatfs * levelds github.com/ipfs/kubo/plugin/plugins/levelds * peerlog github.com/ipfs/kubo/plugin/plugins/peerlog * +fxtest github.com/ipfs/kubo/plugin/plugins/fxtest * diff --git a/plugin/plugins/fxtest/fxtest.go b/plugin/plugins/fxtest/fxtest.go new file mode 100644 index 000000000..175dc6ec6 --- /dev/null +++ b/plugin/plugins/fxtest/fxtest.go @@ -0,0 +1,44 @@ +package fxtest + +import ( + "os" + + logging "github.com/ipfs/go-log" + "github.com/ipfs/kubo/core" + "github.com/ipfs/kubo/plugin" + "go.uber.org/fx" +) + +var log = logging.Logger("fxtestplugin") + +var Plugins = []plugin.Plugin{ + &fxtestPlugin{}, +} + +// fxtestPlugin is used for testing the fx plugin. +// It merely adds an fx option that logs a debug statement, so we can verify that it works in tests. +type fxtestPlugin struct{} + +var _ plugin.PluginFx = (*fxtestPlugin)(nil) + +func (p *fxtestPlugin) Name() string { + return "fx-test" +} + +func (p *fxtestPlugin) Version() string { + return "0.1.0" +} + +func (p *fxtestPlugin) Init(env *plugin.Environment) error { + return nil +} + +func (p *fxtestPlugin) Options(info core.FXNodeInfo) ([]fx.Option, error) { + opts := info.FXOptions + if os.Getenv("TEST_FX_PLUGIN") != "" { + opts = append(opts, fx.Invoke(func() { + log.Debug("invoked test fx function") + })) + } + return opts, nil +} diff --git a/test/sharness/t0280-plugin-fx.sh b/test/sharness/t0280-plugin-fx.sh new file mode 100755 index 000000000..ca4d45f07 --- /dev/null +++ b/test/sharness/t0280-plugin-fx.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash + +test_description="Test fx plugin" + +. lib/test-lib.sh + +test_init_ipfs + +export GOLOG_LOG_LEVEL="fxtestplugin=debug" +export TEST_FX_PLUGIN=1 +test_launch_ipfs_daemon + +test_expect_success "expected log entry should be present" ' + fgrep "invoked test fx function" daemon_err >/dev/null +' + +test_kill_ipfs_daemon + +test_done