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.

Sign up for GitHub

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: plugin install iteration 2 #369

Merged
merged 31 commits into from
Jan 12, 2024
Merged

feat: plugin install iteration 2 #369

Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
cf6d7d4
fix
Two-Hearts Jan 5, 2024
190436c
fix
Two-Hearts Jan 5, 2024
4123784
fix
Two-Hearts Jan 5, 2024
a513dbc
update
Two-Hearts Jan 5, 2024
c8ffdb0
update
Two-Hearts Jan 5, 2024
db54eb2
fix test
Two-Hearts Jan 5, 2024
8678aea
add test
Two-Hearts Jan 5, 2024
4e5e92e
check if missing plugin name
Two-Hearts Jan 5, 2024
fd6a149
fix test
Two-Hearts Jan 5, 2024
cbb7e25
fix test
Two-Hearts Jan 5, 2024
f7c5cc9
update
Two-Hearts Jan 8, 2024
95b7ead
add tests
Two-Hearts Jan 8, 2024
c8abe7e
update
Two-Hearts Jan 8, 2024
6ee86e0
test
Two-Hearts Jan 8, 2024
c695080
test
Two-Hearts Jan 8, 2024
1e32eda
test
Two-Hearts Jan 8, 2024
dc04d90
test
Two-Hearts Jan 8, 2024
733d8d2
test
Two-Hearts Jan 8, 2024
109b652
test
Two-Hearts Jan 8, 2024
8f3ab4b
fix
Two-Hearts Jan 8, 2024
43fee3f
fix
Two-Hearts Jan 8, 2024
73cbc83
fix
Two-Hearts Jan 8, 2024
afb5b93
update
Two-Hearts Jan 8, 2024
938b398
update
Two-Hearts Jan 8, 2024
8a2b3c2
update
Two-Hearts Jan 8, 2024
a733289
update
Two-Hearts Jan 8, 2024
48f0fc2
update per code review
Two-Hearts Jan 10, 2024
48ff47f
add test
Two-Hearts Jan 10, 2024
4d40f81
update
Two-Hearts Jan 10, 2024
cb7e6b2
update per code review
Two-Hearts Jan 12, 2024
3d8621d
update
Two-Hearts Jan 12, 2024
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
41 changes: 27 additions & 14 deletions plugin/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/notaryproject/notation-go/dir"
"github.com/notaryproject/notation-go/internal/file"
"github.com/notaryproject/notation-go/internal/semver"
"github.com/notaryproject/notation-go/log"
"github.com/notaryproject/notation-go/plugin/proto"
)

Expand Down Expand Up @@ -119,30 +120,27 @@ func (m *CLIManager) Install(ctx context.Context, installOpts CLIInstallOptions)
return nil, nil, errors.New("plugin source path cannot be empty")
}
var installFromNonDir bool
pluginExecutableFile, pluginName, err := parsePluginFromDir(installOpts.PluginPath)
pluginExecutableFile, pluginName, err := parsePluginFromDir(ctx, installOpts.PluginPath)
if err != nil {
if !errors.Is(err, file.ErrNotDirectory) {
return nil, nil, fmt.Errorf("failed to read plugin from directory %s: %w", installOpts.PluginPath, err)
return nil, nil, fmt.Errorf("failed to read plugin from input directory: %w", err)
}
// input is not a dir, check if it's a single plugin executable file
installFromNonDir = true
pluginExecutableFile = installOpts.PluginPath
pluginName, err = ParsePluginName(filepath.Base(pluginExecutableFile))
pluginName, err = parsePluginName(filepath.Base(pluginExecutableFile))
if err != nil {
return nil, nil, fmt.Errorf("failed to read plugin name from file path %s: %w", pluginExecutableFile, err)
return nil, nil, fmt.Errorf("failed to read plugin name from input file: %w", err)
}
isExec, err := isExecutableFile(pluginExecutableFile)
if err != nil {
return nil, nil, fmt.Errorf("failed to check if file %s is executable: %w", pluginExecutableFile, err)
return nil, nil, fmt.Errorf("failed to check if input file is executable: %w", err)
}
if !isExec {
return nil, nil, fmt.Errorf("file %s is not executable", pluginExecutableFile)
return nil, nil, errors.New("input file is not executable")
Two-Hearts marked this conversation as resolved.
Show resolved Hide resolved
}
}
// validate and get new plugin metadata
if err := validatePluginFileExtensionAgainstOS(filepath.Base(pluginExecutableFile), pluginName); err != nil {
return nil, nil, err
}
newPlugin, err := NewCLIPlugin(ctx, pluginName, pluginExecutableFile)
if err != nil {
return nil, nil, fmt.Errorf("failed to create new CLI plugin: %w", err)
Expand Down Expand Up @@ -215,12 +213,13 @@ func (m *CLIManager) Uninstall(ctx context.Context, name string) error {
}

// parsePluginFromDir checks if a dir is a valid plugin dir which contains
// one and only one plugin executable file. The dir may contain extra lib files
// and LICENSE files. Sub-directories are ignored.
// one and only one plugin executable file candidate.
// The dir may contain extra lib files and LICENSE files.
// Sub-directories are ignored.
//
// On success, the plugin executable file path, plugin name and
// nil error are returned.
func parsePluginFromDir(path string) (string, string, error) {
func parsePluginFromDir(ctx context.Context, path string) (string, string, error) {
// sanity check
fi, err := os.Stat(path)
if err != nil {
Expand All @@ -229,9 +228,11 @@ func parsePluginFromDir(path string) (string, string, error) {
if !fi.Mode().IsDir() {
return "", "", file.ErrNotDirectory
}
logger := log.GetLogger(ctx)
// walk the path
var pluginExecutableFile, pluginName string
var pluginExecutableFile, pluginName, candidatePluginName string
var foundPluginExecutableFile bool
var filesWithValidNameFormat []string
if err := filepath.WalkDir(path, func(p string, d fs.DirEntry, err error) error {
if err != nil {
return err
Expand All @@ -246,11 +247,12 @@ func parsePluginFromDir(path string) (string, string, error) {
}
// only take regular files
if info.Mode().IsRegular() {
if pluginName, err = ParsePluginName(d.Name()); err != nil {
if candidatePluginName, err = parsePluginName(d.Name()); err != nil {
// file name does not follow the notation-{plugin-name} format,
// continue
return nil
}
filesWithValidNameFormat = append(filesWithValidNameFormat, p)
isExec, err := isExecutableFile(p)
if err != nil {
return err
Expand All @@ -263,12 +265,23 @@ func parsePluginFromDir(path string) (string, string, error) {
}
foundPluginExecutableFile = true
pluginExecutableFile = p
pluginName = candidatePluginName
}
return nil
}); err != nil {
return "", "", err
}
if !foundPluginExecutableFile {
// if no executable file was found, but there's one and only one
// potential candidate, try install the candidate
if len(filesWithValidNameFormat) == 1 {
candidate := filesWithValidNameFormat[0]
if err := setExecutable(candidate); err != nil {
return "", "", fmt.Errorf("no plugin executable file was found: %w", err)
}
logger.Warnf("Found candidate plugin executable file %q without executable permission. Setting user executable bit and trying to install.", filepath.Base(candidate))
return candidate, candidatePluginName, nil
}
return "", "", errors.New("no plugin executable file was found")
Two-Hearts marked this conversation as resolved.
Show resolved Hide resolved
}
return pluginExecutableFile, pluginName, nil
Expand Down
179 changes: 163 additions & 16 deletions plugin/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ var validMetadataBar = proto.GetMetadataResponse{
SupportedContractVersions: []string{"1.0"}, Capabilities: []proto.Capability{proto.CapabilitySignatureGenerator},
}

var validMetadataBarExample = proto.GetMetadataResponse{
Name: "bar.example.plugin", Description: "friendly", Version: "1.0.0", URL: "example.com",
SupportedContractVersions: []string{"1.0"}, Capabilities: []proto.Capability{proto.CapabilitySignatureGenerator},
}

var invalidMetadataName = proto.GetMetadataResponse{
Name: "foobar", Description: "friendly", Version: "1", URL: "example.com",
SupportedContractVersions: []string{"1.0"}, Capabilities: []proto.Capability{proto.CapabilitySignatureGenerator},
Expand Down Expand Up @@ -214,6 +219,36 @@ func TestManager_Install(t *testing.T) {
}
})

t.Run("success install with file extension", func(t *testing.T) {
newPluginFilePath := "testdata/bar/notation-bar.example.plugin"
newPluginDir := filepath.Dir(newPluginFilePath)
if err := os.MkdirAll(newPluginDir, 0777); err != nil {
t.Fatalf("failed to create %s: %v", newPluginDir, err)
}
defer os.RemoveAll(newPluginDir)
if err := createFileAndChmod(newPluginFilePath, 0700); err != nil {
t.Fatal(err)
}
executor = testInstallCommander{
newPluginFilePath: newPluginFilePath,
newPluginStdout: metadataJSON(validMetadataBarExample),
}
defer mgr.Uninstall(context.Background(), "bar.example.plugin")
installOpts := CLIInstallOptions{
PluginPath: newPluginFilePath,
}
existingPluginMetadata, newPluginMetadata, err := mgr.Install(context.Background(), installOpts)
if err != nil {
t.Fatalf("expecting error to be nil, but got %v", err)
}
if existingPluginMetadata != nil {
t.Fatalf("expecting existingPluginMetadata to be nil, but got %v", existingPluginMetadata)
}
if newPluginMetadata.Version != validMetadataBar.Version {
t.Fatalf("new plugin version mismatch, new plugin version: %s, but got: %s", validMetadataBar.Version, newPluginMetadata.Version)
}
})

t.Run("fail to install due to equal version", func(t *testing.T) {
executor = testInstallCommander{
existedPluginFilePath: existedPluginFilePath,
Expand Down Expand Up @@ -265,21 +300,21 @@ func TestManager_Install(t *testing.T) {
installOpts := CLIInstallOptions{
PluginPath: newPluginFilePath,
}
expectedErrorMsg := "failed to read plugin name from file path testdata/bar/bar: invalid plugin executable file name. Plugin file name requires format notation-{plugin-name}, but got bar"
expectedErrorMsg := "failed to read plugin name from input file: invalid plugin executable file name. Plugin file name requires format notation-{plugin-name}, but got bar"
_, _, err := mgr.Install(context.Background(), installOpts)
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expecting error %s, but got %v", expectedErrorMsg, err)
}
})

t.Run("fail to install due to wrong plugin file permission", func(t *testing.T) {
newPluginFilePath := "testdata/bar/notation-bar"
t.Run("fail to install due to plugin executable file name missing plugin name", func(t *testing.T) {
newPluginFilePath := "testdata/bar/notation-"
newPluginDir := filepath.Dir(newPluginFilePath)
if err := os.MkdirAll(newPluginDir, 0777); err != nil {
t.Fatalf("failed to create %s: %v", newPluginDir, err)
}
defer os.RemoveAll(newPluginDir)
if err := createFileAndChmod(newPluginFilePath, 0600); err != nil {
if err := createFileAndChmod(newPluginFilePath, 0700); err != nil {
t.Fatal(err)
}
executor = testInstallCommander{
Expand All @@ -289,21 +324,21 @@ func TestManager_Install(t *testing.T) {
installOpts := CLIInstallOptions{
PluginPath: newPluginFilePath,
}
expectedErrorMsg := "file testdata/bar/notation-bar is not executable"
expectedErrorMsg := "failed to read plugin name from input file: invalid plugin executable file name. Plugin file name requires format notation-{plugin-name}, but got notation-"
_, _, err := mgr.Install(context.Background(), installOpts)
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expecting error %s, but got %v", expectedErrorMsg, err)
}
})

t.Run("fail to install due to invalid new plugin file extension", func(t *testing.T) {
newPluginFilePath := "testdata/bar/notation-bar.exe"
t.Run("fail to install due to wrong plugin file permission", func(t *testing.T) {
newPluginFilePath := "testdata/bar/notation-bar"
newPluginDir := filepath.Dir(newPluginFilePath)
if err := os.MkdirAll(newPluginDir, 0777); err != nil {
t.Fatalf("failed to create %s: %v", newPluginDir, err)
}
defer os.RemoveAll(newPluginDir)
if err := createFileAndChmod(newPluginFilePath, 0700); err != nil {
if err := createFileAndChmod(newPluginFilePath, 0600); err != nil {
t.Fatal(err)
}
executor = testInstallCommander{
Expand All @@ -313,7 +348,7 @@ func TestManager_Install(t *testing.T) {
installOpts := CLIInstallOptions{
PluginPath: newPluginFilePath,
}
expectedErrorMsg := "invalid plugin file extension. Expecting file notation-bar, but got notation-bar.exe"
expectedErrorMsg := "input file is not executable"
_, _, err := mgr.Install(context.Background(), installOpts)
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expecting error %s, but got %v", expectedErrorMsg, err)
Expand All @@ -329,7 +364,7 @@ func TestManager_Install(t *testing.T) {
installOpts := CLIInstallOptions{
PluginPath: newPluginFilePath,
}
expectedErrorMsg := "failed to read plugin from directory testdata/bar/notation-bar: stat testdata/bar/notation-bar: no such file or directory"
expectedErrorMsg := "failed to read plugin from input directory: stat testdata/bar/notation-bar: no such file or directory"
_, _, err := mgr.Install(context.Background(), installOpts)
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expecting error %s, but got %v", expectedErrorMsg, err)
Expand Down Expand Up @@ -397,7 +432,7 @@ func TestManager_Install(t *testing.T) {
t.Run("success to install from plugin dir", func(t *testing.T) {
existedPluginFilePath := "testdata/plugins/foo/notation-foo"
newPluginFilePath := "testdata/foo/notation-foo"
newPluginLibPath := "testdata/foo/libfoo"
newPluginLibPath := "testdata/foo/notation-libfoo"
newPluginDir := filepath.Dir(newPluginFilePath)
if err := os.MkdirAll(newPluginDir, 0777); err != nil {
t.Fatalf("failed to create %s: %v", newPluginDir, err)
Expand Down Expand Up @@ -430,15 +465,19 @@ func TestManager_Install(t *testing.T) {
}
})

t.Run("fail to install from plugin dir due to no plugin executable file", func(t *testing.T) {
t.Run("success to install from plugin dir with no executable file and one valid candidate file", func(t *testing.T) {
existedPluginFilePath := "testdata/plugins/foo/notation-foo"
newPluginFilePath := "testdata/foo/foo"
newPluginFilePath := "testdata/foo/notation-foo"
newPluginLibPath := "testdata/foo/libfoo"
newPluginDir := filepath.Dir(newPluginFilePath)
if err := os.MkdirAll(newPluginDir, 0777); err != nil {
t.Fatalf("failed to create %s: %v", newPluginDir, err)
}
defer os.RemoveAll(newPluginDir)
if err := createFileAndChmod(newPluginFilePath, 0700); err != nil {
if err := createFileAndChmod(newPluginFilePath, 0600); err != nil {
t.Fatal(err)
}
if err := createFileAndChmod(newPluginLibPath, 0600); err != nil {
t.Fatal(err)
}
executor = testInstallCommander{
Expand All @@ -450,7 +489,43 @@ func TestManager_Install(t *testing.T) {
installOpts := CLIInstallOptions{
PluginPath: newPluginDir,
}
expectedErrorMsg := "failed to read plugin from directory testdata/foo: no plugin executable file was found"
existingPluginMetadata, newPluginMetadata, err := mgr.Install(context.Background(), installOpts)
if err != nil {
t.Fatalf("expecting nil error, but got %v", err)
}
if existingPluginMetadata.Version != "1.0.0" {
t.Fatalf("expecting existing plugin metadata to be 1.0.0, but got %s", existingPluginMetadata.Version)
}
if newPluginMetadata.Version != "1.1.0" {
t.Fatalf("expecting new plugin metadata to be 1.1.0, but got %s", newPluginMetadata.Version)
}
})

t.Run("fail to install from plugin dir due to more than one candidate plugin executable files", func(t *testing.T) {
existedPluginFilePath := "testdata/plugins/foo/notation-foo"
newPluginFilePath := "testdata/foo/notation-foo1"
newPluginFilePath2 := "testdata/foo/notation-foo2"
newPluginDir := filepath.Dir(newPluginFilePath)
if err := os.MkdirAll(newPluginDir, 0777); err != nil {
t.Fatalf("failed to create %s: %v", newPluginDir, err)
}
defer os.RemoveAll(newPluginDir)
if err := createFileAndChmod(newPluginFilePath, 0600); err != nil {
t.Fatal(err)
}
if err := createFileAndChmod(newPluginFilePath2, 0600); err != nil {
t.Fatal(err)
}
executor = testInstallCommander{
existedPluginFilePath: existedPluginFilePath,
newPluginFilePath: newPluginFilePath,
existedPluginStdout: metadataJSON(validMetadata),
newPluginStdout: metadataJSON(validMetadataHigherVersion),
}
installOpts := CLIInstallOptions{
PluginPath: newPluginDir,
}
expectedErrorMsg := "failed to read plugin from input directory: no plugin executable file was found"
_, _, err := mgr.Install(context.Background(), installOpts)
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expecting error %s, but got %v", expectedErrorMsg, err)
Expand Down Expand Up @@ -481,7 +556,7 @@ func TestManager_Install(t *testing.T) {
installOpts := CLIInstallOptions{
PluginPath: newPluginDir,
}
expectedErrorMsg := "failed to read plugin from directory testdata/foo: found more than one plugin executable files"
expectedErrorMsg := "failed to read plugin from input directory: found more than one plugin executable files"
_, _, err := mgr.Install(context.Background(), installOpts)
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expecting error %s, but got %v", expectedErrorMsg, err)
Expand Down Expand Up @@ -517,6 +592,78 @@ func TestManager_Uninstall(t *testing.T) {
}
}

func TestParsePluginName(t *testing.T) {
pluginName, err := parsePluginName("notation-my-plugin")
if err != nil {
t.Fatalf("expected nil err, but got %v", err)
}
if pluginName != "my-plugin" {
t.Fatalf("expected plugin name my-plugin, but got %s", pluginName)
}

if runtime.GOOS == "windows" {
pluginName, err = parsePluginName("notation-my-plugin.exe")
if err != nil {
t.Fatalf("expected nil err, but got %v", err)
}
if pluginName != "my-plugin" {
t.Fatalf("expected plugin name my-plugin, but got %s", pluginName)
}

pluginName, err = parsePluginName("notation-com.plugin")
if err != nil {
t.Fatalf("expected nil err, but got %v", err)
}
if pluginName != "com.plugin" {
t.Fatalf("expected plugin name com.plugin, but got %s", pluginName)
}

expectedErrorMsg := "invalid plugin executable file name. Plugin file name requires format notation-{plugin-name}.exe, but got my-plugin.exe"
_, err = parsePluginName("my-plugin.exe")
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expected %s, but got %v", expectedErrorMsg, err)
}

expectedErrorMsg = "invalid plugin executable file name. Plugin file name requires format notation-{plugin-name}.exe, but got notation-.exe"
_, err = parsePluginName("notation-.exe")
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expected %s, but got %v", expectedErrorMsg, err)
}

expectedErrorMsg = "invalid plugin executable file name. Plugin file name requires format notation-{plugin-name}.exe, but got my-plugin"
_, err = parsePluginName("my-plugin")
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expected %s, but got %v", expectedErrorMsg, err)
}
} else {
pluginName, err = parsePluginName("notation-com.example.plugin")
if err != nil {
t.Fatalf("expected nil err, but got %v", err)
}
if pluginName != "com.example.plugin" {
t.Fatalf("expected plugin name com.example.plugin, but got %s", pluginName)
}

expectedErrorMsg := "invalid plugin executable file name. Plugin file name requires format notation-{plugin-name}, but got myPlugin"
_, err = parsePluginName("myPlugin")
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expected %s, but got %v", expectedErrorMsg, err)
}

expectedErrorMsg = "invalid plugin executable file name. Plugin file name requires format notation-{plugin-name}, but got my-plugin"
_, err = parsePluginName("my-plugin")
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expected %s, but got %v", expectedErrorMsg, err)
}

expectedErrorMsg = "invalid plugin executable file name. Plugin file name requires format notation-{plugin-name}, but got notation-"
_, err = parsePluginName("notation-")
if err == nil || err.Error() != expectedErrorMsg {
t.Fatalf("expected %s, but got %v", expectedErrorMsg, err)
}
}
}

func metadataJSON(m proto.GetMetadataResponse) []byte {
d, err := json.Marshal(m)
if err != nil {
Expand Down
Loading
Loading