From 2548973b1d22e2f1de9b708f90218be8c4efd1b8 Mon Sep 17 00:00:00 2001 From: Milos Gajdos Date: Wed, 28 Jun 2023 11:41:22 +0100 Subject: [PATCH 1/3] 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 (cherry picked from commit 6b388b1ba604bf5b970bbb41878f97cc3fc93376) Signed-off-by: Sebastiaan van Stijn --- BUILDING.md | 2 +- Dockerfile | 4 +-- registry/storage/driver/gcs/gcs.go | 33 ++++++++++++------------- registry/storage/driver/gcs/gcs_test.go | 8 +++++- registry/storage/driver/oss/oss.go | 14 ++++------- registry/storage/driver/oss/oss_test.go | 6 +++++ 6 files changed, 37 insertions(+), 30 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index 2981d016..4c43b03c 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -114,4 +114,4 @@ the registry binary generated in the "./bin" directory: ### Optional build tags Optional [build tags](http://golang.org/pkg/go/build/) can be provided using -the environment variable `DOCKER_BUILDTAGS`. +the environment variable `BUILDTAGS`. diff --git a/Dockerfile b/Dockerfile index d6c19395..42b87c06 100644 --- a/Dockerfile +++ b/Dockerfile @@ -22,12 +22,12 @@ RUN --mount=target=. \ FROM base AS build ARG TARGETPLATFORM ARG LDFLAGS="-s -w" -ARG BUILDTAGS="include_oss include_gcs" +ARG BUILDTAGS="include_oss,include_gcs" RUN --mount=type=bind,target=/go/src/github.com/docker/distribution,rw \ --mount=type=cache,target=/root/.cache/go-build \ --mount=target=/go/pkg/mod,type=cache \ --mount=type=bind,source=/tmp/.ldflags,target=/tmp/.ldflags,from=version \ - set -x ; xx-go build -trimpath -ldflags "$(cat /tmp/.ldflags) ${LDFLAGS}" -tags="${BUILDTAGS}" -o /usr/bin/registry ./cmd/registry \ + set -x ; xx-go build -tags "${BUILDTAGS}" -trimpath -ldflags "$(cat /tmp/.ldflags) ${LDFLAGS}" -o /usr/bin/registry ./cmd/registry \ && xx-verify --static /usr/bin/registry FROM scratch AS binary diff --git a/registry/storage/driver/gcs/gcs.go b/registry/storage/driver/gcs/gcs.go index 66d0d1ad..2a6619f5 100644 --- a/registry/storage/driver/gcs/gcs.go +++ b/registry/storage/driver/gcs/gcs.go @@ -1,18 +1,17 @@ +//go:build include_gcs +// +build include_gcs + // Package gcs provides a storagedriver.StorageDriver implementation to // store blobs in Google cloud storage. // // This package leverages the google.golang.org/cloud/storage client library -//for interfacing with gcs. +// for interfacing with gcs. // // Because gcs is a key, value store the Stat call does not support last modification // time for directories (directories are an abstraction for key, value stores) // // Note that the contents of incomplete uploads are not accessible even though // Stat returns their length -// -//go:build include_gcs -// +build include_gcs - package gcs import ( @@ -62,7 +61,6 @@ var rangeHeader = regexp.MustCompile(`^bytes=([0-9])+-([0-9]+)$`) // driverParameters is a struct that encapsulates all of the driver parameters after all values have been set type driverParameters struct { bucket string - config *jwt.Config email string privateKey []byte client *http.Client @@ -88,6 +86,8 @@ func (factory *gcsDriverFactory) Create(parameters map[string]interface{}) (stor return FromParameters(parameters) } +var _ storagedriver.StorageDriver = &driver{} + // driver is a storagedriver.StorageDriver implementation backed by GCS // Objects are stored at absolute keys in the provided bucket. type driver struct { @@ -298,7 +298,7 @@ func (d *driver) Reader(context context.Context, path string, offset int64) (io. if err != nil { return nil, err } - if offset == int64(obj.Size) { + if offset == obj.Size { return ioutil.NopCloser(bytes.NewReader([]byte{})), nil } return nil, storagedriver.InvalidOffsetError{Path: path, Offset: offset} @@ -434,7 +434,6 @@ func putContentsClose(wc *storage.Writer, contents []byte) error { } } if err != nil { - wc.CloseWithError(err) return err } return wc.Close() @@ -614,10 +613,10 @@ func (d *driver) Stat(context context.Context, path string) (storagedriver.FileI //try to get as folder dirpath := d.pathToDirKey(path) - var query *storage.Query - query = &storage.Query{} - query.Prefix = dirpath - query.MaxResults = 1 + query := &storage.Query{ + Prefix: dirpath, + MaxResults: 1, + } objects, err := storageListObjects(gcsContext, d.bucket, query) if err != nil { @@ -639,12 +638,12 @@ func (d *driver) Stat(context context.Context, path string) (storagedriver.FileI } // List returns a list of the objects that are direct descendants of the -//given path. +// given path. func (d *driver) List(context context.Context, path string) ([]string, error) { - var query *storage.Query - query = &storage.Query{} - query.Delimiter = "/" - query.Prefix = d.pathToDirKey(path) + query := &storage.Query{ + Delimiter: "/", + Prefix: d.pathToDirKey(path), + } list := make([]string, 0, 64) for { objects, err := storageListObjects(d.context(context), d.bucket, query) diff --git a/registry/storage/driver/gcs/gcs_test.go b/registry/storage/driver/gcs/gcs_test.go index 4ae9aa3f..ee4a67a8 100644 --- a/registry/storage/driver/gcs/gcs_test.go +++ b/registry/storage/driver/gcs/gcs_test.go @@ -59,7 +59,7 @@ func init() { panic(fmt.Sprintf("Error reading JWT config : %s", err)) } email = jwtConfig.Email - privateKey = []byte(jwtConfig.PrivateKey) + privateKey = jwtConfig.PrivateKey if len(privateKey) == 0 { panic("Error reading JWT config : missing private_key property") } @@ -260,6 +260,9 @@ func TestEmptyRootList(t *testing.T) { } }() keys, err := emptyRootDriver.List(ctx, "/") + if err != nil { + t.Fatalf("unexpected error listing empty root content: %v", err) + } for _, path := range keys { if !storagedriver.PathRegexp.MatchString(path) { t.Fatalf("unexpected string in path: %q != %q", path, storagedriver.PathRegexp) @@ -267,6 +270,9 @@ func TestEmptyRootList(t *testing.T) { } keys, err = slashRootDriver.List(ctx, "/") + if err != nil { + t.Fatalf("unexpected error listing slash root content: %v", err) + } for _, path := range keys { if !storagedriver.PathRegexp.MatchString(path) { t.Fatalf("unexpected string in path: %q != %q", path, storagedriver.PathRegexp) diff --git a/registry/storage/driver/oss/oss.go b/registry/storage/driver/oss/oss.go index 8738b1e0..55f6edc0 100644 --- a/registry/storage/driver/oss/oss.go +++ b/registry/storage/driver/oss/oss.go @@ -1,3 +1,6 @@ +//go:build include_oss +// +build include_oss + // Package oss provides a storagedriver.StorageDriver implementation to // store blobs in Aliyun OSS cloud storage. // @@ -6,10 +9,6 @@ // // Because OSS is a key, value store the Stat call does not support last modification // time for directories (directories are an abstraction for key, value stores) -// -//go:build include_oss -// +build include_oss - package oss import ( @@ -68,6 +67,8 @@ func (factory *ossDriverFactory) Create(parameters map[string]interface{}) (stor return FromParameters(parameters) } +var _ storagedriver.StorageDriver = &driver{} + type driver struct { Client *oss.Client Bucket *oss.Bucket @@ -498,11 +499,6 @@ func parseError(path string, err error) error { return err } -func hasCode(err error, code string) bool { - ossErr, ok := err.(*oss.Error) - return ok && ossErr.Code == code -} - func (d *driver) getOptions() oss.Options { return oss.Options{ServerSideEncryption: d.Encrypt} } diff --git a/registry/storage/driver/oss/oss_test.go b/registry/storage/driver/oss/oss_test.go index e042bb75..aedca987 100644 --- a/registry/storage/driver/oss/oss_test.go +++ b/registry/storage/driver/oss/oss_test.go @@ -128,6 +128,9 @@ func TestEmptyRootList(t *testing.T) { defer rootedDriver.Delete(ctx, filename) keys, err := emptyRootDriver.List(ctx, "/") + if err != nil { + t.Fatalf("unexpected error listing empty root content: %v", err) + } for _, path := range keys { if !storagedriver.PathRegexp.MatchString(path) { t.Fatalf("unexpected string in path: %q != %q", path, storagedriver.PathRegexp) @@ -135,6 +138,9 @@ func TestEmptyRootList(t *testing.T) { } keys, err = slashRootDriver.List(ctx, "/") + if err != nil { + t.Fatalf("unexpected error listing slash root content: %v", err) + } for _, path := range keys { if !storagedriver.PathRegexp.MatchString(path) { t.Fatalf("unexpected string in path: %q != %q", path, storagedriver.PathRegexp) From 2d62a4027add9ed41920784cf10bac0b6486a84b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 21 Aug 2023 13:54:13 +0200 Subject: [PATCH 2/3] s3: add interface assertion This was added for the other drivers in 6b388b1ba604bf5b970bbb41878f97cc3fc93376, but it missed the s3 storage driver. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 5b3be398701517100f417b4949bba6b7c824a7df) Signed-off-by: Sebastiaan van Stijn --- registry/storage/driver/s3-aws/s3.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/registry/storage/driver/s3-aws/s3.go b/registry/storage/driver/s3-aws/s3.go index 77b821f8..5f2dd753 100644 --- a/registry/storage/driver/s3-aws/s3.go +++ b/registry/storage/driver/s3-aws/s3.go @@ -137,6 +137,8 @@ func (factory *s3DriverFactory) Create(parameters map[string]interface{}) (stora return FromParameters(parameters) } +var _ storagedriver.StorageDriver = &driver{} + type driver struct { S3 *s3.S3 Bucket string From 110cb7538dc91e18fdf59893d9274e81cbba7201 Mon Sep 17 00:00:00 2001 From: Milos Gajdos Date: Fri, 19 May 2023 17:51:37 +0100 Subject: [PATCH 3/3] Enable build tags in 2.8 It would appear we were missing the Go build tags on 2.8.X branch so the images would not have the necessary support for some storage drivers causing breakages to end users trying to use them. This commit fixes both the build and linting issues. Signed-off-by: Milos Gajdos --- .github/workflows/ci.yml | 2 +- Makefile | 2 +- registry/storage/driver/oss/oss.go | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ae866df9..337f1e81 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,7 +9,7 @@ jobs: build: runs-on: ubuntu-latest env: - DOCKER_BUILDTAGS: "include_oss include_gcs" + BUILDTAGS: "include_oss,include_gcs" CGO_ENABLED: 1 GO111MODULE: "auto" GOPATH: ${{ github.workspace }} diff --git a/Makefile b/Makefile index 75e11820..dcdbcb54 100644 --- a/Makefile +++ b/Makefile @@ -50,7 +50,7 @@ version/version.go: check: ## run all linters (TODO: enable "unused", "varcheck", "ineffassign", "unconvert", "staticheck", "goimports", "structcheck") @echo "$(WHALE) $@" - @GO111MODULE=off golangci-lint run + @GO111MODULE=off golangci-lint --build-tags "${BUILDTAGS}" run test: ## run tests, except integration test with test.short @echo "$(WHALE) $@" diff --git a/registry/storage/driver/oss/oss.go b/registry/storage/driver/oss/oss.go index 55f6edc0..dfe6fd96 100644 --- a/registry/storage/driver/oss/oss.go +++ b/registry/storage/driver/oss/oss.go @@ -37,12 +37,11 @@ const driverName = "oss" const minChunkSize = 5 << 20 const defaultChunkSize = 2 * minChunkSize -const defaultTimeout = 2 * time.Minute // 2 minute timeout per chunk // listMax is the largest amount of objects you can request from OSS in a list call const listMax = 1000 -//DriverParameters A struct that encapsulates all of the driver parameters after all values have been set +// DriverParameters A struct that encapsulates all of the driver parameters after all values have been set type DriverParameters struct { AccessKeyID string AccessKeySecret string