--sign-by-sigstore infrastructure#1288
Conversation
vrothberg
left a comment
There was a problem hiding this comment.
LGTM
A sad fact of life is +135,121
172d466 to
a28fc17
Compare
Updated that way. Now we are only +17,106 in c/common — and 15,670 of that is |
--sign-by-sigstore infrastructure--sign-by-sigstore infrastructure
|
The interactivity needs to be done in buildah as well. so I think common makes sense for this. |
|
The call stack is Podman CLI → Podman engine abstraction → the non-remote engine → c/common/libimage . That means pushing the interactivity down to c/common/libimage would require passing stdout/stdin through the Podman engine abstraction, which seems aesthetically ugly to me — admittedly the current approach of passing through a The worst case is that “interactivity” means “opening a web browser”, with ~no way to control that by parameters (right now). Doing that, without warning, so deep in the call stack really seems unexpected to me. Also, pushing the interactivity to the top allows creating a Doing the interactivity at the top level is, I think, not that big a deal, because most of the actual logic is provided by a c/image/cli/sigstore. In Podman, it’s 24 extra lines in https://github.com/containers/podman/pull/17088/files Moving that down here to c/common/libimage would mean that Podman wouldn’t need to do the cleanup state tracking, but it would need to pass down stdin/stdout (and in the future potentially a browser opener) all through the call stack. |
|
Right, we should to handle the interactivity in the front end if we can. The code for that could still live somewhere in c/common if there's a considerable amount of boilerplate. |
a28fc17 to
cd1bc49
Compare
|
This is now vendoring from c/image |
cd1bc49 to
fe08b03
Compare
|
containers/podman#17088 isn't passing tests. |
This allows using Fulcio and Rekor, without having to pass around 9 options; and the interactivity required for OIDC authentication is handled by the caller at some higher level (possibly only once for multiple operations). Signed-off-by: Miloslav Trmač <mitr@redhat.com>
fe08b03 to
0f1e996
Compare
|
The Podman PR is finally (mostly) passing tests. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mtrmac, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is infrastructure for Podman supporting something like containers/skopeo#1849 .
RFC . I’m not quite sure how to deal with the inevitable interactivity of Fulcio. Right now Fulcio interactivity is done here in c/common, and passphrase interactivity happens in Podman.
One consistent option would be to move all interactivity to Podman, i.e. to require Podman to provide a finished
c/image/signature/signer.Signer(and then, within Podman, would that interactivity be in the top-level CLI, or only in the local engine implementation?).Another consistent option would be to move all interactivity to c/common, i.e. to implement proper passphrase prompting down here in c/common.
I guess I weakly prefer Podman-side interactivity.
@vrothberg WDYT?