feat: pass logger into repo and client (#385)

* feat: pass logger into repo and client

Signed-off-by: Asra Ali <asraa@google.com>

.golangci.yml

@@ -13,3 +13,4 @@ linters:
     - gosimple
     - unused
     - typecheck
+    - forbidigo

cmd/tuf-client/main.go

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"log"
+	"os"
 
 	docopt "github.com/flynn/go-docopt"
 	tuf "github.com/theupdateframework/go-tuf/client"
@@ -32,7 +33,7 @@ See "tuf-client help <command>" for more information on a specific command.
 
 	if cmd == "help" {
 		if len(cmdArgs) == 0 { // `tuf-client help`
-			fmt.Println(usage)
+			fmt.Fprint(os.Stderr, usage)
 			return
 		} else { // `tuf-client help <command>`
 			cmd = cmdArgs[0]

cmd/tuf/gen_key.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"os"
 	"time"
 
 	"github.com/flynn/go-docopt"
@@ -39,7 +40,7 @@ func cmdGenKey(args *docopt.Args, repo *tuf.Repo) error {
 		string(data.KeySchemeRSASSA_PSS_SHA256):
 		keyScheme = data.KeyScheme(t)
 	default:
-		fmt.Println("Using default key scheme", keyScheme)
+		fmt.Fprint(os.Stderr, "Using default key scheme", keyScheme)
 	}
 
 	var err error
@@ -57,7 +58,7 @@ func cmdGenKey(args *docopt.Args, repo *tuf.Repo) error {
 		return err
 	}
 	for _, id := range keyids {
-		fmt.Println("Generated", role, keyScheme, "key with ID", id)
+		fmt.Fprintf(os.Stdout, "Generated %s %s key with ID %s", role, keyScheme, id)
 	}
 	return nil
 }

cmd/tuf/get_threshold.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/flynn/go-docopt"
 	"github.com/theupdateframework/go-tuf"
@@ -23,6 +24,6 @@ func cmdGetThreshold(args *docopt.Args, repo *tuf.Repo) error {
 		return err
 	}
 
-	fmt.Println("The threshold for", role, "role is", threshold)
+	fmt.Fprintf(os.Stdout, "The threshold for %s role is %d", role, threshold)
 	return nil
 }

cmd/tuf/main.go

@@ -58,7 +58,7 @@ See "tuf help <command>" for more information on a specific command
 
 	if cmd == "help" {
 		if len(cmdArgs) == 0 { // `tuf help`
-			fmt.Println(usage)
+			fmt.Fprint(os.Stderr, usage)
 			return
 		} else { // `tuf help <command>`
 			cmd = cmdArgs[0]
@@ -115,7 +115,11 @@ func runCommand(name string, args []string, dir string, insecure bool) error {
 	if !insecure {
 		p = getPassphrase
 	}
-	repo, err := tuf.NewRepo(tuf.FileSystemStore(dir, p))
+	logger := log.New(os.Stdout, "", 0)
+	storeOpts := tuf.StoreOpts{Logger: logger, PassFunc: p}
+
+	repo, err := tuf.NewRepoWithOpts(tuf.FileSystemStoreWithOpts(dir, storeOpts),
+		tuf.WithLogger(logger))
 	if err != nil {
 		return err
 	}

cmd/tuf/payload.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"os"
 
 	"github.com/flynn/go-docopt"
 	"github.com/theupdateframework/go-tuf"
@@ -20,6 +21,6 @@ func cmdPayload(args *docopt.Args, repo *tuf.Repo) error {
 	if err != nil {
 		return err
 	}
-	fmt.Print(string(p))
+	fmt.Fprint(os.Stdout, string(p))
 	return nil
 }

cmd/tuf/set_threshold.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"os"
 	"strconv"
 
 	"github.com/flynn/go-docopt"
@@ -28,6 +29,6 @@ func cmdSetThreshold(args *docopt.Args, repo *tuf.Repo) error {
 		return err
 	}
 
-	fmt.Println("The threshold for", role, "role is now", threshold)
+	fmt.Fprintf(os.Stdout, "The threshold for %s role is now %d", role, threshold)
 	return nil
 }

cmd/tuf/sign_payload.go

@@ -36,7 +36,7 @@ func cmdSignPayload(args *docopt.Args, repo *tuf.Repo) error {
 	if err != nil {
 		return err
 	}
-	fmt.Print(string(bytes))
+	fmt.Fprint(os.Stdout, string(bytes))
 
 	fmt.Fprintln(os.Stderr, "tuf: signed with", numKeys, "key(s)")
 	return nil

internal/fsutil/perm_test.go

@@ -4,7 +4,6 @@
 package fsutil
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
@@ -59,7 +58,6 @@ func TestEnsureMaxPermissions(t *testing.T) {
 	assert.NoError(t, err)
 	err = EnsureMaxPermissions(fi, os.FileMode(0222))
 	assert.Error(t, err)
-	fmt.Println(err)
 
 	// Check matching due to more restrictive perms on file
 	err = os.Chmod(p, 0444)

local_store.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -197,18 +198,44 @@ type persistedKeys struct {
 	Data      json.RawMessage `json:"data"`
 }
 
+type StoreOpts struct {
+	Logger   *log.Logger
+	PassFunc util.PassphraseFunc
+}
+
 func FileSystemStore(dir string, p util.PassphraseFunc) LocalStore {
 	return &fileSystemStore{
 		dir:            dir,
 		passphraseFunc: p,
+		logger:         log.New(io.Discard, "", 0),
 		signerForKeyID: make(map[string]keys.Signer),
 		keyIDsForRole:  make(map[string][]string),
 	}
 }
 
+func FileSystemStoreWithOpts(dir string, opts ...StoreOpts) LocalStore {
+	store := &fileSystemStore{
+		dir:            dir,
+		passphraseFunc: nil,
+		logger:         log.New(io.Discard, "", 0),
+		signerForKeyID: make(map[string]keys.Signer),
+		keyIDsForRole:  make(map[string][]string),
+	}
+	for _, opt := range opts {
+		if opt.Logger != nil {
+			store.logger = opt.Logger
+		}
+		if opt.PassFunc != nil {
+			store.passphraseFunc = opt.PassFunc
+		}
+	}
+	return store
+}
+
 type fileSystemStore struct {
 	dir            string
 	passphraseFunc util.PassphraseFunc
+	logger         *log.Logger
 
 	signerForKeyID map[string]keys.Signer
 	keyIDsForRole  map[string][]string
@@ -526,7 +553,7 @@ func (f *fileSystemStore) ChangePassphrase(role string) error {
 	keys, _, err := f.loadPrivateKeys(role)
 	if err != nil {
 		if os.IsNotExist(err) {
-			fmt.Printf("Failed to change passphrase. Missing keys file for %s role. \n", role)
+			f.logger.Printf("Failed to change passphrase. Missing keys file for %s role. \n", role)
 		}
 		return err
 	}
@@ -548,7 +575,7 @@ func (f *fileSystemStore) ChangePassphrase(role string) error {
 	if err := util.AtomicallyWriteFile(f.keysPath(role), append(data, '\n'), 0600); err != nil {
 		return err
 	}
-	fmt.Printf("Successfully changed passphrase for %s keys file\n", role)
+	f.logger.Printf("Successfully changed passphrase for %s keys file\n", role)
 	return nil
 }
 

pkg/keys/deprecated_ecdsa.go

@@ -9,7 +9,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
 
 	"github.com/theupdateframework/go-tuf/data"
 )
@@ -98,6 +97,5 @@ func (p *deprecatedP256Verifier) UnmarshalPublicKey(key *data.PublicKey) error {
 	}
 
 	p.key = key
-	fmt.Fprintln(os.Stderr, "tuf: warning using deprecated ecdsa hex-encoded keys")
 	return nil
 }

repo.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log"
 	"path"
 	"sort"
 	"strings"
@@ -48,18 +49,47 @@ type Repo struct {
 	meta           map[string]json.RawMessage
 	prefix         string
 	indent         string
+	logger         *log.Logger
+}
+
+type RepoOpts func(r *Repo)
+
+func WithLogger(logger *log.Logger) RepoOpts {
+	return func(r *Repo) {
+		r.logger = logger
+	}
+}
+
+func WithHashAlgorithms(hashAlgorithms ...string) RepoOpts {
+	return func(r *Repo) {
+		r.hashAlgorithms = hashAlgorithms
+	}
+}
+
+func WithPrefix(prefix string) RepoOpts {
+	return func(r *Repo) {
+		r.prefix = prefix
+	}
+}
+
+func WithIndex(indent string) RepoOpts {
+	return func(r *Repo) {
+		r.indent = indent
+	}
 }
 
 func NewRepo(local LocalStore, hashAlgorithms ...string) (*Repo, error) {
 	return NewRepoIndent(local, "", "", hashAlgorithms...)
 }
 
-func NewRepoIndent(local LocalStore, prefix string, indent string, hashAlgorithms ...string) (*Repo, error) {
+func NewRepoIndent(local LocalStore, prefix string, indent string,
+	hashAlgorithms ...string) (*Repo, error) {
 	r := &Repo{
 		local:          local,
 		hashAlgorithms: hashAlgorithms,
 		prefix:         prefix,
 		indent:         indent,
+		logger:         log.New(io.Discard, "", 0),
 	}
 
 	var err error
@@ -70,6 +100,17 @@ func NewRepoIndent(local LocalStore, prefix string, indent string, hashAlgorithm
 	return r, nil
 }
 
+func NewRepoWithOpts(local LocalStore, opts ...RepoOpts) (*Repo, error) {
+	r, err := NewRepo(local)
+	if err != nil {
+		return nil, err
+	}
+	for _, opt := range opts {
+		opt(r)
+	}
+	return r, nil
+}
+
 func (r *Repo) Init(consistentSnapshot bool) error {
 	t, err := r.topLevelTargets()
 	if err != nil {
@@ -91,7 +132,7 @@ func (r *Repo) Init(consistentSnapshot bool) error {
 		return err
 	}
 
-	fmt.Println("Repository initialized")
+	r.logger.Println("Repository initialized")
 	return nil
 }
 
@@ -533,7 +574,7 @@ func (r *Repo) RevokeKeyWithExpires(keyRole, id string, expires time.Time) error
 
 	err = r.setMeta("root.json", root)
 	if err == nil {
-		fmt.Println("Revoked", keyRole, "key with ID", id, "in root metadata")
+		r.logger.Println("Revoked", keyRole, "key with ID", id, "in root metadata")
 	}
 	return err
 }
@@ -783,7 +824,7 @@ func (r *Repo) Sign(roleFilename string) error {
 	r.meta[roleFilename] = b
 	err = r.local.SetMeta(roleFilename, b)
 	if err == nil {
-		fmt.Println("Signed", roleFilename, "with", numKeys, "key(s)")
+		r.logger.Println("Signed", roleFilename, "with", numKeys, "key(s)")
 	}
 	return err
 }
@@ -1223,7 +1264,7 @@ func (r *Repo) removeTargetsWithExpiresFromMeta(metaName string, paths []string,
 		for _, path := range paths {
 			path = util.NormalizeTarget(path)
 			if _, ok := t.Targets[path]; !ok {
-				fmt.Printf("[%v] The following target is not present: %v\n", metaName, path)
+				r.logger.Printf("[%v] The following target is not present: %v\n", metaName, path)
 				continue
 			}
 			removed = true
@@ -1243,17 +1284,17 @@ func (r *Repo) removeTargetsWithExpiresFromMeta(metaName string, paths []string,
 
 	err = r.setMeta(metaName, t)
 	if err == nil {
-		fmt.Printf("[%v] Removed targets:\n", metaName)
+		r.logger.Printf("[%v] Removed targets:\n", metaName)
 		for _, v := range removed_targets {
-			fmt.Println("*", v)
+			r.logger.Println("*", v)
 		}
 		if len(t.Targets) != 0 {
-			fmt.Printf("[%v] Added/staged targets:\n", metaName)
+			r.logger.Printf("[%v] Added/staged targets:\n", metaName)
 			for k := range t.Targets {
-				fmt.Println("*", k)
+				r.logger.Println("*", k)
 			}
 		} else {
-			fmt.Printf("[%v] There are no added/staged targets\n", metaName)
+			r.logger.Printf("[%v] There are no added/staged targets\n", metaName)
 		}
 	}
 	return err
@@ -1307,7 +1348,7 @@ func (r *Repo) SnapshotWithExpires(expires time.Time) error {
 	}
 	err = r.setMeta("snapshot.json", snapshot)
 	if err == nil {
-		fmt.Println("Staged snapshot.json metadata with expiration date:", snapshot.Expires)
+		r.logger.Println("Staged snapshot.json metadata with expiration date:", snapshot.Expires)
 	}
 	return err
 }
@@ -1339,7 +1380,7 @@ func (r *Repo) TimestampWithExpires(expires time.Time) error {
 
 	err = r.setMeta("timestamp.json", timestamp)
 	if err == nil {
-		fmt.Println("Staged timestamp.json metadata with expiration date:", timestamp.Expires)
+		r.logger.Println("Staged timestamp.json metadata with expiration date:", timestamp.Expires)
 	}
 	return err
 }
@@ -1505,15 +1546,15 @@ func (r *Repo) Commit() error {
 
 	err = r.local.Commit(root.ConsistentSnapshot, versions, hashes)
 	if err == nil {
-		fmt.Println("Committed successfully")
+		r.logger.Println("Committed successfully")
 	}
 	return err
 }
 
 func (r *Repo) Clean() error {
 	err := r.local.Clean()
 	if err == nil {
-		fmt.Println("Removed all staged metadata and target files")
+		r.logger.Println("Removed all staged metadata and target files")
 	}
 	return err
 }