Skip to content

Allow configuration of s3 region for registry backend #10200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions components/content-service-api/go/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ type MinIOConfig struct {

Region string `json:"region"`
ParallelUpload uint `json:"parallelUpload,omitempty"`

BucketName string `json:"bucket"`
}

type PProf struct {
Expand Down
3 changes: 2 additions & 1 deletion components/content-service/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/golang/mock v1.6.0
github.com/google/go-cmp v0.5.8
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
github.com/minio/minio-go/v7 v7.0.11
github.com/minio/minio-go/v7 v7.0.26
github.com/opencontainers/go-digest v1.0.0
github.com/opencontainers/image-spec v1.0.2
github.com/opentracing/opentracing-go v1.2.0
Expand Down Expand Up @@ -65,6 +65,7 @@ require (
github.com/heptiolabs/healthcheck v0.0.0-20211123025425-613501dd5deb // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/klauspost/compress v1.13.5 // indirect
github.com/klauspost/cpuid v1.3.1 // indirect
github.com/magiconair/properties v1.8.5 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
Expand Down
8 changes: 4 additions & 4 deletions components/content-service/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,8 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
github.com/klauspost/compress v1.11.3/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
github.com/klauspost/compress v1.11.13/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
github.com/klauspost/compress v1.13.5 h1:9O69jUPDcsT9fEm74W92rZL9FQY7rCdaXVneq+yyzl4=
github.com/klauspost/compress v1.13.5/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk=
github.com/klauspost/cpuid v1.2.3/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek=
github.com/klauspost/cpuid v1.3.1 h1:5JNjFYYQrZeKRJ0734q51WCEEn2huer72Dc7K+R/b6s=
github.com/klauspost/cpuid v1.3.1/go.mod h1:bYW4mA6ZgKPob1/Dlai2LviZJO7KGI3uoWLd42rAQw4=
Expand Down Expand Up @@ -589,8 +591,8 @@ github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3N
github.com/miekg/pkcs11 v1.0.3/go.mod h1:XsNlhZGX73bx86s2hdc/FuaLm2CPZJemRLMA+WTFxgs=
github.com/minio/md5-simd v1.1.0 h1:QPfiOqlZH+Cj9teu0t9b1nTBfPbyTl16Of5MeuShdK4=
github.com/minio/md5-simd v1.1.0/go.mod h1:XpBqgZULrMYD3R+M28PcmP0CkI7PEMzB3U77ZrKZ0Gw=
github.com/minio/minio-go/v7 v7.0.11 h1:7utSkCtMQPYYB1UB8FR3d0QSiOWE6F/JYXon29imYek=
github.com/minio/minio-go/v7 v7.0.11/go.mod h1:WoyW+ySKAKjY98B9+7ZbI8z8S3jaxaisdcvj9TGlazA=
github.com/minio/minio-go/v7 v7.0.26 h1:D0HK+8793etZfRY/vHhDmFaP+vmT41K3K4JV9vmZCBQ=
github.com/minio/minio-go/v7 v7.0.26/go.mod h1:x81+AX5gHSfCSqw7jxRKHvxUXMlE5uKX0Vb75Xk5yYg=
github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKUJU=
github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=
github.com/mistifyio/go-zfs v2.1.2-0.20190413222219-f784269be439+incompatible/go.mod h1:8AuVvqP/mXw1px98n46wfvcGfQ4ci2FwoAjKYxuo3Z4=
Expand Down Expand Up @@ -891,7 +893,6 @@ golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8U
golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200709230013-948cd5f35899/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
Expand Down Expand Up @@ -1405,7 +1406,6 @@ gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2/go.mod h1:Xk6kEKp8OKb+X14hQBKW
gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc=
gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=
gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/ini.v1 v1.57.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/ini.v1 v1.62.0 h1:duBzk771uxoUuOlyRLkHsygud9+5lrlGjdFBb4mSKDU=
gopkg.in/ini.v1 v1.62.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k=
Expand Down
4 changes: 2 additions & 2 deletions components/content-service/pkg/layer/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,11 @@ func (s *testStorage) DeleteBucket(ctx context.Context, bucket string) error {
return nil
}

func (*testStorage) BackupObject(workspaceID string, name string) string {
func (*testStorage) BackupObject(ownerID string, workspaceID string, name string) string {
return ""
}

func (*testStorage) InstanceObject(workspaceID string, instanceID string, name string) string {
func (*testStorage) InstanceObject(ownerID string, workspaceID string, instanceID string, name string) string {
return ""
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (ls *HeadlessLogService) LogDownloadURL(ctx context.Context, req *api.LogDo
span.SetTag("instanceId", req.InstanceId)
defer tracing.FinishSpan(span, &err)

blobName := ls.s.InstanceObject(req.WorkspaceId, req.InstanceId, logs.UploadedHeadlessLogPath(req.TaskId))
blobName := ls.s.InstanceObject(req.OwnerId, req.WorkspaceId, req.InstanceId, logs.UploadedHeadlessLogPath(req.TaskId))
info, err := ls.s.SignDownload(ctx, ls.s.Bucket(req.OwnerId), blobName, &storage.SignedURLOptions{})
if err != nil {
log.WithFields(log.OWI(req.OwnerId, req.WorkspaceId, "")).
Expand Down Expand Up @@ -87,7 +87,7 @@ func (ls *HeadlessLogService) ListLogs(ctx context.Context, req *api.ListLogsReq
// we do not need to check whether the bucket exists because ListObjects() does that for us

// all files under this prefix are headless log files, named after their respective taskId
prefix := ls.s.InstanceObject(req.WorkspaceId, req.InstanceId, logs.UploadedHeadlessLogPathPrefix)
prefix := ls.s.InstanceObject(req.OwnerId, req.WorkspaceId, req.InstanceId, logs.UploadedHeadlessLogPathPrefix)
objects, err := da.ListObjects(ctx, prefix)
if err != nil {
return nil, err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestListLogs(t *testing.T) {
daFactory: daFactory,
}

s.EXPECT().InstanceObject(gomock.Any(), gomock.Any(), gomock.Any()).
s.EXPECT().InstanceObject(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
Return("")
da.EXPECT().Init(gomock.Any(), gomock.Eq(OwnerId), gomock.Eq(WorkspaceId), gomock.Not(gomock.Eq(""))).
Times(1)
Expand Down
10 changes: 5 additions & 5 deletions components/content-service/pkg/service/workspace-service.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (cs *WorkspaceService) WorkspaceDownloadURL(ctx context.Context, req *api.W
span.SetTag("workspaceId", req.WorkspaceId)
defer tracing.FinishSpan(span, &err)

blobName := cs.s.BackupObject(req.WorkspaceId, storage.DefaultBackup)
blobName := cs.s.BackupObject(req.OwnerId, req.WorkspaceId, storage.DefaultBackup)

info, err := cs.s.SignDownload(ctx, cs.s.Bucket(req.OwnerId), blobName, &storage.SignedURLOptions{})
if err != nil {
Expand Down Expand Up @@ -73,7 +73,7 @@ func (cs *WorkspaceService) DeleteWorkspace(ctx context.Context, req *api.Delete
defer tracing.FinishSpan(span, &err)

if req.IncludeSnapshots {
prefix := cs.s.BackupObject(req.WorkspaceId, "")
prefix := cs.s.BackupObject(req.OwnerId, req.WorkspaceId, "")
if !strings.HasSuffix(prefix, "/") {
prefix = prefix + "/"
}
Expand All @@ -89,7 +89,7 @@ func (cs *WorkspaceService) DeleteWorkspace(ctx context.Context, req *api.Delete
return &api.DeleteWorkspaceResponse{}, nil
}

blobName := cs.s.BackupObject(req.WorkspaceId, storage.DefaultBackup)
blobName := cs.s.BackupObject(req.OwnerId, req.WorkspaceId, storage.DefaultBackup)
err = cs.s.DeleteObject(ctx, cs.s.Bucket(req.OwnerId), &storage.DeleteObjectQuery{Name: blobName})
if err != nil {
if errors.Is(err, storage.ErrNotFound) {
Expand All @@ -100,7 +100,7 @@ func (cs *WorkspaceService) DeleteWorkspace(ctx context.Context, req *api.Delete
return nil, status.Error(codes.Unknown, err.Error())
}

trailPrefix := cs.s.BackupObject(req.WorkspaceId, "trail-")
trailPrefix := cs.s.BackupObject(req.OwnerId, req.WorkspaceId, "trail-")
err = cs.s.DeleteObject(ctx, cs.s.Bucket(req.OwnerId), &storage.DeleteObjectQuery{Prefix: trailPrefix})
if err != nil {
if errors.Is(err, storage.ErrNotFound) {
Expand All @@ -121,7 +121,7 @@ func (cs *WorkspaceService) WorkspaceSnapshotExists(ctx context.Context, req *ap
span.SetTag("filename", req.Filename)
defer tracing.FinishSpan(span, &err)

exists, err := cs.s.ObjectExists(ctx, cs.s.Bucket(req.OwnerId), cs.s.BackupObject(req.WorkspaceId, req.Filename))
exists, err := cs.s.ObjectExists(ctx, cs.s.Bucket(req.OwnerId), cs.s.BackupObject(req.OwnerId, req.WorkspaceId, req.Filename))
if err != nil {
return nil, status.Error(codes.Unknown, err.Error())
}
Expand Down
6 changes: 3 additions & 3 deletions components/content-service/pkg/storage/gcloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -870,11 +870,11 @@ func (p *PresignedGCPStorage) ObjectExists(ctx context.Context, bucket, obj stri
}

// BackupObject returns a backup's object name that a direct downloader would download
func (p *PresignedGCPStorage) BackupObject(workspaceID string, name string) string {
func (p *PresignedGCPStorage) BackupObject(ownerID string, workspaceID string, name string) string {
return fmt.Sprintf("workspaces/%s", gcpWorkspaceBackupObjectName(workspaceID, name))
}

// InstanceObject returns a instance's object name that a direct downloader would download
func (p *PresignedGCPStorage) InstanceObject(workspaceID string, instanceID string, name string) string {
return p.BackupObject(workspaceID, InstanceObjectName(instanceID, name))
func (p *PresignedGCPStorage) InstanceObject(ownerID string, workspaceID string, instanceID string, name string) string {
return p.BackupObject(ownerID, workspaceID, InstanceObjectName(instanceID, name))
}
36 changes: 25 additions & 11 deletions components/content-service/pkg/storage/minio.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"io"
"net/http"
"os"
"path/filepath"
"strings"
"time"

Expand Down Expand Up @@ -129,6 +130,7 @@ func (rs *DirectMinIOStorage) Init(ctx context.Context, owner, workspace, instan
rs.Username = owner
rs.WorkspaceName = workspace
rs.InstanceID = instance

err = rs.Validate()
if err != nil {
return err
Expand Down Expand Up @@ -310,17 +312,21 @@ func (rs *DirectMinIOStorage) Upload(ctx context.Context, source string, name st
return
}

func minioBucketName(ownerID string) string {
func minioBucketName(ownerID, bucketName string) string {
if bucketName != "" {
return bucketName
}

return fmt.Sprintf("gitpod-user-%s", ownerID)
}

func minioWorkspaceBackupObjectName(workspaceID string, name string) string {
return fmt.Sprintf("workspaces/%s/%s", workspaceID, name)
func minioWorkspaceBackupObjectName(ownerID, workspaceID, name string) string {
return filepath.Join(ownerID, "workspaces", workspaceID, name)
}

// Bucket provides the bucket name for a particular user
func (rs *DirectMinIOStorage) Bucket(ownerID string) string {
return minioBucketName(ownerID)
return minioBucketName(ownerID, rs.MinIOConfig.BucketName)
}

// BackupObject returns a backup's object name that a direct downloader would download
Expand All @@ -329,11 +335,15 @@ func (rs *DirectMinIOStorage) BackupObject(name string) string {
}

func (rs *DirectMinIOStorage) bucketName() string {
return minioBucketName(rs.Username)
return minioBucketName(rs.Username, rs.MinIOConfig.BucketName)
}

func (rs *DirectMinIOStorage) objectName(name string) string {
return minioWorkspaceBackupObjectName(rs.WorkspaceName, name)
var username string
if rs.MinIOConfig.BucketName != "" {
username = rs.Username
}
return minioWorkspaceBackupObjectName(username, rs.WorkspaceName, name)
}

func newPresignedMinIOAccess(cfg config.MinIOConfig) (*presignedMinIOStorage, error) {
Expand Down Expand Up @@ -517,7 +527,7 @@ func annotationToAmzMetaHeader(annotation string) string {

// Bucket provides the bucket name for a particular user
func (s *presignedMinIOStorage) Bucket(ownerID string) string {
return minioBucketName(ownerID)
return minioBucketName(ownerID, s.MinIOConfig.BucketName)
}

// BlobObject returns a blob's object name
Expand All @@ -526,13 +536,17 @@ func (s *presignedMinIOStorage) BlobObject(name string) (string, error) {
}

// BackupObject returns a backup's object name that a direct downloader would download
func (s *presignedMinIOStorage) BackupObject(workspaceID string, name string) string {
return minioWorkspaceBackupObjectName(workspaceID, name)
func (s *presignedMinIOStorage) BackupObject(ownerID string, workspaceID, name string) string {
var username string
if s.MinIOConfig.BucketName != "" {
username = ownerID
}
return minioWorkspaceBackupObjectName(username, workspaceID, name)
}

// InstanceObject returns a instance's object name that a direct downloader would download
func (s *presignedMinIOStorage) InstanceObject(workspaceID string, instanceID string, name string) string {
return s.BackupObject(workspaceID, InstanceObjectName(instanceID, name))
func (s *presignedMinIOStorage) InstanceObject(ownerID string, workspaceID string, instanceID string, name string) string {
return s.BackupObject(ownerID, workspaceID, InstanceObjectName(instanceID, name))
}

func translateMinioError(err error) error {
Expand Down
132 changes: 132 additions & 0 deletions components/content-service/pkg/storage/minio_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
// Licensed under the GNU Affero General Public License (AGPL).
// See License-AGPL.txt in the project root for license information.

package storage

import (
"context"
"testing"

config "github.com/gitpod-io/gitpod/content-service/api/config"
)

func TestMinioBucketName(t *testing.T) {
tests := []struct {
Name string
BucketNameConfig string
OwnerID string
ExpectedBucket string
}{
{
Name: "no dedicated bucket",
BucketNameConfig: "",
OwnerID: "fake-owner-id",
ExpectedBucket: "gitpod-user-fake-owner-id",
},
{
Name: "with dedicated bucket",
BucketNameConfig: "root-bucket",
OwnerID: "fake-owner-id",
ExpectedBucket: "root-bucket",
},
}

for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
cfg := config.MinIOConfig{
Endpoint: "fake",
AccessKeyID: "fake_access_key",
SecretAccessKey: "fake_secret",
Region: "none",
BucketName: test.BucketNameConfig,
}
minio, err := newDirectMinIOAccess(cfg)
if err != nil {
t.Fatalf("failed to create minio access: '%v'", err)
}

actualBucketName := minio.Bucket(test.OwnerID)
if actualBucketName != test.ExpectedBucket {
t.Fatalf("[minio] unexpected bucket name: is '%s' but expected '%s'", actualBucketName, test.ExpectedBucket)
}

presignedMinio, err := newPresignedMinIOAccess(cfg)
if err != nil {
t.Fatalf("failed to create presigned minio access: '%v'", err)
}

actualBucketName = presignedMinio.Bucket(test.OwnerID)
if actualBucketName != test.ExpectedBucket {
t.Fatalf("[presigned minio] unexpected bucket name: is '%s' but expected '%s'", actualBucketName, test.ExpectedBucket)
}
})
}
}

func TestMinioBackupObject(t *testing.T) {
tests := []struct {
Name string
BucketNameConfig string
Username string
Workspace string
InstanceID string
ObjectName string
ExpectedBackupObject string
}{
{
Name: "no dedicated bucket",
BucketNameConfig: "",
Username: "test-user",
Workspace: "gitpodio-gitpod-2cx8z8e643x",
InstanceID: "fa9aa2af-b6de-45fc-8b48-534bb440429f",
ObjectName: "backup.tar",
ExpectedBackupObject: "workspaces/gitpodio-gitpod-2cx8z8e643x/backup.tar",
},
{
Name: "with dedicated bucket",
BucketNameConfig: "root-bucket",
Username: "test-user",
Workspace: "gitpodio-gitpod-2cx8z8e643x",
InstanceID: "fa9aa2af-b6de-45fc-8b48-534bb440429f",
ObjectName: "backup.tar",
ExpectedBackupObject: "test-user/workspaces/gitpodio-gitpod-2cx8z8e643x/backup.tar",
},
}

for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
cfg := config.MinIOConfig{
Endpoint: "fake",
AccessKeyID: "fake_access_key",
SecretAccessKey: "fake_secret",
Region: "none",
BucketName: test.BucketNameConfig,
}
minio, err := newDirectMinIOAccess(cfg)
if err != nil {
t.Fatalf("failed to create minio access: '%v'", err)
}
err = minio.Init(context.Background(), test.Username, test.Workspace, test.InstanceID)
if err != nil {
t.Fatalf("failed to init minio access: '%v'", err)
}

actualBackupObject := minio.BackupObject(test.ObjectName)
if actualBackupObject != test.ExpectedBackupObject {
t.Fatalf("[minio] unexpected backup object name: is '%s' but expected '%s'", actualBackupObject, test.ExpectedBackupObject)
}

presignedMinio, err := newPresignedMinIOAccess(cfg)
if err != nil {
t.Fatalf("failed to create presigned minio access: '%v'", err)
}

actualBackupObject = presignedMinio.BackupObject(test.Username, test.Workspace, test.ObjectName)
if actualBackupObject != test.ExpectedBackupObject {
t.Fatalf("[presigned minio] unexpected backup object name: is '%s' but expected '%s'", actualBackupObject, test.ExpectedBackupObject)
}

})
}
}
Loading