Skip to content

feat: new variable postfix_secure_logging defaulting to true - #234

Merged
richm merged 1 commit into
linux-system-roles:mainfrom
spetrosi:parametrize-no-log
May 7, 2026
Merged

richm merged 1 commit into
linux-system-roles:mainfrom
spetrosi:parametrize-no-log

Conversation

@spetrosi

@spetrosi spetrosi commented May 7, 2026

Copy link
Copy Markdown
Contributor

Feature: Introduce the postfix_secure_logging variable that defaults to true.

Reason: Currently, all sensitive tasks use hard-coded no_log: true, which makes debugging difficult. Users cannot see credential-related output even when troubleshooting authentication or secret management issues.

Result:

  • Tasks handling credentials, secrets, and sensitive data now use no_log: "{{ postfix_secure_logging }}", allowing users to set postfix_secure_logging: false for debugging while maintaining secure defaults (true)
  • New variable postfix_secure_logging documented in README.md with guidance on when to disable it
  • Users can now debug credential and secret issues without modifying role code

🤖 Generated with Claude Code

Summary by Sourcery

Introduce a configurable secure logging toggle for sensitive Postfix role tasks, defaulting to secure behavior while allowing opt-out for debugging.

New Features:

  • Add a postfix_secure_logging variable to control whether sensitive Postfix tasks suppress output via no_log.

Enhancements:

  • Update sensitive Postfix tasks to use the postfix_secure_logging variable instead of hard-coded no_log settings for more flexible debugging control.

Documentation:

  • Document the postfix_secure_logging variable in the README, including its purpose, default value, and guidance on when to disable it.

- Replace literal no_log: true with postfix_secure_logging variable
- Add postfix_secure_logging: true to defaults/main.yml
- Document postfix_secure_logging variable in README.md

This change allows users to control logging of potentially sensitive
information by setting postfix_secure_logging: false for debugging,
while maintaining secure defaults.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@spetrosi
spetrosi requested a review from richm as a code owner May 7, 2026 12:56
@sourcery-ai

sourcery-ai Bot commented May 7, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Introduces a configurable postfix_secure_logging variable (defaulting to true) and wires it into sensitive tasks so their no_log behavior can be toggled, while documenting the new variable in the README.

Flow diagram for conditional no_log behavior using postfix_secure_logging

flowchart TD
  A[Start_postfix_role_tasks] --> B[Evaluate_postfix_secure_logging]
  B -->|true| C[Set_no_log_true_on_sensitive_tasks]
  B -->|false| D[Set_no_log_false_on_sensitive_tasks]
  C --> E[Execute_sensitive_tasks_with_suppressed_output]
  D --> F[Execute_sensitive_tasks_with_full_output]
  E --> G[Ansible_logs_hide_credentials]
  F --> H[Ansible_logs_show_credentials_for_debugging]
  G --> I[Finish_role_execution]
  H --> I[Finish_role_execution]
Loading

File-Level Changes

Change Details Files
Make logging of sensitive postfix tasks configurable via a new variable while preserving secure defaults.
  • Add postfix_secure_logging default variable set to true in role defaults
  • Replace hard-coded no_log: true with no_log: "{{ postfix_secure_logging }}" on credential-handling tasks to allow opt-out logging for debugging
  • Document the new postfix_secure_logging variable, its purpose, and safe usage guidance in the README
defaults/main.yml
tasks/main.yml
README.md

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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • Consider casting postfix_secure_logging to a boolean in the no_log expressions (e.g. no_log: "{{ postfix_secure_logging | bool }}") to avoid unexpected behavior if the variable is ever passed as a string via inventory or extra-vars.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider casting `postfix_secure_logging` to a boolean in the `no_log` expressions (e.g. `no_log: "{{ postfix_secure_logging | bool }}"`) to avoid unexpected behavior if the variable is ever passed as a string via inventory or extra-vars.

## Individual Comments

### Comment 1
<location path="tasks/main.yml" line_range="146" />
<code_context>
   loop: "{{ postfix_files }}"
   register: __postfix_postmap_files
-  no_log: true
+  no_log: "{{ postfix_secure_logging }}"
   loop_control:
     loop_var: file
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Coerce `postfix_secure_logging` to a boolean when using it for `no_log`.

To handle cases where `postfix_secure_logging` is defined as a string in inventory/group vars (e.g. `'false'`), please coerce it explicitly: `no_log: "{{ postfix_secure_logging | bool }}"` so `no_log` always receives a proper boolean.

Suggested implementation:

```
  loop: "{{ postfix_files }}"
  register: __postfix_postmap_files
  no_log: "{{ postfix_secure_logging | bool }}"
  loop_control:

```

```
  when:
    - result["changed"]
    - result["file"]["postmap"] | d(false)
  no_log: "{{ postfix_secure_logging | bool }}"
  changed_when: true

```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread tasks/main.yml
loop: "{{ postfix_files }}"
register: __postfix_postmap_files
no_log: true
no_log: "{{ postfix_secure_logging }}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): Coerce postfix_secure_logging to a boolean when using it for no_log.

To handle cases where postfix_secure_logging is defined as a string in inventory/group vars (e.g. 'false'), please coerce it explicitly: no_log: "{{ postfix_secure_logging | bool }}" so no_log always receives a proper boolean.

Suggested implementation:

  loop: "{{ postfix_files }}"
  register: __postfix_postmap_files
  no_log: "{{ postfix_secure_logging | bool }}"
  loop_control:

  when:
    - result["changed"]
    - result["file"]["postmap"] | d(false)
  no_log: "{{ postfix_secure_logging | bool }}"
  changed_when: true

@richm
richm merged commit 413bde1 into linux-system-roles:main May 7, 2026
29 checks passed
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.

2 participants