updates

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>

cmd/notation/internal/plugin/plugin.go

@@ -15,124 +15,163 @@ package plugin
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/notaryproject/notation-go/dir"
+	"github.com/notaryproject/notation-go/log"
 	"github.com/notaryproject/notation-go/plugin"
 	"github.com/notaryproject/notation-go/plugin/proto"
 	notationerrors "github.com/notaryproject/notation/cmd/notation/internal/errors"
 	"github.com/notaryproject/notation/internal/osutil"
+	"github.com/notaryproject/notation/internal/trace"
+	"github.com/notaryproject/notation/internal/version"
 	"github.com/opencontainers/go-digest"
+	"github.com/sirupsen/logrus"
+	"golang.org/x/mod/semver"
+	"oras.land/oras-go/v2/registry/remote/auth"
 )
 
 // PluginSourceType is an enum for plugin source
-type PluginSourceType string
+type PluginSourceType int
 
 const (
-	// TypeFile means plugin source is file
-	TypeFile PluginSourceType = "file"
+	// PluginSourceTypeFile means plugin source is file
+	PluginSourceTypeUnknown PluginSourceType = 1 + iota
 
-	// TypeURL means plugin source is URL
-	TypeURL PluginSourceType = "url"
+	// PluginSourceTypeFile means plugin source is file
+	PluginSourceTypeFile
 
-	// TypeUnknown means unknown plugin source
-	TypeUnknown PluginSourceType = "unknown"
+	// PluginSourceTypeURL means plugin source is URL
+	PluginSourceTypeURL
 )
 
 const (
-	// TypeZip means plugin file is zip
-	TypeZip = "application/zip"
+	// MediaTypeZip means plugin file is zip
+	MediaTypeZip = "application/zip"
 
-	// TypeGzip means plugin file is gzip
-	TypeGzip = "application/x-gzip"
+	// MediaTypeGzip means plugin file is gzip
+	MediaTypeGzip = "application/x-gzip"
 )
 
-// CheckPluginExistence returns true if a plugin already exists
-func CheckPluginExistence(ctx context.Context, pluginName string) (bool, error) {
+// GetPluginMetadataIfExist returns plugin's metadata if it exists in Notation
+func GetPluginMetadataIfExist(ctx context.Context, pluginName string) (*proto.GetMetadataResponse, error) {
 	mgr := plugin.NewCLIManager(dir.PluginFS())
-	_, err := mgr.Get(ctx, pluginName)
+	plugin, err := mgr.Get(ctx, pluginName)
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			return false, nil
-		}
-		return false, err
+		return nil, err
 	}
-	return true, nil
+	return plugin.GetMetadata(ctx, &proto.GetMetadataRequest{})
+}
+
+// GetPluginMetadata returns plugin's metadata given plugin path
+func GetPluginMetadata(ctx context.Context, pluginName, path string) (*proto.GetMetadataResponse, error) {
+	plugin, err := plugin.NewCLIPlugin(ctx, pluginName, path)
+	if err != nil {
+		return nil, err
+	}
+	return plugin.GetMetadata(ctx, &proto.GetMetadataRequest{})
+}
+
+// ComparePluginVersion validates and compares two plugin semantic versions
+func ComparePluginVersion(v, w string) (int, error) {
+	// semantic version strings must begin with a leading "v"
+	if !strings.HasPrefix(v, "v") {
+		v = "v" + v
+	}
+	if !semver.IsValid(v) {
+		return 0, fmt.Errorf("%s is not a valid semantic version", v)
+	}
+	if !strings.HasPrefix(w, "v") {
+		w = "v" + w
+	}
+	if !semver.IsValid(w) {
+		return 0, fmt.Errorf("%s is not a valid semantic version", w)
+	}
+	return semver.Compare(v, w), nil
 }
 
 // ValidateCheckSum returns nil if SHA256 of file at path equals to checkSum.
 func ValidateCheckSum(path string, checkSum string) error {
-	r, err := os.Open(path)
+	rc, err := os.Open(path)
 	if err != nil {
 		return err
 	}
-	defer r.Close()
-	dgst, err := digest.FromReader(r)
+	defer rc.Close()
+	dgst, err := digest.FromReader(rc)
 	if err != nil {
 		return err
 	}
 	enc := dgst.Encoded()
-	if enc != checkSum {
+	if enc != strings.ToLower(checkSum) {
 		return fmt.Errorf("plugin checksum does not match user input. Expecting %s", checkSum)
 	}
 	return nil
 }
 
-// ValidatePluginMetadata validates plugin metadata given plugin name and path,
-// returns the plugin version on success
-func ValidatePluginMetadata(ctx context.Context, pluginName, path string) (string, error) {
-	plugin, err := plugin.NewCLIPlugin(ctx, pluginName, path)
-	if err != nil {
-		return "", err
-	}
-	metadata, err := plugin.GetMetadata(ctx, &proto.GetMetadataRequest{})
-	if err != nil {
-		return "", err
-	}
-	return metadata.Version, nil
-}
-
 // DownloadPluginFromURL downloads plugin file from url to a tmp directory
 // it returns the tmp file path of the downloaded file
-func DownloadPluginFromURL(ctx context.Context, url, tmpDir string) (string, error) {
-	// Create the file
-	tmpFilePath := filepath.Join(tmpDir, "notationPluginTmp")
-	out, err := os.OpenFile(tmpFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
+func DownloadPluginFromURL(ctx context.Context, url string, tmpFile *os.File) error {
+	// Get the data
+	client := getClient(ctx)
+	req, err := http.NewRequest("GET", url, nil)
 	if err != nil {
-		return "", err
+		return err
 	}
-	defer out.Close()
-	// Get the data
-	resp, err := http.Get(url)
+	resp, err := client.Do(req)
 	if err != nil {
-		return "", err
+		return err
 	}
 	defer resp.Body.Close()
 	// Check server response
 	if resp.StatusCode != http.StatusOK {
-		return "", fmt.Errorf("bad status: %s", resp.Status)
+		return fmt.Errorf("bad status: %s", resp.Status)
 	}
-	// Writer the body to file
-	_, err = io.Copy(out, resp.Body)
+	// Write the body to file
+	_, err = io.Copy(tmpFile, resp.Body)
 	if err != nil {
-		return "", err
+		return err
 	}
-	return tmpFilePath, err
+	return nil
 }
 
 // ExtractPluginNameFromExecutableFileName gets plugin name from plugin
 // executable file name based on spec: https://github.com/notaryproject/specifications/blob/main/specs/plugin-extensibility.md#installation
 func ExtractPluginNameFromExecutableFileName(execFileName string) (string, error) {
 	fileName := osutil.FileNameWithoutExtension(execFileName)
+	if !strings.HasPrefix(fileName, proto.Prefix) {
+		return "", notationerrors.ErrorInvalidPluginName{Msg: fmt.Sprintf("invalid plugin executable file name. file name requires format notation-{plugin-name}, but got %s", fileName)}
+	}
 	_, pluginName, found := strings.Cut(fileName, "-")
-	if !found || !strings.HasPrefix(fileName, proto.Prefix) {
+	if !found {
 		return "", notationerrors.ErrorInvalidPluginName{Msg: fmt.Sprintf("invalid plugin executable file name. file name requires format notation-{plugin-name}, but got %s", fileName)}
 	}
 	return pluginName, nil
 }
+
+func setHttpDebugLog(ctx context.Context, authClient *auth.Client) {
+	if logrusLog, ok := log.GetLogger(ctx).(*logrus.Logger); ok && logrusLog.Level != logrus.DebugLevel {
+		return
+	}
+	if authClient.Client == nil {
+		authClient.Client = http.DefaultClient
+	}
+	if authClient.Client.Transport == nil {
+		authClient.Client.Transport = http.DefaultTransport
+	}
+	authClient.Client.Transport = trace.NewTransport(authClient.Client.Transport)
+}
+
+// getClient returns an *auth.Client
+func getClient(ctx context.Context) *auth.Client {
+	client := &auth.Client{
+		Cache:    auth.NewCache(),
+		ClientID: "notation",
+	}
+	client.SetUserAgent("notation/" + version.GetVersion())
+	setHttpDebugLog(ctx, client)
+	return client
+}

cmd/notation/internal/plugin/plugin_test.go

@@ -15,13 +15,38 @@ package plugin
 
 import (
 	"context"
+	"errors"
+	"os"
 	"testing"
 )
 
-func TestCheckPluginExistence(t *testing.T) {
-	exist, err := CheckPluginExistence(context.Background(), "non-exist-plugin")
-	if exist || err != nil {
-		t.Fatalf("expected exist to be false with nil err, got: %v, %s", exist, err)
+func TestGetPluginMetadataIfExist(t *testing.T) {
+	_, err := GetPluginMetadataIfExist(context.Background(), "non-exist-plugin")
+	if !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("expected os.ErrNotExist err, got: %v", err)
+	}
+}
+
+func TestComparePluginVersion(t *testing.T) {
+	comp, err := ComparePluginVersion("v1.0.0", "v1.0.1")
+	if err != nil || comp >= 0 {
+		t.Fatal("expected nil err and negative comp")
+	}
+
+	comp, err = ComparePluginVersion("1.0.0", "1.0.1")
+	if err != nil || comp >= 0 {
+		t.Fatal("expected nil err and negative comp")
+	}
+
+	comp, err = ComparePluginVersion("1.0.1", "1.0.1")
+	if err != nil || comp != 0 {
+		t.Fatal("expected nil err and comp equal to 0")
+	}
+
+	expectedErrMsg := "vabc is not a valid semantic version"
+	_, err = ComparePluginVersion("abc", "1.0.1")
+	if err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected err %s, got %s", expectedErrMsg, err)
 	}
 }