-
Notifications
You must be signed in to change notification settings - Fork 737
[do not merge] feat: support account id in imds / new profile configs #3067
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
Conversation
17dd424 to
49c542f
Compare
Madrigal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor c
| restoreEnv := awstesting.StashEnv() | ||
| defer awstesting.PopEnv(restoreEnv) | ||
|
|
||
| os.Setenv("AWS_EC2_METADATA_SERVICE_ENDPOINT", ec2MetadataURL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: use t.Setenv instead
|
|
||
| func (c EnvConfig) getEC2InstanceProfileName() (string, bool, error) { | ||
| if len(c.EC2InstanceProfileName) == 0 { | ||
| return "", false, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was discussed to err on this rather than leaving it empty. My main concern would be that someone would want to set this to something on their shell, like export AWS_EC2_INSTANCE_PROFILE_NAME=$(some_process_that_returns_a_value), and that something returns an empty string, then they would make a request to the base URL rather than to the profile they were trying to use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was addressed but not here -- it actually uses Lookupenv to check for explicit empty in the main load routine.
207f550 to
d60fb09
Compare
DO NOT MERGE: the new
-extendedmetadata endpoint is not yet available.*-extended/credentials endpoint for IMDS, with automatic fallback to the old oneAWS_EC2_INSTANCE_PROFILE_NAMEand friendsdisable_ec2_metadatashared config setting.