Skip to content

Commit

Permalink
updated per code review
Browse files Browse the repository at this point in the history 
Signed-off-by: Patrick Zheng <[email protected]>
  • Loading branch information
@Two-Hearts
Two-Hearts committed Nov 23, 2023
1 parent e0088d9 commit 0461593
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 42 deletions.
12 changes: 7 additions & 5 deletions cmd/notation/internal/plugin/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ import (
"oras.land/oras-go/v2/registry/remote/auth"
)

// maxPluginSourceBytes specifies the limit on how many response
// bytes are allowed in the server's response to the download from URL request
var maxPluginSourceBytes int64 = 256 * 1024 * 1024 // 256 MiB
// MaxPluginSourceBytes specifies the limit on how many bytes are allowed in the
// server's response to the download from URL request.
// It also specifies the limit of a potentail plugin executable file in a
// .tar.gz or .zip file.
var MaxPluginSourceBytes int64 = 256 * 1024 * 1024 // 256 MiB

// PluginSourceType is an enum for plugin source
type PluginSourceType int
Expand Down Expand Up @@ -92,14 +94,14 @@ func DownloadPluginFromURL(ctx context.Context, pluginURL string, tmpFile io.Wri
// Write the body to file
lr := &io.LimitedReader{
R: resp.Body,
N: maxPluginSourceBytes,
N: MaxPluginSourceBytes,
}
_, err = io.Copy(tmpFile, lr)
if err != nil {
return err
}
if lr.N == 0 {
return fmt.Errorf("https response reaches the %d MiB size limit", maxPluginSourceBytes)
return fmt.Errorf("https response reaches the %d MiB size limit", MaxPluginSourceBytes)
}
return nil
}
Expand Down
40 changes: 24 additions & 16 deletions cmd/notation/plugin/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Example - Install plugin from URL, SHA256 checksum is required:
command.Flags().BoolVar(&opts.isFile, "file", false, "install plugin from a file in file system")
command.Flags().BoolVar(&opts.isUrl, "url", false, "install plugin from an HTTPS URL")
command.Flags().StringVar(&opts.inputChecksum, "sha256sum", "", "must match SHA256 of the plugin source, required when \"--url\" flag is set")
command.Flags().BoolVar(&opts.force, "force", false, "force the installation of a plugin")
command.Flags().BoolVar(&opts.force, "force", false, "force the installation of the plugin")
command.MarkFlagsMutuallyExclusive("file", "url")
command.MarkFlagsOneRequired("file", "url")
return command
Expand All @@ -121,10 +121,10 @@ func installPlugin(command *cobra.Command, opts *pluginInstallOpts) error {
}
pluginURL, err := url.Parse(opts.pluginSource)
if err != nil {
return fmt.Errorf("failed to install from URL: %v", err)
return fmt.Errorf("the plugin download failed: %v", err)
}
if pluginURL.Scheme != "https" {
return fmt.Errorf("failed to install from URL: %q scheme is not HTTPS", opts.pluginSource)
return fmt.Errorf("the plugin download failed: only the HTTPS scheme is supported, but got %s", pluginURL.Scheme)
}
tmpFile, err := os.CreateTemp("", notationPluginDownloadTmpFile)
if err != nil {
Expand All @@ -142,7 +142,7 @@ func installPlugin(command *cobra.Command, opts *pluginInstallOpts) error {
}
return installFromFileSystem(ctx, downloadPath, opts.inputChecksum, opts.force)
default:
return errors.New("failed to install the plugin: plugin source type is unknown")
return errors.New("plugin installation failed: unknown plugin source type")
}
}

Expand All @@ -151,36 +151,36 @@ func installFromFileSystem(ctx context.Context, inputPath string, inputChecksum
// sanity check
inputFileStat, err := os.Stat(inputPath)
if err != nil {
return fmt.Errorf("failed to install the plugin: %w", err)
return fmt.Errorf("plugin installation failed: %w", err)
}
if !inputFileStat.Mode().IsRegular() {
return fmt.Errorf("failed to install the plugin: %s is not a regular file", inputPath)
return fmt.Errorf("plugin installation failed: %s is not a valid file", inputPath)
}
// checksum check
if inputChecksum != "" {
if err := osutil.ValidateChecksum(inputPath, inputChecksum); err != nil {
return fmt.Errorf("failed to install the plugin: %w", err)
return fmt.Errorf("plugin installation failed: %w", err)
}
}
// install the plugin based on file type
fileType, err := osutil.DetectFileType(inputPath)
if err != nil {
return fmt.Errorf("failed to install the plugin: %w", err)
return fmt.Errorf("plugin installation failed: %w", err)
}
switch fileType {
case notationplugin.MediaTypeZip:
if err := installPluginFromZip(ctx, inputPath, force); err != nil {
return fmt.Errorf("failed to install the plugin: %w", err)
return fmt.Errorf("plugin installation failed: %w", err)
}
return nil
case notationplugin.MediaTypeGzip:
// when file is gzip, require to be tar
if err := installPluginFromTarGz(ctx, inputPath, force); err != nil {
return fmt.Errorf("failed to install the plugin: %w", err)
return fmt.Errorf("plugin installation failed: %w", err)
}
return nil
default:
return errors.New("failed to install the plugin: invalid file format. Only support .tar.gz and .zip")
return errors.New("plugin installation failed: invalid file format. Only .tar.gz and .zip formats are supported")
}
}

Expand All @@ -202,7 +202,7 @@ func installPluginFromZip(ctx context.Context, zipPath string, force bool) error
// validate and get plugin name from file name
pluginName, err := notationplugin.ExtractPluginNameFromFileName(f.Name)
if err != nil {
logger.Infof("processing file %s. File name not in format notation-{plugin-name}, skipped", f.Name)
logger.Debugf("processing file %s. File name not in format notation-{plugin-name}, skipped", f.Name)
continue
}
fileInArchive, err := f.Open()
Expand Down Expand Up @@ -270,14 +270,22 @@ func installPluginExecutable(ctx context.Context, fileName string, pluginName st
}
defer os.RemoveAll(tmpDir)
tmpFilePath := filepath.Join(tmpDir, fileName)
pluginFile, err := os.OpenFile(tmpFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0700)
pluginFile, err := os.OpenFile(tmpFilePath, os.O_WRONLY|os.O_CREATE, 0700)
if err != nil {
return err
}
if _, err := io.Copy(pluginFile, fileReader); err != nil {
lr := &io.LimitedReader{
R: fileReader,
N: notationplugin.MaxPluginSourceBytes,
}
if _, err := io.Copy(pluginFile, lr); err != nil || lr.N == 0 {
_ = pluginFile.Close()
return err
if err != nil {
return err
}
return fmt.Errorf("plugin executable file reaches the %d MiB size limit", notationplugin.MaxPluginSourceBytes)
}

if err := pluginFile.Close(); err != nil {
return err
}
Expand Down Expand Up @@ -305,7 +313,7 @@ func installPluginExecutable(ctx context.Context, fileName string, pluginName st
case comp < 0:
return fmt.Errorf("%s. The installing version %s is lower than the existing plugin version %s.\nIt is not recommended to install an older version. To force the installation, use the \"--force\" option", pluginName, pluginVersion, currentPluginVersion)
case comp == 0:
return fmt.Errorf("%s with version %s already exists", pluginName, currentPluginVersion)
return fmt.Errorf("plugin %s with version %s already exists", pluginName, currentPluginVersion)
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ go 1.21

require (
github.com/notaryproject/notation-core-go v1.0.1
github.com/notaryproject/notation-go v1.0.1
github.com/notaryproject/notation-go v1.0.2-0.20231123031546-5de0d58b21c1
github.com/opencontainers/go-digest v1.0.0
github.com/opencontainers/image-spec v1.1.0-rc5
github.com/oras-project/oras-credentials-go v0.3.1
github.com/sirupsen/logrus v1.9.3
github.com/spf13/cobra v1.8.0
github.com/spf13/pflag v1.0.5
golang.org/x/mod v0.13.0
golang.org/x/term v0.13.0
golang.org/x/mod v0.14.0
golang.org/x/term v0.14.0
oras.land/oras-go/v2 v2.3.1
)

Expand All @@ -26,7 +26,7 @@ require (
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/veraison/go-cose v1.1.0 // indirect
github.com/x448/float16 v0.8.4 // indirect
golang.org/x/crypto v0.14.0 // indirect
golang.org/x/crypto v0.15.0 // indirect
golang.org/x/sync v0.4.0 // indirect
golang.org/x/sys v0.13.0 // indirect
golang.org/x/sys v0.14.0 // indirect
)
20 changes: 10 additions & 10 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/notaryproject/notation-core-go v1.0.1 h1:01doxjDERbd0vocLQrlJdusKrRLNNn50OJzp0c5I4Cw=
github.com/notaryproject/notation-core-go v1.0.1/go.mod h1:rayl8WlKgS4YxOZgDO0iGGB4Ef515ZFZUFaZDmsPXgE=
github.com/notaryproject/notation-go v1.0.1 h1:D3fqG3eaBKVESRySV/Tg//MyTg2Q1nTKPh/t2q9LpSw=
github.com/notaryproject/notation-go v1.0.1/go.mod h1:VonyZsbocRQQNIDq/VPV5jKJOQwDH3gvfK4cXNpUA0U=
github.com/notaryproject/notation-go v1.0.2-0.20231123031546-5de0d58b21c1 h1:TuSZ+3Eu3A/XKucl7J95sDT8XoG6t2dEcIipt6ydAls=
github.com/notaryproject/notation-go v1.0.2-0.20231123031546-5de0d58b21c1/go.mod h1:tSCFsAdKAtB7AfKS/BaUf8AXzASA+9TEokMDEDutqPM=
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
github.com/opencontainers/image-spec v1.1.0-rc5 h1:Ygwkfw9bpDvs+c9E34SdgGOj41dX/cbdlwvlWt0pnFI=
Expand Down Expand Up @@ -51,12 +51,12 @@ github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5t
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc=
golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc=
golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4=
golang.org/x/crypto v0.15.0 h1:frVn1TEaCEaZcn3Tmd7Y2b5KKPaZ+I32Q2OA3kYp5TA=
golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72g=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/mod v0.13.0 h1:I/DsJXRlw/8l/0c24sM9yb0T4z9liZTduXvdAWYiysY=
golang.org/x/mod v0.13.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0=
golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
Expand All @@ -76,15 +76,15 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q=
golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU=
golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek=
golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U=
golang.org/x/term v0.14.0 h1:LGK9IlZ8T9jvdy6cTdfKUCltatMFOehAQo9SRC46UQ8=
golang.org/x/term v0.14.0/go.mod h1:TySc+nGkYR6qt8km8wUhuFRTVSMIX3XPR58y2lC8vww=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
Expand Down
6 changes: 3 additions & 3 deletions specs/commandline/plugin.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ Successfully installed plugin <plugin name>, version <x.y.z>
If the entered plugin checksum digest doesn't match the published checksum, Notation will return an error message and will not start installation.

```console
Error: failed to install the plugin: plugin checksum does not match user input. Expecting <sha256sum>
Error: plugin installation failed: plugin checksum does not match user input. Expecting <sha256sum>
```

If the plugin version is higher than the existing plugin, Notation will start installation and overwrite the existing plugin.
Expand All @@ -107,13 +107,13 @@ Successfully installed plugin <plugin name>, updated the version from <x.y.z> to
If the plugin version is equal to the existing plugin, Notation will not start installation and return the following message. Users can use a flag `--force` to skip plugin version check and force the installation.

```console
Error: failed to install the plugin: <plugin-name> with version <x.y.z> already exists.
Error: plugin installation failed: plugin <plugin-name> with version <x.y.z> already exists.
```

If the plugin version is lower than the existing plugin, Notation will return an error message and will not start installation. Users can use a flag `--force` to skip plugin version check and force the installation.

```console
Error: failed to install the plugin: <plugin-name>. The installing plugin version <a.b.c> is lower than the existing plugin version <x.y.z>.
Error: plugin installation failed: <plugin-name>. The installing plugin version <a.b.c> is lower than the existing plugin version <x.y.z>.
It is not recommended to install an older version. To force the installation, use the "--force" option.
```
### Install a plugin from URL
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/suite/plugin/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ var _ = Describe("notation plugin install", func() {
It("with invalid plugin file type", func() {
Host(nil, func(notation *utils.ExecOpts, _ *Artifact, vhost *utils.VirtualHost) {
notation.ExpectFailure().Exec("plugin", "install", "--file", NotationE2EPluginPath).
MatchErrContent("Error: failed to install the plugin: invalid file format. Only support .tar.gz and .zip\n")
MatchErrContent("Error: plugin installation failed: invalid file format. Only .tar.gz and .zip formats are supported\n")
})
})

Expand All @@ -80,7 +80,7 @@ var _ = Describe("notation plugin install", func() {
MatchContent("Succussefully installed plugin e2e-plugin, version 1.0.0\n")

notation.ExpectFailure().Exec("plugin", "install", "--file", NotationE2EPluginTarGzPath).
MatchErrContent("Error: failed to install the plugin: e2e-plugin with version 1.0.0 already exists\n")
MatchErrContent("Error: plugin installation failed: plugin e2e-plugin with version 1.0.0 already exists\n")
})
})

Expand Down Expand Up @@ -111,7 +111,7 @@ var _ = Describe("notation plugin install", func() {
It("with invalid plugin URL scheme", func() {
Host(nil, func(notation *utils.ExecOpts, _ *Artifact, vhost *utils.VirtualHost) {
notation.ExpectFailure().Exec("plugin", "install", "--url", "http://invalid", "--sha256sum", "abcd").
MatchErrContent("Error: failed to install from URL: \"http://invalid\" scheme is not HTTPS\n")
MatchErrContent("Error: the plugin download failed: only the HTTPS scheme is supported, but got \"http\"\n")
})
})

Expand Down

0 comments on commit 0461593

Please sign in to comment.