feat(stepfunctions-tasks): add httpinvoke step functions task#28673
feat(stepfunctions-tasks): add httpinvoke step functions task#28673mergify[bot] merged 26 commits intoaws:mainfrom
Conversation
aws-cdk-automation
left a comment
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
Add task policies: events:RetrieveConnectionCredentials, secretsmanager:GetSecretValue, secretsmanager:DescribeSecret, states:InvokeHTTPEndpoint.
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
…t. Test creates an API Gateway endpoint with basic auth Connection, then uses the connection in its task to invoke the endpoint.
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
… template for README compilation.
…o Rosetta fixture gives duplicate import errors on them. Add SecretValue as core import with others in template.
msambol
left a comment
There was a problem hiding this comment.
Great stuff. Mostly nits on formatting.
TheRealAmazonKendra
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This looks great overall but I just have one suggested change (in addition to the feedback already given).
| * | ||
| * @default - ArrayEncodingFormat.INDICES | ||
| */ | ||
| readonly arrayEncodingFormat?: ArrayEncodingFormat; |
There was a problem hiding this comment.
Given that this field is reliant on urlEncodeBody being set, these two fields should probably be one field. Maybe the only field here should still be called urlEncodeBody but have it take an Encoding (not sure about this name) class with a static function that has an optional input of ArrayEncodingFormat. Something along the lines of
urlEncodeBody: Encoding.set(ArrayEncodingFormat.INDICES);
to include encoding format, or
urlEncodeBody: Encoding.set();
to just include encoding with the default format.
This way, if they leave off this prop altogether, it's defaulted to false.
There was a problem hiding this comment.
I'll also add the note here that set probably isn't a great function name here.
There was a problem hiding this comment.
Thanks for pointing this out, I agree it's a bit awkward with the 2 params. I played around a bit, and I think a single prop can work with a couple extra enum options and renaming it to urlEncodingFormat. You can not pass it or specify URLEncodingFormat.NONE to get no encoding, pass it as URLEncodingFormat.DEFAULT to get encoding with the default array encoding (the underlying service defaults to INDICES), or specify the existing values to use those.
…. Rename enum to URLEncodingFormat and add DEFAULT and NONE options. Restructure the buildTaskParameters method for clarity with the new logic, and define an interface for it. Add specific test case for NONE.
Pull request has been modified.
|
@msambol @TheRealAmazonKendra Thanks for reviewing, I think I've addressed all of your comments. Please let me know if there's anything else! |
TheRealAmazonKendra
left a comment
There was a problem hiding this comment.
Thank you for all your work on this!
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
@Mergifyio update |
❌ Mergify doesn't have permission to updateDetailsFor security reasons, Mergify can't update this pull request. Try updating locally. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
This adds an
HttpInvokeStep Functions task construct, which allows calling public APIs as described here.IConnection, setting required permissions to use its credentials.apiRootprop). This allows passing the relative endpoint path at execution time (apiEndpointprop).Closes #28278
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license