-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
--authfile command line argument for image sign command. #12270
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@edsantiago @vrothberg @mtrmac PTAL |
test/system/011-image.bats
Outdated
test -e "$dir/$sigfile" || die "Missing signature file '$sigfile'" | ||
|
||
# Confirm good signature | ||
GNUPGHOME=$_GNUPGHOME_TMP gpg --verify "$dir/$sigfile" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need this if you have the next line
LGTM once Ed's comment is addressed |
Added Requires: gnupg to fedora rawhide specfile. @lsm5, please cherry-pick that to whatever other branches might need it. @jnovy, please add that to RHEL specfile when appropriate. |
Added to CentOS Stream 9/RHEL9 GA spec @edsantiago Suggest to file a bugzilla for requests like these next time so that QE can test this to assure it's not omitted. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m probably missing something obvious here but this looks rather wrong at a first glance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think authfile is involved in image signing.
Isn’t it involved, if only to determine what image (and digest) to create the signature for?
The only way I can read #10866 is that the request truly is for --authfile
, with the usual --authfile
semantics. The report is not very explicit (in particular “Steps to reproduce the issue:” is downright pointless), but both the --authfile
wording and the mention of “the default auth file location” are a better fit for auth.json
than for the single signature file.
How does --sigfile
and --all
work together? AFAICS it doesn’t work usefully.
Adds the --authfile command line argument to allow users to use alternative authfile paths when signing images. Replaces: containers#10975 Fixes: containers#10866 Signed-off-by: José Guilherme Vanz <[email protected]> Signed-off-by: Daniel J Walsh <[email protected]>
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
A final head nod from @edsantiago / @mtrmac would be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable WRT c/image usage; I don’t know enough about how Podman generally deals with SystemContext to have an opinion on the exact data path (e.g. should this come from ir.Libpod.SystemContext()
? It seems that’s not how it works.)
Adds the --authfile command line argument to allow users to set the location of the auth file to use.
Replaces: #10975
Fixes: #10866
Signed-off-by: José Guilherme Vanz [email protected]
Signed-off-by: Daniel J Walsh [email protected]
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer: