Skip to content

Commit

Permalink
[content-service] fix s3 access when using dedicated bucket
Browse files Browse the repository at this point in the history
  • Loading branch information
@sagor999
sagor999 committed May 23, 2022
1 parent e0244ff commit 450178e
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 30 deletions.
4 changes: 2 additions & 2 deletions components/content-service/pkg/layer/provider_test.go
View file
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
4 changes: 2 additions & 2 deletions components/content-service/pkg/service/headless-log-service.go
View file
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
2 changes: 1 addition & 1 deletion components/content-service/pkg/service/headless-log-service_test.go
View file
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
View file
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
View file
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))
}
18 changes: 13 additions & 5 deletions components/content-service/pkg/storage/minio.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,11 @@ func (rs *DirectMinIOStorage) bucketName() string {
}

func (rs *DirectMinIOStorage) objectName(name string) string {
return minioWorkspaceBackupObjectName(rs.Username, 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 @@ -532,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, 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
16 changes: 8 additions & 8 deletions components/content-service/pkg/storage/mock/mock.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions components/content-service/pkg/storage/noop.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ func (p *PresignedNoopStorage) ObjectExists(ctx context.Context, bucket, obj str
}

// BackupObject returns a backup's object name that a direct downloader would download
func (*PresignedNoopStorage) BackupObject(workspaceID string, name string) string {
func (*PresignedNoopStorage) BackupObject(ownerID string, workspaceID string, name string) string {
return ""
}

// InstanceObject returns a instance's object name that a direct downloader would download
func (*PresignedNoopStorage) InstanceObject(workspaceID string, instanceID string, name string) string {
func (*PresignedNoopStorage) InstanceObject(ownerID string, workspaceID string, instanceID string, name string) string {
return ""
}
4 changes: 2 additions & 2 deletions components/content-service/pkg/storage/storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ type PresignedAccess interface {
ObjectExists(ctx context.Context, bucket string, path string) (bool, error)

// BackupObject returns a backup's object name that a direct downloader would download
BackupObject(workspaceID string, name string) string
BackupObject(ownerID string, workspaceID string, name string) string

// InstanceObject returns a instance's object name that a direct downloader would download
InstanceObject(workspaceID string, instanceID string, name string) string
InstanceObject(ownerID string, workspaceID string, instanceID string, name string) string
}

// ObjectMeta describtes the metadata of a remote object
Expand Down

0 comments on commit 450178e

Please sign in to comment.