docker trust inspect #694

riyazdf · 2017-11-16T00:52:47Z

Introduces a docker trust inspect command for JSON output with the same information presented in the human-readable docker trust view.

I refactored some of the existing view code so that it could be shared with inspect and 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:

🐳 $ docker trust inspect my-image:purple
[
  {
    "Name": "my-image:purple",
    "SignedTags": [
      {
        "SignedTag": "purple",
        "Digest": "941d3dba358621ce3c41ef67b47cf80f701ff80cdf46b5cc86587eaebfe45557",
        "Signers": [
          "alice",
          "bob",
          "carol"
        ]
      }
    ],
    "Signers": [
      {
        "Name": "alice",
        "Keys": [
                {     
                          "ID": "04dd031411ed671ae1e12f47ddc8646d98f135090b01e54c3561e843084484a3"
                },
                {
                          "ID": "6a11e4898a4014d400332ab0e096308c844584ff70943cdd1d6628d577f45fd8"
                }
        ]
      },
      {
        "Name": "bob",
        "Keys": [
                {
                          "ID": "433e245c656ae9733cdcc504bfa560f90950104442c4528c9616daa45824ccba"
                }
        ]
      },
      {
        "Name": "carol",
        "Keys": [
                {
                          "ID": "d32fa8b5ca08273a2880f455fcb318da3dc80aeae1a30610815140deef8f30d9"
                },
                {
                          "ID":  "9a8bbec6ba2af88a5fad6047d428d17e6d05dbdd03d15b4fc8a9a0e8049cd606"
                }
        ]
      }
    ],
    "AdminstrativeKeys": [
      {
        "Name": "Repository",
        "Keys": [
                {
                          "ID": "27df2c8187e7543345c2e0bf3a1262e0bc63a72754e9a7395eac3f747ec23a44"
                }
        ]
      },
      {
        "Name": "Root",
        "Keys": [
                {
                          "ID": "40b66ccc8b176be8c7d365a17f3e046d1c3494e053dd57cfeacfe2e19c4f8e8f"
                }
        ]
      }
    ]
  }
]

Closes #659

rn · 2017-11-16T08:57:26Z

Thanks. the output looks good to me

dnephin · 2017-11-16T16:21:43Z

cli/command/trust/inspect.go

+		Args:  cli.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return inspectTrustInfo(dockerCli, args[0])
+		},


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.

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

eiais · 2017-11-16T19:14:12Z

docs/reference/commandline/trust_inspect.md

+
+
+```bash
+$ docker trust inspect alpine:latest | jq


None of the other inspect command's docs show piping to jq

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)

dnephin

LGTM

vdemeester

LGTM 🐸
cc @thaJeztah for docs

thaJeztah

Thanks! See my comments inline 😄

thaJeztah · 2017-11-17T08:58:20Z

cli/command/trust/inspect.go

+	}, nil
+}
+
+// trustRepo represents consumable information about a trusted repository


nit: can you move these types to the top of this file? (looks like we do so in the other trust commands)

thaJeztah · 2017-11-17T09:10:23Z

docs/reference/commandline/trust_inspect.md

+        "SignedTag": "latest",
+        "Digest": "d6bfc3baf615dc9618209a8d607ba2a8103d9c8a405b3bd8741d88b4bef36478",
+        "Signers": [
+          "Repo Admin"


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 👍

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.

thaJeztah · 2017-11-17T09:10:48Z

docs/reference/commandline/trust_inspect.md

+      {
+        "Name": "Repository",
+        "Keys": [
+          "5a46c9aaa82ff150bb7305a2d17d0c521c2d784246807b2dc611f436a69041fd"


Same for keys ([]string vs []sometype{})

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

updated to use a custom type!

ex:

"Signers": [ { "Name": "bob", "Keys": [ { "ID": "B" } ] }, ...

You forgot to update the examples in the docs 😅

thaJeztah · 2017-11-17T09:21:35Z

cli/command/trust/view.go

+
+// 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) {


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)

thaJeztah · 2017-11-17T09:56:38Z

docs/reference/commandline/trust_inspect.md

+If signers are set up for the repository via other `docker trust` commands, `docker trust inspect` includes a `Signers` key:
+
+```bash
+


Can you remove this line?

thaJeztah · 2017-11-17T11:19:43Z

docs/reference/commandline/trust_inspect.md

+### Get details about signatures for multiple images
+
+```bash
+$ docker trust inspect alpine ubuntu | jq


drop | jq

thaJeztah · 2017-11-17T11:20:08Z

docs/reference/commandline/trust_inspect.md

+```
+
+### Get details about signatures for all image tags in a repository
+


Can you add a short introduction here? Just to lead in the example

👍 added!

thaJeztah · 2017-11-17T11:20:15Z

docs/reference/commandline/trust_inspect.md

+
+
+### Get details about signatures for multiple images
+


Can you add a short introduction here? Just to lead in the example

thaJeztah · 2017-11-17T11:24:38Z

cli/command/trust/inspect.go

+	cmd := &cobra.Command{
+		Use:   "inspect IMAGE[:TAG] [IMAGE[:TAG]...]",
+		Short: "Return low-level information about keys and signatures",
+		Args:  cli.RequiresMinArgs(1),


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 | jq

Produces (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

sounds good, I added Name as the first field 👍

thaJeztah · 2017-11-17T11:25:27Z

cli/command/trust/inspect.go

+
+// trustRepo represents consumable information about a trusted repository
+type trustRepo struct {
+	SignedTags        trustTagRowList `json:",omitempty"`


I think this type needs a property for the repo-name (see my comment for "multiple repositories")

thaJeztah

Left some notes for the docs, but LGTM otherwise

thaJeztah · 2017-11-21T15:19:34Z

docs/reference/commandline/trust_inspect.md

+      {
+        "Name": "Repository",
+        "Keys": [
+          "5a46c9aaa82ff150bb7305a2d17d0c521c2d784246807b2dc611f436a69041fd"


You forgot to update the examples in the docs 😅

thaJeztah · 2017-11-21T15:21:19Z

docs/reference/commandline/trust_inspect.md

+                ]
+            },
+            {
+                "SignedTag": "3.6",


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 😄

thaJeztah

LGTM, but may need some squashing 👍

Signed-off-by: Riyaz Faizullabhoy <[email protected]>

thaJeztah

LGTM, thanks!

riyazdf requested review from mdlinville, thaJeztah and vdemeester as code owners November 16, 2017 00:52

GordonTheTurtle added the status/0-triage label Nov 16, 2017

dnephin added status/2-code-review and removed status/0-triage labels Nov 16, 2017

dnephin reviewed Nov 16, 2017

View reviewed changes

eiais reviewed Nov 16, 2017

View reviewed changes

mdlinville approved these changes Nov 16, 2017

View reviewed changes

riyazdf force-pushed the trust-inspect branch from 5d88927 to 4aa8de2 Compare November 16, 2017 22:11

dnephin approved these changes Nov 16, 2017

View reviewed changes

vdemeester approved these changes Nov 17, 2017

View reviewed changes

thaJeztah added the impact/changelog label Nov 17, 2017

thaJeztah requested changes Nov 17, 2017

View reviewed changes

thaJeztah requested changes Nov 21, 2017

View reviewed changes

thaJeztah approved these changes Nov 29, 2017

View reviewed changes

riyazdf added 3 commits November 29, 2017 10:52

add docker trust inspect command for JSON viewing

cd38d39

Signed-off-by: Riyaz Faizullabhoy <[email protected]>

support multiple arguments to trust inspect

1eb87cc

Signed-off-by: Riyaz Faizullabhoy <[email protected]>

Use default inspect formatting, remove omitempty, update docs

a942828

Signed-off-by: Riyaz Faizullabhoy <[email protected]>

riyazdf force-pushed the trust-inspect branch from 7002e90 to a942828 Compare November 29, 2017 18:52

thaJeztah approved these changes Nov 29, 2017

View reviewed changes

thaJeztah merged commit d921d5c into docker:master Nov 29, 2017

GordonTheTurtle added this to the 17.12.0 milestone Nov 29, 2017

thaJeztah added the kind/experimental label Nov 29, 2017

riyazdf deleted the trust-inspect branch November 29, 2017 19:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docker trust inspect #694

docker trust inspect #694

riyazdf commented Nov 16, 2017 •

edited

Loading

rn commented Nov 16, 2017

dnephin Nov 16, 2017

riyazdf Nov 16, 2017

eiais Nov 16, 2017 •

edited

Loading

thaJeztah Nov 17, 2017

dnephin left a comment

vdemeester left a comment

thaJeztah left a comment

thaJeztah Nov 17, 2017

thaJeztah Nov 17, 2017

riyazdf Nov 17, 2017

thaJeztah Nov 17, 2017

riyazdf Nov 17, 2017

riyazdf Nov 20, 2017

thaJeztah Nov 21, 2017

thaJeztah Nov 17, 2017

thaJeztah Nov 17, 2017

thaJeztah Nov 17, 2017

thaJeztah Nov 17, 2017

riyazdf Nov 17, 2017

thaJeztah Nov 17, 2017

riyazdf Nov 17, 2017

thaJeztah Nov 17, 2017

riyazdf Nov 17, 2017

thaJeztah Nov 17, 2017

thaJeztah left a comment

thaJeztah Nov 21, 2017

thaJeztah Nov 21, 2017

thaJeztah left a comment

thaJeztah left a comment

		If signers are set up for the repository via other `docker trust` commands, `docker trust inspect` includes a `Signers` key:

		```bash

		```

		### Get details about signatures for all image tags in a repository

docker trust inspect #694

docker trust inspect #694

Conversation

riyazdf commented Nov 16, 2017 • edited Loading

rn commented Nov 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eiais Nov 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnephin left a comment

Choose a reason for hiding this comment

vdemeester left a comment

Choose a reason for hiding this comment

thaJeztah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah left a comment

Choose a reason for hiding this comment

thaJeztah left a comment

Choose a reason for hiding this comment

riyazdf commented Nov 16, 2017 •

edited

Loading

eiais Nov 16, 2017 •

edited

Loading