diff --git a/cmd/fluxd/main.go b/cmd/fluxd/main.go index 00d6317bb8..430984b7eb 100644 --- a/cmd/fluxd/main.go +++ b/cmd/fluxd/main.go @@ -134,9 +134,10 @@ func main() { gitTimeout = fs.Duration("git-timeout", 20*time.Second, "duration after which git operations time out") // GPG commit signing - gitImportGPG = fs.StringSlice("git-gpg-key-import", []string{}, "keys at the paths given will be imported for use of signing and verifying commits") - gitSigningKey = fs.String("git-signing-key", "", "if set, commits Flux makes will be signed with this GPG key") - gitVerifySignatures = fs.Bool("git-verify-signatures", false, "if set, the signature of commits will be verified before Flux applies them") + gitImportGPG = fs.StringSlice("git-gpg-key-import", []string{}, "keys at the paths given will be imported for use of signing and verifying commits") + gitSigningKey = fs.String("git-signing-key", "", "if set, commits Flux makes will be signed with this GPG key") + gitVerifySignatures = fs.Bool("git-verify-signatures", false, "(deprecated) sets --git-verify-signatures-mode=all when set") + gitVerifySignaturesModeStr = fs.String("git-verify-signatures-mode", fluxsync.VerifySignaturesModeDefault, fmt.Sprintf("if git-verify-signatures is set, which strategy to use for signature verification (one of %s)", strings.Join([]string{fluxsync.VerifySignaturesModeNone, fluxsync.VerifySignaturesModeAll, fluxsync.VerifySignaturesModeFirstParent}, ","))) // syncing syncInterval = fs.Duration("sync-interval", 5*time.Minute, "apply config in git to cluster at least this often, even if there are no new commits") @@ -202,6 +203,7 @@ func main() { fs.MarkDeprecated("k8s-namespace-whitelist", "changed to --k8s-allow-namespace, use that instead") fs.MarkDeprecated("registry-poll-interval", "changed to --automation-interval, use that instead") fs.MarkDeprecated("k8s-in-cluster", "no longer used") + fs.MarkDeprecated("git-verify-signatures", "changed to --git-verify-signatures-mode, use that instead") var kubeConfig *string { @@ -308,6 +310,19 @@ func main() { *automationInterval = *registryPollInterval } + // Maintain backwards compatibility with the --git-verify-signatures + // flag, but only if the --git-verify-signature-mode is not set to a + // custom (non default) value. + var gitVerifySignaturesMode fluxsync.VerifySignaturesMode + if *gitVerifySignatures && !fs.Changed("git-verify-signatures-mode") { + gitVerifySignaturesMode = fluxsync.VerifySignaturesModeAll + } else { + if gitVerifySignaturesMode, err = fluxsync.ToVerifySignaturesMode(*gitVerifySignaturesModeStr); err != nil { + logger.Log("error", err.Error()) + os.Exit(1) + } + } + // Sort out values for the git tag and notes ref. There are // running deployments that assume the defaults as given, so don't // mess with those unless explicitly told. @@ -345,7 +360,7 @@ func main() { // Import GPG keys, if we've been told where to look for them for _, p := range *gitImportGPG { - keyfiles, err := gpg.ImportKeys(p, *gitVerifySignatures) + keyfiles, err := gpg.ImportKeys(p, gitVerifySignaturesMode != fluxsync.VerifySignaturesModeNone) if err != nil { logger.Log("error", fmt.Sprintf("failed to import GPG key(s) from %s", p), "err", err.Error()) } @@ -655,7 +670,7 @@ func main() { "user", *gitUser, "email", *gitEmail, "signing-key", *gitSigningKey, - "verify-signatures", *gitVerifySignatures, + "verify-signatures-mode", gitVerifySignaturesMode, "sync-tag", *gitSyncTag, "state", *syncState, "readonly", *gitReadonly, @@ -694,7 +709,7 @@ func main() { repo, *gitSyncTag, *gitSigningKey, - *gitVerifySignatures, + gitVerifySignaturesMode, gitConfig, ) if err != nil { @@ -721,13 +736,13 @@ func main() { ManifestGenerationEnabled: *manifestGeneration, GitSecretEnabled: *gitSecret, LoopVars: &daemon.LoopVars{ - SyncInterval: *syncInterval, - SyncTimeout: *syncTimeout, - SyncState: syncProvider, - AutomationInterval: *automationInterval, - GitTimeout: *gitTimeout, - GitVerifySignatures: *gitVerifySignatures, - ImageScanDisabled: *registryDisableScanning, + SyncInterval: *syncInterval, + SyncTimeout: *syncTimeout, + SyncState: syncProvider, + AutomationInterval: *automationInterval, + GitTimeout: *gitTimeout, + GitVerifySignaturesMode: gitVerifySignaturesMode, + ImageScanDisabled: *registryDisableScanning, }, } diff --git a/docs/references/git-gpg.md b/docs/references/git-gpg.md index 8015035eea..bf92f9b284 100644 --- a/docs/references/git-gpg.md +++ b/docs/references/git-gpg.md @@ -240,3 +240,32 @@ b. Sign the sync tag by yourself, with a key that is imported, to point $ git tag --force --local-user= -a -m "Sync pointer" $ git push --force origin ``` + +### Choosing a `--git-verify-signatures-mode` + +#### `"none"` (default) + +By default, Flux skips GPG verification of all commits. + +#### `"all"` + +This is the regular verification behavior, consistent with the original +`--gitVerifySignatures` flag. It will perform GPG verification on every commit +between the tip of the Flux branch and the Flux sync tag, including all parents. +If your `master` branch contains only signed commits ([a flow which GitHub +supports][github-required-gpg]), then this flow ought to work. + +#### `"first-parent"` + +However, there are some arguments for more limited signing behaviors, e.g. [this +parable][gpg-dontsign-horror] and [this thread][gpg-dontsign-linus]). In +particular, it can be useful to allow unsigned commits into `master`, and to +point Flux at a `release` branch containing signed merges from `master`. A merge +commit has two parents: the previous commit "in the branch," as well as the last +commit in the merged branch. In this scenario, use the `"first-parent"` mode -- +only the merge commits "in the branch" should be GPG-verified, since the commits +from master have no signature. + +[github-required-gpg]:https://help.github.com/en/github/administering-a-repository/about-required-commit-signing +[gpg-dontsign-horror]:https://mikegerwitz.com/2012/05/a-git-horror-story-repository-integrity-with-signed-commits +[gpg-dontsign-linus]:http://git.661346.n2.nabble.com/GPG-signing-for-git-commit-tp2582986p2583316.html \ No newline at end of file diff --git a/go.mod b/go.mod index 94d38979f4..c7b4cf83e3 100644 --- a/go.mod +++ b/go.mod @@ -29,6 +29,7 @@ replace github.com/fluxcd/flux/pkg/install => ./pkg/install require ( github.com/Jeffail/gabs v1.4.0 + github.com/Masterminds/semver v1.5.0 // indirect github.com/Masterminds/semver/v3 v3.0.3 github.com/Masterminds/sprig v2.22.0+incompatible // indirect github.com/VividCortex/gohistogram v1.0.0 // indirect diff --git a/go.sum b/go.sum index 0ee83702ed..0b0089c599 100644 --- a/go.sum +++ b/go.sum @@ -63,6 +63,8 @@ github.com/Masterminds/goutils v1.1.0 h1:zukEsf/1JZwCMgHiK3GZftabmxiCw4apj3a28RP github.com/Masterminds/goutils v1.1.0/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= github.com/Masterminds/semver v1.4.2 h1:WBLTQ37jOCzSLtXNdoo8bNM8876KhNqOKvrlGITgsTc= github.com/Masterminds/semver v1.4.2/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= +github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= +github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/Masterminds/semver/v3 v3.0.3 h1:znjIyLfpXEDQjOIEWh+ehwpTU14UzUPub3c3sm36u14= github.com/Masterminds/semver/v3 v3.0.3/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Masterminds/sprig v2.22.0+incompatible h1:z4yfnGrZ7netVz+0EDJ0Wi+5VZCSYp4Z0m2dk6cEM60= diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index a3b57d59d7..fc322902d8 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -249,7 +249,7 @@ func (d *Daemon) makeJobFromUpdate(update updateFunc) jobFunc { var result job.Result err := d.WithWorkingClone(ctx, func(working *git.Checkout) error { var err error - if err = verifyWorkingRepo(ctx, d.Repo, working, d.SyncState); d.GitVerifySignatures && err != nil { + if err = verifyWorkingRepo(ctx, d.Repo, working, d.SyncState, d.GitVerifySignaturesMode); d.GitVerifySignaturesMode != sync.VerifySignaturesModeNone && err != nil { return err } result, err = update(ctx, jobID, working, logger) @@ -373,9 +373,9 @@ func (d *Daemon) sync() jobFunc { if err != nil { return result, err } - if d.GitVerifySignatures { + if d.GitVerifySignaturesMode != sync.VerifySignaturesModeNone { var latestValidRev string - if latestValidRev, _, err = latestValidRevision(ctx, d.Repo, d.SyncState); err != nil { + if latestValidRev, _, err = latestValidRevision(ctx, d.Repo, d.SyncState, d.GitVerifySignaturesMode); err != nil { return result, err } else if head != latestValidRev { result.Revision = latestValidRev @@ -566,7 +566,7 @@ func (d *Daemon) JobStatus(ctx context.Context, jobID job.ID) (job.Status, error if err != nil { return status, errors.Wrap(err, "enumerating commit notes") } - commits, err := d.Repo.CommitsBefore(ctx, "HEAD", d.GitConfig.Paths...) + commits, err := d.Repo.CommitsBefore(ctx, "HEAD", false, d.GitConfig.Paths...) if err != nil { return status, errors.Wrap(err, "checking revisions for status") } @@ -602,7 +602,7 @@ func (d *Daemon) SyncStatus(ctx context.Context, commitRef string) ([]string, er return nil, err } - commits, err := d.Repo.CommitsBetween(ctx, syncMarkerRevision, commitRef, d.GitConfig.Paths...) + commits, err := d.Repo.CommitsBetween(ctx, syncMarkerRevision, commitRef, false, d.GitConfig.Paths...) if err != nil { return nil, err } @@ -880,7 +880,7 @@ func policyEventTypes(u resource.PolicyUpdate) []string { // In case the signature of the tag can not be verified, or it points // towards a revision we can not get a commit range for, it returns an // error. -func latestValidRevision(ctx context.Context, repo *git.Repo, syncState sync.State) (string, git.Commit, error) { +func latestValidRevision(ctx context.Context, repo *git.Repo, syncState sync.State, gitVerifySignaturesMode sync.VerifySignaturesMode) (string, git.Commit, error) { var invalidCommit = git.Commit{} newRevision, err := repo.BranchHead(ctx) if err != nil { @@ -893,15 +893,17 @@ func latestValidRevision(ctx context.Context, repo *git.Repo, syncState sync.Sta return "", invalidCommit, err } + var gitFirstParent = gitVerifySignaturesMode == sync.VerifySignaturesModeFirstParent + var commits []git.Commit if tagRevision == "" { - commits, err = repo.CommitsBefore(ctx, newRevision) + commits, err = repo.CommitsBefore(ctx, newRevision, gitFirstParent) } else { // Assure the commit _at_ the high water mark is a signed and valid commit if err = repo.VerifyCommit(ctx, tagRevision); err != nil { return "", invalidCommit, errors.Wrap(err, "failed to verify signature of last sync'ed revision") } - commits, err = repo.CommitsBetween(ctx, tagRevision, newRevision) + commits, err = repo.CommitsBetween(ctx, tagRevision, newRevision, gitFirstParent) } if err != nil { @@ -925,8 +927,8 @@ func latestValidRevision(ctx context.Context, repo *git.Repo, syncState sync.Sta } // verifyWorkingRepo checks that a working clone is safe to be used for a write operation -func verifyWorkingRepo(ctx context.Context, repo *git.Repo, working *git.Checkout, syncState sync.State) error { - if latestVerifiedRev, _, err := latestValidRevision(ctx, repo, syncState); err != nil { +func verifyWorkingRepo(ctx context.Context, repo *git.Repo, working *git.Checkout, syncState sync.State, gitVerifySignaturesMode sync.VerifySignaturesMode) error { + if latestVerifiedRev, _, err := latestValidRevision(ctx, repo, syncState, gitVerifySignaturesMode); err != nil { return err } else if headRev, err := working.HeadRevision(ctx); err != nil { return err diff --git a/pkg/daemon/daemon_test.go b/pkg/daemon/daemon_test.go index c7891877b9..7bc9d00534 100644 --- a/pkg/daemon/daemon_test.go +++ b/pkg/daemon/daemon_test.go @@ -727,7 +727,7 @@ func mockDaemon(t *testing.T) (*Daemon, func(), func(), *mock.Mock, *mockEventWr manifests := kubernetes.NewManifests(kubernetes.ConstNamespacer("default"), log.NewLogfmtLogger(os.Stdout)) - gitSync, _ := fluxsync.NewGitTagSyncProvider(repo, syncTag, "", false, params) + gitSync, _ := fluxsync.NewGitTagSyncProvider(repo, syncTag, "", fluxsync.VerifySignaturesModeNone, params) // Finally, the daemon d := &Daemon{ @@ -741,7 +741,7 @@ func mockDaemon(t *testing.T) (*Daemon, func(), func(), *mock.Mock, *mockEventWr JobStatusCache: &job.StatusCache{Size: 100}, EventWriter: events, Logger: logger, - LoopVars: &LoopVars{SyncTimeout: timeout, GitTimeout: timeout, SyncState: gitSync}, + LoopVars: &LoopVars{SyncTimeout: timeout, GitTimeout: timeout, SyncState: gitSync, GitVerifySignaturesMode: fluxsync.VerifySignaturesModeNone}, } start := func() { diff --git a/pkg/daemon/loop.go b/pkg/daemon/loop.go index 3f4a80f175..ad4f5a26b4 100644 --- a/pkg/daemon/loop.go +++ b/pkg/daemon/loop.go @@ -14,13 +14,13 @@ import ( ) type LoopVars struct { - SyncInterval time.Duration - SyncTimeout time.Duration - AutomationInterval time.Duration - GitTimeout time.Duration - GitVerifySignatures bool - SyncState fluxsync.State - ImageScanDisabled bool + SyncInterval time.Duration + SyncTimeout time.Duration + AutomationInterval time.Duration + GitTimeout time.Duration + GitVerifySignaturesMode fluxsync.VerifySignaturesMode + SyncState fluxsync.State + ImageScanDisabled bool initOnce sync.Once syncSoon chan struct{} @@ -115,8 +115,8 @@ func (d *Daemon) Loop(stop chan struct{}, wg *sync.WaitGroup, logger log.Logger) var err error ctx, cancel := context.WithTimeout(context.Background(), d.GitTimeout) - if d.GitVerifySignatures { - newSyncHead, invalidCommit, err = latestValidRevision(ctx, d.Repo, d.SyncState) + if d.GitVerifySignaturesMode != fluxsync.VerifySignaturesModeNone { + newSyncHead, invalidCommit, err = latestValidRevision(ctx, d.Repo, d.SyncState, d.GitVerifySignaturesMode) } else { newSyncHead, err = d.Repo.BranchHead(ctx) } diff --git a/pkg/daemon/sync.go b/pkg/daemon/sync.go index 8196cb98a7..597fc8a8a7 100644 --- a/pkg/daemon/sync.go +++ b/pkg/daemon/sync.go @@ -141,10 +141,10 @@ func getChangeSet(ctx context.Context, state revisionRatchet, headRev string, re ctx, cancel := context.WithTimeout(ctx, timeout) if c.oldTagRev != "" { - c.commits, err = repo.CommitsBetween(ctx, c.oldTagRev, c.newTagRev, paths...) + c.commits, err = repo.CommitsBetween(ctx, c.oldTagRev, c.newTagRev, false, paths...) } else { c.initialSync = true - c.commits, err = repo.CommitsBefore(ctx, c.newTagRev, paths...) + c.commits, err = repo.CommitsBefore(ctx, c.newTagRev, false, paths...) } cancel() diff --git a/pkg/daemon/sync_test.go b/pkg/daemon/sync_test.go index a2efefca4f..7ac79071bf 100644 --- a/pkg/daemon/sync_test.go +++ b/pkg/daemon/sync_test.go @@ -155,7 +155,7 @@ func TestPullAndSync_InitialSync(t *testing.T) { } syncTag := "sync" - gitSync, _ := fluxsync.NewGitTagSyncProvider(d.Repo, syncTag, "", false, d.GitConfig) + gitSync, _ := fluxsync.NewGitTagSyncProvider(d.Repo, syncTag, "", fluxsync.VerifySignaturesModeNone, d.GitConfig) syncState := &lastKnownSyncState{logger: d.Logger, state: gitSync} if err := d.Sync(ctx, time.Now().UTC(), head, syncState); err != nil { @@ -188,7 +188,7 @@ func TestPullAndSync_InitialSync(t *testing.T) { // It creates the tag at HEAD if err := d.Repo.Refresh(context.Background()); err != nil { t.Errorf("pulling sync tag: %v", err) - } else if revs, err := d.Repo.CommitsBefore(context.Background(), syncTag); err != nil { + } else if revs, err := d.Repo.CommitsBefore(context.Background(), syncTag, false); err != nil { t.Errorf("finding revisions before sync tag: %v", err) } else if len(revs) <= 0 { t.Errorf("Found no revisions before the sync tag") @@ -243,7 +243,7 @@ func TestDoSync_NoNewCommits(t *testing.T) { t.Fatal(err) } - gitSync, _ := fluxsync.NewGitTagSyncProvider(d.Repo, syncTag, "", false, d.GitConfig) + gitSync, _ := fluxsync.NewGitTagSyncProvider(d.Repo, syncTag, "", fluxsync.VerifySignaturesModeNone, d.GitConfig) syncState := &lastKnownSyncState{logger: d.Logger, state: gitSync} if err := d.Sync(ctx, time.Now().UTC(), head, syncState); err != nil { @@ -266,12 +266,12 @@ func TestDoSync_NoNewCommits(t *testing.T) { } // It doesn't move the tag - oldRevs, err := d.Repo.CommitsBefore(ctx, syncTag) + oldRevs, err := d.Repo.CommitsBefore(ctx, syncTag, false) if err != nil { t.Fatal(err) } - if revs, err := d.Repo.CommitsBefore(ctx, syncTag); err != nil { + if revs, err := d.Repo.CommitsBefore(ctx, syncTag, false); err != nil { t.Errorf("finding revisions before sync tag: %v", err) } else if !reflect.DeepEqual(revs, oldRevs) { t.Errorf("Should have kept the sync tag at HEAD") @@ -362,7 +362,7 @@ func TestDoSync_WithNewCommit(t *testing.T) { t.Fatal(err) } - gitSync, _ := fluxsync.NewGitTagSyncProvider(d.Repo, syncTag, "", false, d.GitConfig) + gitSync, _ := fluxsync.NewGitTagSyncProvider(d.Repo, syncTag, "", fluxsync.VerifySignaturesModeNone, d.GitConfig) syncState := &lastKnownSyncState{logger: d.Logger, state: gitSync} if err := d.Sync(ctx, time.Now().UTC(), head, syncState); err != nil { @@ -397,7 +397,7 @@ func TestDoSync_WithNewCommit(t *testing.T) { defer cancel() if err := d.Repo.Refresh(ctx); err != nil { t.Errorf("pulling sync tag: %v", err) - } else if revs, err := d.Repo.CommitsBetween(ctx, oldRevision, syncTag); err != nil { + } else if revs, err := d.Repo.CommitsBetween(ctx, oldRevision, syncTag, false); err != nil { t.Errorf("finding revisions before sync tag: %v", err) } else if len(revs) <= 0 { t.Errorf("Should have moved sync tag forward") @@ -426,7 +426,7 @@ func TestDoSync_WithErrors(t *testing.T) { } syncTag := "sync" - gitSync, _ := fluxsync.NewGitTagSyncProvider(d.Repo, syncTag, "", false, d.GitConfig) + gitSync, _ := fluxsync.NewGitTagSyncProvider(d.Repo, syncTag, "", fluxsync.VerifySignaturesModeNone, d.GitConfig) syncState := &lastKnownSyncState{logger: d.Logger, state: gitSync} if err := d.Sync(ctx, time.Now().UTC(), head, syncState); err != nil { diff --git a/pkg/git/gittest/repo_test.go b/pkg/git/gittest/repo_test.go index 50b9c92e7a..94d83fa80f 100644 --- a/pkg/git/gittest/repo_test.go +++ b/pkg/git/gittest/repo_test.go @@ -49,7 +49,7 @@ func TestCommit(t *testing.T) { t.Error(err) } - commits, err := repo.CommitsBefore(ctx, "HEAD") + commits, err := repo.CommitsBefore(ctx, "HEAD", false) if err != nil { t.Fatal(err) @@ -105,7 +105,7 @@ func TestSignedCommit(t *testing.T) { t.Error(err) } - commits, err := repo.CommitsBefore(ctx, "HEAD") + commits, err := repo.CommitsBefore(ctx, "HEAD", false) if err != nil { t.Fatal(err) diff --git a/pkg/git/operations.go b/pkg/git/operations.go index 06642a4bcd..78f2988326 100644 --- a/pkg/git/operations.go +++ b/pkg/git/operations.go @@ -251,10 +251,16 @@ func refRevision(ctx context.Context, workingDir, ref string) (string, error) { } // Return the revisions and one-line log commit messages -func onelinelog(ctx context.Context, workingDir, refspec string, subdirs []string) ([]Commit, error) { +func onelinelog(ctx context.Context, workingDir, refspec string, subdirs []string, firstParent bool) ([]Commit, error) { out := &bytes.Buffer{} - args := []string{"log", "--pretty=format:%GK|%G?|%H|%s", refspec} - args = append(args, "--") + args := []string{"log", "--pretty=format:%GK|%G?|%H|%s"} + + if firstParent { + args = append(args, "--first-parent") + } + + args = append(args, refspec, "--") + if len(subdirs) > 0 { args = append(args, subdirs...) } diff --git a/pkg/git/operations_test.go b/pkg/git/operations_test.go index 5b5e7d2e7d..e73e436de8 100644 --- a/pkg/git/operations_test.go +++ b/pkg/git/operations_test.go @@ -192,7 +192,7 @@ func TestOnelinelog_NoGitpath(t *testing.T) { t.Fatal(err) } - commits, err := onelinelog(context.Background(), newDir, "HEAD~2..HEAD", nil) + commits, err := onelinelog(context.Background(), newDir, "HEAD~2..HEAD", nil, false) if err != nil { t.Fatal(err) } @@ -202,6 +202,52 @@ func TestOnelinelog_NoGitpath(t *testing.T) { } } +func TestOnelinelog_NoGitpath_Merged(t *testing.T) { + newDir, cleanup := testfiles.TempDir(t) + defer cleanup() + + subdirs := []string{"dev", "prod"} + err := createRepo(newDir, subdirs) + if err != nil { + t.Fatal(err) + } + + branch := "tmp" + if err = execCommand("git", "-C", newDir, "checkout", "-b", branch); err != nil { + t.Fatal(err) + } + if err = updateDirAndCommit(newDir, "dev", testfiles.FilesUpdated); err != nil { + t.Fatal(err) + } + if err = updateDirAndCommit(newDir, "prod", testfiles.FilesUpdated); err != nil { + t.Fatal(err) + } + if err = execCommand("git", "-C", newDir, "checkout", "master"); err != nil { + t.Fatal(err) + } + if err = execCommand("git", "-C", newDir, "merge", "--no-ff", branch); err != nil { + t.Fatal(err) + } + + commits, err := onelinelog(context.Background(), newDir, "HEAD", nil, false) + if err != nil { + t.Fatal(err) + } + + if len(commits) != 6 { + t.Fatal(commits) + } + + commits, err = onelinelog(context.Background(), newDir, "HEAD", nil, true) + if err != nil { + t.Fatal(err) + } + + if len(commits) != 4 { + t.Fatal(commits) + } +} + func TestOnelinelog_WithGitpath(t *testing.T) { newDir, cleanup := testfiles.TempDir(t) defer cleanup() @@ -220,7 +266,7 @@ func TestOnelinelog_WithGitpath(t *testing.T) { t.Fatal(err) } - commits, err := onelinelog(context.Background(), newDir, "HEAD~2..HEAD", []string{"dev"}) + commits, err := onelinelog(context.Background(), newDir, "HEAD~2..HEAD", []string{"dev"}, false) if err != nil { t.Fatal(err) } @@ -230,6 +276,54 @@ func TestOnelinelog_WithGitpath(t *testing.T) { } } +func TestOnelinelog_WithGitpath_Merged(t *testing.T) { + newDir, cleanup := testfiles.TempDir(t) + defer cleanup() + + subdirs := []string{"dev", "prod"} + err := createRepo(newDir, subdirs) + if err != nil { + t.Fatal(err) + } + + branch := "tmp" + if err = execCommand("git", "-C", newDir, "checkout", "-b", branch); err != nil { + t.Fatal(err) + } + if err = updateDirAndCommit(newDir, "dev", testfiles.FilesUpdated); err != nil { + t.Fatal(err) + } + if err = updateDirAndCommit(newDir, "prod", testfiles.FilesUpdated); err != nil { + t.Fatal(err) + } + if err = execCommand("git", "-C", newDir, "checkout", "master"); err != nil { + t.Fatal(err) + } + if err = execCommand("git", "-C", newDir, "merge", "--no-ff", branch); err != nil { + t.Fatal(err) + } + + // show the 2 update commits as well as init commit + commits, err := onelinelog(context.Background(), newDir, "HEAD", []string{"prod"}, false) + if err != nil { + t.Fatal(err) + } + + if len(commits) != 2 { + t.Fatal(err) + } + + // show the merge commit as well as init commit + commits, err = onelinelog(context.Background(), newDir, "HEAD", []string{"prod"}, true) + if err != nil { + t.Fatal(err) + } + + if len(commits) != 2 { + t.Fatal(err) + } +} + func TestCheckPush(t *testing.T) { upstreamDir, upstreamCleanup := testfiles.TempDir(t) defer upstreamCleanup() diff --git a/pkg/git/repo.go b/pkg/git/repo.go index 5098c23615..ccb5f2fc80 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -230,22 +230,22 @@ func (r *Repo) BranchHead(ctx context.Context) (string, error) { return refRevision(ctx, r.dir, "heads/"+r.branch) } -func (r *Repo) CommitsBefore(ctx context.Context, ref string, paths ...string) ([]Commit, error) { +func (r *Repo) CommitsBefore(ctx context.Context, ref string, firstParent bool, paths ...string) ([]Commit, error) { r.mu.RLock() defer r.mu.RUnlock() if err := r.errorIfNotReady(); err != nil { return nil, err } - return onelinelog(ctx, r.dir, ref, paths) + return onelinelog(ctx, r.dir, ref, paths, firstParent) } -func (r *Repo) CommitsBetween(ctx context.Context, ref1, ref2 string, paths ...string) ([]Commit, error) { +func (r *Repo) CommitsBetween(ctx context.Context, ref1, ref2 string, firstParent bool, paths ...string) ([]Commit, error) { r.mu.RLock() defer r.mu.RUnlock() if err := r.errorIfNotReady(); err != nil { return nil, err } - return onelinelog(ctx, r.dir, ref1+".."+ref2, paths) + return onelinelog(ctx, r.dir, ref1+".."+ref2, paths, firstParent) } func (r *Repo) VerifyTag(ctx context.Context, tag string) (string, error) { diff --git a/pkg/sync/git.go b/pkg/sync/git.go index 217e55f951..ac06c981bd 100644 --- a/pkg/sync/git.go +++ b/pkg/sync/git.go @@ -9,11 +9,11 @@ import ( // GitTagSyncProvider is the mechanism by which a Git tag is used to keep track of the current point fluxd has synced to. type GitTagSyncProvider struct { - repo *git.Repo - syncTag string - signingKey string - verifyTag bool - config git.Config + repo *git.Repo + syncTag string + signingKey string + verifyTagMode VerifySignaturesMode + config git.Config } // NewGitTagSyncProvider creates a new git tag sync provider. @@ -21,15 +21,15 @@ func NewGitTagSyncProvider( repo *git.Repo, syncTag string, signingKey string, - verifyTag bool, + verifyTagMode VerifySignaturesMode, config git.Config, ) (GitTagSyncProvider, error) { return GitTagSyncProvider{ - repo: repo, - syncTag: syncTag, - signingKey: signingKey, - verifyTag: verifyTag, - config: config, + repo: repo, + syncTag: syncTag, + signingKey: signingKey, + verifyTagMode: verifyTagMode, + config: config, }, nil } @@ -47,7 +47,7 @@ func (p GitTagSyncProvider) GetRevision(ctx context.Context) (string, error) { return "", err } - if p.verifyTag { + if p.verifyTagMode != VerifySignaturesModeNone { if _, err := p.repo.VerifyTag(ctx, p.syncTag); err != nil { // if the revision wasn't found, don't treat this as an // error -- but don't supply a revision, either. diff --git a/pkg/sync/provider.go b/pkg/sync/provider.go index 6c87798b06..0d4ddcc9a4 100644 --- a/pkg/sync/provider.go +++ b/pkg/sync/provider.go @@ -2,6 +2,7 @@ package sync import ( "context" + "fmt" ) const ( @@ -12,6 +13,40 @@ const ( NativeStateMode = "secret" ) +// VerifySignaturesMode represents the strategy to use when choosing which commits to GPG-verify between the flux sync tag and the tip of the flux branch +type VerifySignaturesMode string + +const ( + // VerifySignaturesModeDefault - get the default behavior when casting + VerifySignaturesModeDefault = "" + + // VerifySignaturesModeNone (default) - don't verify any commits + VerifySignaturesModeNone = "none" + + // VerifySignaturesModeAll - consider all possible commits + VerifySignaturesModeAll = "all" + + // VerifySignaturesModeFirstParent - consider only commits on the chain of + // first parents (i.e. don't consider commits merged from another branch) + VerifySignaturesModeFirstParent = "first-parent" +) + +// ToVerifySignaturesMode converts a string to a VerifySignaturesMode +func ToVerifySignaturesMode(s string) (VerifySignaturesMode, error) { + switch s { + case VerifySignaturesModeDefault: + return VerifySignaturesModeNone, nil + case VerifySignaturesModeNone: + return VerifySignaturesModeNone, nil + case VerifySignaturesModeAll: + return VerifySignaturesModeAll, nil + case VerifySignaturesModeFirstParent: + return VerifySignaturesModeFirstParent, nil + default: + return VerifySignaturesModeNone, fmt.Errorf("'%s' is not a valid git-verify-signatures-mode", s) + } +} + type State interface { // GetRevision fetches the recorded revision, returning an empty // string if none has been recorded yet.