Skip to content

fix: pass explicit image to Hook to avoid deprecation warning - #595

Merged
myakove merged 2 commits into
RedHatQE:mainfrom
AmenB:fix/hook-explicit-image
Jul 26, 2026
Merged

myakove merged 2 commits into
RedHatQE:mainfrom
AmenB:fix/hook-explicit-image

Conversation

@AmenB

@AmenB AmenB commented Jul 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Adds image="quay.io/konveyor/hook-runner:latest" to the playbook-based Hook creation in utilities/hooks.py
  • Prevents DeprecationWarning from openshift-python-wrapper after PR #2759 changed Hook.image from a hardcoded default to a sentinel that warns when not passed explicitly
  • AAP hook calls (passing aap_job_template_id) are unaffected and don't need image

Why

The next openshift-python-wrapper release will remove the default entirely, breaking any Hook() call that doesn't pass image. This change avoids the deprecation warning now and prevents future breakage.

Test plan

  • Verify playbook-based hook tests (test_post_hook_retain_failed_vm) still pass
  • Confirm no DeprecationWarning in test output for Hook creation

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Hook resources now explicitly use the latest hook runner container image, improving consistency when executing hooks.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

create_hook_for_plan now supplies quay.io/konveyor/hook-runner:latest when creating the Hook custom resource.

Changes

Hook runner image configuration

Layer / File(s) Summary
Hook image assignment
utilities/hooks.py
The Hook creation call now includes the explicit image value alongside the existing playbook value.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: myakove, krcmarik, solenoci

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: explicitly passing the Hook image to avoid the deprecation warning.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@redhat-qe-bot

Copy link
Copy Markdown

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: Disabled for this repository
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
  • /regenerate-welcome - Regenerate this welcome message
  • /security-override - Set security check runs to pass (maintainers only)
  • /security-override cancel - Re-run security checks

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest build-container - Rebuild and test container image
  • /retest conventional-title - Validate commit message format
  • /retest all - Run all available tests

Container Operations

  • /build-and-push-container - Build and push container image (tagged with PR number)
    • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3
  • /cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

  • /rebase - Rebase this PR branch onto its base branch

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. Status Checks: All required status checks must pass
  3. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
  4. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

  • krcmarik
  • myakove
  • solenoci

Reviewers:

  • krcmarik
  • myakove
  • solenoci
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge
AI Features
  • Conventional Title: Mode: fix (claude/claude-opus-4-6-1m)
  • Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6-1m)
  • Test Oracle: Triggers: approved (cursor/gpt-5.4-xhigh-fast); /test-oracle can be used anytime
Security Checks
  • Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
  • Committer Identity Check: Verifies last committer matches PR author
  • Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Container Builds: Container images are automatically tagged with the PR number
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix Hook creation by passing explicit hook-runner image

🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Pass an explicit Hook runner image to suppress openshift-python-wrapper deprecation warnings
• Future-proof playbook-based Hook creation against upcoming removal of default image behavior
Diagram

graph TD
  A["Plan config"] --> B["utilities/hooks.py"] --> C["create_and_store_resource()"] --> D["Hook CR"]
  D --> E["quay.io/konveyor/hook-runner:latest"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize the image as a module-level constant
  • ➕ Avoids repeating the image string if more Hook call sites are added
  • ➕ Makes future image changes a one-line edit
  • ➖ Slightly more indirection for a single current call site
2. Make the image configurable (env var / config file / plan config)
  • ➕ Supports different registries/tags across environments without code changes
  • ➕ Unblocks air-gapped or mirrored-registry setups
  • ➖ Adds configuration surface area and validation requirements
  • ➖ May be unnecessary if image is intended to be fixed

Recommendation: The PR’s approach (passing the explicit image at the call site) is the right minimal fix to remove the deprecation warning and prevent future breakage. If additional playbook-based Hook creation sites appear, consider centralizing the image value as a constant; only add configurability if environment-specific image overrides are a real requirement.

Files changed (1) +1 / -0

Bug fix (1) +1 / -0
hooks.pyPass explicit hook-runner image to Hook creation +1/-0

Pass explicit hook-runner image to Hook creation

• Adds an explicit container image argument when creating playbook-based Hook CRs. This avoids DeprecationWarning behavior introduced in openshift-python-wrapper and prevents future failures when the implicit default is removed.

utilities/hooks.py

@qodo-code-review

qodo-code-review Bot commented Jul 12, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 100 rules

Grey Divider


Remediation recommended

1. Mutable hook image tag ✗ Dismissed 🐞 Bug ☼ Reliability
Description
create_hook_for_plan() now hardcodes image="quay.io/konveyor/hook-runner:latest", making hook
execution depend on a mutable tag and potentially drift across test runs. This also bypasses the
repo’s existing pattern of resolving images via installed CSV + digest mirrors, increasing the
chance of image-pull failures in mirrored/disconnected setups.
Code

utilities/hooks.py[186]

+        image="quay.io/konveyor/hook-runner:latest",
Relevance

⭐⭐⭐ High

Team accepted avoiding mutable image tags and hardcoded image fallbacks for mirrored env reliability
(PRs #371, #305).

PR-#371
PR-#305

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR introduces a hardcoded :latest hook-runner image in Hook creation. Elsewhere, the repo
explicitly documents and implements resolving must-gather images from the installed operator CSV and
digest mirror sets to keep images version-aligned and mirror-friendly, highlighting the
inconsistency and reliability risk of a fixed mutable tag for hooks.

utilities/hooks.py[179-187]
utilities/must_gather.py[101-139]
docs/python-module-reference.md[360-365]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`utilities/hooks.py:create_hook_for_plan()` hardcodes the hook runner image to `quay.io/konveyor/hook-runner:latest`. Using a mutable `:latest` tag makes test behavior non-reproducible and can break in environments that depend on digest-based mirroring or require images to be aligned with the installed MTV operator version.

### Issue Context
The repo already avoids hardcoding critical operational images in other places (e.g., must-gather) by resolving an image reference from the installed operator CSV and building a digest-based reference suitable for mirroring.

### Fix Focus Areas
- utilities/hooks.py[134-189]
- utilities/must_gather.py[101-139]
- docs/python-module-reference.md[360-365]

### Suggested fix
1. Replace the hardcoded `:latest` reference with a configurable value (e.g., `py_config.get("hook_runner_image")` and/or a per-plan override like `hook_config.get("image")`).
2. Default to a pinned, stable reference (version tag or digest). If feasible in this codebase, mirror the `must_gather` approach: resolve the hook-runner image from the installed MTV CSV and/or a mirror configuration so it stays aligned with the cluster under test.
3. Update documentation/config defaults to describe the new setting and its expected format (prefer digest when running in mirrored/disconnected environments).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread utilities/hooks.py Resolved
@myakove

myakove commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

/lgtm
/approve

@redhat-qe-bot2

Copy link
Copy Markdown

Test Oracle server is not responding, skipping test analysis

auto-merge was automatically disabled July 23, 2026 09:32

Head branch was pushed to by a user without write access

@AmenB
AmenB force-pushed the fix/hook-explicit-image branch from 70394df to 9f0becf Compare July 23, 2026 09:32
@redhat-qe-bot

Copy link
Copy Markdown

Clean rebase detected — no code changes compared to previous head (70394df).
The following labels were preserved: approved-myakove, commented-AmenB, lgtm-myakove, lgtm-coderabbitai[bot], commented-qodo-code-review[bot].

1 similar comment
@redhat-qe-bot

Copy link
Copy Markdown

Clean rebase detected — no code changes compared to previous head (70394df).
The following labels were preserved: approved-myakove, commented-AmenB, lgtm-myakove, lgtm-coderabbitai[bot], commented-qodo-code-review[bot].

@qodo-code-review

qodo-code-review Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 100 rules

Grey Divider


Remediation recommended

1. Mutable hook image tag ✗ Dismissed 🐞 Bug ☼ Reliability
Description
create_hook_for_plan() now hardcodes image="quay.io/konveyor/hook-runner:latest", making hook
execution depend on a mutable tag and potentially drift across test runs. This also bypasses the
repo’s existing pattern of resolving images via installed CSV + digest mirrors, increasing the
chance of image-pull failures in mirrored/disconnected setups.
Code

utilities/hooks.py[186]

+        image="quay.io/konveyor/hook-runner:latest",
Relevance

⭐⭐⭐ High

Team accepted avoiding mutable image tags and hardcoded image fallbacks for mirrored env reliability
(PRs #371, #305).

PR-#371
PR-#305

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR introduces a hardcoded :latest hook-runner image in Hook creation. Elsewhere, the repo
explicitly documents and implements resolving must-gather images from the installed operator CSV and
digest mirror sets to keep images version-aligned and mirror-friendly, highlighting the
inconsistency and reliability risk of a fixed mutable tag for hooks.

utilities/hooks.py[179-187]
utilities/must_gather.py[101-139]
docs/python-module-reference.md[360-365]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`utilities/hooks.py:create_hook_for_plan()` hardcodes the hook runner image to `quay.io/konveyor/hook-runner:latest`. Using a mutable `:latest` tag makes test behavior non-reproducible and can break in environments that depend on digest-based mirroring or require images to be aligned with the installed MTV operator version.

### Issue Context
The repo already avoids hardcoding critical operational images in other places (e.g., must-gather) by resolving an image reference from the installed operator CSV and building a digest-based reference suitable for mirroring.

### Fix Focus Areas
- utilities/hooks.py[134-189]
- utilities/must_gather.py[101-139]
- docs/python-module-reference.md[360-365]

### Suggested fix
1. Replace the hardcoded `:latest` reference with a configurable value (e.g., `py_config.get("hook_runner_image")` and/or a per-plan override like `hook_config.get("image")`).
2. Default to a pinned, stable reference (version tag or digest). If feasible in this codebase, mirror the `must_gather` approach: resolve the hook-runner image from the installed MTV CSV and/or a mirror configuration so it stays aligned with the cluster under test.
3. Update documentation/config defaults to describe the new setting and its expected format (prefer digest when running in mirrored/disconnected environments).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit a555365 ⚖️ Balanced

Results up to commit 70394df


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Mutable hook image tag ✗ Dismissed 🐞 Bug ☼ Reliability
Description
create_hook_for_plan() now hardcodes image="quay.io/konveyor/hook-runner:latest", making hook
execution depend on a mutable tag and potentially drift across test runs. This also bypasses the
repo’s existing pattern of resolving images via installed CSV + digest mirrors, increasing the
chance of image-pull failures in mirrored/disconnected setups.
Code

utilities/hooks.py[186]

+        image="quay.io/konveyor/hook-runner:latest",
Relevance

⭐⭐⭐ High

Team accepted avoiding mutable image tags and hardcoded image fallbacks for mirrored env reliability
(PRs #371, #305).

PR-#371
PR-#305

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR introduces a hardcoded :latest hook-runner image in Hook creation. Elsewhere, the repo
explicitly documents and implements resolving must-gather images from the installed operator CSV and
digest mirror sets to keep images version-aligned and mirror-friendly, highlighting the
inconsistency and reliability risk of a fixed mutable tag for hooks.

utilities/hooks.py[179-187]
utilities/must_gather.py[101-139]
docs/python-module-reference.md[360-365]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`utilities/hooks.py:create_hook_for_plan()` hardcodes the hook runner image to `quay.io/konveyor/hook-runner:latest`. Using a mutable `:latest` tag makes test behavior non-reproducible and can break in environments that depend on digest-based mirroring or require images to be aligned with the installed MTV operator version.

### Issue Context
The repo already avoids hardcoding critical operational images in other places (e.g., must-gather) by resolving an image reference from the installed operator CSV and building a digest-based reference suitable for mirroring.

### Fix Focus Areas
- utilities/hooks.py[134-189]
- utilities/must_gather.py[101-139]
- docs/python-module-reference.md[360-365]

### Suggested fix
1. Replace the hardcoded `:latest` reference with a configurable value (e.g., `py_config.get("hook_runner_image")` and/or a per-plan override like `hook_config.get("image")`).
2. Default to a pinned, stable reference (version tag or digest). If feasible in this codebase, mirror the `must_gather` approach: resolve the hook-runner image from the installed MTV CSV and/or a mirror configuration so it stays aligned with the cluster under test.
3. Update documentation/config defaults to describe the new setting and its expected format (prefer digest when running in mirrored/disconnected environments).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 9f0becf ⚖️ Balanced


No changes from previous review

Results up to commit 9bb6822 ⚖️ Balanced


No changes from previous review

Qodo Logo

@myakove

myakove commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

/lgtm
/approve

@myakove
myakove enabled auto-merge (squash) July 23, 2026 09:35
@AmenB

AmenB commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

/verified

openshift-python-wrapper PR #2759 changed Hook so that image no longer
has a hardcoded default — callers that omit it get a DeprecationWarning
now and will break when the default is removed entirely.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
auto-merge was automatically disabled July 23, 2026 09:56

Head branch was pushed to by a user without write access

@AmenB
AmenB force-pushed the fix/hook-explicit-image branch from 9f0becf to 9bb6822 Compare July 23, 2026 09:56
@AmenB

AmenB commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

/verified

@rh-bot-1

Copy link
Copy Markdown

Clean rebase detected — no code changes compared to previous head (9f0becf).
The following labels were preserved: verified, approved-myakove, commented-AmenB, lgtm-myakove, lgtm-coderabbitai[bot], commented-qodo-code-review[bot].

@redhat-qe-bot

Copy link
Copy Markdown

Clean rebase detected — no code changes compared to previous head (9f0becf).
The following labels were preserved: verified, approved-myakove, commented-AmenB, lgtm-myakove, lgtm-coderabbitai[bot], commented-qodo-code-review[bot].

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 9bb6822

@redhat-qe-bot

Copy link
Copy Markdown

Clean rebase detected — no code changes compared to previous head (9bb6822).
The following labels were preserved: verified, approved-myakove, commented-AmenB, lgtm-myakove, lgtm-coderabbitai[bot], commented-qodo-code-review[bot].

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit a555365

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@utilities/hooks.py`:
- Line 186: Replace the mutable hook-runner image value in the hook
configuration with the repository-approved versioned tag or immutable digest,
and define that value as a named constant near the other hook configuration
constants. Add a brief explanatory comment and reference the constant from the
image field instead of embedding the literal.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a3a98a36-b01e-40ed-8c29-8dfc685de50c

📥 Commits

Reviewing files that changed from the base of the PR and between 9f0becf and a555365.

📒 Files selected for processing (1)
  • utilities/hooks.py

Comment thread utilities/hooks.py Resolved
@myakove
myakove merged commit e8f82cf into RedHatQE:main Jul 26, 2026
9 checks passed
@rh-bot-1

Copy link
Copy Markdown

New container for ghcr.io/redhatqe/mtv-api-tests:latest published

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants