From 9725d931e16f3261bac4284d956be33cefb38048 Mon Sep 17 00:00:00 2001 From: Sam Crauwels Date: Mon, 9 Mar 2026 10:38:12 +0100 Subject: [PATCH 1/2] Fix external CA config on first install and remove unnecessary ES restart on cert changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The elasticsearch.yml template checked whether ca.crt existed on disk to decide whether to include certificate_authorities. On first install the CA file hasn't been deployed yet, so the config rendered without it — breaking inter-node transport TLS. Now we check the input variables instead of the file, so the template reflects intent rather than transient state. Closes #78. Elasticsearch auto-reloads SSL certificates from disk, so the Restart Elasticsearch handler notifications on every cert copy task were causing unnecessary restarts. Removed them; the Kibana restart notification is kept since Node.js does not auto-reload. Added config assertions to the custom_certs verify to catch this class of bug: CA presence in both transport and HTTP sections, verification_mode sanity, and cert path existence. --- CLAUDE.md | 15 +++++ .../elasticsearch_custom_certs/verify.yml | 55 ++++++++++++++++--- .../tasks/elasticsearch-security.yml | 13 ----- roles/elasticsearch/tasks/main.yml | 4 +- 4 files changed, 64 insertions(+), 23 deletions(-) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..53ad09d6 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,15 @@ +# Elasticstack Ansible Collection + +## Fix Workflow + +When fixing a bug: + +1. Before implementing, identify which molecule scenario covers this code path. +2. If no existing scenario catches the bug, add a verify assertion to the closest + existing scenario — or extend its converge — rather than creating a new scenario. + A new scenario is a last resort (each one adds ~10 min to CI). +3. Prefer the lightest test that proves the fix: a config assertion in verify.yml + beats a full multi-node deployment. Only add nodes/complexity when the bug + genuinely requires it (e.g. inter-node communication). +4. The test should fail without the fix and pass with it. Confirm this mentally + or by describing the failure mode before implementing. diff --git a/molecule/elasticsearch_custom_certs/verify.yml b/molecule/elasticsearch_custom_certs/verify.yml index 98953518..fa341f52 100644 --- a/molecule/elasticsearch_custom_certs/verify.yml +++ b/molecule/elasticsearch_custom_certs/verify.yml @@ -84,16 +84,53 @@ - ca_cert_stat.stat.exists fail_msg: "CA certificate file not found" - - name: Check elasticsearch.yml has PEM config - ansible.builtin.command: - cmd: grep 'xpack.security.transport.ssl.certificate' /etc/elasticsearch/elasticsearch.yml - register: es_yml_check - changed_when: false + - name: Read elasticsearch.yml + ansible.builtin.slurp: + src: /etc/elasticsearch/elasticsearch.yml + register: es_yml_raw + + - name: Parse elasticsearch.yml content + ansible.builtin.set_fact: + _es_config: "{{ es_yml_raw.content | b64decode }}" - - name: Verify PEM-style config (not keystore) + - name: Verify PEM-style transport config ansible.builtin.assert: that: - - "'transport.crt' in es_yml_check.stdout" + - "'xpack.security.transport.ssl.certificate' in _es_config" + - "'xpack.security.transport.ssl.key' in _es_config" + fail_msg: "Expected PEM-style transport SSL config in elasticsearch.yml" + + - name: Verify CA referenced in both transport and HTTP sections (issue #78) + ansible.builtin.assert: + that: + - "'transport.ssl.certificate_authorities' in _es_config" + - "'http.ssl.certificate_authorities' in _es_config" fail_msg: >- - Expected PEM-style transport config in elasticsearch.yml. - Got: {{ es_yml_check.stdout }} + certificate_authorities missing from elasticsearch.yml — CA was + configured but the template omitted it (first-install rendering bug). + + - name: Verify verification_mode is not 'none' + ansible.builtin.assert: + that: + - "'verification_mode: none' not in _es_config" + fail_msg: "SSL verification_mode is 'none' — certs provide no security" + + - name: Verify cert paths in config match files on disk + ansible.builtin.stat: + path: "/etc/elasticsearch/{{ item }}" + register: _cert_path_check + loop: + - "certs/{{ inventory_hostname }}-transport.crt" + - "certs/{{ inventory_hostname }}-transport.key" + - "certs/{{ inventory_hostname }}-http.crt" + - "certs/{{ inventory_hostname }}-http.key" + - certs/ca.crt + + - name: Assert all referenced cert files exist + ansible.builtin.assert: + that: + - item.stat.exists + fail_msg: "{{ item.item }} referenced in config but not on disk" + loop: "{{ _cert_path_check.results }}" + loop_control: + label: "{{ item.item }}" diff --git a/roles/elasticsearch/tasks/elasticsearch-security.yml b/roles/elasticsearch/tasks/elasticsearch-security.yml index f9143842..222397fb 100644 --- a/roles/elasticsearch/tasks/elasticsearch-security.yml +++ b/roles/elasticsearch/tasks/elasticsearch-security.yml @@ -150,7 +150,6 @@ group: elasticsearch mode: "0640" when: _elasticsearch_transport_content_mode | bool - notify: Restart Elasticsearch - name: Copy transport certificate (from file) ansible.builtin.copy: @@ -161,7 +160,6 @@ mode: "0640" remote_src: "{{ elasticsearch_tls_remote_src }}" when: not (_elasticsearch_transport_content_mode | bool) - notify: Restart Elasticsearch - name: Write transport key (from content) ansible.builtin.copy: @@ -171,7 +169,6 @@ group: elasticsearch mode: "0640" when: _elasticsearch_transport_content_mode | bool - notify: Restart Elasticsearch - name: Copy transport key (from file, PEM only) ansible.builtin.copy: @@ -184,7 +181,6 @@ when: - not (_elasticsearch_transport_content_mode | bool) - _elasticsearch_transport_cert_format == 'pem' - notify: Restart Elasticsearch # -- HTTP cert/key (falls back to transport if not set separately) -- @@ -198,7 +194,6 @@ when: - _elasticsearch_http_content_mode | bool - elasticsearch_http_security | bool - notify: Restart Elasticsearch - name: Copy HTTP certificate (from file) ansible.builtin.copy: @@ -211,7 +206,6 @@ when: - not (_elasticsearch_http_content_mode | bool) - elasticsearch_http_security | bool - notify: Restart Elasticsearch - name: Write HTTP key (from content) ansible.builtin.copy: @@ -223,7 +217,6 @@ when: - _elasticsearch_http_content_mode | bool - elasticsearch_http_security | bool - notify: Restart Elasticsearch - name: Copy HTTP key (from file, PEM only) ansible.builtin.copy: @@ -237,7 +230,6 @@ - not (_elasticsearch_http_content_mode | bool) - elasticsearch_http_security | bool - _elasticsearch_http_cert_format == 'pem' - notify: Restart Elasticsearch # -- CA certificate -- @@ -249,7 +241,6 @@ group: elasticsearch mode: "0640" when: elasticsearch_tls_ca_certificate_content | default('', true) | length > 0 - notify: Restart Elasticsearch - name: Copy CA certificate (from file) ansible.builtin.copy: @@ -262,7 +253,6 @@ when: - elasticsearch_tls_ca_certificate_content | default('', true) | length == 0 - elasticsearch_tls_ca_certificate | length > 0 - notify: Restart Elasticsearch # Extract CA chain from the already-deployed transport cert on the node. # Uses copy+content for idempotency (only writes when content changes). @@ -289,7 +279,6 @@ - elasticsearch_tls_ca_certificate | length == 0 - elasticsearch_tls_ca_certificate_content | default('', true) | length == 0 - _elasticsearch_transport_ca_extracted | bool - notify: Restart Elasticsearch - name: Set effective CA availability fact ansible.builtin.set_fact: @@ -583,7 +572,6 @@ _cert_group: elasticsearch _cert_mode: "0640" _cert_notify: - - Restart Elasticsearch - Restart kibana if available for elasticsearch certificates tags: - certificates @@ -606,7 +594,6 @@ _cert_group: elasticsearch _cert_mode: "0640" _cert_notify: - - Restart Elasticsearch - Restart kibana if available for elasticsearch certificates tags: - certificates diff --git a/roles/elasticsearch/tasks/main.yml b/roles/elasticsearch/tasks/main.yml index 65b5aae1..f0a5a21c 100644 --- a/roles/elasticsearch/tasks/main.yml +++ b/roles/elasticsearch/tasks/main.yml @@ -293,7 +293,9 @@ _elasticsearch_external_has_ca: true when: - elasticsearch_cert_source == 'external' - - _existing_ca_cert.stat.exists | default(false) + - (_existing_ca_cert.stat.exists | default(false)) or + (elasticsearch_tls_ca_certificate | default('') | length > 0) or + (elasticsearch_tls_ca_certificate_content | default('') | length > 0) - name: Warn about conflicting keys in elasticsearch_extra_config ansible.builtin.debug: From c20595098e9ad61429f3e8fec498f64408cf498c Mon Sep 17 00:00:00 2001 From: Sam Crauwels Date: Mon, 9 Mar 2026 10:48:07 +0100 Subject: [PATCH 2/2] Fix yamllint comment error in custom_certs verify --- molecule/elasticsearch_custom_certs/verify.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/molecule/elasticsearch_custom_certs/verify.yml b/molecule/elasticsearch_custom_certs/verify.yml index fa341f52..0ba68736 100644 --- a/molecule/elasticsearch_custom_certs/verify.yml +++ b/molecule/elasticsearch_custom_certs/verify.yml @@ -100,7 +100,7 @@ - "'xpack.security.transport.ssl.key' in _es_config" fail_msg: "Expected PEM-style transport SSL config in elasticsearch.yml" - - name: Verify CA referenced in both transport and HTTP sections (issue #78) + - name: Verify CA referenced in both transport and HTTP sections ansible.builtin.assert: that: - "'transport.ssl.certificate_authorities' in _es_config"