Skip to content

Conversation

@alessfg
Copy link
Member

@alessfg alessfg commented Mar 12, 2025

Proposed changes

Correctly use the nginx_version (if defined) for NGINX module versions. Fixes #861.

Checklist

Before creating a PR, run through this checklist and mark each as complete:

Copilot AI review requested due to automatic review settings March 12, 2025 21:15
@alessfg alessfg self-assigned this Mar 12, 2025
@alessfg alessfg added the bug Something isn't working label Mar 12, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes issue #861 by ensuring that when installing NGINX modules the global nginx_version is used (if defined) instead of the previously hardcoded value.

  • Updated the CHANGELOG to document the fix.
  • Modified tasks/modules/install-modules.yml to use nginx_version as the default value in module installation.
  • Adjusted molecule tests by removing version fields for geoip and perl modules where versioning is no longer supported.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
CHANGELOG.md Updated changelog entry to reflect the unreleased fix.
tasks/modules/install-modules.yml Refactored version default assignment to incorporate nginx_version.
molecule/version/converge.yml Removed unsupported version assignments for geoip and perl modules.
Comments suppressed due to low confidence (3)

tasks/modules/install-modules.yml:31

  • [nitpick] The default filter expression is rather complex; consider refactoring it for clarity, for example by using '{{ item['version'] | default(nginx_version | default('')) }}'.
{{ item['version'] | default((nginx_version is defined) | ternary(nginx_version, '')) }}

molecule/version/converge.yml:38

  • The removal of the version field for the geoip module appears intended; please confirm this change aligns with the module’s behavior regarding version support.
version: "{{ ngx_version }}"

molecule/version/converge.yml:44

  • The removal of the version field for the perl module is appropriate if versioning is not supported; please verify that this behavior is expected.
version: "{{ ngx_version }}"

@alessfg alessfg added this to the 0.25.1 milestone Mar 12, 2025
@alessfg alessfg enabled auto-merge March 12, 2025 22:02
@alessfg alessfg added this pull request to the merge queue Mar 13, 2025
Merged via the queue into main with commit cff9791 Mar 13, 2025
20 checks passed
@alessfg alessfg deleted the fix-module-version branch March 13, 2025 00:52
@github-project-automation github-project-automation bot moved this from In progress to Done in NGINX Ansible roles & collection Mar 13, 2025
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.

nginx_version is applied only on main package but not module

2 participants