fix(kibana): compare keystore values by last output line#162
Conversation
kibana-keystore show can print a runtime warning (e.g. the legacy OpenSSL provider notice) on stdout before the value. The keystore idempotency checks compared the full stdout against the desired value, so they never matched and the keystore add command ran on every play run. Compare against the last output line instead.
📝 WalkthroughWalkthroughFour ChangesKibana Keystore Idempotency Fix
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
roles/kibana/tasks/kibana-keystore.yml (1)
98-113: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThese keystore updates never trigger the restart handler.
Line 105, Line 150, Line 196, and Line 226 force the task to stay
ok, sonotify: Restart Kibanaon the same tasks is inert even whenkibana-keystore addactually rewrites a secret. That leaves Kibana running with stale in-memory credentials/keys after a real update.Suggested fix
- name: kibana-keystore | Set elasticsearch.password in keystore ansible.builtin.command: > /usr/share/kibana/bin/kibana-keystore add -x -f 'elasticsearch.password' args: stdin: "{{ kibana_password.stdout }}" environment: "{{ _kibana_keystore_env }}" - changed_when: false no_log: true when: - kibana_security | bool - >- _kibana_ks_es_password.stdout is undefined or kibana_password.stdout != (_kibana_ks_es_password.stdout_lines[-1] | default('')) notify: - Restart KibanaApply the same change to the other three
Set ... in keystoretasks.As per path instructions,
roles/*/tasks/**must be reviewed for correct handler notifications.Also applies to: 140-161, 189-204, 219-234
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@roles/kibana/tasks/kibana-keystore.yml` around lines 98 - 113, The keystore update tasks in kibana-keystore.yml are forcing unchanged status, which prevents the existing Restart Kibana handler from running when secrets are actually rewritten. Update the relevant “Set ... in keystore” tasks, including the one using ansible.builtin.command for elasticsearch.password and the other matching keystore add tasks in the same file, so they report a change when kibana-keystore add updates a value instead of staying ok; keep the notify: Restart Kibana hook on those tasks so handler execution is triggered correctly.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@roles/kibana/tasks/kibana-keystore.yml`:
- Around line 98-113: The keystore update tasks in kibana-keystore.yml are
forcing unchanged status, which prevents the existing Restart Kibana handler
from running when secrets are actually rewritten. Update the relevant “Set ...
in keystore” tasks, including the one using ansible.builtin.command for
elasticsearch.password and the other matching keystore add tasks in the same
file, so they report a change when kibana-keystore add updates a value instead
of staying ok; keep the notify: Restart Kibana hook on those tasks so handler
execution is triggered correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d45bc64-c405-4ceb-9711-273db218db1d
📒 Files selected for processing (1)
roles/kibana/tasks/kibana-keystore.yml
1 participant
kibana-keystore showcan print a runtime warning on stdout before the value (for example the legacy OpenSSL provider notice). The keystore idempotency checks forelasticsearch.password,server.ssl.keystore.password,xpack.security.encryptionKeyandxpack.encryptedSavedObjects.encryptionKeycompared the desired value against the fullstdout, so they never matched when a warning was present and thekibana-keystore addcommand ran on every play run.Compare against the last line of the command output instead, which is the stored value.
Summary by CodeRabbit