From 878d154e50bebcfe09b0249e4bffb0504b6f2f23 Mon Sep 17 00:00:00 2001 From: Huang Date: Thu, 25 May 2023 15:31:05 -0700 Subject: [PATCH 01/13] chore: add tree-making functionality --- internal/pkg/aws/ecr/ecr_test.go | 4 +- internal/pkg/aws/s3/s3.go | 83 +++++++++++++++++++++++++++- internal/pkg/aws/s3/s3_test.go | 2 +- internal/pkg/describe/service.go | 4 ++ internal/pkg/describe/static_site.go | 12 ++++ 5 files changed, 100 insertions(+), 5 deletions(-) diff --git a/internal/pkg/aws/ecr/ecr_test.go b/internal/pkg/aws/ecr/ecr_test.go index 713162d8a74..64286d5f7a2 100644 --- a/internal/pkg/aws/ecr/ecr_test.go +++ b/internal/pkg/aws/ecr/ecr_test.go @@ -413,7 +413,7 @@ func TestClearRepository(t *testing.T) { }, wantError: nil, }, - "returns error if fail to check repo existance": { + "returns error if fail to check repo existence": { mockECRClient: func(m *mocks.Mockapi) { m.EXPECT().DescribeImages(&ecr.DescribeImagesInput{ RepositoryName: aws.String(mockRepoName), @@ -421,7 +421,7 @@ func TestClearRepository(t *testing.T) { }, wantError: fmt.Errorf("ecr repo mockRepoName describe images: %w", mockAwsError), }, - "returns error if fail to check repo existance because of non-awserr error type": { + "returns error if fail to check repo existence because of non-awserr error type": { mockECRClient: func(m *mocks.Mockapi) { m.EXPECT().DescribeImages(&ecr.DescribeImagesInput{ RepositoryName: aws.String(mockRepoName), diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index 642d01a5c3b..5043ca1b75f 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -7,6 +7,7 @@ package s3 import ( "errors" "fmt" + "github.com/xlab/treeprint" "io" "mime" "path/filepath" @@ -38,6 +39,7 @@ type s3ManagerAPI interface { type s3API interface { ListObjectVersions(input *s3.ListObjectVersionsInput) (*s3.ListObjectVersionsOutput, error) + ListObjectsV2(input *s3.ListObjectsV2Input) (*s3.ListObjectsV2Output, error) DeleteObjects(input *s3.DeleteObjectsInput) (*s3.DeleteObjectsOutput, error) HeadBucket(input *s3.HeadBucketInput) (*s3.HeadBucketOutput, error) } @@ -77,10 +79,10 @@ func (s *S3) EmptyBucket(bucket string) error { var listResp *s3.ListObjectVersionsOutput var err error - // Bucket is exists check to make sure the bucket exists before proceeding in emptying it + // isBucketExists checks to make sure the bucket exists before proceeding to empty it. isExists, err := s.isBucketExists(bucket) if err != nil { - return fmt.Errorf("unable to determine the existance of bucket %s: %w", bucket, err) + return fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) } if !isExists { @@ -206,6 +208,83 @@ func (s *S3) isBucketExists(bucket string) (bool, error) { return true, nil } +// GetBucketTree retrieves the objects in an S3 bucket and creates an ASCII tree representing their folder structure. +func (s *S3) GetBucketTree(bucket string) error { + listResp := &s3.ListObjectsV2Output{} + var err error + + // isBucketExists check to make sure the bucket exists before proceeding. + isExists, err := s.isBucketExists(bucket) + if err != nil { + return fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) + } + if !isExists { + return nil + } + for { + listParams := &s3.ListObjectsV2Input{ + Bucket: aws.String(bucket), + Delimiter: aws.String("/"), + ContinuationToken: listResp.NextContinuationToken, + } + listResp, err = s.s3Client.ListObjectsV2(listParams) + if err != nil { + return fmt.Errorf("list objects for bucket %s: %w", bucket, err) + } + if listResp.NextContinuationToken == nil { + break + } + } + + tree := treeprint.New() + if err := s.addNodes(tree, listResp.CommonPrefixes, bucket); err != nil { + return err + } + + for _, object := range listResp.Contents { + tree.AddNode(aws.StringValue(object.Key)) + } + fmt.Println(tree.String()) + return nil +} + +func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket string) error { + if len(prefixes) == 0 { + return nil + } + for _, prefix := range prefixes { + branch := tree.AddBranch(s.extractFileName(aws.StringValue(prefix.Prefix))) + listParams := &s3.ListObjectsV2Input{ + Bucket: aws.String(bucket), + Delimiter: aws.String("/"), + // continuationtoken? + Prefix: prefix.Prefix, + } + listResp, err := s.s3Client.ListObjectsV2(listParams) + if err != nil { + return fmt.Errorf("list objects for bucket %s: %w", bucket, err) + } + for _, file := range listResp.Contents { + fileName := s.extractFileName(aws.StringValue(file.Key)) + branch.AddNode(fileName) + } + if err := s.addNodes(branch, listResp.CommonPrefixes, bucket); err != nil { + return err + } + } + return nil +} + +// extractFileName returns the names of individual files and the ends of S3 prefixes. +func (s *S3) extractFileName(path string) string { + path, _ = strings.CutSuffix(path, `/`) + if strings.Contains(path, `/`) { + split := strings.Split(path, `/`) + return split[len(split)-1] + } + return path +} + func (s *S3) upload(bucket, key string, buf io.Reader) (string, error) { in := &s3manager.UploadInput{ Body: buf, diff --git a/internal/pkg/aws/s3/s3_test.go b/internal/pkg/aws/s3/s3_test.go index a14f7854052..244e4ffd4d8 100644 --- a/internal/pkg/aws/s3/s3_test.go +++ b/internal/pkg/aws/s3/s3_test.go @@ -274,7 +274,7 @@ func TestS3_EmptyBucket(t *testing.T) { }).Return(nil, awserr.New("Unknown", "message", nil)) }, - wantErr: fmt.Errorf("unable to determine the existance of bucket %s: %w", "mockBucket", + wantErr: fmt.Errorf("unable to determine the existence of bucket %s: %w", "mockBucket", awserr.New("Unknown", "message", nil)), }, "some objects failed to delete": { diff --git a/internal/pkg/describe/service.go b/internal/pkg/describe/service.go index f68e46f1a7a..0663b268cb6 100644 --- a/internal/pkg/describe/service.go +++ b/internal/pkg/describe/service.go @@ -91,6 +91,10 @@ type cwAlarmDescriber interface { AlarmDescriptions([]string) ([]*cloudwatch.AlarmDescription, error) } +type bucketDescriber interface { + GetBucketTree(bucket string) error +} + type ecsSvcDesc struct { Service string `json:"service"` Type string `json:"type"` diff --git a/internal/pkg/describe/static_site.go b/internal/pkg/describe/static_site.go index 1fe9138e752..fe5e76230fd 100644 --- a/internal/pkg/describe/static_site.go +++ b/internal/pkg/describe/static_site.go @@ -7,6 +7,8 @@ import ( "bytes" "encoding/json" "fmt" + "github.com/aws/copilot-cli/internal/pkg/aws/s3" + "github.com/aws/copilot-cli/internal/pkg/aws/sessions" "strings" "text/tabwriter" @@ -30,16 +32,22 @@ type StaticSiteDescriber struct { store DeployedEnvServicesLister initWkldStackDescriber func(string) (workloadDescriber, error) wkldDescribers map[string]workloadDescriber + s3Client bucketDescriber } // NewStaticSiteDescriber instantiates a static site service describer. func NewStaticSiteDescriber(opt NewServiceConfig) (*StaticSiteDescriber, error) { + defaultSess, err := sessions.ImmutableProvider().Default() + if err != nil { + return nil, fmt.Errorf("default session: %v", err) + } describer := &StaticSiteDescriber{ app: opt.App, svc: opt.Svc, enableResources: opt.EnableResources, store: opt.DeployStore, wkldDescribers: make(map[string]workloadDescriber), + s3Client: s3.New(defaultSess), } describer.initWkldStackDescriber = func(env string) (workloadDescriber, error) { if describer, ok := describer.wkldDescribers[env]; ok { @@ -98,6 +106,10 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { }) } } + // TODO(jwh): get bucket name + if err := d.s3Client.GetBucketTree("bugbash-static-test-static-site-bucket-1qdzh62y4l3gj"); err != nil { + return nil, err + } resources := make(map[string][]*stack.Resource) if d.enableResources { for _, env := range environments { From 6ce97e537707f38f7997e793912ce17ad79fa0e8 Mon Sep 17 00:00:00 2001 From: Huang Date: Fri, 26 May 2023 10:39:45 -0700 Subject: [PATCH 02/13] chore: get bucket name dynamically per env --- internal/pkg/describe/service.go | 4 ++++ internal/pkg/describe/static_site.go | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/internal/pkg/describe/service.go b/internal/pkg/describe/service.go index 0663b268cb6..9b8e9ce32ab 100644 --- a/internal/pkg/describe/service.go +++ b/internal/pkg/describe/service.go @@ -95,6 +95,10 @@ type bucketDescriber interface { GetBucketTree(bucket string) error } +type bucketNameGetter interface { + BucketName(app, env, svc string) (string, error) +} + type ecsSvcDesc struct { Service string `json:"service"` Type string `json:"type"` diff --git a/internal/pkg/describe/static_site.go b/internal/pkg/describe/static_site.go index fe5e76230fd..1c75a2e9a91 100644 --- a/internal/pkg/describe/static_site.go +++ b/internal/pkg/describe/static_site.go @@ -7,8 +7,9 @@ import ( "bytes" "encoding/json" "fmt" - "github.com/aws/copilot-cli/internal/pkg/aws/s3" + awsS3 "github.com/aws/copilot-cli/internal/pkg/aws/s3" "github.com/aws/copilot-cli/internal/pkg/aws/sessions" + s3 "github.com/aws/copilot-cli/internal/pkg/s3" "strings" "text/tabwriter" @@ -32,7 +33,8 @@ type StaticSiteDescriber struct { store DeployedEnvServicesLister initWkldStackDescriber func(string) (workloadDescriber, error) wkldDescribers map[string]workloadDescriber - s3Client bucketDescriber + awsS3Client bucketDescriber + s3Client bucketNameGetter } // NewStaticSiteDescriber instantiates a static site service describer. @@ -47,6 +49,7 @@ func NewStaticSiteDescriber(opt NewServiceConfig) (*StaticSiteDescriber, error) enableResources: opt.EnableResources, store: opt.DeployStore, wkldDescribers: make(map[string]workloadDescriber), + awsS3Client: awsS3.New(defaultSess), s3Client: s3.New(defaultSess), } describer.initWkldStackDescriber = func(env string) (workloadDescriber, error) { @@ -105,11 +108,15 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { URL: uri.URI, }) } + bucketName, err := d.s3Client.BucketName(d.app, env, d.svc) + if err != nil { + return nil, fmt.Errorf("get bucket name for %q env: %w", env, err) + } + if err := d.awsS3Client.GetBucketTree(bucketName); err != nil { + return nil, err + } } - // TODO(jwh): get bucket name - if err := d.s3Client.GetBucketTree("bugbash-static-test-static-site-bucket-1qdzh62y4l3gj"); err != nil { - return nil, err - } + resources := make(map[string][]*stack.Resource) if d.enableResources { for _, env := range environments { From 1b743c6e69f5d513d7aab0e635dc4788a951d1c5 Mon Sep 17 00:00:00 2001 From: Huang Date: Fri, 26 May 2023 12:52:21 -0700 Subject: [PATCH 03/13] chore: add tree to describe --- internal/pkg/aws/s3/s3.go | 13 ++++++------- internal/pkg/describe/service.go | 3 ++- internal/pkg/describe/static_site.go | 26 +++++++++++++++++++++++++- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index 5043ca1b75f..dd48220346d 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -209,17 +209,17 @@ func (s *S3) isBucketExists(bucket string) (bool, error) { } // GetBucketTree retrieves the objects in an S3 bucket and creates an ASCII tree representing their folder structure. -func (s *S3) GetBucketTree(bucket string) error { +func (s *S3) GetBucketTree(bucket string) (treeprint.Tree, error) { listResp := &s3.ListObjectsV2Output{} var err error // isBucketExists check to make sure the bucket exists before proceeding. isExists, err := s.isBucketExists(bucket) if err != nil { - return fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) + return nil, fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) } if !isExists { - return nil + return nil, nil } for { listParams := &s3.ListObjectsV2Input{ @@ -229,7 +229,7 @@ func (s *S3) GetBucketTree(bucket string) error { } listResp, err = s.s3Client.ListObjectsV2(listParams) if err != nil { - return fmt.Errorf("list objects for bucket %s: %w", bucket, err) + return nil, fmt.Errorf("list objects for bucket %s: %w", bucket, err) } if listResp.NextContinuationToken == nil { break @@ -238,14 +238,13 @@ func (s *S3) GetBucketTree(bucket string) error { tree := treeprint.New() if err := s.addNodes(tree, listResp.CommonPrefixes, bucket); err != nil { - return err + return tree, err } for _, object := range listResp.Contents { tree.AddNode(aws.StringValue(object.Key)) } - fmt.Println(tree.String()) - return nil + return tree, nil } func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket string) error { diff --git a/internal/pkg/describe/service.go b/internal/pkg/describe/service.go index 9b8e9ce32ab..814c0f4b84d 100644 --- a/internal/pkg/describe/service.go +++ b/internal/pkg/describe/service.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/xlab/treeprint" "io" "net/url" "sort" @@ -92,7 +93,7 @@ type cwAlarmDescriber interface { } type bucketDescriber interface { - GetBucketTree(bucket string) error + GetBucketTree(bucket string) (treeprint.Tree, error) } type bucketNameGetter interface { diff --git a/internal/pkg/describe/static_site.go b/internal/pkg/describe/static_site.go index 1c75a2e9a91..768300e7b67 100644 --- a/internal/pkg/describe/static_site.go +++ b/internal/pkg/describe/static_site.go @@ -10,6 +10,7 @@ import ( awsS3 "github.com/aws/copilot-cli/internal/pkg/aws/s3" "github.com/aws/copilot-cli/internal/pkg/aws/sessions" s3 "github.com/aws/copilot-cli/internal/pkg/s3" + "github.com/xlab/treeprint" "strings" "text/tabwriter" @@ -97,6 +98,7 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { return nil, fmt.Errorf("list deployed environments for service %q: %w", d.svc, err) } var routes []*WebServiceRoute + var objects []*S3ObjectTree for _, env := range environments { uri, err := d.URI(env) if err != nil { @@ -112,9 +114,14 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { if err != nil { return nil, fmt.Errorf("get bucket name for %q env: %w", env, err) } - if err := d.awsS3Client.GetBucketTree(bucketName); err != nil { + tree, err := d.awsS3Client.GetBucketTree(bucketName) + if err != nil { return nil, err } + objects = append(objects, &S3ObjectTree{ + Environment: env, + Tree: tree, + }) } resources := make(map[string][]*stack.Resource) @@ -137,6 +144,7 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { App: d.app, Routes: routes, Resources: resources, + Objects: objects, environments: environments, }, nil @@ -152,12 +160,19 @@ func (d *StaticSiteDescriber) Manifest(env string) ([]byte, error) { return cfn.Manifest() } +// S3ObjectTree contains serialized parameters for an S3 object tree. +type S3ObjectTree struct { + Environment string + Tree treeprint.Tree +} + // staticSiteDesc contains serialized parameters for a static site. type staticSiteDesc struct { Service string `json:"service"` Type string `json:"type"` App string `json:"application"` Routes []*WebServiceRoute `json:"routes"` + Objects []*S3ObjectTree `json:"objects"` Resources deployedSvcResources `json:"resources,omitempty"` environments []string `json:"-"` @@ -191,6 +206,15 @@ func (w *staticSiteDesc) HumanString() string { fmt.Fprintf(writer, " %s\t%s\n", route.Environment, route.URL) } } + if len(w.Objects) != 0 { + fmt.Fprint(writer, color.Bold.Sprint("\nS3 Bucket Objects\n")) + writer.Flush() + for _, object := range w.Objects { + fmt.Fprintf(writer, "\n %s\t%s\n", "Environment", object.Environment) + fmt.Fprintf(writer, object.Tree.String()) + writer.Flush() + } + } if len(w.Resources) != 0 { fmt.Fprint(writer, color.Bold.Sprint("\nResources\n")) writer.Flush() From d05f4a6c10220fd415f57460c78a1571db0e54c1 Mon Sep 17 00:00:00 2001 From: Huang Date: Wed, 7 Jun 2023 14:06:35 -0700 Subject: [PATCH 04/13] chore: add unit tests --- internal/pkg/aws/s3/mocks/mock_s3.go | 15 ++ internal/pkg/aws/s3/s3.go | 63 ++++---- internal/pkg/aws/s3/s3_test.go | 170 ++++++++++++++++++++ internal/pkg/describe/mocks/mock_service.go | 76 +++++++++ internal/pkg/describe/service.go | 3 +- internal/pkg/describe/static_site.go | 19 +-- internal/pkg/describe/static_site_test.go | 49 +++++- 7 files changed, 354 insertions(+), 41 deletions(-) diff --git a/internal/pkg/aws/s3/mocks/mock_s3.go b/internal/pkg/aws/s3/mocks/mock_s3.go index 5dda2a12914..5361e364c5c 100644 --- a/internal/pkg/aws/s3/mocks/mock_s3.go +++ b/internal/pkg/aws/s3/mocks/mock_s3.go @@ -123,6 +123,21 @@ func (mr *Mocks3APIMockRecorder) ListObjectVersions(input interface{}) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListObjectVersions", reflect.TypeOf((*Mocks3API)(nil).ListObjectVersions), input) } +// ListObjectsV2 mocks base method. +func (m *Mocks3API) ListObjectsV2(input *s3.ListObjectsV2Input) (*s3.ListObjectsV2Output, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListObjectsV2", input) + ret0, _ := ret[0].(*s3.ListObjectsV2Output) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListObjectsV2 indicates an expected call of ListObjectsV2. +func (mr *Mocks3APIMockRecorder) ListObjectsV2(input interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListObjectsV2", reflect.TypeOf((*Mocks3API)(nil).ListObjectsV2), input) +} + // MockNamedBinary is a mock of NamedBinary interface. type MockNamedBinary struct { ctrl *gomock.Controller diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index dd48220346d..fea84681f3d 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -79,13 +79,13 @@ func (s *S3) EmptyBucket(bucket string) error { var listResp *s3.ListObjectVersionsOutput var err error - // isBucketExists checks to make sure the bucket exists before proceeding to empty it. - isExists, err := s.isBucketExists(bucket) + // isBucket checks to make sure the bucket exists before proceeding to empty it. + isBucket, err := s.isBucket(bucket) if err != nil { return fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) } - if !isExists { + if !isBucket { return nil } @@ -192,8 +192,8 @@ func FormatARN(partition, location string) string { return fmt.Sprintf("arn:%s:s3:::%s", partition, location) } -// Check whether the bucket exists before proceeding with empty the bucket -func (s *S3) isBucketExists(bucket string) (bool, error) { +// isBucket checks whether the bucket exists before emptying the bucket +func (s *S3) isBucket(bucket string) (bool, error) { input := &s3.HeadBucketInput{ Bucket: aws.String(bucket), } @@ -209,17 +209,17 @@ func (s *S3) isBucketExists(bucket string) (bool, error) { } // GetBucketTree retrieves the objects in an S3 bucket and creates an ASCII tree representing their folder structure. -func (s *S3) GetBucketTree(bucket string) (treeprint.Tree, error) { +func (s *S3) GetBucketTree(bucket string) (string, error) { listResp := &s3.ListObjectsV2Output{} var err error - // isBucketExists check to make sure the bucket exists before proceeding. - isExists, err := s.isBucketExists(bucket) + // isBucket check to make sure the bucket exists before proceeding. + exists, err := s.isBucket(bucket) if err != nil { - return nil, fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) + return "", fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) } - if !isExists { - return nil, nil + if !exists { + return "", nil } for { listParams := &s3.ListObjectsV2Input{ @@ -229,7 +229,7 @@ func (s *S3) GetBucketTree(bucket string) (treeprint.Tree, error) { } listResp, err = s.s3Client.ListObjectsV2(listParams) if err != nil { - return nil, fmt.Errorf("list objects for bucket %s: %w", bucket, err) + return "", fmt.Errorf("list objects for bucket %s: %w", bucket, err) } if listResp.NextContinuationToken == nil { break @@ -237,31 +237,39 @@ func (s *S3) GetBucketTree(bucket string) (treeprint.Tree, error) { } tree := treeprint.New() - if err := s.addNodes(tree, listResp.CommonPrefixes, bucket); err != nil { - return tree, err - } - + // Add top-level files. for _, object := range listResp.Contents { tree.AddNode(aws.StringValue(object.Key)) } - return tree, nil + // Recursively add folders and their children. + if err := s.addNodes(tree, listResp.CommonPrefixes, bucket); err != nil { + return tree.String(), err + } + return tree.String(), nil } func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket string) error { if len(prefixes) == 0 { return nil } + listResp := &s3.ListObjectsV2Output{} + var err error for _, prefix := range prefixes { branch := tree.AddBranch(s.extractFileName(aws.StringValue(prefix.Prefix))) - listParams := &s3.ListObjectsV2Input{ - Bucket: aws.String(bucket), - Delimiter: aws.String("/"), - // continuationtoken? - Prefix: prefix.Prefix, - } - listResp, err := s.s3Client.ListObjectsV2(listParams) - if err != nil { - return fmt.Errorf("list objects for bucket %s: %w", bucket, err) + for { + listParams := &s3.ListObjectsV2Input{ + Bucket: aws.String(bucket), + Delimiter: aws.String("/"), + ContinuationToken: listResp.ContinuationToken, + Prefix: prefix.Prefix, + } + listResp, err = s.s3Client.ListObjectsV2(listParams) + if err != nil { + return fmt.Errorf("list objects for bucket %s: %w", bucket, err) + } + if listResp.NextContinuationToken == nil { + break + } } for _, file := range listResp.Contents { fileName := s.extractFileName(aws.StringValue(file.Key)) @@ -274,7 +282,8 @@ func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket s return nil } -// extractFileName returns the names of individual files and the ends of S3 prefixes. +// extractFileName returns the last element of a path, whether the name of +// an individual file or the end of an S3 prefix. func (s *S3) extractFileName(path string) string { path, _ = strings.CutSuffix(path, `/`) if strings.Contains(path, `/`) { diff --git a/internal/pkg/aws/s3/s3_test.go b/internal/pkg/aws/s3/s3_test.go index 244e4ffd4d8..78f5cfc0305 100644 --- a/internal/pkg/aws/s3/s3_test.go +++ b/internal/pkg/aws/s3/s3_test.go @@ -488,3 +488,173 @@ func TestS3_FormatARN(t *testing.T) { }) } } + +func TestS3_GetBucketTree(t *testing.T) { + type s3Mocks struct { + s3API *mocks.Mocks3API + s3ManagerAPI *mocks.Mocks3ManagerAPI + } + mockBucket := aws.String("bucketName") + delimiter := aws.String("/") + nonexistentError := awserr.New(errCodeNotFound, "msg", errors.New("some error")) + + firstResp := s3.ListObjectsV2Output{ + CommonPrefixes: []*s3.CommonPrefix{ + {Prefix: aws.String("Images")}, + {Prefix: aws.String("css")}, + {Prefix: aws.String("top")}, + }, + Contents: []*s3.Object{ + {Key: aws.String("README.md")}, + {Key: aws.String("error.html")}, + {Key: aws.String("index.html")}, + }, + Delimiter: delimiter, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + } + imagesResp := s3.ListObjectsV2Output{ + Contents: []*s3.Object{ + {Key: aws.String("firstImage.PNG")}, + {Key: aws.String("secondImage.PNG")}, + }, + Delimiter: delimiter, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + } + cssResp := s3.ListObjectsV2Output{ + Contents: []*s3.Object{ + {Key: aws.String("Style.css")}, + {Key: aws.String("bootstrap.min.css")}, + }, + Delimiter: delimiter, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + } + topResp := s3.ListObjectsV2Output{ + CommonPrefixes: []*s3.CommonPrefix{ + {Prefix: aws.String("middle")}, + }, + Delimiter: delimiter, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + } + middleResp := s3.ListObjectsV2Output{ + Contents: []*s3.Object{ + {Key: aws.String("bottom.html")}, + }, + Delimiter: delimiter, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + } + testCases := map[string]struct { + setupMocks func(mocks s3Mocks) + + wantTree string + wantErr error + }{ + "should return all objects within the bucket as a tree string": { + setupMocks: func(m s3Mocks) { + m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, nil) + m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: mockBucket, + ContinuationToken: nil, + Delimiter: delimiter, + Prefix: nil, + }).Return(&firstResp, nil) + m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: mockBucket, + Delimiter: delimiter, + Prefix: aws.String("Images"), + }).Return(&imagesResp, nil) + m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: mockBucket, + Delimiter: delimiter, + Prefix: aws.String("css"), + }).Return(&cssResp, nil) + m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: mockBucket, + Delimiter: delimiter, + Prefix: aws.String("top"), + }).Return(&topResp, nil) + m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: mockBucket, + Delimiter: delimiter, + Prefix: aws.String("middle"), + }).Return(&middleResp, nil) + }, + wantTree: `. +├── README.md +├── error.html +├── index.html +├── Images +│ ├── firstImage.PNG +│ └── secondImage.PNG +├── css +│ ├── Style.css +│ └── bootstrap.min.css +└── top + └── middle + └── bottom.html +`, + }, + "return nil if bucket doesn't exist": { + setupMocks: func(m s3Mocks) { + m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, nonexistentError) + }, + }, + "return err if cannot determine if bucket exists": { + setupMocks: func(m s3Mocks) { + m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, errors.New("some error")) + }, + wantErr: errors.New("unable to determine the existence of bucket bucketName: some error"), + }, + "should wrap error if fail to list objects": { + setupMocks: func(m s3Mocks) { + m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, nil) + m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: mockBucket, + ContinuationToken: nil, + Delimiter: delimiter, + Prefix: nil, + }).Return(nil, errors.New("some error")) + }, + wantErr: errors.New("list objects for bucket bucketName: some error"), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // GIVEN + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockS3Client := mocks.NewMocks3API(ctrl) + + mockS3Manager := mocks.NewMocks3ManagerAPI(ctrl) + + s3mocks := s3Mocks{ + s3API: mockS3Client, + s3ManagerAPI: mockS3Manager, + } + service := S3{ + s3Client: mockS3Client, + } + tc.setupMocks(s3mocks) + + gotTree, gotErr := service.GetBucketTree(aws.StringValue(mockBucket)) + if tc.wantErr != nil { + require.EqualError(t, gotErr, tc.wantErr.Error()) + return + } + require.NoError(t, gotErr) + require.Equal(t, tc.wantTree, gotTree) + }) + + } +} diff --git a/internal/pkg/describe/mocks/mock_service.go b/internal/pkg/describe/mocks/mock_service.go index ad70b9e9e0d..3e8e69b6986 100644 --- a/internal/pkg/describe/mocks/mock_service.go +++ b/internal/pkg/describe/mocks/mock_service.go @@ -708,3 +708,79 @@ func (mr *MockcwAlarmDescriberMockRecorder) AlarmDescriptions(arg0 interface{}) mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AlarmDescriptions", reflect.TypeOf((*MockcwAlarmDescriber)(nil).AlarmDescriptions), arg0) } + +// MockbucketDescriber is a mock of bucketDescriber interface. +type MockbucketDescriber struct { + ctrl *gomock.Controller + recorder *MockbucketDescriberMockRecorder +} + +// MockbucketDescriberMockRecorder is the mock recorder for MockbucketDescriber. +type MockbucketDescriberMockRecorder struct { + mock *MockbucketDescriber +} + +// NewMockbucketDescriber creates a new mock instance. +func NewMockbucketDescriber(ctrl *gomock.Controller) *MockbucketDescriber { + mock := &MockbucketDescriber{ctrl: ctrl} + mock.recorder = &MockbucketDescriberMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockbucketDescriber) EXPECT() *MockbucketDescriberMockRecorder { + return m.recorder +} + +// GetBucketTree mocks base method. +func (m *MockbucketDescriber) GetBucketTree(bucket string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetBucketTree", bucket) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetBucketTree indicates an expected call of GetBucketTree. +func (mr *MockbucketDescriberMockRecorder) GetBucketTree(bucket interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBucketTree", reflect.TypeOf((*MockbucketDescriber)(nil).GetBucketTree), bucket) +} + +// MockbucketNameGetter is a mock of bucketNameGetter interface. +type MockbucketNameGetter struct { + ctrl *gomock.Controller + recorder *MockbucketNameGetterMockRecorder +} + +// MockbucketNameGetterMockRecorder is the mock recorder for MockbucketNameGetter. +type MockbucketNameGetterMockRecorder struct { + mock *MockbucketNameGetter +} + +// NewMockbucketNameGetter creates a new mock instance. +func NewMockbucketNameGetter(ctrl *gomock.Controller) *MockbucketNameGetter { + mock := &MockbucketNameGetter{ctrl: ctrl} + mock.recorder = &MockbucketNameGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockbucketNameGetter) EXPECT() *MockbucketNameGetterMockRecorder { + return m.recorder +} + +// BucketName mocks base method. +func (m *MockbucketNameGetter) BucketName(app, env, svc string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BucketName", app, env, svc) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// BucketName indicates an expected call of BucketName. +func (mr *MockbucketNameGetterMockRecorder) BucketName(app, env, svc interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BucketName", reflect.TypeOf((*MockbucketNameGetter)(nil).BucketName), app, env, svc) +} diff --git a/internal/pkg/describe/service.go b/internal/pkg/describe/service.go index 814c0f4b84d..f2001e601b1 100644 --- a/internal/pkg/describe/service.go +++ b/internal/pkg/describe/service.go @@ -7,7 +7,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/xlab/treeprint" "io" "net/url" "sort" @@ -93,7 +92,7 @@ type cwAlarmDescriber interface { } type bucketDescriber interface { - GetBucketTree(bucket string) (treeprint.Tree, error) + GetBucketTree(bucket string) (string, error) } type bucketNameGetter interface { diff --git a/internal/pkg/describe/static_site.go b/internal/pkg/describe/static_site.go index 768300e7b67..2d4f3e32d10 100644 --- a/internal/pkg/describe/static_site.go +++ b/internal/pkg/describe/static_site.go @@ -10,7 +10,6 @@ import ( awsS3 "github.com/aws/copilot-cli/internal/pkg/aws/s3" "github.com/aws/copilot-cli/internal/pkg/aws/sessions" s3 "github.com/aws/copilot-cli/internal/pkg/s3" - "github.com/xlab/treeprint" "strings" "text/tabwriter" @@ -116,12 +115,14 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { } tree, err := d.awsS3Client.GetBucketTree(bucketName) if err != nil { - return nil, err + return nil, fmt.Errorf("get tree representation of bucket contents: %w", err) + } + if tree != "" { + objects = append(objects, &S3ObjectTree{ + Environment: env, + Tree: tree, + }) } - objects = append(objects, &S3ObjectTree{ - Environment: env, - Tree: tree, - }) } resources := make(map[string][]*stack.Resource) @@ -163,7 +164,7 @@ func (d *StaticSiteDescriber) Manifest(env string) ([]byte, error) { // S3ObjectTree contains serialized parameters for an S3 object tree. type S3ObjectTree struct { Environment string - Tree treeprint.Tree + Tree string } // staticSiteDesc contains serialized parameters for a static site. @@ -172,7 +173,7 @@ type staticSiteDesc struct { Type string `json:"type"` App string `json:"application"` Routes []*WebServiceRoute `json:"routes"` - Objects []*S3ObjectTree `json:"objects"` + Objects []*S3ObjectTree `json:"objects,omitempty"` Resources deployedSvcResources `json:"resources,omitempty"` environments []string `json:"-"` @@ -211,7 +212,7 @@ func (w *staticSiteDesc) HumanString() string { writer.Flush() for _, object := range w.Objects { fmt.Fprintf(writer, "\n %s\t%s\n", "Environment", object.Environment) - fmt.Fprintf(writer, object.Tree.String()) + fmt.Fprintf(writer, object.Tree) writer.Flush() } } diff --git a/internal/pkg/describe/static_site_test.go b/internal/pkg/describe/static_site_test.go index 803bc7e3669..4d604832929 100644 --- a/internal/pkg/describe/static_site_test.go +++ b/internal/pkg/describe/static_site_test.go @@ -17,6 +17,8 @@ import ( type staticSiteDescriberMocks struct { wkldDescriber *mocks.MockworkloadDescriber store *mocks.MockDeployedEnvServicesLister + awsS3Client *mocks.MockbucketDescriber + s3Client *mocks.MockbucketNameGetter } func TestStaticSiteDescriber_URI(t *testing.T) { @@ -107,6 +109,7 @@ func TestStaticSiteDescriber_Describe(t *testing.T) { mockSvc = "static" ) mockErr := errors.New("some error") + mockBucket := "bucketName" testCases := map[string]struct { shouldOutputResources bool @@ -124,13 +127,15 @@ func TestStaticSiteDescriber_Describe(t *testing.T) { }, wantedError: fmt.Errorf(`list deployed environments for service "static": some error`), }, - "success without resources flag": { + "success without resources flag or objects in bucket": { setupMocks: func(m staticSiteDescriberMocks) { gomock.InOrder( m.store.EXPECT().ListEnvironmentsDeployedTo(mockApp, mockSvc).Return([]string{"test"}, nil), m.wkldDescriber.EXPECT().Outputs().Return(map[string]string{ "CloudFrontDistributionDomainName": "dut843shvcmvn.cloudfront.net", }, nil), + m.s3Client.EXPECT().BucketName(mockApp, mockEnv, mockSvc).Return(mockBucket, nil), + m.awsS3Client.EXPECT().GetBucketTree(mockBucket).Return("", nil), ) }, wantedHuman: `About @@ -155,12 +160,14 @@ Routes m.wkldDescriber.EXPECT().Outputs().Return(map[string]string{ "CloudFrontDistributionDomainName": "dut843shvcmvn.cloudfront.net", }, nil), + m.s3Client.EXPECT().BucketName(mockApp, mockEnv, mockSvc).Return(mockBucket, nil), + m.awsS3Client.EXPECT().GetBucketTree(mockBucket).Return("", nil), m.wkldDescriber.EXPECT().StackResources().Return(nil, mockErr), ) }, wantedError: fmt.Errorf("retrieve service resources: some error"), }, - "success with resources flag": { + "success with resources flag and objects in bucket": { shouldOutputResources: true, setupMocks: func(m staticSiteDescriberMocks) { gomock.InOrder( @@ -168,6 +175,21 @@ Routes m.wkldDescriber.EXPECT().Outputs().Return(map[string]string{ "CloudFrontDistributionDomainName": "dut843shvcmvn.cloudfront.net", }, nil), + m.s3Client.EXPECT().BucketName(mockApp, mockEnv, mockSvc).Return(mockBucket, nil), + m.awsS3Client.EXPECT().GetBucketTree(mockBucket).Return(`. +├── README.md +├── error.html +├── index.html +├── Images +│ ├── firstImage.PNG +│ └── secondImage.PNG +├── css +│ ├── Style.css +│ └── bootstrap.min.css +└── top + └── middle + └── bottom.html +`, nil), m.wkldDescriber.EXPECT().StackResources().Return([]*stack.Resource{ { Type: "AWS::S3::Bucket", @@ -194,13 +216,30 @@ Routes ----------- --- test dut843shvcmvn.cloudfront.net +S3 Bucket Objects + + Environment test +. +├── README.md +├── error.html +├── index.html +├── Images +│ ├── firstImage.PNG +│ └── secondImage.PNG +├── css +│ ├── Style.css +│ └── bootstrap.min.css +└── top + └── middle + └── bottom.html + Resources test AWS::S3::Bucket demo-test-mystatic-bucket-h69vu7y72ga9 AWS::S3::BucketPolicy demo-test-mystatic-BucketPolicyForCloudFront-8AITX9Q7K13R `, - wantedJSON: "{\"service\":\"static\",\"type\":\"Static Site\",\"application\":\"phonetool\",\"routes\":[{\"environment\":\"test\",\"url\":\"dut843shvcmvn.cloudfront.net\"}],\"resources\":{\"test\":[{\"type\":\"AWS::S3::Bucket\",\"physicalID\":\"demo-test-mystatic-bucket-h69vu7y72ga9\",\"logicalID\":\"Bucket\"},{\"type\":\"AWS::S3::BucketPolicy\",\"physicalID\":\"demo-test-mystatic-BucketPolicyForCloudFront-8AITX9Q7K13R\",\"logicalID\":\"BucketPolicy\"}]}}\n", + wantedJSON: "{\"service\":\"static\",\"type\":\"Static Site\",\"application\":\"phonetool\",\"routes\":[{\"environment\":\"test\",\"url\":\"dut843shvcmvn.cloudfront.net\"}],\"objects\":[{\"Environment\":\"test\",\"Tree\":\".\\n├── README.md\\n├── error.html\\n├── index.html\\n├── Images\\n│ ├── firstImage.PNG\\n│ └── secondImage.PNG\\n├── css\\n│ ├── Style.css\\n│ └── bootstrap.min.css\\n└── top\\n └── middle\\n └── bottom.html\\n\"}],\"resources\":{\"test\":[{\"type\":\"AWS::S3::Bucket\",\"physicalID\":\"demo-test-mystatic-bucket-h69vu7y72ga9\",\"logicalID\":\"Bucket\"},{\"type\":\"AWS::S3::BucketPolicy\",\"physicalID\":\"demo-test-mystatic-BucketPolicyForCloudFront-8AITX9Q7K13R\",\"logicalID\":\"BucketPolicy\"}]}}\n", }, } @@ -212,6 +251,8 @@ Resources mocks := staticSiteDescriberMocks{ store: mocks.NewMockDeployedEnvServicesLister(ctrl), wkldDescriber: mocks.NewMockworkloadDescriber(ctrl), + awsS3Client: mocks.NewMockbucketDescriber(ctrl), + s3Client: mocks.NewMockbucketNameGetter(ctrl), } tc.setupMocks(mocks) @@ -223,6 +264,8 @@ Resources store: mocks.store, initWkldStackDescriber: func(string) (workloadDescriber, error) { return mocks.wkldDescriber, nil }, wkldDescribers: make(map[string]workloadDescriber), + awsS3Client: mocks.awsS3Client, + s3Client: mocks.s3Client, } // WHEN From 2940f29705e400ba23e4b1a282a679cc2b9c1149 Mon Sep 17 00:00:00 2001 From: Huang Date: Wed, 7 Jun 2023 14:17:24 -0700 Subject: [PATCH 05/13] chore: clean up --- internal/pkg/aws/s3/s3.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index fea84681f3d..ae533095001 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -221,6 +221,7 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { if !exists { return "", nil } + for { listParams := &s3.ListObjectsV2Input{ Bucket: aws.String(bucket), @@ -243,7 +244,7 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { } // Recursively add folders and their children. if err := s.addNodes(tree, listResp.CommonPrefixes, bucket); err != nil { - return tree.String(), err + return "", err } return tree.String(), nil } @@ -255,7 +256,7 @@ func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket s listResp := &s3.ListObjectsV2Output{} var err error for _, prefix := range prefixes { - branch := tree.AddBranch(s.extractFileName(aws.StringValue(prefix.Prefix))) + branch := tree.AddBranch(s.extractNameFromPath(aws.StringValue(prefix.Prefix))) for { listParams := &s3.ListObjectsV2Input{ Bucket: aws.String(bucket), @@ -272,7 +273,7 @@ func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket s } } for _, file := range listResp.Contents { - fileName := s.extractFileName(aws.StringValue(file.Key)) + fileName := s.extractNameFromPath(aws.StringValue(file.Key)) branch.AddNode(fileName) } if err := s.addNodes(branch, listResp.CommonPrefixes, bucket); err != nil { @@ -282,9 +283,9 @@ func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket s return nil } -// extractFileName returns the last element of a path, whether the name of -// an individual file or the end of an S3 prefix. -func (s *S3) extractFileName(path string) string { +// extractNameFromPath returns the last element of a path, whether the name of +// an individual file or the end of an S3 prefix (folder name). +func (s *S3) extractNameFromPath(path string) string { path, _ = strings.CutSuffix(path, `/`) if strings.Contains(path, `/`) { split := strings.Split(path, `/`) From 6c8831aa42432a1464b406af6a847eb823a3c3a2 Mon Sep 17 00:00:00 2001 From: Huang Date: Wed, 7 Jun 2023 15:48:25 -0700 Subject: [PATCH 06/13] chore: address ph fb part 1 --- internal/pkg/aws/s3/s3.go | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index ae533095001..dc578c53527 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -31,6 +31,9 @@ const ( // Object location prefixes. s3URIPrefix = "s3://" + + // Delimiter for ListObjectsV2Input. + slashDelimiter = "/" ) type s3ManagerAPI interface { @@ -79,7 +82,6 @@ func (s *S3) EmptyBucket(bucket string) error { var listResp *s3.ListObjectVersionsOutput var err error - // isBucket checks to make sure the bucket exists before proceeding to empty it. isBucket, err := s.isBucket(bucket) if err != nil { return fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) @@ -192,7 +194,6 @@ func FormatARN(partition, location string) string { return fmt.Sprintf("arn:%s:s3:::%s", partition, location) } -// isBucket checks whether the bucket exists before emptying the bucket func (s *S3) isBucket(bucket string) (bool, error) { input := &s3.HeadBucketInput{ Bucket: aws.String(bucket), @@ -210,10 +211,6 @@ func (s *S3) isBucket(bucket string) (bool, error) { // GetBucketTree retrieves the objects in an S3 bucket and creates an ASCII tree representing their folder structure. func (s *S3) GetBucketTree(bucket string) (string, error) { - listResp := &s3.ListObjectsV2Output{} - var err error - - // isBucket check to make sure the bucket exists before proceeding. exists, err := s.isBucket(bucket) if err != nil { return "", fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) @@ -222,10 +219,11 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { return "", nil } + listResp := &s3.ListObjectsV2Output{} for { listParams := &s3.ListObjectsV2Input{ Bucket: aws.String(bucket), - Delimiter: aws.String("/"), + Delimiter: aws.String(slashDelimiter), ContinuationToken: listResp.NextContinuationToken, } listResp, err = s.s3Client.ListObjectsV2(listParams) @@ -256,11 +254,11 @@ func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket s listResp := &s3.ListObjectsV2Output{} var err error for _, prefix := range prefixes { - branch := tree.AddBranch(s.extractNameFromPath(aws.StringValue(prefix.Prefix))) + branch := tree.AddBranch(filepath.Base(aws.StringValue(prefix.Prefix))) for { listParams := &s3.ListObjectsV2Input{ Bucket: aws.String(bucket), - Delimiter: aws.String("/"), + Delimiter: aws.String(slashDelimiter), ContinuationToken: listResp.ContinuationToken, Prefix: prefix.Prefix, } @@ -273,7 +271,7 @@ func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket s } } for _, file := range listResp.Contents { - fileName := s.extractNameFromPath(aws.StringValue(file.Key)) + fileName := filepath.Base(aws.StringValue(file.Key)) branch.AddNode(fileName) } if err := s.addNodes(branch, listResp.CommonPrefixes, bucket); err != nil { @@ -283,17 +281,6 @@ func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket s return nil } -// extractNameFromPath returns the last element of a path, whether the name of -// an individual file or the end of an S3 prefix (folder name). -func (s *S3) extractNameFromPath(path string) string { - path, _ = strings.CutSuffix(path, `/`) - if strings.Contains(path, `/`) { - split := strings.Split(path, `/`) - return split[len(split)-1] - } - return path -} - func (s *S3) upload(bucket, key string, buf io.Reader) (string, error) { in := &s3manager.UploadInput{ Body: buf, From ecce7902c19bd215d748f11cfcfd503167de927d Mon Sep 17 00:00:00 2001 From: Huang Date: Wed, 7 Jun 2023 17:38:23 -0700 Subject: [PATCH 07/13] chore: collect all resp items if cont. tokens --- internal/pkg/aws/s3/s3.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index dc578c53527..f0470df94c4 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -219,6 +219,8 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { return "", nil } + var contents []*s3.Object + var prefixes []*s3.CommonPrefix listResp := &s3.ListObjectsV2Output{} for { listParams := &s3.ListObjectsV2Input{ @@ -230,6 +232,8 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { if err != nil { return "", fmt.Errorf("list objects for bucket %s: %w", bucket, err) } + contents = append(contents, listResp.Contents...) + prefixes = append(prefixes, listResp.CommonPrefixes...) if listResp.NextContinuationToken == nil { break } @@ -237,11 +241,11 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { tree := treeprint.New() // Add top-level files. - for _, object := range listResp.Contents { + for _, object := range contents { tree.AddNode(aws.StringValue(object.Key)) } // Recursively add folders and their children. - if err := s.addNodes(tree, listResp.CommonPrefixes, bucket); err != nil { + if err := s.addNodes(tree, prefixes, bucket); err != nil { return "", err } return tree.String(), nil @@ -251,9 +255,12 @@ func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket s if len(prefixes) == 0 { return nil } + listResp := &s3.ListObjectsV2Output{} var err error for _, prefix := range prefixes { + var respContents []*s3.Object + var respPrefixes []*s3.CommonPrefix branch := tree.AddBranch(filepath.Base(aws.StringValue(prefix.Prefix))) for { listParams := &s3.ListObjectsV2Input{ @@ -266,15 +273,17 @@ func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket s if err != nil { return fmt.Errorf("list objects for bucket %s: %w", bucket, err) } + respContents = append(respContents, listResp.Contents...) + respPrefixes = append(respPrefixes, listResp.CommonPrefixes...) if listResp.NextContinuationToken == nil { break } } - for _, file := range listResp.Contents { + for _, file := range respContents { fileName := filepath.Base(aws.StringValue(file.Key)) branch.AddNode(fileName) } - if err := s.addNodes(branch, listResp.CommonPrefixes, bucket); err != nil { + if err := s.addNodes(branch, respPrefixes, bucket); err != nil { return err } } From 490bb2d0e217cb719bcf84a68a389613f99c4b75 Mon Sep 17 00:00:00 2001 From: Huang Date: Wed, 7 Jun 2023 17:40:50 -0700 Subject: [PATCH 08/13] chore: rename method --- internal/pkg/aws/s3/s3.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index f0470df94c4..888162f31dd 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -82,12 +82,12 @@ func (s *S3) EmptyBucket(bucket string) error { var listResp *s3.ListObjectVersionsOutput var err error - isBucket, err := s.isBucket(bucket) + bucketExists, err := s.bucketExists(bucket) if err != nil { return fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) } - if !isBucket { + if !bucketExists { return nil } @@ -194,7 +194,7 @@ func FormatARN(partition, location string) string { return fmt.Sprintf("arn:%s:s3:::%s", partition, location) } -func (s *S3) isBucket(bucket string) (bool, error) { +func (s *S3) bucketExists(bucket string) (bool, error) { input := &s3.HeadBucketInput{ Bucket: aws.String(bucket), } @@ -211,7 +211,7 @@ func (s *S3) isBucket(bucket string) (bool, error) { // GetBucketTree retrieves the objects in an S3 bucket and creates an ASCII tree representing their folder structure. func (s *S3) GetBucketTree(bucket string) (string, error) { - exists, err := s.isBucket(bucket) + exists, err := s.bucketExists(bucket) if err != nil { return "", fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) } From 10b0d96bf6eb8316909be6fe548c931c7c3e7da9 Mon Sep 17 00:00:00 2001 From: Huang Date: Wed, 7 Jun 2023 17:42:28 -0700 Subject: [PATCH 09/13] chore: unwrap err --- internal/pkg/aws/s3/s3.go | 2 +- internal/pkg/aws/s3/s3_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index 888162f31dd..9e22d94c518 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -213,7 +213,7 @@ func (s *S3) bucketExists(bucket string) (bool, error) { func (s *S3) GetBucketTree(bucket string) (string, error) { exists, err := s.bucketExists(bucket) if err != nil { - return "", fmt.Errorf("unable to determine the existence of bucket %s: %w", bucket, err) + return "", err } if !exists { return "", nil diff --git a/internal/pkg/aws/s3/s3_test.go b/internal/pkg/aws/s3/s3_test.go index 78f5cfc0305..0c186d1574f 100644 --- a/internal/pkg/aws/s3/s3_test.go +++ b/internal/pkg/aws/s3/s3_test.go @@ -612,7 +612,7 @@ func TestS3_GetBucketTree(t *testing.T) { setupMocks: func(m s3Mocks) { m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, errors.New("some error")) }, - wantErr: errors.New("unable to determine the existence of bucket bucketName: some error"), + wantErr: errors.New("some error"), }, "should wrap error if fail to list objects": { setupMocks: func(m s3Mocks) { From faa1bb83306cc6743dca3551351d643e5a7eb0e8 Mon Sep 17 00:00:00 2001 From: Huang Date: Thu, 8 Jun 2023 10:34:21 -0700 Subject: [PATCH 10/13] chore: add unit test for pagination --- internal/pkg/aws/s3/s3_test.go | 42 ++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/internal/pkg/aws/s3/s3_test.go b/internal/pkg/aws/s3/s3_test.go index 0c186d1574f..011342a37ad 100644 --- a/internal/pkg/aws/s3/s3_test.go +++ b/internal/pkg/aws/s3/s3_test.go @@ -497,6 +497,7 @@ func TestS3_GetBucketTree(t *testing.T) { mockBucket := aws.String("bucketName") delimiter := aws.String("/") nonexistentError := awserr.New(errCodeNotFound, "msg", errors.New("some error")) + mockContinuationToken := "next" firstResp := s3.ListObjectsV2Output{ CommonPrefixes: []*s3.CommonPrefix{ @@ -601,6 +602,47 @@ func TestS3_GetBucketTree(t *testing.T) { └── top └── middle └── bottom.html +`, + }, + "should handle multiple pages of objects": { + setupMocks: func(m s3Mocks) { + m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, nil) + m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: mockBucket, + ContinuationToken: nil, + Delimiter: delimiter, + Prefix: nil, + }).Return( + &s3.ListObjectsV2Output{ + Contents: []*s3.Object{ + {Key: aws.String("README.md")}, + }, + Delimiter: delimiter, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + NextContinuationToken: &mockContinuationToken, + }, nil) + m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: mockBucket, + ContinuationToken: &mockContinuationToken, + Delimiter: delimiter, + Prefix: nil, + }).Return( + &s3.ListObjectsV2Output{ + Contents: []*s3.Object{ + {Key: aws.String("READMETOO.md")}, + }, + Delimiter: delimiter, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + NextContinuationToken: nil, + }, nil) + }, + wantTree: `. +├── README.md +└── READMETOO.md `, }, "return nil if bucket doesn't exist": { From 40dceaadc66e7daaa77b1cb38a9c8af68ceab347 Mon Sep 17 00:00:00 2001 From: Huang Date: Fri, 9 Jun 2023 16:15:18 -0700 Subject: [PATCH 11/13] chore: address wx fb --- internal/pkg/describe/service.go | 8 ++++--- internal/pkg/describe/static_site.go | 27 ++++++++++++++--------- internal/pkg/describe/static_site_test.go | 3 +-- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/internal/pkg/describe/service.go b/internal/pkg/describe/service.go index f2001e601b1..124276a7fe0 100644 --- a/internal/pkg/describe/service.go +++ b/internal/pkg/describe/service.go @@ -17,6 +17,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/aws/apprunner" "github.com/aws/copilot-cli/internal/pkg/aws/cloudwatch" awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" + "github.com/aws/copilot-cli/internal/pkg/aws/sessions" "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/describe/stack" "github.com/aws/copilot-cli/internal/pkg/ecs" @@ -127,9 +128,10 @@ type appRunnerServiceDescriber struct { // NewServiceConfig contains fields that initiates service describer struct. type NewServiceConfig struct { - App string - Svc string - ConfigStore ConfigStoreSvc + App string + Svc string + ConfigStore ConfigStoreSvc + SessionProvider *sessions.Provider EnableResources bool DeployStore DeployedEnvServicesLister diff --git a/internal/pkg/describe/static_site.go b/internal/pkg/describe/static_site.go index 2d4f3e32d10..f8acd4f47f3 100644 --- a/internal/pkg/describe/static_site.go +++ b/internal/pkg/describe/static_site.go @@ -33,24 +33,17 @@ type StaticSiteDescriber struct { store DeployedEnvServicesLister initWkldStackDescriber func(string) (workloadDescriber, error) wkldDescribers map[string]workloadDescriber - awsS3Client bucketDescriber - s3Client bucketNameGetter + initS3Client func(string) (bucketDescriber, bucketNameGetter, error) } // NewStaticSiteDescriber instantiates a static site service describer. func NewStaticSiteDescriber(opt NewServiceConfig) (*StaticSiteDescriber, error) { - defaultSess, err := sessions.ImmutableProvider().Default() - if err != nil { - return nil, fmt.Errorf("default session: %v", err) - } describer := &StaticSiteDescriber{ app: opt.App, svc: opt.Svc, enableResources: opt.EnableResources, store: opt.DeployStore, wkldDescribers: make(map[string]workloadDescriber), - awsS3Client: awsS3.New(defaultSess), - s3Client: s3.New(defaultSess), } describer.initWkldStackDescriber = func(env string) (workloadDescriber, error) { if describer, ok := describer.wkldDescribers[env]; ok { @@ -67,6 +60,17 @@ func NewStaticSiteDescriber(opt NewServiceConfig) (*StaticSiteDescriber, error) describer.wkldDescribers[env] = svcDescr return svcDescr, nil } + describer.initS3Client = func(env string) (bucketDescriber, bucketNameGetter, error) { + environment, err := opt.ConfigStore.GetEnvironment(opt.App, env) + if err != nil { + return nil, nil, fmt.Errorf("get environment %s: %w", env, err) + } + sess, err := sessions.ImmutableProvider().FromRole(environment.ManagerRoleARN, environment.Region) + if err != nil { + return nil, nil, err + } + return awsS3.New(sess), s3.New(sess), nil + } return describer, nil } @@ -99,6 +103,7 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { var routes []*WebServiceRoute var objects []*S3ObjectTree for _, env := range environments { + bucketDescriber, bucketNameDescriber, err := d.initS3Client(env) uri, err := d.URI(env) if err != nil { return nil, fmt.Errorf("retrieve service URI: %w", err) @@ -109,11 +114,11 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { URL: uri.URI, }) } - bucketName, err := d.s3Client.BucketName(d.app, env, d.svc) + bucketName, err := bucketNameDescriber.BucketName(d.app, env, d.svc) if err != nil { return nil, fmt.Errorf("get bucket name for %q env: %w", env, err) } - tree, err := d.awsS3Client.GetBucketTree(bucketName) + tree, err := bucketDescriber.GetBucketTree(bucketName) if err != nil { return nil, fmt.Errorf("get tree representation of bucket contents: %w", err) } @@ -213,8 +218,8 @@ func (w *staticSiteDesc) HumanString() string { for _, object := range w.Objects { fmt.Fprintf(writer, "\n %s\t%s\n", "Environment", object.Environment) fmt.Fprintf(writer, object.Tree) - writer.Flush() } + writer.Flush() } if len(w.Resources) != 0 { fmt.Fprint(writer, color.Bold.Sprint("\nResources\n")) diff --git a/internal/pkg/describe/static_site_test.go b/internal/pkg/describe/static_site_test.go index 4d604832929..d9db6e8f8cf 100644 --- a/internal/pkg/describe/static_site_test.go +++ b/internal/pkg/describe/static_site_test.go @@ -264,8 +264,7 @@ Resources store: mocks.store, initWkldStackDescriber: func(string) (workloadDescriber, error) { return mocks.wkldDescriber, nil }, wkldDescribers: make(map[string]workloadDescriber), - awsS3Client: mocks.awsS3Client, - s3Client: mocks.s3Client, + initS3Client: func(string) (bucketDescriber, bucketNameGetter, error) { return mocks.awsS3Client, mocks.s3Client, nil }, } // WHEN From 1b21114a93166de7828992365166b87cd9ba0190 Mon Sep 17 00:00:00 2001 From: Huang Date: Fri, 9 Jun 2023 16:19:03 -0700 Subject: [PATCH 12/13] chore: catch err --- internal/pkg/describe/static_site.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/pkg/describe/static_site.go b/internal/pkg/describe/static_site.go index f8acd4f47f3..bf7d7c83ca9 100644 --- a/internal/pkg/describe/static_site.go +++ b/internal/pkg/describe/static_site.go @@ -104,6 +104,9 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { var objects []*S3ObjectTree for _, env := range environments { bucketDescriber, bucketNameDescriber, err := d.initS3Client(env) + if err != nil { + return nil, err + } uri, err := d.URI(env) if err != nil { return nil, fmt.Errorf("retrieve service URI: %w", err) From 40fb90b80aff487ffaf0b30c9c6b7749e80eb8be Mon Sep 17 00:00:00 2001 From: Huang Date: Mon, 12 Jun 2023 15:13:34 -0700 Subject: [PATCH 13/13] chore: remove unnecessary field --- internal/pkg/describe/service.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/pkg/describe/service.go b/internal/pkg/describe/service.go index 124276a7fe0..f2001e601b1 100644 --- a/internal/pkg/describe/service.go +++ b/internal/pkg/describe/service.go @@ -17,7 +17,6 @@ import ( "github.com/aws/copilot-cli/internal/pkg/aws/apprunner" "github.com/aws/copilot-cli/internal/pkg/aws/cloudwatch" awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" - "github.com/aws/copilot-cli/internal/pkg/aws/sessions" "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/describe/stack" "github.com/aws/copilot-cli/internal/pkg/ecs" @@ -128,10 +127,9 @@ type appRunnerServiceDescriber struct { // NewServiceConfig contains fields that initiates service describer struct. type NewServiceConfig struct { - App string - Svc string - ConfigStore ConfigStoreSvc - SessionProvider *sessions.Provider + App string + Svc string + ConfigStore ConfigStoreSvc EnableResources bool DeployStore DeployedEnvServicesLister