feat: add ability to specify a different write and base branch (#304)

* add ability to specify a different write and base branch

* format/spelling

* add sha1 to template and truncate branch name length

* add hasher error checking

* document

* spelling

* correction to template docs

* use sha256 and add warning to truncation

* condense annotations into one

* add some tests

* add docs on omitting base branch

4 files changed, 265 insertions, 23 deletions

diff --git a/pkg/argocd/git.go b/pkg/argocd/git.go
index 1b32500..ff1c9dc 100644
--- a/pkg/argocd/git.go
+++ b/pkg/argocd/git.go
@@ -2,6 +2,8 @@ package argocd
 
 import (
 	"bytes"
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -58,11 +60,75 @@ func TemplateCommitMessage(tpl *template.Template, appName string, changeList []
 	return cmBuf.String()
 }
 
+// TemplateBranchName parses a string to a template, and returns a
+// branch name from that new template. If a branch name can not be
+// rendered, it returns an empty value.
+func TemplateBranchName(branchName string, changeList []ChangeEntry) string {
+	var cmBuf bytes.Buffer
+
+	tpl, err1 := template.New("branchName").Parse(branchName)
+
+	if err1 != nil {
+		log.Errorf("could not create template for Git branch name: %v", err1)
+		return ""
+	}
+
+	type imageChange struct {
+		Name   string
+		Alias  string
+		OldTag string
+		NewTag string
+	}
+
+	type branchNameTemplate struct {
+		Images []imageChange
+		SHA256 string
+	}
+
+	// Let's add a unique hash to the template
+	hasher := sha256.New()
+
+	// We need to transform the change list into something more viable for the
+	// writer of a template.
+	changes := make([]imageChange, 0)
+	for _, c := range changeList {
+		changes = append(changes, imageChange{c.Image.ImageName, c.Image.ImageAlias, c.OldTag.String(), c.NewTag.String()})
+		id := fmt.Sprintf("%v-%v-%v,", c.Image.ImageName, c.OldTag.String(), c.NewTag.String())
+		_, hasherErr := hasher.Write([]byte(id))
+		log.Infof("writing to hasher %v", id)
+		if hasherErr != nil {
+			log.Errorf("could not write image string to hasher: %v", hasherErr)
+			return ""
+		}
+	}
+
+	tplData := branchNameTemplate{
+		Images: changes,
+		SHA256: hex.EncodeToString(hasher.Sum(nil)),
+	}
+
+	err2 := tpl.Execute(&cmBuf, tplData)
+	if err2 != nil {
+		log.Errorf("could not execute template for Git branch name: %v", err2)
+		return ""
+	}
+
+	toReturn := cmBuf.String()
+
+	if len(toReturn) > 255 {
+		trunc := toReturn[:255]
+		log.Warnf("write-branch name %v exceeded 255 characters and was truncated to %v", toReturn, trunc)
+		return trunc
+	} else {
+		return toReturn
+	}
+}
+
 type changeWriter func(app *v1alpha1.Application, wbc *WriteBackConfig, gitC git.Client) (err error, skip bool)
 
 // commitChanges commits any changes required for updating one or more images
 // after the UpdateApplication cycle has finished.
-func commitChangesGit(app *v1alpha1.Application, wbc *WriteBackConfig, write changeWriter) error {
+func commitChangesGit(app *v1alpha1.Application, wbc *WriteBackConfig, changeList []ChangeEntry, write changeWriter) error {
 	creds, err := wbc.GetCreds(app)
 	if err != nil {
 		return fmt.Errorf("could not get creds for repo '%s': %v", app.Spec.Source.RepoURL, err)
@@ -125,6 +191,26 @@ func commitChangesGit(app *v1alpha1.Application, wbc *WriteBackConfig, write cha
 		return err
 	}
 
+	// The push branch is by default the same as the checkout branch, unless
+	// specified after a : separator git-branch annotation, in which case a
+	// new branch will be made following a template that can use the list of
+	// changed images.
+	pushBranch := checkOutBranch
+
+	if wbc.GitWriteBranch != "" {
+		log.Debugf("Using branch template: %s", wbc.GitWriteBranch)
+		pushBranch = TemplateBranchName(wbc.GitWriteBranch, changeList)
+		if pushBranch != "" {
+			log.Debugf("Creating branch '%s' and using that for push operations", pushBranch)
+			err = gitC.Branch(checkOutBranch, pushBranch)
+			if err != nil {
+				return err
+			}
+		} else {
+			return fmt.Errorf("Git branch name could not be created from the template: %s", wbc.GitWriteBranch)
+		}
+	}
+
 	if err, skip := write(app, wbc, gitC); err != nil {
 		return err
 	} else if skip {
@@ -152,7 +238,7 @@ func commitChangesGit(app *v1alpha1.Application, wbc *WriteBackConfig, write cha
 	if err != nil {
 		return err
 	}
-	err = gitC.Push("origin", checkOutBranch, false)
+	err = gitC.Push("origin", pushBranch, false)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/argocd/git_test.go b/pkg/argocd/git_test.go
index 92731d5..69f7027 100644
--- a/pkg/argocd/git_test.go
+++ b/pkg/argocd/git_test.go
@@ -42,6 +42,70 @@ updates image bar/baz tag '2.0' to '2.1'
 	})
 }
 
+func Test_TemplateBranchName(t *testing.T) {
+	t.Run("Template branch name with image name", func(t *testing.T) {
+		exp := `image-updater-foo/bar-1.1-bar/baz-2.1`
+		tpl := "image-updater{{range .Images}}-{{.Name}}-{{.NewTag}}{{end}}"
+		cl := []ChangeEntry{
+			{
+				Image:  image.NewFromIdentifier("foo/bar"),
+				OldTag: tag.NewImageTag("1.0", time.Now(), ""),
+				NewTag: tag.NewImageTag("1.1", time.Now(), ""),
+			},
+			{
+				Image:  image.NewFromIdentifier("bar/baz"),
+				OldTag: tag.NewImageTag("2.0", time.Now(), ""),
+				NewTag: tag.NewImageTag("2.1", time.Now(), ""),
+			},
+		}
+		r := TemplateBranchName(tpl, cl)
+		assert.NotEmpty(t, r)
+		assert.Equal(t, exp, r)
+	})
+	t.Run("Template branch name with alias", func(t *testing.T) {
+		exp := `image-updater-bar-1.1`
+		tpl := "image-updater{{range .Images}}-{{.Alias}}-{{.NewTag}}{{end}}"
+		cl := []ChangeEntry{
+			{
+				Image:  image.NewFromIdentifier("bar=0001.dkr.ecr.us-east-1.amazonaws.com/bar"),
+				OldTag: tag.NewImageTag("1.0", time.Now(), ""),
+				NewTag: tag.NewImageTag("1.1", time.Now(), ""),
+			},
+		}
+		r := TemplateBranchName(tpl, cl)
+		assert.NotEmpty(t, r)
+		assert.Equal(t, exp, r)
+	})
+	t.Run("Template branch name with hash", func(t *testing.T) {
+		// Expected value generated from https://emn178.github.io/online-tools/sha256.html
+		exp := `image-updater-0fcc2782543e4bb067c174c21bf44eb947f3e55c0d62c403e359c1c209cbd041`
+		tpl := "image-updater-{{.SHA256}}"
+		cl := []ChangeEntry{
+			{
+				Image:  image.NewFromIdentifier("foo/bar"),
+				OldTag: tag.NewImageTag("1.0", time.Now(), ""),
+				NewTag: tag.NewImageTag("1.1", time.Now(), ""),
+			},
+		}
+		r := TemplateBranchName(tpl, cl)
+		assert.NotEmpty(t, r)
+		assert.Equal(t, exp, r)
+	})
+	t.Run("Template branch over 255 chars", func(t *testing.T) {
+		tpl := "image-updater-lorem-ipsum-dolor-sit-amet-consectetur-" +
+			"adipiscing-elit-phasellus-imperdiet-vitae-elit-quis-pulvinar-" +
+			"suspendisse-pulvinar-lacus-vel-semper-congue-enim-purus-posuere-" +
+			"orci-ut-vulputate-mi-ipsum-quis-ipsum-quisque-elit-arcu-lobortis-" +
+			"in-blandit-vel-pharetra-vel-urna-aliquam-euismod-elit-vel-mi"
+		exp := tpl[:255]
+		cl := []ChangeEntry{}
+		r := TemplateBranchName(tpl, cl)
+		assert.NotEmpty(t, r)
+		assert.Equal(t, exp, r)
+		assert.Len(t, r, 255)
+	})
+}
+
 func Test_parseImageOverride(t *testing.T) {
 	cases := []struct {
 		name     string
diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go
index 0f5e31e..541a0f8 100644
--- a/pkg/argocd/update.go
+++ b/pkg/argocd/update.go
@@ -61,6 +61,7 @@ type WriteBackConfig struct {
 	GitClient        git.Client
 	GetCreds         GitCredsSource
 	GitBranch        string
+	GitWriteBranch   string
 	GitCommitUser    string
 	GitCommitEmail   string
 	GitCommitMessage string
@@ -313,7 +314,7 @@ func UpdateApplication(updateConf *UpdateConfiguration, state *SyncIterationStat
 		log.Debugf("Using commit message: %s", wbc.GitCommitMessage)
 		if !updateConf.DryRun {
 			logCtx.Infof("Committing %d parameter update(s) for application %s", result.NumImagesUpdated, app)
-			err := commitChangesLocked(&updateConf.UpdateApp.Application, wbc, state)
+			err := commitChangesLocked(&updateConf.UpdateApp.Application, wbc, state, changeList)
 			if err != nil {
 				logCtx.Errorf("Could not update application spec: %v", err)
 				result.NumErrors += 1
@@ -447,7 +448,14 @@ func parseTarget(target string, sourcePath string) (kustomizeBase string) {
 func parseGitConfig(app *v1alpha1.Application, kubeClient *kube.KubernetesClient, wbc *WriteBackConfig, creds string) error {
 	branch, ok := app.Annotations[common.GitBranchAnnotation]
 	if ok {
-		wbc.GitBranch = strings.TrimSpace(branch)
+		branches := strings.Split(strings.TrimSpace(branch), ":")
+		if len(branches) > 2 {
+			return fmt.Errorf("invalid format for git-branch annotation: %v", branch)
+		}
+		wbc.GitBranch = branches[0]
+		if len(branches) == 2 {
+			wbc.GitWriteBranch = branches[1]
+		}
 	}
 	credsSource, err := getGitCredsSource(creds, kubeClient)
 	if err != nil {
@@ -457,19 +465,19 @@ func parseGitConfig(app *v1alpha1.Application, kubeClient *kube.KubernetesClient
 	return nil
 }
 
-func commitChangesLocked(app *v1alpha1.Application, wbc *WriteBackConfig, state *SyncIterationState) error {
+func commitChangesLocked(app *v1alpha1.Application, wbc *WriteBackConfig, state *SyncIterationState, changeList []ChangeEntry) error {
 	if wbc.RequiresLocking() {
 		lock := state.GetRepositoryLock(app.Spec.Source.RepoURL)
 		lock.Lock()
 		defer lock.Unlock()
 	}
 
-	return commitChanges(app, wbc)
+	return commitChanges(app, wbc, changeList)
 }
 
 // commitChanges commits any changes required for updating one or more images
 // after the UpdateApplication cycle has finished.
-func commitChanges(app *v1alpha1.Application, wbc *WriteBackConfig) error {
+func commitChanges(app *v1alpha1.Application, wbc *WriteBackConfig, changeList []ChangeEntry) error {
 	switch wbc.Method {
 	case WriteBackApplication:
 		_, err := wbc.ArgoClient.UpdateSpec(context.TODO(), &application.ApplicationUpdateSpecRequest{
@@ -482,9 +490,9 @@ func commitChanges(app *v1alpha1.Application, wbc *WriteBackConfig) error {
 	case WriteBackGit:
 		// if the kustomize base is set, the target is a kustomization
 		if wbc.KustomizeBase != "" {
-			return commitChangesGit(app, wbc, writeKustomization)
+			return commitChangesGit(app, wbc, changeList, writeKustomization)
 		}
-		return commitChangesGit(app, wbc, writeOverrides)
+		return commitChangesGit(app, wbc, changeList, writeOverrides)
 	default:
 		return fmt.Errorf("unknown write back method set: %d", wbc.Method)
 	}
diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go
index 698ece1..e31f1ed 100644
--- a/pkg/argocd/update_test.go
+++ b/pkg/argocd/update_test.go
@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/argoproj-labs/argocd-image-updater/ext/git"
 	gitmock "github.com/argoproj-labs/argocd-image-updater/ext/git/mocks"
@@ -17,6 +18,7 @@ import (
 	"github.com/argoproj-labs/argocd-image-updater/pkg/kube"
 	"github.com/argoproj-labs/argocd-image-updater/pkg/registry"
 	regmock "github.com/argoproj-labs/argocd-image-updater/pkg/registry/mocks"
+	"github.com/argoproj-labs/argocd-image-updater/pkg/tag"
 	"github.com/argoproj-labs/argocd-image-updater/test/fake"
 	"github.com/argoproj-labs/argocd-image-updater/test/fixture"
 
@@ -1239,7 +1241,7 @@ func Test_GetWriteBackConfig(t *testing.T) {
 				Annotations: map[string]string{
 					"argocd-image-updater.argoproj.io/image-list":        "nginx",
 					"argocd-image-updater.argoproj.io/write-back-method": "git",
-					"argocd-image-updater.argoproj.io/git-branch":        "mybranch",
+					"argocd-image-updater.argoproj.io/git-branch":        "mybranch:mytargetbranch",
 				},
 			},
 			Spec: v1alpha1.ApplicationSpec{
@@ -1264,6 +1266,58 @@ func Test_GetWriteBackConfig(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, wbc)
 		assert.Equal(t, wbc.Method, WriteBackGit)
+		assert.Equal(t, "mybranch", wbc.GitBranch)
+		assert.Equal(t, "mytargetbranch", wbc.GitWriteBranch)
+	})
+
+	t.Run("Valid git branch name determiniation - write branch only", func(t *testing.T) {
+		app := v1alpha1.Application{
+			ObjectMeta: v1.ObjectMeta{
+				Name: "testapp",
+				Annotations: map[string]string{
+					"argocd-image-updater.argoproj.io/write-back-method": "git",
+					"argocd-image-updater.argoproj.io/git-branch":        ":mytargetbranch",
+				},
+			},
+		}
+
+		argoClient := argomock.ArgoCD{}
+		argoClient.On("UpdateSpec", mock.Anything, mock.Anything).Return(nil, nil)
+
+		kubeClient := kube.KubernetesClient{
+			Clientset: fake.NewFakeKubeClient(),
+		}
+
+		wbc, err := getWriteBackConfig(&app, &kubeClient, &argoClient)
+		require.NoError(t, err)
+		require.NotNil(t, wbc)
+		assert.Equal(t, "", wbc.GitBranch)
+		assert.Equal(t, "mytargetbranch", wbc.GitWriteBranch)
+	})
+
+	t.Run("Valid git branch name determiniation - base branch only", func(t *testing.T) {
+		app := v1alpha1.Application{
+			ObjectMeta: v1.ObjectMeta{
+				Name: "testapp",
+				Annotations: map[string]string{
+					"argocd-image-updater.argoproj.io/write-back-method": "git",
+					"argocd-image-updater.argoproj.io/git-branch":        "mybranch",
+				},
+			},
+		}
+
+		argoClient := argomock.ArgoCD{}
+		argoClient.On("UpdateSpec", mock.Anything, mock.Anything).Return(nil, nil)
+
+		kubeClient := kube.KubernetesClient{
+			Clientset: fake.NewFakeKubeClient(),
+		}
+
+		wbc, err := getWriteBackConfig(&app, &kubeClient, &argoClient)
+		require.NoError(t, err)
+		require.NotNil(t, wbc)
+		assert.Equal(t, "mybranch", wbc.GitBranch)
+		assert.Equal(t, "", wbc.GitWriteBranch)
 	})
 
 	t.Run("Valid write-back config - argocd", func(t *testing.T) {
@@ -1682,7 +1736,7 @@ func Test_CommitUpdates(t *testing.T) {
 		require.NoError(t, err)
 		wbc.GitClient = gitMock
 
-		err = commitChanges(&app, wbc)
+		err = commitChanges(&app, wbc, nil)
 		assert.NoError(t, err)
 	})
 
@@ -1702,7 +1756,7 @@ func Test_CommitUpdates(t *testing.T) {
 		wbc.GitClient = gitMock
 		wbc.GitBranch = "mybranch"
 
-		err = commitChanges(&app, wbc)
+		err = commitChanges(&app, wbc, nil)
 		assert.NoError(t, err)
 	})
 
@@ -1723,7 +1777,37 @@ func Test_CommitUpdates(t *testing.T) {
 		app.Spec.Source.TargetRevision = "HEAD"
 		wbc.GitBranch = ""
 
-		err = commitChanges(app, wbc)
+		err = commitChanges(app, wbc, nil)
+		assert.NoError(t, err)
+	})
+
+	t.Run("Good commit to different than base branch", func(t *testing.T) {
+		gitMock, _, cleanup := mockGit(t)
+		defer cleanup()
+		gitMock.On("Checkout", mock.Anything).Run(func(args mock.Arguments) {
+			args.Assert(t, "mydefaultbranch")
+		}).Return(nil)
+		gitMock.On("Add", mock.Anything).Return(nil)
+		gitMock.On("Branch", mock.Anything, mock.Anything).Return(nil)
+		gitMock.On("Commit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
+		gitMock.On("Push", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+		gitMock.On("SymRefToBranch", mock.Anything).Return("mydefaultbranch", nil)
+
+		wbc, err := getWriteBackConfig(&app, &kubeClient, &argoClient)
+		require.NoError(t, err)
+		wbc.GitClient = gitMock
+		wbc.GitBranch = "mydefaultbranch"
+		wbc.GitWriteBranch = "image-updater{{range .Images}}-{{.Name}}-{{.NewTag}}{{end}}"
+
+		cl := []ChangeEntry{
+			{
+				Image:  image.NewFromIdentifier("foo/bar"),
+				OldTag: tag.NewImageTag("1.0", time.Now(), ""),
+				NewTag: tag.NewImageTag("1.1", time.Now(), ""),
+			},
+		}
+
+		err = commitChanges(&app, wbc, cl)
 		assert.NoError(t, err)
 	})
 
@@ -1754,7 +1838,7 @@ replacements: []
 		app.Spec.Source.TargetRevision = "HEAD"
 		wbc.GitBranch = ""
 
-		err = commitChanges(app, wbc)
+		err = commitChanges(app, wbc, nil)
 		assert.NoError(t, err)
 		kust, err := ioutil.ReadFile(kf)
 		assert.NoError(t, err)
@@ -1773,7 +1857,7 @@ replacements: []
 
 		// test the merge case too
 		app.Spec.Source.Kustomize.Images = v1alpha1.KustomizeImages{"foo:123", "bar=qux"}
-		err = commitChanges(app, wbc)
+		err = commitChanges(app, wbc, nil)
 		assert.NoError(t, err)
 		kust, err = ioutil.ReadFile(kf)
 		assert.NoError(t, err)
@@ -1812,7 +1896,7 @@ replacements: []
 		wbc.GitCommitUser = "someone"
 		wbc.GitCommitEmail = "someone@example.com"
 
-		err = commitChanges(app, wbc)
+		err = commitChanges(app, wbc, nil)
 		assert.NoError(t, err)
 	})
 
@@ -1839,7 +1923,7 @@ replacements: []
 		wbc.GitCommitUser = "someone"
 		wbc.GitCommitEmail = "someone@example.com"
 
-		err = commitChanges(app, wbc)
+		err = commitChanges(app, wbc, nil)
 		assert.Errorf(t, err, "could not configure git")
 	})
 
@@ -1854,7 +1938,7 @@ replacements: []
 		require.NoError(t, err)
 		wbc.GitClient = gitMock
 
-		err = commitChanges(&app, wbc)
+		err = commitChanges(&app, wbc, nil)
 		assert.Errorf(t, err, "cannot init")
 	})
 
@@ -1869,7 +1953,7 @@ replacements: []
 		require.NoError(t, err)
 		wbc.GitClient = gitMock
 
-		err = commitChanges(&app, wbc)
+		err = commitChanges(&app, wbc, nil)
 		assert.Errorf(t, err, "cannot init")
 	})
 	t.Run("Cannot checkout", func(t *testing.T) {
@@ -1883,7 +1967,7 @@ replacements: []
 		require.NoError(t, err)
 		wbc.GitClient = gitMock
 
-		err = commitChanges(&app, wbc)
+		err = commitChanges(&app, wbc, nil)
 		assert.Errorf(t, err, "cannot checkout")
 	})
 
@@ -1898,7 +1982,7 @@ replacements: []
 		require.NoError(t, err)
 		wbc.GitClient = gitMock
 
-		err = commitChanges(&app, wbc)
+		err = commitChanges(&app, wbc, nil)
 		assert.Errorf(t, err, "cannot commit")
 	})
 
@@ -1913,7 +1997,7 @@ replacements: []
 		require.NoError(t, err)
 		wbc.GitClient = gitMock
 
-		err = commitChanges(&app, wbc)
+		err = commitChanges(&app, wbc, nil)
 		assert.Errorf(t, err, "cannot push")
 	})
 
@@ -1932,7 +2016,7 @@ replacements: []
 		app.Spec.Source.TargetRevision = "HEAD"
 		wbc.GitBranch = ""
 
-		err = commitChanges(app, wbc)
+		err = commitChanges(app, wbc, nil)
 		assert.Errorf(t, err, "failed to resolve ref")
 	})
 }