Skip to content

Fix AIP boolean assertion for Ansible 12 jinja2_native mode#14950

Closed
dguido wants to merge 1 commit intomasterfrom
fix/aip-boolean-assertion-14948
Closed

Fix AIP boolean assertion for Ansible 12 jinja2_native mode#14950
dguido wants to merge 1 commit intomasterfrom
fix/aip-boolean-assertion-14948

Conversation

@dguido
Copy link
Copy Markdown
Member

@dguido dguido commented Jan 30, 2026

Summary

  • Fix assert.that clause in AIP task that fails with Ansible 12+ due to jinja2_native mode
  • The ansible.utils.ipv4 filter returns the IP string (not a boolean), which causes "Conditional result was derived from value of type 'str'" error
  • Add | bool filter to convert the truthy string to a proper boolean

Fixes #14948

Test plan

  • ansible-playbook main.yml --syntax-check passes
  • ansible-lint roles/common/tasks/aip/main.yml passes (0 failures, 0 warnings)
  • pytest tests/unit/ -q passes (91 tests)

🤖 Generated with Claude Code

The ansible.utils.ipv4 filter returns the IP string when valid, not a
boolean. Ansible 12+ enables jinja2_native by default, which requires
assert.that clauses to evaluate to actual booleans.

Add | bool filter to convert the truthy string to a proper boolean.

Fixes #14948

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@dguido dguido requested a review from jackivanov as a code owner January 30, 2026 03:31
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Jan 30, 2026

Code Review

Summary

This PR correctly fixes the Ansible 12+ jinja2_native mode compatibility issue in the AIP (Alternative Ingress IP) task. The change is minimal, targeted, and aligns with the project's documented best practices.

Positive Aspects

Correct Fix for jinja2_native Mode: The addition of | bool follows the exact guidance in CLAUDE.md:

"Boolean conditionals require actual booleans" - The ansible.utils.ipv4 filter returns a string representation of the IP address (truthy) when valid, which Ansible 12+ rejects in conditionals.

Minimal Change: Single line modification that solves exactly the issue reported in #14948 without introducing side effects.

Quality Gates Passed: The PR description confirms all mandatory checks passed:

  • ansible-playbook main.yml --syntax-check
  • ansible-lint (0 failures, 0 warnings)
  • pytest tests/unit/ (91 tests)

Proper Parenthesization: Uses (snat_aipv4 | ansible.utils.ipv4) | bool to ensure correct filter order - validates IP first, then converts to boolean.

Technical Analysis

The fix addresses the core issue: ansible.utils.ipv4 returns the IP string itself when valid (e.g., "192.168.1.1"), or an empty string when invalid. In Ansible 11, this worked as a conditional because any truthy string was accepted. Ansible 12's jinja2_native mode requires explicit boolean types in assertions.

Filter Behavior:

  • Valid IP: "192.168.1.1" | ansible.utils.ipv4"192.168.1.1" (truthy string)
  • Invalid IP: "invalid" | ansible.utils.ipv4"" (empty string)
  • With | bool: Converts truthy/falsy strings to proper booleans

Considerations

Test Coverage: The only usage of ansible.utils.ipv4 in the codebase is this specific line, so no other locations need updating. However, consider whether a unit test could verify this assertion doesn't regress with future Ansible versions. Current test suite passes, but direct AIP functionality testing appears limited.

Documentation: The error message is clear and actionable: "The SNAT IPv4 address not found. Cannot proceed with the alternative ingress ip."

Recommendation

Approve and merge. This is a clean fix that follows project standards and solves a real compatibility issue blocking DigitalOcean deployments with Ansible 12+.

Additional Notes

Verified against CLAUDE.md section on Jinja2 Native Mode (lines 163-187), which explicitly covers this pattern. The fix is idiomatic and future-proof.

@dguido
Copy link
Copy Markdown
Member Author

dguido commented Jan 30, 2026

Closing in favor of #14949 which uses the more correct approach: using the is ansible.utils.ipv4_address test (returns native boolean) rather than my filter + | bool workaround. Their | trim also handles potential whitespace from the metadata endpoint.

@dguido dguido closed this Jan 30, 2026
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.

AIP functionality broken

1 participant