refactor: use molecule instead of kitchen#42
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplace Kitchen/Serverspec test infra with Molecule: add Molecule scenarios, playbooks, files, a GitHub Actions workflow and Python requirements; remove Test Kitchen / Ruby / Serverspec artifacts; and qualify Ansible built-in module names in tasks. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Repo as Repository
participant Py as Python Env
participant Molecule as Molecule/Ansible
participant Docker as Docker Engine
participant ES as Elasticsearch (container)
GH->>Repo: checkout
GH->>Py: setup Python 3.12 & pip cache
GH->>Repo: pip install -r requirements.txt
GH->>Molecule: run "molecule test -s <scenario>"
Molecule->>Docker: create/start container (privileged, cgroupns_mode host)
Molecule->>Molecule: run Ansible provisioner (converge / prepare)
Molecule->>ES: install/configure Elasticsearch role
Molecule->>ES: run verifier playbooks (service + HTTP/HTTPS checks)
Molecule->>GH: return test results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
.github/workflows/molecule.yml (1)
21-23: Use one dependency source for lint and molecule jobs.Line 21–Line 23 install floating packages directly, while Line 46–Line 47 use
requirements.txt. This can create version drift between jobs.♻️ Proposed change
- - name: Install ansible-lint - run: pip install ansible ansible-lint + - name: Install dependencies + run: pip install -r requirements.txtAlso applies to: 46-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/molecule.yml around lines 21 - 23, The workflow installs ansible and ansible-lint directly in the "Install ansible-lint" step which can drift from the packages installed in the later steps that use requirements.txt; update the workflow so both the lint job step (named "Install ansible-lint") and the molecule job step(s) use the same dependency source by installing from the shared requirements file (e.g., pip install -r requirements.txt) instead of pip installing floating packages, ensuring consistent versions across jobs.molecule/security/converge.yml (1)
10-11: Avoid hardcoding bootstrap credentials in repo-tracked test code.Line 11 embeds a static password. Prefer injecting it from CI env to reduce leakage risk and bad security precedent.
♻️ Proposed change
- es_api_basic_auth_password: "changeme" + es_api_basic_auth_password: "{{ lookup('env', 'MOLECULE_ES_BOOTSTRAP_PASSWORD') }}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@molecule/security/converge.yml` around lines 10 - 11, Replace the hardcoded bootstrap credential by reading it from an external/CI-provided variable: remove the literal "changeme" value for es_api_basic_auth_password and instead reference an injected variable (e.g., use an environment/Ansible variable like ES_API_BASIC_AUTH_PASSWORD or a Vault secret) so the file reads something like a variable interpolation for es_api_basic_auth_password while keeping es_api_basic_auth_username as-is; ensure CI sets ES_API_BASIC_AUTH_PASSWORD and adjust molecule/Ansible config to pass that env var into the playbook.molecule/default/molecule.yml (1)
7-7: Pin the Docker image instead of using:latest.This mutable tag makes scenario behavior drift over time and can break CI unexpectedly. The same issue appears across multiple Molecule scenarios (default, security, and custom-config).
♻️ Proposed change
- image: geerlingguy/docker-ubuntu2204-ansible:latest + image: geerlingguy/docker-ubuntu2204-ansible@sha256:<pinned_digest>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@molecule/default/molecule.yml` at line 7, Replace the mutable Docker image tag used in the Molecule scenarios by pinning to an immutable identifier: update the image field (currently "image: geerlingguy/docker-ubuntu2204-ansible:latest") in the default, security, and custom-config scenario molecule.yml files to a specific version tag or image digest (e.g., a released tag or sha256 digest) so the scenario behavior is stable; ensure all occurrences of the image key in the Molecule scenario configs are changed consistently and commit the updated tags.requirements.txt (1)
1-4: Constrain major versions to keep CI deterministic.Lines 1–4 currently allow unbounded major upgrades. For CI tooling, this can introduce breaking changes without any repo diff.
♻️ Proposed change
-molecule>=6.0 -molecule-plugins[docker]>=23.0 -ansible>=9.0 -ansible-lint>=6.0 +molecule>=6.0,<7.0 +molecule-plugins[docker]>=23.0,<24.0 +ansible>=9.0,<10.0 +ansible-lint>=6.0,<7.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@requirements.txt` around lines 1 - 4, The requirements.txt entries for molecule, molecule-plugins[docker], ansible, and ansible-lint allow unbounded major upgrades which can break CI; update these package specs in requirements.txt to constrain major versions by adding upper bounds (for example use <7 for molecule, <24 for molecule-plugins[docker], <10 for ansible, and <7 for ansible-lint) or use compatible release operators (e.g., ~=) so CI remains deterministic while keeping the same major release line for each package.molecule/custom-config/converge.yml (1)
8-8: Consider centralizing the Elasticsearch version literal.Line 8 hardcodes
8.19.10; this version is also asserted in verifier playbooks. Defining it once (scenario var/default) will reduce drift during upgrades.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@molecule/custom-config/converge.yml` at line 8, The es_version hardcoded as "8.19.10" in converge.yml should be centralized: add a single scenario/role variable (e.g., es_version) in the Molecule scenario vars or the role defaults (referenced as es_version) and replace the literal in molecule/custom-config/converge.yml with a reference to that variable; also update the verifier playbooks to assert against the same es_version variable so both converge.yml and verifier checks use the single canonical value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/molecule.yml:
- Line 10: Add an explicit top-level permissions block for the GITHUB_TOKEN
granting least privilege (e.g., "contents: read") so the workflow does not rely
on default token scopes; insert this permissions block above the "jobs:" key in
the workflow and ensure only repository read access is granted to support
checkout operations.
In `@molecule/security/converge.yml`:
- Line 18: The security scenario's converge.yml currently sets
es_validate_certs: false which disables TLS certificate validation; change
es_validate_certs to true in molecule/security/converge.yml (or make it
configurable via an environment variable or test parameter) so the scenario
validates TLS certs during convergence; update any test harness or documentation
referencing es_validate_certs to reflect the stricter default and add a clear
comment why cert validation must be enabled for security tests.
In `@molecule/security/prepare.yml`:
- Around line 11-18: The task "Ensure SSL directories exist" currently creates
/etc/ssl/private with mode "0755" making private keys world-traversable; change
the directory mode for /etc/ssl/private to "0700" while leaving /etc/ssl/certs
at "0755", and ensure any task that creates or deploys the private key (e.g.,
the key file referenced as test.key in the later task around lines 31-37) sets
its file mode to "0600" (owner read/write only). Locate the ansible task named
"Ensure SSL directories exist" and the task that writes the private key file and
update their mode values accordingly to restrict access.
---
Nitpick comments:
In @.github/workflows/molecule.yml:
- Around line 21-23: The workflow installs ansible and ansible-lint directly in
the "Install ansible-lint" step which can drift from the packages installed in
the later steps that use requirements.txt; update the workflow so both the lint
job step (named "Install ansible-lint") and the molecule job step(s) use the
same dependency source by installing from the shared requirements file (e.g.,
pip install -r requirements.txt) instead of pip installing floating packages,
ensuring consistent versions across jobs.
In `@molecule/custom-config/converge.yml`:
- Line 8: The es_version hardcoded as "8.19.10" in converge.yml should be
centralized: add a single scenario/role variable (e.g., es_version) in the
Molecule scenario vars or the role defaults (referenced as es_version) and
replace the literal in molecule/custom-config/converge.yml with a reference to
that variable; also update the verifier playbooks to assert against the same
es_version variable so both converge.yml and verifier checks use the single
canonical value.
In `@molecule/default/molecule.yml`:
- Line 7: Replace the mutable Docker image tag used in the Molecule scenarios by
pinning to an immutable identifier: update the image field (currently "image:
geerlingguy/docker-ubuntu2204-ansible:latest") in the default, security, and
custom-config scenario molecule.yml files to a specific version tag or image
digest (e.g., a released tag or sha256 digest) so the scenario behavior is
stable; ensure all occurrences of the image key in the Molecule scenario configs
are changed consistently and commit the updated tags.
In `@molecule/security/converge.yml`:
- Around line 10-11: Replace the hardcoded bootstrap credential by reading it
from an external/CI-provided variable: remove the literal "changeme" value for
es_api_basic_auth_password and instead reference an injected variable (e.g., use
an environment/Ansible variable like ES_API_BASIC_AUTH_PASSWORD or a Vault
secret) so the file reads something like a variable interpolation for
es_api_basic_auth_password while keeping es_api_basic_auth_username as-is;
ensure CI sets ES_API_BASIC_AUTH_PASSWORD and adjust molecule/Ansible config to
pass that env var into the playbook.
In `@requirements.txt`:
- Around line 1-4: The requirements.txt entries for molecule,
molecule-plugins[docker], ansible, and ansible-lint allow unbounded major
upgrades which can break CI; update these package specs in requirements.txt to
constrain major versions by adding upper bounds (for example use <7 for
molecule, <24 for molecule-plugins[docker], <10 for ansible, and <7 for
ansible-lint) or use compatible release operators (e.g., ~=) so CI remains
deterministic while keeping the same major release line for each package.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 170229cb-cda8-47dd-88e6-c55cff746746
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (66)
.github/workflows/molecule.yml.gitignore.kitchen.yml.ruby-versionGemfileMakefilemolecule/custom-config/converge.ymlmolecule/custom-config/files/log4j2.propertiesmolecule/custom-config/molecule.ymlmolecule/custom-config/verify.ymlmolecule/default/converge.ymlmolecule/default/molecule.ymlmolecule/default/verify.ymlmolecule/security/converge.ymlmolecule/security/molecule.ymlmolecule/security/prepare.ymlmolecule/security/verify.ymlrequirements.txttasks/main.ymltest/integration/custom-config.ymltest/integration/custom-config/custom_config.ymltest/integration/custom-config/serverspec/default_spec.rbtest/integration/debug.ymltest/integration/default.ymltest/integration/default/default.ymltest/integration/default/serverspec/default_spec.rbtest/integration/files/certs/keystore-password-ca.p12test/integration/files/certs/keystore-password.p12test/integration/files/certs/shared-store-no-password-ca.p12test/integration/files/certs/shared-store-no-password.p12test/integration/files/certs/truststore-password-ca.p12test/integration/files/certs/truststore-password.p12test/integration/files/custom_config/elasticsearchtest/integration/files/custom_config/jvm.optionstest/integration/files/custom_config/log4j2.propertiestest/integration/files/templates-6.x/basic.jsontest/integration/files/templates-7.x/basic.jsontest/integration/helpers/serverspec/Gemfiletest/integration/helpers/serverspec/custom_config_spec.rbtest/integration/helpers/serverspec/license_spec.rbtest/integration/helpers/serverspec/security_spec.rbtest/integration/helpers/serverspec/shared_spec.rbtest/integration/helpers/serverspec/spec_helper.rbtest/integration/license.ymltest/integration/license/license.ymltest/integration/license/serverspec/default_spec.rbtest/integration/oss-to-default-upgrade.ymltest/integration/oss-to-default-upgrade/oss-to-default-upgrade.ymltest/integration/oss-to-default-upgrade/serverspec/default_spec.rbtest/integration/oss-upgrade.ymltest/integration/oss-upgrade/oss.ymltest/integration/oss-upgrade/serverspec/default_spec.rbtest/integration/oss.ymltest/integration/oss/oss.ymltest/integration/oss/serverspec/default_spec.rbtest/integration/security.ymltest/integration/security/security.ymltest/integration/security/serverspec/default_spec.rbtest/integration/trial.ymltest/integration/trial/serverspec/default_spec.rbtest/integration/trial/trial.ymltest/integration/upgrade.ymltest/integration/upgrade/serverspec/default_spec.rbtest/integration/upgrade/upgrade.ymltest/matrix-6x.ymltest/matrix.yml
💤 Files with no reviewable changes (46)
- .gitignore
- .ruby-version
- test/integration/oss/oss.yml
- test/integration/oss-upgrade/oss.yml
- test/integration/custom-config/custom_config.yml
- test/integration/helpers/serverspec/Gemfile
- test/integration/license/license.yml
- test/integration/files/templates-6.x/basic.json
- test/integration/security/security.yml
- test/integration/debug.yml
- test/integration/trial/trial.yml
- test/integration/files/custom_config/elasticsearch
- test/integration/files/templates-7.x/basic.json
- test/integration/files/custom_config/jvm.options
- test/integration/default/serverspec/default_spec.rb
- test/integration/license/serverspec/default_spec.rb
- test/integration/trial/serverspec/default_spec.rb
- test/integration/oss.yml
- test/integration/files/custom_config/log4j2.properties
- test/integration/upgrade.yml
- test/integration/custom-config.yml
- test/matrix.yml
- test/integration/helpers/serverspec/spec_helper.rb
- test/integration/oss-upgrade/serverspec/default_spec.rb
- test/matrix-6x.yml
- test/integration/helpers/serverspec/custom_config_spec.rb
- test/integration/custom-config/serverspec/default_spec.rb
- test/integration/oss-to-default-upgrade.yml
- test/integration/license.yml
- test/integration/oss-upgrade.yml
- test/integration/upgrade/serverspec/default_spec.rb
- test/integration/security.yml
- test/integration/security/serverspec/default_spec.rb
- test/integration/helpers/serverspec/license_spec.rb
- Gemfile
- test/integration/oss/serverspec/default_spec.rb
- test/integration/trial.yml
- test/integration/oss-to-default-upgrade/oss-to-default-upgrade.yml
- test/integration/helpers/serverspec/security_spec.rb
- test/integration/default.yml
- test/integration/helpers/serverspec/shared_spec.rb
- test/integration/upgrade/upgrade.yml
- test/integration/default/default.yml
- Makefile
- .kitchen.yml
- test/integration/oss-to-default-upgrade/serverspec/default_spec.rb
e989eaa to
5508a8b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
meta/main.yml (1)
6-9: Remove stale EL6-only task logic to match declared platform scope.Since support metadata has been tightened, the CentOS 6.x-only task in
tasks/elasticsearch-RedHat.yml(Line 9–12 in the provided snippet) is now dead path and should be removed to keep implementation aligned with declared support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@meta/main.yml` around lines 6 - 9, The role metadata now excludes EL6, so remove the CentOS 6.x–only branch from tasks/elasticsearch-RedHat.yml: locate the conditional block that checks for CentOS/RedHat 6 (the EL6-only task logic) and delete that task branch so there are no dead paths; ensure any variables or handlers referenced exclusively by that EL6 branch are also cleaned up or consolidated into the remaining RedHat flow to avoid orphaned references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@meta/main.yml`:
- Around line 6-9: The role metadata now excludes EL6, so remove the CentOS
6.x–only branch from tasks/elasticsearch-RedHat.yml: locate the conditional
block that checks for CentOS/RedHat 6 (the EL6-only task logic) and delete that
task branch so there are no dead paths; ensure any variables or handlers
referenced exclusively by that EL6 branch are also cleaned up or consolidated
into the remaining RedHat flow to avoid orphaned references.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ad030158-0d52-42e8-9560-653aa571a569
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (68)
.github/workflows/molecule.yml.gitignore.kitchen.yml.ruby-versionGemfileMakefileREADME.mdmeta/main.ymlmolecule/custom-config/converge.ymlmolecule/custom-config/files/log4j2.propertiesmolecule/custom-config/molecule.ymlmolecule/custom-config/verify.ymlmolecule/default/converge.ymlmolecule/default/molecule.ymlmolecule/default/verify.ymlmolecule/security/converge.ymlmolecule/security/molecule.ymlmolecule/security/prepare.ymlmolecule/security/verify.ymlrequirements.txttasks/main.ymltest/integration/custom-config.ymltest/integration/custom-config/custom_config.ymltest/integration/custom-config/serverspec/default_spec.rbtest/integration/debug.ymltest/integration/default.ymltest/integration/default/default.ymltest/integration/default/serverspec/default_spec.rbtest/integration/files/certs/keystore-password-ca.p12test/integration/files/certs/keystore-password.p12test/integration/files/certs/shared-store-no-password-ca.p12test/integration/files/certs/shared-store-no-password.p12test/integration/files/certs/truststore-password-ca.p12test/integration/files/certs/truststore-password.p12test/integration/files/custom_config/elasticsearchtest/integration/files/custom_config/jvm.optionstest/integration/files/custom_config/log4j2.propertiestest/integration/files/templates-6.x/basic.jsontest/integration/files/templates-7.x/basic.jsontest/integration/helpers/serverspec/Gemfiletest/integration/helpers/serverspec/custom_config_spec.rbtest/integration/helpers/serverspec/license_spec.rbtest/integration/helpers/serverspec/security_spec.rbtest/integration/helpers/serverspec/shared_spec.rbtest/integration/helpers/serverspec/spec_helper.rbtest/integration/license.ymltest/integration/license/license.ymltest/integration/license/serverspec/default_spec.rbtest/integration/oss-to-default-upgrade.ymltest/integration/oss-to-default-upgrade/oss-to-default-upgrade.ymltest/integration/oss-to-default-upgrade/serverspec/default_spec.rbtest/integration/oss-upgrade.ymltest/integration/oss-upgrade/oss.ymltest/integration/oss-upgrade/serverspec/default_spec.rbtest/integration/oss.ymltest/integration/oss/oss.ymltest/integration/oss/serverspec/default_spec.rbtest/integration/security.ymltest/integration/security/security.ymltest/integration/security/serverspec/default_spec.rbtest/integration/trial.ymltest/integration/trial/serverspec/default_spec.rbtest/integration/trial/trial.ymltest/integration/upgrade.ymltest/integration/upgrade/serverspec/default_spec.rbtest/integration/upgrade/upgrade.ymltest/matrix-6x.ymltest/matrix.yml
💤 Files with no reviewable changes (46)
- test/integration/custom-config/custom_config.yml
- .ruby-version
- .gitignore
- Gemfile
- test/integration/oss-to-default-upgrade/oss-to-default-upgrade.yml
- test/integration/oss-upgrade/oss.yml
- test/integration/oss/oss.yml
- test/integration/security/security.yml
- test/integration/oss-to-default-upgrade/serverspec/default_spec.rb
- test/integration/helpers/serverspec/Gemfile
- test/integration/default/default.yml
- test/integration/license/license.yml
- test/integration/debug.yml
- test/integration/upgrade/upgrade.yml
- test/integration/files/custom_config/elasticsearch
- test/integration/files/custom_config/jvm.options
- test/integration/license/serverspec/default_spec.rb
- test/integration/upgrade/serverspec/default_spec.rb
- test/integration/security/serverspec/default_spec.rb
- test/integration/trial/serverspec/default_spec.rb
- test/integration/files/templates-6.x/basic.json
- test/integration/oss/serverspec/default_spec.rb
- test/integration/files/templates-7.x/basic.json
- test/integration/trial/trial.yml
- test/matrix-6x.yml
- test/integration/custom-config.yml
- test/integration/helpers/serverspec/custom_config_spec.rb
- test/integration/oss.yml
- test/integration/custom-config/serverspec/default_spec.rb
- test/integration/default.yml
- test/integration/oss-upgrade.yml
- test/integration/oss-to-default-upgrade.yml
- test/integration/oss-upgrade/serverspec/default_spec.rb
- Makefile
- test/integration/helpers/serverspec/spec_helper.rb
- test/integration/helpers/serverspec/license_spec.rb
- test/integration/upgrade.yml
- test/integration/files/custom_config/log4j2.properties
- test/integration/security.yml
- test/integration/license.yml
- test/integration/trial.yml
- test/integration/helpers/serverspec/security_spec.rb
- .kitchen.yml
- test/matrix.yml
- test/integration/helpers/serverspec/shared_spec.rb
- test/integration/default/serverspec/default_spec.rb
✅ Files skipped from review due to trivial changes (13)
- README.md
- requirements.txt
- tasks/main.yml
- molecule/custom-config/molecule.yml
- molecule/default/molecule.yml
- molecule/security/converge.yml
- molecule/security/prepare.yml
- molecule/security/molecule.yml
- molecule/custom-config/verify.yml
- molecule/default/verify.yml
- molecule/custom-config/converge.yml
- molecule/default/converge.yml
- molecule/custom-config/files/log4j2.properties
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/molecule.yml
- molecule/security/verify.yml
6266058 to
0e9b426
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/molecule.yml:
- Around line 12-30: The workflow job "molecule" declares needs: lint but the
"lint" job is commented out, causing an invalid dependency; either
restore/uncomment the "lint" job block (so a job named lint exists) or remove
the dependency by deleting or changing the needs: lint line in the "molecule"
job; locate the "molecule" job and the commented lint block in the workflow and
apply one of these fixes so the job graph is valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 852306f1-e2b9-40c0-8f53-e5c12e8c2c59
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (68)
.github/workflows/molecule.yml.gitignore.kitchen.yml.ruby-versionGemfileMakefileREADME.mdmeta/main.ymlmolecule/custom-config/converge.ymlmolecule/custom-config/files/log4j2.propertiesmolecule/custom-config/molecule.ymlmolecule/custom-config/verify.ymlmolecule/default/converge.ymlmolecule/default/molecule.ymlmolecule/default/verify.ymlmolecule/security/converge.ymlmolecule/security/molecule.ymlmolecule/security/prepare.ymlmolecule/security/verify.ymlrequirements.txttasks/main.ymltest/integration/custom-config.ymltest/integration/custom-config/custom_config.ymltest/integration/custom-config/serverspec/default_spec.rbtest/integration/debug.ymltest/integration/default.ymltest/integration/default/default.ymltest/integration/default/serverspec/default_spec.rbtest/integration/files/certs/keystore-password-ca.p12test/integration/files/certs/keystore-password.p12test/integration/files/certs/shared-store-no-password-ca.p12test/integration/files/certs/shared-store-no-password.p12test/integration/files/certs/truststore-password-ca.p12test/integration/files/certs/truststore-password.p12test/integration/files/custom_config/elasticsearchtest/integration/files/custom_config/jvm.optionstest/integration/files/custom_config/log4j2.propertiestest/integration/files/templates-6.x/basic.jsontest/integration/files/templates-7.x/basic.jsontest/integration/helpers/serverspec/Gemfiletest/integration/helpers/serverspec/custom_config_spec.rbtest/integration/helpers/serverspec/license_spec.rbtest/integration/helpers/serverspec/security_spec.rbtest/integration/helpers/serverspec/shared_spec.rbtest/integration/helpers/serverspec/spec_helper.rbtest/integration/license.ymltest/integration/license/license.ymltest/integration/license/serverspec/default_spec.rbtest/integration/oss-to-default-upgrade.ymltest/integration/oss-to-default-upgrade/oss-to-default-upgrade.ymltest/integration/oss-to-default-upgrade/serverspec/default_spec.rbtest/integration/oss-upgrade.ymltest/integration/oss-upgrade/oss.ymltest/integration/oss-upgrade/serverspec/default_spec.rbtest/integration/oss.ymltest/integration/oss/oss.ymltest/integration/oss/serverspec/default_spec.rbtest/integration/security.ymltest/integration/security/security.ymltest/integration/security/serverspec/default_spec.rbtest/integration/trial.ymltest/integration/trial/serverspec/default_spec.rbtest/integration/trial/trial.ymltest/integration/upgrade.ymltest/integration/upgrade/serverspec/default_spec.rbtest/integration/upgrade/upgrade.ymltest/matrix-6x.ymltest/matrix.yml
💤 Files with no reviewable changes (46)
- test/integration/license/serverspec/default_spec.rb
- test/integration/custom-config/custom_config.yml
- test/integration/oss/oss.yml
- .gitignore
- .ruby-version
- test/integration/oss-to-default-upgrade/serverspec/default_spec.rb
- test/integration/debug.yml
- test/integration/trial/trial.yml
- test/integration/helpers/serverspec/Gemfile
- test/integration/security/security.yml
- test/integration/oss-upgrade/oss.yml
- test/integration/default/serverspec/default_spec.rb
- test/integration/default/default.yml
- test/matrix-6x.yml
- Gemfile
- test/integration/license/license.yml
- test/integration/oss.yml
- test/integration/files/custom_config/elasticsearch
- test/integration/trial/serverspec/default_spec.rb
- test/integration/custom-config.yml
- test/integration/oss/serverspec/default_spec.rb
- test/integration/files/templates-7.x/basic.json
- test/integration/upgrade/upgrade.yml
- test/matrix.yml
- test/integration/files/templates-6.x/basic.json
- test/integration/custom-config/serverspec/default_spec.rb
- test/integration/oss-to-default-upgrade.yml
- test/integration/helpers/serverspec/custom_config_spec.rb
- test/integration/oss-upgrade/serverspec/default_spec.rb
- test/integration/upgrade.yml
- test/integration/default.yml
- test/integration/files/custom_config/jvm.options
- test/integration/helpers/serverspec/license_spec.rb
- test/integration/oss-upgrade.yml
- test/integration/helpers/serverspec/spec_helper.rb
- Makefile
- test/integration/files/custom_config/log4j2.properties
- test/integration/security.yml
- test/integration/security/serverspec/default_spec.rb
- test/integration/upgrade/serverspec/default_spec.rb
- .kitchen.yml
- test/integration/trial.yml
- test/integration/helpers/serverspec/security_spec.rb
- test/integration/oss-to-default-upgrade/oss-to-default-upgrade.yml
- test/integration/helpers/serverspec/shared_spec.rb
- test/integration/license.yml
✅ Files skipped from review due to trivial changes (12)
- requirements.txt
- README.md
- molecule/default/molecule.yml
- tasks/main.yml
- molecule/security/converge.yml
- molecule/security/verify.yml
- molecule/security/molecule.yml
- molecule/default/converge.yml
- molecule/security/prepare.yml
- molecule/custom-config/molecule.yml
- molecule/default/verify.yml
- molecule/custom-config/converge.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- molecule/custom-config/verify.yml
- meta/main.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
molecule/default/verify.yml (1)
2-4: Avoid duplicating the Elasticsearch version literal in verify assertions.
8.19.10is hardcoded in both converge and verify, which is easy to desync during upgrades. Keep one variable in this play and reuse it in the assertion/fail message.♻️ Proposed refactor
- name: Verify hosts: all become: true + vars: + expected_es_version: "8.19.10" tasks: @@ - name: Assert correct version ansible.builtin.assert: that: - - "api_response.json.version.number == '8.19.10'" - fail_msg: "Expected 8.19.10, got {{ api_response.json.version.number }}" + - "api_response.json.version.number == expected_es_version" + fail_msg: "Expected {{ expected_es_version }}, got {{ api_response.json.version.number | default('unknown') }}"Also applies to: 24-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@molecule/default/verify.yml` around lines 2 - 4, Define a single variable (e.g., es_version) at the play level in the Verify play (the play with name "Verify") and set it to the Elasticsearch version currently used; then replace the hardcoded "8.19.10" literals in the assertion and fail message (the checks around lines 24-28) with that variable (use the play variable reference like {{ es_version }}) so the verify play reuses the single source of truth for the ES version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@molecule/default/verify.yml`:
- Around line 2-4: Define a single variable (e.g., es_version) at the play level
in the Verify play (the play with name "Verify") and set it to the Elasticsearch
version currently used; then replace the hardcoded "8.19.10" literals in the
assertion and fail message (the checks around lines 24-28) with that variable
(use the play variable reference like {{ es_version }}) so the verify play
reuses the single source of truth for the ES version.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d956b2c-a37f-48f5-8e67-f1f23ba50e29
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (68)
.github/workflows/molecule.yml.gitignore.kitchen.yml.ruby-versionGemfileMakefileREADME.mdmeta/main.ymlmolecule/custom-config/converge.ymlmolecule/custom-config/files/log4j2.propertiesmolecule/custom-config/molecule.ymlmolecule/custom-config/verify.ymlmolecule/default/converge.ymlmolecule/default/molecule.ymlmolecule/default/verify.ymlmolecule/security/converge.ymlmolecule/security/molecule.ymlmolecule/security/prepare.ymlmolecule/security/verify.ymlrequirements.txttasks/main.ymltest/integration/custom-config.ymltest/integration/custom-config/custom_config.ymltest/integration/custom-config/serverspec/default_spec.rbtest/integration/debug.ymltest/integration/default.ymltest/integration/default/default.ymltest/integration/default/serverspec/default_spec.rbtest/integration/files/certs/keystore-password-ca.p12test/integration/files/certs/keystore-password.p12test/integration/files/certs/shared-store-no-password-ca.p12test/integration/files/certs/shared-store-no-password.p12test/integration/files/certs/truststore-password-ca.p12test/integration/files/certs/truststore-password.p12test/integration/files/custom_config/elasticsearchtest/integration/files/custom_config/jvm.optionstest/integration/files/custom_config/log4j2.propertiestest/integration/files/templates-6.x/basic.jsontest/integration/files/templates-7.x/basic.jsontest/integration/helpers/serverspec/Gemfiletest/integration/helpers/serverspec/custom_config_spec.rbtest/integration/helpers/serverspec/license_spec.rbtest/integration/helpers/serverspec/security_spec.rbtest/integration/helpers/serverspec/shared_spec.rbtest/integration/helpers/serverspec/spec_helper.rbtest/integration/license.ymltest/integration/license/license.ymltest/integration/license/serverspec/default_spec.rbtest/integration/oss-to-default-upgrade.ymltest/integration/oss-to-default-upgrade/oss-to-default-upgrade.ymltest/integration/oss-to-default-upgrade/serverspec/default_spec.rbtest/integration/oss-upgrade.ymltest/integration/oss-upgrade/oss.ymltest/integration/oss-upgrade/serverspec/default_spec.rbtest/integration/oss.ymltest/integration/oss/oss.ymltest/integration/oss/serverspec/default_spec.rbtest/integration/security.ymltest/integration/security/security.ymltest/integration/security/serverspec/default_spec.rbtest/integration/trial.ymltest/integration/trial/serverspec/default_spec.rbtest/integration/trial/trial.ymltest/integration/upgrade.ymltest/integration/upgrade/serverspec/default_spec.rbtest/integration/upgrade/upgrade.ymltest/matrix-6x.ymltest/matrix.yml
💤 Files with no reviewable changes (46)
- test/integration/custom-config/custom_config.yml
- test/integration/oss-to-default-upgrade/oss-to-default-upgrade.yml
- test/integration/trial/trial.yml
- .ruby-version
- test/integration/oss/oss.yml
- test/integration/oss-upgrade/oss.yml
- test/integration/license/license.yml
- .gitignore
- test/integration/oss-to-default-upgrade/serverspec/default_spec.rb
- Gemfile
- test/integration/debug.yml
- test/integration/oss.yml
- test/integration/helpers/serverspec/Gemfile
- test/integration/default/serverspec/default_spec.rb
- test/integration/default/default.yml
- test/integration/custom-config.yml
- test/matrix-6x.yml
- test/integration/upgrade.yml
- test/matrix.yml
- test/integration/files/templates-6.x/basic.json
- test/integration/custom-config/serverspec/default_spec.rb
- test/integration/oss/serverspec/default_spec.rb
- test/integration/upgrade/serverspec/default_spec.rb
- test/integration/oss-upgrade/serverspec/default_spec.rb
- test/integration/default.yml
- test/integration/trial/serverspec/default_spec.rb
- test/integration/security/serverspec/default_spec.rb
- test/integration/oss-upgrade.yml
- test/integration/files/custom_config/jvm.options
- test/integration/helpers/serverspec/spec_helper.rb
- test/integration/helpers/serverspec/custom_config_spec.rb
- test/integration/oss-to-default-upgrade.yml
- test/integration/helpers/serverspec/license_spec.rb
- Makefile
- test/integration/files/custom_config/elasticsearch
- test/integration/security.yml
- test/integration/files/custom_config/log4j2.properties
- test/integration/files/templates-7.x/basic.json
- test/integration/security/security.yml
- test/integration/trial.yml
- test/integration/license.yml
- test/integration/license/serverspec/default_spec.rb
- test/integration/upgrade/upgrade.yml
- test/integration/helpers/serverspec/shared_spec.rb
- .kitchen.yml
- test/integration/helpers/serverspec/security_spec.rb
✅ Files skipped from review due to trivial changes (14)
- README.md
- requirements.txt
- tasks/main.yml
- molecule/custom-config/molecule.yml
- molecule/default/molecule.yml
- .github/workflows/molecule.yml
- molecule/security/converge.yml
- meta/main.yml
- molecule/security/molecule.yml
- molecule/security/verify.yml
- molecule/custom-config/verify.yml
- molecule/security/prepare.yml
- molecule/default/converge.yml
- molecule/custom-config/converge.yml
149b89a to
fb4d0ae
Compare
fb4d0ae to
2de42ff
Compare
The Ruby/Kitchen stack is legacy at this point
Summary by CodeRabbit
Tests
Chores
Documentation