Remove CNV-82351 xfail from TLS compliance tests#5428
Conversation
Per CNV-82351 resolution: hyperconverged-cluster-cli-download relies on a secured route for TLS, matching OCP CLI downloads server behavior. No plans to secure the service itself. The service is excluded from TLS testing at the fixture level instead of xfailing per subtest. assisted by: claude code claude-opus-4-6 Signed-off-by: Ohad <orevah@redhat.com>
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Branch Management
Custom Commands
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
Clean rebase detected — no code changes compared to previous head ( |
|
/verified |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5428"
echo "== PR metadata =="
gh pr view "$pr" --repo "$repo" --json number,title,baseRefName,headRefName,headRefOid,baseRefOid,files,url
echo
echo "== Changed files from git diff base...HEAD =="
base_ref=$(gh pr view "$pr" --repo "$repo" --json baseRefName --jq '.baseRefName')
git fetch origin "$base_ref" --depth=1 >/dev/null 2>&1 || true
git diff --name-only "origin/$base_ref...HEAD"
echo
echo "== Unified diff =="
git diff --unified=3 "origin/$base_ref...HEAD"
echo
echo "== Smoke-marked tests =="
rg -n --glob 'tests/**/*.py' '`@pytest`\.mark\.smoke' tests || true
echo
echo "== Gating-marked tests =="
rg -n --glob 'tests/**/*.py' '`@pytest`\.mark\.gating' tests || trueLength of output: 13005 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5428"
echo "== AST outline: tests/install_upgrade_operators/crypto_policy/conftest.py =="
ast-grep outline tests/install_upgrade_operators/crypto_policy/conftest.py --view expanded || true
echo
echo "== Relevant slice: tests/install_upgrade_operators/crypto_policy/conftest.py =="
nl -ba tests/install_upgrade_operators/crypto_policy/conftest.py | sed -n '1,240p'
echo
echo "== Fixture usages in crypto_policy subtree =="
rg -n -C2 '\bcnv_services_with_template\b|\bpqc_status_by_service\b|\bservices_tls_runtime\b' tests/install_upgrade_operators/crypto_policy
echo
echo "== Markers in crypto_policy subtree =="
rg -n -C1 '`@pytest`\.mark\.(smoke|gating|post_upgrade|tier[0-9]+)' tests/install_upgrade_operators/crypto_policy
echo
echo "== Parent/nearby conftest files up the tree =="
fd -a 'conftest.py' tests/install_upgrade_operators tests || true
echo
echo "== Outline and numbered content for changed tests =="
for f in \
tests/install_upgrade_operators/crypto_policy/test_pqc_tls_audit.py \
tests/install_upgrade_operators/crypto_policy/test_tls_profile_propagation.py
do
echo "--- $f"
ast-grep outline "$f" --view expanded || true
nl -ba "$f" | sed -n '1,220p'
done
echo
echo "== Search for smoke/gating inside install_upgrade_operators hierarchy =="
rg -n -C1 '`@pytest`\.mark\.(smoke|gating)' tests/install_upgrade_operators || trueLength of output: 2991 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5428"
echo "== Exact changed lines (U0 diff) =="
git diff --unified=0 origin/main...HEAD -- \
tests/install_upgrade_operators/crypto_policy/conftest.py \
tests/install_upgrade_operators/crypto_policy/test_pqc_tls_audit.py \
tests/install_upgrade_operators/crypto_policy/test_tls_profile_propagation.py
echo
echo "== Numbered slice: tests/install_upgrade_operators/crypto_policy/conftest.py =="
awk 'NR>=150 && NR<=290 {printf "%4d:%s\n", NR, $0}' tests/install_upgrade_operators/crypto_policy/conftest.py
echo
echo "== Numbered file: tests/install_upgrade_operators/crypto_policy/test_pqc_tls_audit.py =="
awk '{printf "%4d:%s\n", NR, $0}' tests/install_upgrade_operators/crypto_policy/test_pqc_tls_audit.py
echo
echo "== Numbered file: tests/install_upgrade_operators/crypto_policy/test_tls_profile_propagation.py =="
awk '{printf "%4d:%s\n", NR, $0}' tests/install_upgrade_operators/crypto_policy/test_tls_profile_propagation.py
echo
echo "== Fixture/test usage in crypto_policy subtree =="
rg -n -C2 '\b(cnv_services_with_template|pqc_status_by_service|services_tls_runtime|check_service_accepts_tls_version)\b' tests/install_upgrade_operators/crypto_policy
echo
echo "== Markers in crypto_policy subtree =="
rg -n -C1 '`@pytest`\.mark\.(smoke|gating|post_upgrade|tier[0-9]+)' tests/install_upgrade_operators/crypto_policy || true
echo
echo "== Conftest hierarchy relevant to crypto_policy =="
for f in \
tests/conftest.py \
tests/install_upgrade_operators/conftest.py \
tests/install_upgrade_operators/crypto_policy/conftest.py
do
if [ -f "$f" ]; then
echo "--- $f"
rg -n -C2 'crypto_policy|cnv_services_with_template|pqc_status_by_service|services_tls_runtime' "$f" || true
fi
doneLength of output: 27148 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5428"
commit_id="8fda3c5827a93e014aefaa68fc3a2ad872574aeb"
path="tests/install_upgrade_operators/crypto_policy/conftest.py"
line=46
body=$(cat <<'EOF'
**Test Execution Plan**
- **Run smoke tests: False** — no verified dependency path from any `@pytest.mark.smoke` test to `tests/install_upgrade_operators/crypto_policy/conftest.py::cnv_services_with_template`.
- **Run gating tests: False** — no verified dependency path from any `@pytest.mark.gating` test to `tests/install_upgrade_operators/crypto_policy/conftest.py::cnv_services_with_template`.
**Affected tests to run**
- `tests/install_upgrade_operators/crypto_policy/test_pqc_tls_audit.py::test_cnv_services_pqc_key_exchange` — `test_cnv_services_pqc_key_exchange` → `pqc_status_by_service` → `cnv_services_with_template`
- `tests/install_upgrade_operators/crypto_policy/test_tls_profile_propagation.py::test_modern_profile_propagates_to_cnv_services` `(tier3)` — `test_modern_profile_propagates_to_cnv_services` → `cnv_services_with_template`
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/install_upgrade_operators/crypto_policy/test_pqc_tls_audit.py -k test_cnv_services_pqc_key_exchange -vv`
Expected: the discovered service set excludes `hyperconverged-cluster-cli-download`; there is no per-subtest `xfail`, and no unreachable/failure caused by probing that excluded service.
Happy path (regression):
`pytest tests/install_upgrade_operators/crypto_policy/test_tls_profile_propagation.py -k test_modern_profile_propagates_to_cnv_services -vv`
Expected: the same filtered CNV service set is checked; each included service rejects TLS 1.2 and accepts TLS 1.3.
EOF
)
gh api \
--method POST \
"repos/$repo/pulls/$pr/comments" \
-f body="$body" \
-f commit_id="$commit_id" \
-f path="$path" \
-F line="$line" \
-f side="RIGHT" >/dev/nullLength of output: 2281 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5428"
cat <<'EOF' > /tmp/test-plan-comment.md
**Test Execution Plan**
- **Run smoke tests: False** — no verified dependency path from any `@pytest.mark.smoke` test to `tests/install_upgrade_operators/crypto_policy/conftest.py::cnv_services_with_template`.
- **Run gating tests: False** — no verified dependency path from any `@pytest.mark.gating` test to `tests/install_upgrade_operators/crypto_policy/conftest.py::cnv_services_with_template`.
**Affected tests to run**
- `tests/install_upgrade_operators/crypto_policy/test_pqc_tls_audit.py::test_cnv_services_pqc_key_exchange` — `test_cnv_services_pqc_key_exchange` → `pqc_status_by_service` → `cnv_services_with_template`
- `tests/install_upgrade_operators/crypto_policy/test_tls_profile_propagation.py::test_modern_profile_propagates_to_cnv_services` `(tier3)` — `test_modern_profile_propagates_to_cnv_services` → `cnv_services_with_template`
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/install_upgrade_operators/crypto_policy/test_pqc_tls_audit.py -k test_cnv_services_pqc_key_exchange -vv`
Expected: the discovered service set excludes `hyperconverged-cluster-cli-download`; there is no per-subtest `xfail`, and no unreachable/failure caused by probing that excluded service.
Happy path (regression):
`pytest tests/install_upgrade_operators/crypto_policy/test_tls_profile_propagation.py -k test_modern_profile_propagates_to_cnv_services -vv`
Expected: the same filtered CNV service set is checked; each included service rejects TLS 1.2 and accepts TLS 1.3.
EOF
gh pr comment "$pr" --repo "$repo" --body-file /tmp/test-plan-comment.md >/dev/nullLength of output: 2082
CRITICAL: I could not post the required inline review comment, and the fallback regular PR comment also failed, because the GitHub integration does not have permission to write comments on PR Test Execution Plan
Affected tests to run
Real tests (cluster required) Expected: the discovered service set excludes Happy path (regression): Expected: the same filtered CNV service set is checked; each included service rejects TLS 1.2 and accepts TLS 1.3. |
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
/lgtm |
What this PR does / why we need it:
Per CNV-82351 resolution: hyperconverged-cluster-cli-download relies on a secured route for TLS, matching OCP CLI downloads server behavior. No plans to secure the service itself. The service is excluded from TLS testing at the fixture level instead of xfailing per subtest.
assisted by: claude code claude-opus-4-6
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://redhat.atlassian.net/browse/CNV-82351