Standardize the fetching of creds

[tas50]

Some of the plugins can pull from the env and others require CLI. In the long run we want to do this 100% by config, but for now we should support CLI/ENV for all plugins. Copy paste code here. Pretty simple change.

[multani]

Related issues:

• https://github.com/sensu-plugins/sensu-plugins-aws/issues/2
• https://github.com/sensu-plugins/sensu-plugins-aws/issues/168
• https://github.com/sensu-plugins/sensu-plugins-aws/issues/219
• https://github.com/sensu-plugins/sensu-plugins-aws/issues/254
• https://github.com/sensu-plugins/sensu-plugins-aws/issues/216 (I guess?)

[multani]

@majormoses I'd be happy to help with this issue and propose one (or several) pull request(s).

As far as I understand (cf. https://github.com/sensu-plugins/sensu-plugins-aws#authentication) that would require:

1. to migrate all the plugins to use the AWS SDK (v2 preferably), which is tracked in #240
2. remove all the command line options to which allow to set the credentials
   • there are lot of legacy code such as the aws_config method in check-elb-certs.rb (also present in other checks) to remove as well. AFAIK:
     • this overrides the imported aws_config from the Common class
     • I'm not sure how it works, since the AWS related config options are not defined anywhere
3. I think the Common could nearly be removed unless:
   • we keep the aws_region flag in the checks which are relevant
   • in this case, we let the last bits of this Common class which sets the region.
4. Maybe write some kind of guidelines for future checks on how to write them and handle authentication?

Obviously, that would be a major breaking change, but that would make the configuration more uniform and would be a nice simplification/cleanup of all these checks.

[majormoses]

Thanks for taking a look and volunteering to help, let me try to address inline. @eheydrick correct me if I am missing anything.

to migrate all the plugins to use the AWS SDK (v2 preferably), which is tracked in #240

Yup, any work on this front is greatly appreciated

remove all the command line options to which allow to set the credentials

Yup, I'd do it in the same PR as updating to sdk v2. Example: https://github.com/sensu-plugins/sensu-plugins-aws/pull/248

there are lot of legacy code such as the aws_config method in check-elb-certs.rb (also present in other checks) to remove as well. AFAIK

Yes, I'd remove it in the same PR as updating to sdk v2.

this overrides the imported aws_config from the Common class

Hmm I was under the impression only the checks that have been migrated to v2 had included Common which after checking the example you show there is a namespace collision.

I'm not sure how it works, since the AWS related config options are not defined anywhere

Well it basically updates those options if defined otherwise relies on the base aws config class which automatically searches for credentials from the ENV and the .credentials file. here is the documentation for this: http://docs.aws.amazon.com/sdkforruby/api/ (scroll down to "Configuration" section).

I think the Common could nearly be removed unless: we keep the aws_region flag in the checks which are relevant

It is very relevant and 3/4 of the checks need a region.

Maybe write some kind of guidelines for future checks on how to write them and handle authentication?

If you include Common, are using aws-sdk v2, and do not have anything explicitly sending credentials, that is all that is required to set up authentication for checks.

[zbintliff]

Just a question. Why remove the ability to specify via command line? The code you documented, allows for CLI, Env, and .aws/config. The only thing it doesn't allow (but can with a very simple change) is specifying role by cli.

[eheydrick]

The issue with credentials on the command line is that it's far too easy to do it insecurely. If you don't redact the keys you'll end up exposing them in logs, the API, source control, the dashboard, etc. By removing that option we avoid the exposure potential.