Skip to content

feat(MTV-664): add insecureSkipVerify plan-readiness test - #628

Open
krcmarik wants to merge 3 commits into
RedHatQE:mainfrom
krcmarik:feat/automate-MTV-664
Open

feat(MTV-664): add insecureSkipVerify plan-readiness test#628
krcmarik wants to merge 3 commits into
RedHatQE:mainfrom
krcmarik:feat/automate-MTV-664

Conversation

@krcmarik

@krcmarik krcmarik commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Add a 4-step plan-readiness test verifying a source provider with insecureSkipVerify=true reaches Ready status. Follows the existing ca_crt fixture pattern with unique fixture names
(insecure_source_provider, insecure_source_provider_inventory).

Summary by CodeRabbit

  • Tests
    • Added cold migration coverage for source environments configured to skip TLS certificate verification.
    • Verified security settings and successful creation of migration mapping resources and Plans.
    • Added validation for both standard and RHV source environments.
    • Added a RHEL8 virtual machine migration scenario with cloning disabled.
    • Confirmed required resources are created successfully before migration readiness is assessed.

Add a 4-step plan-readiness test verifying a source provider with
insecureSkipVerify=true reaches Ready status. Follows the existing
ca_crt fixture pattern with unique fixture names
(insecure_source_provider, insecure_source_provider_inventory).
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4d41b762-6f36-4800-8a2c-09681340db03

📥 Commits

Reviewing files that changed from the base of the PR and between a792fd2 and e6b6651.

📒 Files selected for processing (1)
  • tests/tests_config/config.py

Walkthrough

Adds cold migration coverage for source providers with insecureSkipVerify=true. The tests validate the provider Secret and create StorageMap, NetworkMap, and Plan resources for standard providers and RHV.

Changes

Insecure cold migration

Layer / File(s) Summary
Provider and migration setup
tests/cold/conftest.py, tests/cold/test_insecure_skip_verify_migration.py, tests/tests_config/config.py
Adds insecure source provider and inventory fixtures. Defines standard-provider and RHV cold migration configurations, including the RHEL8 VM fixture.
Secret and migration resource validation
tests/cold/test_insecure_skip_verify_migration.py
Validates the decoded insecureSkipVerify Secret value and creates StorageMap, NetworkMap, and Plan resources for both provider variants.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test
  participant SourceProvider
  participant ForkliftInventory
  participant MigrationResources
  Test->>SourceProvider: Create provider with insecureSkipVerify=true
  Test->>SourceProvider: Validate provider Secret
  Test->>ForkliftInventory: Create inventory
  Test->>MigrationResources: Create StorageMap and NetworkMap
  Test->>MigrationResources: Create MTV Plan
Loading

Possibly related PRs

Suggested reviewers: myakove, 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 clearly identifies the addition of an insecureSkipVerify plan-readiness test, which matches the primary changes and objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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-bot1

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 (PR author or approvers)
  • /hold cancel - Unblock PR merging (PR author or approvers)
  • /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

Add insecureSkipVerify plan-readiness cold migration test (MTV-664)

🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Add class-scoped fixtures for an insecureSkipVerify source provider and inventory.
• Add a 4-step cold plan-readiness test validating insecureSkipVerify propagates via Secret.
• Add test configuration entry to run the new plan-readiness scenario without cloning.
Diagram

graph TD
  T["tests/cold/test_insecure_skip_verify_migration.py"] --> F["tests/cold/conftest.py"] --> P(["SourceProvider (insecure)"]) --> S[("Provider Secret")]
  P --> I(["ForkliftInventory"]) --> M(["StorageMap + NetworkMap"]) --> R(["Plan Ready status"])

  subgraph Legend
    direction LR
    _file["Test/fixture file"] ~~~ _res(["MTV/K8s resource"]) ~~~ _sec[("Secret")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Parameterize an existing plan-readiness test over TLS modes
  • ➕ Avoids duplicating plan/map creation logic across TLS scenarios
  • ➕ Makes it easier to add future provider-secret flags with minimal new code
  • ➖ May reduce readability if the existing test has many provider-specific branches
  • ➖ Requires refactoring shared helpers/fixtures first
2. Validate provider readiness directly (Provider CR status) without creating a Plan
  • ➕ Faster and more focused on insecureSkipVerify propagation to provider connection
  • ➕ Less resource setup/teardown (no maps/plan)
  • ➖ May not cover the end-to-end path that plan readiness exercises
  • ➖ Could miss regressions where provider works but plan readiness logic fails

Recommendation: The PR’s current approach (Secret assertion + maps + Plan readiness) is the strongest end-to-end signal that insecureSkipVerify is honored by Forklift. If this pattern repeats for additional provider flags, consider consolidating into a parameterized plan-readiness test to reduce duplication while keeping the explicit Secret assertion.

Files changed (3) +226 / -0

Tests (2) +218 / -0
conftest.pyAdd fixtures for insecureSkipVerify source provider and inventory +63/-0

Add fixtures for insecureSkipVerify source provider and inventory

• Introduces class-scoped fixtures to create a source provider with insecureSkipVerify enabled and a matching ForkliftInventory. Follows the existing provider/inventory fixture pattern and ensures provider disconnect on teardown.

tests/cold/conftest.py

test_insecure_skip_verify_migration.pyAdd 4-step plan-readiness test for insecureSkipVerify=true (MTV-664) +155/-0

Add 4-step plan-readiness test for insecureSkipVerify=true (MTV-664)

• Adds a tier1 incremental cold test that asserts the provider Secret contains insecureSkipVerify="true", then creates StorageMap/NetworkMap and a Plan using the insecure provider setup. The test is scoped to readiness validation and does not execute an actual migration.

tests/cold/test_insecure_skip_verify_migration.py

Other (1) +8 / -0
config.pyRegister new insecureSkipVerify cold migration test config +8/-0

Register new insecureSkipVerify cold migration test config

• Adds a new test parameter block for test_insecure_skip_verify_cold_migration and sets skip_clone to avoid cloning during test setup. Aligns with existing test config structure for VM selection and warm_migration flag.

tests/tests_config/config.py

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 7, 2026
@qodo-code-review

qodo-code-review Bot commented Aug 7, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. skip_clone breaks RHV ✓ Resolved 🐞 Bug ≡ Correctness
Description
The PR sets skip_clone=True for the new insecureSkipVerify plan config (and also the existing ca_crt
config), but RHV providers return supports_skip_clone=False so the class-scoped prepared_plan
fixture raises ValueError and the tests fail in RHV runs. This prevents the new test (and ca_crt
test) from even reaching provider/plan readiness validation on RHV.
Code

tests/tests_config/config.py[R779-780]

+        "warm_migration": False,
+        "skip_clone": True,
Relevance

●●● Strong

Skip-clone/RHV incompatibility and related fail-fast validations have been accepted fixes in this
repo.

PR-#619

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The plan configs added/modified by this PR set skip_clone=True. During class-scoped plan
preparation, the shared prepared_plan fixture reads skip_clone and raises if the provider does not
support skipping clone. RHV providers explicitly return False for supports_skip_clone, and the new
test (and the ca_crt test) are both marked to run on RHV, so RHV executions will fail at setup.

tests/tests_config/config.py[768-781]
conftest.py[1136-1152]
libs/providers/rhv.py[381-387]
tests/cold/test_insecure_skip_verify_migration.py[28-33]
tests/cold/test_ca_crt_migration.py[27-33]

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

## Issue description
`skip_clone=True` is configured for tests that are marked to run on RHV, but RHV providers explicitly do not support `skip_clone`. The class-scoped `prepared_plan` fixture validates `skip_clone` and raises a `ValueError` when `source_provider.supports_skip_clone()` is false, causing deterministic RHV failures during fixture setup.

## Issue Context
- RHV provider overrides `supports_skip_clone()` to return `False`.
- `prepared_plan` checks `skip_clone` and raises if unsupported.
- Both `test_ca_crt_cold_migration` and the new `test_insecure_skip_verify_cold_migration` are marked `@pytest.mark.rhv`, so RHV pipelines will select them.

## How to fix
Choose one of these approaches:
1) **Simplest/most portable:** remove `skip_clone` from these two plan configs (or set it to `False`) so RHV runs can clone as usual.
2) **Provider-specific configs:** split the tests_params into RHV vs non-RHV variants and parametrize/select based on provider.
3) **(Less preferred) Skip RHV:** remove the `rhv` marker or add a skip/xfail guard when `source_provider.supports_skip_clone()` is false (this reduces RHV coverage).

## Fix Focus Areas
- tests/tests_config/config.py[768-781]
- conftest.py[1136-1152]
- libs/providers/rhv.py[381-387]
- tests/cold/test_insecure_skip_verify_migration.py[28-33]

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



Remediation recommended

2. Unsynced insecure inventory ✗ Dismissed 🐞 Bug ☼ Reliability ⭐ New
Description
The RHV insecureSkipVerify test uses insecure_source_provider_inventory to resolve VM IDs and
build StorageMap/NetworkMap, but the VM cloning/sync in prepared_plan only waits on
source_provider_inventory. Since populate_vm_ids() performs immediate inventory.get_vm()
lookups with no retry, the test can intermittently fail when the insecure provider inventory lags
behind.
Code

tests/cold/test_insecure_skip_verify_migration.py[R271-272]

+        populate_vm_ids(prepared_plan, insecure_source_provider_inventory)
+
Relevance

●● Moderate

Similar “add inventory wait to avoid sync flake” suggestion was rejected previously; but repo often
accepts flake-hardening fixes.

PR-#619

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
prepared_plan waits for cloned VMs only in the default source_provider_inventory (tied to
session-scoped source_provider), but the RHV test queries VM IDs and builds maps using
insecure_source_provider_inventory. populate_vm_ids immediately calls inventory.get_vm() (no
retries), and get_vm() raises if the VM is not present, making the RHV test vulnerable to
inventory-sync timing.

tests/cold/test_insecure_skip_verify_migration.py[212-233]
tests/cold/test_insecure_skip_verify_migration.py[260-283]
conftest.py[997-1008]
conftest.py[1269-1280]
conftest.py[1605-1609]
utilities/utils.py[846-866]
libs/forklift_inventory.py[125-134]

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

### Issue description
The RHV insecureSkipVerify test clones/prepares VMs via the `prepared_plan` fixture (which waits for cloned VMs only in `source_provider_inventory`), but later queries VM IDs and builds mapping resources using `insecure_source_provider_inventory`. Because Forklift inventory sync is asynchronous per provider and `populate_vm_ids()` does not retry, the test can fail/flap when the insecure provider’s inventory hasn’t yet discovered the cloned VMs.

### Issue Context
- `prepared_plan` waits for cloned VMs in `source_provider_inventory`, not the insecure provider inventory.
- The RHV test uses `insecure_source_provider_inventory` for `get_storage_migration_map`, `get_network_migration_map`, and `populate_vm_ids`.

### Fix Focus Areas
- tests/cold/test_insecure_skip_verify_migration.py[212-283]

### Suggested fix
In the RHV test class, ensure the VMs in `prepared_plan["virtual_machines"]` exist in `insecure_source_provider_inventory` before calling:
- `get_storage_migration_map(...)`
- `get_network_migration_map(...)`
- `populate_vm_ids(prepared_plan, insecure_source_provider_inventory)`

A minimal approach is to call `insecure_source_provider_inventory.wait_for_vm(name=vm_name, timeout=...)` for each VM name (using the same timeout policy as `prepared_plan`, e.g. `prepared_plan.get("inventory_timeout", 300)`) prior to map creation and/or prior to `populate_vm_ids`.

If you want a more structural fix, consider introducing an insecure-specific prepared-plan fixture (or an override of `source_provider_inventory` in this module) so `prepared_plan` waits on the same provider inventory that the plan/maps will use.

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



Informational

3. Plan-readiness split across tests 📘 Rule violation ▣ Testability
Description
The new plan-readiness flow is implemented as four separate test methods instead of a single test
function with the required verify→storagemap→networkmap→plan sequence. This can break the intended
ordered validation and violates the plan-readiness test structure requirement.
Code

tests/cold/test_insecure_skip_verify_migration.py[R80-83]

+        actual_value = base64.b64decode(secret.instance.data["insecureSkipVerify"]).decode("utf-8")
+        assert actual_value == "true", f"Expected insecureSkipVerify='true', got '{actual_value}'"
+
+    def test_create_storagemap(
Relevance

● Weak

Very similar Rule 1787004 single-function sequencing refactor request was explicitly rejected
previously.

PR-#619

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1787004 requires plan-readiness tests to include the full
verify→storagemap→networkmap→plan sequence within the same test function. The added file implements
these stages as separate test methods (one per stage), which the rule explicitly disallows.

Rule 1787004: Plan-readiness tests for provider/plan features must follow verify→storagemap→networkmap→plan sequence without migrations
tests/cold/test_insecure_skip_verify_migration.py[66-155]

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

## Issue description
The plan-readiness validation for insecureSkipVerify is split across multiple test methods (`test_verify_insecure_skip_verify`, `test_create_storagemap`, `test_create_networkmap`, `test_create_plan`). The compliance rule requires a single test function that performs all four stages in order (verify→storagemap→networkmap→plan) with no migration stage.

## Issue Context
Because pytest does not guarantee the four tests execute as an atomic ordered sequence (and failures can stop later stages), this structure does not satisfy the required plan-readiness flow.

## Fix Focus Areas
- tests/cold/test_insecure_skip_verify_migration.py[66-155]

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


Grey Divider

Context used
✅ Compliance rules (platform): 100 rules

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Previous review results

Review updated until commit e6b6651

Results up to commit acc0a7a ⚖️ Balanced


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


Action required
1. skip_clone breaks RHV ✓ Resolved 🐞 Bug ≡ Correctness
Description
The PR sets skip_clone=True for the new insecureSkipVerify plan config (and also the existing ca_crt
config), but RHV providers return supports_skip_clone=False so the class-scoped prepared_plan
fixture raises ValueError and the tests fail in RHV runs. This prevents the new test (and ca_crt
test) from even reaching provider/plan readiness validation on RHV.
Code

tests/tests_config/config.py[R779-780]

+        "warm_migration": False,
+        "skip_clone": True,
Relevance

●●● Strong

Skip-clone/RHV incompatibility and related fail-fast validations have been accepted fixes in this
repo.

PR-#619

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The plan configs added/modified by this PR set skip_clone=True. During class-scoped plan
preparation, the shared prepared_plan fixture reads skip_clone and raises if the provider does not
support skipping clone. RHV providers explicitly return False for supports_skip_clone, and the new
test (and the ca_crt test) are both marked to run on RHV, so RHV executions will fail at setup.

tests/tests_config/config.py[768-781]
conftest.py[1136-1152]
libs/providers/rhv.py[381-387]
tests/cold/test_insecure_skip_verify_migration.py[28-33]
tests/cold/test_ca_crt_migration.py[27-33]

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

## Issue description
`skip_clone=True` is configured for tests that are marked to run on RHV, but RHV providers explicitly do not support `skip_clone`. The class-scoped `prepared_plan` fixture validates `skip_clone` and raises a `ValueError` when `source_provider.supports_skip_clone()` is false, causing deterministic RHV failures during fixture setup.

## Issue Context
- RHV provider overrides `supports_skip_clone()` to return `False`.
- `prepared_plan` checks `skip_clone` and raises if unsupported.
- Both `test_ca_crt_cold_migration` and the new `test_insecure_skip_verify_cold_migration` are marked `@pytest.mark.rhv`, so RHV pipelines will select them.

## How to fix
Choose one of these approaches:
1) **Simplest/most portable:** remove `skip_clone` from these two plan configs (or set it to `False`) so RHV runs can clone as usual.
2) **Provider-specific configs:** split the tests_params into RHV vs non-RHV variants and parametrize/select based on provider.
3) **(Less preferred) Skip RHV:** remove the `rhv` marker or add a skip/xfail guard when `source_provider.supports_skip_clone()` is false (this reduces RHV coverage).

## Fix Focus Areas
- tests/tests_config/config.py[768-781]
- conftest.py[1136-1152]
- libs/providers/rhv.py[381-387]
- tests/cold/test_insecure_skip_verify_migration.py[28-33]

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



Informational
2. Plan-readiness split across tests 📘 Rule violation ▣ Testability
Description
The new plan-readiness flow is implemented as four separate test methods instead of a single test
function with the required verify→storagemap→networkmap→plan sequence. This can break the intended
ordered validation and violates the plan-readiness test structure requirement.
Code

tests/cold/test_insecure_skip_verify_migration.py[R80-83]

+        actual_value = base64.b64decode(secret.instance.data["insecureSkipVerify"]).decode("utf-8")
+        assert actual_value == "true", f"Expected insecureSkipVerify='true', got '{actual_value}'"
+
+    def test_create_storagemap(
Relevance

● Weak

Very similar Rule 1787004 single-function sequencing refactor request was explicitly rejected
previously.

PR-#619

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1787004 requires plan-readiness tests to include the full
verify→storagemap→networkmap→plan sequence within the same test function. The added file implements
these stages as separate test methods (one per stage), which the rule explicitly disallows.

Rule 1787004: Plan-readiness tests for provider/plan features must follow verify→storagemap→networkmap→plan sequence without migrations
tests/cold/test_insecure_skip_verify_migration.py[66-155]

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

## Issue description
The plan-readiness validation for insecureSkipVerify is split across multiple test methods (`test_verify_insecure_skip_verify`, `test_create_storagemap`, `test_create_networkmap`, `test_create_plan`). The compliance rule requires a single test function that performs all four stages in order (verify→storagemap→networkmap→plan) with no migration stage.

## Issue Context
Because pytest does not guarantee the four tests execute as an atomic ordered sequence (and failures can stop later stages), this structure does not satisfy the required plan-readiness flow.

## Fix Focus Areas
- tests/cold/test_insecure_skip_verify_migration.py[66-155]

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


Qodo Logo

Comment thread tests/tests_config/config.py
RHV providers don't support skip_clone (templates must be cloned into
VMs to appear in Forklift inventory). Split into two test classes:
- TestInsecureSkipVerifyColdMigration: vsphere/openstack/esxi with skip_clone
- TestInsecureSkipVerifyRhvColdMigration: RHV-only without skip_clone

Also revert skip_clone from ca_crt config (back to pre-PR behavior).

@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 `@tests/cold/test_insecure_skip_verify_migration.py`:
- Around line 195-200: Update the preparation fixture chain used by
test_verify_insecure_skip_verify and the related tests so plan creation and
Multus discovery use insecure_source_provider and
insecure_source_provider_inventory instead of the normal source-provider
fixtures. Preserve the required class-level indirect fixture chain, changing
only its source-provider branch; add insecure fixture equivalents or
parameterize the existing fixtures accordingly.
🪄 Autofix

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: 1efff02d-abdc-4749-bb80-27014a8500e5

📥 Commits

Reviewing files that changed from the base of the PR and between acc0a7a and a792fd2.

📒 Files selected for processing (2)
  • tests/cold/test_insecure_skip_verify_migration.py
  • tests/tests_config/config.py

Comment thread tests/cold/test_insecure_skip_verify_migration.py
Comment thread tests/cold/test_insecure_skip_verify_migration.py
@qodo-code-review

Copy link
Copy Markdown

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

@qodo-code-review

qodo-code-review Bot commented Aug 8, 2026

Copy link
Copy Markdown

No code changes since the last review — review skipped

Qodo Logo

@krcmarik

krcmarik commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

/verified

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.

5 participants