Skip to content

Conversation

@JacobCallahan
Copy link
Member

Configuration:

  • Updated broker dependency from 0.7.3 to 0.8.0 in requirements.txt.

Refactoring:

  • Removed explicit calls to broker_log_setup from robottelo/logging.py and pytest_plugins/logging_hooks.py.
  • broker version 0.8 now manages its logging configuration internally or through different mechanisms, making the previous explicit setup calls redundant or incompatible.

Features:

  • Integrated broker.logging.RedactingFilter to automatically redact sensitive information, such as passwords and tokens, from broker log output.
  • This enhances log security by preventing the accidental leakage of credentials.
  • The TRACE log level is now automatically registered upon importing broker.logging.

Configuration:
- Updated `broker` dependency from `0.7.3` to `0.8.0` in `requirements.txt`.

Refactoring:
- Removed explicit calls to `broker_log_setup` from `robottelo/logging.py` and `pytest_plugins/logging_hooks.py`.
- `broker` version 0.8 now manages its logging configuration internally or through different mechanisms, making the previous explicit setup calls redundant or incompatible.

Features:
- Integrated `broker.logging.RedactingFilter` to automatically redact sensitive information, such as passwords and tokens, from `broker` log output.
- This enhances log security by preventing the accidental leakage of credentials.
- The TRACE log level is now automatically registered upon importing `broker.logging`.
@JacobCallahan JacobCallahan self-assigned this Jan 7, 2026
Copilot AI review requested due to automatic review settings January 7, 2026 22:04
@JacobCallahan JacobCallahan requested a review from a team as a code owner January 7, 2026 22:04
@JacobCallahan JacobCallahan added dependencies Pull requests that update a dependency file CherryPick PR needs CherryPick to previous branches 6.15.z 6.16.z 6.17.z 6.18.z Introduced in or relating directly to Satellite 6.18 labels Jan 7, 2026
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Consider logging a clear warning or info message when broker.logging.RedactingFilter cannot be imported, so that loss of redaction behavior is visible rather than silently ignored.
  • The hard-coded sensitive list in robottelo/logging.py could be made configurable or centralized to avoid divergence if other components need to redact additional fields in the future.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider logging a clear warning or info message when `broker.logging.RedactingFilter` cannot be imported, so that loss of redaction behavior is visible rather than silently ignored.
- The hard-coded `sensitive` list in `robottelo/logging.py` could be made configurable or centralized to avoid divergence if other components need to redact additional fields in the future.

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.

@JacobCallahan
Copy link
Member Author

trigger: test-robottelo
pytest: tests/foreman/ -k 'test_host_registration_end_to_end or test_positive_erratum_applicability or test_positive_upload_content'

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR upgrades the broker dependency from version 0.7.3 to 0.8.0 and refactors the logging configuration to align with broker's new internal logging management approach while enhancing security through automatic credential redaction.

Key Changes:

  • Upgraded broker dependency to version 0.8.0, which now handles its own logging configuration internally
  • Integrated RedactingFilter from broker to automatically redact sensitive information (passwords, tokens) from broker logs
  • Removed redundant explicit broker_log_setup calls that are no longer compatible or necessary with broker 0.8

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
requirements.txt Updated broker dependency from 0.7.3 to 0.8.0 with required extras
robottelo/logging.py Removed broker_log_setup import and call; added conditional import of RedactingFilter and configured it for broker logger with sensitive field patterns
pytest_plugins/logging_hooks.py Removed imports of broker_log_setup and logging_yaml; removed broker logging setup in worker configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

file_level=logging_yaml.robottelo.fileLevel,
path=str(robottelo_log_file),
)

Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The RedactingFilter setup lacks documentation explaining its purpose and behavior. Consider adding a comment above this block explaining that the filter automatically redacts sensitive information (passwords, tokens) from broker log output to prevent credential leakage, which is a security enhancement introduced in broker 0.8.0.

Suggested change
# Configure the broker logger to automatically redact sensitive information
# (passwords, tokens, etc.) from its log output. This prevents credential
# leakage in logs and relies on the RedactingFilter security enhancement
# introduced in broker 0.8.0.

Copilot uses AI. Check for mistakes.
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 13964
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ -k test_host_registration_end_to_end or test_positive_erratum_applicability or test_positive_upload_content --external-logging
Test Result : ======== 17 passed, 5831 deselected, 134 warnings in 4968.55s (1:22:48) ========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.15.z 6.16.z 6.17.z 6.18.z Introduced in or relating directly to Satellite 6.18 CherryPick PR needs CherryPick to previous branches dependencies Pull requests that update a dependency file PRT-Passed Indicates that latest PRT run is passed for the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants