-
Notifications
You must be signed in to change notification settings - Fork 2k
docker trust inspect #694
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
docker trust inspect #694
Conversation
|
Thanks. the output looks good to me |
| Args: cli.ExactArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| return inspectTrustInfo(dockerCli, args[0]) | ||
| }, |
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 believe all other inspect commands support multiple args for inspecting multiple objects at once, so they return a list instead of a single JSON object.
Other inspect commands also use a formatter in cli/command/formatter to support the --format flag.
I think it might be good to be consistent here.
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.
sounds good. I've added multiple args support. I'd like to hold off on the --format flag for now until we've stabilized the JSON format
|
|
||
|
|
||
| ```bash | ||
| $ docker trust inspect alpine:latest | jq |
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.
None of the other inspect command's docs show piping to jq
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.
Agreed: can you remove the | jq? We can keep the output as-is (it's documentation, so should be reasonable to output for readability - we may need to add a note / "conventions" section about that somewhere central)
5d88927 to
4aa8de2
Compare
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
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 🐸
cc @thaJeztah for docs
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.
Thanks! See my comments inline 😄
cli/command/trust/inspect.go
Outdated
| }, nil | ||
| } | ||
|
|
||
| // trustRepo represents consumable information about a trusted repository |
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.
nit: can you move these types to the top of this file? (looks like we do so in the other trust commands)
| "SignedTag": "latest", | ||
| "Digest": "d6bfc3baf615dc9618209a8d607ba2a8103d9c8a405b3bd8741d88b4bef36478", | ||
| "Signers": [ | ||
| "Repo Admin" |
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 wondering if more information about signers is available, or will be available/needed at some point; basically, I love the simplicity of a []string, but if we anticipate more information is needed at some point, perhaps a []sometype{} ?
Definitely open to input here 👍
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.
if there's more info it will be specified in the top level Signers key struct - do you think that's ok? Let me know if I'm misinterpreting your comment.
| { | ||
| "Name": "Repository", | ||
| "Keys": [ | ||
| "5a46c9aaa82ff150bb7305a2d17d0c521c2d784246807b2dc611f436a69041fd" |
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.
Same for keys ([]string vs []sometype{})
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 happy to make the change but don't think we'll need it - notary only exposes []string for Key IDs in its role structs
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.
updated to use a custom type!
ex:
"Signers": [
{
"Name": "bob",
"Keys": [
{
"ID": "B"
}
]
},
...
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 forgot to update the examples in the docs 😅
cli/command/trust/view.go
Outdated
|
|
||
| // lookupTrustInfo returns processed signature and role information about a notary repository. | ||
| // This information is to be pretty printed or serialized into a machine-readable format. | ||
| func lookupTrustInfo(cli command.Cli, remote string) (trustTagRowList, []client.RoleWithSignatures, []data.Role, error) { |
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.
lookupTrustInfo() is now shared between inspect and view; perhaps it (and the tests) should be moved to a separate file common.go ? I see there's a helpers.go, but not sure this qualifies as a "helper".
Same for:
getDelegationRoleToKeyMap()matchReleasedSignatures()
(Possibly same for trustTagKey and the other types defined at the top of this file)
| If signers are set up for the repository via other `docker trust` commands, `docker trust inspect` includes a `Signers` key: | ||
|
|
||
| ```bash | ||
|
|
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.
Can you remove this line?
| ### Get details about signatures for multiple images | ||
|
|
||
| ```bash | ||
| $ docker trust inspect alpine ubuntu | jq |
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.
drop | jq
| ``` | ||
|
|
||
| ### Get details about signatures for all image tags in a repository | ||
|
|
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.
Can you add a short introduction here? Just to lead in the example
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.
👍 added!
|
|
||
|
|
||
| ### Get details about signatures for multiple images | ||
|
|
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.
Can you add a short introduction here? Just to lead in the example
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.
added!
| cmd := &cobra.Command{ | ||
| Use: "inspect IMAGE[:TAG] [IMAGE[:TAG]...]", | ||
| Short: "Return low-level information about keys and signatures", | ||
| Args: cli.RequiresMinArgs(1), |
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.
We should either disallow specifying multiple repositories, or add the repository name to the output. The output as it is, is not useful if you specify multiple repositories;
$ docker trust inspect alpine hello-world | jqProduces (removed some entries to keep it short):
[
{
"SignedTags": [
{
"SignedTag": "2.6",
"Digest": "9ace551613070689a12857d62c30ef0daa9a376107ec0fff0e34786cedb3399b",
"Signers": [
"Repo Admin"
]
},
{
"SignedTag": "latest",
"Digest": "d6bfc3baf615dc9618209a8d607ba2a8103d9c8a405b3bd8741d88b4bef36478",
"Signers": [
"Repo Admin"
]
}
],
"AdminstrativeKeys": [
{
"Name": "Root",
"Keys": [
"a2489bcac7a79aa67b19b96c4a3bf0c675ffdf00c6d2fabe1a5df1115e80adce"
]
},
{
"Name": "Repository",
"Keys": [
"5a46c9aaa82ff150bb7305a2d17d0c521c2d784246807b2dc611f436a69041fd"
]
}
]
},
{
"SignedTags": [
{
"SignedTag": "latest",
"Digest": "0e06ef5e1945a718b02a8c319e15bae44f47039005530bc617a5d071190ed3fc",
"Signers": [
"Repo Admin"
]
},
{
"SignedTag": "linux",
"Digest": "452733afb5319f624b88aec51006a077bccf036a87167fd657057e2e31f42736",
"Signers": [
"Repo Admin"
]
},
],
"AdminstrativeKeys": [
{
"Name": "Root",
"Keys": [
"ff544c7f4ffa0a61b2fb836fb581182181c971acd58a7023c9a3b049dc952edd"
]
},
{
"Name": "Repository",
"Keys": [
"8d2aaa7461b4305f74dc80bb946ccad962829a57bd0412a480e17a2413b18ec9"
]
}
]
}
]With that information alone, I don't know which repository the :latest tag belongs to
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.
sounds good, I added Name as the first field 👍
cli/command/trust/inspect.go
Outdated
|
|
||
| // trustRepo represents consumable information about a trusted repository | ||
| type trustRepo struct { | ||
| SignedTags trustTagRowList `json:",omitempty"` |
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 think this type needs a property for the repo-name (see my comment for "multiple repositories")
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.
Left some notes for the docs, but LGTM otherwise
| { | ||
| "Name": "Repository", | ||
| "Keys": [ | ||
| "5a46c9aaa82ff150bb7305a2d17d0c521c2d784246807b2dc611f436a69041fd" |
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 forgot to update the examples in the docs 😅
| ] | ||
| }, | ||
| { | ||
| "SignedTag": "3.6", |
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.
When updating for the new format, can you also shorten the output of this example a bit? Only 1 .. 2 tags should be needed to show what it does 😄
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, but may need some squashing 👍
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
7002e90 to
a942828
Compare
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, thanks!

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

Introduces a
docker trust inspectcommand for JSON output with the same information presented in the human-readabledocker trust view.I refactored some of the existing
viewcode so that it could be shared withinspectand added a docs page.cc @ashfall @eiais for additional review, and @rn to confirm that this information is what he was looking for from the original issue (#659)
shortened and modified example here:
Closes #659