cli/trust: TagTrusted: lazily request APIClient

[thaJeztah]

Seeing various test-failures that could be a regression, related to

• move some trust-related code to trust package #5894

cli/trust: TagTrusted: remove, and inline code

Commit e37d814 moved the image.TagTrusted
function to the trust package, but changed the signature slightly to accept
an API client, instead of requiring the command.Cli. However, this could
result in situations where the Client obtained from the CLI was not correctly
initialized, resulting in failures in our e2e test;

=== FAIL: e2e/global TestPromptExitCode/plugin_upgrade (9.14s)
cli_test.go:203: assertion failed: Command: docker plugin push registry:5000/plugin-content-trust-upgrade:next
ExitCode: 1
Error: exit status 1
Stdout: The push refers to repository [registry:5000/plugin-content-trust-upgrade]
24ec5b45d59b: Preparing
6a594992d358: Preparing
224414d1b129: Preparing
24ec5b45d59b: Preparing
6a594992d358: Preparing
224414d1b129: Preparing
Stderr: error pushing plugin: failed to do request: Head "https://registry:5000/v2/plugin-content-trust-upgrade/blobs/sha256:6a594992d358facbbc4ab134bbbba77cb91e0adee6ff0d6103403ff94a9b796c": http: server gave HTTP response to HTTPS client
Failures:
ExitCode was 1 expected 0
Expected no error

This patch changes the signature to accept an "APIClientProvider" so that
the Client is obtained the moment when used.

We should look what exactly causes this situation, and if we can make
sure that requesting the Client() will always produce the client with
the expected configuration.

While looking at the code, I also noticed that Client.ImageTag already
parses and normalizes tags given, so we don't need to convert them to
their "familiar" form, other than for printing the message;

https://github.com/moby/moby/blob/b4bdf12daec84caaf809a639f923f7370d4926ad/client/image_tag.go#L11-L37

With that taken into account, trust.TrustedPush has no real value, other
than printing an informational message, so removing the function and
inlining it in the locations where it's used.

⚠️ WARNING: looks like the test is still flaky after this change, so it
may just be a bad test, or tests affecting each-other (same port, but
different config?). That said; these changes may still be ok to
include.

- What I did

- How I did it

- How to verify it

- Human readable description for the release notes

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

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.

Project coverage is 59.27%. Comparing base (2eec746) to head (1e32cb2).

Additional details and impacted files

@@ Coverage Diff @@## master #5905 +/- ##
==========================================
+ Coverage 59.26% 59.27% +0.01% 
==========================================
Files 357 358 +1 Lines 29771 29771 ==========================================
+ Hits 17645 17648 +3 + Misses 11153 11150 -3 
Partials 973 973 

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[thaJeztah]

Looks like it passes with this revert; wondering if it's because the .Client() would be called as part of the TagTrusted, but now we're calling it early 🤔 (possibly before things related to trust were initialised?)

[thaJeztah]

☝️ So doesn't look like the cause, and very likely just a flaky test in general.

[thaJeztah]

This looks unrelated; some other issue with GitHub?

#2 ERROR: failed to fetch remote https://github.com/docker/cli.git: git stderr:
error: RPC failed; HTTP 504 curl 22 The requested URL returned error: 504
fatal: expected flush after ref listing
: exit status 128
------
> [internal] load git source https://github.com/docker/cli.git#refs/pull/5905/merge:
0.198 b20aa460db47688497ecfde3433650ab17957c4e	refs/pull/5905/merge
600.1 error: RPC failed; HTTP 504 curl 22 The requested URL returned error: 504
600.1 fatal: expected flush after ref listing
------
ERROR: failed to solve: failed to read dockerfile: failed to fetch remote https://github.com/docker/cli.git: git stderr:
error: RPC failed; HTTP 504 curl 22 The requested URL returned error: 504
fatal: expected flush after ref listing
: exit status 128

[thaJeztah]

Yup, looks like that may be it

[thaJeztah]

Ugh; looks like it's still happening, so need to dig deeper, or it was just something that was flaky before?

 Stderr: error pushing plugin: failed to do request: Head "https://registry:5000/v2/plugin-content-trust/blobs/sha256:f8208ac0663752e1e4fa3ccbc2e4640e2d87b6de7c2a0df4482e1420607bb1b3": http: server gave HTTP response to HTTPS client

[thaJeztah]

OK, so looks like it's really just some flaky test. I think these changes still make sense to include, but not sure what's causing those tests to be flaky, other than multiple tests affecting each other possibly.

[vvoland]

I don't think it actually makes it that much lazier?

Looks like it only moves it after the
_, _ = fmt.Fprintf(out, "Tagging %s as %s\n", trustedFamiliarRef, familiarRef).

It also duplicates the TagTrusted in two other places.

[Benehiko]

oh wait, yeah i see there are merge conflicts and needs some adjustments.