Adding Github Action for CodeGuru Reviewer #1000
Conversation
|
Somehow the action fails to assume the role in the Pull Request; probably because I am not a member of the Smithy repo. Created an issue with aws-actions/configure-aws-credentials. It works if the person starting the action has permissions on the repo. See here |
| uses: aws-actions/[email protected] | ||
| continue-on-error: false | ||
| with: | ||
| s3_bucket: codeguru-reviewer-github-profiler-demo-048169001733-uw2 |
There was a problem hiding this comment.
Where does this bucket come from, and who owns it?
There was a problem hiding this comment.
It's owned by the CodeGuru team and was created using this Stack:
https://github.com/aws-samples/aws-codeguru-reviewer-cicd-cdk-sample
This reverts commit f4feebe.
|
Ugh ... that's a problem. The permissions are per account, so it makes sense that it doesn't work. You don't want people that do a PR against your repo be allowed to assume a role in your account. Would it be ok if the action just succeeds if it cannot assume the role? |
|
Yes, I think that's fine, but who would this role assumption work for? |
|
On second thought, the only way to do this is to trigger the action only on |
|
Oh wait ... this is a different issue: https://github.com/awslabs/smithy/runs/4458443115?check_suite_focus=true#step:8:38 here the analysis actually timed out |
|
https://github.com/awslabs/smithy/runs/4459885012?check_suite_focus=true is the run that caused me to do the revert |
|
Yeah, this makes sense. This is because JordonPhillips is not allowed to assume the IAM Role. I'll add proper error handling to the action. |
|
Who has permission to assume it? Jordon's a core smithy developer |
|
This works via OICD, so it works by repo. The container creates this JWT token to authenticate which is matched against an allow-list of orgs. So only if the container is run under Fwiw, this is an IAM thing, not a CodeGuru thing. |
|
I changed it so it only does any of the steps if assuming the role succeeded: |
|
Will that require a new PR, or were the changes on your side such that a revert of the revert should be sufficient? |
|
No, it requires code changes, I'll do a new PR |
Description of changes:
Runs CodeGuru Reviewer on push and pull_request events and posts recommendations in the Security tab.
Example run can be seen here: https://github.com/martinschaef/smithy/runs/4414789132?check_suite_focus=true
This uses the new OIDC-way to assume an IAM role. That is, no credentials are required. Only authorized repos can assume the role.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.