Commit graph

402 commits

Author SHA1 Message Date
Milos Gajdos
7686bdc294
fix: fix broken build
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>
2023-11-02 23:23:11 -07:00
Milos Gajdos
bd0e476910
Hide our misuses of contexts from the public interface (#4128) 2023-11-03 05:05:19 +00:00
Milos Gajdos
1d7526dea0
cleanup: make chunk sizes easier to understand and change writer append (#4139) 2023-10-31 19:47:06 +00:00
Milos Gajdos
d153e1dc5b
cleanup: a small Azure driver cleanup (#4138) 2023-10-31 15:13:07 +00:00
Cory Snider
b4dc4f3474 storage/driver: plumb contexts into middlewares
Signed-off-by: Cory Snider <csnider@mirantis.com>
2023-10-27 17:48:57 -04:00
Cory Snider
b45b6d18b8 storage/driver: plumb contexts into factories
...and driver constructors when applicable.

Signed-off-by: Cory Snider <csnider@mirantis.com>
2023-10-27 17:48:57 -04:00
Cory Snider
f089932de0 storage/driver: replace URLFor method
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>
2023-10-27 10:58:37 -04:00
Cory Snider
9157226e7b Extract request utilities into its own package
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>
2023-10-27 10:58:37 -04:00
Cory Snider
d0f5aa670b Move context package internal
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>
2023-10-27 10:58:37 -04:00
Milos Gajdos
852de2c2bb
cleanup: make chunk sizes easier to understand and change writer append
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>
2023-10-27 10:57:54 +01:00
Milos Gajdos
e8e46b2195
cleanup: a small Azure driver cleanup
Just so we can make the code a bit more consistent with the rest of the
storage drivers.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-10-27 10:44:53 +01:00
Milos Gajdos
708bc6f3e9
Make S3 tests pass with MinIO (#4107) 2023-10-20 16:20:30 +01:00
Milos Gajdos
5aee8e1917
feat: Add context to storagedriver.(Filewriter).Commit() (#4109) 2023-10-19 11:41:55 +01:00
Milos Gajdos
cb0d083d8d
feat: Add context to storagedriver.(Filewriter).Commit()
This commit changes storagedriver.Filewriter interface
by adding context.Context as an argument to its Commit
func.

We pass the context appropriately where need be throughout
the distribution codebase to all the writers and tests.

S3 driver writer unfortunately must maintain the context
passed down to it from upstream so it contnues to
implement io.Writer and io.Closer interfaces which do not
allow accepting the context in any of their funcs.

Co-authored-by: Cory Snider <corhere@gmail.com>
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-10-19 11:27:27 +01:00
Milos Gajdos
ea41722902
refactor: Storage driver errors
Small refactoring of storagedriver errors.
We change the Enclosed field to Detail and make sure
Errors get properly serialized to JSON.
We also add tests.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-10-18 10:02:21 +01:00
James Hewitt
eac199875e
Remove test for nested file delete on S3
Nested files aren't supported on MinIO, and as our storage layout is
filesystem based, we don't actually use nest files in the code.

Remove the test so that we can support MinIO.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-10-17 09:13:15 +01:00
James Hewitt
647ec33c33
Bump minio version and test less storage classes
This fixes some of the tests for minio.

The walk tests needs a version of minio that contains https://github.com/minio/minio/pull/18099

The storage classes minio supports are a subset of the s3 classes.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-10-17 02:10:43 +01:00
Glyn Owen Hanmer
fee6faef70 json encode storage driver enclosed error
Signed-off-by: Glyn Owen Hanmer <1295698+glynternet@users.noreply.github.com>
2023-10-11 17:53:27 -06:00
Milos Gajdos
a70964c2fc
Merge pull request #4076 from flavianmissi/s3-loglevel
registry: add loglevel support for aws s3 storage driver
2023-10-04 14:13:15 +01:00
Flavian Missi
3df7e28f44 registry: add loglevel support for aws s3 storage driver
based on the work from
https://github.com/distribution/distribution/pull/3057.

Co-authored-by: Simon Compston <compston@gmail.com>
Signed-off-by: Flavian Missi <fmissi@redhat.com>
2023-10-02 15:47:02 +02:00
Milos Gajdos
735c161b53
Merge pull request #4066 from milosgajdos/optimise-s3-push
Optimise push in S3 driver
2023-09-29 13:47:20 +01:00
Milos Gajdos
4fce3c0028
Move completedParts type back to the original position
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-09-28 15:58:02 +01:00
Milos Gajdos
b888b14b39
Optimise push in S3 driver
This commit cleans up and attempts to optimise the performance of image push in S3 driver.
There are 2 main changes:
* we refactor the S3 driver Writer where instead of using separate bytes
  slices for ready and pending parts which get constantly appended data
  into them causing unnecessary allocations we use optimised bytes
  buffers; we make sure these are used efficiently when written to.
* we introduce a memory pool that is used for allocating the byte
  buffers introduced above

These changes should alleviate high memory pressure on the push path to S3.

Co-authored-by: Cory Snider <corhere@gmail.com>
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-09-27 21:33:22 +01:00
Neil Wilson
71c532e60c
driver testsuite: Add zero byte file checks
Add two new checks to the testsuite that check
the driver can handle zero byte files and appends to zero
byte files correctly

Signed-off-by: Neil Wilson <neil@aldur.co.uk>
2023-09-26 10:48:46 +01:00
Milos Gajdos
9790bc806c
Merge pull request #4037 from milosgajdos/enable-prealloc
Enable prealloc linter
2023-09-04 16:57:29 +01:00
Milos Gajdos
1089800643
Preallocate created slice in S3 tests
In case drvr.PutContent fails and returns error we'd have
some extra memory allocated, though in this case
(test with known size of the slice being iterated), that's fine.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-09-03 23:26:32 +01:00
Milos Gajdos
a9d31ec7b9
Avoid unnecessary type assertion in mfs driver
We already make sure the node in *dir

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-09-03 23:23:25 +01:00
Milos Gajdos
59fd8656ac
Enable prealloc linter
This will give us nice little performance gains in some code paths.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-09-03 22:41:51 +01:00
Milos Gajdos
dcdd8bb740
Propagate storage driver context to S3 API calls
Only some of the S3 storage driver calls were propagating context to the
S3 API calls. This commit updates the S3 storage drivers so the context
is propagated to all the S3 API calls.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-09-03 21:54:54 +01:00
James Hewitt
e22f7cbc73
Pass the last paging flag to storage drivers
Storage drivers may be able to take advantage of the hint to start
their walk more efficiently.

For S3: The API takes a start-after parameter. Registries with many
repositories can drastically reduce calls to s3 by telling s3 to only
list results lexographically after the last parameter.

For the fallback: We can start deeper in the tree and avoid statting
the files and directories before the hint in a walk. For a filesystem
this improves performance a little, but many of the API based drivers
are currently treated like a filesystem, so this drastically improves
the performance of GCP and Azure blob.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-08-29 11:27:42 +01:00
James Hewitt
1a3e73cb84
Handle rand deprecations in go 1.20
Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-08-28 09:33:12 +01:00
Milos Gajdos
59dd684cc8
Merge pull request #3713 from Jamstah/s3-tests 2023-08-21 13:48:43 +01:00
Sebastiaan van Stijn
5b3be39870
s3: add interface assertion
This was added for the other drivers in 6b388b1ba6,
but it missed the s3 storage driver.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-08-21 13:54:13 +02:00
Milos Gajdos
3dbfbc7255
Enable bodyclose linter
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-08-19 09:45:44 +01:00
Milos Gajdos
3f1859af26
Remove oss storage driver and alicdn storage driver middleware
This commit removes `oss` storage driver from distribution as well as
`alicdn` storage middleware which only works with the `oss` driver.

There are several reasons for it:
* no real-life expertise among the maintainers
* oss is compatible with S3 API operations required by S3 storage driver

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-08-16 08:39:20 +01:00
Milos Gajdos
65b57464f9
Merge pull request #3982 from milosgajdos/remove-swift-storage-driver
Remove SWIFT storage driver
2023-08-16 07:47:42 +01:00
James Hewitt
46ff5f8528
Fix Azure tests
The Azure tests fail if there is no Azure configuration available,
instead they should be skipped.

Also, one of the Azure tests is wrong and doesn't match the code.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-08-15 16:46:36 +01:00
James Hewitt
7622d0a453
Don't return the from of a walk
Other storage drivers will only return children and below, s3 should do
the same. The only reason it was returning was because of the addition
of a / to ensure we treat the from as a directory.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-08-15 16:26:37 +01:00
James Hewitt
f7bdd9127b
Don't test the OUTPOSTS storage class
This test will only work on an s3 bucket on an s3 outpost. Most
developers won't have access to one of these.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-08-15 16:18:52 +01:00
James Hewitt
6ceb904c3e
Don't check returned storage class if we use NONE
If we haven't set a storage class there's no point in checking the
storage class applied to the object - s3 will choose one.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-08-15 16:18:51 +01:00
James Hewitt
2d316a12d3
We don't use gocheck in these tests
Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-08-15 16:18:51 +01:00
James Hewitt
f78d81e78a
Remove test as S3 does not support empty directories
Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
2023-08-15 16:18:48 +01:00
Milos Gajdos
c6b9944ab1
Remove SWIFT storage driver
This commit removes swift storage driver from distribution.
There are several reasons for it:
* no real life expertise among the maintainers
* swift is compatible with S3 API operations required by S3 storage driver

This will also remove depedencies that are also hard to keep up with.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-08-15 09:14:11 +01:00
Milos Gajdos
d5c1b39b8b
Merge pull request #3206 from takmatsu/suppurt-path-in-middleware
Make redirect middleware can use path
2023-07-14 10:50:29 +01:00
Milos Gajdos
316e1c6b82
Get rid of unnecessary import alias
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-07-14 10:37:42 +01:00
MATSUMOTO TAKEAKI
a3eb956464
use path.Join() for building path
Signed-off-by: MATSUMOTO TAKEAKI <takeaki.matsumoto@linecorp.com>
2023-07-14 10:37:21 +01:00
MATSUMOTO TAKEAKI
a1cfd267c8
Make redirect middleware can use path
Signed-off-by: MATSUMOTO TAKEAKI <takeaki.matsumoto@linecorp.com>
2023-07-14 10:36:23 +01:00
Milos Gajdos
6b388b1ba6
Enable Go build tags
This enables go build tags so the GCS and OSS driver support is
available in the binary distributed via the image build by Dockerfile.

This led to quite a few fixes in the GCS and OSS packages raised as
warning by golang-ci linter.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-06-28 11:41:22 +01:00
Milos Gajdos
22725209e3
Merge pull request #3936 from flavianmissi/azure-path-not-found
Fix path not found error in Azure
2023-06-26 13:21:00 +01:00
Flavian Missi
2b72c4d1ca registry/storage/driver/azure: fix Move method
Something seems broken on azure/azure sdk side - it is currently not
possible to copy a blob of type AppendBlob using `CopyFromURL`.
Using the AppendBlob client via NewAppendBlobClient does not work
either.

According to Azure the correct way to do this is by using
StartCopyFromURL. Because this is an async operation, we need to do
polling ourselves. A simple backoff mechanism is used, where during each
iteration, the configured delay is multiplied by the retry number.

Also introduces two new config options for the Azure driver:
copy_status_poll_max_retry, and copy_status_poll_delay.

Signed-off-by: Flavian Missi <fmissi@redhat.com>
2023-06-26 13:47:30 +02:00