fix: Avoid append() in sudoers file template

[martinpitt]

Cause: Ansible 2.19 does not allow calling .append() on a list any more (ansible/ansible#85202). You also can't reassign variables in a loop, so list addition does not work either.

Consequence: The role failed on Ansible 2.19.

Fix: Rewrite the template to using filters instead of explicit dict iteration.

---

This is supposed to fix the Ansible 2.19 failure. It took me hours to get the spacing exactly right, but I developed this against 2.17 (to make sure I don't regress). Turns out in 2.19 it is still making a mess, and I don't know how. Parking my current state as draft PR, today I don't have the energy to work on this further.

Summary by Sourcery

Refactor the sudoers Jinja2 template to eliminate unsupported list .append() calls and rely on built-in filters for list rendering, restoring compatibility with Ansible 2.19 and streamlining template logic.

Bug Fixes:

• Remove all .append() operations and loop-based list building in the sudoers template to fix failures on Ansible 2.19

Enhancements:

• Simplify render_aliases and render_override_spec macros by directly joining list variables using Jinja2 filters
• Streamline secure_path and user specification sections to use conditional checks and join filters instead of manual list assembly

[sourcery-ai]

Reviewer's Guide

Rewritten the sudoers.j2 template to remove deprecated list.append calls and manual accumulation in macros and loops, replacing them with direct Jinja2 filter-based joins and inline conditionals to restore compatibility with Ansible 2.19.

Class diagram for updated Jinja2 template macros in sudoers.j2

classDiagram
    class render_aliases {
        +alias_desc
        +alias_str
        +alias_dict
        +alias_subdict_name
        +render_aliases(item)
    }
    class render_override_spec {
        +spec
        +spec_type
        +spec_dict
        +sign
        +render_override_spec(spec)
    }
    render_aliases --|> render_override_spec : uses

Loading

Flow diagram for sudoers.j2 template rendering logic after refactor

flowchart TD
    A[Start template rendering] --> B{item.aliases defined?}
    B -- Yes --> C[Render aliases using join filter]
    B -- No --> D[Skip alias rendering]
    C --> E{item.defaults defined?}
    D --> E
    E -- Yes --> F[Render defaults using join filter]
    E -- No --> G[Skip defaults]
    F --> H{item.user_specifications defined?}
    G --> H
    H -- Yes --> I[Render user specifications with inline joins and conditionals]
    H -- No --> J[Skip user specifications]
    I --> K[Render default_overrides if defined]
    J --> K
    K --> L[End template rendering]

Loading

File-Level Changes

Change	Details	Files
Refactor alias rendering macro to eliminate manual list building	Removed inner loops and _alias list initialisation Directly joined alias_item[alias_subdict_name] with join filter Added empty-string fallback in else branch	templates/sudoers.j2
Simplify override spec macro by using filters instead of accumulation	Removed _spec_original and _spec_default lists and append calls Joined spec[spec_dict] and spec.defaults directly with join filter Added empty-string fallback when spec.type mismatches	templates/sudoers.j2
Streamline secure_path default handling	Eliminated paths list and append loop Used values	join(":") directly for secure_path
Collapse user_specifications processing to filter-based output	Removed all intermediate lists (user_spec, spec_* etc.) and append loops Built user_spec output by chaining join filters and inline conditionals Checked presence of spec attributes before joining to avoid empty output	templates/sudoers.j2

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[martinpitt]

I've sat down another hour, also with Claude, GPT, etc. I have a template version which eliminates all macros and hence is very very verbose, and that (almost) works. But this is too frustrating right now.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@2c4f3eb). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #62   +/-   ##
=======================================
  Coverage        ?   52.31%           
=======================================
  Files           ?        1           
  Lines           ?      346           
  Branches        ?        0           
=======================================
  Hits            ?      181           
  Misses          ?      165           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[martinpitt]

@richm so I think the change here is actually valid -- it's required but not sufficient. It additionally needs some toad/crab treatment {{- '' - }}?

[martinpitt]

[citest]

[sourcery-ai]

Hey @martinpitt - I've reviewed your changes and they look great!

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[richm]

@richm so I think the change here is actually valid -- it's required but not sufficient. It additionally needs some toad/crab treatment {{- '' - }}?

Looks like it - seeing things like NoneNoneNoneNoneDefaults! /usr/bin/ls !requiretty

[richm]

@martinpitt with the two suggested changes, all tests pass with 2.19 on my local system

[martinpitt]

@richm nice, thanks! Doing a quick CI run, if green I'll squash and update the commit description.

[martinpitt]

[citest]

[richm]

[citest]

[martinpitt]

Nice! Will squash on merge then.

[martinpitt]

err, wait, many CI tests are red, but for infra reasons.

[sourcery-ai]

Hey @martinpitt - I've reviewed your changes - here's some feedback:

• Ensure the combined if-check in the user_spec macro (for users, hosts, commands) doesn’t accidentally skip specs when optional fields (like operators or tags) are empty—consider handling each optional field separately so you don’t drop entire lines.
• The explicit else blocks that emit an empty string are redundant; you can remove them and rely on Jinja’s whitespace control to avoid unwanted blank lines.
• Review the use of {%- -%} whitespace control to confirm the rendered sudoers file still has the correct line breaks and alignment required by sudoers syntax.

Prompt for AI Agents

Please address the comments from this code review:
## Overall Comments
- Ensure the combined if-check in the user_spec macro (for users, hosts, commands) doesn’t accidentally skip specs when optional fields (like operators or tags) are empty—consider handling each optional field separately so you don’t drop entire lines.
- The explicit else blocks that emit an empty string are redundant; you can remove them and rely on Jinja’s whitespace control to avoid unwanted blank lines.
- Review the use of {%- -%} whitespace control to confirm the rendered sudoers file still has the correct line breaks and alignment required by sudoers syntax.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[martinpitt]

[citest_bad]

[martinpitt]

That's a case of successful run, but failed overall status. I see the same in linux-system-roles/nbde_server#188 , so presumably an infra outage? The workflow says "gateway timeout"