awslabs / collectd-cloudwatch

A collectd plugin for sending data to Amazon CloudWatch
MIT License
200 stars 132 forks source link

Failure parsing aws credential file for line entry "[default]" #7

Closed bradleyjames closed 7 years ago

bradleyjames commented 8 years ago

With the following structure the plugin will fail to read the aws configuration file.

[default]
aws_access_key_id = XXX
aws_secret_access_key = XXXX

The issue is https://github.com/awslabs/collectd-cloudwatch/blob/master/src/cloudwatch/modules/configuration/readerutils.py#L40. It attempts to unpack 2 values from the split. When it doesn't find a "=" and thus doesn't return 2 values it fails with "need more than 1 value to unpack".

I don't have time to make a patch just yet so wanted to get the bug out there. Hopefully I'll get to it tonight.

bradleyjames commented 7 years ago

Thanks @npx for the patch!

After digging around in this more I'm a little confused about the expectations of the credentials file. The library seems to not support the default AWS CLI configuration file and defines it's own. Is there a reason for this? The reasons I say this are:

  1. The issue outlined above.
  2. It looks for 'aws_access_key' rather than 'aws_access_key_id' and 'aws_secret_key' and not 'aws_secret_access_key'.

I applied @npx's patch and also added the keys expected by the library to my credentials file to get moving but like I said it would be great if it just supported the normal AWS credentials file keys and format.

Patching would be easy, just not sure if there were reasons for going this different route.

bradleyjames commented 7 years ago

Not sure how I missed it but this is all documented in the README. It would be great if it used the credentials of the SDK/CLI but it will work with a separate file specific to the plugin.

Resolving. Not an issue.