-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add programmatic credential provider for the EC2 client #14
Add programmatic credential provider for the EC2 client #14
Conversation
src/discovery/Akka.Discovery.AwsApi/Ec2/Ec2ConfigurationProvider.cs
Outdated
Show resolved
Hide resolved
src/discovery/Akka.Discovery.AwsApi/Ec2/Ec2ConfigurationProvider.cs
Outdated
Show resolved
Hide resolved
public override AWSCredentials ClientCredentials { get; } = new AnonymousAWSCredentials(); | ||
} | ||
|
||
public sealed class Ec2InstanceMetadataCredentialProvider : Ec2CredentialProvider |
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.
Any reason this shouldn't be the default? Anonymous will almost always fail for these calls.
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.
good point, i'll change that
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.
Done
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.
Might want to rename the class too (i.e. change it to anonymous
something something), just to clear it up, and add an XML-DOC comment indicating that the instance metadata is the default. Then we should be good to go.
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.
Changed the name of the class from "Default" to "Anonymous" to avoid confusion
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.
LGTM
public abstract AWSCredentials ClientCredentials { get; } | ||
} | ||
|
||
public sealed class AnonymousEc2CredentialProvider : Ec2CredentialProvider |
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.
LGTM
{ | ||
if (!EC2InstanceMetadata.IsIMDSEnabled) | ||
{ | ||
_log.Warning("Could not obtain EC2 client credentials because instance metadata is disabled. Using anonymous credentials instead."); |
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.
LGTM
#### Ec2InstanceMetadataCredentialProvider | ||
`Ec2InstanceMetadataCredentialProvider` will try its best to retrieve the correct session | ||
credential provider using the AWS EC2 instance metadata API. It will return an `AnonymousAWSCredential` | ||
if it fails to obtain a credential from the metadata API service. |
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.
LGTM
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.
We should probably add a link back to the relevant AWS documentation here though
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.
Its on the bottom of the readme
No description provided.