Skip to content
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

Move Docker Trust out of experimental #935

Merged
merged 2 commits into from
Mar 13, 2018

Conversation

n4ss
Copy link
Contributor

@n4ss n4ss commented Mar 9, 2018

Signed-off-by: Nassim 'Nass' Eddequiouaq eddequiouaq.nassim@gmail.com

This is a follow-up on #934 which moves docker trust view to docker trust inspect --pretty.


- What I did
Moved docker trust commands out of experimental

- How to verify it
Run docker trust commands successfully without "experimental" enabled in the CLI config.json.

- Description for the changelog
Moved Docker Trust commands out of experimental.


Important: This PR shouldn't be merged before #934

@n4ss
Copy link
Contributor Author

n4ss commented Mar 9, 2018

Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>
@vieux vieux force-pushed the trust-out-of-experimental2 branch from fa8060d to ac35e85 Compare March 9, 2018 21:29
Copy link
Contributor

@vieux vieux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM #934 was merged, so we can merge this one.

@thaJeztah
Copy link
Member

This also requires the code changes to remove it from experimental, for example;

Annotations: map[string]string{"experimentalCLI": ""},

@thaJeztah
Copy link
Member

And possibly documentation changes

@codecov-io
Copy link

codecov-io commented Mar 9, 2018

Codecov Report

Merging #935 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #935      +/-   ##
==========================================
- Coverage   53.92%   53.91%   -0.01%     
==========================================
  Files         262      262              
  Lines       16605    16600       -5     
==========================================
- Hits         8954     8950       -4     
  Misses       7050     7050              
+ Partials      601      600       -1

@vieux
Copy link
Contributor

vieux commented Mar 9, 2018

@thaJeztah I don't understand your comment, Annotations: map[string]string{"experimentalCLI": ""}, was removed.

@thaJeztah
Copy link
Member

oh, never mind, I thought there were more locations it was in 😊 but we only put it on the root (trust) command

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM

but we need some updates in the docs, (e.g. https://github.com/docker/cli/blob/master/docs/reference/commandline/trust_view.md needs to be merged with the "inspect" docs)

@vieux
Copy link
Contributor

vieux commented Mar 13, 2018

@thaJeztah I pushed a doc update here

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spotted a typo and some nits, but looks good otherwise

### Get details about signatures for a single image tag

```bash
$ docker trust inspect --pretyy alpine:latest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo pretyy


The administrative keys listed specify the root key of trust, as well as the administrative repository key. These keys are responsible for modifying signers, and rotating keys for the signed repository.

If signers are set up for the repository via other `docker trust` commands, `docker trust inspect --pretty` displays them appropriately as a `SIGNER` and specify their `KEYS`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you wrap these lines to 80 chars?


The `SIGNED TAG` is the signed image tag with a unique content-addressable `DIGEST`. `SIGNERS` lists all entities who have signed.

The administrative keys listed specify the root key of trust, as well as the administrative repository key. These keys are responsible for modifying signers, and rotating keys for the signed repository.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you wrap these lines to 80 chars?

Administrative keys for my-image:
Repository Key: 27df2c8187e7543345c2e0bf3a1262e0bc63a72754e9a7395eac3f747ec23a44
Root Key: 40b66ccc8b176be8c7d365a17f3e046d1c3494e053dd57cfeacfe2e19c4f8e8f
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a newline at the end

Signed-off-by: Victor Vieux <victorvieux@gmail.com>
@vieux vieux force-pushed the trust-out-of-experimental2 branch from 8c906ce to 09ec6d4 Compare March 13, 2018 23:51
@vieux
Copy link
Contributor

vieux commented Mar 13, 2018

@thaJeztah I addressed your comments

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@thaJeztah thaJeztah modified the milestones: 18.04.0, 18.03.0 Mar 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants