Commit graph

363 commits

Author SHA1 Message Date
Cory Snider
938ed9a1ed distribution/xfer: make off-by-one error a feature
maxDownloadAttempts maps to the daemon configuration flag

    --max-download-attempts int
      Set the max download attempts for each pull (default 5)

and the daemon configuration machinery interprets a value of 0 as "apply
the default value" and not a valid user value (config validation/
normalization bugs notwithstanding). The intention is clearly that this
configuration value should be an upper limit on the number of times the
daemon should try to download a particular layer before giving up. So it
is surprising to have the configuration value interpreted as a _retry_
limit. The daemon will make up to N+1 attempts to download a layer! This
also means users cannot disable retries even if they wanted to.

As this is a longstanding bug, not a recent regression, it would not be
appropriate to backport the fix (97921915a8)
in a patch release. Update the test to assert on the buggy behaviour so
it passes again.

Signed-off-by: Cory Snider <csnider@mirantis.com>
2023-02-22 12:34:59 -05:00
Cory Snider
ca712d6947 Fix loop-closure bugs in tests
...which were flagged by golangci-lint v1.51.

Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit 0c68b655f6)
Signed-off-by: Cory Snider <csnider@mirantis.com>
2023-02-22 12:26:38 -05:00
Sebastiaan van Stijn
47069ae18c
Merge pull request #44617 from thaJeztah/23.0_backport_images_errdefs
[23.0 backport] image/store: Use errdefs for errors
2022-12-09 21:31:25 +01:00
Sebastiaan van Stijn
c0c146fc82
Remove uses of deprecated go-digest.NewDigestFromHex, go-digest.Digest.Hex
Both of these were deprecated in 55f675811a,
but the format of the GoDoc comments didn't follow the correct format, which
caused them not being picked up by tools as "deprecated".

This patch updates uses in the codebase to use the alternatives.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 0f7c9cd27e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-09 00:17:18 +01:00
Sebastiaan van Stijn
4952dc2844
distribution: remove unused RequireSchema2
It's never set, so we can remove it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 85fddc0081)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-03 18:09:17 +01:00
Sebastiaan van Stijn
e7b26fa1b1
use ad-hoc libtrust key
This is only used for tests, and the key is not verified anymore, so
instead of creating a key and storing it, we can just use an ad-hoc
one.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8feeaecb84)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-03 18:09:16 +01:00
Sebastiaan van Stijn
f2463e1e86
distribution: use ad-hoc trustkey for tests
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit e854b2a459)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-03 18:09:16 +01:00
Sebastiaan van Stijn
304c56f7f5
distribution: checkSupportedMediaType: allow additional media-types
This addresses a regression introduced in 407e3a4552,
which turned out to be "too strict", as there's old images that use, for example;

    docker pull python:3.5.1-alpine
    3.5.1-alpine: Pulling from library/python
    unsupported media type application/octet-stream

Before 407e3a4552, such mediatypes were accepted;

    docker pull python:3.5.1-alpine
    3.5.1-alpine: Pulling from library/python
    e110a4a17941: Pull complete
    30dac23631f0: Pull complete
    202fc3980a36: Pull complete
    Digest: sha256:f88925c97b9709dd6da0cb2f811726da9d724464e9be17a964c70f067d2aa64a
    Status: Downloaded newer image for python:3.5.1-alpine
    docker.io/library/python:3.5.1-alpine

This patch copies the additional media-types, using the list of types that
were added in a215e15cb1, which fixed a
similar issue.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a6a539497a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-02 01:31:51 +01:00
Tianon Gravi
7175841ebd
Remove long-deprecated "arm" fallback
This fallback is used when we filter the manifest list by the user-provided platform and find no matches such that we match the previous Docker behavior (before it supported variant matching).  This has been deprecated long enough that I think it's time we finally stop supporting this weird fallback, especially since it makes for buggy behavior like `docker pull --platform linux/arm/v5 alpine:3.16` leading to a `linux/arm/v6` image being pulled (I specified a variant, every manifest list entry specifies a variant, so clearly the only behavior I as a user could reasonably expect is an error that `linux/arm/v5` is not supported, but instead I get an explicitly incompatible image despite doing everything I as a user can to prevent that situation).

Signed-off-by: Tianon Gravi <admwiggin@gmail.com>
(cherry picked from commit 5bc17c3e54)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-05 18:23:43 +01:00
Paweł Gronowski
07e84005ac
distribution: Error when pulling OCI artifacts
Currently an attempt to pull a reference which resolves to an OCI
artifact (Helm chart for example), results in a bit unrelated error
message `invalid rootfs in image configuration`.

This provides a more meaningful error in case a user attempts to
download a media type which isn't image related.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2022-11-03 19:41:51 +01:00
Brian Goff
727c4fdee3
Validate digest in repo for pull by digest
This is accomplished by storing the distribution source in the content
labels. If the distribution source is not found then we check to the
registry to see if the digest exists in the repo, if it does exist then
the puller will use it.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 27530efedb)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-21 01:50:09 +02:00
Sebastiaan van Stijn
f8e5145e96
runconfig, oci, image, layer, distribution: fix empty-lines (revive)
runconfig/config_test.go:23:46: empty-lines: extra empty line at the start of a block (revive)
    runconfig/config_test.go:75:55: empty-lines: extra empty line at the start of a block (revive)

    oci/devices_linux.go:57:34: empty-lines: extra empty line at the start of a block (revive)
    oci/devices_linux.go:60:69: empty-lines: extra empty line at the start of a block (revive)

    image/fs_test.go:53:38: empty-lines: extra empty line at the end of a block (revive)
    image/tarexport/save.go:88:29: empty-lines: extra empty line at the end of a block (revive)

    layer/layer_unix_test.go:21:34: empty-lines: extra empty line at the end of a block (revive)

    distribution/xfer/download.go:302:9: empty-lines: extra empty line at the end of a block (revive)
    distribution/manifest_test.go:154:99: empty-lines: extra empty line at the end of a block (revive)
    distribution/manifest_test.go:329:52: empty-lines: extra empty line at the end of a block (revive)
    distribution/manifest_test.go:354:59: empty-lines: extra empty line at the end of a block (revive)

    registry/config_test.go:323:42: empty-lines: extra empty line at the end of a block (revive)
    registry/config_test.go:350:33: empty-lines: extra empty line at the end of a block (revive)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8a2e1245d4)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-10-01 00:01:14 +02:00
Sebastiaan van Stijn
968ff5ab44
fix some minor linting issues
libnetwork/firewall_linux.go:11:21: var-declaration: should drop = nil from declaration of var ctrl; it is the zero value (revive)
        ctrl *controller = nil
                           ^
    distribution/pull_v2_test.go:213:4: S1038: should use t.Fatalf(...) instead of t.Fatal(fmt.Sprintf(...)) (gosimple)
                t.Fatal(fmt.Sprintf("expected formatPlatform to show windows platform with a version, but got '%s'", result))
                ^
    integration-cli/docker_cli_build_test.go:5951:3: S1038: should use c.Skipf(...) instead of c.Skip(fmt.Sprintf(...)) (gosimple)
            c.Skip(fmt.Sprintf("Bug fixed in 18.06 or higher.Skipping it for %s", testEnv.DaemonInfo.ServerVersion))
            ^
    integration-cli/docker_cli_daemon_test.go:240:3: S1038: should use c.Skipf(...) instead of c.Skip(fmt.Sprintf(...)) (gosimple)
            c.Skip(fmt.Sprintf("New base device size (%v) must be greater than (%s)", units.HumanSize(float64(newBasesizeBytes)), units.HumanSize(float64(oldBasesizeBytes))))
            ^

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-07-04 10:15:28 +02:00
Tonis Tiigi
9adad264d2 distribution: match manifest list resolution with containerd
Make finding the correct runtime image from image index
more compliant with OCI spec and match containerd implementation.

Changes:
- Manifest list is allowed to contain manifest lists
- Unknown mediatype inside manifest list is skipped instead of causing an error
- Platform in descriptor is optional 

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2022-06-02 11:21:07 -07:00
Sebastiaan van Stijn
14fdd97b69
Remove unused distribution/metadata/V1IDService
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-05-10 23:39:22 +02:00
Eng Zer Jun
7873c27cfb
all: replace strings.Replace with strings.ReplaceAll
strings.ReplaceAll(s, old, new) is a wrapper function for
strings.Replace(s, old, new, -1). But strings.ReplaceAll is more
readable and removes the hardcoded -1.

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
2022-05-09 19:45:40 +08:00
Sebastiaan van Stijn
2b0da89366
distribution: move newPusher() and newPuller() together with definition
Also moving writeStatus() to the puller, which is where it's used, and makes
it slightly easier to consume.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-04-21 23:13:39 +02:00
Sebastiaan van Stijn
566c8db66d
distribution: add GetRepository(), un-export NewV2Repository, ValidateRepoName
These were only exported to facilitate ImageService.GetRepository() (used for
the `GET /distribution/{name:.*}/json` endpoint.

Moving the core functionality of that to the distribution package makes it
more consistent with (e.g.) "pull" operations, and allows us to keep more things
internal.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-04-21 23:12:02 +02:00
Sebastiaan van Stijn
572c7e0184
distribution: remove RootFSFromConfig(), PlatformFromConfig() from ImageConfigStore
These functions did not require the ImageConfigStore, so could just be local
utilities.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-04-21 23:12:00 +02:00
Sebastiaan van Stijn
1e75ab0ab9
distribution: remove Pusher interface, NewPusher(), and redundant V1 checks
It's only used internally, so we can refer to the implementation itself. Given
that RegistryService.LookupPushEndpoints now only returns V2 endpoints, we
no longer need to check if an endpoint is possibly V1.

Also rename some types that had "v2" in their name, now that we only support v2.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-04-21 23:11:54 +02:00
Sebastiaan van Stijn
41999abcbe
distribution: remove Puller interface, remove redundant V1 checks
It's only used internally, so we can refer to the implementation itself. Given
that RegistryService.LookupPullEndpoints now only returns V2 endpoints, we
no longer need to check if an endpoint is possibly V1.

Also rename some types that had "v2" in their name, now that we only support v2.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-04-21 23:11:07 +02:00
Sebastiaan van Stijn
074e41679d
distribution: un-export internal errors and error-utilities
un-exports errors that were only used internally:

- Remove ErrNoSupport as it was not emitted anywhere
- ImageConfigPullError -> imageConfigPullError
- TranslatePullError() -> translatePullError()

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-04-21 22:53:02 +02:00
Sebastiaan van Stijn
fb5485f5d0
distribution: un-export ImageTypes, make ImagePullConfig.Schema2Types optional
Use the default list of accepted mediaTypes if none were passed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-04-21 22:36:56 +02:00
Tonis Tiigi
482d1d15bf distribution: use the maximum compatible platform by default
When no specific platform is set, pull the platform that
most matches the current host.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2022-03-31 15:20:59 -07:00
Tonis Tiigi
fcc42d5682 distribution: use correct platform matcher for containerd
This uses the correct comparison with compatibility
checks for variants.

The deprecated arm variant matcher is left as is.
Although it is not needed for valid cases it is not
fully compatible as also matches some invalid
combinations, so should be removed separately.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2022-03-27 19:46:45 -07:00
Pete Woods
9f3b1a9664
distribution: retry downloading schema config on retryable error
fixes #43267

Signed-off-by: Pete Woods <pete.woods@circleci.com>
2022-03-05 19:36:48 +01:00
Sebastiaan van Stijn
a0230f3d9a
remove unneeded "digest" alias for "go-digest"
I think this was there for historic reasons (may have been goimports expected
this, and we used to have a linter that wanted it), but it's not needed, so
let's remove it (to make my IDE less complaining about unneeded aliases).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-03-04 14:49:42 +01:00
Akihiro Suda
d809ad98e3
Merge pull request #43183 from thaJeztah/cleanup_distribution
distribution/xfer: refactor to reduce public api/interface
2022-02-26 23:50:03 +09:00
Sebastiaan van Stijn
ff408210da
distribution: remove use of deprecated dial.DualStack
From the field's description [1]:

    DualStack previously enabled RFC 6555 Fast Fallback
    support, also known as "Happy Eyeballs", in which IPv4 is
    tried soon if IPv6 appears to be misconfigured and
    hanging.

    Deprecated: Fast Fallback is enabled by default. To
    disable, set FallbackDelay to a negative value.

This field was deprecated in efc185029b,
which is included in Go 1.12beta1 and up.

[1]: 2ebe77a2fd/src/net/dial.go (L54-L61)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-22 16:34:12 +01:00
Sebastiaan van Stijn
79ea1b16b1
distribution/xfer: un-export DoFunc type
It's only used internally and by non-exported functions, so it doesn't
have to be exported.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:42 +01:00
Sebastiaan van Stijn
4d6c6a5000
distribution/xfer: smaller interface, rename to DigestRegisterer
All uses of this interface already accept a DownloadDescriptor; keeping the
interface small to allow this functionality to be used by other download-descriptors,
while still being able to check for the actual functionality (to be able to register
a digest).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:40 +01:00
Sebastiaan van Stijn
047e032461
distribution/xfer: define DownloadOption type
This is mostly for documentation purposes; defining a type makes
the option(s) show up grouped on pkg.go.dev (and in godoc).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:38 +01:00
Sebastiaan van Stijn
69b0913e1f
distribution/xfer: add TODO for return error on Registered()
There's currently four implementations in our code: one in distribution, two in
BuildKit-related code, and one "mock" for testing:

- 65b8bcc321/builder/builder-next/worker/worker.go (L452-L455)
- 65b8bcc321/builder/builder-next/adapters/containerimage/pull.go (L648-L651)
- 65b8bcc321/distribution/pull_v2.go (L318-L321)
- 65b8bcc321/distribution/xfer/download_test.go (L186-L188)

All of these call a metadata-service to register the DiffID, and all of those
may return an error, which currently gets ignored.

We should consider changing the signature to return possible errors, so that
the callsite can decide how to handle them (which could be just "log" the error
or handle them in other ways).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:36 +01:00
Sebastiaan van Stijn
3309ea9419
distribution/xfer: slightly more idomatic type assertion
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:35 +01:00
Sebastiaan van Stijn
4c7dc9806c
distribution/xfer: un-export transfer interface accessors
This interface is internal to the package, so there's no need to export
it's methods.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:33 +01:00
Sebastiaan van Stijn
849d8c2d02
distribution/xfer: un-export Watcher
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:31 +01:00
Sebastiaan van Stijn
96a750aab4
distribution/xfer: un-export Transfer interface
The only implementations and uses are internal to this package.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:29 +01:00
Sebastiaan van Stijn
765844e419
distribution/xfer: un-export NewTransfer()
This is also only used internally, so no need to export.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:28 +01:00
Sebastiaan van Stijn
587c474b57
distribution/xfer: use "transferManager" in godoc
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:26 +01:00
Sebastiaan van Stijn
874b11495b
distribution/xfer: un-export transferManager.setConcurrency() and .transfer()
They're only used within the package itself, so no need to have them public.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:24 +01:00
Sebastiaan van Stijn
d746a836fc
distribution/xfer: remove TransferManager interface, un-export newTransferManager
The `TransferManager` interface only had a single implementation, and neither
`LayerDownloadManager`, nor `LayerUploadManager` currently had an option to
provide a custom implementation, so we can un-export this.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:22 +01:00
Sebastiaan van Stijn
cf31aa0fa0
distribution/xfer: fix some nits in tests
- use const instead of a variable
- remove unneeded type-conversion
- use keys in literals

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-18 16:58:19 +01:00
Sebastiaan van Stijn
32e5fe5099
Merge pull request #43182 from thaJeztah/layer_remove_unused_error
layer: remove unused error return from .Size() and .DiffSize()
2022-02-17 20:51:45 +01:00
Akihiro Suda
65b8bcc321
Merge pull request #43174 from thaJeztah/move_platformcheck
distribution: remove RootFSDownloadManager interface, and remove "os" argument from Download()
2022-01-26 14:08:44 +09:00
Sebastiaan van Stijn
f9a1846ca2
distribution: PushLayer.Size(): remove unused error return
None of the implementations returned an error for this function, so removing it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-01-24 18:45:51 +01:00
Sebastiaan van Stijn
e1ea911aba
layer: remove unused error return from .Size() and .DiffSize()
None of the implementations used return an error, so removing the error
return can simplify using these.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-01-24 18:45:47 +01:00
Sebastiaan van Stijn
bb66ebd621
distribution: xfer.LayerDownloadManager.Download(): remove "os" argument
This argument was added for LCOW support, but it was only used to verify if
the passed platform (OS) matched the host. Given that all uses of this function
(except for one) passed runtime.GOOS, we may as well move the check to that
location.

We should do more cleaning up after this, and perform such validations early,
instead of passing platform around in too many places where it's only used for
similar validations. This is a first step in that direction.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-01-21 15:07:55 +01:00
Sebastiaan van Stijn
0b0a995d9d
distribution: remove RootFSDownloadManager interface
This interface only had a single implementation (xfer.LayerDownloadManager),
and all places where it was used already imported the xfer package.
Removing the interface, also makes it a closer match to the "upload" part,
as `xfer.LayerUploadManager()` did not use an interface.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-01-21 13:53:36 +01:00
Ziheng Liu
3dda7311cd xfer: fix a simple goroutine leak in unit test
Signed-off-by: Ziheng Liu <lzhfromustc@gmail.com>
2022-01-07 20:28:37 -08:00
Samuel Karp
7c216bcabe
distribution: validate blob type
Signed-off-by: Samuel Karp <skarp@amazon.com>
2021-11-23 17:19:01 -08:00