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

(aws-stepfunctions-tasks): CallAwsService to allow custom iam policyStatement #22006

Closed
2 tasks
sindhurpillai opened this issue Sep 12, 2022 · 3 comments · Fixed by #22070
Closed
2 tasks

(aws-stepfunctions-tasks): CallAwsService to allow custom iam policyStatement #22006

sindhurpillai opened this issue Sep 12, 2022 · 3 comments · Fixed by #22070
Labels
@aws-cdk/aws-stepfunctions-tasks effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1

Comments

@sindhurpillai
Copy link

sindhurpillai commented Sep 12, 2022

Describe the feature

CallAwsService attaches an IAM permission policy with single action to StepFunction Role.

Use Case

I have Stepfunction that needs to call rekognition service to detect labels in S3 object.
This requires the step function IAM role to have 2 permissions:

  1. rekognition:detectLabels
  2. s3:GetObject

Currently CallAwsService only adds rekognition:detectLabels and so the stepfunction execution fails

CDK snippet:

new tasks.CallAwsService(this, 'detectLabels',{
          service: 'rekognition',
          action: 'detectLabels',
          parameters: {
            Image: {
              S3Object: {
                Bucket: {bucketName},
                Name: {imageName}
              }
            }
        },
      iamResources:['*']
      })

Proposed Solution

Possible Solutions:

  1. Allow user to manually pass 'PolicyStatement' in CallAwsService. If present it will be used to create the role for StateFunction.
  2. Depending on service:action, rekognition:detectLabels in this case, add additional service policy permissions.

Other Information

Current work around is to manually grant the additional permissions after defining stepfunction.
Example:
{S3Object}.grantRead({stepFunctionObject})

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

v2

Environment details (OS name and version, etc.)

macOS

@sindhurpillai sindhurpillai added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Sep 12, 2022
@kaizencc
Copy link
Contributor

Looks like CallAwsService exposes a taskPolicies property that is a list of policy statements, but it is protected. The way to get what you need is probably to add a addTaskPolicy() method to CallAwsService that can modify taskPolicies on your behalf.

cc'ing @jogold if you can think of a better way.

@kaizencc kaizencc added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Sep 14, 2022
@kaizencc kaizencc removed their assignment Sep 14, 2022
@jogold
Copy link
Contributor

jogold commented Sep 15, 2022

The way to get what you need is probably to add a addTaskPolicy() method to CallAwsService that can modify taskPolicies on your behalf.

@kaizencc This or just a iamStatements: iam.PolicyStatement[] prop for "additional IAM statements required by the call"?

jogold added a commit to jogold/aws-cdk that referenced this issue Sep 16, 2022
…ice integration

Add a `additionalIamStatements` prop to pass additional IAM statements.
To be used when the call requires more than a single statement to be
executed.

Closes aws#22006
jogold added a commit to jogold/aws-cdk that referenced this issue Sep 16, 2022
…ice integration

Add a `additionalIamStatements` prop to pass additional IAM statements.
To be used when the call requires more than a single statement to be
executed.

Closes aws#22006
@mergify mergify bot closed this as completed in #22070 Sep 21, 2022
mergify bot pushed a commit that referenced this issue Sep 21, 2022
…ice integration (#22070)

Add a `additionalIamStatements` prop to pass additional IAM statements. To be used when the call requires more than a single statement to be executed.

Closes #22006


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

homakk pushed a commit to homakk/aws-cdk that referenced this issue Dec 1, 2022
…ice integration (aws#22070)

Add a `additionalIamStatements` prop to pass additional IAM statements. To be used when the call requires more than a single statement to be executed.

Closes aws#22006


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions-tasks effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants