Skip to content

Commit

Permalink
fix: add AWS partition to S3 bucket region cache key
Browse files Browse the repository at this point in the history
Signed-off-by: Patrick Decat <[email protected]>
  • Loading branch information
pdecat committed Apr 24, 2024
1 parent d4cd029 commit d8bdb5c
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
21 changes: 12 additions & 9 deletions aws/table_aws_s3_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,12 +321,15 @@ func listS3Buckets(ctx context.Context, d *plugin.QueryData, h *plugin.HydrateDa
return nil, nil
}

func doGetBucketRegion(ctx context.Context, d *plugin.QueryData, bucket string) (string, error) {
func doGetBucketRegion(ctx context.Context, d *plugin.QueryData, h *plugin.HydrateData, bucket string) (string, error) {
// Have we already resolved and cached the bucket name?
// FIXME: include the partition name as:
// > Bucket names must be unique across all AWS accounts in all the AWS Regions within a partition
// See https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html#general-purpose-bucket-names
cacheKey := "getBucketRegion" + bucket
c, err := getCommonColumns(ctx, d, h)
if err != nil {
plugin.Logger(ctx).Error("aws_s3_bucket.doGetBucketRegion", "get_common_columns_error", err)
return "", err
}
commonColumnData := c.(*awsCommonColumnData)
cacheKey := "getBucketRegion/" + commonColumnData.Partition + "/" + bucket
if cachedData, ok := d.ConnectionManager.Cache.Get(cacheKey); ok {
return cachedData.(string), nil
}
Expand All @@ -339,19 +342,19 @@ func doGetBucketRegion(ctx context.Context, d *plugin.QueryData, bucket string)
// it may be better to define a default Steampipe limiter once actual AWS limits are discovered.
resp, err := http.Head(fmt.Sprintf("https://s3.amazonaws.com/%s", bucket))
if err != nil {
plugin.Logger(ctx).Error("aws_s3_bucket.getBucketRegion", "http_head_error", err)
plugin.Logger(ctx).Error("aws_s3_bucket.doGetBucketRegion", "http_head_error", err)
return "", err
}

// We only care about the 400 and 404 HTTP status codes which respectively mean the request is invalid or the bucket does not exist at all.
// FIXME: do 429 responses happen and also include the header?
if resp.StatusCode == 400 || resp.StatusCode == 404 {
plugin.Logger(ctx).Error("aws_s3_bucket.getBucketRegion", "http_head_status_code", resp.StatusCode)
plugin.Logger(ctx).Error("aws_s3_bucket.doGetBucketRegion", "http_head_status_code", resp.StatusCode)
}

// In the other situations (i.e. 200, 301 and 403, the x-amz-bucket-region header is always present
bucketRegion := resp.Header.Get("x-amz-bucket-region")
plugin.Logger(ctx).Debug("aws_s3_bucket.getBucketRegion", "bucket", bucket, "region", bucketRegion, "status_code", resp.StatusCode)
plugin.Logger(ctx).Debug("aws_s3_bucket.doGetBucketRegion", "bucket", bucket, "region", bucketRegion, "status_code", resp.StatusCode)

d.ConnectionManager.Cache.Set(cacheKey, bucketRegion)
return bucketRegion, nil
Expand All @@ -360,7 +363,7 @@ func doGetBucketRegion(ctx context.Context, d *plugin.QueryData, bucket string)
func getBucketRegion(ctx context.Context, d *plugin.QueryData, h *plugin.HydrateData) (interface{}, error) {
bucketName := h.Item.(types.Bucket).Name

return doGetBucketRegion(ctx, d, *bucketName)
return doGetBucketRegion(ctx, d, h, *bucketName)
}

func getS3BucketEventNotificationConfigurations(ctx context.Context, d *plugin.QueryData, h *plugin.HydrateData) (interface{}, error) {
Expand Down
4 changes: 2 additions & 2 deletions aws/table_aws_s3_bucket_intelligent_tiering_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func listBucketIntelligentTieringConfigurations(ctx context.Context, d *plugin.Q
return nil, nil
}

bucketRegion, err := doGetBucketRegion(ctx, d, *bucket.Name)
bucketRegion, err := doGetBucketRegion(ctx, d, h, *bucket.Name)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -140,7 +140,7 @@ func getBucketIntelligentTieringConfiguration(ctx context.Context, d *plugin.Que
return nil, nil
}

bucketRegion, err := doGetBucketRegion(ctx, d, bucketName)
bucketRegion, err := doGetBucketRegion(ctx, d, h, bucketName)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion aws/table_aws_s3_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ func tableAwsS3Object(_ context.Context) *plugin.Table {
func getBucketRegionForObjects(ctx context.Context, d *plugin.QueryData, h *plugin.HydrateData) (interface{}, error) {
bucketName := d.EqualsQuals["bucket_name"].GetStringValue()

return doGetBucketRegion(ctx, d, bucketName)
return doGetBucketRegion(ctx, d, h, bucketName)
}

func listS3Objects(ctx context.Context, d *plugin.QueryData, h *plugin.HydrateData) (interface{}, error) {
Expand Down

0 comments on commit d8bdb5c

Please sign in to comment.