Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

[18.03] backport content trust tests, windows, test-refactoring #504

Merged

Conversation

thaJeztah
Copy link
Member

This brings in new tests for Docker Trust that were previously disabled due to changes in the CLI. In addition, a lot of refactoring and improvements to the tests are included to facilitate e2e testing, and testing on Windows. This also removes the file watcher dependency, which broke CI.

git checkout -b 18.03-backport-content-trust-tests upstream/18.03

# revert https://github.com/docker/docker-ce/pull/437 (which disabled the trust tests)
git revert -s -S a720337d2e359f9215422b47c9ef66421d582f24
git revert -s -S d5f8753b884085b96d9d828771ba23adf7d6700a

# https://github.com/moby/moby/pull/36515 Remove DockerTrustSuite to docker/cli e2e tests
git cherry-pick -s -S -x -Xsubtree=components/engine 5433ceb12ead305d8c85e8e27c4b4d842ef88ae0

# https://github.com/docker/cli/pull/911 Update gotestyourself dependency
git cherry-pick -s -S -x -Xsubtree=components/cli 98ba439f675cf4ed9fd37d86c3a74beaf065f40a

# https://github.com/docker/cli/pull/879  Replace testify assertions with gotestyourself/assert
git cherry-pick -s -S -x -Xsubtree=components/cli 93615dd967b7b88371207973e55c138ee8cb21d4
git cherry-pick -s -S -x -Xsubtree=components/cli 5ef8835f239bcb3156f02dd39f30bdca44544894
git cherry-pick -s -S -x -Xsubtree=components/cli 39c2ca57c1c237bd942175602dcd00b124b4220b
git cherry-pick -s -S -x -Xsubtree=components/cli 5155cda716ad7f519e608c380e43d1ca90f855c0

# https://github.com/docker/cli/pull/918 Remove testutil
git cherry-pick -s -S -x -Xsubtree=components/cli 681c921528a892e0a087fa90124838e4ad29f940

# https://github.com/docker/cli/pull/921 testing: Use canonical asserts
git cherry-pick -s -S -x -Xsubtree=components/cli 0f11a310fd7d64a697d0ed9098205b6cdd585f1b
git cherry-pick -s -S -x -Xsubtree=components/cli baf65a5502580a46090c7e523dfa3a3a075d7c8f
git cherry-pick -s -S -x -Xsubtree=components/cli 078cbc9c4b8e66ed0537f80913e4b488f2100190
git cherry-pick -s -S -x -Xsubtree=components/cli f21276575f2679793a0544f40fc518763b0b80a8
git cherry-pick -s -S -x -Xsubtree=components/cli e15b208e9688efa0e0caba36d8e8e631a3a46374

# https://github.com/docker/cli/pull/916 Use new APIClient interface
git cherry-pick -s -S -x -Xsubtree=components/cli cff874122c92fdd6eebe3c94bf8b194dce8b8bf4

# https://github.com/docker/cli/pull/917 Don't set a default filename for ConfigFile
git cherry-pick -s -S -x -Xsubtree=components/cli 7c8b5708ebea29ee325045e265fad8257c08f737
git cherry-pick -s -S -x -Xsubtree=components/cli 789acb526c865f8d0ab22bccd1f59fb150841889


# https://github.com/docker/cli/pull/929 Refactor content_trust cli/flags handling
git cherry-pick -s -S -x -Xsubtree=components/cli 6e21829af477c1d925545ddfc1b9943ead5c9655
git cherry-pick -s -S -x -Xsubtree=components/cli feae0e9756db152712473d61245546767b6a5282

# https://github.com/docker/cli/pull/945 Small content trust enhancement 
git cherry-pick -s -S -x -Xsubtree=components/cli 63ebcae382f083dce20f7c7ca852493260f13fff

# https://github.com/docker/cli/pull/905 Add appveyor setup to build and unit test
git cherry-pick -s -S -x -Xsubtree=components/cli facb22573d0d215c511b840db6417aebd2b1122e
git cherry-pick -s -S -x -Xsubtree=components/cli 0cf2e6353a88a12b78da72db6a7e7c7d049d3ed8
git cherry-pick -s -S -x -Xsubtree=components/cli 10baa756b264778ae0c64ba058c8aeea733ed6fa

# https://github.com/docker/cli/pull/925 Add a build unit test for symlinked context
git cherry-pick -s -S -x -Xsubtree=components/cli 00b803b2d8d52ad86ad4d70dae6c27200b3df855

# https://github.com/docker/cli/pull/924 Add some content trust tests
git cherry-pick -s -S -x -Xsubtree=components/cli 8b00c5cfd87ac653e2a1dc4b95df30ca23e1d40d

# https://github.com/docker/cli/pull/955 Remove filewatcher
git cherry-pick -s -S -x -Xsubtree=components/cli c0588a9c8feecf197da1478d91e8f10d26261847

git push -u origin 18.03-backport-content-trust-tests

ping @vdemeester @n4ss @andrewhsu @seemethere PTAL: there's some small changes in the actual code, so please double check these are desirable

thaJeztah and others added 26 commits April 10, 2018 11:51
This reverts commit a720337.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This reverts commit d5f8753.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 5433ceb12ead305d8c85e8e27c4b4d842ef88ae0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 98ba439f675cf4ed9fd37d86c3a74beaf065f40a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
and fix some tests

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 93615dd967b7b88371207973e55c138ee8cb21d4)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 5ef8835f239bcb3156f02dd39f30bdca44544894)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 39c2ca57c1c237bd942175602dcd00b124b4220b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Fix tests that failed when using cmp.Compare()
internal/test/testutil/assert
InDelta
Fix DeepEqual with kube metav1.Time
Convert some ErrorContains to assert

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 5155cda716ad7f519e608c380e43d1ca90f855c0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 681c921528a892e0a087fa90124838e4ad29f940)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 0f11a310fd7d64a697d0ed9098205b6cdd585f1b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Using:

  git grep -l '^\s\+assert\.Check(t, err)$' | \
    xargs sed -i -e 's/^\(\s\+assert\)\.Check(t, err)$/\1.NilError(t, err)/'

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit baf65a5502580a46090c7e523dfa3a3a075d7c8f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
git grep -l -P '^\s+assert\.Check\(t, ' | \
    xargs perl -pi -e 's/^(\s+assert)\.Check(\(t, (?!is).*(\.Execute\(|\.Set\(|\.Write\(|\.Close\(|\.Untar\(|\.WriteFile\(|Validate\().*\)$)/\1.NilError\2/'

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 078cbc9c4b8e66ed0537f80913e4b488f2100190)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit f21276575f2679793a0544f40fc518763b0b80a8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
git grep -l -P '^\s+assert\.Check\(t, is\.Error\(' | \
    xargs perl -pi -e 's/^(\s+assert\.)Check\(t, is\.Error\((.*)\)$/\1Error(t, \2/'

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit e15b208e9688efa0e0caba36d8e8e631a3a46374)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit cff874122c92fdd6eebe3c94bf8b194dce8b8bf4)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
With a default filename tests will leave a file in the working directory
that is never cleaned up.

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 7c8b5708ebea29ee325045e265fad8257c08f737)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 789acb526c865f8d0ab22bccd1f59fb150841889)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Remove the global variable used. Allows easier unit testing.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 6e21829af477c1d925545ddfc1b9943ead5c9655)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Rename IsTrusted to ContentTrustEnabled

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit feae0e9756db152712473d61245546767b6a5282)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- `replaceDockerfileForContentTrust` is only used when content trust is
  enabled, so remove the boolean.
- rename `isContentTrustEnabled` to `contentTrustEnabled`

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 63ebcae382f083dce20f7c7ca852493260f13fff)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Adds a `make.ps1` powershell script to make it easy to compile and test.

```
.\scripts\make.ps1 -Binary
INFO: make.ps1 starting at 03/01/2018 14:37:28
INFO: Building...

 ________   ____  __.
 \_____  \ |    |/ _|
 /   |   \|      <
 /    |    \    |  \
 \_______  /____|__ \
         \/        \/

INFO: make.ps1 ended at 03/01/2018 14:37:30

.\scripts\make.ps1 -TestUnit
```

The next step is to run e2e tests on windows too.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit facb22573d0d215c511b840db6417aebd2b1122e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Some of them are skipped for now (because the feature is not supported
or needs more work), some of them are fixed.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 0cf2e6353a88a12b78da72db6a7e7c7d049d3ed8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 10baa756b264778ae0c64ba058c8aeea733ed6fa)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 00b803b2d8d52ad86ad4d70dae6c27200b3df855)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Importing from moby's DockerTrustSuite tests.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
(cherry picked from commit 8b00c5cfd87ac653e2a1dc4b95df30ca23e1d40d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit c0588a9c8feecf197da1478d91e8f10d26261847)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@andrewhsu
Copy link
Contributor

Seeing a failure on docker cli unit test:

--- FAIL: TestTrustInspectPrettyCommandEmptyNotaryRepoErrors (0.00s)
	inspect_pretty_test.go:112: assertion failed: string "\nNo signatures for reg/img\n\n\nAdministrative keys for reg/img\n\n  Repository Key:\ttargetsID\n  Root Key:\trootID\n" does not contain "Administrative keys for reg/img:"

@andrewhsu
Copy link
Contributor

I think the assertion is looking for too many colons.

assert.Contains(t, cli.OutBuffer().String(), "Administrative keys for reg/img")
assert.NilError(t, cmd.Execute())
assert.Check(t, is.Contains(cli.OutBuffer().String(), "No signatures for reg/img"))
assert.Check(t, is.Contains(cli.OutBuffer().String(), "Administrative keys for reg/img:"))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not include the : in the assertion

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think needs cherry-pick commits from docker/cli#934

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, I saw the PR itself was already backported; #461, but the backport PR says the tests didn't cherry-pick clean, so probably that's what we're running into

Copy link
Member Author

Choose a reason for hiding this comment

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

@seemethere ah; I see what's happened; they're cherry-picked in the reverse order, so that fix got lost indeed; I'll add a commit to fix that

This fix was part of 8c3d0b93d631f948376a81c7baecc6dad88ce248,
but was reverted due to the order in which other changes were
backported.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@andrewhsu andrewhsu added this to the 18.03.1 milestone Apr 11, 2018
@andrewhsu
Copy link
Contributor

Looks like a failure on one of the unit tests:

FAIL	github.com/docker/cli/cli/compose/template [build failed]

@n4ss
Copy link
Contributor

n4ss commented Apr 11, 2018

yeah, weird that there is no additional details than
00:54:09 FAIL github.com/docker/cli/cli/compose/template [build failed]
and I can't find something in the changelog that would make the file not build anymore..

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

Failure was due to imports in a test file;

# github.com/docker/cli/cli/compose/template
cli/compose/template/template_test.go:4:2: imported and not used: "reflect"
cli/compose/template/template_test.go:23:18: undefined: is
cli/compose/template/template_test.go:47:19: undefined: is
cli/compose/template/template_test.go:55:19: undefined: is
cli/compose/template/template_test.go:63:19: undefined: is
cli/compose/template/template_test.go:70:18: undefined: is
cli/compose/template/template_test.go:76:18: undefined: is
cli/compose/template/template_test.go:82:18: undefined: is


--- PASS: TestValidateIsolation (0.00s)
PASS
ok  	github.com/docker/cli/cli/compose/schema	0.034s
FAIL	github.com/docker/cli/cli/compose/template [build failed]
?   	github.com/docker/cli/cli/compose/types	[no test files]

pushed a commit to fix those

@thaJeztah
Copy link
Member Author

Note that those are due to docker/cli#893 not being in 18.03. I'm still on the fence if we should add it, so that compose-file parsing is on par with what docker-compose supports

Copy link
Contributor

@seemethere seemethere left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewhsu andrewhsu merged commit 1556d2d into docker-archive:18.03 Apr 11, 2018
@thaJeztah thaJeztah deleted the 18.03-backport-content-trust-tests branch April 11, 2018 21:09
@n4ss
Copy link
Contributor

n4ss commented Apr 11, 2018

LGTM (post-merge sorry)

@thaJeztah
Copy link
Member Author

Also includes docker/cli#920

docker-jenkins pushed a commit that referenced this pull request Oct 13, 2020
[19.03 backport] static: add containerd-shim-runc-v2
Upstream-commit: fe470afee9281bb63dbb3515c632de632ba4b696
Component: packaging
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants