fix:v3.1.1 #108
fix:v3.1.1 #108
Conversation
# This is the 1st commit message: chore: clean up code for PR # The commit message #2 will be skipped: # test: added role-arn as env var to test interpolation # The commit message #3 will be skipped: # fix: expanding role-arn parameter # The commit message #4 will be skipped: # fix: eval parameter # The commit message #5 will be skipped: # fix: fixed typos
|
Note: this PR does not remove the unused executor, suggested via my closed PR #107. You may have decided it should stay, and that's fine; just fyi. :) |
| fi | ||
|
|
||
| # shellcheck disable=SC2034 | ||
| AWS_STS_COMMAND=$( |
There was a problem hiding this comment.
this looks to me like it'll attempt to assume role twice, both here on line 9 and again on line 21? i'll admit to not being familiar with the pipe status exiting you're doing. curious on how this works :)
There was a problem hiding this comment.
I took the dev published version of this orb via this PR and tested on our side. While it works, I confirm our CloudTrail logs show duplicate assume roles with web identities with this code?
Screen shot shows me testing the pipeline twice, but you can see four (2x2) logs recorded of attempted assume roles with web identities:
This of course is not ideal as it means double audit logs; every user will look like they're logging in twice with OpenID. So this needs to somehow be refactored into a single call I believe?
There was a problem hiding this comment.
(and btw i did note in my issue report that the root here, the failing when there is a failed Assume Role for Web Identity, now correctly fails; so we just want this to not double authenticate)
There was a problem hiding this comment.
@adamdmharvey, thanks for letting me know, I really appreciate it! Having double logs is definitely not ideal. Let me investigate some more and see what we can do.
There was a problem hiding this comment.
@adamdmharvey - I don't know why I had to make is so hard 😂 I've added validation that checks to see if the keys and session token were actually generated. If the aws sts assume-role-with-web-identity command fails, the keys and token will be null. I tested it by forcing it to fail and it gave me the correct error from AWS and the job did fail.
As for the executor, we decided to leave it in case there are others using it.
This patch update addresses the following issues: