diff --git a/cmd/fluxctl/args.go b/cmd/fluxctl/args.go index 2943b3f69..c574d9b9b 100644 --- a/cmd/fluxctl/args.go +++ b/cmd/fluxctl/args.go @@ -1,7 +1,10 @@ package main import ( - "os/user" + "bytes" + "fmt" + "os/exec" + "strings" "github.com/spf13/cobra" @@ -16,11 +19,61 @@ func parseServiceOption(s string) (update.ServiceSpec, error) { } func AddCauseFlags(cmd *cobra.Command, opts *update.Cause) { - username := "" - user, err := user.Current() - if err == nil { - username = user.Username - } + authorInfo := getUserGitconfig() + username := getCommitAuthor(authorInfo) + cmd.Flags().StringVarP(&opts.Message, "message", "m", "", "attach a message to the update") - cmd.Flags().StringVar(&opts.User, "user", username, "override the user reported as initating the update") + cmd.Flags().StringVar(&opts.User, "user", username, "override the user reported as initiating the update") +} + +func getCommitAuthor(authorInfo map[string]string) string { + userName := authorInfo["user.name"] + userEmail := authorInfo["user.email"] + + switch { + case userName != "" && userEmail != "": + return fmt.Sprintf("%s <%s>", userName, userEmail) + case userEmail != "": + return userEmail + case userName != "": + return userName + } + return "" +} + +func getUserGitconfig() map[string]string { + var out bytes.Buffer + userGitconfig := make(map[string]string) + cmd := exec.Command("git", "config", "--list") + cmd.Stdout = &out + + err := cmd.Run() + if err != nil { + return userGitconfig + } + res := out.String() + return userGitconfigMap(res) +} + +func userGitconfigMap(s string) map[string]string { + c := make(map[string]string) + lines := splitList(s) + for _, l := range lines { + if l == "" { + continue + } + prop := strings.SplitN(l, "=", 2) + p := strings.TrimSpace(prop[0]) + v := strings.TrimSpace(prop[1]) + c[p] = v + } + return c +} + +func splitList(s string) []string { + outStr := strings.TrimSpace(s) + if outStr == "" { + return []string{} + } + return strings.Split(outStr, "\n") } diff --git a/cmd/fluxctl/args_test.go b/cmd/fluxctl/args_test.go new file mode 100644 index 000000000..f5af3c8b1 --- /dev/null +++ b/cmd/fluxctl/args_test.go @@ -0,0 +1,188 @@ +package main + +import ( + "reflect" + "testing" +) + +func TestUserGitconfigMap_EmptyString(t *testing.T) { + d := "" + userGitconfigInfo := userGitconfigMap(d) + if len(userGitconfigInfo) != 0 { + t.Fatal("expected map with no keys") + } +} + +func TestUserGitconfigMap(t *testing.T) { + d := `push.default=simple + merge.conflictstyle=diff3 + pull.ff=only + core.repositoryformatversion=0 + core.filemode=true + core.bare=false` + expected := map[string]string{ + "push.default": "simple", + "merge.conflictstyle": "diff3", + "pull.ff": "only", + "core.repositoryformatversion": "0", + "core.filemode": "true", + "core.bare": "false", + } + + userGitconfigInfo := userGitconfigMap(d) + if len(userGitconfigInfo) != 6 { + t.Fatal("got map with unexpected number of keys") + } + if !reflect.DeepEqual(userGitconfigInfo, expected) { + t.Fatal("result does not match expected structure") + } +} + +func TestUserGitconfigMap_WithEmptyLines(t *testing.T) { + d := ` + user.name=Jane Doe + push.default=simple + merge.conflictstyle=diff3 + pull.ff=only + + core.repositoryformatversion=0 + core.filemode=true + core.bare=false + + ` + expected := map[string]string{ + "user.name": "Jane Doe", + "push.default": "simple", + "merge.conflictstyle": "diff3", + "pull.ff": "only", + "core.repositoryformatversion": "0", + "core.filemode": "true", + "core.bare": "false", + } + userGitconfigInfo := userGitconfigMap(d) + + if len(userGitconfigInfo) != 7 { + t.Fatal("got map with unexpected number of keys") + } + if !reflect.DeepEqual(userGitconfigInfo, expected) { + t.Fatal("result does not match expected structure") + } +} + +func TestUserGitconfigMap_WithNoKeys(t *testing.T) { + d := ` + ` + expected := make(map[string]string) + + userGitconfigInfo := userGitconfigMap(d) + if len(userGitconfigInfo) != 0 { + t.Fatal("expected map with no keys") + } + if !reflect.DeepEqual(userGitconfigInfo, expected) { + t.Fatal("result does not match expected structure") + } +} + +func TestGetCommitAuthor_BothNameAndEmail(t *testing.T) { + input := map[string]string{ + "user.name": "Jane Doe", + "user.email": "jd@j.d", + "push.default": "simple", + "merge.conflictstyle": "diff3", + "pull.ff": "only", + "core.repositoryformatversion": "0", + "core.filemode": "true", + "core.bare": "false", + } + expected := "Jane Doe " + author := getCommitAuthor(input) + if author != expected { + t.Fatal("author did not match expected value") + } +} + +func TestGetCommitAuthor_OnlyName(t *testing.T) { + input := map[string]string{ + "user.name": "Jane Doe", + "push.default": "simple", + "merge.conflictstyle": "diff3", + "pull.ff": "only", + "core.repositoryformatversion": "0", + "core.filemode": "true", + "core.bare": "false", + } + expected := "Jane Doe" + author := getCommitAuthor(input) + if author != expected { + t.Fatal("author did not match expected value") + } +} + +func TestGetCommitAuthor_OnlyEmail(t *testing.T) { + input := map[string]string{ + "user.email": "jd@j.d", + "push.default": "simple", + "merge.conflictstyle": "diff3", + "pull.ff": "only", + "core.repositoryformatversion": "0", + "core.filemode": "true", + "core.bare": "false", + } + expected := "jd@j.d" + author := getCommitAuthor(input) + if author != expected { + t.Fatal("author did not match expected value") + } +} + +func TestGetCommitAuthor_NoNameNoEmail(t *testing.T) { + input := map[string]string{ + "push.default": "simple", + "merge.conflictstyle": "diff3", + "pull.ff": "only", + "core.repositoryformatversion": "0", + "core.filemode": "true", + "core.bare": "false", + } + expected := "" + author := getCommitAuthor(input) + if author != expected { + t.Fatal("author did not match expected value") + } +} + +func TestGetCommitAuthor_NameAndEmptyEmail(t *testing.T) { + input := map[string]string{ + "user.name": "Jane Doe", + "user.email": "", + "push.default": "simple", + "merge.conflictstyle": "diff3", + "pull.ff": "only", + "core.repositoryformatversion": "0", + "core.filemode": "true", + "core.bare": "false", + } + expected := "Jane Doe" + author := getCommitAuthor(input) + if author != expected { + t.Fatal("author did not match expected value") + } +} + +func TestGetCommitAuthor_EmailAndEmptyName(t *testing.T) { + input := map[string]string{ + "user.name": "", + "user.email": "jd@j.d", + "push.default": "simple", + "merge.conflictstyle": "diff3", + "pull.ff": "only", + "core.repositoryformatversion": "0", + "core.filemode": "true", + "core.bare": "false", + } + expected := "jd@j.d" + author := getCommitAuthor(input) + if author != expected { + t.Fatal("author did not match expected value") + } +} diff --git a/cmd/fluxd/main.go b/cmd/fluxd/main.go index 88c4e33eb..dd18977b2 100644 --- a/cmd/fluxd/main.go +++ b/cmd/fluxd/main.go @@ -20,6 +20,7 @@ import ( "k8s.io/client-go/1.5/rest" "context" + "github.com/weaveworks/flux" "github.com/weaveworks/flux/cluster" "github.com/weaveworks/flux/cluster/kubernetes" @@ -71,12 +72,13 @@ func main() { kubernetesKubectl = fs.String("kubernetes-kubectl", "", "Optional, explicit path to kubectl tool") versionFlag = fs.Bool("version", false, "Get version number") // Git repo & key etc. - gitURL = fs.String("git-url", "", "URL of git repo with Kubernetes manifests; e.g., git@github.com:weaveworks/flux-example") - gitBranch = fs.String("git-branch", "master", "branch of git repo to use for Kubernetes manifests") - gitPath = fs.String("git-path", "", "path within git repo to locate Kubernetes manifests (relative path)") - gitUser = fs.String("git-user", "Weave Flux", "username to use as git committer") - gitEmail = fs.String("git-email", "support@weave.works", "email to use as git committer") - gitLabel = fs.String("git-label", "", "label to keep track of sync progress; overrides both --git-sync-tag and --git-notes-ref") + gitURL = fs.String("git-url", "", "URL of git repo with Kubernetes manifests; e.g., git@github.com:weaveworks/flux-example") + gitBranch = fs.String("git-branch", "master", "branch of git repo to use for Kubernetes manifests") + gitPath = fs.String("git-path", "", "path within git repo to locate Kubernetes manifests (relative path)") + gitUser = fs.String("git-user", "Weave Flux", "username to use as git committer") + gitEmail = fs.String("git-email", "support@weave.works", "email to use as git committer") + gitSetAuthor = fs.Bool("git-set-author", false, "If set, the author of git commits will reflect the user who initiated the commit and will differ from the git committer.") + gitLabel = fs.String("git-label", "", "label to keep track of sync progress; overrides both --git-sync-tag and --git-notes-ref") // Old git config; still used if --git-label is not supplied, but --git-label is preferred. gitSyncTag = fs.String("git-sync-tag", defaultGitSyncTag, "tag to use to mark sync progress for this cluster") gitNotesRef = fs.String("git-notes-ref", defaultGitNotesRef, "ref to use for keeping commit annotations in git notes") @@ -360,6 +362,7 @@ func main() { NotesRef: *gitNotesRef, UserName: *gitUser, UserEmail: *gitEmail, + SetAuthor: *gitSetAuthor, } for checkout == nil { @@ -382,7 +385,8 @@ func main() { "user", *gitUser, "email", *gitEmail, "sync-tag", *gitSyncTag, - "notes-ref", *gitNotesRef) + "notes-ref", *gitNotesRef, + "set-author", *gitSetAuthor) checkout = working } } diff --git a/daemon/daemon.go b/daemon/daemon.go index 569ff8229..0912f219d 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -10,6 +10,7 @@ import ( "github.com/pkg/errors" "context" + "github.com/weaveworks/flux" "github.com/weaveworks/flux/cluster" // fluxerr "github.com/weaveworks/flux/errors" @@ -252,7 +253,12 @@ func (d *Daemon) updatePolicy(spec update.Spec, updates policy.Updates) DaemonJo return metadata, nil } - if err := working.CommitAndPush(ctx, policyCommitMessage(updates, spec.Cause), &git.Note{JobID: jobID, Spec: spec}); err != nil { + commitAuthor := "" + if d.Checkout.Config.SetAuthor { + commitAuthor = spec.Cause.User + } + commitAction := &git.CommitAction{Author: commitAuthor, Message: policyCommitMessage(updates, spec.Cause)} + if err := working.CommitAndPush(ctx, commitAction, &git.Note{JobID: jobID, Spec: spec}); err != nil { // On the chance pushing failed because it was not // possible to fast-forward, ask for a sync so the // next attempt is more likely to succeed. @@ -286,7 +292,12 @@ func (d *Daemon) release(spec update.Spec, c release.Changes) DaemonJobFunc { if commitMsg == "" { commitMsg = c.CommitMessage() } - if err := working.CommitAndPush(ctx, commitMsg, &git.Note{JobID: jobID, Spec: spec, Result: result}); err != nil { + commitAuthor := "" + if d.Checkout.Config.SetAuthor { + commitAuthor = spec.Cause.User + } + commitAction := &git.CommitAction{Author: commitAuthor, Message: commitMsg} + if err := working.CommitAndPush(ctx, commitAction, &git.Note{JobID: jobID, Spec: spec, Result: result}); err != nil { // On the chance pushing failed because it was not // possible to fast-forward, ask for a sync so the // next attempt is more likely to succeed. @@ -315,7 +326,7 @@ func (d *Daemon) SyncNotify() error { return nil } -// Ask the daemon how far it's got committing things; in particular, is the job +// JobStatus - Ask the daemon how far it's got committing things; in particular, is the job // queued? running? committed? If it is done, the commit ref is returned. func (d *Daemon) JobStatus(jobID job.ID) (job.Status, error) { ctx, cancel := context.WithTimeout(context.Background(), defaultHandlerTimeout) diff --git a/daemon/loop_test.go b/daemon/loop_test.go index 77e90f389..7580c9919 100644 --- a/daemon/loop_test.go +++ b/daemon/loop_test.go @@ -12,6 +12,7 @@ import ( "github.com/go-kit/kit/log" "context" + "github.com/weaveworks/flux" "github.com/weaveworks/flux/cluster" "github.com/weaveworks/flux/cluster/kubernetes" @@ -206,7 +207,9 @@ func TestDoSync_WithNewCommit(t *testing.T) { }); err != nil { t.Fatal(err) } - if err := d.Checkout.CommitAndPush(context.Background(), "test commit", nil); err != nil { + + commitAction := &git.CommitAction{Author: "", Message: "test commit"} + if err := d.Checkout.CommitAndPush(context.Background(), commitAction, nil); err != nil { t.Fatal(err) } newRevision, err := d.Checkout.HeadRevision(context.Background()) diff --git a/git/gittest/repo_test.go b/git/gittest/repo_test.go index 66add1ba3..59ce57600 100644 --- a/git/gittest/repo_test.go +++ b/git/gittest/repo_test.go @@ -7,6 +7,7 @@ import ( "testing" "context" + "github.com/weaveworks/flux" "github.com/weaveworks/flux/cluster/kubernetes/testfiles" "github.com/weaveworks/flux/git" @@ -63,7 +64,8 @@ func TestCheckout(t *testing.T) { changedFile = file break } - if err := working.CommitAndPush(ctx, "Changed file", nil); err != nil { + commitAction := &git.CommitAction{Author: "", Message: "Changed file"} + if err := working.CommitAndPush(ctx, commitAction, nil); err != nil { t.Fatal(err) } @@ -86,7 +88,8 @@ func TestCheckout(t *testing.T) { }, }, } - if err := working.CommitAndPush(ctx, "Changed file again", &expectedNote); err != nil { + commitAction = &git.CommitAction{Author: "", Message: "Changed file again"} + if err := working.CommitAndPush(ctx, commitAction, &expectedNote); err != nil { t.Fatal(err) } diff --git a/git/operations.go b/git/operations.go index c4f55b02e..a0fe94343 100644 --- a/git/operations.go +++ b/git/operations.go @@ -12,6 +12,7 @@ import ( "strings" "context" + "github.com/pkg/errors" "github.com/weaveworks/flux/ssh" ) @@ -41,11 +42,22 @@ func clone(ctx context.Context, workingDir string, keyRing ssh.KeyRing, repoURL, return repoPath, nil } -func commit(ctx context.Context, workingDir, commitMessage string) error { +func commit(ctx context.Context, workingDir string, commitAction *CommitAction) error { + commitAuthor := commitAction.Author + if commitAuthor != "" { + if err := execGitCmd(ctx, + workingDir, nil, nil, + "commit", + "--no-verify", "-a", "--author", commitAuthor, "-m", commitAction.Message, + ); err != nil { + return errors.Wrap(err, "git commit") + } + return nil + } if err := execGitCmd(ctx, workingDir, nil, nil, "commit", - "--no-verify", "-a", "-m", commitMessage, + "--no-verify", "-a", "-m", commitAction.Message, ); err != nil { return errors.Wrap(err, "git commit") } diff --git a/git/repo.go b/git/repo.go index f471a7225..09edb020e 100644 --- a/git/repo.go +++ b/git/repo.go @@ -8,9 +8,10 @@ import ( "sync" "context" + "time" + "github.com/weaveworks/flux" "github.com/weaveworks/flux/ssh" - "time" ) const ( @@ -43,6 +44,7 @@ type Config struct { NotesRef string UserName string UserEmail string + SetAuthor bool } type Commit struct { @@ -50,6 +52,12 @@ type Commit struct { Message string } +// CommitAction - struct holding commit information +type CommitAction struct { + Author string + Message string +} + // Get a local clone of the upstream repo, and use the config given. func (r Repo) Clone(ctx context.Context, c Config) (*Checkout, error) { if r.URL == "" { @@ -135,13 +143,13 @@ func (c *Checkout) ManifestDir() string { // CommitAndPush commits changes made in this checkout, along with any // extra data as a note, and pushes the commit and note to the remote repo. -func (c *Checkout) CommitAndPush(ctx context.Context, commitMessage string, note *Note) error { +func (c *Checkout) CommitAndPush(ctx context.Context, commitAction *CommitAction, note *Note) error { c.Lock() defer c.Unlock() if !check(ctx, c.Dir, c.repo.Path) { return ErrNoChanges } - if err := commit(ctx, c.Dir, commitMessage); err != nil { + if err := commit(ctx, c.Dir, commitAction); err != nil { return err } diff --git a/sync/sync_test.go b/sync/sync_test.go index 2a3ddd75a..b9cd4986c 100644 --- a/sync/sync_test.go +++ b/sync/sync_test.go @@ -14,6 +14,7 @@ import ( // "github.com/weaveworks/flux" "context" + "github.com/weaveworks/flux/cluster" "github.com/weaveworks/flux/cluster/kubernetes" "github.com/weaveworks/flux/cluster/kubernetes/testfiles" @@ -44,11 +45,12 @@ func TestSync(t *testing.T) { } checkClusterMatchesFiles(t, manifests, clus, checkout.ManifestDir()) - for file, _ := range testfiles.Files { + for file := range testfiles.Files { if err := execCommand("rm", filepath.Join(checkout.ManifestDir(), file)); err != nil { t.Fatal(err) } - if err := checkout.CommitAndPush(context.Background(), "deleted "+file, nil); err != nil { + commitAction := &git.CommitAction{Author: "", Message: "deleted " + file} + if err := checkout.CommitAndPush(context.Background(), commitAction, nil); err != nil { t.Fatal(err) } break @@ -66,7 +68,7 @@ func TestSync(t *testing.T) { // --- -var gitconf git.Config = git.Config{ +var gitconf = git.Config{ SyncTag: "test-sync", NotesRef: "test-notes", UserName: "testuser",