Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: nice errors when failing to load plugins #7429

Merged
merged 1 commit into from
Aug 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions plugin/loader/load_nocgo.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// +build !cgo,!noplugin
// +build linux darwin

package loader

import (
"errors"

iplugin "github.com/ipfs/go-ipfs/plugin"
)

func init() {
loadPluginFunc = nocgoLoadPlugin
}

func nocgoLoadPlugin(fi string) ([]iplugin.Plugin, error) {
return nil, errors.New("not built with cgo support")
}
17 changes: 17 additions & 0 deletions plugin/loader/load_noplugin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// +build noplugin

package loader

import (
"errors"

iplugin "github.com/ipfs/go-ipfs/plugin"
)

func init() {
loadPluginFunc = nopluginLoadPlugin
}

func nopluginLoadPlugin(string) ([]iplugin.Plugin, error) {
return nil, errors.New("not built with plugin support")
}
44 changes: 4 additions & 40 deletions plugin/loader/load_unix.go
Original file line number Diff line number Diff line change
@@ -1,56 +1,20 @@
// +build !noplugin
// +build linux,cgo darwin,cgo
// +build cgo,!noplugin
// +build linux darwin

package loader

import (
"errors"
"fmt"
"os"
"path/filepath"
"plugin"

iplugin "github.com/ipfs/go-ipfs/plugin"
)

func init() {
loadPluginsFunc = linuxLoadFunc
loadPluginFunc = unixLoadPlugin
}

func linuxLoadFunc(pluginDir string) ([]iplugin.Plugin, error) {
var plugins []iplugin.Plugin

err := filepath.Walk(pluginDir, func(fi string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() {
if fi != pluginDir {
log.Warnf("found directory inside plugins directory: %s", fi)
}
return nil
}

if info.Mode().Perm()&0111 == 0 {
// file is not executable let's not load it
// this is to prevent loading plugins from for example non-executable
// mounts, some /tmp mounts are marked as such for security
log.Errorf("non-executable file in plugins directory: %s", fi)
return nil
}

if newPlugins, err := loadPlugin(fi); err == nil {
plugins = append(plugins, newPlugins...)
} else {
return fmt.Errorf("loading plugin %s: %s", fi, err)
}
return nil
})

return plugins, err
}

func loadPlugin(fi string) ([]iplugin.Plugin, error) {
func unixLoadPlugin(fi string) ([]iplugin.Plugin, error) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before, we wouldn't try loading plugins on platforms that didn't support plugins.

Now, we try loading plugins everywhere, but return an error if we find a plugin that we can't load (either because the platform is unsupported, or because something is wrong with the plugin).

pl, err := plugin.Open(fi)
if err != nil {
return nil, err
Expand Down
36 changes: 33 additions & 3 deletions plugin/loader/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"
"os"
"path/filepath"
"runtime"
"strings"

config "github.com/ipfs/go-ipfs-config"
Expand All @@ -30,8 +31,8 @@ func Preload(plugins ...plugin.Plugin) {

var log = logging.Logger("plugin/loader")

var loadPluginsFunc = func(string) ([]plugin.Plugin, error) {
return nil, nil
var loadPluginFunc = func(string) ([]plugin.Plugin, error) {
return nil, fmt.Errorf("unsupported platform %s", runtime.GOOS)
}

type loaderState int
Expand Down Expand Up @@ -182,7 +183,36 @@ func loadDynamicPlugins(pluginDir string) ([]plugin.Plugin, error) {
return nil, err
}

return loadPluginsFunc(pluginDir)
var plugins []plugin.Plugin

err = filepath.Walk(pluginDir, func(fi string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() {
if fi != pluginDir {
log.Warnf("found directory inside plugins directory: %s", fi)
}
return nil
}

if info.Mode().Perm()&0111 == 0 {
// file is not executable let's not load it
// this is to prevent loading plugins from for example non-executable
// mounts, some /tmp mounts are marked as such for security
log.Errorf("non-executable file in plugins directory: %s", fi)
return nil
}

if newPlugins, err := loadPluginFunc(fi); err == nil {
plugins = append(plugins, newPlugins...)
} else {
return fmt.Errorf("loading plugin %s: %s", fi, err)
}
return nil
})

return plugins, err
}

// Initialize initializes all loaded plugins
Expand Down
10 changes: 10 additions & 0 deletions test/sharness/t0280-plugin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,14 @@ test_expect_success "configure the plugin" '

test_plugin true "$IPFS_PATH" "foobar"

test_expect_success "noplugin flag works" '
test_must_fail go run -tags=noplugin github.com/ipfs/go-ipfs/cmd/ipfs id > output 2>&1
test_should_contain "not built with plugin support" output
'

test_expect_success "noplugin flag works" '
CGO_ENABLED=0 test_must_fail go run github.com/ipfs/go-ipfs/cmd/ipfs id > output 2>&1
test_should_contain "not built with cgo support" output
'

test_done