Skip to content

Conversation

afeefghannam89
Copy link
Member

@afeefghannam89 afeefghannam89 commented Feb 13, 2023

afeefghannam89 and others added 3 commits February 13, 2023 09:46
* Enable renew elasticsearch cert
* Enable renew Logstash cert
* Enable renew kibana cert
* Enable renew beats cert
* Let Logstash right data on disk
* Prevent Logstash to stick by elasticsearch dead connection
Copy link
Member

@widhalmt widhalmt left a comment

Choose a reason for hiding this comment

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

I love how you work with tags everywhere and how you use tags for one-shot runs like when renewing certificates.

I'm not 100% sure if renewing client certificates wouldn't make sense as a variable. This way you could just enable it and have them replaced as soon as they close in on the end of their validity.

Most comments are things I'm wondering and I'm not really sure if there needs to be a change. Please don't read them as a request for an actual change. It's more of a "did you have time to test"?

The only real changes I see are that handlers are missing (we talked about that offline) and the per queue setting (with global default) of queue type and size.

@afeefghannam89
Copy link
Member Author

afeefghannam89 commented Feb 13, 2023

I love how you work with tags everywhere and how you use tags for one-shot runs like when renewing certificates.

I'm not 100% sure if renewing client certificates wouldn't make sense as a variable. This way you could just enable it and have them replaced as soon as they close in on the end of their validity.

Most comments are things I'm wondering and I'm not really sure if there needs to be a change. Please don't read them as a request for an actual change. It's more of a "did you have time to test"?

The only real changes I see are that handlers are missing (we talked about that offline) and the per queue setting (with global default) of queue type and size.

Thanks :) I like tags more than variable, because they are more flexible by management of changes, the user should not change any variable value when he would like to run a snippet of Ansible code. The variable will come later when we check the validity of certificate. This variable will be set to the conditions of certificate renew tasks. So the user does not have to manipulate any variable value, this will enforce automatism Principe

@afeefghannam89 afeefghannam89 force-pushed the enhancement/renew_certificates branch from 507c02d to efc1e3b Compare February 14, 2023 21:11
@widhalmt widhalmt added this to the 1.0.0 milestone Feb 16, 2023
Copy link
Member

@widhalmt widhalmt left a comment

Choose a reason for hiding this comment

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

Wow, that was a big one to review. :-)

The only real change I can request is the missing "newline" character. The rest is more "I'm not sure, can you give me information about it". So I guess, it's very close to being finished.

The "boolean" for the queue type was just an idea. I guess, it's not worth editing everything again just for that.

@widhalmt
Copy link
Member

Please merge main into this branch. It should help with fixing some problems.

afeefghannam89 and others added 3 commits February 17, 2023 11:53
* Fix logstash connction problem mit elasticserch when ca renew
Logstash will not restart when the ca renew and elasticsearch log
is full with client conntion certificate problem with logstash
http client did not trust this server's certificate, closing connection Netty4HttpChannel
@widhalmt
Copy link
Member

I fixed some lint so checks can go through. I'm right now testing this on a local system.

@widhalmt
Copy link
Member

Changing the Logstash certificate works for me. But only for the elasticsearch output, not for monitoring Since xpack.monitoring was deprecated with Elastic Stack 7 anyway, I'll add a condition so that it will be removed from 8 setups.

Copy link
Member

@widhalmt widhalmt left a comment

Choose a reason for hiding this comment

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

I just found one typo and 2 missing prefixes for variable names. Everything else looks not just good but excellent to me!

auto-merge was automatically disabled February 20, 2023 16:30

Merge queue setting changed

@afeefghannam89 afeefghannam89 enabled auto-merge (squash) February 20, 2023 17:25
auto-merge was automatically disabled February 20, 2023 17:26

Merge queue setting changed

auto-merge was automatically disabled February 20, 2023 17:32

Merge queue setting changed

@afeefghannam89 afeefghannam89 enabled auto-merge (squash) February 20, 2023 17:32
@afeefghannam89 afeefghannam89 merged commit d02ca42 into main Feb 20, 2023
@afeefghannam89 afeefghannam89 deleted the enhancement/renew_certificates branch February 20, 2023 17:41
ivareri pushed a commit to ivareri/ansible-collection-elasticstack that referenced this pull request Jun 17, 2025
* Rework variables names
* Enable security with full stack automatically

fixes NETWAYS#46
ivareri pushed a commit to ivareri/ansible-collection-elasticstack that referenced this pull request Jun 17, 2025
* Enable renew CA
* Enable renew Elasticsearch cert
* Enable renew Logstash cert
* Enable renew Kibana cert
* Enable renew beats cert
* Check CA and Certs validity date and renew them 
* Let Logstash write data on disk
* Prevent Logstash to stick by Elasticsearch dead connection
* Make sure certificate handling is idempotent and reacts to changes NETWAYS#35
* Make Kibana use its certificate and not share the on from
elasticsearch NETWAYS#56
* Let Kibana start after reboot NETWAYS#57 NETWAYS#69
* Allow to use persisted queues per pipeline NETWAYS#60
* Restart the available beat service when create or renew certificates
NETWAYS#83
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants