podman image trust overhaul, incl. sigstore #15533
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We can always recover it from git, but it seems to serve no purpose anyway. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Split the existing code into policy.go and registries.go, depending on which files it concerns. Only moves unchanged code, should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Nothing uses it outside the package. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Only process the incoming args[] (which is a single-element array for some reason) once, and use a semantic variable name for the value we care about. Should not change behavior, the only caller already supposedly ensures that len(args) == 1. Signed-off-by: Miloslav Trmač <[email protected]>
This will allow us to write unit tests without setting up the complete Podman runtime (and without the Linux dependency). Also, actually add a basic smoke test of the core functionality. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
- Also reject public keys with types that don't use them - Reject unknown trust types - And add unit tests Signed-off-by: Miloslav Trmač <[email protected]>
That way, we don't have to switch over trustType twice. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
NOTE: This does not edit the use-sigstore-attachments value in registries.d, similarly to how (podman image trust set) didn't set the lookaside paths for simple signing. Signed-off-by: Miloslav Trmač <[email protected]>
This will allow us to write unit tests without setting up the complete Podman runtime (and without the Linux dependency). Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
We now have only a few entrypoints that are called externally, so make the rest private. This will make it more obvious that we are not breaking any external users. Signed-off-by: Miloslav Trmač <[email protected]>
Sort map keys instead of iterating in the Go-imposed random order. Signed-off-by: Miloslav Trmač <[email protected]>
Add at least a basic unit test for the various entry types. So that we don't have to actually deal with GPG keys and /usr/bin/gpg*, parametrize the code with a gpgIDReader , and pass a fake one in the unit test. Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
This will evetually allow us to use it for the default scope as well, which currently uses a simplified version. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Now that it is the primary return value of a small function, the long name only makes reading harder. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
Just so that we don't have a boolean-named function returning a struct. Also reorder the parameters to have the container first, and the lookup key second. Shoud not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
... instead of taking a shortcut, e.g. not listing any keys if they are required. Signed-off-by: Miloslav Trmač <[email protected]>
Do the registries.d lookup once, separately from building an entry, so that we can share it across entries. Also prepare a separate res to allow adding multiple entries. Signed-off-by: Miloslav Trmač <[email protected]>
…iple requirements Currently - the output uses the first entry's type, even if the requirements are different (notably signedBy + sigstoreSIgned) - all public keys IDs are collected to a single line, even if some of them are interchangeable, and some are required (e.g. two signedBy requirements could require an image to be signed by (redhatProd OR redhatBeta) AND (vendor1 OR vendor2) So, stop collapsing the requirements, and return a separate entry for each one. Multiple GPG IDs on a single line used to mean AND or OR, now they always mean AND. Signed-off-by: Miloslav Trmač <[email protected]>
sigstoreSigned does not have GPG IDs, so we add N/A in that column. NOTE: this does not show the use-sigstore-attachments value from registries.d. Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
... to go from top to bottom. Should not change behavior. Signed-off-by: Miloslav Trmač <[email protected]>
…set) We are unmarshaling and re-marshaling JSON, which can _silently_ drop data with the Go design decision.data. Try harder, by using json.RawMessage at least for the data we care about. Alternatively, this could use json.Decoder.DisallowUnknownFields. Signed-off-by: Miloslav Trmač <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mtrmac 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 |
openshift-ci
bot
added
the
approved
The apiv2 test hardcoded the tag of the alpine image. Remove it to unblock CI. Fixes: containers#15388 Signed-off-by: Valentin Rothberg <[email protected]>
|
Re-running https://api.cirrus-ci.com/v1/artifact/task/6204450858598400/html/sys-remote-fedora-36-rootless-host.log.html , it’s very likely to be unrelated and it seems to be tracked in #15367 . |
vrothberg
reviewed
Aug 30, 2022
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See release note. Currently this is an unmodified backport of #15466 .
Does this PR introduce a user-facing change?