Switch to go-git for all remote git interactions including auth (issue #651)

cmd/argocd/commands/repo.go

@@ -87,7 +87,7 @@ func NewRepoAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
 	}
 	command.Flags().StringVar(&repo.Username, "username", "", "username to the repository")
 	command.Flags().StringVar(&repo.Password, "password", "", "password to the repository")
-	command.Flags().StringVar(&sshPrivateKeyPath, "sshPrivateKeyPath", "", "path to the private ssh key (e.g. ~/.ssh/id_rsa)")
+	command.Flags().StringVar(&sshPrivateKeyPath, "ssh-private-key-path", "", "path to the private ssh key (e.g. ~/.ssh/id_rsa)")
 	command.Flags().BoolVar(&upsert, "upsert", false, "Override an existing repository with the same name even if the spec differs")
 	return command
 }

controller/secretcontroller.go

@@ -3,16 +3,9 @@ package controller
 import (
 	"context"
 	"encoding/json"
-	"time"
-
 	"runtime/debug"
+	"time"
 
-	"github.com/argoproj/argo-cd/common"
-	"github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
-	"github.com/argoproj/argo-cd/reposerver"
-	"github.com/argoproj/argo-cd/reposerver/repository"
-	"github.com/argoproj/argo-cd/util"
-	"github.com/argoproj/argo-cd/util/db"
 	log "github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -25,6 +18,12 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
+
+	"github.com/argoproj/argo-cd/common"
+	"github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
+	"github.com/argoproj/argo-cd/reposerver"
+	"github.com/argoproj/argo-cd/util/db"
+	"github.com/argoproj/argo-cd/util/git"
 )
 
 type SecretController struct {
@@ -93,22 +92,13 @@ func (ctrl *SecretController) getRepoConnectionState(repo *v1alpha1.Repository)
 		ModifiedAt: repo.ConnectionState.ModifiedAt,
 		Status:     v1alpha1.ConnectionStatusUnknown,
 	}
-	closer, client, err := ctrl.repoClientset.NewRepositoryClient()
-	if err != nil {
-		log.Errorf("Unable to create repository client: %v", err)
-		return state
-	}
-
-	defer util.Close(closer)
-
-	_, err = client.ListDir(context.Background(), &repository.ListDirRequest{Repo: repo, Path: ".gitignore"})
+	err := git.TestRepo(repo.Repo, repo.Username, repo.Password, repo.SSHPrivateKey)
 	if err == nil {
 		state.Status = v1alpha1.ConnectionStatusSuccessful
 	} else {
 		state.Status = v1alpha1.ConnectionStatusFailed
 		state.Message = err.Error()
 	}
-
 	return state
 }
 

reposerver/repository/repository.go

@@ -76,10 +76,6 @@ func (s *Service) ListDir(ctx context.Context, q *ListDirRequest) (*FileList, er
 
 	s.repoLock.Lock(gitClient.Root())
 	defer s.repoLock.Unlock(gitClient.Root())
-	err = gitClient.Init()
-	if err != nil {
-		return nil, err
-	}
 	commitSHA, err = checkoutRevision(gitClient, commitSHA)
 	if err != nil {
 		return nil, err
@@ -119,10 +115,6 @@ func (s *Service) GetFile(ctx context.Context, q *GetFileRequest) (*GetFileRespo
 
 	s.repoLock.Lock(gitClient.Root())
 	defer s.repoLock.Unlock(gitClient.Root())
-	err = gitClient.Init()
-	if err != nil {
-		return nil, err
-	}
 	commitSHA, err = checkoutRevision(gitClient, commitSHA)
 	if err != nil {
 		return nil, err
@@ -165,10 +157,6 @@ func (s *Service) GenerateManifest(c context.Context, q *ManifestRequest) (*Mani
 
 	s.repoLock.Lock(gitClient.Root())
 	defer s.repoLock.Unlock(gitClient.Root())
-	err = gitClient.Init()
-	if err != nil {
-		return nil, err
-	}
 	commitSHA, err = checkoutRevision(gitClient, commitSHA)
 	if err != nil {
 		return nil, err
@@ -309,13 +297,17 @@ func IdentifyAppSourceTypeByAppPath(appFilePath string) AppSourceType {
 // checkoutRevision is a convenience function to initialize a repo, fetch, and checkout a revision
 // Returns the 40 character commit SHA after the checkout has been performed
 func checkoutRevision(gitClient git.Client, commitSHA string) (string, error) {
-	err := gitClient.Fetch()
+	err := gitClient.Init()
+	if err != nil {
+		return "", status.Errorf(codes.Internal, "Failed to initialize git repo: %v", err)
+	}
+	err = gitClient.Fetch()
 	if err != nil {
-		return "", err
+		return "", status.Errorf(codes.Internal, "Failed to fetch git repo: %v", err)
 	}
 	err = gitClient.Checkout(commitSHA)
 	if err != nil {
-		return "", err
+		return "", status.Errorf(codes.Internal, "Failed to checkout %s: %v", commitSHA, err)
 	}
 	return gitClient.CommitSHA()
 }

server/server.go

@@ -367,6 +367,8 @@ func (a *ArgoCDServer) newGRPCServer() *grpc.Server {
 	sensitiveMethods := map[string]bool{
 		"/session.SessionService/Create":         true,
 		"/account.AccountService/UpdatePassword": true,
+		"/repository.RepositoryService/Create":   true,
+		"/repository.RepositoryService/Update":   true,
 	}
 	// NOTE: notice we do not configure the gRPC server here with TLS (e.g. grpc.Creds(creds))
 	// This is because TLS handshaking occurs in cmux handling

util/git/client.go

@@ -2,11 +2,8 @@ package git
 
 import (
 	"fmt"
-	"io/ioutil"
-	"net/url"
 	"os"
 	"os/exec"
-	"path"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -29,7 +26,6 @@ type Client interface {
 	LsRemote(revision string) (string, error)
 	LsFiles(path string) ([]string, error)
 	CommitSHA() (string, error)
-	Reset() error
 }
 
 // ClientFactory is a factory of Git Clients
@@ -40,12 +36,9 @@ type ClientFactory interface {
 
 // nativeGitClient implements Client interface using git CLI
 type nativeGitClient struct {
-	repoURL       string
-	root          string
-	username      string
-	password      string
-	sshPrivateKey string
-	auth          transport.AuthMethod
+	repoURL string
+	root    string
+	auth    transport.AuthMethod
 }
 
 type factory struct{}
@@ -56,11 +49,8 @@ func NewFactory() ClientFactory {
 
 func (f *factory) NewClient(repoURL, path, username, password, sshPrivateKey string) (Client, error) {
 	clnt := nativeGitClient{
-		repoURL:       repoURL,
-		root:          path,
-		username:      username,
-		password:      password,
-		sshPrivateKey: sshPrivateKey,
+		repoURL: repoURL,
+		root:    path,
 	}
 	if sshPrivateKey != "" {
 		signer, err := ssh.ParsePrivateKey([]byte(sshPrivateKey))
@@ -83,91 +73,61 @@ func (m *nativeGitClient) Root() string {
 
 // Init initializes a local git repository and sets the remote origin
 func (m *nativeGitClient) Init() error {
-	var needInit bool
-	if _, err := os.Stat(m.root); os.IsNotExist(err) {
-		needInit = true
-	} else {
-		_, err = m.runCmd("git", "status")
-		needInit = err != nil
+	_, err := git.PlainOpen(m.root)
+	if err == nil {
+		return nil
 	}
-	if needInit {
-		log.Infof("Initializing %s to %s", m.repoURL, m.root)
-		_, err := exec.Command("rm", "-rf", m.root).Output()
-		if err != nil {
-			return fmt.Errorf("unable to clean repo at %s: %v", m.root, err)
-		}
-		err = os.MkdirAll(m.root, 0755)
-		if err != nil {
-			return err
-		}
-		if _, err := m.runCmd("git", "init"); err != nil {
-			return err
-		}
-		if _, err := m.runCmd("git", "remote", "add", "origin", m.repoURL); err != nil {
-			return err
-		}
+	if err != git.ErrRepositoryNotExists {
+		return err
 	}
-	// always set credentials since it can change
-	err := m.setCredentials()
+	log.Infof("Initializing %s to %s", m.repoURL, m.root)
+	_, err = exec.Command("rm", "-rf", m.root).Output()
 	if err != nil {
-		return err
+		return fmt.Errorf("unable to clean repo at %s: %v", m.root, err)
 	}
-	return nil
-}
-
-// setCredentials sets a local credentials file to connect to a remote git repository
-func (m *nativeGitClient) setCredentials() error {
-	if m.password != "" {
-		log.Debug("Setting password credentials")
-		gitCredentialsFile := path.Join(m.root, ".git", "credentials")
-		urlObj, err := url.ParseRequestURI(m.repoURL)
-		if err != nil {
-			return err
-		}
-		urlObj.User = url.UserPassword(m.username, m.password)
-		cmdURL := urlObj.String()
-		err = ioutil.WriteFile(gitCredentialsFile, []byte(cmdURL), 0600)
-		if err != nil {
-			return fmt.Errorf("failed to set git credentials: %v", err)
-		}
-		_, err = m.runCmd("git", "config", "--local", "credential.helper", fmt.Sprintf("store --file=%s", gitCredentialsFile))
-		if err != nil {
-			return err
-		}
+	err = os.MkdirAll(m.root, 0755)
+	if err != nil {
+		return err
 	}
-	if IsSSHURL(m.repoURL) {
-		sshCmd := gitSSHCommand
-		if m.sshPrivateKey != "" {
-			log.Debug("Setting SSH credentials")
-			sshPrivateKeyFile := path.Join(m.root, ".git", "ssh-private-key")
-			err := ioutil.WriteFile(sshPrivateKeyFile, []byte(m.sshPrivateKey), 0600)
-			if err != nil {
-				return fmt.Errorf("failed to set git credentials: %v", err)
-			}
-			sshCmd += " -i " + sshPrivateKeyFile
-		}
-		_, err := m.runCmd("git", "config", "--local", "core.sshCommand", sshCmd)
-		if err != nil {
-			return err
-		}
+	repo, err := git.PlainInit(m.root, false)
+	if err != nil {
+		return err
 	}
-	return nil
+	_, err = repo.CreateRemote(&config.RemoteConfig{
+		Name: git.DefaultRemoteName,
+		URLs: []string{m.repoURL},
+	})
+	return err
 }
 
 // Fetch fetches latest updates from origin
 func (m *nativeGitClient) Fetch() error {
-	var err error
 	log.Debugf("Fetching repo %s at %s", m.repoURL, m.root)
-	if _, err = m.runCmd("git", "fetch", "origin", "--tags", "--force"); err != nil {
+	repo, err := git.PlainOpen(m.root)
+	if err != nil {
 		return err
 	}
+	log.Debug("git fetch origin --tags --force")
+	err = repo.Fetch(&git.FetchOptions{
+		RemoteName: git.DefaultRemoteName,
+		Auth:       m.auth,
+		Tags:       git.AllTags,
+		Force:      true,
+	})
+	if err == git.NoErrAlreadyUpToDate {
+		return nil
+	}
+	return err
 	// git fetch does not update the HEAD reference. The following command will update the local
 	// knowledge of what remote considers the “default branch”
 	// See: https://stackoverflow.com/questions/8839958/how-does-origin-head-get-set
-	if _, err := m.runCmd("git", "remote", "set-head", "origin", "-a"); err != nil {
-		return err
-	}
-	return nil
+	// NOTE(jessesuen): disabling the following code because:
+	// 1. we no longer perform a `git checkout HEAD`, instead relying on `ls-remote` and checking
+	//    out a specific SHA1.
+	// 2. This command is the only other command that we use (excluding fetch/ls-remote) which
+	//    requires remote access, and there appears to be no go-git equivalent to this command.
+	// _, err = m.runCmd("git", "remote", "set-head", "origin", "-a")
+	// return err
 }
 
 // LsFiles lists the local working tree, including only files that are under source control
@@ -181,34 +141,6 @@ func (m *nativeGitClient) LsFiles(path string) ([]string, error) {
 	return ss[:len(ss)-1], nil
 }
 
-// Reset resets local changes in a repository
-func (m *nativeGitClient) Reset() error {
-	if _, err := m.runCmd("git", "reset", "--hard", "origin/HEAD"); err != nil {
-		return err
-	}
-	// Delete all local branches (we must first detach so we are not checked out a branch we are about to delete)
-	if _, err := m.runCmd("git", "checkout", "--detach", "origin/HEAD"); err != nil {
-		return err
-	}
-	branchesOut, err := m.runCmd("git", "for-each-ref", "--format=%(refname:short)", "refs/heads/")
-	if err != nil {
-		return err
-	}
-	branchesOut = strings.TrimSpace(branchesOut)
-	if branchesOut != "" {
-		branches := strings.Split(branchesOut, "\n")
-		args := []string{"branch", "-D"}
-		args = append(args, branches...)
-		if _, err = m.runCmd("git", args...); err != nil {
-			return err
-		}
-	}
-	if _, err := m.runCmd("git", "clean", "-fd"); err != nil {
-		return err
-	}
-	return nil
-}
-
 // Checkout checkout specified git sha
 func (m *nativeGitClient) Checkout(revision string) error {
 	if revision == "" || revision == "HEAD" {
@@ -310,6 +242,8 @@ func (m *nativeGitClient) runCmd(command string, args ...string) (string, error)
 	log.Debug(strings.Join(cmd.Args, " "))
 	cmd.Dir = m.root
 	env := os.Environ()
+	env = append(env, "HOME=/dev/null")
+	env = append(env, "GIT_CONFIG_NOSYSTEM=true")
 	env = append(env, "GIT_ASKPASS=")
 	cmd.Env = env
 	out, err := cmd.Output()