Skip to content

Commit 92ddc70

Browse files
authored
Validate bucket is in same region as cluster (#777)
1 parent 334550b commit 92ddc70

File tree

4 files changed

+29
-5
lines changed

4 files changed

+29
-5
lines changed

pkg/lib/aws/errors.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ const (
3333
ErrInvalidS3Path
3434
ErrAuth
3535
ErrBucketInaccessible
36+
ErrBucketNotFound
3637
ErrInstanceTypeLimitIsZero
3738
ErrNoValidSpotPrices
3839
ErrReadCredentials
@@ -45,6 +46,7 @@ var _errorKinds = []string{
4546
"err_invalid_s3_path",
4647
"err_auth",
4748
"err_bucket_inaccessible",
49+
"err_bucket_not_found",
4850
"err_instance_type_limit_is_zero",
4951
"err_no_valid_spot_prices",
5052
"err_read_credentials",
@@ -157,6 +159,13 @@ func ErrorBucketInaccessible(bucket string) error {
157159
})
158160
}
159161

162+
func ErrorBucketNotFound(bucket string) error {
163+
return errors.WithStack(Error{
164+
Kind: ErrBucketNotFound,
165+
message: fmt.Sprintf("bucket \"%s\" not found", bucket),
166+
})
167+
}
168+
160169
func ErrorInstanceTypeLimitIsZero(instanceType string, region string) error {
161170
return errors.WithStack(Error{
162171
Kind: ErrInstanceTypeLimitIsZero,

pkg/lib/aws/s3.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ func GetBucketRegion(bucket string) (string, error) {
433433
sess := session.Must(session.NewSession()) // credentials are not necessary for this request, and will not be used
434434
region, err := s3manager.GetBucketRegion(aws.BackgroundContext(), sess, bucket, endpoints.UsWest2RegionID)
435435
if err != nil {
436-
return "", ErrorBucketInaccessible(bucket)
436+
return "", ErrorBucketNotFound(bucket)
437437
}
438438
return region, nil
439439
}

pkg/types/clusterconfig/clusterconfig.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/cortexlabs/cortex/pkg/lib/aws"
2727
cr "github.com/cortexlabs/cortex/pkg/lib/configreader"
2828
"github.com/cortexlabs/cortex/pkg/lib/errors"
29+
"github.com/cortexlabs/cortex/pkg/lib/hash"
2930
"github.com/cortexlabs/cortex/pkg/lib/pointer"
3031
"github.com/cortexlabs/cortex/pkg/lib/prompt"
3132
"github.com/cortexlabs/cortex/pkg/lib/sets/strset"
@@ -406,12 +407,16 @@ func (cc *Config) Validate(awsClient *aws.Client) error {
406407
return ErrorMinInstancesGreaterThanMax(*cc.MinInstances, *cc.MaxInstances)
407408
}
408409

410+
bucketRegion, _ := aws.GetBucketRegion(*cc.Bucket)
411+
if bucketRegion != "" && bucketRegion != *cc.Region { // if the bucket didn't exist, we will create it in the correct region, so there is no error
412+
return ErrorS3RegionDiffersFromCluster(*cc.Bucket, bucketRegion, *cc.Region)
413+
}
414+
409415
if _, ok := aws.InstanceMetadatas[*cc.Region][*cc.InstanceType]; !ok {
410416
return errors.Wrap(ErrorInstanceTypeNotSupportedInRegion(*cc.InstanceType, *cc.Region), InstanceTypeKey)
411417
}
412418

413-
err := awsClient.VerifyInstanceQuota(*cc.InstanceType)
414-
if err != nil {
419+
if err := awsClient.VerifyInstanceQuota(*cc.InstanceType); err != nil {
415420
return errors.Wrap(err, InstanceTypeKey)
416421
}
417422

@@ -666,12 +671,13 @@ func RegionPrompt(clusterConfig *Config) error {
666671
func InstallPrompt(clusterConfig *Config, awsClient *aws.Client) error {
667672
defaults := applyPromptDefaults(*clusterConfig)
668673

669-
_, hashedAccountID, err := awsClient.GetCachedAccountID()
674+
accountID, _, err := awsClient.GetCachedAccountID()
670675
if err != nil {
671676
return err
672677
}
678+
bucketID := hash.String(accountID + *clusterConfig.Region)[:10]
673679

674-
defaultBucket := clusterConfig.ClusterName + "-" + hashedAccountID[:10]
680+
defaultBucket := clusterConfig.ClusterName + "-" + bucketID
675681
if len(defaultBucket) > 63 {
676682
defaultBucket = defaultBucket[:63]
677683
}

pkg/types/clusterconfig/errors.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ const (
4545
ErrConfigCannotBeChangedOnUpdate
4646
ErrInvalidAvailabilityZone
4747
ErrDidNotMatchStrictS3Regex
48+
ErrS3RegionDiffersFromCluster
4849
ErrInvalidInstanceType
4950
)
5051

@@ -66,6 +67,7 @@ var _errorKinds = []string{
6667
"err_config_cannot_be_changed_on_update",
6768
"err_invalid_availability_zone",
6869
"err_did_not_match_strict_s3_regex",
70+
"err_s3_region_differs_from_cluster",
6971
"err_invalid_instance_type",
7072
}
7173

@@ -228,6 +230,13 @@ func ErrorDidNotMatchStrictS3Regex() error {
228230
})
229231
}
230232

233+
func ErrorS3RegionDiffersFromCluster(bucketName string, bucketRegion string, clusterRegion string) error {
234+
return errors.WithStack(Error{
235+
Kind: ErrS3RegionDiffersFromCluster,
236+
message: fmt.Sprintf("the %s bucket is in %s, but your cluster is in %s; either change the region of your cluster to %s, use a bucket that is in %s, or remove your bucket configuration to allow cortex to make the bucket for you", bucketName, bucketRegion, clusterRegion, bucketRegion, clusterRegion),
237+
})
238+
}
239+
231240
func ErrorInvalidInstanceType(instanceType string) error {
232241
return errors.WithStack(Error{
233242
Kind: ErrInvalidInstanceType,

0 commit comments

Comments
 (0)