From eab904e98ae007509e0295c46e5205b2cc2b03c7 Mon Sep 17 00:00:00 2001 From: Huang Date: Tue, 13 Jun 2023 10:52:53 -0700 Subject: [PATCH 01/10] chore: add svc status for static sites --- internal/pkg/aws/cloudwatch/cloudwatch.go | 10 +- internal/pkg/aws/s3/s3.go | 36 ++++++- internal/pkg/cli/svc_status.go | 15 ++- internal/pkg/describe/service.go | 4 + internal/pkg/describe/status.go | 35 ++++++- internal/pkg/describe/status_describe.go | 112 +++++++++++++++++----- 6 files changed, 176 insertions(+), 36 deletions(-) diff --git a/internal/pkg/aws/cloudwatch/cloudwatch.go b/internal/pkg/aws/cloudwatch/cloudwatch.go index cfba4438bc5..e1f55d1fb35 100644 --- a/internal/pkg/aws/cloudwatch/cloudwatch.go +++ b/internal/pkg/aws/cloudwatch/cloudwatch.go @@ -54,9 +54,9 @@ type AlarmStatus struct { // AlarmDescription contains CloudWatch alarm config. // Also available: MetricName, ComparisonOperator, DatapointsToAlarm, EvaluationPeriods, Threshold, Unit. type AlarmDescription struct { - Name string `json:"name"` - Description string `json:"description"` - Environment string `json:"environment"` + Name string `json:"name"` + Description string `json:"description"` + Environment string `json:"environment"` } // New returns a CloudWatch struct configured against the input session. @@ -181,8 +181,8 @@ func (cw *CloudWatch) metricAlarmsDescriptions(alarms []*cloudwatch.MetricAlarm) continue } alarmDescriptionsList = append(alarmDescriptionsList, &AlarmDescription{ - Name: aws.StringValue(alarm.AlarmName), - Description: aws.StringValue(alarm.AlarmDescription), + Name: aws.StringValue(alarm.AlarmName), + Description: aws.StringValue(alarm.AlarmDescription), }) } return alarmDescriptionsList diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index 9e22d94c518..40d6a707e5e 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/dustin/go-humanize" "github.com/xlab/treeprint" "io" "mime" @@ -218,7 +219,6 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { if !exists { return "", nil } - var contents []*s3.Object var prefixes []*s3.CommonPrefix listResp := &s3.ListObjectsV2Output{} @@ -251,6 +251,40 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { return tree.String(), nil } +// GetBucketSizeAndCount returns the total size and number of objects in an S3 bucket. +func (s *S3) GetBucketSizeAndCount(bucket string) (string, int, error) { + exists, err := s.bucketExists(bucket) + if err != nil { + return "", 0, err + } + if !exists { + return "", 0, nil + } + var objects []*s3.Object + var size int64 + var number int + listResp := &s3.ListObjectsV2Output{} + for { + listParams := &s3.ListObjectsV2Input{ + Bucket: aws.String(bucket), + ContinuationToken: listResp.NextContinuationToken, + } + listResp, err = s.s3Client.ListObjectsV2(listParams) + if err != nil { + return "", 0, fmt.Errorf("list objects for bucket %s: %w", bucket, err) + } + objects = append(objects, listResp.Contents...) + if listResp.NextContinuationToken == nil { + break + } + } + for _, object := range objects { + size = size + aws.Int64Value(object.Size) + number = number + 1 + } + return humanize.Bytes(uint64(size)), number, nil +} + func (s *S3) addNodes(tree treeprint.Tree, prefixes []*s3.CommonPrefix, bucket string) error { if len(prefixes) == 0 { return nil diff --git a/internal/pkg/cli/svc_status.go b/internal/pkg/cli/svc_status.go index c6dde4388a7..6e20d542b03 100644 --- a/internal/pkg/cli/svc_status.go +++ b/internal/pkg/cli/svc_status.go @@ -74,7 +74,18 @@ func newSvcStatusOpts(vars svcStatusVars) (*svcStatusOpts, error) { ConfigStore: configStore, }) if err != nil { - return fmt.Errorf("creating status describer for apprunner service %s in application %s: %w", o.svcName, o.appName, err) + return fmt.Errorf("create status describer for App Runner service %s in application %s: %w", o.svcName, o.appName, err) + } + o.statusDescriber = d + } else if wkld.Type == manifestinfo.StaticSiteType { + d, err := describe.NewStaticSiteStatusDescriber(&describe.NewServiceStatusConfig{ + App: o.appName, + Env: o.envName, + Svc: o.svcName, + ConfigStore: configStore, + }) + if err != nil { + return fmt.Errorf("create status describer for Static Site service %s in application %s: %w", o.svcName, o.appName, err) } o.statusDescriber = d } else { @@ -85,7 +96,7 @@ func newSvcStatusOpts(vars svcStatusVars) (*svcStatusOpts, error) { ConfigStore: configStore, }) if err != nil { - return fmt.Errorf("creating status describer for service %s in application %s: %w", o.svcName, o.appName, err) + return fmt.Errorf("create status describer for service %s in application %s: %w", o.svcName, o.appName, err) } o.statusDescriber = d } diff --git a/internal/pkg/describe/service.go b/internal/pkg/describe/service.go index f2001e601b1..79d4758ec94 100644 --- a/internal/pkg/describe/service.go +++ b/internal/pkg/describe/service.go @@ -95,6 +95,10 @@ type bucketDescriber interface { GetBucketTree(bucket string) (string, error) } +type bucketDataGetter interface { + GetBucketSizeAndCount(bucket string) (string, int, error) +} + type bucketNameGetter interface { BucketName(app, env, svc string) (string, error) } diff --git a/internal/pkg/describe/status.go b/internal/pkg/describe/status.go index f95e7c3f116..978fe911e20 100644 --- a/internal/pkg/describe/status.go +++ b/internal/pkg/describe/status.go @@ -48,12 +48,19 @@ type ecsServiceStatus struct { TargetHealthDescriptions []taskTargetHealth `json:"targetHealthDescriptions"` } -// appRunnerServiceStatus contains the status for an AppRunner service. +// appRunnerServiceStatus contains the status for an App Runner service. type appRunnerServiceStatus struct { Service apprunner.Service LogEvents []*cloudwatchlogs.Event } +// staticSiteServiceStatus contains the status for a Static Site service. +type staticSiteServiceStatus struct { + BucketName string `json:"bucketName"` + Size string `json:"totalSize"` + Count int `json:"totalObjects"` +} + type taskTargetHealth struct { HealthStatus elbv2.HealthStatus `json:"healthStatus"` TaskID string `json:"taskID"` // TaskID is empty if the target cannot be traced to a task. @@ -97,7 +104,16 @@ func (a *appRunnerServiceStatus) JSONString() (string, error) { return fmt.Sprintf("%s\n", b), nil } -// HumanString returns the stringified ecsServiceStatus struct with human readable format. +// JSONString returns the stringified staticSiteServiceStatus struct with json format. +func (s *staticSiteServiceStatus) JSONString() (string, error) { + b, err := json.Marshal(s) + if err != nil { + return "", fmt.Errorf("marshal services: %w", err) + } + return fmt.Sprintf("%s\n", b), nil +} + +// HumanString returns the stringified ecsServiceStatus struct in human-readable format. func (s *ecsServiceStatus) HumanString() string { var b bytes.Buffer writer := tabwriter.NewWriter(&b, statusMinCellWidth, tabWidth, statusCellPaddingWidth, paddingChar, noAdditionalFormatting) @@ -130,7 +146,7 @@ func (s *ecsServiceStatus) HumanString() string { return b.String() } -// HumanString returns the stringified appRunnerServiceStatus struct with human readable format. +// HumanString returns the stringified appRunnerServiceStatus struct in human-readable format. func (a *appRunnerServiceStatus) HumanString() string { var b bytes.Buffer writer := tabwriter.NewWriter(&b, minCellWidth, tabWidth, statusCellPaddingWidth, paddingChar, noAdditionalFormatting) @@ -159,6 +175,19 @@ func (a *appRunnerServiceStatus) HumanString() string { return b.String() } +// HumanString returns the stringified staticSiteServiceStatus struct in human-readable format. +func (s *staticSiteServiceStatus) HumanString() string { + var b bytes.Buffer + writer := tabwriter.NewWriter(&b, minCellWidth, tabWidth, statusCellPaddingWidth, paddingChar, noAdditionalFormatting) + fmt.Fprint(writer, color.Bold.Sprint("Bucket Summary\n\n")) + writer.Flush() + fmt.Fprintf(writer, " Bucket Name %s\n", s.BucketName) + fmt.Fprintf(writer, " Total Objects %s\n", strconv.Itoa(s.Count)) + fmt.Fprintf(writer, " Total Size %s\n", s.Size) + writer.Flush() + return b.String() +} + func (s *ecsServiceStatus) writeTaskSummary(writer io.Writer) { // NOTE: all the `bar` need to be fully colored. Observe how all the second parameter for all `summaryBar` function // is a list of strings that are colored (e.g. `[]string{color.Green.Sprint("■"), color.Grey.Sprint("□")}`) diff --git a/internal/pkg/describe/status_describe.go b/internal/pkg/describe/status_describe.go index 46de2a8a91a..3cadd6cb86c 100644 --- a/internal/pkg/describe/status_describe.go +++ b/internal/pkg/describe/status_describe.go @@ -5,6 +5,8 @@ package describe import ( "fmt" + awsS3 "github.com/aws/copilot-cli/internal/pkg/aws/s3" + "github.com/aws/copilot-cli/internal/pkg/s3" "sort" "github.com/aws/aws-sdk-go/aws" @@ -71,6 +73,15 @@ type appRunnerStatusDescriber struct { eventsGetter logGetter } +type staticSiteStatusDescriber struct { + app string + env string + svc string + + svcDescriber *StaticSiteDescriber + initS3Client func(string) (bucketDataGetter, bucketNameGetter, error) +} + // NewServiceStatusConfig contains fields that initiates ServiceStatus struct. type NewServiceStatusConfig struct { App string @@ -103,7 +114,7 @@ func NewECSStatusDescriber(opt *NewServiceStatusConfig) (*ecsStatusDescriber, er // NewAppRunnerStatusDescriber instantiates a new appRunnerStatusDescriber struct. func NewAppRunnerStatusDescriber(opt *NewServiceStatusConfig) (*appRunnerStatusDescriber, error) { - ecsSvcDescriber, err := newAppRunnerServiceDescriber(NewServiceConfig{ + appRunnerSvcDescriber, err := newAppRunnerServiceDescriber(NewServiceConfig{ App: opt.App, Svc: opt.Svc, @@ -117,12 +128,42 @@ func NewAppRunnerStatusDescriber(opt *NewServiceStatusConfig) (*appRunnerStatusD app: opt.App, env: opt.Env, svc: opt.Svc, - svcDescriber: ecsSvcDescriber, - eventsGetter: cloudwatchlogs.New(ecsSvcDescriber.sess), + svcDescriber: appRunnerSvcDescriber, + eventsGetter: cloudwatchlogs.New(appRunnerSvcDescriber.sess), }, nil } -// Describe returns status of an ECS service. +// NewStaticSiteStatusDescriber instantiates a new staticSiteStatusDescriber struct. +func NewStaticSiteStatusDescriber(opt *NewServiceStatusConfig) (*staticSiteStatusDescriber, error) { + staticSiteSvcDescriber, err := NewStaticSiteDescriber(NewServiceConfig{ + App: opt.App, + Svc: opt.Svc, + //ConfigStore: opt.ConfigStore, + }) + if err != nil { + return nil, err + } + describer := &staticSiteStatusDescriber{ + app: opt.App, + env: opt.Env, + svc: opt.Svc, + svcDescriber: staticSiteSvcDescriber, + } + describer.initS3Client = func(env string) (bucketDataGetter, 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 +} + +// Describe returns the status of an ECS service. func (s *ecsStatusDescriber) Describe() (HumanJSONStringer, error) { svcDesc, err := s.svcDescriber.DescribeService(s.app, s.env, s.svc) if err != nil { @@ -211,6 +252,48 @@ func (s *ecsStatusDescriber) Describe() (HumanJSONStringer, error) { }, nil } +// Describe returns the status of an AppRunner service. +func (a *appRunnerStatusDescriber) Describe() (HumanJSONStringer, error) { + svc, err := a.svcDescriber.Service() + if err != nil { + return nil, fmt.Errorf("get AppRunner service description for App Runner service %s in environment %s: %w", a.svc, a.env, err) + } + logGroupName := fmt.Sprintf(fmtAppRunnerSvcLogGroupName, svc.Name, svc.ID) + logEventsOpts := cloudwatchlogs.LogEventsOpts{ + LogGroup: logGroupName, + Limit: aws.Int64(defaultServiceLogsLimit), + } + logEventsOutput, err := a.eventsGetter.LogEvents(logEventsOpts) + if err != nil { + return nil, fmt.Errorf("get log events for log group %s: %w", logGroupName, err) + } + return &appRunnerServiceStatus{ + Service: *svc, + LogEvents: logEventsOutput.Events, + }, nil +} + +// Describe returns the status of a Static Site service. +func (d *staticSiteStatusDescriber) Describe() (HumanJSONStringer, error) { + bucketDataGetter, bucketNameGetter, err := d.initS3Client(d.env) + if err != nil { + return nil, err + } + bucketName, err := bucketNameGetter.BucketName(d.app, d.env, d.svc) + if err != nil { + return nil, fmt.Errorf("get bucket name for %s env: %w", d.svc, err) + } + size, count, err := bucketDataGetter.GetBucketSizeAndCount(bucketName) + if err != nil { + return nil, fmt.Errorf("get size and count data for %s S3 bucket's contents: %w", bucketName, err) + } + return &staticSiteServiceStatus{ + BucketName: bucketName, + Size: size, + Count: count, + }, nil +} + func (s *ecsStatusDescriber) ecsServiceAutoscalingAlarms(cluster, service string) ([]cloudwatch.AlarmStatus, error) { alarmNames, err := s.aasSvcGetter.ECSServiceAlarmNames(cluster, service) if err != nil { @@ -241,27 +324,6 @@ func (s *ecsStatusDescriber) ecsServiceRollbackAlarms(app, env, svc string) ([]c return alarms, nil } -// Describe returns status of an AppRunner service. -func (a *appRunnerStatusDescriber) Describe() (HumanJSONStringer, error) { - svc, err := a.svcDescriber.Service() - if err != nil { - return nil, fmt.Errorf("get AppRunner service description for App Runner service %s in environment %s: %w", a.svc, a.env, err) - } - logGroupName := fmt.Sprintf(fmtAppRunnerSvcLogGroupName, svc.Name, svc.ID) - logEventsOpts := cloudwatchlogs.LogEventsOpts{ - LogGroup: logGroupName, - Limit: aws.Int64(defaultServiceLogsLimit), - } - logEventsOutput, err := a.eventsGetter.LogEvents(logEventsOpts) - if err != nil { - return nil, fmt.Errorf("get log events for log group %s: %w", logGroupName, err) - } - return &appRunnerServiceStatus{ - Service: *svc, - LogEvents: logEventsOutput.Events, - }, nil -} - // targetHealthForTasks finds the corresponding task, if any, for each target health in a target group. func targetHealthForTasks(targetsHealth []*elbv2.TargetHealth, tasks []*awsecs.Task, targetGroupARN string) []taskTargetHealth { var out []taskTargetHealth From f504e8cb10a34f184f5918429617161464e2fd97 Mon Sep 17 00:00:00 2001 From: Huang Date: Tue, 13 Jun 2023 15:11:45 -0700 Subject: [PATCH 02/10] chore: add unit tests --- internal/pkg/aws/s3/s3_test.go | 144 ++++++++++++++++++ internal/pkg/describe/mocks/mock_service.go | 39 +++++ internal/pkg/describe/status_describe.go | 20 +-- internal/pkg/describe/status_describe_test.go | 75 +++++++++ internal/pkg/describe/status_test.go | 32 ++++ 5 files changed, 295 insertions(+), 15 deletions(-) diff --git a/internal/pkg/aws/s3/s3_test.go b/internal/pkg/aws/s3/s3_test.go index 011342a37ad..514fe9ac8a2 100644 --- a/internal/pkg/aws/s3/s3_test.go +++ b/internal/pkg/aws/s3/s3_test.go @@ -700,3 +700,147 @@ func TestS3_GetBucketTree(t *testing.T) { } } + +func TestS3_GetBucketSizeAndCount(t *testing.T) { + type s3Mocks struct { + s3API *mocks.Mocks3API + s3ManagerAPI *mocks.Mocks3ManagerAPI + } + mockBucket := aws.String("bucketName") + nonexistentError := awserr.New(errCodeNotFound, "msg", errors.New("some error")) + mockContinuationToken := "next" + + resp := s3.ListObjectsV2Output{ + Contents: []*s3.Object{ + { + Key: aws.String("README.md"), + Size: aws.Int64(111111), + }, + { + Key: aws.String("error.html"), + Size: aws.Int64(222222), + }, + { + Key: aws.String("index.html"), + Size: aws.Int64(333333), + }, + }, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + } + + testCases := map[string]struct { + setupMocks func(mocks s3Mocks) + + wantSize string + wantCount int + wantErr error + }{ + "should return correct size and count": { + 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, + Prefix: nil, + }).Return(&resp, nil) + }, + wantSize: "667 kB", + wantCount: 3, + }, + "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, + Prefix: nil, + }).Return( + &s3.ListObjectsV2Output{ + Contents: []*s3.Object{ + { + Key: aws.String("README.md"), + Size: aws.Int64(123), + }, + }, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + NextContinuationToken: &mockContinuationToken, + }, nil) + m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ + Bucket: mockBucket, + ContinuationToken: &mockContinuationToken, + Prefix: nil, + }).Return( + &s3.ListObjectsV2Output{ + Contents: []*s3.Object{ + { + Key: aws.String("READMETOO.md"), + Size: aws.Int64(321), + }, + }, + KeyCount: aws.Int64(14), + MaxKeys: aws.Int64(1000), + Name: mockBucket, + NextContinuationToken: nil, + }, nil) + }, + wantSize: "444 B", + wantCount: 2, + }, + "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("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, + 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) + + gotSize, gotCount, gotErr := service.GetBucketSizeAndCount(aws.StringValue(mockBucket)) + if tc.wantErr != nil { + require.EqualError(t, gotErr, tc.wantErr.Error()) + return + } + require.NoError(t, gotErr) + require.Equal(t, tc.wantSize, gotSize) + require.Equal(t, tc.wantCount, gotCount) + }) + + } +} diff --git a/internal/pkg/describe/mocks/mock_service.go b/internal/pkg/describe/mocks/mock_service.go index 3e8e69b6986..b9b4e03d2b7 100644 --- a/internal/pkg/describe/mocks/mock_service.go +++ b/internal/pkg/describe/mocks/mock_service.go @@ -747,6 +747,45 @@ func (mr *MockbucketDescriberMockRecorder) GetBucketTree(bucket interface{}) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBucketTree", reflect.TypeOf((*MockbucketDescriber)(nil).GetBucketTree), bucket) } +// MockbucketDataGetter is a mock of bucketDataGetter interface. +type MockbucketDataGetter struct { + ctrl *gomock.Controller + recorder *MockbucketDataGetterMockRecorder +} + +// MockbucketDataGetterMockRecorder is the mock recorder for MockbucketDataGetter. +type MockbucketDataGetterMockRecorder struct { + mock *MockbucketDataGetter +} + +// NewMockbucketDataGetter creates a new mock instance. +func NewMockbucketDataGetter(ctrl *gomock.Controller) *MockbucketDataGetter { + mock := &MockbucketDataGetter{ctrl: ctrl} + mock.recorder = &MockbucketDataGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockbucketDataGetter) EXPECT() *MockbucketDataGetterMockRecorder { + return m.recorder +} + +// GetBucketSizeAndCount mocks base method. +func (m *MockbucketDataGetter) GetBucketSizeAndCount(bucket string) (string, int, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetBucketSizeAndCount", bucket) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(int) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetBucketSizeAndCount indicates an expected call of GetBucketSizeAndCount. +func (mr *MockbucketDataGetterMockRecorder) GetBucketSizeAndCount(bucket interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBucketSizeAndCount", reflect.TypeOf((*MockbucketDataGetter)(nil).GetBucketSizeAndCount), bucket) +} + // MockbucketNameGetter is a mock of bucketNameGetter interface. type MockbucketNameGetter struct { ctrl *gomock.Controller diff --git a/internal/pkg/describe/status_describe.go b/internal/pkg/describe/status_describe.go index 3cadd6cb86c..8fd3909ddb5 100644 --- a/internal/pkg/describe/status_describe.go +++ b/internal/pkg/describe/status_describe.go @@ -78,7 +78,6 @@ type staticSiteStatusDescriber struct { env string svc string - svcDescriber *StaticSiteDescriber initS3Client func(string) (bucketDataGetter, bucketNameGetter, error) } @@ -135,19 +134,10 @@ func NewAppRunnerStatusDescriber(opt *NewServiceStatusConfig) (*appRunnerStatusD // NewStaticSiteStatusDescriber instantiates a new staticSiteStatusDescriber struct. func NewStaticSiteStatusDescriber(opt *NewServiceStatusConfig) (*staticSiteStatusDescriber, error) { - staticSiteSvcDescriber, err := NewStaticSiteDescriber(NewServiceConfig{ - App: opt.App, - Svc: opt.Svc, - //ConfigStore: opt.ConfigStore, - }) - if err != nil { - return nil, err - } describer := &staticSiteStatusDescriber{ - app: opt.App, - env: opt.Env, - svc: opt.Svc, - svcDescriber: staticSiteSvcDescriber, + app: opt.App, + env: opt.Env, + svc: opt.Svc, } describer.initS3Client = func(env string) (bucketDataGetter, bucketNameGetter, error) { environment, err := opt.ConfigStore.GetEnvironment(opt.App, env) @@ -281,11 +271,11 @@ func (d *staticSiteStatusDescriber) Describe() (HumanJSONStringer, error) { } bucketName, err := bucketNameGetter.BucketName(d.app, d.env, d.svc) if err != nil { - return nil, fmt.Errorf("get bucket name for %s env: %w", d.svc, err) + return nil, fmt.Errorf("get bucket name for %q Static Site service in %q environment: %w", d.svc, d.env, err) } size, count, err := bucketDataGetter.GetBucketSizeAndCount(bucketName) if err != nil { - return nil, fmt.Errorf("get size and count data for %s S3 bucket's contents: %w", bucketName, err) + return nil, fmt.Errorf("get size and count data for %q S3 bucket: %w", bucketName, err) } return &staticSiteServiceStatus{ BucketName: bucketName, diff --git a/internal/pkg/describe/status_describe_test.go b/internal/pkg/describe/status_describe_test.go index 6eed75e7d4f..19b39c254f2 100644 --- a/internal/pkg/describe/status_describe_test.go +++ b/internal/pkg/describe/status_describe_test.go @@ -31,6 +31,8 @@ type serviceStatusDescriberMocks struct { aas *mocks.MockautoscalingAlarmNamesGetter logGetter *mocks.MocklogGetter targetHealthGetter *mocks.MocktargetHealthGetter + s3Client *mocks.MockbucketNameGetter + bucketDataGetter *mocks.MockbucketDataGetter } func TestServiceStatus_Describe(t *testing.T) { @@ -766,6 +768,79 @@ func TestAppRunnerStatusDescriber_Describe(t *testing.T) { } } +func TestStaticSiteStatusDescriber_Describe(t *testing.T) { + appName := "testapp" + envName := "test" + svcName := "frontend" + mockBucket := "jimmyBuckets" + mockError := errors.New("some error") + + testCases := map[string]struct { + setupMocks func(mocks serviceStatusDescriberMocks) + desc *staticSiteServiceStatus + + wantedError error + wantedContent *staticSiteServiceStatus + }{ + "success": { + setupMocks: func(m serviceStatusDescriberMocks) { + m.s3Client.EXPECT().BucketName(appName, envName, svcName).Return(mockBucket, nil) + m.bucketDataGetter.EXPECT().GetBucketSizeAndCount(mockBucket).Return("mockSize", 123, nil) + }, + wantedContent: &staticSiteServiceStatus{ + BucketName: mockBucket, + Size: "mockSize", + Count: 123, + }, + }, + "error getting bucket name": { + setupMocks: func(m serviceStatusDescriberMocks) { + m.s3Client.EXPECT().BucketName(appName, envName, svcName).Return("", mockError) + }, + wantedError: fmt.Errorf(`get bucket name for "frontend" Static Site service in "test" environment: %w`, mockError), + }, + "error getting bucket size and count": { + setupMocks: func(m serviceStatusDescriberMocks) { + m.s3Client.EXPECT().BucketName(appName, envName, svcName).Return(mockBucket, nil) + m.bucketDataGetter.EXPECT().GetBucketSizeAndCount(mockBucket).Return("", 0, mockError) + }, + wantedError: fmt.Errorf(`get size and count data for "jimmyBuckets" S3 bucket: %w`, mockError), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + //mockSvcDesc := mocks.NewMockstaticSiteDescriber(ctrl) + mocks := serviceStatusDescriberMocks{ + s3Client: mocks.NewMockbucketNameGetter(ctrl), + bucketDataGetter: mocks.NewMockbucketDataGetter(ctrl), + } + tc.setupMocks(mocks) + + d := &staticSiteStatusDescriber{ + app: appName, + env: envName, + svc: svcName, + initS3Client: func(string) (bucketDataGetter, bucketNameGetter, error) { + return mocks.bucketDataGetter, mocks.s3Client, nil + }, + } + + statusDesc, err := d.Describe() + + if tc.wantedError != nil { + require.EqualError(t, err, tc.wantedError.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantedContent, statusDesc, "expected output content match") + } + }) + } +} + func Test_targetHealthForTasks(t *testing.T) { testCases := map[string]struct { inTargetsHealth []*elbv2.TargetHealth diff --git a/internal/pkg/describe/status_test.go b/internal/pkg/describe/status_test.go index 348cc5ce512..3452c142056 100644 --- a/internal/pkg/describe/status_test.go +++ b/internal/pkg/describe/status_test.go @@ -618,6 +618,38 @@ System Logs } } +func TestServiceStatusDesc_StaticSiteServiceString(t *testing.T) { + testCases := map[string]struct { + desc *staticSiteServiceStatus + human string + json string + }{ + "success": { + desc: &staticSiteServiceStatus{ + BucketName: "Jimmy Buckets", + Size: "999 MB", + Count: 22, + }, + human: `Bucket Summary + + Bucket Name Jimmy Buckets + Total Objects 22 + Total Size 999 MB +`, + json: `{"bucketName":"Jimmy Buckets","totalSize":"999 MB","totalObjects":22}` + "\n", + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + json, err := tc.desc.JSONString() + require.NoError(t, err) + require.Equal(t, tc.human, tc.desc.HumanString()) + require.Equal(t, tc.json, json) + }) + } +} + func TestECSTaskStatus_humanString(t *testing.T) { // from the function changes (ex: from "1 month ago" to "2 months ago"). To make our tests stable, oldHumanize := humanizeTime From 55d7a49775ef0f2e522b487d6cb8117763ba90b8 Mon Sep 17 00:00:00 2001 From: Huang Date: Tue, 13 Jun 2023 15:19:05 -0700 Subject: [PATCH 03/10] chore: add unit test for empty bucket --- internal/pkg/aws/s3/s3_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/pkg/aws/s3/s3_test.go b/internal/pkg/aws/s3/s3_test.go index 514fe9ac8a2..5d5ad884021 100644 --- a/internal/pkg/aws/s3/s3_test.go +++ b/internal/pkg/aws/s3/s3_test.go @@ -790,6 +790,21 @@ func TestS3_GetBucketSizeAndCount(t *testing.T) { wantSize: "444 B", wantCount: 2, }, + "empty bucket": { + 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, + Prefix: nil, + }).Return(&s3.ListObjectsV2Output{ + Contents: nil, + Name: mockBucket, + }, nil) + }, + wantSize: "0 B", + wantCount: 0, + }, "return nil if bucket doesn't exist": { setupMocks: func(m s3Mocks) { m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, nonexistentError) From bfc28c408f9cd7aa27e054fd72fb49828b676b81 Mon Sep 17 00:00:00 2001 From: Huang Date: Fri, 16 Jun 2023 11:55:26 -0700 Subject: [PATCH 04/10] chore: separate out listobjects functionality --- internal/pkg/aws/s3/s3.go | 94 +++++++++---------- internal/pkg/aws/s3/s3_test.go | 14 ++- internal/pkg/describe/mocks/mock_service.go | 24 ++--- internal/pkg/describe/service.go | 4 +- internal/pkg/describe/static_site.go | 2 +- internal/pkg/describe/static_site_test.go | 6 +- internal/pkg/describe/status_describe.go | 2 +- internal/pkg/describe/status_describe_test.go | 4 +- 8 files changed, 76 insertions(+), 74 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index 40d6a707e5e..a5f7a4c5814 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -195,50 +195,21 @@ func FormatARN(partition, location string) string { return fmt.Sprintf("arn:%s:s3:::%s", partition, location) } -func (s *S3) bucketExists(bucket string) (bool, error) { - input := &s3.HeadBucketInput{ - Bucket: aws.String(bucket), - } - _, err := s.s3Client.HeadBucket(input) - if err != nil { - if aerr, ok := err.(awserr.Error); ok && aerr.Code() == errCodeNotFound { - return false, nil - } - return false, err - } - - 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) (string, error) { - exists, err := s.bucketExists(bucket) +// BucketTree creates an ASCII tree representing the folder structure of a bucket's objects. +func (s *S3) BucketTree(bucket string) (string, error) { + outputs, err := s.listObjects(bucket, "/") if err != nil { return "", err } - if !exists { + if outputs == nil { return "", nil } var contents []*s3.Object var prefixes []*s3.CommonPrefix - listResp := &s3.ListObjectsV2Output{} - for { - listParams := &s3.ListObjectsV2Input{ - Bucket: aws.String(bucket), - Delimiter: aws.String(slashDelimiter), - ContinuationToken: listResp.NextContinuationToken, - } - listResp, err = s.s3Client.ListObjectsV2(listParams) - 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 - } + for _, output := range outputs { + contents = append(contents, output.Contents...) + prefixes = append(prefixes, output.CommonPrefixes...) } - tree := treeprint.New() // Add top-level files. for _, object := range contents { @@ -251,45 +222,72 @@ func (s *S3) GetBucketTree(bucket string) (string, error) { return tree.String(), nil } -// GetBucketSizeAndCount returns the total size and number of objects in an S3 bucket. -func (s *S3) GetBucketSizeAndCount(bucket string) (string, int, error) { - exists, err := s.bucketExists(bucket) +// BucketSizeAndCount returns the total size and number of objects in an S3 bucket. +func (s *S3) BucketSizeAndCount(bucket string) (string, int, error) { + outputs, err := s.listObjects(bucket, "") if err != nil { return "", 0, err } - if !exists { + if outputs == nil { return "", 0, nil } - var objects []*s3.Object var size int64 var number int + for _, output := range outputs { + for _, object := range output.Contents { + size = size + aws.Int64Value(object.Size) + number = number + 1 + } + } + return humanize.Bytes(uint64(size)), number, nil +} + +func (s *S3) listObjects(bucket, delimiter string) ([]s3.ListObjectsV2Output, error) { + exists, err := s.bucketExists(bucket) + if err != nil { + return nil, err + } + if !exists { + return nil, nil + } + var outputs []s3.ListObjectsV2Output listResp := &s3.ListObjectsV2Output{} for { listParams := &s3.ListObjectsV2Input{ Bucket: aws.String(bucket), + Delimiter: aws.String(delimiter), ContinuationToken: listResp.NextContinuationToken, } listResp, err = s.s3Client.ListObjectsV2(listParams) if err != nil { - return "", 0, fmt.Errorf("list objects for bucket %s: %w", bucket, err) + return nil, fmt.Errorf("list objects for bucket %s: %w", bucket, err) } - objects = append(objects, listResp.Contents...) + outputs = append(outputs, *listResp) if listResp.NextContinuationToken == nil { break } } - for _, object := range objects { - size = size + aws.Int64Value(object.Size) - number = number + 1 + return outputs, nil +} + +func (s *S3) bucketExists(bucket string) (bool, error) { + input := &s3.HeadBucketInput{ + Bucket: aws.String(bucket), + } + _, err := s.s3Client.HeadBucket(input) + if err != nil { + if aerr, ok := err.(awserr.Error); ok && aerr.Code() == errCodeNotFound { + return false, nil + } + return false, err } - return humanize.Bytes(uint64(size)), number, nil + return true, 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 { diff --git a/internal/pkg/aws/s3/s3_test.go b/internal/pkg/aws/s3/s3_test.go index 5d5ad884021..bf1485f1f9f 100644 --- a/internal/pkg/aws/s3/s3_test.go +++ b/internal/pkg/aws/s3/s3_test.go @@ -489,7 +489,7 @@ func TestS3_FormatARN(t *testing.T) { } } -func TestS3_GetBucketTree(t *testing.T) { +func TestS3_BucketTree(t *testing.T) { type s3Mocks struct { s3API *mocks.Mocks3API s3ManagerAPI *mocks.Mocks3ManagerAPI @@ -689,7 +689,7 @@ func TestS3_GetBucketTree(t *testing.T) { } tc.setupMocks(s3mocks) - gotTree, gotErr := service.GetBucketTree(aws.StringValue(mockBucket)) + gotTree, gotErr := service.BucketTree(aws.StringValue(mockBucket)) if tc.wantErr != nil { require.EqualError(t, gotErr, tc.wantErr.Error()) return @@ -701,7 +701,7 @@ func TestS3_GetBucketTree(t *testing.T) { } } -func TestS3_GetBucketSizeAndCount(t *testing.T) { +func TestS3_BucketSizeAndCount(t *testing.T) { type s3Mocks struct { s3API *mocks.Mocks3API s3ManagerAPI *mocks.Mocks3ManagerAPI @@ -742,6 +742,7 @@ func TestS3_GetBucketSizeAndCount(t *testing.T) { m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, nil) m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ Bucket: mockBucket, + Delimiter: aws.String(""), ContinuationToken: nil, Prefix: nil, }).Return(&resp, nil) @@ -754,6 +755,7 @@ func TestS3_GetBucketSizeAndCount(t *testing.T) { m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, nil) m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ Bucket: mockBucket, + Delimiter: aws.String(""), ContinuationToken: nil, Prefix: nil, }).Return( @@ -771,6 +773,7 @@ func TestS3_GetBucketSizeAndCount(t *testing.T) { }, nil) m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ Bucket: mockBucket, + Delimiter: aws.String(""), ContinuationToken: &mockContinuationToken, Prefix: nil, }).Return( @@ -795,6 +798,7 @@ func TestS3_GetBucketSizeAndCount(t *testing.T) { m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, nil) m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ Bucket: mockBucket, + Delimiter: aws.String(""), ContinuationToken: nil, Prefix: nil, }).Return(&s3.ListObjectsV2Output{ @@ -821,8 +825,8 @@ func TestS3_GetBucketSizeAndCount(t *testing.T) { m.s3API.EXPECT().HeadBucket(&s3.HeadBucketInput{Bucket: mockBucket}).Return(&s3.HeadBucketOutput{}, nil) m.s3API.EXPECT().ListObjectsV2(&s3.ListObjectsV2Input{ Bucket: mockBucket, + Delimiter: aws.String(""), ContinuationToken: nil, - Prefix: nil, }).Return(nil, errors.New("some error")) }, wantErr: errors.New("list objects for bucket bucketName: some error"), @@ -847,7 +851,7 @@ func TestS3_GetBucketSizeAndCount(t *testing.T) { } tc.setupMocks(s3mocks) - gotSize, gotCount, gotErr := service.GetBucketSizeAndCount(aws.StringValue(mockBucket)) + gotSize, gotCount, gotErr := service.BucketSizeAndCount(aws.StringValue(mockBucket)) if tc.wantErr != nil { require.EqualError(t, gotErr, tc.wantErr.Error()) return diff --git a/internal/pkg/describe/mocks/mock_service.go b/internal/pkg/describe/mocks/mock_service.go index b9b4e03d2b7..8f94d099966 100644 --- a/internal/pkg/describe/mocks/mock_service.go +++ b/internal/pkg/describe/mocks/mock_service.go @@ -732,19 +732,19 @@ func (m *MockbucketDescriber) EXPECT() *MockbucketDescriberMockRecorder { return m.recorder } -// GetBucketTree mocks base method. -func (m *MockbucketDescriber) GetBucketTree(bucket string) (string, error) { +// BucketTree mocks base method. +func (m *MockbucketDescriber) BucketTree(bucket string) (string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetBucketTree", bucket) + ret := m.ctrl.Call(m, "BucketTree", 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 { +// BucketTree indicates an expected call of BucketTree. +func (mr *MockbucketDescriberMockRecorder) BucketTree(bucket interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBucketTree", reflect.TypeOf((*MockbucketDescriber)(nil).GetBucketTree), bucket) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BucketTree", reflect.TypeOf((*MockbucketDescriber)(nil).BucketTree), bucket) } // MockbucketDataGetter is a mock of bucketDataGetter interface. @@ -770,20 +770,20 @@ func (m *MockbucketDataGetter) EXPECT() *MockbucketDataGetterMockRecorder { return m.recorder } -// GetBucketSizeAndCount mocks base method. -func (m *MockbucketDataGetter) GetBucketSizeAndCount(bucket string) (string, int, error) { +// BucketSizeAndCount mocks base method. +func (m *MockbucketDataGetter) BucketSizeAndCount(bucket string) (string, int, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetBucketSizeAndCount", bucket) + ret := m.ctrl.Call(m, "BucketSizeAndCount", bucket) ret0, _ := ret[0].(string) ret1, _ := ret[1].(int) ret2, _ := ret[2].(error) return ret0, ret1, ret2 } -// GetBucketSizeAndCount indicates an expected call of GetBucketSizeAndCount. -func (mr *MockbucketDataGetterMockRecorder) GetBucketSizeAndCount(bucket interface{}) *gomock.Call { +// BucketSizeAndCount indicates an expected call of BucketSizeAndCount. +func (mr *MockbucketDataGetterMockRecorder) BucketSizeAndCount(bucket interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBucketSizeAndCount", reflect.TypeOf((*MockbucketDataGetter)(nil).GetBucketSizeAndCount), bucket) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BucketSizeAndCount", reflect.TypeOf((*MockbucketDataGetter)(nil).BucketSizeAndCount), bucket) } // MockbucketNameGetter is a mock of bucketNameGetter interface. diff --git a/internal/pkg/describe/service.go b/internal/pkg/describe/service.go index 79d4758ec94..89ac09c050b 100644 --- a/internal/pkg/describe/service.go +++ b/internal/pkg/describe/service.go @@ -92,11 +92,11 @@ type cwAlarmDescriber interface { } type bucketDescriber interface { - GetBucketTree(bucket string) (string, error) + BucketTree(bucket string) (string, error) } type bucketDataGetter interface { - GetBucketSizeAndCount(bucket string) (string, int, error) + BucketSizeAndCount(bucket string) (string, int, error) } type bucketNameGetter interface { diff --git a/internal/pkg/describe/static_site.go b/internal/pkg/describe/static_site.go index bf7d7c83ca9..7014fe5f722 100644 --- a/internal/pkg/describe/static_site.go +++ b/internal/pkg/describe/static_site.go @@ -121,7 +121,7 @@ func (d *StaticSiteDescriber) Describe() (HumanJSONStringer, error) { if err != nil { return nil, fmt.Errorf("get bucket name for %q env: %w", env, err) } - tree, err := bucketDescriber.GetBucketTree(bucketName) + tree, err := bucketDescriber.BucketTree(bucketName) if err != nil { return nil, fmt.Errorf("get tree representation of bucket contents: %w", err) } diff --git a/internal/pkg/describe/static_site_test.go b/internal/pkg/describe/static_site_test.go index d9db6e8f8cf..9979bb64edf 100644 --- a/internal/pkg/describe/static_site_test.go +++ b/internal/pkg/describe/static_site_test.go @@ -135,7 +135,7 @@ func TestStaticSiteDescriber_Describe(t *testing.T) { "CloudFrontDistributionDomainName": "dut843shvcmvn.cloudfront.net", }, nil), m.s3Client.EXPECT().BucketName(mockApp, mockEnv, mockSvc).Return(mockBucket, nil), - m.awsS3Client.EXPECT().GetBucketTree(mockBucket).Return("", nil), + m.awsS3Client.EXPECT().BucketTree(mockBucket).Return("", nil), ) }, wantedHuman: `About @@ -161,7 +161,7 @@ Routes "CloudFrontDistributionDomainName": "dut843shvcmvn.cloudfront.net", }, nil), m.s3Client.EXPECT().BucketName(mockApp, mockEnv, mockSvc).Return(mockBucket, nil), - m.awsS3Client.EXPECT().GetBucketTree(mockBucket).Return("", nil), + m.awsS3Client.EXPECT().BucketTree(mockBucket).Return("", nil), m.wkldDescriber.EXPECT().StackResources().Return(nil, mockErr), ) }, @@ -176,7 +176,7 @@ Routes "CloudFrontDistributionDomainName": "dut843shvcmvn.cloudfront.net", }, nil), m.s3Client.EXPECT().BucketName(mockApp, mockEnv, mockSvc).Return(mockBucket, nil), - m.awsS3Client.EXPECT().GetBucketTree(mockBucket).Return(`. + m.awsS3Client.EXPECT().BucketTree(mockBucket).Return(`. ├── README.md ├── error.html ├── index.html diff --git a/internal/pkg/describe/status_describe.go b/internal/pkg/describe/status_describe.go index 8fd3909ddb5..3238bbc84c3 100644 --- a/internal/pkg/describe/status_describe.go +++ b/internal/pkg/describe/status_describe.go @@ -273,7 +273,7 @@ func (d *staticSiteStatusDescriber) Describe() (HumanJSONStringer, error) { if err != nil { return nil, fmt.Errorf("get bucket name for %q Static Site service in %q environment: %w", d.svc, d.env, err) } - size, count, err := bucketDataGetter.GetBucketSizeAndCount(bucketName) + size, count, err := bucketDataGetter.BucketSizeAndCount(bucketName) if err != nil { return nil, fmt.Errorf("get size and count data for %q S3 bucket: %w", bucketName, err) } diff --git a/internal/pkg/describe/status_describe_test.go b/internal/pkg/describe/status_describe_test.go index 19b39c254f2..7459d90ea9a 100644 --- a/internal/pkg/describe/status_describe_test.go +++ b/internal/pkg/describe/status_describe_test.go @@ -785,7 +785,7 @@ func TestStaticSiteStatusDescriber_Describe(t *testing.T) { "success": { setupMocks: func(m serviceStatusDescriberMocks) { m.s3Client.EXPECT().BucketName(appName, envName, svcName).Return(mockBucket, nil) - m.bucketDataGetter.EXPECT().GetBucketSizeAndCount(mockBucket).Return("mockSize", 123, nil) + m.bucketDataGetter.EXPECT().BucketSizeAndCount(mockBucket).Return("mockSize", 123, nil) }, wantedContent: &staticSiteServiceStatus{ BucketName: mockBucket, @@ -802,7 +802,7 @@ func TestStaticSiteStatusDescriber_Describe(t *testing.T) { "error getting bucket size and count": { setupMocks: func(m serviceStatusDescriberMocks) { m.s3Client.EXPECT().BucketName(appName, envName, svcName).Return(mockBucket, nil) - m.bucketDataGetter.EXPECT().GetBucketSizeAndCount(mockBucket).Return("", 0, mockError) + m.bucketDataGetter.EXPECT().BucketSizeAndCount(mockBucket).Return("", 0, mockError) }, wantedError: fmt.Errorf(`get size and count data for "jimmyBuckets" S3 bucket: %w`, mockError), }, From 5288c49e4966571455327dc59c3f98523060a189 Mon Sep 17 00:00:00 2001 From: Janice Huang <60631893+huanjani@users.noreply.github.com> Date: Tue, 20 Jun 2023 13:11:21 -0700 Subject: [PATCH 05/10] Update internal/pkg/describe/status_describe.go Co-authored-by: Danny Randall <10566468+dannyrandall@users.noreply.github.com> --- internal/pkg/describe/status_describe.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/describe/status_describe.go b/internal/pkg/describe/status_describe.go index 3238bbc84c3..ac81400ff0c 100644 --- a/internal/pkg/describe/status_describe.go +++ b/internal/pkg/describe/status_describe.go @@ -246,7 +246,7 @@ func (s *ecsStatusDescriber) Describe() (HumanJSONStringer, error) { func (a *appRunnerStatusDescriber) Describe() (HumanJSONStringer, error) { svc, err := a.svcDescriber.Service() if err != nil { - return nil, fmt.Errorf("get AppRunner service description for App Runner service %s in environment %s: %w", a.svc, a.env, err) + return nil, fmt.Errorf("get App Runner service description for App Runner service %s in environment %s: %w", a.svc, a.env, err) } logGroupName := fmt.Sprintf(fmtAppRunnerSvcLogGroupName, svc.Name, svc.ID) logEventsOpts := cloudwatchlogs.LogEventsOpts{ From f63de23d68f2edb7da9a33c1141bb787fa0b1ee4 Mon Sep 17 00:00:00 2001 From: Huang Date: Tue, 20 Jun 2023 13:16:26 -0700 Subject: [PATCH 06/10] chore: addr dnrnd fb --- internal/pkg/aws/s3/s3.go | 11 ++++------- internal/pkg/describe/status_describe.go | 6 +++--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index a5f7a4c5814..4e2e9bba7ef 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -198,10 +198,7 @@ func FormatARN(partition, location string) string { // BucketTree creates an ASCII tree representing the folder structure of a bucket's objects. func (s *S3) BucketTree(bucket string) (string, error) { outputs, err := s.listObjects(bucket, "/") - if err != nil { - return "", err - } - if outputs == nil { + if err != nil || outputs == nil { return "", nil } var contents []*s3.Object @@ -232,14 +229,14 @@ func (s *S3) BucketSizeAndCount(bucket string) (string, int, error) { return "", 0, nil } var size int64 - var number int + var count int for _, output := range outputs { for _, object := range output.Contents { size = size + aws.Int64Value(object.Size) - number = number + 1 + count = count + 1 } } - return humanize.Bytes(uint64(size)), number, nil + return humanize.Bytes(uint64(size)), count, nil } func (s *S3) listObjects(bucket, delimiter string) ([]s3.ListObjectsV2Output, error) { diff --git a/internal/pkg/describe/status_describe.go b/internal/pkg/describe/status_describe.go index ac81400ff0c..3f24b1cdb55 100644 --- a/internal/pkg/describe/status_describe.go +++ b/internal/pkg/describe/status_describe.go @@ -265,15 +265,15 @@ func (a *appRunnerStatusDescriber) Describe() (HumanJSONStringer, error) { // Describe returns the status of a Static Site service. func (d *staticSiteStatusDescriber) Describe() (HumanJSONStringer, error) { - bucketDataGetter, bucketNameGetter, err := d.initS3Client(d.env) + dataGetter, nameGetter, err := d.initS3Client(d.env) if err != nil { return nil, err } - bucketName, err := bucketNameGetter.BucketName(d.app, d.env, d.svc) + bucketName, err := nameGetter.BucketName(d.app, d.env, d.svc) if err != nil { return nil, fmt.Errorf("get bucket name for %q Static Site service in %q environment: %w", d.svc, d.env, err) } - size, count, err := bucketDataGetter.BucketSizeAndCount(bucketName) + size, count, err := dataGetter.BucketSizeAndCount(bucketName) if err != nil { return nil, fmt.Errorf("get size and count data for %q S3 bucket: %w", bucketName, err) } From acb496b524fd3ee84fe309ae4126507c67e422f3 Mon Sep 17 00:00:00 2001 From: Huang Date: Tue, 20 Jun 2023 14:00:53 -0700 Subject: [PATCH 07/10] chore: return err --- internal/pkg/aws/s3/s3.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index 4e2e9bba7ef..5eeca3246a5 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -199,7 +199,7 @@ func FormatARN(partition, location string) string { func (s *S3) BucketTree(bucket string) (string, error) { outputs, err := s.listObjects(bucket, "/") if err != nil || outputs == nil { - return "", nil + return "", err } var contents []*s3.Object var prefixes []*s3.CommonPrefix From 809ba597fa18540c70605eca2aed02601bafb5fd Mon Sep 17 00:00:00 2001 From: Huang Date: Tue, 20 Jun 2023 14:12:50 -0700 Subject: [PATCH 08/10] chore: addr dnrnd fb --- internal/pkg/aws/s3/s3.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index 5eeca3246a5..d001b237bc2 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -7,7 +7,6 @@ package s3 import ( "errors" "fmt" - "github.com/dustin/go-humanize" "github.com/xlab/treeprint" "io" "mime" @@ -222,18 +221,15 @@ func (s *S3) BucketTree(bucket string) (string, error) { // BucketSizeAndCount returns the total size and number of objects in an S3 bucket. func (s *S3) BucketSizeAndCount(bucket string) (string, int, error) { outputs, err := s.listObjects(bucket, "") - if err != nil { + if err != nil || outputs == nil { return "", 0, err } - if outputs == nil { - return "", 0, nil - } var size int64 var count int for _, output := range outputs { for _, object := range output.Contents { - size = size + aws.Int64Value(object.Size) - count = count + 1 + size += aws.Int64Value(object.Size) + count++ } } return humanize.Bytes(uint64(size)), count, nil @@ -241,12 +237,9 @@ func (s *S3) BucketSizeAndCount(bucket string) (string, int, error) { func (s *S3) listObjects(bucket, delimiter string) ([]s3.ListObjectsV2Output, error) { exists, err := s.bucketExists(bucket) - if err != nil { + if err != nil || !exists { return nil, err } - if !exists { - return nil, nil - } var outputs []s3.ListObjectsV2Output listResp := &s3.ListObjectsV2Output{} for { @@ -273,7 +266,8 @@ func (s *S3) bucketExists(bucket string) (bool, error) { } _, err := s.s3Client.HeadBucket(input) if err != nil { - if aerr, ok := err.(awserr.Error); ok && aerr.Code() == errCodeNotFound { + var aerr *awserr.Error + if errors.As(err, &aerr) && aerr.Code() == errCodeNotFound { return false, nil } return false, err From 75af207fbb48fd2c164d6121bd3ccc4f31f8f012 Mon Sep 17 00:00:00 2001 From: Huang Date: Tue, 20 Jun 2023 14:20:48 -0700 Subject: [PATCH 09/10] chore: addr ph fb --- internal/pkg/aws/s3/s3.go | 5 +++-- internal/pkg/cli/svc_status.go | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/pkg/aws/s3/s3.go b/internal/pkg/aws/s3/s3.go index d001b237bc2..5db761192cc 100644 --- a/internal/pkg/aws/s3/s3.go +++ b/internal/pkg/aws/s3/s3.go @@ -7,7 +7,6 @@ package s3 import ( "errors" "fmt" - "github.com/xlab/treeprint" "io" "mime" "path/filepath" @@ -20,6 +19,8 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/s3" "github.com/aws/aws-sdk-go/service/s3/s3manager" + "github.com/dustin/go-humanize" + "github.com/xlab/treeprint" ) const ( @@ -266,7 +267,7 @@ func (s *S3) bucketExists(bucket string) (bool, error) { } _, err := s.s3Client.HeadBucket(input) if err != nil { - var aerr *awserr.Error + var aerr awserr.Error if errors.As(err, &aerr) && aerr.Code() == errCodeNotFound { return false, nil } diff --git a/internal/pkg/cli/svc_status.go b/internal/pkg/cli/svc_status.go index 6e20d542b03..81a53a62358 100644 --- a/internal/pkg/cli/svc_status.go +++ b/internal/pkg/cli/svc_status.go @@ -66,7 +66,8 @@ func newSvcStatusOpts(vars svcStatusVars) (*svcStatusOpts, error) { if err != nil { return fmt.Errorf("retrieve %s from application %s: %w", o.appName, o.svcName, err) } - if wkld.Type == manifestinfo.RequestDrivenWebServiceType { + switch wkld.Type { + case manifestinfo.RequestDrivenWebServiceType: d, err := describe.NewAppRunnerStatusDescriber(&describe.NewServiceStatusConfig{ App: o.appName, Env: o.envName, @@ -77,7 +78,7 @@ func newSvcStatusOpts(vars svcStatusVars) (*svcStatusOpts, error) { return fmt.Errorf("create status describer for App Runner service %s in application %s: %w", o.svcName, o.appName, err) } o.statusDescriber = d - } else if wkld.Type == manifestinfo.StaticSiteType { + case manifestinfo.StaticSiteType: d, err := describe.NewStaticSiteStatusDescriber(&describe.NewServiceStatusConfig{ App: o.appName, Env: o.envName, @@ -88,7 +89,7 @@ func newSvcStatusOpts(vars svcStatusVars) (*svcStatusOpts, error) { return fmt.Errorf("create status describer for Static Site service %s in application %s: %w", o.svcName, o.appName, err) } o.statusDescriber = d - } else { + default: d, err := describe.NewECSStatusDescriber(&describe.NewServiceStatusConfig{ App: o.appName, Env: o.envName, From 607feecd7c2e69e498d2237add9bd5c25102eb7b Mon Sep 17 00:00:00 2001 From: Huang Date: Tue, 20 Jun 2023 14:40:39 -0700 Subject: [PATCH 10/10] chore: tweak unit test error wording --- internal/pkg/describe/status_describe_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/describe/status_describe_test.go b/internal/pkg/describe/status_describe_test.go index 7459d90ea9a..50bcbb274e1 100644 --- a/internal/pkg/describe/status_describe_test.go +++ b/internal/pkg/describe/status_describe_test.go @@ -719,7 +719,7 @@ func TestAppRunnerStatusDescriber_Describe(t *testing.T) { ) }, - wantedError: fmt.Errorf("get AppRunner service description for App Runner service frontend in environment test: some error"), + wantedError: fmt.Errorf("get App Runner service description for App Runner service frontend in environment test: some error"), }, "success": { setupMocks: func(m serviceStatusDescriberMocks) {