Skip to content
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

fix(elasticsearch sink): Wrap provider call with a tokio runtime #1104

Merged
merged 1 commit into from
Oct 28, 2019

Conversation

LucioFranco
Copy link
Contributor

This change fixes the usage of rusoto's credentials
the issue arises when a user wants to fetch credentials
via the network with the InstanceMetadataProvider type.
This type sets a timer that depends on the tokio-timer
being set. Previously, this sink used the wait combinator
that does not wrap the future with a tokio timer. Thus,
when a user tried to run this it would fail out because
the timer was not set. This changes the code to allow
the credentials fetch future to be wrapped in a tokio
timer.

Closes #1093

Signed-off-by: Lucio Franco luciofranco14@gmail.com

This change fixes the usage of `rusoto`'s credentials
the issue arises when a user wants to fetch credentials
via the network with the `InstanceMetadataProvider` type.
This type sets a timer that depends on the `tokio-timer`
being set. Previously, this sink used the `wait` combinator
that _does_ not wrap the future with a tokio timer. Thus,
when a user tried to run this it would fail out because
the timer was not set. This changes the code to allow
the `credentials` fetch future to be wrapped in a tokio
timer.

Closes #1093

Signed-off-by: Lucio Franco <luciofranco14@gmail.com>
@lukesteensen
Copy link
Member

Can you explain why this will close the linked issue? It seems like this will just allow a timeout to happen, not a previously failing call to succeed.

@LucioFranco
Copy link
Contributor Author

@lukesteensen This closes the issue because when InstanceMetadataProvider attempts to fetch the credentials it sets up a timer to time out the http request. This timer is not wrapped in any runtime and thus bails out because there is no tokio timer set in the TLS. I was able to reproduce the error on an ec2 instance with an IAM role and once wrapping the future and using Runtime::block_on I was able to fetch the credentials. What I noticed was that when calling the specific provider manually I was able to get the tokio error out of it but it seems that rusoto hides this and just spits out a generic error when using the ChainProvider.

This type of issue with the missing timer is a very common one so its just the fact that we were using Future::wait which is an issue if you're trying to use any of the tokio items.

Copy link
Member

@lukesteensen lukesteensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to reproduce the error on an ec2 instance with an IAM role and once wrapping the future and using Runtime::block_on I was able to fetch the credentials.

Lead with this next time 😄

DefaultCredentialsProvider::new()
.context(AWSCredentialsProviderFailed)?
.credentials()
.wait()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure we should just swap out wait() for sync() here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. Weird that they're not using RusotoFuture. Carry on.

@LucioFranco LucioFranco merged commit f9a6776 into master Oct 28, 2019
@LucioFranco LucioFranco deleted the lucio/fix-es-awscreds branch October 28, 2019 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ElasticSearch sink fails to get EC2 instance credentials
3 participants