Skip to content

Commit

Permalink
fix: access graph fails to fetch s3 bucket details
Browse files Browse the repository at this point in the history 
With SDK V1, AWS get bucket details such as policies, acls, status...
didn't require the specification of the target s3 bucket location. With
the recent changes to support newer versions of the AWS SDK, the get
bucket details started to fail with the following error:

```
BucketRegionError: incorrect region, the bucket is not in 'ap-south-1' region at endpoint '', bucket is in 'eu-central-1' region
status code: 301, request id: QS5C24H12ZV3VNM4, host id: mzVDk4010MPTCFdxyE/XwERX9W35MSge85PG+h5Jvwyvi7MhxdXLaysb2PTZCMY9r1ngBi6Gv6g=, failed to fetch bucket "cyz" acls polic
```

This PR uses `HeadBucket` to retrieve the location of the s3 bucket and
then uses the bucket location client to retrieve the s3 bucket details.

Fixes #50757
  • Loading branch information
@tigrato
tigrato committed Jan 6, 2025
1 parent 4ea3566 commit 4b1352b
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 54 deletions.
3 changes: 3 additions & 0 deletions lib/cloud/aws/policy_statements.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,9 @@ func StatementAccessGraphAWSSync() *Statement {
"s3:ListBucket",
"s3:GetBucketLocation",
"s3:GetBucketTagging",
"s3:GetBucketPolicyStatus",
"s3:GetBucketAcl",

// IAM IAM
"iam:ListUsers",
"iam:GetUser",
Expand Down
176 changes: 122 additions & 54 deletions lib/srv/discovery/fetchers/aws-sync/s3.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,23 +76,40 @@ func (a *awsFetcher) fetchS3Buckets(ctx context.Context) ([]*accessgraphv1alpha.
region = a.Regions[0]
}

s3Client, err := a.CloudClients.GetAWSS3Client(
ctx,
region,
a.getAWSOptions()...,
)
if err != nil {
return nil, trace.Wrap(err)
}
rsp, err := s3Client.ListBucketsWithContext(
ctx,
&s3.ListBucketsInput{},
)
if err != nil {
return existing.S3Buckets, trace.Wrap(err)
var buckets []*s3.Bucket
var getBucketRegion func(*string) (string, error)
{
globalS3Client, err := a.CloudClients.GetAWSS3Client(
ctx,
region,
a.getAWSOptions()...,
)
if err != nil {
return nil, trace.Wrap(err)
}
rsp, err := globalS3Client.ListBucketsWithContext(
ctx,
&s3.ListBucketsInput{},
)
if err != nil {
return existing.S3Buckets, trace.Wrap(err)
}
buckets = rsp.Buckets
getBucketRegion = func(bucket *string) (string, error) {
rsp, err := globalS3Client.HeadBucketWithContext(
ctx,
&s3.HeadBucketInput{
Bucket: bucket,
},
)
if err != nil {
return "", trace.Wrap(err, "failed to fetch bucket %q region", aws.ToString(bucket))
}
return aws.ToString(rsp.BucketRegion), nil
}
}

for _, bucket := range rsp.Buckets {
for _, bucket := range buckets {
bucket := bucket
eG.Go(func() error {
var failedReqs failedRequests
Expand All @@ -101,54 +118,24 @@ func (a *awsFetcher) fetchS3Buckets(ctx context.Context) ([]*accessgraphv1alpha.
return b.Name == aws.ToString(bucket.Name) && b.AccountId == a.AccountID
},
)
policy, err := s3Client.GetBucketPolicyWithContext(ctx, &s3.GetBucketPolicyInput{
Bucket: bucket.Name,
})
bucketRegion, err := getBucketRegion(bucket.Name)
if err != nil {
errs = append(errs,
trace.Wrap(err, "failed to fetch bucket %q inline policy", aws.ToString(bucket.Name)),
trace.Wrap(err, "failed to fetch bucket %q region", aws.ToString(bucket.Name)),
)
failedReqs.policyFailed = true
}

policyStatus, err := s3Client.GetBucketPolicyStatusWithContext(ctx, &s3.GetBucketPolicyStatusInput{
Bucket: bucket.Name,
})
if err != nil {
errs = append(errs,
trace.Wrap(err, "failed to fetch bucket %q policy status", aws.ToString(bucket.Name)),
)
failedReqs.failedPolicyStatus = true
}

acls, err := s3Client.GetBucketAclWithContext(ctx, &s3.GetBucketAclInput{
Bucket: bucket.Name,
})
if err != nil {
errs = append(errs,
trace.Wrap(err, "failed to fetch bucket %q acls policies", aws.ToString(bucket.Name)),
)
failedReqs.failedAcls = true
}

tagsOutput, err := s3Client.GetBucketTaggingWithContext(ctx, &s3.GetBucketTaggingInput{
Bucket: bucket.Name,
})
var awsErr awserr.Error
const noSuchTagSet = "NoSuchTagSet" // error code when there are no tags or the bucket does not support them
if errors.As(err, &awsErr) && awsErr.Code() == noSuchTagSet {
// If there are no tags, set the error to nil.
err = nil
}
if err != nil {
errs = append(errs,
trace.Wrap(err, "failed to fetch bucket %q tags", aws.ToString(bucket.Name)),
)
failedReqs.failedTags = true
newBucket := awsS3Bucket(aws.ToString(bucket.Name), nil, nil, nil, nil, a.AccountID)
collect(mergeS3Protos(existingBucket, newBucket, failedReqs), trace.NewAggregate(errs...))
return nil
}

newBucket := awsS3Bucket(aws.ToString(bucket.Name), policy, policyStatus, acls, tagsOutput, a.AccountID)
collect(mergeS3Protos(existingBucket, newBucket, failedReqs), trace.NewAggregate(errs...))
details, failedReqs, errsL := a.getS3BucketDetails(ctx, bucket, bucketRegion)

newBucket := awsS3Bucket(aws.ToString(bucket.Name), details.policy, details.policyStatus, details.acls, details.tags, a.AccountID)
collect(mergeS3Protos(existingBucket, newBucket, failedReqs), trace.NewAggregate(append(errs, errsL...)...))
return nil
})
}
Expand Down Expand Up @@ -212,6 +199,7 @@ type failedRequests struct {
failedPolicyStatus bool
failedAcls bool
failedTags bool
headFailed bool
}

func mergeS3Protos(existing, new *accessgraphv1alpha.AWSS3BucketV1, failedReqs failedRequests) *accessgraphv1alpha.AWSS3BucketV1 {
Expand All @@ -237,3 +225,83 @@ func mergeS3Protos(existing, new *accessgraphv1alpha.AWSS3BucketV1, failedReqs f

return clone
}

type s3Details struct {
policy *s3.GetBucketPolicyOutput
policyStatus *s3.GetBucketPolicyStatusOutput
acls *s3.GetBucketAclOutput
tags *s3.GetBucketTaggingOutput
}

func (a *awsFetcher) getS3BucketDetails(ctx context.Context, bucket *s3.Bucket, bucketRegion string) (s3Details, failedRequests, []error) {
var failedReqs failedRequests
var errs []error
var details s3Details

s3Client, err := a.CloudClients.GetAWSS3Client(
ctx,
bucketRegion,
a.getAWSOptions()...,
)
if err != nil {
errs = append(errs,
trace.Wrap(err, "failed to create s3 client for bucket %q", aws.ToString(bucket.Name)),
)
return s3Details{},
failedRequests{
headFailed: true,
policyFailed: true,
failedPolicyStatus: true,
failedAcls: true,
failedTags: true,
}, errs
}

details.policy, err = s3Client.GetBucketPolicyWithContext(ctx, &s3.GetBucketPolicyInput{
Bucket: bucket.Name,
})
if err != nil {
errs = append(errs,
trace.Wrap(err, "failed to fetch bucket %q inline policy", aws.ToString(bucket.Name)),
)
failedReqs.policyFailed = true
}

details.policyStatus, err = s3Client.GetBucketPolicyStatusWithContext(ctx, &s3.GetBucketPolicyStatusInput{
Bucket: bucket.Name,
})
if err != nil {
errs = append(errs,
trace.Wrap(err, "failed to fetch bucket %q policy status", aws.ToString(bucket.Name)),
)
failedReqs.failedPolicyStatus = true
}

details.acls, err = s3Client.GetBucketAclWithContext(ctx, &s3.GetBucketAclInput{
Bucket: bucket.Name,
})
if err != nil {
errs = append(errs,
trace.Wrap(err, "failed to fetch bucket %q acls policies", aws.ToString(bucket.Name)),
)
failedReqs.failedAcls = true
}

details.tags, err = s3Client.GetBucketTaggingWithContext(ctx, &s3.GetBucketTaggingInput{
Bucket: bucket.Name,
})
var awsErr awserr.Error
const noSuchTagSet = "NoSuchTagSet" // error code when there are no tags or the bucket does not support them
if errors.As(err, &awsErr) && awsErr.Code() == noSuchTagSet {
// If there are no tags, set the error to nil.
err = nil
}
if err != nil {
errs = append(errs,
trace.Wrap(err, "failed to fetch bucket %q tags", aws.ToString(bucket.Name)),
)
failedReqs.failedTags = true
}

return details, failedReqs, errs
}

0 comments on commit 4b1352b

Please sign in to comment.