OAPE-494: Add RHCOS 10/RHEL 10 compatibility testing infrastructure#376
OAPE-494: Add RHCOS 10/RHEL 10 compatibility testing infrastructure#376chiragkyal wants to merge 4 commits intoopenshift:masterfrom
Conversation
Add comprehensive testing guide for verifying cert-manager operator compatibility with RHCOS 10 and RHEL 10 on OpenShift 4.20, 4.21, and 4.22. The testing guide provides step-by-step procedures for: - Deployment verification - E2E test execution - Crypto library compatibility verification - Cloud provider integration testing - Troubleshooting and debug information collection The results template provides a standardized format for documenting test outcomes, including cluster information, test results, bugs found, and recommendations. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add automated testing scripts for RHCOS 10 compatibility verification: verify-rhcos10-crypto.sh: - Verifies RHCOS/RHEL version on cluster nodes - Checks OpenSSL versions on nodes and in containers - Verifies FIPS mode status - Tests TLS connectivity from cert-manager pods - Tests certificate generation with various algorithms (RSA, ECDSA) - Checks for crypto-related errors in logs - Generates detailed verification report test-rhcos10-compatibility.sh: - Orchestrates comprehensive compatibility testing - Collects cluster information - Verifies deployment status - Runs E2E test suite - Executes crypto verification - Collects diagnostic information - Generates detailed test report Both scripts support skip flags for selective test execution and provide detailed output for troubleshooting. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add Makefile targets for RHCOS 10 compatibility testing: - test-rhcos10: Run comprehensive RHCOS 10 compatibility test suite - verify-rhcos10-crypto: Verify RHCOS 10 crypto library compatibility - report-rhcos10: Generate RHCOS 10 test report from results These targets provide convenient access to the RHCOS 10 testing automation scripts and follow existing project conventions for test execution. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@chiragkyal: This pull request references OAPE-494 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThe PR adds RHCOS 10 compatibility testing infrastructure for cert-manager, including new Makefile targets, comprehensive documentation guides and templates, and two orchestration scripts that automate testing workflows and report generation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TestScript as test-rhcos10-compatibility.sh
participant Cluster as OpenShift Cluster
participant CertMgr as cert-manager
participant CryptoScript as verify-rhcos10-crypto.sh
participant Reporter as Report Generator
User->>TestScript: make test-rhcos10
TestScript->>TestScript: Parse options & validate prerequisites
TestScript->>Cluster: Collect cluster info (version, platform, RHCOS)
Cluster-->>TestScript: Return cluster metadata
TestScript->>CertMgr: Verify deployment (namespaces, operators, pods)
CertMgr-->>TestScript: Return status
alt E2E tests not skipped
TestScript->>TestScript: Run E2E tests (make test-e2e)
TestScript-->>TestScript: Capture E2E results
end
alt Crypto verification not skipped
TestScript->>CryptoScript: Execute verify-rhcos10-crypto.sh
CryptoScript->>Cluster: Check RHCOS/OpenSSL versions
CryptoScript->>CertMgr: Inspect pod crypto configuration
CryptoScript->>CertMgr: Test TLS connectivity & ciphers
CryptoScript->>CertMgr: Scan logs for crypto errors
CryptoScript->>Cluster: Create test certificates (RSA, ECDSA)
CryptoScript-->>TestScript: Return crypto verification results
end
TestScript->>Cluster: Collect diagnostics (pods, logs, events)
TestScript->>Reporter: Generate markdown report
Reporter-->>TestScript: Return formatted report
TestScript-->>User: Output report & diagnostics location
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
@chiragkyal: This pull request references OAPE-494 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: chiragkyal The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@chiragkyal: This pull request references OAPE-494 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/rhcos10-test-results-template.md`:
- Around line 71-73: Replace plain-text fenced code blocks with explicit "text"
language identifiers: change fences like ``` to ```text for the plain-text
sections (e.g., the block containing '[Describe any deployment issues here, or
write "None"]' and all other non-code fenced blocks noted in the review). Update
every plain-text fence in the file so the opening fence is ```text (keep the
existing contents and closing fence unchanged) to satisfy MD040 and markdown
linting.
In `@docs/rhcos10-testing.md`:
- Around line 382-384: Under the "Symptoms" section, change the two unlabeled
fenced code blocks that currently contain error message snippets (the blocks
showing "Error: failed to load private key: crypto/rsa: unsupported key size"
and the other snippet later) to use a language label by replacing the opening
``` with ```text; this resolves MD040 by marking the snippets as plain text.
Locate the two fenced blocks under the "Symptoms" heading and update their
opening fences to ```text so both error-message snippets are explicitly labeled.
In `@hack/test-rhcos10-compatibility.sh`:
- Around line 325-327: The parent script invokes verify-rhcos10-crypto.sh
without propagating OUTPUT_DIR, causing the child to use its default _output
while the parent expects ${OUTPUT_DIR}/rhcos10-crypto-verification-report.txt;
update the invocation (the line calling bash
"${SCRIPT_DIR}/verify-rhcos10-crypto.sh") to pass the OUTPUT_DIR into the
subprocess (for example by prefixing the call with OUTPUT_DIR="${OUTPUT_DIR}" or
exporting OUTPUT_DIR before calling) so the verifier writes the report to the
same directory the parent is later reading.
- Around line 121-128: The argument parsing dereferences $2 for --e2e-timeout
and --output-dir without validating it, and OUTPUT_DIR is not exported before
the child script invocation; update the case handlers for the flags (the
--e2e-timeout and --output-dir branches) to check that "${2:-}" exists and is
not another flag (e.g. empty or starts with '-') and emit a clear error/exit if
missing, then assign E2E_TIMEOUT/OUTPUT_DIR and set TEST_REPORT accordingly;
also export OUTPUT_DIR in the parent script before the child script invocation
so the child inherits the specified OUTPUT_DIR rather than falling back to its
own default.
In `@hack/verify-rhcos10-crypto.sh`:
- Around line 46-59: The post-increment idiom in the logging helpers can cause
exits under set -e because ((var++)) returns non-zero when var was 0; update
log_success, log_error, and log_warning to use pre-increment (change
((CHECKS_PASSED++)), ((CHECKS_FAILED++)), ((CHECKS_WARNING++)) to
((++CHECKS_PASSED)), ((++CHECKS_FAILED)), ((++CHECKS_WARNING))) and similarly
replace any other occurrences of post-increment on these counters elsewhere in
the script (the other places where CHECKS_PASSED / CHECKS_FAILED /
CHECKS_WARNING are incremented) to use pre-increment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46af8606-2376-4b31-87df-8b055a322fcb
📒 Files selected for processing (5)
Makefiledocs/rhcos10-test-results-template.mddocs/rhcos10-testing.mdhack/test-rhcos10-compatibility.shhack/verify-rhcos10-crypto.sh
docs/rhcos10-testing.md
Outdated
| ``` | ||
| Error: failed to load private key: crypto/rsa: unsupported key size | ||
| ``` |
There was a problem hiding this comment.
Specify a language for Symptoms fenced snippets.
Line 382 and Line 406 use unlabeled fenced blocks, which triggers MD040. Use text for these error-message snippets.
Also applies to: 406-408
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 382-382: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/rhcos10-testing.md` around lines 382 - 384, Under the "Symptoms"
section, change the two unlabeled fenced code blocks that currently contain
error message snippets (the blocks showing "Error: failed to load private key:
crypto/rsa: unsupported key size" and the other snippet later) to use a language
label by replacing the opening ``` with ```text; this resolves MD040 by marking
the snippets as plain text. Locate the two fenced blocks under the "Symptoms"
heading and update their opening fences to ```text so both error-message
snippets are explicitly labeled.
There was a problem hiding this comment.
Done. Added text language identifier to both error message fenced code blocks under the Symptoms sections.
AI-assisted response via Claude Code
| --e2e-timeout) | ||
| E2E_TIMEOUT="$2" | ||
| shift 2 | ||
| ;; | ||
| --output-dir) | ||
| OUTPUT_DIR="$2" | ||
| TEST_REPORT="${OUTPUT_DIR}/rhcos10-compatibility-report.md" | ||
| shift 2 |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
# Check if file exists and get line count
wc -l hack/test-rhcos10-compatibility.sh 2>/dev/null || echo "File not found"
# Verify lines 121-128 and surrounding context
sed -n '115,135p' hack/test-rhcos10-compatibility.sh 2>/dev/null || echo "Cannot read file"Repository: openshift/cert-manager-operator
Repository: openshift/cert-manager-operator
Exit code: 0
stdout:
648 hack/test-rhcos10-compatibility.sh
shift
;;
--skip-crypto)
RUN_CRYPTO_VERIFICATION=false
shift
;;
--e2e-timeout)
E2E_TIMEOUT="$2"
shift 2
;;
--output-dir)
OUTPUT_DIR="$2"
TEST_REPORT="${OUTPUT_DIR}/rhcos10-compatibility-report.md"
shift 2
;;
-h|--help)
usage
exit 0
;;
*)
log_error "Unknown option: $1"
Script executed:
# Search for set options at the beginning
head -20 hack/test-rhcos10-compatibility.sh
# Search for the --e2e-timeout and --output-dir argument parsing
rg -n -A2 -B2 -- '--e2e-timeout|--output-dir' hack/test-rhcos10-compatibility.shRepository: openshift/cert-manager-operator
Repository: openshift/cert-manager-operator
Exit code: 0
stdout:
#!/usr/bin/env bash
# Copyright 2024 Red Hat, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# test-rhcos10-compatibility.sh - Run comprehensive RHCOS 10 compatibility tests
#
# This script orchestrates comprehensive compatibility testing for cert-manager
# on RHCOS 10, including deployment verification, e2e tests, crypto library checks,
82- --skip-e2e Skip e2e test suite execution
83- --skip-crypto Skip crypto library verification
84: --e2e-timeout DURATION Set e2e test timeout (default: 2h)
85: --output-dir DIR Set output directory (default: _output)
86- -h, --help Show this help message
87-
--
99-
100- # Run with custom timeout
101: $0 --e2e-timeout 3h
102-
103- # Run with specific e2e test filter
--
119- shift
120- ;;
121: --e2e-timeout)
122- E2E_TIMEOUT="$2"
123- shift 2
124- ;;
125: --output-dir)
126- OUTPUT_DIR="$2"
127- TEST_REPORT="${OUTPUT_DIR}/rhcos10-compatibility-report.md"
Script executed:
# Verify child script invocation and OUTPUT_DIR propagation
rg -n -B5 -A5 'verify-rhcos10-crypto' hack/test-rhcos10-compatibility.sh
# Check if OUTPUT_DIR is exported before child script call
rg -n 'export.*OUTPUT_DIR|OUTPUT_DIR=' hack/test-rhcos10-compatibility.shRepository: openshift/cert-manager-operator
Repository: openshift/cert-manager-operator
Exit code: 0
stdout:
321- CRYPTO_PASSED=true # Mark as passed if skipped
322- return 0
323- fi
324-
325- # Run crypto verification script
326: if bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"; then
327- log_success "Crypto verification passed"
328- CRYPTO_PASSED=true
329- return 0
330- else
331- log_error "Crypto verification failed"
38:OUTPUT_DIR="${OUTPUT_DIR:-${PROJECT_ROOT}/_output}"
126: OUTPUT_DIR="$2"
Script executed:
# Check line 219 for the SC2155 export issue
sed -n '215,225p' hack/test-rhcos10-compatibility.shRepository: openshift/cert-manager-operator
Repository: openshift/cert-manager-operator
Exit code: 0
stdout:
# Export for report
export CLUSTER_OCP_VERSION="$ocp_version"
export CLUSTER_RHCOS_VERSION="$rhcos_version"
export CLUSTER_PLATFORM="$platform"
export CLUSTER_NAME="$(oc config current-context)"
}
# Verify cert-manager deployment
verify_deployment() {
log_section "Verifying cert-manager Deployment"
Script executed:
# Check for set command and shebang area more thoroughly
head -50 hack/test-rhcos10-compatibility.sh | grep -n 'set\|#!/'
# Also check if there's a set statement later
rg -n '^set ' hack/test-rhcos10-compatibility.sh | head -5Repository: openshift/cert-manager-operator
Repository: openshift/cert-manager-operator
Exit code: 0
stdout:
1:#!/usr/bin/env bash
23:set -euo pipefail
23:set -euo pipefail
Script executed:
# Verify child script defaults and if OUTPUT_DIR is exported elsewhere
rg -n 'export OUTPUT_DIR|^OUTPUT_DIR' hack/test-rhcos10-compatibility.sh
# Check verify-rhcos10-crypto.sh to see how it handles OUTPUT_DIR
head -50 hack/verify-rhcos10-crypto.sh | grep -A2 -B2 'OUTPUT_DIR'Repository: openshift/cert-manager-operator
Repository: openshift/cert-manager-operator
Exit code: 0
stdout:
38:OUTPUT_DIR="${OUTPUT_DIR:-${PROJECT_ROOT}/_output}"
# Output directory
OUTPUT_DIR="${OUTPUT_DIR:-_output}"
REPORT_FILE="${OUTPUT_DIR}/rhcos10-crypto-verification-report.txt"
# Logging functions
Validate required values for --e2e-timeout and --output-dir.
Lines 122 and 126 dereference $2 without checking it exists. With set -euo pipefail on line 23, --e2e-timeout or --output-dir without a value exits abruptly with an unhelpful unbound variable error. Add value validation before dereferencing.
Additionally, the child script invocation at line 326 does not export OUTPUT_DIR, causing it to use its own default (_output) instead of the parent's (${PROJECT_ROOT}/_output), which splits artifacts and reports across different locations when users specify --output-dir.
🛠️ Suggested fix for argument parsing
--e2e-timeout)
+ if [[ $# -lt 2 || "$2" == -* ]]; then
+ log_error "--e2e-timeout requires a value (e.g., 2h)"
+ exit 1
+ fi
E2E_TIMEOUT="$2"
shift 2
;;
--output-dir)
+ if [[ $# -lt 2 || "$2" == -* ]]; then
+ log_error "--output-dir requires a directory path"
+ exit 1
+ fi
OUTPUT_DIR="$2"
TEST_REPORT="${OUTPUT_DIR}/rhcos10-compatibility-report.md"
shift 2
;;For environment propagation, export OUTPUT_DIR before invoking the child script at line 326:
+ export OUTPUT_DIR
if bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/test-rhcos10-compatibility.sh` around lines 121 - 128, The argument
parsing dereferences $2 for --e2e-timeout and --output-dir without validating
it, and OUTPUT_DIR is not exported before the child script invocation; update
the case handlers for the flags (the --e2e-timeout and --output-dir branches) to
check that "${2:-}" exists and is not another flag (e.g. empty or starts with
'-') and emit a clear error/exit if missing, then assign E2E_TIMEOUT/OUTPUT_DIR
and set TEST_REPORT accordingly; also export OUTPUT_DIR in the parent script
before the child script invocation so the child inherits the specified
OUTPUT_DIR rather than falling back to its own default.
| # Run crypto verification script | ||
| if bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"; then | ||
| log_success "Crypto verification passed" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the exact code at lines 325-327 and surrounding context
head -350 hack/test-rhcos10-compatibility.sh | tail -30Repository: openshift/cert-manager-operator
Length of output: 871
🏁 Script executed:
# Also check if OUTPUT_DIR is used in the crypto verification script
cat hack/verify-rhcos10-crypto.sh | head -50Repository: openshift/cert-manager-operator
Length of output: 1421
🏁 Script executed:
# Search for OUTPUT_DIR usage patterns in both scripts
rg -n 'OUTPUT_DIR' hack/test-rhcos10-compatibility.sh hack/verify-rhcos10-crypto.shRepository: openshift/cert-manager-operator
Length of output: 2503
Propagate OUTPUT_DIR to the crypto verifier subprocess.
Line 326 launches verify-rhcos10-crypto.sh without passing OUTPUT_DIR. The child script defaults to _output when unset (line 38 of verify-rhcos10-crypto.sh), but the parent script explicitly looks for the verification report at ${OUTPUT_DIR}/rhcos10-crypto-verification-report.txt (line 383-384). When --output-dir is used, this mismatch causes the report collection to fail silently.
🛠️ Suggested fix
- if bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"; then
+ if OUTPUT_DIR="$OUTPUT_DIR" bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"; then
log_success "Crypto verification passed"
CRYPTO_PASSED=true
return 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Run crypto verification script | |
| if bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"; then | |
| log_success "Crypto verification passed" | |
| # Run crypto verification script | |
| if OUTPUT_DIR="$OUTPUT_DIR" bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"; then | |
| log_success "Crypto verification passed" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/test-rhcos10-compatibility.sh` around lines 325 - 327, The parent script
invokes verify-rhcos10-crypto.sh without propagating OUTPUT_DIR, causing the
child to use its default _output while the parent expects
${OUTPUT_DIR}/rhcos10-crypto-verification-report.txt; update the invocation (the
line calling bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh") to pass the
OUTPUT_DIR into the subprocess (for example by prefixing the call with
OUTPUT_DIR="${OUTPUT_DIR}" or exporting OUTPUT_DIR before calling) so the
verifier writes the report to the same directory the parent is later reading.
There was a problem hiding this comment.
Done. Updated script invocation to propagate OUTPUT_DIR to child process: OUTPUT_DIR="$OUTPUT_DIR" bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"
AI-assisted response via Claude Code
|
@chiragkyal: This pull request references OAPE-494 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
docs/rhcos10-test-results-template.md (1)
71-73:⚠️ Potential issue | 🟡 MinorAdd language identifiers to plain-text fenced blocks.
Unlabeled fences at Line 71 and the other listed sections still trigger MD040. Use
```textfor these plain-text blocks.Also applies to: 104-106, 155-157, 181-183, 190-192, 207-214, 230-232, 240-248, 252-254, 258-260, 287-294
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rhcos10-test-results-template.md` around lines 71 - 73, The plain-text fenced code blocks in the template (the triple-backtick blocks like ``` ... ```) are unlabeled and trigger MD040; update each of those plain fenced blocks (e.g., the block containing "[Describe any deployment issues here, or write "None"]" and the other occurrences listed) to use a language identifier by replacing the opening fence with ```text so they become ```text ... ```; ensure every plain-text fence across the document is labeled consistently to satisfy the linter.hack/test-rhcos10-compatibility.sh (2)
326-326:⚠️ Potential issue | 🟠 MajorPropagate
OUTPUT_DIRto the crypto verifier subprocess.Line 326 runs the child script without
OUTPUT_DIR, while this script expects the report in${OUTPUT_DIR}(see Line 383). Custom output directories can therefore break crypto report collection.🛠️ Suggested fix
- if bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"; then + if OUTPUT_DIR="$OUTPUT_DIR" bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh"; then log_success "Crypto verification passed"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/test-rhcos10-compatibility.sh` at line 326, The child call to verify-rhcos10-crypto.sh omits propagating OUTPUT_DIR, so when a custom OUTPUT_DIR is set this parent script cannot find the crypto report; update the invocation at the bash "${SCRIPT_DIR}/verify-rhcos10-crypto.sh" line to export or pass the OUTPUT_DIR into the subprocess (e.g., prefix the call with OUTPUT_DIR="${OUTPUT_DIR}" or export OUTPUT_DIR beforehand) so verify-rhcos10-crypto.sh sees the same OUTPUT_DIR used later when reading the report.
121-128:⚠️ Potential issue | 🟠 MajorValidate valued flags before reading
$2.Line 122 and Line 126 dereference
$2without checking presence. Withset -u,--e2e-timeoutor--output-dirwithout a value exits with an unhelpful unbound-variable error.🛠️ Suggested fix
--e2e-timeout) + if [[ $# -lt 2 || -z "${2:-}" || "${2:-}" == -* ]]; then + log_error "--e2e-timeout requires a value (e.g., 2h)" + exit 1 + fi E2E_TIMEOUT="$2" shift 2 ;; --output-dir) + if [[ $# -lt 2 || -z "${2:-}" || "${2:-}" == -* ]]; then + log_error "--output-dir requires a directory path" + exit 1 + fi OUTPUT_DIR="$2" TEST_REPORT="${OUTPUT_DIR}/rhcos10-compatibility-report.md" shift 2 ;;docs/rhcos10-testing.md (1)
382-384:⚠️ Potential issue | 🟡 MinorSpecify a language for the
Symptomsfenced snippets.These fences are still unlabeled and trigger MD040. Use
```textfor both blocks.Also applies to: 406-408
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/rhcos10-testing.md` around lines 382 - 384, The Symptoms code fences are unlabeled and trigger MD040 — update both unlabeled fenced code blocks in the Symptoms sections (the blocks that currently start with ``` and contain the error text "Error: failed to load private key: crypto/rsa: unsupported key size") to use a language label by changing the opening fence to ```text (do the same for the other block referenced around the second Symptoms occurrence), leaving the contents and closing ``` unchanged.hack/verify-rhcos10-crypto.sh (1)
48-59:⚠️ Potential issue | 🔴 Critical
((var++))can abort execution withset -e.At Line 48/53/58 and Line 126/138/276, post-increment may return non-zero on first increment and terminate the script unexpectedly.
🛠️ Suggested fix
log_success() { echo -e "${GREEN}[PASS]${NC} $*" - ((CHECKS_PASSED++)) + ((++CHECKS_PASSED)) } log_error() { echo -e "${RED}[FAIL]${NC} $*" - ((CHECKS_FAILED++)) + ((++CHECKS_FAILED)) } log_warning() { echo -e "${YELLOW}[WARN]${NC} $*" - ((CHECKS_WARNING++)) + ((++CHECKS_WARNING)) } @@ - ((total_nodes++)) + ((++total_nodes)) @@ - ((rhcos10_nodes++)) + ((++rhcos10_nodes)) @@ - ((running_pods++)) + ((++running_pods))Also applies to: 126-126, 138-138, 276-276
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/verify-rhcos10-crypto.sh` around lines 48 - 59, Replace unsafe arithmetic post-increments that can cause early exit under set -e with explicit, safe increments: locate the three increment uses in the functions that call CHECKS_PASSED, log_error, and log_warning (symbols CHECKS_PASSED, CHECKS_FAILED, CHECKS_WARNING) and replace the constructs using ((VAR++)) with a deterministic increment such as VAR=$((VAR+1)) or ((VAR+=1)); apply the same change at the other reported occurrences (the increments referenced at the other comment locations) so all CHECKS_* increments no longer use post-increment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/rhcos10-testing.md`:
- Line 482: The documentation references the wrong output filename; update the
text that currently says `_output/rhcos10-test-report.md` to the actual
generated/read filename `_output/rhcos10-compatibility-report.md` so it matches
the output from the test script `hack/test-rhcos10-compatibility.sh` and the
Makefile target; ensure any other mentions in `rhcos10-testing.md` use
`_output/rhcos10-compatibility-report.md` as well.
---
Duplicate comments:
In `@docs/rhcos10-test-results-template.md`:
- Around line 71-73: The plain-text fenced code blocks in the template (the
triple-backtick blocks like ``` ... ```) are unlabeled and trigger MD040; update
each of those plain fenced blocks (e.g., the block containing "[Describe any
deployment issues here, or write "None"]" and the other occurrences listed) to
use a language identifier by replacing the opening fence with ```text so they
become ```text ... ```; ensure every plain-text fence across the document is
labeled consistently to satisfy the linter.
In `@docs/rhcos10-testing.md`:
- Around line 382-384: The Symptoms code fences are unlabeled and trigger MD040
— update both unlabeled fenced code blocks in the Symptoms sections (the blocks
that currently start with ``` and contain the error text "Error: failed to load
private key: crypto/rsa: unsupported key size") to use a language label by
changing the opening fence to ```text (do the same for the other block
referenced around the second Symptoms occurrence), leaving the contents and
closing ``` unchanged.
In `@hack/test-rhcos10-compatibility.sh`:
- Line 326: The child call to verify-rhcos10-crypto.sh omits propagating
OUTPUT_DIR, so when a custom OUTPUT_DIR is set this parent script cannot find
the crypto report; update the invocation at the bash
"${SCRIPT_DIR}/verify-rhcos10-crypto.sh" line to export or pass the OUTPUT_DIR
into the subprocess (e.g., prefix the call with OUTPUT_DIR="${OUTPUT_DIR}" or
export OUTPUT_DIR beforehand) so verify-rhcos10-crypto.sh sees the same
OUTPUT_DIR used later when reading the report.
In `@hack/verify-rhcos10-crypto.sh`:
- Around line 48-59: Replace unsafe arithmetic post-increments that can cause
early exit under set -e with explicit, safe increments: locate the three
increment uses in the functions that call CHECKS_PASSED, log_error, and
log_warning (symbols CHECKS_PASSED, CHECKS_FAILED, CHECKS_WARNING) and replace
the constructs using ((VAR++)) with a deterministic increment such as
VAR=$((VAR+1)) or ((VAR+=1)); apply the same change at the other reported
occurrences (the increments referenced at the other comment locations) so all
CHECKS_* increments no longer use post-increment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d309e501-9a9f-43a4-a071-a5912e0e23cf
📒 Files selected for processing (5)
Makefiledocs/rhcos10-test-results-template.mddocs/rhcos10-testing.mdhack/test-rhcos10-compatibility.shhack/verify-rhcos10-crypto.sh
| make report-rhcos10 | ||
| ``` | ||
|
|
||
| This will create a report in `_output/rhcos10-test-report.md`. |
There was a problem hiding this comment.
Fix report filename mismatch in documentation.
Line 482 references _output/rhcos10-test-report.md, but the generated/read filename is _output/rhcos10-compatibility-report.md (from hack/test-rhcos10-compatibility.sh Line 39 and Makefile Line 301).
🛠️ Suggested fix
-This will create a report in `_output/rhcos10-test-report.md`.
+This will create a report in `_output/rhcos10-compatibility-report.md`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This will create a report in `_output/rhcos10-test-report.md`. | |
| This will create a report in `_output/rhcos10-compatibility-report.md`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/rhcos10-testing.md` at line 482, The documentation references the wrong
output filename; update the text that currently says
`_output/rhcos10-test-report.md` to the actual generated/read filename
`_output/rhcos10-compatibility-report.md` so it matches the output from the test
script `hack/test-rhcos10-compatibility.sh` and the Makefile target; ensure any
other mentions in `rhcos10-testing.md` use
`_output/rhcos10-compatibility-report.md` as well.
- Fix critical set -e bug with post-increment in verify-rhcos10-crypto.sh (change ((var++)) to ((++var)) to prevent unexpected exits) - Fix OUTPUT_DIR propagation to child crypto verification script - Add text language identifiers to markdown fenced code blocks (MD040) Fixes identified by CodeRabbit code review. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@coderabbitai Re: rhcos10-test-results-template.md:73 Done. Added AI-assisted response via Claude Code |
|
@coderabbitai Re: rhcos10-testing.md:384 Done. Added AI-assisted response via Claude Code |
|
Tip For best results, initiate chat on the files or code changes.
|
|
@coderabbitai Re: test-rhcos10-compatibility.sh:327 Done. Updated script invocation to propagate OUTPUT_DIR to child process: AI-assisted response via Claude Code |
|
@coderabbitai Re: verify-rhcos10-crypto.sh:59 Done. Changed all post-increment operations to pre-increment to prevent unexpected exits under
AI-assisted response via Claude Code |
|
Tip For best results, initiate chat on the files or code changes.
|
|
Tip For best results, initiate chat on the files or code changes.
|
2 participants
Summary
This PR adds comprehensive testing infrastructure for verifying cert-manager operator compatibility with RHCOS 10 and RHEL 10 on OpenShift 4.20, 4.21, and 4.22.
Changes
Documentation
docs/rhcos10-testing.md: Comprehensive testing guide covering:docs/rhcos10-test-results-template.md: Standardized template for documenting test resultsAutomation Scripts
hack/verify-rhcos10-crypto.sh: Automated crypto library verification:hack/test-rhcos10-compatibility.sh: Comprehensive test orchestration:Makefile Targets
make test-rhcos10: Run complete RHCOS 10 compatibility test suitemake verify-rhcos10-crypto: Run crypto library verification onlymake report-rhcos10: Display test reportTesting
The scripts have been tested for:
Ready for testing on actual RHCOS 10 clusters.
Usage
Related Issues
Addresses OAPE-494 as part of the RHCOS 10/RHEL 10 readiness effort.
🤖 Generated with Claude Code via
/jira:solve [OAPE-494](https://issues.redhat.com/browse/OAPE-494) cert-manager-operatorSummary by CodeRabbit
New Features
Documentation