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

Add GetAWSCredentials function in libbeat common #12727

Merged
merged 15 commits into from
Jul 22, 2019
Merged

Add GetAWSCredentials function in libbeat common #12727

merged 15 commits into from
Jul 22, 2019

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Jun 30, 2019

This PR creates a function in libbeat common to get aws credentials. If access_key_id and secret_access_key are given, then these will be used as aws credentials to make API calls. If they are not given, then this function will load default aws config instead from ~/.aws/credentials for Linux & Mac or %USERPROFILE%\.aws\credentials for Windows. This PR also introduced a new config parameter credential_profile_name. It gives GetAWSCredentials function the ability to load aws config from a specific profile. Please see https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-profiles.html for more details.

This function is from #12701 and I realized both metricbeat and filebeat will need this. That's why I'm moving this method to libbeat common. Also functionbeat will be able to use it.

closes: #12708

How to test it

In order to test GetAWSCredentials, you can use aws Metricbeat module or aws Filebeat module:

  • For aws module, change aws.yml to use credential_profile_name:
- module: aws
  period: 300s
  credential_profile_name: <profile name>
  metricsets:
    - ec2
  regions:
    - us-east-1
  • Make sure valid aws credentials are stored in ~/.aws/credentials under
  • Start metricbeat and aws module should run just fine with reading credentials from the profile in ~/.aws/credentials file.
  • You can test this by putting aws credentials under a specific or under default. If it's under default, then credential_profile_name: default is not needed.

@kaiyan-sheng kaiyan-sheng self-assigned this Jun 30, 2019
@kaiyan-sheng kaiyan-sheng added Team:Integrations Label for the Integrations team [zube]: In Progress labels Jun 30, 2019
@kaiyan-sheng kaiyan-sheng marked this pull request as ready for review July 2, 2019 22:33
@kaiyan-sheng kaiyan-sheng requested a review from a team as a code owner July 2, 2019 22:33
@kaiyan-sheng kaiyan-sheng requested a review from a team as a code owner July 3, 2019 00:00
@kaiyan-sheng
Copy link
Contributor Author

@kvch @andrewvc This PR created a common function to get AWS credentials in x-pack/libbeat. Will this be useful to functionbeat? I see we can probably use this at https://github.com/elastic/beats/pull/12401/files#diff-d30e2477c79a898c9f035f2980beb071R55 in @andrewvc's PR for adding auto discover provider for AWS ELB.

Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

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

The changes I've suggested should get your docs building locally. Just reach out to me if you have more issues. After you have the structure like you want it, we'll need to update the sources section in the conf.yaml so the full doc build can find the new libbeat directory under x-pack. See https://github.com/elastic/docs/blob/master/conf.yaml#L731.

metricbeat/docs/modules/aws.asciidoc Show resolved Hide resolved
metricbeat/docs/modules/aws.asciidoc Outdated Show resolved Hide resolved
x-pack/libbeat/docs/aws-credentials-config.asciidoc Outdated Show resolved Hide resolved
Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

Looking good overall, I would love input from @andrewvc @kvch // @ph to see if we can get a similar behavior across the board

x-pack/libbeat/common/aws/credentials.go Outdated Show resolved Hide resolved
@kaiyan-sheng
Copy link
Contributor Author

jenkins, test this please

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

Code LGTM, good stuff! Left some minor corrections for the docs/copy.

Looking forward to using this in #9043 .

x-pack/libbeat/docs/aws-credentials-config.asciidoc Outdated Show resolved Hide resolved
x-pack/libbeat/docs/aws-credentials-config.asciidoc Outdated Show resolved Hide resolved
x-pack/metricbeat/module/aws/_meta/docs.asciidoc Outdated Show resolved Hide resolved
Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

LGTM

@kaiyan-sheng
Copy link
Contributor Author

CI failure is not related: #13009

@kaiyan-sheng kaiyan-sheng merged commit 4742cf9 into elastic:master Jul 22, 2019
@kaiyan-sheng kaiyan-sheng deleted the default_aws_config branch July 22, 2019 18:07
@kaiyan-sheng
Copy link
Contributor Author

This PR broke docs CI. When merging this PR into 7.x, https://github.com/elastic/docs/blob/master/conf.yaml#L745 needs to be changed again, 7.x needs to be removed from the list of excluded branches.

@andresrc andresrc added the v7.4.0 label Aug 7, 2019
@kaiyan-sheng kaiyan-sheng added the test-plan Add this PR to be manual test plan label Aug 22, 2019
@kaiyan-sheng
Copy link
Contributor Author

Done updating the description with how to test this. Thanks for adding the label @andresrc !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan v7.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS module support sdk credential provider chain
7 participants