updated per code review

Signed-off-by: Patrick Zheng <[email protected]>

cmd/notation/internal/plugin/plugin.go

@@ -19,7 +19,7 @@ import (
 	"io"
 	"net/http"
 
-	notationauth "github.com/notaryproject/notation/internal/auth"
+	"github.com/notaryproject/notation/internal/httputil"
 )
 
 // MaxPluginSourceBytes specifies the limit on how many bytes are allowed in the
@@ -50,8 +50,8 @@ const (
 // DownloadPluginFromURL downloads plugin file from url to a tmp directory
 func DownloadPluginFromURL(ctx context.Context, pluginURL string, tmpFile io.Writer) error {
 	// Get the data
-	client := notationauth.NewAuthClient(ctx)
-	req, err := http.NewRequest("GET", pluginURL, nil)
+	client := httputil.NewAuthClient(ctx)
+	req, err := http.NewRequest(http.MethodGet, pluginURL, nil)
 	if err != nil {
 		return err
 	}

cmd/notation/plugin/install.go

@@ -47,12 +47,10 @@ type pluginInstallOpts struct {
 	pluginSource     string
 	inputChecksum    string
 	isFile           bool
-	isUrl            bool
+	isURL            bool
 	force            bool
 }
 
-var ErrNoPluginExecutableFileWasFound = errors.New("no plugin executable file was found")
-
 func installCommand(opts *pluginInstallOpts) *cobra.Command {
 	if opts == nil {
 		opts = &pluginInstallOpts{}
@@ -83,26 +81,29 @@ Example - Install plugin from URL, SHA256 checksum is required:
 				if opts.isFile {
 					return errors.New("missing plugin file path")
 				}
-				if opts.isUrl {
+				if opts.isURL {
 					return errors.New("missing plugin URL")
 				}
 				return errors.New("missing plugin source")
 			}
+			if len(args) > 1 {
+				return fmt.Errorf("can only insall one plugin at a time, but got %v", args)
+			}
 			opts.pluginSource = args[0]
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if opts.isFile {
 				opts.pluginSourceType = notationplugin.PluginSourceTypeFile
-			} else if opts.isUrl {
+			} else if opts.isURL {
 				opts.pluginSourceType = notationplugin.PluginSourceTypeURL
 			}
 			return install(cmd, opts)
 		},
 	}
 	opts.LoggingFlagOpts.ApplyFlags(command.Flags())
 	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().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 the plugin")
 	command.MarkFlagsMutuallyExclusive("file", "url")
@@ -156,7 +157,7 @@ func installPlugin(ctx context.Context, inputPath string, inputChecksum string,
 	}
 	// checksum check
 	if inputChecksum != "" {
-		if err := osutil.ValidateChecksum(inputPath, inputChecksum); err != nil {
+		if err := osutil.ValidateSHA256Sum(inputPath, inputChecksum); err != nil {
 			return fmt.Errorf("plugin installation failed: %w", err)
 		}
 	}
@@ -173,9 +174,6 @@ func installPlugin(ctx context.Context, inputPath string, inputChecksum string,
 		}
 		defer rc.Close()
 		if err := installPluginFromFS(ctx, rc, force); err != nil {
-			if errors.Is(err, ErrNoPluginExecutableFileWasFound) {
-				return fmt.Errorf("plugin installation failed: no valid plugin file was found in %s. Plugin file name must in format notation-{plugin-name}", inputPath)
-			}
 			return fmt.Errorf("plugin installation failed: %w", err)
 		}
 		return nil

cmd/notation/registry.go

@@ -23,6 +23,7 @@ import (
 	notationregistry "github.com/notaryproject/notation-go/registry"
 	"github.com/notaryproject/notation/cmd/notation/internal/experimental"
 	notationauth "github.com/notaryproject/notation/internal/auth"
+	"github.com/notaryproject/notation/internal/httputil"
 	"github.com/notaryproject/notation/pkg/configutil"
 	credentials "github.com/oras-project/oras-credentials-go"
 	"oras.land/oras-go/v2/registry"
@@ -140,7 +141,7 @@ func getAuthClient(ctx context.Context, opts *SecureFlagOpts, ref registry.Refer
 	}
 
 	// build authClient
-	authClient := notationauth.NewAuthClient(ctx)
+	authClient := httputil.NewAuthClient(ctx)
 	if !withCredential {
 		return authClient, insecureRegistry, nil
 	}

internal/auth/client.go → internal/httputil/client.go

@@ -11,7 +11,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package auth
+package httputil
 
 import (
 	"context"
@@ -28,6 +28,6 @@ func NewAuthClient(ctx context.Context) *auth.Client {
 		ClientID: "notation",
 	}
 	client.SetUserAgent("notation/" + version.GetVersion())
-	trace.SetHttpDebugLog(ctx, client)
+	trace.SetHTTPDebugLog(ctx, client)
 	return client
 }

internal/osutil/file.go

@@ -136,15 +136,15 @@ func DetectFileType(path string) (string, error) {
 	}
 	defer rc.Close()
 	lr := io.LimitReader(rc, 512)
-	header := make([]byte, 512)
-	if _, err := lr.Read(header); err != nil {
+	header, err := io.ReadAll(lr)
+	if err != nil {
 		return "", err
 	}
 	return http.DetectContentType(header), nil
 }
 
-// ValidateChecksum returns nil if SHA256 of file at path equals to checksum.
-func ValidateChecksum(path string, checksum string) error {
+// ValidateSHA256Sum returns nil if SHA256 of file at path equals to checksum.
+func ValidateSHA256Sum(path string, checksum string) error {
 	rc, err := os.Open(path)
 	if err != nil {
 		return err
@@ -156,7 +156,7 @@ func ValidateChecksum(path string, checksum string) error {
 	}
 	sha256sum := sha256Hash.Sum(nil)
 	enc := strings.ToLower(hex.EncodeToString(sha256sum[:]))
-	if enc != strings.ToLower(checksum) {
+	if !strings.EqualFold(enc, checksum) {
 		return fmt.Errorf("plugin checksum does not match user input. Expecting %s", checksum)
 	}
 	return nil

internal/osutil/file_test.go

@@ -26,7 +26,7 @@ func validFileContent(t *testing.T, filename string, content []byte) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if bytes.Compare(content, b) != 0 {
+	if !bytes.Equal(content, b) {
 		t.Fatal("file content is not correct")
 	}
 }
@@ -262,10 +262,10 @@ func TestCopyToDir(t *testing.T) {
 
 func TestValidateChecksum(t *testing.T) {
 	expectedErrorMsg := "plugin checksum does not match user input. Expecting abcd123"
-	if err := ValidateChecksum("./testdata/test", "abcd123"); err == nil || err.Error() != expectedErrorMsg {
+	if err := ValidateSHA256Sum("./testdata/test", "abcd123"); err == nil || err.Error() != expectedErrorMsg {
 		t.Fatalf("expected err %s, got %v", expectedErrorMsg, err)
 	}
-	if err := ValidateChecksum("./testdata/test", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"); err != nil {
+	if err := ValidateSHA256Sum("./testdata/test", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"); err != nil {
 		t.Fatalf("expected nil err, got %v", err)
 	}
 }

internal/trace/transport.go

@@ -86,8 +86,8 @@ func logHeader(header http.Header, e log.Logger) {
 	}
 }
 
-// SetHttpDebugLog sets up http debug log with logrus.Logger
-func SetHttpDebugLog(ctx context.Context, authClient *auth.Client) {
+// SetHTTPDebugLog sets up http debug log with logrus.Logger
+func SetHTTPDebugLog(ctx context.Context, authClient *auth.Client) {
 	if logrusLog, ok := log.GetLogger(ctx).(*logrus.Logger); ok && logrusLog.Level != logrus.DebugLevel {
 		return
 	}