Commit Graph

45 Commits (tcl/master)

Author SHA1 Message Date
Eng Zer Jun ed5d493405
refactor: apply suggestions from code review
This commit apply the following suggestions:

	1. https://github.com/distribution/distribution/pull/4185#discussion_r1419694460
	2. https://github.com/distribution/distribution/pull/4185#discussion_r1419697921
	3. https://github.com/distribution/distribution/pull/4185#discussion_r1419699112
	4. https://github.com/distribution/distribution/pull/4185#discussion_r1419702609

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
2023-12-13 09:22:48 +00:00
Eng Zer Jun bcbf0431d1
testing: replace legacy `gopkg.in/check.v1`
This commit replaces the legacy `gopkg.in/check.v1` testing dependency
with `github.com/stretchr/testify`.

Closes https://github.com/distribution/distribution/issues/3884.

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
2023-12-13 09:22:43 +00:00
Milos Gajdos 7ce129d63b
feat(linter): enable errcheck linter in golangci-lint
Also, bump the linter version to the latest available version.

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
2023-11-18 07:19:24 +00:00
Milos Gajdos bd0e476910
Hide our misuses of contexts from the public interface (#4128) 2023-11-03 05:05:19 +00:00
Milos Gajdos d153e1dc5b
cleanup: a small Azure driver cleanup (#4138) 2023-10-31 15:13:07 +00: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
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 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 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
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 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
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
Flavian Missi 0c33bb1092 registry/storage/driver/azure: consider CannotVerifyCopySource as 404
Azure will return CannotVerifyCopySource with a 404 status code from a
call to Move when the source blob does not exist.
Details: https://learn.microsoft.com/en-us/rest/api/storageservices/blob-service-error-codes.

This fixes the TestMoveNonexistent test case for the Azure driver.

Signed-off-by: Flavian Missi <fmissi@redhat.com>
2023-05-30 15:12:08 +02:00
Flavian Missi d2e16fc74a registry/storage/driver/azure: fix driver parameters on tests
fixes "azure: no Host in request URL" tests failure, allowing azure
driver tests to run.

Signed-off-by: Flavian Missi <fmissi@redhat.com>
2023-05-30 15:12:08 +02:00
Flavian Missi 90ece48d77 registry/storage/driver: add test call to Stat on "/"
Stat(ctx, "/") is called by the registry healthcheck.
Also fixes blob name building in the Azure driver so it no longer
returns empty blob names. This was causing errors in the healthcheck
call to Stat for Azure.

Signed-off-by: Flavian Missi <fmissi@redhat.com>
2023-05-30 15:12:00 +02:00
Sebastiaan van Stijn 3fa6d5a33b
remove dot-imports for gopkg.in/check.v1
Dot-imports were only used in a couple of places, and replacing them
makes it more explicit what's imported.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-04-29 17:08:47 +02:00
Kirat Singh ba4a6bbe02 Update Azure SDK and support additional authentication schemes
Microsoft has updated the golang Azure SDK significantly.  Update the
azure storage driver to use the new SDK.  Add support for client
secret and MSI authentication schemes in addition to shared key
authentication.

Implement rootDirectory support for the azure storage driver to mirror
the S3 driver.

Signed-off-by: Kirat Singh <kirat.singh@beacon.io>

Co-authored-by: Cory Snider <corhere@gmail.com>
2023-04-25 17:23:20 +00:00
Sebastiaan van Stijn f8b3af78fc
replace deprecated io/ioutil
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-04 23:47:52 +01:00
Sebastiaan van Stijn e0281dc609
format code with gofumpt
gofumpt (https://github.com/mvdan/gofumpt) provides a supserset of `gofmt` / `go fmt`,
and addresses various formatting issues that linters may be checking for.

We can consider enabling the `gofumpt` linter to verify the formatting in CI, although
not every developer may have it installed, so for now this runs it once to get formatting
in shape.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-11-03 22:48:20 +01:00
Collin Shoop cf81f67a16 storagedriver/s3: Optimized Walk implementation + bugfix
Optimized S3 Walk impl by no longer listing files recursively. Overall gives a huge performance increase both in terms of runtime and S3 calls (up to ~500x).

Fixed a bug in WalkFallback where ErrSkipDir for was not handled as documented for non-directory.

Signed-off-by: Collin Shoop <cshoop@digitalocean.com>
2021-08-16 16:07:25 -04:00
Sebastiaan van Stijn 1d33874951
go.mod: change imports to github.com/distribution/distribution/v3
Go 1.13 and up enforce import paths to be versioned if a project
contains a go.mod and has released v2 or up.

The current v2.x branches (and releases) do not yet have a go.mod,
and therefore are still allowed to be imported with a non-versioned
import path (go modules add a `+incompatible` annotation in that case).

However, now that this project has a `go.mod` file, incompatible
import paths will not be accepted by go modules, and attempting
to use code from this repository will fail.

This patch uses `v3` for the import-paths (not `v2`), because changing
import paths itself is a breaking change, which means that  the
next release should increment the "major" version to comply with
SemVer (as go modules dictate).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-02-08 18:30:46 +01:00
Manish Tomar da8db4666b Fix gometalint errors
Signed-off-by: Manish Tomar <manish.tomar@docker.com>
2019-02-04 16:01:04 -08:00
Derek McGowan db0a4ec1c8
Enable static checks
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
2018-08-06 14:49:11 -07:00
Yu Wang 62797237b9 closes #2496 and #2552
Update Azure SDK with release v16.2.1
Update Azure autorest SDK with release v10.8.1

Signed-off-by: Yu Wang <yuwa@microsoft.com>
2018-06-08 16:53:02 -07:00
Sargun Dhillon 32ac467992 Introduce Walk Method Per Storage Driver
Move the Walk types into registry/storage/driver, and add a Walk method to each
storage driver. Although this is yet another API to implement, there is a fall
back implementation that relies on List and Stat. For some filesystems this is
very slow.

Also, this WalkDir Method conforms better do a traditional WalkDir (a la filepath).

This change is in preparation for refactoring.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
2018-01-07 22:45:17 -08:00
Stephen J Day 9c88801a12
context: remove definition of Context
Back in the before time, the best practices surrounding usage of Context
weren't quite worked out. We defined our own type to make usage easier.
As this packaged was used elsewhere, it make it more and more
challenging to integrate with the forked `Context` type. Now that it is
available in the standard library, we can just use that one directly.

To make usage more consistent, we now use `dcontext` when referring to
the distribution context package.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
2017-08-11 15:53:31 -07:00
Yu Wang (UC) ac05d143d8 closes #2224: re-vendor the latest Azure Storage SDK for better performance
Signed-off-by: Yu Wang <yuwa@microsoft.com>
2017-04-14 14:20:18 -07:00
Ahmet Alp Balkan 0a1ce58e2c
azure: revendor + remove hacky solution in is404
Removing the temporary workaround in is404() method by re-vendoring
the azure-sdk-for-go.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
2017-01-09 17:22:28 -08:00
Ahmet Alp Balkan 78d0660319
azure: fix race condition in PutContent()
See #2077 for background.

The PR #1438 which was not reviewed by azure folks basically introduced
a race condition around uploads to the same blob by multiple clients
concurrently as it used the "writer" type for PutContent(), introduced in #1438.
This does chunked upload of blobs using "AppendBlob" type, which was not atomic.

Usage of "writer" type and thus AppendBlobs on metadata files is currently not
concurrency-safe and generally, they are not the right type of blob for the job.

This patch fixes PutContent() to use the atomic upload operation that works
for uploads smaller than 64 MB and creates blobs with "BlockBlob" type. To be
backwards compatible, we query the type of the blob first and if it is not
a "BlockBlob" we delete the blob first before doing an atomic PUT. This
creates a small inconsistency/race window "only once". Once the blob is made
"BlockBlob", it is overwritten with a single PUT atomicallly next time.

Therefore, going forward, PutContent() will be producing BlockBlobs and it
will silently migrate the AppendBlobs introduced in #1438 to BlockBlobs with
this patch.

Tested with existing code side by side, both registries with and without this
patch work fine without breaking each other. So this should be good from a
backwards/forward compatiblity perspective, with a cost of doing an extra
HEAD checking the blob type.

Fixes #2077.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
2016-11-30 12:40:43 -08:00
Derek McGowan a2611c7520 Merge pull request #2027 from ahmetalpbalkan/pr-azure-memleak2
Update vendored azure-sdk-for-go
2016-11-04 10:08:40 -07:00
Ahmet Alp Balkan 2ab25288a2
Update vendored azure-sdk-for-go
Updating to a recent version of Azure Storage SDK to be
able to patch some memory leaks through configurable HTTP client
changes which were made possible by recent patches to it.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
2016-11-03 13:24:57 -07:00
Ahmet Alp Balkan 6d2a0bafcd
storagedriver/azure: close leaking response body
In GetContent() we read the bytes from a blob but do not close
the underlying response body.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
2016-10-28 15:13:22 -07:00
Richard Scothern 772e6f4057 Update missing blob error checking with latest Azure API
Signed-off-by: Richard Scothern <richard.scothern@docker.com>
2016-03-14 15:59:03 -07:00
Brian Bland ff03381d49 Adds new storagedriver.FileWriter interface
Updates registry storage code to use this for better resumable writes.
Implements this interface for the following drivers:
 + Inmemory
 + Filesystem
 + S3
 + Azure

Signed-off-by: Brian Bland <brian.bland@docker.com>
2016-03-08 16:37:44 -08:00
Stephen J Day d3d4423ff7 Remove half-baked Storage Driver IPC support
This removes documentation and code related to IPC based storage driver
plugins. The existence of this functionality was an original feature goal but
is now not maintained and actively confusing incoming contributions. We will
likely explore some driver plugin mechanism in the future but we don't need
this laying around in the meantime.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
2015-06-29 16:52:12 -07:00
Ahmet Alp Balkan daa22cacba storage/driver/azure: Update vendored Azure SDK
This change refreshes the updated version of Azure SDK
for Go that has the latest changes.

I manually vendored the new SDK (github.com/Azure/azure-sdk-for-go)
and I removed `management/` `core/` packages manually simply because
they're not used here and they have a fork of `net/http` and `crypto/tls`
for a particular reason. It was introducing a 44k SLOC change otherwise...

This also undoes the `include_azure` flag (actually Steven removed the
driver from imports but forgot to add the build flag apparently, so the
flag wasn't really including azure. 😄 ). This also must be obsolete
now.

Fixes #620, #175.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
2015-06-16 17:13:44 -07:00
Richard ae216e365a Make Storage Driver API calls context aware.
- Change driver interface to take a context as its first argument
     - Make newFileReader take a context as its first argument
     - Make newFileWriter take a context as its first argument
     - Make blobstore exists and delete take a context as a first argument
     - Pass the layerreader's context to the storage layer
     - Pass the app's context to purgeuploads
     - Store the app's context into the blobstore (was previously null)
     - Pass the trace'd context to the storage drivers

Signed-off-by: Richard Scothern <richard.scothern@gmail.com>
2015-04-27 15:58:58 -07:00
Stephen J Day 9e146437e4 Require storage drivers to report their name
Signed-off-by: Stephen J Day <stephen.day@docker.com>
2015-04-22 17:30:01 -07:00
Stephen Day 02b8c7b8cf Merge pull request #294 from fkautz/pr_out_updating_msopentech_azure_sdk_for_go_to_latest_master
Updating MSOpenTech/azure-sdk-for-go to latest master
2015-03-24 13:27:19 -07:00
Frederick F. Kautz IV 11db8185bc Updating MSOpenTech/azure-sdk-for-go to latest master
Signed-off-by: Frederick F. Kautz IV <fkautz@alumni.cmu.edu>
2015-03-23 21:59:21 -07:00
Ahmet Alp Balkan ce9f500a31 storage/driver/azure: Allow non-default realms
This enables Azure storage driver to be used with non-default
cloud endpoints like Azure China or Azure Government that does
not use `.blob.core.windows.net` FQDN suffix.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
2015-03-23 20:41:16 -07:00
Stephen J Day 41b1e22f97 doc: move storage driver readmes into docs
Signed-off-by: Stephen J Day <stephen.day@docker.com>
2015-03-05 17:23:33 -08:00
Ahmet Alp Balkan 76f1abfa07 storagedriver/azure: Add README
Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
2015-02-11 14:40:51 -08:00
Stephen J Day 65b0d73cb7 Move storagedriver package to registry/storage/driver
This change is slightly more complex than previous package maves in that the
package name changed. To address this, we simply always reference the package
driver as storagedriver to avoid compatbility issues with existing code. While
unfortunate, this can be cleaned up over time.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
2015-02-11 12:43:04 -08:00