feat(controller): more git pkg enhancements (#2486)

Signed-off-by: Kent Rancourt <[email protected]>

internal/controller/git/bare_repo.go

@@ -81,11 +81,10 @@ func CloneBare(
 	}
 	b := &bareRepo{
 		baseRepo: &baseRepo{
-			creds:                 clientOpts.Credentials,
-			dir:                   filepath.Join(homeDir, "repo"),
-			homeDir:               homeDir,
-			insecureSkipTLSVerify: cloneOpts.InsecureSkipTLSVerify,
-			url:                   repoURL,
+			creds:   clientOpts.Credentials,
+			dir:     filepath.Join(homeDir, "repo"),
+			homeDir: homeDir,
+			url:     repoURL,
 		},
 	}
 	if err = b.setupClient(clientOpts); err != nil {
@@ -110,8 +109,7 @@ func (b *bareRepo) clone() error {
 }
 
 type LoadBareRepoOptions struct {
-	Credentials           *RepoCredentials
-	InsecureSkipTLSVerify bool
+	Credentials *RepoCredentials
 }
 
 func LoadBareRepo(path string, opts *LoadBareRepoOptions) (BareRepo, error) {
@@ -120,9 +118,8 @@ func LoadBareRepo(path string, opts *LoadBareRepoOptions) (BareRepo, error) {
 	}
 	b := &bareRepo{
 		baseRepo: &baseRepo{
-			creds:                 opts.Credentials,
-			dir:                   path,
-			insecureSkipTLSVerify: opts.InsecureSkipTLSVerify,
+			creds: opts.Credentials,
+			dir:   path,
 		},
 	}
 	if err := b.loadHomeDir(); err != nil {
@@ -160,11 +157,10 @@ func (b *bareRepo) AddWorkTree(path, ref string) (WorkTree, error) {
 	}
 	return &workTree{
 		baseRepo: &baseRepo{
-			creds:                 b.creds,
-			dir:                   path,
-			homeDir:               b.homeDir,
-			insecureSkipTLSVerify: b.insecureSkipTLSVerify,
-			url:                   b.url,
+			creds:   b.creds,
+			dir:     path,
+			homeDir: b.homeDir,
+			url:     b.url,
 		},
 		bareRepo: b,
 	}, nil
@@ -211,11 +207,10 @@ func (b *bareRepo) WorkTrees() ([]WorkTree, error) {
 	for i, workTreePath := range workTreePaths {
 		workTrees[i] = &workTree{
 			baseRepo: &baseRepo{
-				creds:                 b.creds,
-				dir:                   workTreePath,
-				homeDir:               b.homeDir,
-				insecureSkipTLSVerify: b.insecureSkipTLSVerify,
-				url:                   b.url,
+				creds:   b.creds,
+				dir:     workTreePath,
+				homeDir: b.homeDir,
+				url:     b.url,
 			},
 			bareRepo: b,
 		}

internal/controller/git/bare_repo_test.go

@@ -59,7 +59,7 @@ func TestBareRepo(t *testing.T) {
 	require.NoError(t, err)
 	err = setupRep.AddAllAndCommit(fmt.Sprintf("initial commit %s", uuid.NewString()))
 	require.NoError(t, err)
-	err = setupRep.Push(false)
+	err = setupRep.Push(nil)
 	require.NoError(t, err)
 	err = setupRep.Close()
 	require.NoError(t, err)

internal/controller/git/base_repo.go

@@ -15,11 +15,10 @@ import (
 // single working tree, a bare repository, or working tree associated with a
 // bare repository.
 type baseRepo struct {
-	creds                 *RepoCredentials
-	dir                   string
-	homeDir               string
-	insecureSkipTLSVerify bool
-	url                   string
+	creds   *RepoCredentials
+	dir     string
+	homeDir string
+	url     string
 }
 
 // ClientOptions represents options for a repository-specific Git client.
@@ -30,6 +29,9 @@ type ClientOptions struct {
 	User *User
 	// Credentials represents the authentication information.
 	Credentials *RepoCredentials
+	// InsecureSkipTLSVerify indicates whether to ignore certificate verification
+	// errors when interacting with the remote repository.
+	InsecureSkipTLSVerify bool
 }
 
 // setupClient configures the git CLI for authentication using either SSH or
@@ -47,6 +49,14 @@ func (b *baseRepo) setupClient(opts *ClientOptions) error {
 		return fmt.Errorf("error configuring the credentials: %w", err)
 	}
 
+	if opts.InsecureSkipTLSVerify {
+		cmd := b.buildGitCommand("config", "--global", "http.sslVerify", "false")
+		cmd.Dir = b.homeDir // Override the cmd.Dir that's set by b.buildGitCommand()
+		if _, err := libExec.Exec(cmd); err != nil {
+			return fmt.Errorf("error configuring http.sslVerify: %w", err)
+		}
+	}
+
 	return nil
 }
 
@@ -223,9 +233,6 @@ func (b *baseRepo) buildGitCommand(arg ...string) *exec.Cmd {
 			fmt.Sprintf("GIT_PASSWORD=%s", b.creds.Password),
 		)
 	}
-	if b.insecureSkipTLSVerify {
-		cmd.Env = append(cmd.Env, "GIT_SSL_NO_VERIFY=true")
-	}
 	return cmd
 }
 

internal/controller/git/repo.go

@@ -55,10 +55,6 @@ type CloneOptions struct {
 	// - https://github.blog/2020-12-21-get-up-to-speed-with-partial-clone-and-shallow-clone/
 	// - https://docs.gitlab.com/ee/topics/git/partial_clone.html
 	Filter string
-	// InsecureSkipTLSVerify specifies whether certificate verification errors
-	// should be ignored when cloning the repository. The setting will be
-	// remembered for subsequent interactions with the remote repository.
-	InsecureSkipTLSVerify bool
 	// SingleBranch indicates whether the clone should be a single-branch clone.
 	// This option is ignored if Bare is true.
 	SingleBranch bool
@@ -90,11 +86,10 @@ func Clone(
 			fmt.Errorf("error resolving symlinks in path %s: %w", homeDir, err)
 	}
 	baseRepo := &baseRepo{
-		creds:                 clientOpts.Credentials,
-		dir:                   filepath.Join(homeDir, "repo"),
-		homeDir:               homeDir,
-		insecureSkipTLSVerify: cloneOpts.InsecureSkipTLSVerify,
-		url:                   repoURL,
+		creds:   clientOpts.Credentials,
+		dir:     filepath.Join(homeDir, "repo"),
+		homeDir: homeDir,
+		url:     repoURL,
 	}
 	r := &repo{
 		baseRepo: baseRepo,
@@ -138,18 +133,16 @@ func (r *repo) clone(opts *CloneOptions) error {
 }
 
 type LoadRepoOptions struct {
-	Credentials           *RepoCredentials
-	InsecureSkipTLSVerify bool
+	Credentials *RepoCredentials
 }
 
 func LoadRepo(path string, opts *LoadRepoOptions) (Repo, error) {
 	if opts == nil {
 		opts = &LoadRepoOptions{}
 	}
 	baseRepo := &baseRepo{
-		creds:                 opts.Credentials,
-		dir:                   path,
-		insecureSkipTLSVerify: opts.InsecureSkipTLSVerify,
+		creds: opts.Credentials,
+		dir:   path,
 	}
 	r := &repo{
 		baseRepo: baseRepo,

internal/controller/git/repo_test.go

@@ -140,7 +140,7 @@ func TestRepo(t *testing.T) {
 		require.False(t, exists)
 	})
 
-	err = rep.Push(false)
+	err = rep.Push(nil)
 	require.NoError(t, err)
 
 	t.Run("can push", func(t *testing.T) {

internal/controller/git/work_tree.go

@@ -67,8 +67,8 @@ type WorkTree interface {
 	// CommitMessage returns the text of the most recent commit message associated
 	// with the specified commit ID.
 	CommitMessage(id string) (string, error)
-	// Push pushes from the current branch to a remote branch by the same name.
-	Push(force bool) error
+	// Push pushes from the local repository to the remote repository.
+	Push(*PushOptions) error
 	// RefsHaveDiffs returns whether there is a diff between two commits/branches
 	RefsHaveDiffs(commit1 string, commit2 string) (bool, error)
 	// RemoteBranchExists returns a bool indicating if the specified branch exists
@@ -88,8 +88,7 @@ type workTree struct {
 }
 
 type LoadWorkTreeOptions struct {
-	Credentials           *RepoCredentials
-	InsecureSkipTLSVerify bool
+	Credentials *RepoCredentials
 }
 
 func LoadWorkTree(path string, opts *LoadWorkTreeOptions) (WorkTree, error) {
@@ -98,9 +97,8 @@ func LoadWorkTree(path string, opts *LoadWorkTreeOptions) (WorkTree, error) {
 	}
 	w := &workTree{
 		baseRepo: &baseRepo{
-			creds:                 opts.Credentials,
-			dir:                   path,
-			insecureSkipTLSVerify: opts.InsecureSkipTLSVerify,
+			creds: opts.Credentials,
+			dir:   path,
 		},
 	}
 	res, err := libExec.Exec(w.buildGitCommand(
@@ -122,8 +120,7 @@ func LoadWorkTree(path string, opts *LoadWorkTreeOptions) (WorkTree, error) {
 		return nil, fmt.Errorf("error configuring the credentials: %w", err)
 	}
 	br, err := LoadBareRepo(repoPath, &LoadBareRepoOptions{
-		Credentials:           opts.Credentials,
-		InsecureSkipTLSVerify: opts.InsecureSkipTLSVerify,
+		Credentials: opts.Credentials,
 	})
 	if err != nil {
 		return nil, err
@@ -181,6 +178,9 @@ func (w *workTree) Checkout(branch string) error {
 type CommitOptions struct {
 	// AllowEmpty indicates whether an empty commit should be allowed.
 	AllowEmpty bool
+	// Author is the author of the commit. If nil, the default author already
+	// configured in the git repository will be used.
+	Author *User
 }
 
 func (w *workTree) Commit(message string, opts *CommitOptions) error {
@@ -451,9 +451,27 @@ func (w *workTree) ListTags() ([]TagMetadata, error) {
 	return tags, nil
 }
 
-func (w *workTree) Push(force bool) error {
-	args := []string{"push", "origin", "HEAD"}
-	if force {
+// PushOptions represents options for pushing changes to a remote git
+// repository.
+type PushOptions struct {
+	// Force indicates whether the push should be forced.
+	Force bool
+	// TargetBranch specifies the branch to push to. If empty, the current branch
+	// will be pushed to a remote branch by the same name.
+	TargetBranch string
+}
+
+func (w *workTree) Push(opts *PushOptions) error {
+	if opts == nil {
+		opts = &PushOptions{}
+	}
+	args := []string{"push", "origin"}
+	if opts.TargetBranch != "" {
+		args = append(args, fmt.Sprintf("HEAD:%s", opts.TargetBranch))
+	} else {
+		args = append(args, "HEAD")
+	}
+	if opts.Force {
 		args = append(args, "--force")
 	}
 	if _, err := libExec.Exec(w.buildGitCommand(args...)); err != nil {

internal/controller/git/work_tree_test.go

@@ -58,7 +58,7 @@ func TestWorkTree(t *testing.T) {
 	require.NoError(t, err)
 	err = setupRep.AddAllAndCommit(fmt.Sprintf("initial commit %s", uuid.NewString()))
 	require.NoError(t, err)
-	err = setupRep.Push(false)
+	err = setupRep.Push(nil)
 	require.NoError(t, err)
 	err = setupRep.Close()
 	require.NoError(t, err)

internal/controller/promotion/git.go

@@ -197,12 +197,11 @@ func (g *gitMechanism) doSingleUpdate(
 	repo, err := git.Clone(
 		update.RepoURL,
 		&git.ClientOptions{
-			User:        author,
-			Credentials: creds,
-		},
-		&git.CloneOptions{
+			User:                  author,
+			Credentials:           creds,
 			InsecureSkipTLSVerify: update.InsecureSkipTLSVerify,
 		},
+		nil,
 	)
 	if err != nil {
 		return fmt.Errorf("error cloning git repo %q: %w", update.RepoURL, err)
@@ -464,7 +463,7 @@ func (g *gitMechanism) gitCommit(
 		if err = repo.AddAllAndCommit(commitMsg); err != nil {
 			return "", fmt.Errorf("error committing updates to git repo %q: %w", update.RepoURL, err)
 		}
-		if err = repo.Push(false); err != nil {
+		if err = repo.Push(nil); err != nil {
 			return "", fmt.Errorf("error pushing updates to git repo %q: %w", update.RepoURL, err)
 		}
 	}

internal/controller/promotion/pullrequest.go

@@ -41,7 +41,7 @@ func preparePullRequestBranch(repo git.Repo, prBranch string, base string) error
 		); err != nil {
 			return err
 		}
-		if err = repo.Push(false); err != nil {
+		if err = repo.Push(nil); err != nil {
 			return err
 		}
 	} else if err = repo.Checkout(base); err != nil {
@@ -56,7 +56,7 @@ func preparePullRequestBranch(repo git.Repo, prBranch string, base string) error
 		if err := repo.CreateChildBranch(prBranch); err != nil {
 			return err
 		}
-		if err := repo.Push(false); err != nil {
+		if err := repo.Push(nil); err != nil {
 			return err
 		}
 	} else {
@@ -80,7 +80,7 @@ func preparePullRequestBranch(repo git.Repo, prBranch string, base string) error
 			if err = repo.CreateChildBranch(prBranch); err != nil {
 				return err
 			}
-			if err = repo.Push(true); err != nil {
+			if err = repo.Push(&git.PushOptions{Force: true}); err != nil {
 				return err
 			}
 		}

internal/controller/warehouses/git.go

@@ -69,15 +69,15 @@ func (r *reconciler) discoverCommits(
 
 		// Clone the Git repository.
 		cloneOpts := &git.CloneOptions{
-			Branch:                sub.Branch,
-			SingleBranch:          true,
-			Filter:                git.FilterBlobless,
-			InsecureSkipTLSVerify: sub.InsecureSkipTLSVerify,
+			Branch:       sub.Branch,
+			SingleBranch: true,
+			Filter:       git.FilterBlobless,
 		}
 		repo, err := r.gitCloneFn(
 			sub.RepoURL,
 			&git.ClientOptions{
-				Credentials: repoCreds,
+				Credentials:           repoCreds,
+				InsecureSkipTLSVerify: sub.InsecureSkipTLSVerify,
 			},
 			cloneOpts,
 		)