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

Replace quotations for passwords with special characters #285

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

Conversation

widhalmt
Copy link
Member

Using special characters like $ in some passwords breaks the role. This PR introduces a test (by setting passwords with special characters) and introduces a fix to handle them correctly.

fixes #282

@widhalmt widhalmt added the bug Something isn't working label Oct 11, 2023
@widhalmt widhalmt self-assigned this Oct 11, 2023
@widhalmt widhalmt marked this pull request as draft October 11, 2023 16:10
@ivareri
Copy link
Contributor

ivareri commented Oct 11, 2023

I haven't tested, but is there a reason not to use ansible.builtin.command instead of shell for this?

Something like this might work

- name: Set bootstrap password
  ansible.builtin.command: /usr/share/elasticsearch/bin/elasticsearch-keystore add -x 'bootstrap.password'
  args:
    stdin: {{ elasticsearch_bootstrap_pw }}
  when: "'bootstrap.password' not in elasticsearch_keystore.stdout_lines"

@widhalmt
Copy link
Member Author

I haven't tested, but is there a reason not to use ansible.builtin.command instead of shell for this?

Something like this might work

- name: Set bootstrap password
  ansible.builtin.command: /usr/share/elasticsearch/bin/elasticsearch-keystore add -x 'bootstrap.password'
  args:
    stdin: {{ elasticsearch_bootstrap_pw }}
  when: "'bootstrap.password' not in elasticsearch_keystore.stdout_lines"

To be honest, I don't see a reason. Maybe the contributer back then had an idea I do not recognize. I guess, in the current implementation, command should be fine and make things even faster.

In the long term, it might be a good idea to replace these tasks with a custom module, though.

@widhalmt
Copy link
Member Author

widhalmt commented Feb 9, 2024

@ivareri I guess it was that the original contributor (like me) didn't think of the possibility to implement it with stdin.

I still think this would fit into another PR so not to change too many things at once. Maybe a module would be even better.

@widhalmt widhalmt marked this pull request as ready for review February 9, 2024 16:57
@widhalmt widhalmt requested a review from a team February 12, 2024 13:09
@widhalmt widhalmt added this to the 0.1.0 milestone Feb 12, 2024
@widhalmt widhalmt added this pull request to the merge queue Jun 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jun 7, 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.

elasticsearch_tls_key_passphrase containing $ not working
3 participants