move some trust-related code to trust package#5894
Conversation
thaJeztah
added
status/2-code-review
area/trust
kind/refactor
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (2.58%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #5894 +/- ##
=======================================
Coverage 59.28% 59.29%
=======================================
Files 355 357 +2
Lines 29758 29771 +13
=======================================
+ Hits 17643 17653 +10
- Misses 11143 11144 +1
- Partials 972 974 +2 🚀 New features to boost your workflow:
|
This function was only used by "docker trust sign"; inline the code and deprecate the function. This function has no known external consumers, so we should remove it on the first possible ocassion (which could be a minor release). Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was shared between "trust" "image" and "plugin" packages, all of which needed the trust package, so move it there instead. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
more_internalize
branch
3 times, most recently
from
283ba0e to
7debfaf
Compare
This function was shared between "image" and "container" packages, all of which needed the trust package, so move it there instead. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
more_internalize
branch
from
7debfaf to
e37d814
Compare
| // Use familiar references when interacting with client and output | ||
| familiarRef := reference.FamiliarString(tagged) | ||
| trustedFamiliarRef := reference.FamiliarString(trustedRef) | ||
| _, _ = fmt.Fprintf(cli.Err(), "Tagging %s as %s\n", trustedFamiliarRef, familiarRef) | ||
| if err := cli.Client().ImageTag(ctx, trustedFamiliarRef, familiarRef); err != nil { |
There was a problem hiding this comment.
FWIW; there's a couple of changes I want to start making; I just had a look at the image references and formats we're using, and, even slightly simplified in my test, we're literally converting the image name 11 times, including juggling between a string, to a reference (various permutations), back to a string, before we even make a request with the engine 🫠
=== RUN TestJuggleFormats
STEP 1: busybox (string)
STEP 2: docker.io/library/busybox (reference.repository)
STEP 3: docker.io/library/busybox:latest (reference.taggedReference)
STEP 4: docker.io/library/busybox:latest (string)
STEP 5: docker.io/library/busybox:latest (reference.taggedReference)
STEP 6A: docker.io/library/busybox@sha256:498a000f370d8c37927118ed80afe8adc38d1edcbfc071627d17b25c88efcab0 (reference.canonicalReference)
STEP 6B: docker.io/library/busybox:latest (reference.taggedReference)
--> Tagging busybox@sha256:498a000f370d8c37927118ed80afe8adc38d1edcbfc071627d17b25c88efcab0 as busybox:latest
STEP 7A: busybox@sha256:498a000f370d8c37927118ed80afe8adc38d1edcbfc071627d17b25c88efcab0 (string)
STEP 7B: busybox:latest (string)
STEP 8: docker.io/library/busybox:latest (reference.taggedReference)
STEP 9: docker.io/library/busybox:latest (reference.taggedReference)
--> SENDING TO DAEMON: /images/busybox@sha256:498a000f370d8c37927118ed80afe8adc38d1edcbfc071627d17b25c88efcab0/tag?repo=busybox&tag=latest
STEP 10: docker.io/library/busybox (reference.repository)
STEP 11: docker.io/library/busybox:latest (reference.taggedReference)
STEP 12: busybox (string)
--> backend.GetImage
--> backend.TagImage
| if taggedRef, ok := namedRef.(reference.NamedTagged); ok && trustedRef != nil { | ||
| return image.TagTrusted(ctx, dockerCli, trustedRef, taggedRef) | ||
| return trust.TagTrusted(ctx, dockerCli.Client(), dockerCli.Err(), trustedRef, taggedRef) | ||
| } |
There was a problem hiding this comment.
Looks like this introduced a regression; most likely because before this dockerCli.Client() would be initialised lazily, and it's possible some settings were loaded elsewhere.
I opened a PR that reverts some of the changes;
There was a problem hiding this comment.
Looks like reverting those changes (PR above) still makes things fail occasionally.
I think it's just a bad test and/or multiple tests affecting each other (perhaps using same port or otherwise)
4 participants
cli/command/image: deprecate and internalize TrustedPush
This function was only used by "docker trust sign"; inline the code
and deprecate the function.
This function has no known external consumers, so we should remove
it on the first possible ocassion (which could be a minor release).
cli/command/image: deprecate PushTrustedReference, move to trust
This function was shared between "trust" "image" and "plugin" packages,
all of which needed the trust package, so move it there instead.
cli/command/image: deprecate TagTrusted, move to cli/trust
This function was shared between "image" and "container" packages,
all of which needed the trust package, so move it there instead.
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)