Merge pull request #2292 from posit-dev/mnv-creds-logs

Credentials service with a logger

cmd/publisher/commands/credentials.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/posit-dev/publisher/internal/cli_types"
 	"github.com/posit-dev/publisher/internal/credentials"
+	"github.com/posit-dev/publisher/internal/logging"
 )
 
 type CredentialsCommand struct {
@@ -24,7 +25,7 @@ type CreateCredentialCommand struct {
 }
 
 func (cmd *CreateCredentialCommand) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) error {
-	cs := credentials.CredentialsService{}
+	cs := credentials.NewCredentialsService(logging.NewDiscardLogger())
 	cred, err := cs.Set(cmd.Name, cmd.URL, cmd.ApiKey)
 	if err != nil {
 		return err
@@ -44,7 +45,7 @@ type DeleteCredentialCommand struct {
 }
 
 func (cmd *DeleteCredentialCommand) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) error {
-	cs := credentials.CredentialsService{}
+	cs := credentials.NewCredentialsService(logging.NewDiscardLogger())
 	err := cs.Delete(cmd.GUID)
 	if err != nil {
 		return err
@@ -59,7 +60,7 @@ type GetCredentialCommand struct {
 }
 
 func (cmd *GetCredentialCommand) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) error {
-	cs := credentials.CredentialsService{}
+	cs := credentials.NewCredentialsService(logging.NewDiscardLogger())
 	cred, err := cs.Get(cmd.GUID)
 	if err != nil {
 		return err
@@ -78,7 +79,7 @@ type ListCredentialsCommand struct {
 }
 
 func (cmd *ListCredentialsCommand) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) error {
-	cs := credentials.CredentialsService{}
+	cs := credentials.NewCredentialsService(logging.NewDiscardLogger())
 	creds, err := cs.List()
 	if err != nil {
 		return err

internal/accounts/account_list.go

@@ -31,7 +31,7 @@ var _ AccountList = &defaultAccountList{}
 func NewAccountList(fs afero.Fs, log logging.Logger) *defaultAccountList {
 	return &defaultAccountList{
 		providers: []AccountProvider{
-			NewCredentialsProvider(),
+			NewCredentialsProvider(log),
 		},
 		log: log,
 	}

internal/accounts/provider_credentials.go

@@ -2,14 +2,19 @@
 
 package accounts
 
-import "github.com/posit-dev/publisher/internal/credentials"
+import (
+	"github.com/posit-dev/publisher/internal/credentials"
+	"github.com/posit-dev/publisher/internal/logging"
+)
 
 type CredentialsProvider struct {
 	cs credentials.CredentialsService
 }
 
-func NewCredentialsProvider() *CredentialsProvider {
-	return &CredentialsProvider{cs: credentials.CredentialsService{}}
+func NewCredentialsProvider(log logging.Logger) *CredentialsProvider {
+	return &CredentialsProvider{
+		cs: credentials.NewCredentialsService(log),
+	}
 }
 
 func (p *CredentialsProvider) Load() ([]Account, error) {

internal/credentials/credentials.go

@@ -32,6 +32,7 @@ import (
 	"net/url"
 
 	"github.com/google/uuid"
+	"github.com/posit-dev/publisher/internal/logging"
 	"github.com/posit-dev/publisher/internal/util"
 	"github.com/spf13/afero"
 	"github.com/zalando/go-keyring"
@@ -74,8 +75,21 @@ func (cr *CredentialRecord) ToCredential() (*Credential, error) {
 	}
 }
 
-type CredentialsService struct {
+type CredentialsService interface {
+	FileCredentialRecordFactory() (*CredentialRecord, error)
+	Delete(guid string) error
+	Get(guid string) (*Credential, error)
+	List() ([]Credential, error)
+	Set(name string, url string, ak string) (*Credential, error)
+}
+
+type credentialsService struct {
 	afs afero.Fs
+	log logging.Logger
+}
+
+func NewCredentialsService(log logging.Logger) *credentialsService {
+	return &credentialsService{log: log}
 }
 
 // FileCredentialRecordFactory creates a Credential based on the presence of the
@@ -86,7 +100,7 @@ type fileCredential struct {
 	Key string `toml:"key"`
 }
 
-func (cs *CredentialsService) FileCredentialRecordFactory() (*CredentialRecord, error) {
+func (cs *credentialsService) FileCredentialRecordFactory() (*CredentialRecord, error) {
 	homeDir, err := util.UserHomeDir(cs.afs)
 	if err != nil {
 		return nil, err
@@ -148,7 +162,7 @@ func (cs *CredentialsService) FileCredentialRecordFactory() (*CredentialRecord,
 	return nil, nil
 }
 
-func (cs *CredentialsService) checkForConflicts(
+func (cs *credentialsService) checkForConflicts(
 	table *map[string]CredentialRecord,
 	c *Credential) error {
 	// Check if Credential attributes (URL or name) are already used by another credential
@@ -187,7 +201,7 @@ func (cs *CredentialsService) checkForConflicts(
 
 // Delete removes a Credential by its guid.
 // If lookup by guid fails, a NotFoundError is returned.
-func (cs *CredentialsService) Delete(guid string) error {
+func (cs *credentialsService) Delete(guid string) error {
 
 	table, err := cs.load()
 	if err != nil {
@@ -196,6 +210,7 @@ func (cs *CredentialsService) Delete(guid string) error {
 
 	_, exists := table[guid]
 	if !exists {
+		cs.log.Debug("Credential does not exist", "credential", guid)
 		return &NotFoundError{GUID: guid}
 	}
 
@@ -209,22 +224,23 @@ func (cs *CredentialsService) Delete(guid string) error {
 }
 
 // Get retrieves a Credential by its guid.
-func (cs *CredentialsService) Get(guid string) (*Credential, error) {
+func (cs *credentialsService) Get(guid string) (*Credential, error) {
 	table, err := cs.load()
 	if err != nil {
 		return nil, err
 	}
 
 	cr, exists := table[guid]
 	if !exists {
+		cs.log.Debug("Credential does not exist", "credential", guid)
 		return nil, &NotFoundError{GUID: guid}
 	}
 
 	return cr.ToCredential()
 }
 
 // List retrieves all Credentials
-func (cs *CredentialsService) List() ([]Credential, error) {
+func (cs *credentialsService) List() ([]Credential, error) {
 	records, err := cs.load()
 	if err != nil {
 		return nil, err
@@ -242,7 +258,7 @@ func (cs *CredentialsService) List() ([]Credential, error) {
 
 // Set creates a Credential.
 // A guid is assigned to the Credential using the UUIDv4 specification.
-func (cs *CredentialsService) Set(name string, url string, ak string) (*Credential, error) {
+func (cs *credentialsService) Set(name string, url string, ak string) (*Credential, error) {
 	table, err := cs.load()
 	if err != nil {
 		return nil, err
@@ -286,7 +302,7 @@ func (cs *CredentialsService) Set(name string, url string, ak string) (*Credenti
 }
 
 // Saves the CredentialTable, but removes Env Credentials first
-func (cs *CredentialsService) save(table CredentialTable) error {
+func (cs *credentialsService) save(table CredentialTable) error {
 
 	// remove any environment variable credential from the table
 	// before saving
@@ -299,7 +315,7 @@ func (cs *CredentialsService) save(table CredentialTable) error {
 }
 
 // Saves the CredentialTable to keyring
-func (cs *CredentialsService) saveToKeyRing(table CredentialTable) error {
+func (cs *credentialsService) saveToKeyRing(table CredentialTable) error {
 	data, err := json.Marshal(table)
 	if err != nil {
 		return fmt.Errorf("failed to serialize credentials: %v", err)
@@ -314,24 +330,28 @@ func (cs *CredentialsService) saveToKeyRing(table CredentialTable) error {
 }
 
 // Loads the CredentialTable with keyring and env values
-func (cs *CredentialsService) load() (CredentialTable, error) {
+func (cs *credentialsService) load() (CredentialTable, error) {
 	table, err := cs.loadFromKeyRing()
 	if err != nil {
+		cs.log.Debug("Failed to load credentials from system keyring", "error", err.Error())
 		return nil, err
 	}
 
 	// insert a possible file-based credential before returning
 	record, err := cs.FileCredentialRecordFactory()
 	if err != nil {
+		cs.log.Debug("Failed to create file based credentials records instance", "error", err.Error())
 		return nil, err
 	}
 	if record != nil {
 		c, err := record.ToCredential()
 		if err != nil {
+			cs.log.Debug("Error building credentials from file record", "error", err.Error())
 			return nil, err
 		}
 		err = cs.checkForConflicts(&table, c)
 		if err != nil {
+			cs.log.Debug("Conflict on credential record", "error", err.Error())
 			return nil, err
 		}
 		table[EnvVarGUID] = *record
@@ -340,7 +360,7 @@ func (cs *CredentialsService) load() (CredentialTable, error) {
 }
 
 // Loads the CredentialTable from keyRing
-func (cs *CredentialsService) loadFromKeyRing() (CredentialTable, error) {
+func (cs *credentialsService) loadFromKeyRing() (CredentialTable, error) {
 	ks := KeyringService{}
 	data, err := ks.Get(ServiceName, "credentials")
 	if err != nil {