Skip to content

Add content trust tests for run command - #1596

Merged
thaJeztah merged 1 commit into
docker:masterfrom
glefloch:951-run-trust-tests
Jan 23, 2020
Merged

thaJeztah merged 1 commit into
docker:masterfrom
glefloch:951-run-trust-tests

Conversation

@glefloch

@glefloch glefloch commented Jan 2, 2019

Copy link
Copy Markdown
Contributor

Signed-off-by: glefloch glfloch@gmail.com

- What I did
I add two tests on trust content for the run command. This intend to partially close #951

- How I did it
I migrated code from the pull request removing the testsuite from moby repository ( TestUntrustedRun and TestTrustedRunFromBadTrustServer)

- How to verify it
You can verify it by running the e2e testsuite

- Description for the changelog
Adding trust test for run command

- A picture of a cute animal (not mandatory but encouraged)

image

@codecov-io

codecov-io commented Jan 2, 2019 •

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (master@d443b74). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1596   +/-   ##
=========================================
  Coverage          ?   55.25%           
=========================================
  Files             ?      289           
  Lines             ?    19395           
  Branches          ?        0           
=========================================
  Hits              ?    10716           
  Misses            ?     7983           
  Partials          ?      696

@thaJeztah

Copy link
Copy Markdown
Member

ping @justincormack PTAL

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

two minor nits, but otherwise SGTM (I'll do a quick rebase of the PR to trigger CI again, and will address those nits while I'm doing so)

Comment thread e2e/container/run_test.go Outdated
func TestUntrustedRun(t *testing.T) {
dir := fixtures.SetupConfigFile(t)
defer dir.Remove()
image := "registry:5000/alpine:untrusted"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: we should probably use the registryPrefix const here;

const registryPrefix = "registry:5000"

Comment thread e2e/container/run_test.go Outdated
}

func TestTrustedRunFromBadTrustServer(t *testing.T) {
evilImageName := "registry:5000/evil-alpine:latest"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here

Signed-off-by: Guillaume Le Floch <glfloch@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztah force-pushed the 951-run-trust-tests branch from 455e62d to 348f24c Compare January 20, 2020 13:22
@glefloch

Copy link
Copy Markdown
Contributor Author

@thaJeztah thank you for the review. I was going to do the modification but you already did them, thanks!

@thaJeztah

Copy link
Copy Markdown
Member

Oops! Wanted to leave a comment that I did that, but forgot 😊 (I had the branch checked out locally, so thought I'd just as well push the change)

@silvin-lubecki silvin-lubecki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM !

@silvin-lubecki

Copy link
Copy Markdown
Contributor

Thank you @glefloch for your contribution, especially for adding tests 👍

@thaJeztah
thaJeztah merged commit 74fb129 into docker:master Jan 23, 2020
@thaJeztah thaJeztah added this to the 20.03.0 milestone Feb 19, 2020
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.

[quality] content trust tests coverage

5 participants