[content-service] Review Comments

- Ensure all items are fetched
- Remove double init of client
- Specify region and credentials from config
- Remove insecure s3 test (deprecated)

Author: Furisto

components/content-service-api/go/config/config.go

@@ -111,7 +111,9 @@ type MinIOConfig struct {
 
 // S3Config configures the S3 remote storage backend
 type S3Config struct {
-	Bucket string `json:"bucket"`
+	Bucket          string `json:"bucket"`
+	Region          string `json:"region"`
+	CredentialsFile string `json:"credentialsFile"`
 }
 
 type PProf struct {

components/content-service/cmd/test.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2021 Gitpod GmbH. All rights reserved.
+// 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.
 
@@ -269,7 +269,9 @@ func createDirectAccess() (direct storage.DirectAccess, err error) {
 		cfg = &config.StorageConfig{
 			Kind: config.S3Storage,
 			S3Config: &config.S3Config{
-				Bucket: "gitpod-s3",
+				Bucket:          "gitpod-s3",
+				Region:          "eu-central-1",
+				CredentialsFile: "./credentials",
 			},
 		}
 	}
@@ -305,7 +307,9 @@ func createPresignedAccess() (presigned storage.PresignedAccess, err error) {
 		cfg = &config.StorageConfig{
 			Kind: config.S3Storage,
 			S3Config: &config.S3Config{
-				Bucket: "gitpod-s3",
+				Bucket:          "gitpod-s3",
+				Region:          "eu-central-1",
+				CredentialsFile: "./credentials",
 			},
 		}
 	}

components/content-service/pkg/storage/s3.go

@@ -18,7 +18,6 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
-	awsconfig "github.com/aws/aws-sdk-go-v2/config"
 	s3manager "github.com/aws/aws-sdk-go-v2/feature/s3/manager"
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
@@ -309,13 +308,6 @@ func (*s3Storage) EnsureExists(ctx context.Context) error {
 
 // Init implements DirectAccess
 func (s3st *s3Storage) Init(ctx context.Context, owner string, workspace string, instance string) error {
-	cfg, err := awsconfig.LoadDefaultConfig(ctx)
-	if err != nil {
-		return err
-	}
-
-	s3st.client = s3.NewFromConfig(cfg)
-
 	s3st.OwnerID = owner
 	s3st.WorkspaceID = workspace
 	s3st.InstanceID = instance
@@ -328,17 +320,25 @@ func (s3st *s3Storage) ListObjects(ctx context.Context, prefix string) ([]string
 		return nil, xerrors.Errorf("prefix must start with the owner ID")
 	}
 
-	objs, err := s3st.client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{
+	var res []string
+	listParams := &s3.ListObjectsV2Input{
 		Bucket: aws.String(s3st.Config.Bucket),
 		Prefix: aws.String(prefix),
-	})
-	if err != nil {
-		return nil, xerrors.Errorf("cannot list objects: %w", err)
 	}
+	fetchObjects := true
+	for fetchObjects {
+		objs, err := s3st.client.ListObjectsV2(ctx, listParams)
+
+		if err != nil {
+			return nil, xerrors.Errorf("cannot list objects: %w", err)
+		}
+
+		for _, o := range objs.Contents {
+			res = append(res, *o.Key)
+		}
 
-	res := make([]string, len(objs.Contents))
-	for i := range objs.Contents {
-		res[i] = *objs.Contents[i].Key
+		listParams.ContinuationToken = objs.NextContinuationToken
+		fetchObjects = objs.IsTruncated
 	}
 
 	return res, nil

components/content-service/pkg/storage/storage.go

@@ -228,10 +228,12 @@ func NewDirectAccess(c *config.StorageConfig) (DirectAccess, error) {
 	case config.MinIOStorage:
 		return newDirectMinIOAccess(c.MinIOConfig)
 	case config.S3Storage:
-		cfg, err := awsconfig.LoadDefaultConfig(context.TODO())
+		cfg, err := awsconfig.LoadDefaultConfig(context.TODO(), awsconfig.WithSharedConfigFiles([]string{c.S3Config.CredentialsFile}))
 		if err != nil {
 			return nil, err
 		}
+
+		cfg.Region = c.S3Config.Region
 		return newDirectS3Access(s3.NewFromConfig(cfg), S3Config{
 			Bucket: c.S3Config.Bucket,
 		}), nil
@@ -253,10 +255,12 @@ func NewPresignedAccess(c *config.StorageConfig) (PresignedAccess, error) {
 	case config.MinIOStorage:
 		return newPresignedMinIOAccess(c.MinIOConfig)
 	case config.S3Storage:
-		cfg, err := awsconfig.LoadDefaultConfig(context.TODO())
+		cfg, err := awsconfig.LoadDefaultConfig(context.TODO(), awsconfig.WithSharedConfigFiles([]string{c.S3Config.CredentialsFile}))
 		if err != nil {
 			return nil, err
 		}
+
+		cfg.Region = c.S3Config.Region
 		return NewPresignedS3Access(s3.NewFromConfig(cfg), S3Config{
 			Bucket: c.S3Config.Bucket,
 		}), nil

install/installer/cmd/testdata/render/aws-setup/config.yaml

@@ -24,7 +24,6 @@ metadata:
 objectStorage:
   inCluster: false
   s3:
-    region: eu-west-2
     endpoint: s3.amazonaws.com
     bucket: s3-bucket
     credentials: