search

logstash-plugins / logstash-input-s3

Apache License 2.0
58 stars 152 forks source link

Fix Proxy Support #52

Closed torrancew closed 9 years ago

torrancew commented 9 years ago

Earlier this week, in IRC, it was determined that proxy support is not functional for this plugin, for a few reasons. This PR seeks to resolve those.

Firstly, as is pointed out in #46, the if @credentials check always returned true, as that variable defaults to []. My solution to that was to rely on aws_options_hash to normalize the configuration, regardless of the Logstash config keys in use.

This fix uncovered another problem - namely that the aws_service_endpoint method was originally private in this plugin. Now that it is actually getting called by the AwsConfig mixin, this needs to be reconciled. After checking the s3 output, I decided to follow that plugin's lead, and mark this method as public.

Finally, it was revealed that aws_service_endpoint did not yield a value appropriate for use with the upstream class, so for good measure, the functionality of this method was copied over from the s3 output.

torrancew commented 9 years ago

If accepted, this could close #30

jordansissel commented 9 years ago

Glanced at the code. Seems OK. Haven't tested it.

ph commented 9 years ago

Code LGTM, test run the new test seem to cover the new cases. I am ok with merging this.

elasticsearch-bot commented 9 years ago

Merged sucessfully into master!

jordansissel commented 9 years ago

Someone on IRC is requesting this be backported to api v1

elasticsearch-bot commented 9 years ago

Merged sucessfully into core-api-v1!