Skip to content

Commit f259c19

Browse files
committed
Add unit test for s3 bucket create/delete/get region/list
1 parent 6aebd85 commit f259c19

File tree

12 files changed

+415
-47
lines changed

12 files changed

+415
-47
lines changed

.golangci.yml

-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ linters:
2020
- asciicheck
2121
- bodyclose
2222
- dogsled
23-
- dupl
2423
- durationcheck
2524
- errorlint
2625
- exhaustive

app/domain/service/errors.go renamed to app/domain/errors.go

+15-4
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,22 @@
1-
// Package service is an abstraction layer for accessing external services.
2-
package service
1+
package domain
32

43
import "errors"
54

65
var (
6+
// ErrInvalidRegion is an error that occurs when the region is invalid.
7+
ErrInvalidRegion = errors.New("invalid region")
8+
// ErrEmptyRegion is an error that occurs when the region is empty.
9+
ErrEmptyRegion = errors.New("region is empty")
10+
// ErrInvalidBucketName is an error that occurs when the bucket name is invalid.
11+
ErrInvalidBucketName = errors.New("bucket name is invalid")
12+
// ErrNoSuchBucket is an error that occurs when the bucket does not exist.
13+
ErrNoSuchBucket = errors.New("the specified bucket does not exist")
14+
// ErrInvalidDomain is an error that occurs when the domain is invalid.
15+
ErrInvalidDomain = errors.New("invalid domain")
16+
// ErrNotDetectContentType is an error that occurs when the content type cannot be detected.
17+
ErrNotDetectContentType = errors.New("failed to detect content type")
18+
// ErrInvalidEndpoint is an error that occurs when the endpoint is invalid.
19+
ErrInvalidEndpoint = errors.New("invalid endpoint")
720
// ErrBucketAlreadyExistsOwnedByOther is an error that occurs when the bucket already exists and is owned by another account.
821
ErrBucketAlreadyExistsOwnedByOther = errors.New("bucket already exists and is owned by another account")
922
// ErrBucketAlreadyOwnedByYou is an error that occurs when the bucket already exists and is owned by you.
@@ -16,8 +29,6 @@ var (
1629
ErrCDNAlreadyExists = errors.New("CDN already exists")
1730
// ErrOriginAccessIdentifyAlreadyExists is an error that occurs when the origin access identify already exists.
1831
ErrOriginAccessIdentifyAlreadyExists = errors.New("origin access identify already exists")
19-
// ErrNotDetectContentType is an error that occurs when the content type cannot be detected.
20-
ErrNotDetectContentType = errors.New("failed to detect content type")
2132
// ErrFileUpload is an error that occurs when the file upload fails.
2233
ErrFileUpload = errors.New("failed to upload file")
2334
)

app/domain/model/domain.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/url"
77
"strings"
88

9+
"github.com/nao1215/rainbow/app/domain"
910
"github.com/nao1215/rainbow/utils/errfmt"
1011
)
1112

@@ -22,7 +23,7 @@ func (d Domain) String() string {
2223
func (d Domain) Validate() error {
2324
for _, part := range strings.Split(d.String(), ".") {
2425
if !isAlphaNumeric(part) {
25-
return errfmt.Wrap(ErrInvalidDomain, fmt.Sprintf("domain %s is invalid", d))
26+
return errfmt.Wrap(domain.ErrInvalidDomain, fmt.Sprintf("domain %s is invalid", d))
2627
}
2728
}
2829
return nil
@@ -80,16 +81,16 @@ func (e Endpoint) String() string {
8081
// Validate validates Endpoint. If Endpoint is invalid, it returns an error.
8182
func (e Endpoint) Validate() error {
8283
if e == "" {
83-
return errfmt.Wrap(ErrInvalidEndpoint, "endpoint is empty")
84+
return errfmt.Wrap(domain.ErrInvalidEndpoint, "endpoint is empty")
8485
}
8586

8687
parsedURL, err := url.Parse(e.String())
8788
if err != nil {
88-
return errfmt.Wrap(ErrInvalidDomain, err.Error())
89+
return errfmt.Wrap(domain.ErrInvalidDomain, err.Error())
8990
}
9091
host := parsedURL.Host
9192
if host == "" || parsedURL.Scheme == "" {
92-
return errfmt.Wrap(ErrInvalidDomain, host)
93+
return errfmt.Wrap(domain.ErrInvalidDomain, host)
9394
}
9495
return nil
9596
}

app/domain/model/domain_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package model
33
import (
44
"errors"
55
"testing"
6+
7+
"github.com/nao1215/rainbow/app/domain"
68
)
79

810
const (
@@ -53,7 +55,7 @@ func TestDomainValidate(t *testing.T) {
5355
{
5456
name: "failure. protocol is included",
5557
d: exampleComWithProtocol,
56-
wantErr: ErrInvalidDomain,
58+
wantErr: domain.ErrInvalidDomain,
5759
},
5860
{
5961
name: "success. domain is empty",

app/domain/model/errors.go

-18
This file was deleted.

app/domain/model/s3.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/aws/aws-sdk-go-v2/aws"
1414
"github.com/aws/aws-sdk-go-v2/service/s3/types"
15+
"github.com/nao1215/rainbow/app/domain"
1516
"github.com/nao1215/rainbow/utils/errfmt"
1617
"github.com/nao1215/rainbow/utils/xregex"
1718
"github.com/wailsapp/mimetype"
@@ -122,9 +123,9 @@ func (r Region) Validate() error {
122123
RegionSASouth1, RegionUSGovEast1, RegionUSGovWest1:
123124
return nil
124125
case Region(""):
125-
return ErrEmptyRegion
126+
return domain.ErrEmptyRegion
126127
default:
127-
return ErrInvalidRegion
128+
return domain.ErrInvalidRegion
128129
}
129130
}
130131

@@ -235,7 +236,7 @@ func (b Bucket) Split() (Bucket, S3Key) {
235236
// Bucket naming rules: https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html
236237
func (b Bucket) Validate() error {
237238
if b.Empty() {
238-
return errfmt.Wrap(ErrInvalidBucketName, "s3 bucket name is empty")
239+
return errfmt.Wrap(domain.ErrInvalidBucketName, "s3 bucket name is empty")
239240
}
240241

241242
validators := []func() error{
@@ -267,7 +268,7 @@ var s3RegexPattern xregex.Regex //nolint:gochecknoglobals
267268
func (b Bucket) validatePattern() error {
268269
s3RegexPattern.InitOnce(`^[a-z0-9][a-z0-9.-]*[a-z0-9]$`)
269270
if err := s3RegexPattern.MatchString(string(b)); err != nil {
270-
return errfmt.Wrap(ErrInvalidBucketName, "s3 bucket name must use only lowercase letters, numbers, periods, and hyphens")
271+
return errfmt.Wrap(domain.ErrInvalidBucketName, "s3 bucket name must use only lowercase letters, numbers, periods, and hyphens")
271272
}
272273
return nil
273274
}
@@ -276,7 +277,7 @@ func (b Bucket) validatePattern() error {
276277
func (b Bucket) validatePrefix() error {
277278
for _, prefix := range []string{"xn--", "sthree-", "sthree-configurator"} {
278279
if strings.HasPrefix(string(b), prefix) {
279-
return errfmt.Wrap(ErrInvalidBucketName, "s3 bucket name must not start with \"xn--\", \"sthree-\", or \"sthree-configurator\"")
280+
return errfmt.Wrap(domain.ErrInvalidBucketName, "s3 bucket name must not start with \"xn--\", \"sthree-\", or \"sthree-configurator\"")
280281
}
281282
}
282283
return nil
@@ -286,7 +287,7 @@ func (b Bucket) validatePrefix() error {
286287
func (b Bucket) validateSuffix() error {
287288
for _, suffix := range []string{"-s3alias", "--ol-s3"} {
288289
if strings.HasSuffix(string(b), suffix) {
289-
return errfmt.Wrap(ErrInvalidBucketName, "s3 bucket name must not end with \"-s3alias\" or \"--ol-s3\"")
290+
return errfmt.Wrap(domain.ErrInvalidBucketName, "s3 bucket name must not end with \"-s3alias\" or \"--ol-s3\"")
290291
}
291292
}
292293
return nil
@@ -295,7 +296,7 @@ func (b Bucket) validateSuffix() error {
295296
// validateCharSequence validates the character sequence of the bucket name.
296297
func (b Bucket) validateCharSequence() error {
297298
if strings.Contains(string(b), "..") || strings.Contains(string(b), "--") {
298-
return errfmt.Wrap(ErrInvalidBucketName, "s3 bucket name must not contain consecutive periods or hyphens")
299+
return errfmt.Wrap(domain.ErrInvalidBucketName, "s3 bucket name must not contain consecutive periods or hyphens")
299300
}
300301
return nil
301302
}

app/domain/model/s3_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/aws/aws-sdk-go-v2/service/s3/types"
1616
"github.com/google/go-cmp/cmp"
1717
"github.com/google/go-cmp/cmp/cmpopts"
18+
"github.com/nao1215/rainbow/app/domain"
1819
)
1920

2021
func TestRegionString(t *testing.T) {
@@ -61,13 +62,13 @@ func TestRegionValidate(t *testing.T) {
6162
name: "failure. region is empty",
6263
r: Region(""),
6364
wantErr: true,
64-
e: ErrEmptyRegion,
65+
e: domain.ErrEmptyRegion,
6566
},
6667
{
6768
name: "failure. region is invalid",
6869
r: Region("invalid"),
6970
wantErr: true,
70-
e: ErrInvalidRegion,
71+
e: domain.ErrInvalidRegion,
7172
},
7273
}
7374

app/domain/service/s3.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Package service
1+
// Package service implements the service layer. This package is used only for application domain.
22
package service
33

44
import (

app/external/s3.go

+18-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/aws/aws-sdk-go-v2/service/s3"
1313
"github.com/aws/aws-sdk-go-v2/service/s3/types"
1414
"github.com/google/wire"
15+
"github.com/nao1215/rainbow/app/domain"
1516
"github.com/nao1215/rainbow/app/domain/model"
1617
"github.com/nao1215/rainbow/app/domain/service"
1718
)
@@ -59,6 +60,14 @@ func (c *S3BucketCreator) CreateS3Bucket(ctx context.Context, input *service.S3B
5960
CreateBucketConfiguration: locationContstraint,
6061
})
6162
if err != nil {
63+
var alreadyExistsErr *types.BucketAlreadyExists
64+
var alreadyOwnedByYouErr *types.BucketAlreadyOwnedByYou
65+
if errors.As(err, &alreadyExistsErr) {
66+
return nil, fmt.Errorf("%w: region=%s, bucket name=%s", domain.ErrBucketAlreadyExistsOwnedByOther, input.Region.String(), input.Bucket.String())
67+
}
68+
if errors.As(err, &alreadyOwnedByYouErr) {
69+
return nil, fmt.Errorf("%w: region=%s, bucket name=%s", domain.ErrBucketAlreadyOwnedByYou, input.Region.String(), input.Bucket.String())
70+
}
6271
return nil, fmt.Errorf("%w: region=%s, bucket name=%s", err, input.Region.String(), input.Bucket.String())
6372
}
6473
return &service.S3BucketCreatorOutput{}, nil
@@ -91,7 +100,7 @@ func (c *S3BucketLister) ListS3Buckets(ctx context.Context, _ *service.S3BucketL
91100
return nil, err
92101
}
93102

94-
var buckets model.BucketSets
103+
buckets := make(model.BucketSets, 0, len(out.Buckets))
95104
for _, b := range out.Buckets {
96105
buckets = append(buckets, model.BucketSet{
97106
Bucket: model.Bucket(*b.Name),
@@ -127,6 +136,10 @@ func (c *S3BucketLocationGetter) GetS3BucketLocation(ctx context.Context, input
127136
Bucket: aws.String(input.Bucket.String()),
128137
})
129138
if err != nil {
139+
var noSuchBucket *types.NoSuchBucket
140+
if errors.As(err, &noSuchBucket) {
141+
return nil, fmt.Errorf("%w: bucket name=%s", domain.ErrNoSuchBucket, input.Bucket.String())
142+
}
130143
return nil, err
131144
}
132145

@@ -170,6 +183,10 @@ func (c *S3BucketDeleter) DeleteS3Bucket(ctx context.Context, input *service.S3B
170183
o.Region = input.Region.String()
171184
})
172185
if err != nil {
186+
var noSuchBucket *types.NoSuchBucket
187+
if errors.As(err, &noSuchBucket) {
188+
return nil, fmt.Errorf("%w: bucket name=%s", domain.ErrNoSuchBucket, input.Bucket.String())
189+
}
173190
return nil, err
174191
}
175192
return &service.S3BucketDeleterOutput{}, nil

app/external/s3_helper_test.go

+24-3
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@ import (
44
"context"
55
"testing"
66

7+
"github.com/aws/aws-sdk-go-v2/aws"
78
"github.com/aws/aws-sdk-go-v2/service/s3"
9+
"github.com/aws/aws-sdk-go-v2/service/s3/types"
810
"github.com/nao1215/rainbow/app/domain/model"
911
)
1012

11-
func s3client(t *testing.T) *s3.Client {
13+
// S3Client returns a new S3 client.
14+
func S3Client(t *testing.T) *s3.Client {
1215
t.Helper()
1316
config, err := model.NewAWSConfig(context.Background(), model.NewAWSProfile(""), model.RegionAPNortheast1)
1417
if err != nil {
@@ -21,7 +24,24 @@ func s3client(t *testing.T) *s3.Client {
2124
return client
2225
}
2326

24-
func deleteAllS3BucketDelete(t *testing.T, client *s3.Client) {
27+
// CreateS3Buckets creates S3 buckets. Region is fixed to ap-northeast-1.
28+
func CreateS3Buckets(t *testing.T, client *s3.Client, buckets []model.Bucket) {
29+
t.Helper()
30+
31+
for _, bucket := range buckets {
32+
if _, err := client.CreateBucket(context.Background(), &s3.CreateBucketInput{
33+
Bucket: aws.String(bucket.String()),
34+
CreateBucketConfiguration: &types.CreateBucketConfiguration{
35+
LocationConstraint: types.BucketLocationConstraint(model.RegionAPNortheast1.String()),
36+
},
37+
}); err != nil {
38+
t.Fatal(err)
39+
}
40+
}
41+
}
42+
43+
// DeleteAllS3BucketDelete deletes all S3 buckets.
44+
func DeleteAllS3BucketDelete(t *testing.T, client *s3.Client) {
2545
t.Helper()
2646

2747
buckets, err := client.ListBuckets(context.Background(), &s3.ListBucketsInput{})
@@ -36,7 +56,8 @@ func deleteAllS3BucketDelete(t *testing.T, client *s3.Client) {
3656
}
3757
}
3858

39-
func existS3Bucket(t *testing.T, client *s3.Client, bucket model.Bucket) bool {
59+
// ExistS3Bucket returns true if the bucket exists.
60+
func ExistS3Bucket(t *testing.T, client *s3.Client, bucket model.Bucket) bool {
4061
t.Helper()
4162

4263
buckets, err := client.ListBuckets(context.Background(), &s3.ListBucketsInput{})

app/external/s3_policy.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/aws/aws-sdk-go-v2/service/s3"
88
"github.com/aws/aws-sdk-go-v2/service/s3/types"
99
"github.com/google/wire"
10+
"github.com/nao1215/rainbow/app/domain"
1011
"github.com/nao1215/rainbow/app/domain/service"
1112
"github.com/nao1215/rainbow/utils/errfmt"
1213
)
@@ -42,7 +43,7 @@ func (s *S3BucketPublicAccessBlocker) BlockS3BucketPublicAccess(ctx context.Cont
4243
RestrictPublicBuckets: aws.Bool(true),
4344
},
4445
}); err != nil {
45-
return nil, errfmt.Wrap(service.ErrBucketPublicAccessBlock, err.Error())
46+
return nil, errfmt.Wrap(domain.ErrBucketPublicAccessBlock, err.Error())
4647
}
4748
return &service.S3BucketPublicAccessBlockerOutput{}, nil
4849
}
@@ -78,7 +79,7 @@ func (s *S3BucketPolicySetter) SetS3BucketPolicy(ctx context.Context, input *ser
7879
Bucket: aws.String(input.Bucket.String()),
7980
Policy: aws.String(policy),
8081
}); err != nil {
81-
return nil, errfmt.Wrap(service.ErrBucketPolicySet, err.Error())
82+
return nil, errfmt.Wrap(domain.ErrBucketPolicySet, err.Error())
8283
}
8384
return &service.S3BucketPolicySetterOutput{}, nil
8485
}

0 commit comments

Comments
 (0)