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

Re-add AWS credential check from cloudwatch output #3587

Conversation

adamchainz
Copy link
Contributor

@adamchainz adamchainz commented Dec 14, 2017

Re-add the check removed in #3583, but this time using the right API call that doesn't require any additional permissions. Fixes #3474.

I tested this by running telegraf locally with my personal credentials, and also running $ aws sts get-caller-identity on an EC2 instance that had an IAM Role/instance profile with zero permissions.

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.-
  • Has appropriate unit tests.

Re-add the check removed in influxdata#3583, but this time using the right API call that doesn't require any additional permissions.

I tested this by running telegraf locally with my personal credentials, and also running `$ aws sts get-caller-identity` on an EC2 instance that had an IAM Role/instance profile with zero permissions.
@adamchainz
Copy link
Contributor Author

@danielnelson @sbalagopal @arohter please check :)

@danielnelson
Copy link
Contributor

Thank you @adamchainz, I triggered circleci to create packages in case it is helpful for testing:

Let me know if anyone needs a different package type.

@adamchainz
Copy link
Contributor Author

Thanks! I extracted and ran the amd64 binary on my ec2 instance with an instance profile and it started fine:

ubuntu@ip-172-30-0-85:~$ ./usr/bin/telegraf --config test.conf
2017-12-15T05:57:08Z D! Attempting connection to output: cloudwatch
2017-12-15T05:57:09Z D! Successfully connected to output: cloudwatch
2017-12-15T05:57:09Z I! Starting Telegraf v1.6.0~53cb07b9
2017-12-15T05:57:09Z I! Loaded outputs: cloudwatch
2017-12-15T05:57:09Z I! Loaded inputs: inputs.cpu
2017-12-15T05:57:09Z I! Tags enabled: host=ip-172-30-0-85
2017-12-15T05:57:09Z I! Agent Config: Interval:10s, Quiet:false, Hostname:"ip-172-30-0-85", Flush Interval:10s
^C2017-12-15T05:57:13Z I! Hang on, flushing any cached metrics before shutdown
2017-12-15T05:57:13Z D! Output [cloudwatch] buffer fullness: 0 / 10000 metrics.

I then detached the instance profile and ran again, and the connect test failed:

ubuntu@ip-172-30-0-85:~$ ./usr/bin/telegraf --config test.conf
2017-12-15T05:58:37Z D! Attempting connection to output: cloudwatch
2017-12-15T05:58:37Z E! cloudwatch: Cannot use credentials to connect to AWS : NoCredentialProviders: no valid providers in chain. Deprecated.
	For verbose messaging see aws.Config.CredentialsChainVerboseErrors
2017-12-15T05:58:37Z E! Failed to connect to output cloudwatch, retrying in 15s, error was 'NoCredentialProviders: no valid providers in chain. Deprecated.
	For verbose messaging see aws.Config.CredentialsChainVerboseErrors'

@adamchainz
Copy link
Contributor Author

(using simple test config:

[agent]
  debug = true

[[outputs.cloudwatch]]
  region = "eu-west-1"
  namespace = "InfluxData/Telegraf"

[[inputs.cpu]]

)

@sbalagopal
Copy link

sbalagopal commented Dec 18, 2017 via email

@danielnelson danielnelson added this to the 1.6.0 milestone Jan 3, 2018
@danielnelson danielnelson added area/aws AWS plugins including cloudwatch, ecs, kinesis feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin labels Jan 3, 2018
@danielnelson danielnelson merged commit 56be3d3 into influxdata:master Jan 3, 2018
@adamchainz adamchainz deleted the issue_3474_check_credentials_properly branch January 3, 2018 11:25
@adamchainz
Copy link
Contributor Author

👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/aws AWS plugins including cloudwatch, ecs, kinesis feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants