StepFunctions: Multi-accounts compatibility #9119
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TO BE REVERTED BEFORE MERGE
viren-nadkarni
added
the
semver: minor
viren-nadkarni
force-pushed
the
stepfunctions-multi-accounts
branch
from
c6c71f9
to
14eb9a2
Compare
viren-nadkarni
requested review from
dominikschubert and
MEPalma
as code owners
MEPalma
requested changes
Sep 14, 2023
There was a problem hiding this comment.
Thank you for the changes! I would like the separation between building the evaluation tree and evaluating runtime values to remain consistent. We achieve this with Component and EvalComponent classes. Adding these runtime values to the environment is valid, but not in the preprocessor. Also a question about parameter validation of the new clients.
...ctions/asl/component/state/state_execution/state_task/service/state_task_service_dynamodb.py
Outdated
Show resolved
Hide resolved
...ck/services/stepfunctions/asl/component/state/state_execution/state_task/service/resource.py
Outdated
Show resolved
Hide resolved
MEPalma
approved these changes
Oct 2, 2023
There was a problem hiding this comment.
Resources are now evaluation components for ResourceRuntimePart which includes the region and account. This means that the request's account and region are used if none is declared in the resource Arn. All other staticResource properties are still accessible from the declaration objects themselves.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
This PR introduces multi-account support to the legacy StepFunctions provider. It also makes multi-account friendly fixes to the work-in-progress StepFunctions v2 provider.
cc: @MEPalma
Implementation
The legacy provider uses StepFunctions Local which itself does not support namespacing. So we take an approach similar to Kinesis. Each account+region combo will run its own instance of SFLocal. Of course this is not optimal but should fulfill simple usecases.
However, SFLocal does not respect the
--account-idargument and overwrites ARNs like Lambda Functions during invocations. So I guess the way forward is to only support true multi-accounts in the new SF provider and keep this limitation in the legacy one. The multi-region parameter is maintained.Criteria for merging: all stepfunctions tests (legacy as well as the ci/circleci: itest-sfn-v2-provider job) pass when
AWS_ACCESS_KEY_IDis set to a non-default account ID or whenAWS_REGION_NAMEis set to non-us-east-1region.79ec46e must be reverted before merge
Related
Closes #8205