fix: Default username in RoleSessionName #20188
Conversation
In case user does not have entry in `/etc/passwd` the `os.userInfo()` call will throw `SystemError` exception as documented: https://nodejs.org/docs/latest-v16.x/api/os.html#osuserinfooptions Fixes aws#19401 issue. It can be tested inside Docker for ad-hoc 1234 user ID: ```sh docker run -u 1234 -e CDK_HOME=/tmp npm run cdk diff ``` The `CDK_HOME=/tmp` is a workaround for aws#7937 issue, where CDK complains that it can't write cached info in user homedir, because it does not exists. Once aws#7937 will be fixed then aws#19401 will most likely hit users. However above workaround is a viable option. Hence those two issues are related, but not duplicated.
|
|
|
Linter is complaining about missing change in test file. I can add superficial test for this helper function, but it's not going to be useful. |
|
The code change I've done is just basically handling |
|
@comcalvi thanks for review and approve. Can you disable lint rule for missing test, please? I don’t think the test will provide any value here. It seems the the outstanding thing to do here. |
comcalvi
added
pr-linter/exempt-test
Test case is essentially a copy of the following one: `assuming a role sanitizes the username into the session name`
…-cdk into default_username_for_session
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
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 master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
In case user does not have entry in
/etc/passwdtheos.userInfo()call will throw
SystemErrorexception as documented:https://nodejs.org/docs/latest-v16.x/api/os.html#osuserinfooptions
Fixes #19401 issue.
It can be tested inside Docker for ad-hoc 1234 user ID:
The
CDK_HOME=/tmpis a workaround for #7937 issue, where CDK complainsthat it can't write cached info in user homedir, because it does not
exists.
Once #7937 will be fixed then #19401 will most likely hit users. However
above workaround is a viable option. Hence those two issues are related,
but not duplicated.
All Submissions:
Yes, followed the guide.
Adding new Unconventional Dependencies:
No new dependencies.
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?No, it's a bugfix, not a feature.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license