Skip to content

Add … push --sign-by-sigstore#17088

Merged
openshift-merge-robot merged 1 commit intocontainers:mainfrom
mtrmac:sign-by-sigstore
Jan 27, 2023
Merged

Add … push --sign-by-sigstore#17088
openshift-merge-robot merged 1 commit intocontainers:mainfrom
mtrmac:sign-by-sigstore

Conversation

@mtrmac
Copy link
Copy Markdown
Contributor

@mtrmac mtrmac commented Jan 11, 2023 •
edited
Loading

podman push and podman manifest push sync now support --sign-by-sigstore=param-file,
using the containers-sigstore-signing-params.yaml(5) file format.

That notably adds support for Fulcio and Rekor signing.

Depends on unmerged containers/image#1787 ; see that PR for documentation of the YAML file format, as well as example files.

See also containers/common#1288 for more discussion about where the interactive prompting pieces should be.

Untested so far.

Does this PR introduce a user-facing change?

`podman push` and `podman manifest push` now support a `--sign-by-sigstore` option, which allows using Fulcio and Rekor.

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 11, 2023
mtrmac
mtrmac commented Jan 11, 2023
View reviewed changes
@mtrmac
Copy link
Copy Markdown
Contributor Author

mtrmac commented Jan 12, 2023

Reworked, together with the c/common part; now the top-level CLI fully creates a signer.Signer, handling all the interactivity upfront.

This is now ready for review, apart from relying on unmerged dependencies.

@mtrmac mtrmac changed the title Early review: UNTESTED: --sign-by-sigstore Add … push --sign-by-sigstore Jan 12, 2023
@mtrmac mtrmac force-pushed the sign-by-sigstore branch 3 times, most recently from a49e237 to c4562e1 Compare January 12, 2023 21:06
vrothberg
vrothberg reviewed Jan 13, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@mtrmac mtrmac force-pushed the sign-by-sigstore branch 3 times, most recently from 86e2296 to 4d666c3 Compare January 13, 2023 17:55
TomSweeneyRedHat
TomSweeneyRedHat reviewed Jan 13, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

LGTM
A blog would be most welcomed for this functionality once it goes live.

@mtrmac mtrmac mentioned this pull request Jan 14, 2023
Merged
@mtrmac mtrmac force-pushed the sign-by-sigstore branch 7 times, most recently from 0b158c4 to cf20b3b Compare January 17, 2023 21:25
@mtrmac
Copy link
Copy Markdown
Contributor Author

mtrmac commented Jan 17, 2023

Now ready for review and possible merging.

@mtrmac mtrmac marked this pull request as ready for review January 17, 2023 21:26
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 17, 2023
@vrothberg vrothberg added the bloat_approved Approve a PR in which binary file size grows by over 50k label Jan 18, 2023
vrothberg
vrothberg approved these changes Jan 18, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 18, 2023
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Jan 18, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mtrmac, vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2023
@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jan 18, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2023
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 19, 2023
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 26, 2023
This was referenced Jan 26, 2023
Closed
Merged
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 26, 2023
@mtrmac
Add (podman {image,manifest} push --sign-by-sigstore=param-file.yaml)
069edc3 
(podman push) and (podman manifest push) now support --sign-by-sigstore=param-file,
using the containers-sigstore-signing-params.yaml(5) file format.

That notably adds support for Fulcio and Rekor signing.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac
Copy link
Copy Markdown
Contributor Author

mtrmac commented Jan 27, 2023

Rebased, tests pass. Please merge.

(The changes since the last approved version are in go.mod, and an update in test mechanisms; no changes in non-test code.)

ashley-cui
ashley-cui reviewed Jan 27, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ashley-cui ashley-cui left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 27, 2023
@mtrmac
Copy link
Copy Markdown
Contributor Author

mtrmac commented Jan 27, 2023

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2023
@openshift-merge-robot openshift-merge-robot merged commit 1401249 into containers:main Jan 27, 2023
@mtrmac mtrmac deleted the sign-by-sigstore branch January 27, 2023 19:05
@edsantiago edsantiago mentioned this pull request Jan 30, 2023
Open
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 13, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@ashley-cui ashley-cui ashley-cui left review comments

+1 more reviewer

@vrothberg vrothberg vrothberg approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rhatdan rhatdan

@vrothberg vrothberg

@ashley-cui ashley-cui

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bloat_approved Approve a PR in which binary file size grows by over 50k lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mtrmac @rhatdan @vrothberg @TomSweeneyRedHat @ashley-cui @openshift-merge-robot