chore: logging/err cleanup

7 files changed, 50 insertions, 47 deletions

diff --git a/internal/admin/model.go b/internal/admin/model.go
index 8618d69..fef73ca 100644
--- a/internal/admin/model.go
+++ b/internal/admin/model.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"io/fs"
 	"log"
+	"log/slog"
 	"os"
 	"path/filepath"
 
@@ -103,14 +104,11 @@ func loadFromGit(gitURL, filePath string) ([]byte, error) {
 		URL: gitURL,
 	})
 	if err != nil {
-		// log.error
-		fmt.Printf("couldn't clone mgmt repo %s", err)
-		return []byte(""), errors.New("couldn't clone mgmt repo")
+		return []byte(""), fmt.Errorf("couldn't clone mgmt repo %w", err)
 	}
 	file, err := fs.Open(filePath)
 	if err != nil {
-		fmt.Printf("Failed to open gitserver config %s", err)
-		return []byte(""), errors.New("couldn't open git config file from mgmt repo")
+		return []byte(""), fmt.Errorf("couldn't open file in repo %w", err)
 	}
 	defer file.Close()
 	return io.ReadAll(file)
@@ -119,14 +117,12 @@ func loadFromGit(gitURL, filePath string) ([]byte, error) {
 func loadLocalFile(path string) ([]byte, error) {
 	file, err := os.Open(path)
 	if err != nil {
-		log.Printf("config file not opened %s", path)
-		return []byte{}, err
+		return []byte{}, fmt.Errorf("config file not opened or found %w", err)
 	}
 	defer file.Close()
 	configBytes, err := io.ReadAll(file)
 	if err != nil {
-		log.Print("config file not read")
-		return []byte{}, err
+		return []byte{}, fmt.Errorf("config file not read %w", err)
 	}
 	return configBytes, nil
 }
@@ -149,14 +145,14 @@ func loadServerConfig(mgmtRepo bool, baseDir, configPath string) (*ServerRepos,
 		configBytes, err = loadFromGit(repoURI, configPath)
 		if err != nil {
 			// log.error
-			log.Print("Failed to load config file from git")
+			slog.Error("Failed to load config file from git", "path", configPath)
 			return &ServerRepos{}, err
 		}
 	} else {
 		configBytes, err = loadLocalFile(configPath)
 		if err != nil {
 			// log.error
-			log.Print("Failed to load config file from file system")
+			slog.Error("Local server config couldn't be loaded")
 			return &ServerRepos{}, err
 		}
 	}
@@ -178,10 +174,11 @@ func (s *ServerRepos) ServerPolicies() [][]string {
 }
 
 // ConfigureRepos run reconciler for all repos
-func (s *ServerRepos) ConfigureRepos() {
+func (s *ServerRepos) ConfigureRepos() error {
 	for _, repo := range s.Repos {
-		repo.ReconcileRepo(s.basePath)
+		return repo.ReconcileRepo(s.basePath)
 	}
+	return nil
 }
 
 func readOnlyPaths(role, repoName string) [][]string {
@@ -219,7 +216,7 @@ func (r *GitRepo) CasbinPolicies() [][]string {
 }
 
 // ReconcileRepo update repo export settings, update web config
-func (r *GitRepo) ReconcileRepo(basePath string) {
+func (r *GitRepo) ReconcileRepo(basePath string) error {
 	// if exist -> continue
 	repoBase := filepath.Join(basePath, fmt.Sprintf("%s.git", r.Name))
 	_, err := os.Stat(repoBase)
@@ -237,7 +234,7 @@ func (r *GitRepo) ReconcileRepo(basePath string) {
 		f, err := os.Create(okExport)
 		f.Close()
 		if err != nil {
-			log.Fatalf("%s couldn't be created %s", GitExportMagic, err)
+			return fmt.Errorf("%s couldn't be created %w", GitExportMagic, err)
 		}
 	}
 	r.ConfigureExport(repoBase)
@@ -245,6 +242,7 @@ func (r *GitRepo) ReconcileRepo(basePath string) {
 		r.GitWebConfig = &GitWeb{}
 	}
 	r.GitWebConfig.ReconcileGitConf(repoBase)
+	return nil
 }
 
 // ConfigureExport setup repo for sharing and configure web settings
diff --git a/internal/admin/model_test.go b/internal/admin/model_test.go
index 85d6ed1..6f8531d 100644
--- a/internal/admin/model_test.go
+++ b/internal/admin/model_test.go
@@ -299,7 +299,7 @@ func TestRepoReconcile(t *testing.T) {
 		},
 	}
 	repoPath := filepath.Join(tempDir, fmt.Sprintf("%s.git", repo.Name))
-	repo.ReconcileRepo(tempDir)
+	_ = repo.ReconcileRepo(tempDir)
 	if _, err := os.Stat(repoPath); errors.Is(err, fs.ErrNotExist) {
 		t.Fatal("expected repo to be created, but does not exist")
 	}
@@ -314,7 +314,7 @@ bare = true
 		t.Fatal(err)
 	}
 	// re-reconcile
-	repo.ReconcileRepo(tempDir)
+	_ = repo.ReconcileRepo(tempDir)
 	content, err := os.ReadFile(tempConfigFile)
 	if err != nil {
 		t.Fatal(err)
diff --git a/internal/admin/service.go b/internal/admin/service.go
index 267a243..182c153 100644
--- a/internal/admin/service.go
+++ b/internal/admin/service.go
@@ -2,7 +2,8 @@ package admin
 
 import (
 	"errors"
-	"log"
+	"fmt"
+	"log/slog"
 
 	casbin "github.com/casbin/casbin/v2"
 )
@@ -21,57 +22,58 @@ func (s *Servicer) Reload() {
 	tmpConfig, err := loadServerConfig(s.mgmtRepo, s.reposDir, s.serverConfigPath)
 	if err != nil {
 		// log.error
-		log.Printf("failed to load config %s", err)
-		log.Print("refusing to reload config")
+		slog.Error("failed to load config", "error", err)
+		slog.Error("refusing to reload config")
 		return
 	}
 	s.Conf = tmpConfig
-	s.InitServer()
+	_ = s.InitServer()
 }
 
 // InitServer initialize a git server and configure
-func (s *Servicer) InitServer() {
+func (s *Servicer) InitServer() error {
 	policies := s.Conf.ServerPolicies()
-	log.Print("policies generated")
 	numAdded := 0
 	for _, policy := range policies {
 		added, err := s.AddPolicy(policy[0], policy[1], policy[2])
 		if err != nil {
 			// log.error
-			log.Printf("error adding policy %s %s %s error %s", policy[0], policy[1], policy[2], err)
+			slog.Error("error adding policy", "role", policy[0], "slug", policy[1], "action", policy[2], "err", err)
 			continue
 		}
 		if added {
 			numAdded++
 		}
 	}
-	log.Printf("policies added %d", numAdded)
+	slog.Info("policy generated", "added", numAdded)
 	if err := s.SavePolicy(); err != nil {
-		log.Print("couldn't save policy")
+		return fmt.Errorf("policy couldn't be saved %w", err)
 	}
-	log.Printf("policies saved")
+	slog.Info("policy saved")
 	if err := s.LoadPolicy(); err != nil {
-		log.Print("cloudn't load policy")
+		return fmt.Errorf("cloudn't load policy %w", err)
 	}
-	log.Print("policies loaded")
-	s.Conf.ConfigureRepos()
-	log.Print("configured repos")
+	slog.Info("policy loaded")
+	if err := s.Conf.ConfigureRepos(); err != nil {
+		return fmt.Errorf("couldn't configure repos %w", err)
+	}
+	slog.Info("configured repos")
+	return nil
 }
 
 // NewService create a new admin service, load config, and generate policies
-func NewService(modelPath, policyPath, serverConfigPath, reposDir string, mgmtRepo bool) *Servicer {
+func NewService(modelPath, policyPath, serverConfigPath, reposDir string, mgmtRepo bool) (*Servicer, error) {
 	enf, err := casbin.NewSyncedEnforcer(modelPath, policyPath)
 	if err != nil {
-		log.Fatalf("Couldn't load the enforcer encountered the following error: %s", err)
+		return &Servicer{}, fmt.Errorf("Couldn't load the enforcer encountered the following error: %w", err)
 	}
 	conf, err := loadServerConfig(mgmtRepo, reposDir, serverConfigPath)
 	if errors.Is(err, ErrMgmtRepoNotFound) {
-		log.Print("no server config found, using default")
+		slog.Info("no server config found, using default")
 		conf = defaultServerConfig
 
 	} else if err != nil {
-		// log.error
-		log.Fatalf("Coudln't load server config %s", err)
+		return &Servicer{}, fmt.Errorf("Coudln't load server config. %w", err)
 	}
 	conf.basePath = reposDir
 	svc := &Servicer{
@@ -81,6 +83,8 @@ func NewService(modelPath, policyPath, serverConfigPath, reposDir string, mgmtRe
 		reposDir,
 		mgmtRepo,
 	}
-	svc.InitServer()
-	return svc
+	if err := svc.InitServer(); err != nil {
+		return nil, err
+	}
+	return svc, nil
 }
diff --git a/internal/admin/service_test.go b/internal/admin/service_test.go
index 8931c9d..bbdab85 100644
--- a/internal/admin/service_test.go
+++ b/internal/admin/service_test.go
@@ -84,7 +84,7 @@ func TestInitServer(t *testing.T) {
 	// end config
 
 	t.Run("test reload config success", func(t *testing.T) {
-		svc := NewService(destModelFile,
+		svc, _ := NewService(destModelFile,
 			destPolicyFile,
 			filepath.Join(tempRepoDir, "gitserver.yaml"),
 			tempRepoDir,
@@ -106,7 +106,7 @@ func TestInitServer(t *testing.T) {
 		}
 	})
 	t.Run("test reload config err", func(t *testing.T) {
-		svc := NewService(destModelFile,
+		svc, _ := NewService(destModelFile,
 			destPolicyFile,
 			// TODO set abs path
 			filepath.Join(tempRepoDir, "gitserver.yaml"),
@@ -131,7 +131,7 @@ func TestInitServer(t *testing.T) {
 	})
 	t.Run("test an unitialized server config", func(t *testing.T) {
 		tempRepoDir := t.TempDir()
-		svc := NewService(destModelFile,
+		svc, _ := NewService(destModelFile,
 			destPolicyFile,
 			// TODO set abs path
 			"gitserver.yaml",
diff --git a/internal/authz/middleware.go b/internal/authz/middleware.go
index 3c029b0..9031c99 100644
--- a/internal/authz/middleware.go
+++ b/internal/authz/middleware.go
@@ -5,7 +5,7 @@ import (
 	"context"
 	"encoding/hex"
 	"fmt"
-	"log"
+	"log/slog"
 	"net/http"
 
 	"git.ofmax.li/go-git-server/internal/admin"
@@ -60,16 +60,16 @@ func Authorization(adminSvc *admin.Servicer, next http.Handler) http.Handler {
 		action := req.Method
 		ok, err := adminSvc.Enforce(urn, repo, action)
 		if err != nil {
-			log.Printf("error running enforce %s", err)
+			slog.Info("error unning enforce", "error", err)
 			http.Error(rw, "Bad Request", http.StatusBadRequest)
 			return
 		}
 		if !ok {
-			log.Printf("Not Authorized - %s attempted access %s", urn, repo)
+			slog.Info("Not Authorized", "urn", urn, "repo", repo)
 			http.Error(rw, "Access denied", http.StatusForbidden)
 			return
 		}
-		log.Printf("Method %s Url %s", action, repo)
+		slog.Debug("Access Attempt", "action", action, "repo", repo)
 		next.ServeHTTP(rw, req.WithContext(ctx))
 	})
 }
diff --git a/internal/authz/middleware_test.go b/internal/authz/middleware_test.go
index 99ca0b0..3dfa997 100644
--- a/internal/authz/middleware_test.go
+++ b/internal/authz/middleware_test.go
@@ -113,7 +113,7 @@ func TestAuthorization(t *testing.T) {
 			body:           []byte("Access denied\n"),
 		},
 	}
-	svcr := admin.NewService(
+	svcr, _ := admin.NewService(
 		"../../auth_model.ini",
 		"../../tests/testpolicy.csv",
 		"../../gitserver.yaml",
diff --git a/internal/authz/model.go b/internal/authz/model.go
index 1ed61cf..32730c2 100644
--- a/internal/authz/model.go
+++ b/internal/authz/model.go
@@ -5,6 +5,7 @@ import (
 	"encoding/csv"
 	"encoding/hex"
 	"fmt"
+	"log/slog"
 	"math/big"
 	"os"
 
@@ -24,7 +25,7 @@ func (tm TokenMap) LoadTokensFromFile(path string) error {
 	// TODO this should be configurable
 	contents, err := os.Open(path)
 	if err != nil {
-		fmt.Println("File reading error", err)
+		slog.Error("File reading error", err)
 		return err
 	}
 	defer contents.Close()