Making logger an interface

Signed-off-by: Pritesh Bandi <[email protected]>

cli/cli.go

@@ -20,15 +20,23 @@ import (
 )
 
 type CLI struct {
-	name string
-	pl   plugin.Plugin
+	name   string
+	pl     plugin.Plugin
+	logger log.Logger
 }
 
 // New creates a new CLI using given plugin
 func New(executableName string, pl plugin.Plugin) *CLI {
+	return NewWithLogger(executableName, pl, &log.DiscardLogger{})
+}
+
+// NewWithLogger creates a new CLI using given plugin and logger
+func NewWithLogger(executableName string, pl plugin.Plugin, l log.Logger) *CLI {
 	return &CLI{
-		name: executableName,
-		pl:   pl}
+		name:   executableName,
+		pl:     pl,
+		logger: l,
+	}
 }
 
 // Execute is main controller that reads/validates commands, parses input, executes relevant plugin functions
@@ -40,41 +48,40 @@ func (c *CLI) Execute(ctx context.Context, args []string) {
 	command := args[1]
 	var resp any
 	var err error
-	logger := log.GetLogger(ctx)
 	switch plugin.Command(command) {
 	case plugin.CommandGetMetadata:
 		var request plugin.GetMetadataRequest
-		err = unmarshalRequest(ctx, &request)
+		err = c.unmarshalRequest(&request)
 		if err == nil {
-			logger.Debugf("executing %s plugin's GetMetadata function", reflect.TypeOf(c.pl))
+			c.logger.Debugf("executing %s plugin's GetMetadata function", reflect.TypeOf(c.pl))
 			resp, err = c.pl.GetMetadata(ctx, &request)
 		}
 	case plugin.CommandGenerateEnvelope:
 		var request plugin.GenerateEnvelopeRequest
-		err = unmarshalRequest(ctx, &request)
+		err = c.unmarshalRequest(&request)
 		if err == nil {
-			logger.Debugf("executing %s plugin's GenerateEnvelope function", reflect.TypeOf(c.pl))
+			c.logger.Debugf("executing %s plugin's GenerateEnvelope function", reflect.TypeOf(c.pl))
 			resp, err = c.pl.GenerateEnvelope(ctx, &request)
 		}
 	case plugin.CommandVerifySignature:
 		var request plugin.VerifySignatureRequest
-		err = unmarshalRequest(ctx, &request)
+		err = c.unmarshalRequest(&request)
 		if err == nil {
-			logger.Debugf("executing %s plugin's VerifySignature function", reflect.TypeOf(c.pl))
+			c.logger.Debugf("executing %s plugin's VerifySignature function", reflect.TypeOf(c.pl))
 			resp, err = c.pl.VerifySignature(ctx, &request)
 		}
 	case plugin.CommandDescribeKey:
 		var request plugin.DescribeKeyRequest
-		err = unmarshalRequest(ctx, &request)
+		err = c.unmarshalRequest(&request)
 		if err == nil {
-			logger.Debugf("executing %s plugin's DescribeKey function", reflect.TypeOf(c.pl))
+			c.logger.Debugf("executing %s plugin's DescribeKey function", reflect.TypeOf(c.pl))
 			resp, err = c.pl.DescribeKey(ctx, &request)
 		}
 	case plugin.CommandGenerateSignature:
 		var request plugin.VerifySignatureRequest
-		err = unmarshalRequest(ctx, &request)
+		err = c.unmarshalRequest(&request)
 		if err == nil {
-			logger.Debugf("executing %s plugin's VerifySignature function", reflect.TypeOf(c.pl))
+			c.logger.Debugf("executing %s plugin's VerifySignature function", reflect.TypeOf(c.pl))
 			resp, err = c.pl.VerifySignature(ctx, &request)
 		}
 	case plugin.Version:
@@ -86,7 +93,7 @@ func (c *CLI) Execute(ctx context.Context, args []string) {
 		deliverError(plugin.NewGenericError("something went wrong").Error())
 	}
 
-	op, pluginErr := marshalResponse(ctx, resp, err)
+	op, pluginErr := c.marshalResponse(resp, err)
 	rescueStdOut()
 	if pluginErr != nil {
 		deliverError(pluginErr.Error())
@@ -96,68 +103,75 @@ func (c *CLI) Execute(ctx context.Context, args []string) {
 
 // printVersion prints version of executable
 func (c *CLI) printVersion(ctx context.Context) {
-	md := getMetadata(ctx, c.pl)
+	md := c.getMetadata(ctx, c.pl)
 
-	fmt.Printf("%s - %s\nVersion: %s", md.Name, md.Description, md.Version)
+	fmt.Printf("%s - %s\nVersion: %s \n", md.Name, md.Description, md.Version)
 	os.Exit(0)
 }
 
 // validateArgs validate commands/arguments passed to executable.
 func (c *CLI) validateArgs(ctx context.Context, args []string) {
-	md := getMetadata(ctx, c.pl)
+	md := c.getMetadata(ctx, c.pl)
 	if !(len(args) == 2 && slices.Contains(getValidArgs(md), args[1])) {
 		deliverError(fmt.Sprintf("Invalid command, valid choices are: %s %s", c.name, getValidArgsString(md)))
 	}
 }
 
-// deferStdout is used to make sure that nothing get emitted to stdout and stderr until intentionally rescued.
-// This is required to make sure that the plugin or its dependency doesn't interfere with notation <-> plugin communication
-func deferStdout() func() {
-	// Ignoring error because we dont want plugin to fail if `os.DevNull` is misconfigured.
-	null, _ := os.Open(os.DevNull)
-	sout := os.Stdout
-	serr := os.Stderr
-	os.Stdout = null
-	os.Stderr = null
-
-	return func() {
-		err := null.Close()
-		if err != nil {
-			return
-		}
-		os.Stdout = sout
-		os.Stderr = serr
-	}
-}
-
 // unmarshalRequest reads input from std.in and unmarshal it into given request struct
-func unmarshalRequest(ctx context.Context, request any) error {
+func (c *CLI) unmarshalRequest(request any) error {
 	if err := json.NewDecoder(os.Stdin).Decode(request); err != nil {
-		logger := log.GetLogger(ctx)
-		logger.Errorf("%s unmarshalling error :%v", reflect.TypeOf(request), err)
+		c.logger.Errorf("%s unmarshalling error :%v", reflect.TypeOf(request), err)
 		return plugin.NewJSONParsingError(plugin.ErrorMsgMalformedInput)
 	}
 	return nil
 }
 
+func (c *CLI) getMetadata(ctx context.Context, p plugin.Plugin) *plugin.GetMetadataResponse {
+	md, err := p.GetMetadata(ctx, &plugin.GetMetadataRequest{})
+	if err != nil {
+		c.logger.Errorf("GetMetadataRequest error :%v", err)
+		deliverError("Error: Failed to get plugin metadata.")
+	}
+	return md
+}
+
 // marshalResponse marshals the given response struct into json
-func marshalResponse(ctx context.Context, response any, err error) (string, *plugin.Error) {
-	logger := log.GetLogger(ctx)
+func (c *CLI) marshalResponse(response any, err error) (string, *plugin.Error) {
 	if err != nil {
-		logger.Errorf("%s error: %v", reflect.TypeOf(response), err)
+		c.logger.Errorf("%s error: %v", reflect.TypeOf(response), err)
 		if plgErr, ok := err.(*plugin.Error); ok {
 			return "", plgErr
 		}
 		return "", plugin.NewGenericError(err.Error())
 	}
 
-	logger.Debug("marshalling response")
+	c.logger.Debug("marshalling response")
 	jsonResponse, err := json.Marshal(response)
 	if err != nil {
-		logger.Errorf("%s marshalling error: %v", reflect.TypeOf(response), err)
+		c.logger.Errorf("%s marshalling error: %v", reflect.TypeOf(response), err)
 		return "", plugin.NewGenericErrorf(plugin.ErrorMsgMalformedOutputFmt, err.Error())
 	}
 
-	logger.Debugf("%s response: %s", reflect.TypeOf(response), jsonResponse)
+	c.logger.Debugf("%s response: %s", reflect.TypeOf(response), jsonResponse)
 	return string(jsonResponse), nil
 }
+
+// deferStdout is used to make sure that nothing get emitted to stdout and stderr until intentionally rescued.
+// This is required to make sure that the plugin or its dependency doesn't interfere with notation <-> plugin communication
+func deferStdout() func() {
+	// Ignoring error because we don't want plugin to fail if `os.DevNull` is misconfigured.
+	null, _ := os.Open(os.DevNull)
+	sout := os.Stdout
+	serr := os.Stderr
+	os.Stdout = null
+	os.Stderr = null
+
+	return func() {
+		err := null.Close()
+		if err != nil {
+			return
+		}
+		os.Stdout = sout
+		os.Stderr = serr
+	}
+}

cli/cli_test.go

@@ -5,21 +5,24 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"os/exec"
 	"reflect"
 	"strings"
 	"testing"
 
+	"github.com/notaryproject/notation-plugin-framework-go/cli/internal/mock"
 	"github.com/notaryproject/notation-plugin-framework-go/plugin"
 )
 
+var cli = New("mockCli", &mockPlugin{})
+
 func TestMarshalResponse(t *testing.T) {
-	ctx := context.Background()
 	res := plugin.GenerateEnvelopeResponse{
 		SignatureEnvelope:     []byte("envelope"),
 		Annotations:           map[string]string{"key": "value"},
 		SignatureEnvelopeType: "envelopeType",
 	}
-	op, err := marshalResponse(ctx, res, nil)
+	op, err := cli.marshalResponse(res, nil)
 	if err != nil {
 		t.Errorf("Error found in marshalResponse: %v", err)
 	}
@@ -31,26 +34,24 @@ func TestMarshalResponse(t *testing.T) {
 }
 
 func TestMarshalResponseError(t *testing.T) {
-	ctx := context.Background()
 
-	_, err := marshalResponse(ctx, nil, fmt.Errorf("expected error thrown"))
+	_, err := cli.marshalResponse(nil, fmt.Errorf("expected error thrown"))
 	assertErr(t, err, plugin.CodeGeneric)
 
-	_, err = marshalResponse(ctx, nil, plugin.NewValidationError("expected validation error thrown"))
+	_, err = cli.marshalResponse(nil, plugin.NewValidationError("expected validation error thrown"))
 	assertErr(t, err, plugin.CodeValidation)
 
-	_, err = marshalResponse(ctx, make(chan int), nil)
+	_, err = cli.marshalResponse(make(chan int), nil)
 	assertErr(t, err, plugin.CodeGeneric)
 }
 
 func TestUnmarshalRequest(t *testing.T) {
-	ctx := context.Background()
 	content := "{\"contractVersion\":\"1.0\",\"keyId\":\"someKeyId\",\"pluginConfig\":{\"pc1\":\"pk1\"}}"
 	closer := setupReader(content)
 	defer closer()
 
 	var request plugin.DescribeKeyRequest
-	if err := unmarshalRequest(ctx, &request); err != nil {
+	if err := cli.unmarshalRequest(&request); err != nil {
 		t.Errorf("unmarshalRequest() failed with error: %v", err)
 	}
 
@@ -60,12 +61,11 @@ func TestUnmarshalRequest(t *testing.T) {
 }
 
 func TestUnmarshalRequestError(t *testing.T) {
-	ctx := context.Background()
 	closer := setupReader("InvalidJson")
 	defer closer()
 
 	var request plugin.DescribeKeyRequest
-	err := unmarshalRequest(ctx, &request)
+	err := cli.unmarshalRequest(&request)
 	if err == nil {
 		t.Fatalf("unmarshalRequest() expected error but not found")
 	}
@@ -79,7 +79,30 @@ func TestUnmarshalRequestError(t *testing.T) {
 	if plgErr.Error() != expectedErrStr {
 		t.Fatalf("unmarshalRequest() expected error string to be %s but found %s", expectedErrStr, plgErr.Error())
 	}
+}
 
+func TestGetMetadata(t *testing.T) {
+	pl := mock.NewPlugin(false)
+	ctx := context.Background()
+
+	cli.getMetadata(ctx, pl)
+}
+
+func TestGetMetadataError(t *testing.T) {
+	if os.Getenv("TEST_OS_EXIT") == "1" {
+		pl := mock.NewPlugin(true)
+		ctx := context.Background()
+
+		cli.getMetadata(ctx, pl)
+		return
+	}
+	cmd := exec.Command(os.Args[0], "-test.run=TestGetMetadataError")
+	cmd.Env = append(os.Environ(), "TEST_OS_EXIT=1")
+	err := cmd.Run()
+	if e, ok := err.(*exec.ExitError); ok && !e.Success() {
+		return
+	}
+	t.Fatalf("process ran with err %v, want exit status 1", err)
 }
 
 func setupReader(content string) func() {
@@ -116,3 +139,27 @@ func assertErr(t *testing.T, err error, code plugin.Code) {
 
 	t.Errorf("expected error of type PluginError but found %s", reflect.TypeOf(err))
 }
+
+type mockPlugin struct {
+}
+
+func (p *mockPlugin) DescribeKey(_ context.Context, _ *plugin.DescribeKeyRequest) (*plugin.DescribeKeyResponse, error) {
+	return nil, plugin.NewUnsupportedError("DescribeKey operation is not implemented by example plugin")
+}
+
+func (p *mockPlugin) GenerateSignature(_ context.Context, _ *plugin.GenerateSignatureRequest) (*plugin.GenerateSignatureResponse, error) {
+	return nil, plugin.NewUnsupportedError("GenerateSignature operation is not implemented by example plugin")
+}
+
+func (p *mockPlugin) GenerateEnvelope(_ context.Context, _ *plugin.GenerateEnvelopeRequest) (*plugin.GenerateEnvelopeResponse, error) {
+	return nil, plugin.NewUnsupportedError("GenerateEnvelope operation is not implemented by example plugin")
+}
+
+func (p *mockPlugin) VerifySignature(_ context.Context, _ *plugin.VerifySignatureRequest) (*plugin.VerifySignatureResponse, error) {
+	return nil, plugin.NewUnsupportedError("VerifySignature operation is not implemented by example plugin")
+
+}
+
+func (p *mockPlugin) GetMetadata(_ context.Context, _ *plugin.GetMetadataRequest) (*plugin.GetMetadataResponse, error) {
+	return nil, plugin.NewUnsupportedError("GetMetadata operation is not implemented by mock plugin")
+}

cli/utils.go

@@ -1,14 +1,12 @@
 package cli
 
 import (
-	"context"
 	"fmt"
 	"os"
 	"sort"
 	"strings"
 
 	"github.com/notaryproject/notation-plugin-framework-go/internal/slices"
-	"github.com/notaryproject/notation-plugin-framework-go/log"
 	"github.com/notaryproject/notation-plugin-framework-go/plugin"
 )
 
@@ -37,16 +35,6 @@ func getValidArgs(md *plugin.GetMetadataResponse) []string {
 	return opts
 }
 
-func getMetadata(ctx context.Context, p plugin.Plugin) *plugin.GetMetadataResponse {
-	md, err := p.GetMetadata(ctx, &plugin.GetMetadataRequest{})
-	if err != nil {
-		logger := log.GetLogger(ctx)
-		logger.Errorf("GetMetadataRequest error :%v", err)
-		deliverError("Error: Failed to get plugin metadata.")
-	}
-	return md
-}
-
 // deliverError print to standard error and then return nonzero exit code
 func deliverError(message string) {
 	_, _ = fmt.Fprintln(os.Stderr, message)