Add ability to specify the commit message (and use this message in the PR details), also enhance the default message. (#49)

Author: dibbles

cmd/services/main.go

@@ -21,6 +21,7 @@ const (
 	toFlag          = "to"
 	serviceFlag     = "service"
 	cacheDirFlag    = "cache-dir"
+	msgFlag         = "commit-message"
 	nameFlag        = "commit-name"
 	emailFlag       = "commit-email"
 	debugFlag       = "debug"
@@ -77,6 +78,12 @@ var (
 			Required: false,
 			EnvVars:  []string{"COMMIT_EMAIL"},
 		},
+		&cli.StringFlag{
+			Name:     msgFlag,
+			Usage:    "the msg to use on the resultant commit and pull request",
+			Required: false,
+			EnvVars:  []string{"COMMIT_MSG"},
+		},
 		&cli.BoolFlag{
 			Name:     debugFlag,
 			Usage:    "additional debug logging output",
@@ -121,6 +128,7 @@ func promoteAction(c *cli.Context) error {
 	newBranchName := c.String(branchNameFlag)
 	debug := c.Bool(debugFlag)
 	keepCache := c.Bool(keepCacheFlag)
+	msg := c.String(msgFlag)
 
 	cacheDir, err := homedir.Expand(c.String(cacheDirFlag))
 	if err != nil {
@@ -131,7 +139,7 @@ func promoteAction(c *cli.Context) error {
 	if err != nil {
 		return fmt.Errorf("unable to establish credentials: %w", err)
 	}
-	return avancement.New(cacheDir, author, avancement.WithDebug(debug)).Promote(service, fromRepo, toRepo, newBranchName, keepCache)
+	return avancement.New(cacheDir, author, avancement.WithDebug(debug)).Promote(service, fromRepo, toRepo, newBranchName, msg, keepCache)
 }
 
 func newAuthor(c *cli.Context) (*git.Author, error) {

pkg/avancement/pull_request.go

@@ -10,20 +10,28 @@ import (
 // TODO: OptionFuncs for Base and Title?
 // TODO: For the Head, should this try and determine whether or not this is a
 // fork ("user" of both repoURLs) and if so, simplify the Head?
-func makePullRequestInput(fromURL, toURL, branchName string) (*scm.PullRequestInput, error) {
-	_, fromRepo, err := util.ExtractUserAndRepo(fromURL)
-	if err != nil {
-		return nil, err
-	}
+func makePullRequestInput(fromLocal bool, fromURL, toURL, branchName, prBody string) (*scm.PullRequestInput, error) {
+	var title string
+
 	fromUser, toRepo, err := util.ExtractUserAndRepo(toURL)
 	if err != nil {
 		return nil, err
 	}
-	title := fmt.Sprintf("promotion from %s to %s", fromRepo, toRepo)
+
+	if fromLocal {
+		title = fmt.Sprintf("promotion from local filesystem directory to %s", toRepo)
+	} else {
+		_, fromRepo, err := util.ExtractUserAndRepo(fromURL)
+		if err != nil {
+			return nil, err
+		}
+		title = fmt.Sprintf("promotion from %s to %s", fromRepo, toRepo)
+	}
+
 	return &scm.PullRequestInput{
 		Title: title,
 		Head:  fmt.Sprintf("%s:%s", fromUser, branchName),
 		Base:  "master",
-		Body:  "this is a test body",
+		Body:  prBody,
 	}, nil
 }

pkg/avancement/pull_request_test.go

@@ -9,7 +9,7 @@ import (
 )
 
 func TestMakePullRequestInput(t *testing.T) {
-	pr, err := makePullRequestInput("https://example.com/project/dev-env.git", "https://example.com/project/prod-env.git", "my-test-branch")
+	pr, err := makePullRequestInput(false, "https://example.com/project/dev-env.git", "https://example.com/project/prod-env.git", "my-test-branch", "foo bar wibble")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -18,7 +18,7 @@ func TestMakePullRequestInput(t *testing.T) {
 		Title: "promotion from dev-env to prod-env",
 		Head:  "project:my-test-branch",
 		Base:  "master",
-		Body:  "this is a test body",
+		Body:  "foo bar wibble",
 	}
 
 	if diff := cmp.Diff(want, pr); diff != "" {

pkg/avancement/service_manager.go

@@ -26,10 +26,6 @@ type ServiceManager struct {
 	debug         bool
 }
 
-const (
-	defaultCommitMsg = "this is a commit"
-)
-
 type scmClientFactory func(token string) *scm.Client
 type repoFactory func(url, localPath string, debug bool) (git.Repo, error)
 type localFactory func(localPath string, debug bool) git.Source
@@ -75,7 +71,7 @@ const fromBranch = "master"
 //
 // It uses a Git cache to checkout the code to, and will copy the environment
 // configuration for the `fromURL` to the `toURL` in a named branch.
-func (s *ServiceManager) Promote(serviceName, fromURL, toURL, newBranchName string, keepCache bool) error {
+func (s *ServiceManager) Promote(serviceName, fromURL, toURL, newBranchName, message string, keepCache bool) error {
 	var source, destination git.Repo
 	var reposToDelete []git.Repo
 
@@ -92,7 +88,8 @@ func (s *ServiceManager) Promote(serviceName, fromURL, toURL, newBranchName stri
 
 	var localSource git.Source
 	var errorSource error
-	if fromLocalRepo(fromURL) {
+	isLocal := fromLocalRepo(fromURL)
+	if isLocal {
 		localSource = s.localFactory(fromURL, s.debug)
 	} else {
 		source, errorSource = s.checkoutSourceRepo(fromURL, fromBranch)
@@ -113,7 +110,7 @@ func (s *ServiceManager) Promote(serviceName, fromURL, toURL, newBranchName stri
 	reposToDelete = append(reposToDelete, destination)
 
 	var copied []string
-	if fromLocalRepo(fromURL) {
+	if isLocal {
 		copied, err = local.CopyConfig(serviceName, localSource, destination)
 		if err != nil {
 			return fmt.Errorf("failed to setup local repo: %w", err)
@@ -125,18 +122,28 @@ func (s *ServiceManager) Promote(serviceName, fromURL, toURL, newBranchName stri
 		}
 	}
 
+	commitMsg := message
+	if commitMsg == "" {
+		if isLocal {
+			commitMsg = fmt.Sprintf("Promotion of service `%s` from local filesystem directory `%s`.", serviceName, fromURL)
+		} else {
+			commitMsg = generateDefaultCommitMsg(source, serviceName, fromURL, fromBranch)
+		}
+	}
+
 	if err := destination.StageFiles(copied...); err != nil {
 		return fmt.Errorf("failed to stage files: %w", err)
 	}
-	if err := destination.Commit(defaultCommitMsg, s.author); err != nil {
+	if err := destination.Commit(commitMsg, s.author); err != nil {
 		return fmt.Errorf("failed to commit: %w", err)
 	}
+
 	if err := destination.Push(newBranchName); err != nil {
 		return fmt.Errorf("failed to push: %w", err)
 	}
 
 	ctx := context.Background()
-	pr, err := createPullRequest(ctx, fromURL, toURL, newBranchName, s.clientFactory(s.author.Token))
+	pr, err := createPullRequest(ctx, fromURL, toURL, newBranchName, commitMsg, s.clientFactory(s.author.Token), isLocal)
 	if err != nil {
 		return fmt.Errorf("failed to create a pull-request for branch %s in %v: %w", newBranchName, toURL, err)
 	}
@@ -185,13 +192,12 @@ func (s *ServiceManager) cloneRepo(repoURL, branch string) (git.Repo, error) {
 	return repo, nil
 }
 
-func createPullRequest(ctx context.Context, fromURL, toURL, newBranchName string, client *scm.Client) (*scm.PullRequest, error) {
-	prInput, err := makePullRequestInput(fromURL, toURL, newBranchName)
+func createPullRequest(ctx context.Context, fromURL, toURL, newBranchName, commitMsg string, client *scm.Client, fromLocal bool) (*scm.PullRequest, error) {
+	prInput, err := makePullRequestInput(fromLocal, fromURL, toURL, newBranchName, commitMsg)
 	if err != nil {
 		// TODO: improve this error message
 		return nil, err
 	}
-
 	user, repo, err := util.ExtractUserAndRepo(toURL)
 	if err != nil {
 		// TODO: improve this error message
@@ -218,6 +224,7 @@ func addCredentialsIfNecessary(s string, a *git.Author) (string, error) {
 	parsed.User = url.UserPassword("promotion", a.Token)
 	return parsed.String(), nil
 }
+
 func fromLocalRepo(s string) bool {
 	parsed, err := url.Parse(s)
 	if err != nil || parsed.Scheme == "" {
@@ -230,6 +237,13 @@ func generateBranch(repo git.Repo) string {
 	uniqueString := uuid.New()
 	runes := []rune(uniqueString.String())
 	branchName := repo.GetName() + "-" + repo.GetCommitID() + "-" + string(runes[0:5])
-	branchName = strings.Replace(branchName, "\n","",-1)
+	branchName = strings.Replace(branchName, "\n", "", -1)
 	return branchName
 }
+
+// generateDefaultCommitMsg constructs a default commit message based on the source information.
+func generateDefaultCommitMsg(sourceRepo git.Repo, serviceName, from, fromBranch string) string {
+	commit := sourceRepo.GetCommitID()
+	msg := fmt.Sprintf("Promoting service `%s` at commit `%s` from branch `%s` in `%s`.", serviceName, commit, fromBranch, from)
+	return msg
+}

pkg/avancement/service_manager_test.go

@@ -2,11 +2,12 @@ package avancement
 
 import (
 	"errors"
+	"fmt"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
-	"regexp"
 
 	"github.com/jenkins-x/go-scm/scm"
 	fakescm "github.com/jenkins-x/go-scm/scm/driver/fake"
@@ -21,14 +22,18 @@ const (
 )
 
 func TestPromoteWithSuccessKeepCacheTrue(t *testing.T) {
-	promoteWithSuccess(t, true)
+	promoteWithSuccess(t, true, "")
 }
 
 func TestPromoteWithSuccessKeepCacheFalse(t *testing.T) {
-	promoteWithSuccess(t, false)
+	promoteWithSuccess(t, false, "")
+}
+
+func TestPromoteWithSuccessCustomMsg(t *testing.T) {
+	promoteLocalWithSuccess(t, true, "custom commit message here")
 }
 
-func promoteWithSuccess(t *testing.T, keepCache bool) {
+func promoteWithSuccess(t *testing.T, keepCache bool, msg string) {
 	dstBranch := "test-branch"
 	author := &git.Author{Name: "Testing User", Email: "[email protected]", Token: "test-token"}
 	devRepo, stagingRepo := mock.New("/dev", "master"), mock.New("/staging", "master")
@@ -46,14 +51,20 @@ func promoteWithSuccess(t *testing.T, keepCache bool) {
 	}
 	devRepo.AddFiles("/services/my-service/base/config/myfile.yaml")
 
-	err := sm.Promote("my-service", dev, staging, dstBranch, keepCache)
+	err := sm.Promote("my-service", dev, staging, dstBranch, msg, keepCache)
 	if err != nil {
 		t.Fatal(err)
 	}
 
+	expectedCommitMsg := msg
+	if msg == "" {
+		commit := devRepo.GetCommitID()
+		expectedCommitMsg = fmt.Sprintf("Promoting service `my-service` at commit `%s` from branch `master` in `%s`.", commit, dev)
+	}
+
 	stagingRepo.AssertBranchCreated(t, "master", dstBranch)
 	stagingRepo.AssertFileCopiedInBranch(t, dstBranch, "/dev/services/my-service/base/config/myfile.yaml", "/staging/services/my-service/base/config/myfile.yaml")
-	stagingRepo.AssertCommit(t, dstBranch, defaultCommitMsg, author)
+	stagingRepo.AssertCommit(t, dstBranch, expectedCommitMsg, author)
 	stagingRepo.AssertPush(t, dstBranch)
 
 	if keepCache {
@@ -66,14 +77,18 @@ func promoteWithSuccess(t *testing.T, keepCache bool) {
 }
 
 func TestPromoteLocalWithSuccessKeepCacheFalse(t *testing.T) {
-	promoteLocalWithSuccess(t, false)
+	promoteLocalWithSuccess(t, false, "")
 }
 
 func TestPromoteLocalWithSuccessKeepCacheTrue(t *testing.T) {
-	promoteLocalWithSuccess(t, true)
+	promoteLocalWithSuccess(t, true, "")
 }
 
-func promoteLocalWithSuccess(t *testing.T, keepCache bool) {
+func TestPromoteLocalWithSuccessCustomMsg(t *testing.T) {
+	promoteLocalWithSuccess(t, true, "custom commit message supplied")
+}
+
+func promoteLocalWithSuccess(t *testing.T, keepCache bool, msg string) {
 	dstBranch := "test-branch"
 	author := &git.Author{Name: "Testing User", Email: "[email protected]", Token: "test-token"}
 	stagingRepo := mock.New("/staging", "master")
@@ -93,14 +108,19 @@ func promoteLocalWithSuccess(t *testing.T, keepCache bool) {
 	sm.debug = true
 	devRepo.AddFiles("/config/myfile.yaml")
 
-	err := sm.Promote("my-service", ldev, staging, dstBranch, keepCache)
+	err := sm.Promote("my-service", ldev, staging, dstBranch, msg, keepCache)
 	if err != nil {
 		t.Fatal(err)
 	}
 
+	expectedCommitMsg := msg
+	if expectedCommitMsg == "" {
+		expectedCommitMsg = "Promotion of service `my-service` from local filesystem directory `/root/repo`."
+	}
+
 	stagingRepo.AssertBranchCreated(t, "master", dstBranch)
 	stagingRepo.AssertFileCopiedInBranch(t, dstBranch, "/dev/config/myfile.yaml", "/staging/services/my-service/base/config/myfile.yaml")
-	stagingRepo.AssertCommit(t, dstBranch, defaultCommitMsg, author)
+	stagingRepo.AssertCommit(t, dstBranch, expectedCommitMsg, author)
 	stagingRepo.AssertPush(t, dstBranch)
 
 	if keepCache {
@@ -161,22 +181,25 @@ func TestPromoteWithCacheDeletionFailure(t *testing.T) {
 	}
 	devRepo.AddFiles("/services/my-service/base/config/myfile.yaml")
 
-	err := sm.Promote("my-service", dev, staging, dstBranch, false)
+	err := sm.Promote("my-service", dev, staging, dstBranch, "", false)
 	if err != nil {
 		t.Fatal(err)
 	}
 
+	commit := devRepo.GetCommitID()
+	expectedCommitMsg := fmt.Sprintf("Promoting service `my-service` at commit `%s` from branch `master` in `%s`.", commit, dev)
+
 	stagingRepo.AssertBranchCreated(t, "master", dstBranch)
 	stagingRepo.AssertFileCopiedInBranch(t, dstBranch, "/dev/services/my-service/base/config/myfile.yaml", "/staging/services/my-service/base/config/myfile.yaml")
-	stagingRepo.AssertCommit(t, dstBranch, defaultCommitMsg, author)
+	stagingRepo.AssertCommit(t, dstBranch, expectedCommitMsg, author)
 	stagingRepo.AssertPush(t, dstBranch)
 
 	stagingRepo.AssertNotDeletedFromCache(t)
 	devRepo.AssertDeletedFromCache(t)
 }
 
 func TestGenerateBranchWithSuccess(t *testing.T) {
-    repo := mock.New("/dev", "master")
+	repo := mock.New("/dev", "master")
 	GenerateBranchWithSuccess(t, repo)
 }
 

pkg/git/mock/mock.go

@@ -186,7 +186,7 @@ func (m *Repository) AssertFileCopiedInBranch(t *testing.T, branch, from, name s
 // message and auth token.
 func (m *Repository) AssertCommit(t *testing.T, branch, msg string, a *git.Author) {
 	if !hasString(key(branch, msg, a.Token), m.commits) {
-		t.Fatalf("no matching commit %#v in branch %s using token %s", msg, branch, a.Token)
+		t.Fatalf("no matching commit %#v in branch %s using token %s.  Commits available: %+v", msg, branch, a.Token, m.commits)
 	}
 }