Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(stepfunctions-tasks): incorrect S3 permissions for AthenaStartQueryExecution #11203

Merged
merged 8 commits into from
Nov 3, 2020
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import * as s3 from '@aws-cdk/aws-s3';
import * as sfn from '@aws-cdk/aws-stepfunctions';
import * as cdk from '@aws-cdk/core';
import { Construct } from 'constructs';
Expand Down Expand Up @@ -93,22 +94,28 @@ export class AthenaStartQueryExecution extends sfn.TaskStateBase {

policyStatements.push(
new iam.PolicyStatement({
actions: ['s3:AbortMultipartUpload',
's3:CreateBucket',
's3:GetBucketLocation',
's3:GetObject',
actions: ['s3:CreateBucket',
's3:ListBucket',
's3:GetBucketLocation',
's3:GetObject'],
Sumeet-Badyal marked this conversation as resolved.
Show resolved Hide resolved
resources: ['*'], // Need * permissions to create new output location https://docs.aws.amazon.com/athena/latest/ug/security-iam-athena.html
}),
);

policyStatements.push(
new iam.PolicyStatement({
actions: ['s3:AbortMultipartUpload',
's3:ListBucketMultipartUploads',
's3:ListMultipartUploadParts',
's3:PutObject'],
resources: [this.props.resultConfiguration?.outputLocation ?? '*'], // Need S3 location where data is stored https://docs.aws.amazon.com/athena/latest/ug/security-iam-athena.html
's3:PutObject'], //s3://query-results-bucket/folder/
Sumeet-Badyal marked this conversation as resolved.
Show resolved Hide resolved
resources: [this.props.resultConfiguration?.outputLocation?.bucketName ? 'arn:aws:s3:::' + this.props.resultConfiguration?.outputLocation?.bucketName + '/' + this.props.resultConfiguration?.outputLocation?.objectKey + '/*' : '*'], // Need S3 location where data is stored or Athena throws an Unable to verify/create output bucket https://docs.aws.amazon.com/athena/latest/ug/security-iam-athena.html
}),
);

policyStatements.push(
new iam.PolicyStatement({
actions: ['lakeformation:GetDataAccess'],
resources: [this.props.resultConfiguration?.outputLocation ?? '*'], // Workflow role permissions https://docs.aws.amazon.com/lake-formation/latest/dg/permissions-reference.html
resources: ['*'], // State machines scoped to output location fail and * permissions are required as per documentation https://docs.aws.amazon.com/lake-formation/latest/dg/permissions-reference.html
Sumeet-Badyal marked this conversation as resolved.
Show resolved Hide resolved
}),
);

Expand Down Expand Up @@ -167,25 +174,46 @@ export class AthenaStartQueryExecution extends sfn.TaskStateBase {
* @internal
*/
protected _renderTask(): any {
return {
Resource: integrationResourceArn('athena', 'startQueryExecution', this.integrationPattern),
Parameters: sfn.FieldUtils.renderObject({
QueryString: this.props.queryString,
ClientRequestToken: this.props.clientRequestToken,
QueryExecutionContext: {
Catalog: this.props.queryExecutionContext?.catalogName,
Database: this.props.queryExecutionContext?.databaseName,
},
ResultConfiguration: {
EncryptionConfiguration: {
EncryptionOption: this.props.resultConfiguration?.encryptionConfiguration?.encryptionOption,
KmsKey: this.props.resultConfiguration?.encryptionConfiguration?.encryptionKey,
if (this.props.resultConfiguration?.outputLocation) {
return {
Resource: integrationResourceArn('athena', 'startQueryExecution', this.integrationPattern),
Parameters: sfn.FieldUtils.renderObject({
QueryString: this.props.queryString,
ClientRequestToken: this.props.clientRequestToken,
QueryExecutionContext: {
Catalog: this.props.queryExecutionContext?.catalogName,
Database: this.props.queryExecutionContext?.databaseName,
},
OutputLocation: this.props.resultConfiguration?.outputLocation,
},
WorkGroup: this.props.workGroup,
}),
};
ResultConfiguration: {
EncryptionConfiguration: {
EncryptionOption: this.props.resultConfiguration?.encryptionConfiguration?.encryptionOption,
KmsKey: this.props.resultConfiguration?.encryptionConfiguration?.encryptionKey,
},
OutputLocation: 's3://' + this.props.resultConfiguration?.outputLocation?.bucketName + '/' + this.props.resultConfiguration?.outputLocation?.objectKey + '/',
Sumeet-Badyal marked this conversation as resolved.
Show resolved Hide resolved
},
WorkGroup: this.props.workGroup,
}),
};
} else {
return {
Resource: integrationResourceArn('athena', 'startQueryExecution', this.integrationPattern),
Parameters: sfn.FieldUtils.renderObject({
QueryString: this.props.queryString,
ClientRequestToken: this.props.clientRequestToken,
QueryExecutionContext: {
Catalog: this.props.queryExecutionContext?.catalogName,
Database: this.props.queryExecutionContext?.databaseName,
},
ResultConfiguration: {
EncryptionConfiguration: {
EncryptionOption: this.props.resultConfiguration?.encryptionConfiguration?.encryptionOption,
KmsKey: this.props.resultConfiguration?.encryptionConfiguration?.encryptionKey,
},
},
WorkGroup: this.props.workGroup,
}),
};
}
}
}

Expand All @@ -203,7 +231,7 @@ export interface ResultConfiguration {
* @default - Query Result Location set in Athena settings for this workgroup
* @example s3://query-results-bucket/folder/
*/
readonly outputLocation?: string;
readonly outputLocation?: s3.Location;

/**
* Encryption option used if enabled in S3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,17 @@
},
{
"Action": [
"s3:AbortMultipartUpload",
"s3:CreateBucket",
"s3:GetBucketLocation",
"s3:GetObject",
"s3:ListBucket",
"s3:GetBucketLocation",
"s3:GetObject"
],
"Effect": "Allow",
"Resource": "*"
},
{
"Action": [
"s3:AbortMultipartUpload",
"s3:ListBucketMultipartUploads",
"s3:ListMultipartUploadParts",
"s3:PutObject"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,17 @@
},
{
"Action": [
"s3:AbortMultipartUpload",
"s3:CreateBucket",
"s3:GetBucketLocation",
"s3:GetObject",
"s3:ListBucket",
"s3:GetBucketLocation",
"s3:GetObject"
],
"Effect": "Allow",
"Resource": "*"
},
{
"Action": [
"s3:AbortMultipartUpload",
"s3:ListBucketMultipartUploads",
"s3:ListMultipartUploadParts",
"s3:PutObject"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,17 @@
},
{
"Action": [
"s3:AbortMultipartUpload",
"s3:CreateBucket",
"s3:GetBucketLocation",
"s3:GetObject",
"s3:ListBucket",
"s3:GetBucketLocation",
"s3:GetObject"
],
"Effect": "Allow",
"Resource": "*"
},
{
"Action": [
"s3:AbortMultipartUpload",
"s3:ListBucketMultipartUploads",
"s3:ListMultipartUploadParts",
"s3:PutObject"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,17 @@
},
{
"Action": [
"s3:AbortMultipartUpload",
"s3:CreateBucket",
"s3:GetBucketLocation",
"s3:GetObject",
"s3:ListBucket",
"s3:GetBucketLocation",
"s3:GetObject"
],
"Effect": "Allow",
"Resource": "*"
},
{
"Action": [
"s3:AbortMultipartUpload",
"s3:ListBucketMultipartUploads",
"s3:ListMultipartUploadParts",
"s3:PutObject"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ describe('Start Query Execution', () => {
},
resultConfiguration: {
encryptionConfiguration: { encryptionOption: EncryptionOption.S3_MANAGED },
outputLocation: 'https://s3.Region.amazonaws.com/bucket-name/key-name',
outputLocation: {
bucketName: 'query-results-bucket',
objectKey: 'folder',
},
},
workGroup: 'primary',
});
Expand Down Expand Up @@ -47,7 +50,7 @@ describe('Start Query Execution', () => {
},
ResultConfiguration: {
EncryptionConfiguration: { EncryptionOption: EncryptionOption.S3_MANAGED },
OutputLocation: 'https://s3.Region.amazonaws.com/bucket-name/key-name',
OutputLocation: 's3://query-results-bucket/folder/',
},
WorkGroup: 'primary',
},
Expand Down