Some S3 compatible object storage systems like R2 require that all
multipart chunks are the same size. This was mostly true before, except
the final chunk was larger than the requested chunk size which causes
uploads to fail.
In addition, the two byte slices have been replaced with a single
*bytes.Buffer and the surrounding code simplified significantly.
Fixes: #3873
Signed-off-by: Thomas Way <thomas@6f.io>
when a directory is empty, the s3 api lists it with a trailing slash.
this causes the path to be appended twice to the walkInfo slice, causing
purge uploads path transformations to panic when the `_uploads` is
emtpy.
this adds a check for file paths ending on slash, and do not append
those as regular files to the walkInfo slice.
fixes#4358
Signed-off-by: Flavian Missi <fmissi@redhat.com>
The latest golangci-lint spits out some govet issues.
This commit fixes them. We are also bumping the linter version.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
https://github.com/distribution/distribution/pull/4146 introduced a new
rewrite storage middleware but somehow missed to update the init logging
message. This commit fixes that.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Stat always calls ListObjects when stat-ing S3 key.
Unfortauntely ListObjects is not a free call - both in terms of egress
and actual AWS costs (likely because of the egress).
This changes the behaviour of Stat such that we always attempt the
HeadObject call first and only ever fall through to ListObjects if the
HeadObject returns an AWS API error.
Note, that the official docs mention that the only error returned by
HEAD is NoSuchKey; experiments show that this is demonstrably wrong and
the AWS docs are simply outdated at the time of this commit.
HeadObject actually returns the following errors:
* NotFound: if the queried key does not exist
* NotFound: if the queried key contains subkeys i.e. it's a prefix
* BucketRegionError: if the bucket does not exist
* Forbidden: if Head operation is not allows via IAM/ACLs
Co-authored-by: Cory Snider <corhere@gmail.com>
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
This allows to rewrite 'URLFor' of the storage driver to use a specific
host/trim the base path.
It is different from the 'redirect' middleware, as it still calls the
storage driver URLFor.
For example, with Azure storage provider, this allows to transform the
SAS Azure Blob Storage URL into the URL compatible with Azure Front
Door.
Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
With the current logic we only verifies the region and return if it's
empty; we were not validating the regionEndpoint parameter.
Signed-off-by: Ankur Kothiwal <ankur.kothiwal@cern.com>
Currently, the `forcepathstyle` parameter for the s3 storage driver is
considered only if the `regionendpoint` parameter is set. Since setting
a region endpoint explicitly is discouraged with AWS s3, it is not clear
how to enforce path style URLs with AWS s3.
This also means, that the default value (true) only applies if a region
endpoint is configured.
This change makes sure we always forward the `forcepathstyle` parameter
to the aws-sdk if present in the config. This is a breaking change where
a `regionendpoint` is configured but no explicit `forcepathstyle` value
is set.
Signed-off-by: Benjamin Schanzel <benjamin.schanzel@bmw.de>
This reverts https://github.com/distribution/distribution/pull/3556
This feature is currently broken and requires more fundamental changes
in the S3 driver. Until then it's better to remove it.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Unfortunately one of the changes we merged in broken the support for
http.ProxyFromEnvironment https://pkg.go.dev/net/http#ProxyFromEnvironment
This commit attempts to fix that by cloning the http.DefaultTransport
and updating it accordingly.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
This commit updates (writer).Writer() method in S3 storage driver to
handle the case where an append is attempted to a zer-size content.
S3 does not allow appending to already committed content, so we are
optiing to provide the following case as a narrowed down behaviour:
Writer can only append to zero byte content - in that case, a new S3
MultipartUpload is created that will be used for overriding the already
committed zero size content.
Appending to non-zero size content fails with error.
Co-authored-by: Cory Snider <corhere@gmail.com>
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
GCS storage driver used to be conditionally built due to its being
outdated and basically unmaintained. Recently the driver has gone
through a rework and updates. Let's remove the build tag so we have less
headaches dealing with it and try keeping it up to date.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
This linter both prevents parallel test races as well as
suggests parallel tests where appropriate:
See: https://github.com/moricho/tparallel
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
We make sure they're not hiding at the bottom or in the middle
which makes debugging an utter nightmare!
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
This commit refactors the GCS storage driver from the ground up and makes
it more consistent with the rest of the storage drivers.
We are also fixing GCS authentication using default app credentials:
When the default application credentials are used we don't initialize the
GCS storage client which then panics.
Co-authored-by: Cory Snider <corhere@gmail.com>
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
For some reason a PR we merged passed the build even though it was
missing various func parameters. This commmit fixes it.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Several storage drivers and storage middlewares need to introspect the
client HTTP request in order to construct content-redirect URLs. The
request is indirectly passed into the driver interface method URLFor()
through the context argument, which is bad practice. The request should
be passed in as an explicit argument as the method is only called from
request handlers.
Replace the URLFor() method with a RedirectURL() method which takes an
HTTP request as a parameter instead of a context. Drop the options
argument from URLFor() as in practice it only ever encoded the request
method, which can now be fetched directly from the request. No URLFor()
callers ever passed in an "expiry" option, either.
Signed-off-by: Cory Snider <csnider@mirantis.com>
The RemoteAddr and RemoteIP functions operate on *http.Request values,
not contexts. They have very low cohesion with the rest of the package.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Our context package predates the establishment of current best practices
regarding context usage and it shows. It encourages bad practices such
as using contexts to propagate non-request-scoped values like the
application version and using string-typed keys for context values. Move
the package internal to remove it from the API surface of
distribution/v3@v3.0.0 so we are free to iterate on it without being
constrained by compatibility.
Signed-off-by: Cory Snider <csnider@mirantis.com>
This commit make the S3 driver chunk size constants more straightforward
to understand -- instead of remembering the bit shifts we make this more
explicit.
We are also updating append parameter to the `(writer).Write` to follow
the new convention we are trying to establish.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
