feat(stepfunctions): expectedBucketOwner parameter for cross-account s3 access #35881
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue # (if applicable)
N/A, but has been a point of inquiry on the step functions Slack channel multiple times.
Reason for this change
Step functions support cross account bucket access but it only works if we include the
ExpectedBucketOwnerparameter in the S3 API call, we can do that by including theExpectedBucketOwnerattribute in the parameters ofItemReaderDescription of changes
What code changes did you make?
Added the
ExpectedBucketOwneras a optional parameter in theItemReaderconstruct, anItemReaderconstruct is used when a distributed map state wants to perform a file/object read from S3 and distribute the results. Changing theItemReaderhas a cascading effect of also changingS3ObjectsItemReader,S3FileItemReaderand their children constructs as all of them inherit theItemReaderProps.I also changed the render method for both of the object and file reader constructs to add the optional
ExpectedBucketOwnerattribute in the JSON object that it returns.Why do these changes address the issue?
We needed to have the
ExpectedBucketOwnerattribute in the parameters in order of a cross account bucket access request made in a distributed map state to go through.What alternatives did you consider and reject?
The only alternative to this would be to inject ASL directly into CDK rather than using the CDK constructs, but I deemed that to be unsafe due to linting/rendering and other human errors which could be injected because of that.
What design decisions have you made?
The only choice as such was to make the
expectedBucketOwnerattribute as an optional attribute to make sure that theItemReaderstill works without the attribute because it is not strictly required if you are not making a cross account bucket access.Describe any new or updated permissions being added
N/A
Description of how you validated changes
Confirmed that ItemReader can use the ExpectedBucketOwner attribute as documented here
I made changes in unit tests wherein I added the
expectedBucketOwner: '1234567890'attribute to theItemReaderand made sure that it rendered correctly in the the output. I also decided to make sure to leave out the attribute in some of the unit tests to make sure theItemReaderworks correctly without it as it is supposed to be an optional parameter. I ran all the unit tests in the aws-stepfunctions suite and they all passed.For the integration tests I added the
expectedBucketOwnerattribute and set it to the current account and saw that the CDK template rendered correctly and the integration test also completed.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license