Skip to content

Commit 984831b

Browse files
dguidoclaude
andauthored
fix: add explicit bool filters for Ansible 12 jinja2_native compatibility (#14963)
* fix: add explicit bool filters for Ansible 12 jinja2_native compatibility Ansible 12 enables jinja2_native by default, which means string values like "true"/"false" are no longer automatically coerced to booleans in when: conditions and Jinja2 if statements. Add | bool filters to all boolean variable references in tasks, templates, and handlers. Also reformats long single-line Jinja2 conditionals into multi-line for readability, fixes GCE default() calls for native mode, adds help command to the algo script, and updates test fixtures to register the bool filter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: add j2lint for Jinja2 template linting Add j2lint (aristanetworks/j2lint) to catch syntax errors, spacing issues, and operator formatting in Jinja2 templates. Integrated into pre-commit hooks, lint.yml CI, and smart-tests.yml. Rules S3/S5/S6/S7/V1 are ignored — they enforce conventions incompatible with Ansible's config-file-embedded templates. Also fixes int+1 → int + 1 operator spacing in server.conf.j2. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve all ansible-lint warnings and enforce zero-tolerance policy Fix 18 jinja[spacing] errors across 12 files by moving Jinja2 block delimiters to prevent YAML >- folding from introducing trailing spaces. Fix 27 key-order[task] warnings across 17 files by reordering task keys to canonical order (name → when → tags → environment → become → block). Promote key-order[task] and yaml[line-length] from warn_list to hard errors by removing warn_list entirely from .ansible-lint. Add zero-tolerance warning policy to CLAUDE.md explaining why warnings are unacceptable in a security tool and documenting resolution order. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0056bc7 commit 984831b

48 files changed

Lines changed: 348 additions & 158 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.ansible-lint

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ skip_list:
2222
- 'no-handler' # Handler usage - some legitimate non-handler use cases
2323
- 'name[missing]' # All tasks should be named - 113 issues to fix (temporary)
2424

25-
warn_list:
26-
- yaml[line-length] # Line length - informational only
27-
- key-order[task] # Task key ordering - many existing violations, fix gradually
28-
2925
# Enable additional rules
3026
enable_list:
3127
- no-log-password

.github/workflows/lint.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,28 @@ jobs:
5151
- name: Run yamllint
5252
run: uv run --with yamllint yamllint -c .yamllint .
5353

54+
jinja2-lint:
55+
name: Jinja2 template linting
56+
runs-on: ubuntu-22.04
57+
steps:
58+
- uses: actions/checkout@0c366fd6a839edf440554fa01a7085ccba70ac98 # v5.0.1
59+
with:
60+
persist-credentials: false
61+
62+
- name: Setup uv environment
63+
uses: ./.github/actions/setup-uv
64+
65+
- name: Run j2lint
66+
run: |
67+
# Lint Jinja2 templates for syntax and style issues
68+
# Ignored rules (incompatible with Ansible config-file templates):
69+
# S3: indentation (dictated by output format, not Jinja style)
70+
# S5: tabs (some config formats require them)
71+
# S6: whitespace-control delimiters ({%- -%} are standard Ansible)
72+
# S7: single-statement-per-line (inline Jinja in config output)
73+
# V1: lowercase variables (existing names like IP_subject_alt_name)
74+
uv run --with j2lint j2lint roles/ --ignore S3 S5 S6 S7 V1
75+
5476
python-lint:
5577
name: Python linting
5678
runs-on: ubuntu-22.04

.github/workflows/smart-tests.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ jobs:
5959
- '**/*.yml'
6060
- '**/*.yaml'
6161
- '**/*.sh'
62+
- '**/*.j2'
6263
- '.ansible-lint'
6364
- '.yamllint'
6465
- 'pyproject.toml'
@@ -254,6 +255,11 @@ jobs:
254255
# Run Ansible linter
255256
uv run --with ansible-lint ansible-lint
256257
258+
# Check Jinja2 templates
259+
if git diff --name-only "${BASE_SHA}" "${HEAD_SHA}" | grep -q '\.j2$'; then
260+
uv run --with j2lint j2lint roles/ --ignore S3 S5 S6 S7 V1
261+
fi
262+
257263
# Check shell scripts if any changed
258264
if git diff --name-only "${BASE_SHA}" "${HEAD_SHA}" | grep -q '\.sh$'; then
259265
find . -name "*.sh" -type f -not -path "./.git/*" -exec shellcheck {} +

.pre-commit-config.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ repos:
5353
types: [python]
5454
pass_filenames: false
5555

56+
- id: j2lint
57+
name: Jinja2 template lint
58+
entry: bash -c 'uv run j2lint roles/ --ignore S3 S5 S6 S7 V1'
59+
language: system
60+
files: '\.j2$'
61+
pass_filenames: false
62+
5663
- id: ansible-lint
5764
name: Ansible-lint
5865
entry: bash -c 'uv run ansible-lint --force-color || echo "Ansible-lint had issues - check output"'

CLAUDE.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,22 @@ Common lint issues to fix before submitting:
6767
- Jinja2 spacing errors (`{{foo}}` should be `{{ foo }}`)
6868
- Missing `mode:` on file/directory tasks
6969

70+
### Zero-Tolerance Warning Policy
71+
72+
**No warnings are tolerated in CI.** Every linter finding must be either fixed or explicitly allowlisted in the tool's config file (`.ansible-lint`, `pyproject.toml`, etc.).
73+
74+
Why this matters for Algo:
75+
- **Security tool** - VPN misconfigurations silently break privacy guarantees. A "cosmetic" warning today hides a real bug tomorrow.
76+
- **Ansible complexity** - YAML+Jinja2 linting catches real runtime failures (wrong key order breaks `when` evaluation, spacing errors cause template failures). Warnings in Ansible are not style nits.
77+
- **CI signal integrity** - If 30 warnings scroll by on every run, the 31st one (a real regression) goes unnoticed. Zero warnings means every new finding gets human attention.
78+
79+
Resolution order of preference:
80+
1. **Fix it** - Preferred. Most findings have straightforward fixes.
81+
2. **Allowlist in config** - If the rule is wrong for this project, add to `skip_list` with a comment explaining why.
82+
3. **Inline suppress** - Last resort. Use `# noqa: rule-name` with a comment justifying the exception.
83+
84+
Never use `warn_list` in `.ansible-lint` — it exists as a migration tool, not a permanent home. Rules either pass or are explicitly skipped.
85+
7086
### Design Requirements
7187

7288
When adding or modifying features, verify these before requesting review:

algo

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,30 @@ fi
160160

161161
# Run the appropriate playbook
162162
case "$1" in
163+
help|-h|--help)
164+
echo "Usage: ./algo [COMMAND] [ANSIBLE_OPTIONS]"
165+
echo ""
166+
echo "Set up a personal VPN in the cloud."
167+
echo ""
168+
echo "Commands:"
169+
echo " (default) Deploy a new VPN server"
170+
echo " update-users Add or remove users on an existing server"
171+
echo ""
172+
echo "Configuration:"
173+
echo " Edit config.cfg to set users, DNS, and VPN options before deploying."
174+
echo ""
175+
echo "Non-interactive deployment:"
176+
echo " ./algo -e 'provider=digitalocean server_name=algo region=nyc3 do_token=TOKEN'"
177+
echo ""
178+
echo "Common Ansible options (passed through):"
179+
echo " -e KEY=VALUE Set variable (bypass interactive prompts)"
180+
echo " -v, -vvv Increase output verbosity"
181+
echo " --skip-tags TAG Skip specific components"
182+
echo " -t, --tags TAG Run only specific components"
183+
echo ""
184+
echo "Docs: https://trailofbits.github.io/algo/"
185+
exit 0
186+
;;
163187
update-users)
164188
uv run ansible-playbook users.yml "${@:2}" -t update-users ;;
165189
*)

input.yml

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,22 +109,44 @@
109109
- name: Set facts based on the input
110110
set_fact:
111111
algo_server_name: >-
112-
{% if server_name is defined %}{% set _server = server_name %}{%- elif _algo_server_name.user_input is defined and _algo_server_name.user_input | length > 0 -%}
113-
{%- set _server = _algo_server_name.user_input -%}
114-
{%- else %}{% set _server = defaults['server_name'] %}{% endif -%}
112+
{%- if server_name is defined -%}{% set _server = server_name %}{%-
113+
elif _algo_server_name.user_input is defined and _algo_server_name.user_input | length > 0 -%}{%-
114+
set _server = _algo_server_name.user_input -%}{%-
115+
else -%}{% set _server = defaults['server_name'] %}{%-
116+
endif -%}
115117
{{ _server | regex_replace('(?!\.)(\W|_)', '-') }}
116118
algo_ondemand_cellular: >-
117-
{% if ondemand_cellular is defined %}{{ ondemand_cellular | bool }}{%- elif _ondemand_cellular.user_input is defined %}{{ booleans_map[_ondemand_cellular.user_input] | default(defaults['ondemand_cellular']) }}{%- else %}{{ false }}{% endif %}
119+
{%- if ondemand_cellular is defined -%}{{ ondemand_cellular | bool }}{%-
120+
elif _ondemand_cellular.user_input is defined -%}{{ booleans_map[_ondemand_cellular.user_input] | default(defaults['ondemand_cellular']) }}{%-
121+
else -%}{{ false }}{%-
122+
endif -%}
118123
algo_ondemand_wifi: >-
119-
{% if ondemand_wifi is defined %}{{ ondemand_wifi | bool }}{%- elif _ondemand_wifi.user_input is defined %}{{ booleans_map[_ondemand_wifi.user_input] | default(defaults['ondemand_wifi']) }}{%- else %}{{ false }}{% endif %}
124+
{%- if ondemand_wifi is defined -%}{{ ondemand_wifi | bool }}{%-
125+
elif _ondemand_wifi.user_input is defined -%}{{ booleans_map[_ondemand_wifi.user_input] | default(defaults['ondemand_wifi']) }}{%-
126+
else -%}{{ false }}{%-
127+
endif -%}
120128
algo_ondemand_wifi_exclude: >-
121-
{% if ondemand_wifi_exclude is defined %}{{ ondemand_wifi_exclude | b64encode }}{%- elif _ondemand_wifi_exclude.user_input is defined and _ondemand_wifi_exclude.user_input | length > 0 -%}
122-
{{ _ondemand_wifi_exclude.user_input | b64encode }}{%- else %}{{ '_null' | b64encode }}{% endif %}
129+
{%- if ondemand_wifi_exclude is defined -%}{{ ondemand_wifi_exclude | b64encode }}{%-
130+
elif _ondemand_wifi_exclude.user_input is defined and _ondemand_wifi_exclude.user_input | length > 0 -%}
131+
{{ _ondemand_wifi_exclude.user_input | b64encode }}{%-
132+
else -%}{{ '_null' | b64encode }}{%-
133+
endif -%}
123134
algo_dns_adblocking: >-
124-
{% if dns_adblocking is defined %}{{ dns_adblocking | bool }}{%- elif _dns_adblocking.user_input is defined %}{{ booleans_map[_dns_adblocking.user_input] | default(defaults['dns_adblocking']) }}{%- else %}{{ false }}{% endif %}
135+
{%- if dns_adblocking is defined -%}{{ dns_adblocking | bool }}{%-
136+
elif _dns_adblocking.user_input is defined -%}{{ booleans_map[_dns_adblocking.user_input] | default(defaults['dns_adblocking']) }}{%-
137+
else -%}{{ false }}{%-
138+
endif -%}
125139
algo_ssh_tunneling: >-
126-
{% if ssh_tunneling is defined %}{{ ssh_tunneling | bool }}{%- elif _ssh_tunneling.user_input is defined %}{{ booleans_map[_ssh_tunneling.user_input] | default(defaults['ssh_tunneling']) }}{%- else %}{{ false }}{% endif %}
140+
{%- if ssh_tunneling is defined -%}{{ ssh_tunneling | bool }}{%-
141+
elif _ssh_tunneling.user_input is defined -%}{{ booleans_map[_ssh_tunneling.user_input] | default(defaults['ssh_tunneling']) }}{%-
142+
else -%}{{ false }}{%-
143+
endif -%}
127144
algo_store_pki: >-
128-
{% if ipsec_enabled %}{%- if store_pki is defined %}{{ store_pki | bool }}{%- elif _store_pki.user_input is defined %}{{ booleans_map[_store_pki.user_input] | default(defaults['store_pki']) }}{%- else %}{{ false }}{% endif %}{% endif %}
145+
{%- if ipsec_enabled -%}
146+
{%- if store_pki is defined -%}{{ store_pki | bool }}{%-
147+
elif _store_pki.user_input is defined -%}{{ booleans_map[_store_pki.user_input] | default(defaults['store_pki']) }}{%-
148+
else -%}{{ false }}{%-
149+
endif -%}
150+
{%- endif -%}
129151
rescue:
130152
- include_tasks: playbooks/rescue.yml

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ dev-dependencies = [
119119
"ruff>=0.8.0", # Python linter and formatter
120120
"yamllint>=1.35.0", # YAML linter
121121
"ansible-lint>=24.0.0", # Ansible linter
122+
"j2lint>=1.2.0", # Jinja2 template linter
122123
]
123124

124125
[tool.pytest.ini_options]

roles/cloud-azure/tasks/main.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
- set_fact:
66
algo_region: >-
7-
{% if region is defined %}{{ region }}{%- elif _algo_region.user_input %}{{ azure_regions[_algo_region.user_input | int - 1]['name'] }}{%- else %}{{ azure_regions[default_region | int - 1]['name'] }}{% endif %}
7+
{%- if region is defined -%}{{ region }}{%-
8+
elif _algo_region.user_input -%}{{ azure_regions[_algo_region.user_input | int - 1]['name'] }}{%-
9+
else -%}{{ azure_regions[default_region | int - 1]['name'] }}{%-
10+
endif -%}
811
912
- name: Create AlgoVPN Server
1013
azure.azcollection.azure_rm_deployment:

roles/cloud-azure/tasks/prompts.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
subscription_id: "{{ azure_subscription_id | default(lookup('env', 'AZURE_SUBSCRIPTION_ID'), true) }}"
77
no_log: true
88

9-
- block:
9+
- when: region is undefined
10+
block:
1011
- name: Set the default region
1112
set_fact:
1213
default_region: >-
@@ -22,4 +23,3 @@
2223
Enter the number of your desired region
2324
[{{ default_region }}]
2425
register: _algo_region
25-
when: region is undefined

0 commit comments

Comments
 (0)