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.

Sign up for GitHub

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

Jump to bottom

Allow adding extra fields #32

Closed
wants to merge 4 commits into from
Closed

Allow adding extra fields #32

wants to merge 4 commits into from

Conversation

mosheavni
Copy link

@mosheavni mosheavni commented Mar 24, 2021 •
edited
Loading

Closes #31
Allows to add extra fields to the auto discover and the standard configs without having to copy the entire block.
Also some markdown linting.

@mosheavni mosheavni changed the title wip WIP: Allow adding extra fields Mar 24, 2021
@mosheavni mosheavni changed the title WIP: Allow adding extra fields Allow adding extra fields Mar 24, 2021
@Doron-Bargo
Copy link
Contributor

Hi @mosheavni - thanks for your contribution
Please let us know when you finish with your PR so we can review it

@mosheavni
Copy link
Author

sure @Doron-Bargo I'm done now.
Tested with these:

helm template -s templates/configmap.yaml --set configType="custom" .
helm template -s templates/configmap.yaml --set configType="custom" --set-file filebeatConfig.customConfig=/tmp/filebeat.yaml .
helm template -s templates/configmap.yaml --set configType="standard"
helm template -s templates/configmap.yaml --set configType="autodiscover"

All of these generates a valid filebeat.yml in the configmap.

@mosheavni
Copy link
Author

@mirii1994 @Doron-Bargo
I added another value for any custom processor to add to the default configs.

@mirii1994
Copy link
Contributor

Hi @mosheavni thanks for your contribution! I'll review your PR in the next couple of days :)

@mosheavni
Copy link
Author

@mirii1994 @Doron-Bargo anything?

mirii1994
mirii1994 suggested changes Apr 14, 2021
View reviewed changes
Copy link
Contributor

@mirii1994 mirii1994 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @mosheavni great work!! :)
I commented out environment & foo in values.yaml extraFields, so that they would use an examples, bumped the chart's version, and edited some of the readme.
Please undo the linting in the README.md file.

@mosheavni
Copy link
Author

thanks @mirii1994
the README changes are an important part of the PR. Can you still review it? if you need specific changes, I'll gladly add them.
Thanks again :)

@mirii1994
Copy link
Contributor

@mosheavni please undo the linting on the README.md file, so that I can review it :)

@mosheavni
Copy link
Author

@mosheavni please undo the linting on the README.md file, so that I can review it :)

there was no linting made, only necessary relevant changes that reflects the changes of the code.

@mosheavni mosheavni requested a review from mirii1994 May 23, 2021 15:17
@mosheavni mosheavni closed this Aug 12, 2021
@mosheavni mosheavni deleted the add-extra-fields branch August 12, 2021 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mirii1994 mirii1994 Awaiting requested review from mirii1994

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow setting extra fields from values
3 participants
@mosheavni @Doron-Bargo @mirii1994