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

Use common variable for TLS #274

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Use common variable for TLS #274

wants to merge 4 commits into from

Conversation

widhalmt
Copy link
Member

@widhalmt widhalmt commented Sep 12, 2023

fixes #248

The metricbeat.yml and auditbeat.yml templates have code for both connecting to Logstash or Elasticseach. They used different variables to determine whether to turn on TLS or not. That resulted in a missing default and therefore Beats setup breaking when connecting to Elasticseach. I changed the variable that's queried to determine whether to turn on TLS or not.

This is something between a workaround and a partial solution. While it makes more sense to use a role related variable for both Logstash and Elasticsearch output (other than before) it's still not the goal we want.

So overhauling the whole process of determining whether we need TLS or not is due. I'll start a discussion and we need to adjust all roles to the solution we find. Now there are similarities but it's not exactly the same in every role.

fixes #248

This is something between a workaround and a partial solution. While it
makes more sense to use a role related variable for both Logstash and
Elasticsearch output (other than before) it's still not the goal we
want.

So overhauling the whole process of determining whether we need TLS or
not is due. I'll start a discussion and we need to adjust all roles to
the solution we find. Now there are similarities but it's not exactly
the same in every role.
@widhalmt widhalmt added the bug Something isn't working label Sep 12, 2023
@widhalmt widhalmt added this to the 1.0.0 milestone Sep 12, 2023
@widhalmt widhalmt requested a review from a team September 12, 2023 14:14
@widhalmt widhalmt self-assigned this Sep 12, 2023
@widhalmt widhalmt enabled auto-merge September 12, 2023 14:20
@lcndsmr
Copy link
Member

lcndsmr commented Sep 20, 2023

Is the documentation still valid with these changes?

@widhalmt
Copy link
Member Author

... I hope, I didn't miss anything. But I'll have a second look.

@danopt
Copy link
Member

danopt commented Sep 26, 2023

@widhalmt Can I push documentation for elasticsearch_http_security and elasticsearch_http_protocol to this PR. I can check and add documentation for beats_security, if required, too.

@widhalmt
Copy link
Member Author

@danopt of course! Go ahead.

@lcndsmr
Copy link
Member

lcndsmr commented Oct 13, 2023

Whats going on here? Can i merge main? Are you still working on changes?

@widhalmt
Copy link
Member Author

@lcndsmr sorry for the eternal wait... yes, the documentation already has the new variables. So this fix actually makes code matching the docs better.

@widhalmt
Copy link
Member Author

widhalmt commented Feb 9, 2024

@lcndsmr could you recheck?

@widhalmt widhalmt modified the milestones: 1.0.0, 0.1.0 Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metricbeat can't automatically connect
3 participants