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

Added sts_http_proxy parameter to assume_role_credentials configuration stanza #136

Conversation

emerlinsky
Copy link

@emerlinsky emerlinsky commented Dec 7, 2017

This comes handy to support use case when flunetd on ec2 instance can communicate to kinesis api via kinesis endpoint but can talk to sts api only via http proxy.

This contribution is released under Apache 2 license and awslabs can use, modify, copy, and redistribute this contribution.

Copy link
Contributor

@riywo riywo left a comment

Choose a reason for hiding this comment

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

Sorry for delaying my comment. This is a very nice feature and I'd like to merge it.

Before merging, could you address my tiny comment? Also could you confirm that "this contribution is released under Apache 2 license and we can use, modify, copy, and redistribute this contribution"?

@@ -38,6 +38,8 @@ module ClientParams
config_param :duration_seconds, :integer, default: nil
desc "A unique identifier that is used by third parties when assuming roles in their customers' accounts."
config_param :external_id, :string, default: nil, secret: true
desc "A http proxy url for requests to aws sts service"
config_param :sts_http_proxy, :string, default: nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you make this parameter secret like this:

config_param :sts_http_proxy, :string, default: nil, secret: true

Since this parameter can contain username and password in plain text.

Copy link
Author

Choose a reason for hiding this comment

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

@riywo, your request for change makes total sense. I will push the corresponding change.
Also, I will idd license statement to the PR description.

@riywo
Copy link
Contributor

riywo commented Dec 15, 2017

Thank you for your contribution! The last thing I'd like to ask you is please squash all commits to a single commit so that everyone can easily recognize what it is.

Then, I'll merge this and release v2.1.0

…on stanza to support use case when flunetd on ec2 instance can communicate to kinesis api via kinesis endpoint but can talk to sts api only via http proxy

Treat sts_http_proxy as a secure parameter since it may have proxy user name with password in it

Typo fix
@emerlinsky emerlinsky force-pushed the added_sts_http_proxy_option_to_assume_role_credentials_section branch from 3a6c6e3 to e29053c Compare December 15, 2017 02:16
@emerlinsky
Copy link
Author

@riywo, you are welcome, I badly need this functionality for my project :)
Squashed and pushed.

@riywo
Copy link
Contributor

riywo commented Dec 17, 2017

@emerlinsky ,

Could you confirm again to the formal statement below again?:

"Please confirm this contribution is under the terms of the Apache 2.0 license."

This is the last step. Sorry for taking so long time...

@emerlinsky
Copy link
Author

I confirm that this contribution is under the terms of the Apache 2.0 license.

@riywo riywo merged commit 7952b36 into awslabs:master Dec 18, 2017
@riywo
Copy link
Contributor

riywo commented Dec 18, 2017

Thank you for your contribution. I'll release v2.1.0 soon.

@riywo
Copy link
Contributor

riywo commented Dec 19, 2017

@emerlinsky

I've released v2.1.0 at rubygems. Please let me know if you have any issue with this version.
https://rubygems.org/gems/fluent-plugin-kinesis/versions/2.1.0

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.

2 participants