Skip to content

fix(php): accept short names in php_extensions_custom - #1679

Open
faisalahammad wants to merge 1 commit into
roots:masterfrom
faisalahammad:fix/1666-php-extensions-short-names
Open

fix(php): accept short names in php_extensions_custom#1679
faisalahammad wants to merge 1 commit into
roots:masterfrom
faisalahammad:fix/1666-php-extensions-short-names

Conversation

@faisalahammad

Copy link
Copy Markdown
Contributor

Summary

Lets users write a plain list of short names in php_extensions_custom instead of the verbose php{{ php_version }}-<name> dict form. The legacy dict form is still accepted.

Fixes #1666

Changes

roles/php/defaults/main.yml

Before:

php_extensions_custom: {}
php_extensions: "{{ php_extensions_default | combine(php_extensions_custom) }}"

After:

php_extensions_custom: []
php_extensions_custom_normalized: "{{ php_extensions_custom | php_extensions(php_version, apt_package_state) }}"
php_extensions: "{{ php_extensions_default | combine(php_extensions_custom_normalized) }}"

Why: combine() needs a dict on the right side. A new php_extensions filter coerces list input to {php<version>-<name>: <state>} and passes dict input through unchanged.

lib/trellis/plugins/filter/filters.py

After:

def php_extensions(value, php_version, package_state):
    if isinstance(value, dict):
        return value
    if isinstance(value, list):
        return {f"php{php_version}-{name}": package_state for name in value}
    return {}

Why: Single source of truth for the list vs dict shape decision.

roles/common/tasks/main.yml

Before: one package_vars dict, all entries checked with rejectattr('value', 'mapping'). php_extensions_custom was treated as dict-only.

After: split into package_vars_dict_only (mapping check) and package_vars_list_or_dict (mapping OR non-string sequence). The wrong-format list is the union of both wrong-key sets, so a string or int input to php_extensions_custom is still rejected.

Why: is sequence in Jinja2 is also true for strings, so the new predicate computes the valid-key set explicitly.

Testing

Test 1: list form accepted

  1. Set php_extensions_custom: [soap, gd] in group_vars/all/main.yml.
  2. Run pytest -q -ra tests/templates.
    Result: 6 new tests in tests/templates/test_php_extensions_custom.py pass, plus the 9 pre-existing tests.

Test 2: dict form still works

  1. Set php_extensions_custom: { \"php{{ php_version }}-soap\": \"{{ apt_package_state }}\" }.
  2. Run pytest -q -ra tests/templates and ansible-playbook --syntax-check -e env=development server.yml.
    Result: same merged dict as before the change. Syntax check clean.

Test 3: bad input rejected

  1. Set php_extensions_custom: \"oops\" (a bare string).
  2. Run any play that includes the common role.
    Result: verify task fails with a message naming php_extensions_custom as a wrong-format variable.

Lets users write a plain list of short names in group_vars/all/main.yml:

  php_extensions_custom:
    - soap
    - gd

Each entry expands to "php<version>-<name>": "<apt_package_state>" and
merges into php_extensions_default alongside the existing dict form, which
is still accepted unchanged.

The normalization lives in a small php_extensions filter so the shape
decision is in one place. The verify task in roles/common now checks
mapping for the other package vars and accepts mapping or non-string
sequence for php_extensions_custom.

Fixes roots#1666
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simplify php_extensions_custom syntax to accept short names

1 participant