Refactoring and some cleanup

Return values need to be changed ASAP (Sorry)

Author: RegWin32

controllers/git/git.go

@@ -1,201 +1,163 @@
-/*
-Package git holds every implemention needed
-to do various git operations.
-This is a interface-first implemention.
-*/
 package git
 
 import (
 	"net/url"
 	"os"
-	"strings"
 
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing"
-	"github.com/go-git/go-git/v5/plumbing/transport"
-	"github.com/go-git/go-git/v5/plumbing/transport/ssh"
 	"github.com/go-logr/logr"
-	gossh "golang.org/x/crypto/ssh"
 )
 
 // GitOperator defines the interface for git operations.
 type GitOperator interface {
-	Clone(repoURL string, secretRef []byte, directory string, logger logr.Logger) (err error)
-	Poll(repo string, secretRef []byte, branch string, directory string, logger logr.Logger) (changes bool, err error)
-	Hash(repo string, secretRef []byte, branch string, logger logr.Logger) (hash string, err error)
+	Clone(repoURL string, secretRef []byte, directory string, logger logr.Logger) error
+	Poll(repoURL string, secretRef []byte, branch string, directory string, logger logr.Logger) (bool, error)
+	Hash(repo string, secretRef []byte, branch string, logger logr.Logger) (string, error)
 }
 
 // GitImplementer implements the GitOperator interface.
 type GitImplementer struct{}
 
-// Clone clones the given repository to a local directory.
-func (g *GitImplementer) Clone(repoURL string, secretRef []byte, directory string, logger logr.Logger) (err error) {
-	var auth transport.AuthMethod
+// getSSHAuth is a helper function to create an ssh.PublicKeys AuthMethod.
+func getSSHAuth(secretRef []byte) (transport.AuthMethod, error) {
+	if len(secretRef) == 0 {
+		return nil, fmt.Errorf("SSH secret reference is empty")
+	}
 
-	logger.Info("Creating directory")
-	err = os.Mkdir(directory, 0755)
+	auth, err := ssh.NewPublicKeys("git", secretRef, "")
 	if err != nil {
-		logger.Error(err, "Failed to create directory", "directory", directory)
-
-		return err
+		return nil, fmt.Errorf("failed to create public keys from secret: %w", err)
 	}
 
-	// Check if repo and directory are empty.
-	logger.Info("Checking if repo and or directory is empty")
-	if empty(repoURL, directory) {
-		logger.Error(err, "repo and or directory is empty", "repoURL", repoURL, "directory", directory)
+	// Insecurely ignore host key
+	// ToDO: Use a proper host key verification callback.
+	auth.HostKeyCallback = gossh.InsecureIgnoreHostKey()
 
-		return err
+	return auth, nil
+}
+
+// Clone clones the given repository to a local directory.
+func (g *GitImplementer) Clone(repoURL string, secretRef []byte, directory string, logger logr.Logger) error {
+	if repoURL == "" || directory == "" {
+		return fmt.Errorf("repoURL and directory must not be empty")
 	}
 
-	logger.Info("Retrieving auth Token")
-	auth, err = ssh.NewPublicKeys("git", secretRef, "")
-	if err != nil {
-		logger.Error(err, "Failed on retrieve the token from the tokenContent")
+	logger.Info("Creating directory", "directory", directory)
+	if err := os.MkdirAll(directory, 0755); err != nil {
+		return fmt.Errorf("failed to create directory %s: %w", directory, err)
+	}
 
+	logger.Info("Retrieving auth method")
+	auth, err := getSSHAuth(secretRef)
+	if err != nil {
 		return err
 	}
 
-	logger.Info("Plain Cloning Repo")
+	logger.Info("Cloning repository", "repoURL", repoURL)
 	_, err = git.PlainClone(directory, false, &git.CloneOptions{
-		URL: repoURL,
-		Auth: auth, 
-		Depth: 1,
+		URL:      repoURL,
+		Auth:     auth,
+		Progress: os.Stdout, // Optional: show progress
+		Depth:    1,
 	})
 	if err != nil {
-		logger.Error(err, "Failed to clone repo", "repo", repoURL)
-
-		return err
+		return fmt.Errorf("failed to clone repo %s: %w", repoURL, err)
 	}
 
-	return err
+	return nil
 }
 
-// Poll polls for changes for the given remote git repository. Returns true, if current local commit hash and remote hash are not equal.
-func (g *GitImplementer) Poll(repo string, secretRef []byte, branch string, directory string, logger logr.Logger) (changes bool, err error) {
-	// Defaults to false. We only change to true if there is a difference between the hashes.
-	changes = false
-
-	// Check if repo and directory are empty.
-	if empty(repo, directory) {
-		logger.Error(err, "repo and or directory is empty", "repo", repo, "directory", directory)
-
-		return changes, err
+// Poll polls for changes for the given remote git repository.
+// Returns true if current local commit hash and remote hash are different.
+func (g *GitImplementer) Poll(repoURL string, secretRef []byte, branch string, directory string, logger logr.Logger) (bool, error) {
+	if repoURL == "" || directory == "" {
+		return false, fmt.Errorf("repoURL and directory must not be empty")
 	}
 
 	// Get hash from local repo.
-	localHash, err := g.Hash(directory, secretRef, branch, logger)
+	localHash, err := g.Hash(directory, nil, "", logger) // No secret or branch needed for local HEAD
 	if err != nil {
-		logger.Error(err, "Failed to get hash", "repo", repo, "directory", directory)
-
-		return changes, err
+		return false, fmt.Errorf("failed to get local hash from %s: %w", directory, err)
 	}
 
 	// Get Hash from remote repo
-	remoteHash, err := g.Hash(repo, secretRef, branch, logger)
+	remoteHash, err := g.Hash(repoURL, secretRef, branch, logger)
 	if err != nil {
-		logger.Error(err, "Failed to get hash", "repo", repo, "directory", directory)
-
-		return changes, err
+		return false, fmt.Errorf("failed to get remote hash for %s: %w", repoURL, err)
 	}
 
-	if localHash != remoteHash {
-		changes = true
-	}
+	changes := localHash != remoteHash
+	logger.Info("Polling result", "localHash", localHash, "remoteHash", remoteHash, "changes", changes)
 
-	return changes, err
+	return changes, nil
 }
 
 // Hash retrieves the hash of the given repository.
-func (g *GitImplementer) Hash(repo string, secretRef []byte, branch string, logger logr.Logger) (hash string, err error) {
-	var auth transport.AuthMethod
+func (g *GitImplementer) Hash(repo string, secretRef []byte, branch string, logger logr.Logger) (string, error) {
+	if isURL(repo) {
+		return g.remoteHash(repo, secretRef, branch, logger)
+	}
+	return g.localHash(repo, logger)
+}
 
-	auth, err = ssh.NewPublicKeys("git", secretRef, "")
+// localHash retrieves the HEAD commit hash from a local repository.
+func (g *GitImplementer) localHash(path string, logger logr.Logger) (string, error) {
+	localRepo, err := git.PlainOpen(path)
 	if err != nil {
-		logger.Error(err, "Failed to retrieve the token from the tokenContent")
-
-		return hash, err
+		return "", fmt.Errorf("failed to open local repo at %s: %w", path, err)
 	}
 
-	pkAuth, ok := auth.(*ssh.PublicKeys)
-	if !ok {
-		logger.Error(err, "pkAuth error")
+	headRef, err := localRepo.Head()
+	if err != nil {
+		return "", fmt.Errorf("failed to get HEAD for local repo at %s: %w", path, err)
 	}
 
-	pkAuth.HostKeyCallback = gossh.InsecureIgnoreHostKey()
-
-	switch {
-	case isURL(repo):
-		remoterepo := git.NewRemote(nil, &config.RemoteConfig{
-			URLs: []string{repo},
-			Name: "origin",
-		})
-
-		refs, err := remoterepo.List(&git.ListOptions{
-			Auth: auth,
-		})
-		if err != nil {
-			logger.Error(err, "Failed to list remote repo", "repo", repo)
-
-			return hash, err
-		}
-
-		refName := plumbing.NewBranchReferenceName(branch)
-		for _, ref := range refs {
-			if ref.Name() == refName {
-				return ref.Hash().String(), err
-			}
-		}
-
-		return hash, err
-	case !isURL(repo):
-		localRepo, err := git.PlainOpen(repo)
-		if err != nil {
-			logger.Error(err, "Failed to open local repo", "repo", repo)
+	return headRef.Hash().String(), nil
+}
 
-			return hash, err
-		}
+// remoteHash retrieves the commit hash for a specific branch from a remote repository.
+func (g *GitImplementer) remoteHash(repoURL string, secretRef []byte, branch string, logger logr.Logger) (string, error) {
+	if branch == "" {
+		return "", fmt.Errorf("branch must be specified for remote repository")
+	}
 
-		headRef, err := localRepo.Head()
-		if err != nil {
-			logger.Error(err, "failed to get head for local git repo", "repo", repo)
+	auth, err := getSSHAuth(secretRef)
+	if err != nil {
+		return "", err
+	}
 
-			return hash, err
-		}
+	remoterepo := git.NewRemote(nil, &config.RemoteConfig{
+		URLs: []string{repoURL},
+	})
 
-		hash = headRef.Hash().String()
-		if hash == "" {
-			logger.Error(err, "failed to get hash for local git repo", "repo", repo)
+	refs, err := remoterepo.List(&git.ListOptions{
+		Auth: auth,
+	})
+	if err != nil {
+		return "", fmt.Errorf("failed to list remote repo %s: %w", repoURL, err)
+	}
 
-			return hash, err
+	refName := plumbing.NewBranchReferenceName(branch)
+	for _, ref := range refs {
+		if ref.Name() == refName {
+			return ref.Hash().String(), nil
 		}
-
-		return hash, err
 	}
 
-	return hash, err
+	return "", fmt.Errorf("branch '%s' not found in remote repository %s", branch, repoURL)
 }
 
-// isURL checks if the given string is a valid URL.
-func isURL(repo string) bool {
-	if repo == "" {
-		return false
-	}
-	parsedURL, err := url.ParseRequestURI(repo)
-	if err == nil && parsedURL.Scheme != "" {
+// isUrl checks if the given string is a valid URL.
+func isURL(str string) bool {
+	// A simple check for SCP-like syntax or a scheme.
+	// https://en.wikipedia.org/wiki/Secure_copy_protocol :-)
+	if strings.Contains(str, "@") && strings.Contains(str, ":") {
 		return true
 	}
-
-	if strings.Contains(repo, "@") && strings.Contains(repo, ":") {
-		return true
-	}
-	// if parsedURL.Scheme != "" {
-	// 	return true
-	// } else {
-	// 	return false
-	// }
-	return false
+	u, err := url.ParseRequestURI(str)
+	return err == nil && u.Scheme != "" && u.Host != ""
 }
 
 // empty checks if the repo and directory strings are empty.