osc: Add TLS scanner periodic jobs for sandboxed-containers-operator#79389
osc: Add TLS scanner periodic jobs for sandboxed-containers-operator#79389thejasn wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdates the devel periodic CI config for sandboxed-containers-operator: changes ChangesOpenShift Sandboxed Containers Operator Periodic Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Suggested labels
🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: thejasn 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__periodics.yaml (1)
25-138: 🏗️ Heavy liftConsider extracting duplicated Pod lifecycle steps to shared step-registry entries.
The two test definitions are 99% identical—they differ only in the
PQC_CHECKenvironment variable (line 89). The entire Pod manifest (lines 42-64 vs 99-121), creation logic, wait command, and deletion logic are duplicated across both tests.This duplication increases maintenance burden: any change to the Pod spec or lifecycle commands must be applied in two places, risking inconsistency.
Consider refactoring by:
- Creating a shared step-registry entry for peer-pod creation/deletion
- Parameterizing the Pod manifest via environment variables if needed
- Passing test-specific environment variables (like
PQC_CHECK) through the step configurationThis would reduce the ~110 lines to approximately half while preserving both test variants.
🤖 Prompt for 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. In `@ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__periodics.yaml` around lines 25 - 138, Both periodic tests (as: tls-scanner-default and as: tls-scanner-pqc) duplicate the peer-pod Pod manifest and lifecycle steps; extract those into reusable step-registry entries and parameterize them. Create shared step-registry steps (e.g., create-peer-pod and delete-peer-pod) that accept env parameters like POD_NAME, SCAN_NAMESPACE (or NAMESPACE), RUNTIMECLASS, IMAGE, and TIMEOUT, then replace the inline commands in both test definitions with references to those steps; keep test-specific env (PQC_CHECK) only in the top-level env for tls-scanner-pqc so the common steps read parameters from the passed env. Ensure the step names match the existing ref usages (ref: create-peer-pod and ref: delete-peer-pod) so workflow sandboxed-containers-operator-e2e-aws continues to work.
🤖 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.
Nitpick comments:
In
`@ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__periodics.yaml`:
- Around line 25-138: Both periodic tests (as: tls-scanner-default and as:
tls-scanner-pqc) duplicate the peer-pod Pod manifest and lifecycle steps;
extract those into reusable step-registry entries and parameterize them. Create
shared step-registry steps (e.g., create-peer-pod and delete-peer-pod) that
accept env parameters like POD_NAME, SCAN_NAMESPACE (or NAMESPACE),
RUNTIMECLASS, IMAGE, and TIMEOUT, then replace the inline commands in both test
definitions with references to those steps; keep test-specific env (PQC_CHECK)
only in the top-level env for tls-scanner-pqc so the common steps read
parameters from the passed env. Ensure the step names match the existing ref
usages (ref: create-peer-pod and ref: delete-peer-pod) so workflow
sandboxed-containers-operator-e2e-aws continues to work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9353e89e-afd5-4951-89ae-1c3d364881f0
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__periodics.yaml
|
/pj-rehearse periodic-ci-openshift-sandboxed-containers-operator-devel-periodics-tls-scanner-default |
|
@thejasn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Add two 72h periodic jobs on the devel branch to verify TLS compliance of the sandboxed-containers operator stack with an active peer pod: - tls-scanner-default: baseline TLS scan (certs, ciphers, protocols) - tls-scanner-pqc: PQC readiness check (TLS 1.3 + ML-KEM/X25519MLKEM768) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Thejas N <thn@redhat.com>
thejasn
force-pushed
the
thn/osc-tls-scanner
branch
from
86c7095 to
31b37be
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-sandboxed-containers-operator-devel-periodics-tls-scanner-default |
|
@thejasn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-sandboxed-containers-operator-devel-periodics-tls-scanner-default |
|
@thejasn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@thejasn: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
Add two 72h periodic jobs on the devel branch to verify TLS compliance of the sandboxed-containers operator stack with an active peer pod:
Updates the OpenShift CI periodic configuration for the openshift/sandboxed-containers-operator devel periodics to add TLS-scanning periodic jobs and associated CI image inputs.
Practical impact
Jobs added / behavior
CI environment and resource defaults
Other notes
Related to: rhjira#KATA-5101