Skip to content

Conversation

pondrejk
Copy link
Contributor

Problem Statement

downstream registry no longer picks latest when no tag is specified

Solution

specifying tags explicitely

Related Issues

@pondrejk pondrejk self-assigned this Oct 20, 2025
@pondrejk pondrejk requested a review from a team as a code owner October 20, 2025 13:14
@pondrejk pondrejk added CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.18.z Introduced in or relating directly to Satellite 6.18 labels Oct 20, 2025
@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/sys/test_mcp.py -k test_positive_call_mcp_server

Copy link

@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 there - I've reviewed your changes - here's some feedback:

Blocking issues:

  • Detected possible formatted SQL query. Use parameterized queries instead. (link)
  • Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option. (link)

General comments:

  • Avoid hardcoding the tag value ("6.18") in the function—make the version tag configurable via settings or an environment variable so it stays in sync with future image updates.
  • The run_cmd uses image_name, but it isn’t explicitly tagged—ensure image_name includes the same tag you pull (or append :{tag}) to prevent pulling one version and running another.
  • Re-adding a .strip() (or otherwise normalizing whitespace) on the multiline run_cmd string can help avoid unintended spaces or newlines when executing the command.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Avoid hardcoding the tag value ("6.18") in the function—make the version tag configurable via settings or an environment variable so it stays in sync with future image updates.
- The run_cmd uses `image_name`, but it isn’t explicitly tagged—ensure `image_name` includes the same tag you pull (or append `:{tag}`) to prevent pulling one version and running another.
- Re-adding a `.strip()` (or otherwise normalizing whitespace) on the multiline `run_cmd` string can help avoid unintended spaces or newlines when executing the command.

## Individual Comments

### Comment 1
<location> `pytest_fixtures/component/mcp.py:79` </location>
<code_context>
    assert target_sat.execute(pull_cmd).status == 0
</code_context>

<issue_to_address>
**security (python.lang.security.audit.formatted-sql-query):** Detected possible formatted SQL query. Use parameterized queries instead.

*Source: opengrep*
</issue_to_address>

### Comment 2
<location> `pytest_fixtures/component/mcp.py:79` </location>
<code_context>
    assert target_sat.execute(pull_cmd).status == 0
</code_context>

<issue_to_address>
**security (python.sqlalchemy.security.sqlalchemy-execute-raw-query):** Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.

*Source: opengrep*
</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.

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 13235
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/sys/test_mcp.py -k test_positive_call_mcp_server --external-logging
Test Result : ====== 1 failed, 1 passed, 5 deselected, 25 warnings in 774.49s (0:12:54) ======

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Oct 20, 2025
Copy link

@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.

New security issues found

@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/sys/test_mcp.py -k test_positive_call_mcp_server

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 13246
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/sys/test_mcp.py -k test_positive_call_mcp_server --external-logging
Test Result : =========== 2 passed, 5 deselected, 38 warnings in 711.84s (0:11:51) ===========

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Oct 21, 2025
@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Oct 21, 2025
@pondrejk
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/sys/test_mcp.py -k test_positive_call_mcp_server

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 13247
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/sys/test_mcp.py -k test_positive_call_mcp_server --external-logging
Test Result : =========== 2 passed, 5 deselected, 39 warnings in 863.91s (0:14:23) ===========

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

Labels

6.18.z Introduced in or relating directly to Satellite 6.18 AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing CherryPick PR needs CherryPick to previous branches 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