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

Enable use of uniqe tls passwords pr host #344

Merged
merged 6 commits into from
Oct 25, 2024

Conversation

ivareri
Copy link
Contributor

@ivareri ivareri commented Aug 29, 2024

Allow use of unique elasticsearch_tls_key_passphrase pr host

(not sure if this is a fix or a feature, but I went with fix as that's the behaviour I expected)

@widhalmt
Copy link
Member

Hm... it seems like the hostvars variant of a variable does not fall back to a group var when it's not set. Shouldn't the current code already be able to use per host passwords?

@ivareri
Copy link
Contributor Author

ivareri commented Oct 2, 2024

Hum, my understanding was that hostvars should include everything as the host would see it, including group_vars. I guess I'll do some testing.

The problem I'm trying to fix is caused by the certificate generation loop only running when inventory_hostname == elasticstack_ca. The current code therefore uses the elasticsearch_tls_key_passphrase for the ca host when generating the certs. It does however use the proper elasticsearch_tls_key_passphrase when configuring the nodes. This leaves a broken install on every node but elasticstack_ca, as the password set in the configuration does not match the password used when generating the certificate.

@ivareri
Copy link
Contributor Author

ivareri commented Oct 24, 2024

Apparently there are 3 variable scopes in Ansible, and hostvars only covers one of them by design.

I've fixed the PR so it uses the variable from hostvars if it is set, otherwise it will fallback to the current behavior.

@widhalmt
Copy link
Member

Thanks! Let's see what the tests are saying. :-)

@widhalmt
Copy link
Member

Great. Could please just fix the linter findings?

@ivareri
Copy link
Contributor Author

ivareri commented Oct 24, 2024

Fixed linting.

Note: This PR also changes one of the molecule scenarios in order to fully test the implemented fix

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.

Yes, thank you! This looks like a great addidtion.

@widhalmt widhalmt added this pull request to the merge queue Oct 25, 2024
Merged via the queue into NETWAYS:main with commit 16039fb Oct 25, 2024
8 checks passed
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