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

[Filebeat] Add support for specifying AWS cred file #15656

Merged
merged 2 commits into from
Jan 22, 2020

Conversation

leehinman
Copy link
Contributor

  • add "shared_credential_file" to cloudtrail config

Fixes #15652

@leehinman leehinman added bug Filebeat Filebeat needs_backport PR is waiting to be backported to other branches. Team:SIEM labels Jan 17, 2020
@leehinman leehinman requested a review from a team as a code owner January 17, 2020 19:52
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

@leehinman leehinman changed the title Add support for specifying AWS cred file [Filebeat] Add support for specifying AWS cred file Jan 17, 2020
@leehinman leehinman force-pushed the 15652_cloudtrail_fix branch from 81d7a5b to 9f98663 Compare January 17, 2020 19:54
#var.credential_profile_name: fb-aws

# filename of shared credential file
# Use "" to specify default
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to speicify "" to get default? If var.shared_credential_file is not specified in the aws.yml config, it should goes to default ~/.aws/credentials right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kaiyan-sheng When I was testing if var.shared_credential_file wasn't set at all, I would get an error template map has no entry for key. I tried testing for the existence of the key (if, with & index) but got the same error. If we could conditionally test for the presence of the key that would be ideal. Any ideas?

Copy link
Member

@andrewkroh andrewkroh Jan 17, 2020

Choose a reason for hiding this comment

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

In config template you can check if the variables is defined and handle it however you want. Just make sure you don't have a default value in the module manifest. For example:

shared_credential_file: {{ if .shared_credential_file }}{{ .shared_credential_file }}{{ end }}

or

shared_credential_file: {{ if .shared_credential_file }}{{ .shared_credential_file }}{{ else }}/some/file{{ end }}

and this is my preferred way because you are not duplicating default values in several places.

{{ if .shared_credential_file }}
shared_credential_file: {{ .shared_credential_file }}
{{ end }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewkroh It's likely I'm making a stupid mistake, but I tried the last one and got:

2020-01-20T20:47:59.256-0600	ERROR	instance/beat.go:921	Exiting: Error getting config for fileset aws/cloudtrail: Error interpreting the template of the input: template: text:6:6: executing "text" at <.shared_credential_file>: map has no entry for key "shared_credential_file"
Exiting: Error getting config for fileset aws/cloudtrail: Error interpreting the template of the input: template: text:6:6: executing "text" at <.shared_credential_file>: map has no entry for key "shared_credential_file"

I'm wondering if this is because of

	tpl := template.New("text").Option("missingkey=error")

line 263 in fileset.go . The missingkey=error got added in November.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, changing to missingkey=zero made it so that

{{ if .shared_credential_file }}
shared_credential_file: {{ .shared_credential_file }}
{{ end }}

worked.

Copy link
Member

Choose a reason for hiding this comment

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

The variable isn't declared in the manifest. Try adding it to this file, but without a default. Then when its not set a zero value will be added to the template data. It should behave like this.

https://github.com/elastic/beats/blob/9f986638e3c1d26a09f7ca3d6f52c5e6c713d04a/x-pack/filebeat/module/aws/cloudtrail/manifest.yml#L3-L5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That worked. Thanks. Knew I was missing something.

Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

Could you also add this variable to the rest of the filesets in aws module please?

@leehinman
Copy link
Contributor Author

Could you also add this variable to the rest of the filesets in aws module please?

Sure.

- add "shared_credential_file" to cloudtrail config

Fixes elastic#15652
- also make credential_profile_name optional
@leehinman leehinman force-pushed the 15652_cloudtrail_fix branch from 9f98663 to bee24fd Compare January 21, 2020 16:53
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding this!

@leehinman leehinman merged commit 005f474 into elastic:master Jan 22, 2020
@leehinman leehinman deleted the 15652_cloudtrail_fix branch January 22, 2020 14:53
leehinman added a commit to leehinman/beats that referenced this pull request Jan 22, 2020
* Add optional AWS shared_credential_file to all s3 input modules
* Made AWS credential_profile_name optional for all s3 input modules

Fixes elastic#15652

(cherry picked from commit 005f474)
@leehinman leehinman added v7.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Jan 22, 2020
leehinman added a commit to leehinman/beats that referenced this pull request Jan 22, 2020
* Add optional AWS shared_credential_file to all s3 input modules
* Made AWS credential_profile_name optional for all s3 input modules

Fixes elastic#15652

(cherry picked from commit 005f474)
leehinman added a commit that referenced this pull request Jan 22, 2020
* Add optional AWS shared_credential_file to all s3 input modules
* Made AWS credential_profile_name optional for all s3 input modules

Fixes #15652

(cherry picked from commit 005f474)
leehinman added a commit that referenced this pull request Jan 22, 2020
* Add optional AWS shared_credential_file to all s3 input modules
* Made AWS credential_profile_name optional for all s3 input modules

Fixes #15652

(cherry picked from commit 005f474)
kaiyan-sheng added a commit that referenced this pull request Jan 28, 2020
…cred file (#15909)

* [Filebeat] Add support for specifying AWS cred file (#15656)

* Add optional AWS shared_credential_file to all s3 input modules
* Made AWS credential_profile_name optional for all s3 input modules

Fixes #15652

(cherry picked from commit 005f474)

* update aws.asciidoc

* update variables with default for 7.5 only

Co-authored-by: Lee Hinman <[email protected]>
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…ng AWS cred file (elastic#15909)

* [Filebeat] Add support for specifying AWS cred file (elastic#15656)

* Add optional AWS shared_credential_file to all s3 input modules
* Made AWS credential_profile_name optional for all s3 input modules

Fixes elastic#15652

(cherry picked from commit a6a9c37)

* update aws.asciidoc

* update variables with default for 7.5 only

Co-authored-by: Lee Hinman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Filebeat] Cloudtrail auth issues when run under systemd
4 participants