Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Support multiple signature verification strategies with --gitVerifySignaturesMode #2803

Merged
merged 1 commit into from
Feb 19, 2020
Merged

Support multiple signature verification strategies with --gitVerifySignaturesMode #2803

merged 1 commit into from
Feb 19, 2020

Conversation

jstevans
Copy link
Contributor

Add a new flag that puts the --gitVerifySignatures behavior into one
of two modes:

* "all" (default) uses the existing `--gitVerifySignatures` behavior

* "first-parent" uses the behavior described in issue #2704, wherein
    only each first-parent of each commit from the tip of the flux
    branch are considered when verifying GPG signatures

Fixes issue #2704

@jstevans jstevans requested a review from hiddeco January 30, 2020 19:35
@hiddeco hiddeco self-assigned this Feb 3, 2020
hiddeco
hiddeco reviewed Feb 4, 2020
View reviewed changes
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @jstevans 🥇

Couple of small comments about (minor) implementation details, PTAL whenever you have time.

cmd/fluxd/main.go Outdated Show resolved Hide resolved
pkg/install/generated_templates.gogen.go Outdated Show resolved Hide resolved
docs/references/git-gpg.md Outdated Show resolved Hide resolved
hiddeco
hiddeco reviewed Feb 5, 2020
View reviewed changes
cmd/fluxd/main.go Outdated Show resolved Hide resolved
cmd/fluxd/main.go Show resolved Hide resolved
pkg/sync/provider.go Outdated Show resolved Hide resolved
@hiddeco hiddeco added this to the 1.18.0 milestone Feb 5, 2020
@hiddeco hiddeco changed the title Add --gitVerifySignaturesMode (#2704) Add --gitVerifySignaturesMode Feb 5, 2020
@jstevans
Copy link
Contributor Author

jstevans commented Feb 5, 2020

@hiddeco I'm having trouble what went wrong in the CI build -- I've read over the contributing docs, but I don't think I understand how to investigate this issue. Can you give some guidance?

@2opremio 2opremio modified the milestones: 1.18.0, 1.19.0 Feb 6, 2020
hiddeco
hiddeco reviewed Feb 10, 2020
View reviewed changes
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the last round of minor comments. Thanks for bearing with me 🌷

Can you please rebase on top of master and smash your commits together? The former to ensure no surprises happen when it gets merged in to master, and the latter to keep our commit history there clean. Thank you!

cmd/fluxd/main.go Outdated Show resolved Hide resolved
pkg/daemon/daemon_test.go Outdated Show resolved Hide resolved
pkg/sync/provider.go Outdated Show resolved Hide resolved
cmd/fluxd/main.go Outdated Show resolved Hide resolved
@jstevans
Copy link
Contributor Author

@hiddeco done!

@hiddeco
Copy link
Member

hiddeco commented Feb 11, 2020

One additional request I forgot to mention, can you please remove references to PR from the commit message and the description?

The GitHub feature is amazing for most use-cases but it causes endless pings (including from forks) to the issues whenever the commit is in some way touched and seen as 'new'.

@jstevans
Copy link
Contributor Author

@hiddeco Done! Is there anywhere I should add a reference to the issue?

@hiddeco hiddeco changed the title Add --gitVerifySignaturesMode Support different git signature verification strategies with --gitVerifySignaturesMode Feb 14, 2020
@hiddeco hiddeco changed the title Support different git signature verification strategies with --gitVerifySignaturesMode Support multiple signature verification strategies with --gitVerifySignaturesMode Feb 14, 2020
@hiddeco
Copy link
Member

hiddeco commented Feb 14, 2020

@jstevans PR message itself so that the PR is linked to the issue (which you have already done). So all good now.

@jstevans jstevans force-pushed the gitVerifySignaturesMode branch 2 times, most recently from 5670a42 to 9ef7d65 Compare February 18, 2020 23:50
@jstevans
Support multiple git verification strategies
42d6f18 
Add a new flag that puts the `--gitVerifySignatures` behavior into one
of two modes:

    * "all" (default) uses the existing `--gitVerifySignatures` behavior

    * "first-parent" uses an alternative behavior, wherein only each
      first-parent of each commit from the tip of the flux branch are
      considered when verifying GPG signatures
@jstevans
Copy link
Contributor Author

GitHub PR flow takes some getting used to, I hope this is correct :)

hiddeco
hiddeco approved these changes Feb 19, 2020
View reviewed changes
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your patience with the reviews, and for the feature itself @jstevans. LGTM 🌻

@hiddeco hiddeco removed their assignment Feb 19, 2020
@hiddeco hiddeco merged commit 3e01223 into fluxcd:master Feb 19, 2020
kingdonb pushed a commit to fluxcd/helm-operator that referenced this pull request Aug 30, 2022
set firstParent to false in CommitsBetween
4d0666e 
the CommitsBetween method added a firstParent boolean here, in:

fluxcd/flux@42d6f18

fluxcd/flux#2803

If we are calling CommitsBetween here, expecting the old behavior, then
it should be set to "false"

Signed-off-by: Kingdon Barrett <kingdon@weave.works>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hiddeco hiddeco hiddeco approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
1.19.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jstevans @hiddeco @2opremio