CNF-25916: Add TLS scanner CI jobs for PTP operator - #82254
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PTP release CI adds a TLS scanner image dependency, scan and TLS 1.3 settings, and scanner test references. The Telco5G PTP test script conditionally runs the scanner, publishes artifacts, records failures, and cleans up resources. ChangesPTP TLS scanner integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant PTPTests
participant OpenShift
participant TLSScannerPod
participant Artifacts
CI->>PTPTests: run configured TLS scan workflow
PTPTests->>OpenShift: create scanner namespace and resources
PTPTests->>TLSScannerPod: start TLS scan
TLSScannerPod-->>PTPTests: report completion status
PTPTests->>Artifacts: copy scan outputs and JUnit XML
PTPTests->>OpenShift: delete scanner resources
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ 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 |
|
@jzding: GitHub didn't allow me to request PR reviews from the following users: openshift/telco-5g-ptp. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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 kubernetes-sigs/prow repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@ci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-commands.sh`:
- Around line 532-533: Replace the cluster-admin and privileged SCC grants on
the default service account in the telco5g PTP setup with a dedicated scanner
service account, granting only the validated permissions required by the
scanner. Add cleanup in the script’s EXIT trap to revoke any exceptional
SCC/RBAC grants before deleting ${TLS_NS}, ensuring recreated tls-scanner
workloads cannot retain cluster-wide access.
- Around line 528-585: Update the TLS scan block around the tls-scanner Pod
lifecycle to track failures from namespace setup, Pod creation/execution,
timeout, artifact collection, and JUnit generation instead of allowing them
through set +e or unconditional || true. Require junit_tls_scan.xml after
collection, register cleanup of TLS_NS via a trap, and exit non-zero after
cleanup when any scan step fails while preserving result collection.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: fcb500db-5c55-40f5-af10-2552bb102798
📒 Files selected for processing (3)
ci-operator/config/openshift/ptp-operator/openshift-ptp-operator-release-5.0.yamlci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-commands.shci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-ref.yaml
openshift-ci
Bot
added
the
approved
|
/pj-rehearse pull-ci-openshift-ptp-operator-release-5.0-operator-e2e |
|
@jzding: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
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 `@ci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-commands.sh`:
- Around line 525-526: Update the TLS scanner condition in the version-gating
block to require an explicit T5CI_VERSION matching 4.22 or newer, excluding
empty values and 4.0–4.9. Preserve the existing PULL_SPEC_TLS_SCANNER_TOOL
requirement and ensure unsupported versions cannot enable scanning.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c89a5a39-1b1d-46d3-aca3-fae08e2c327c
📒 Files selected for processing (3)
ci-operator/config/openshift/ptp-operator/openshift-ptp-operator-release-5.0.yamlci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-commands.shci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- ci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-ref.yaml
- ci-operator/config/openshift/ptp-operator/openshift-ptp-operator-release-5.0.yaml
f42746a to
511ac4b
Compare
There was a problem hiding this comment.
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
`@ci-operator/config/openshift/ptp-operator/openshift-ptp-operator-release-5.0.yaml`:
- Around line 97-136: Run make update after adding the tls-pqc-readiness and
tls13-adherence test definitions, then include all regenerated Prow and CI
configuration outputs so both entries are registered consistently with the new
metadata.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 54a9e52d-d831-4540-8ac3-d968e70ae73a
📒 Files selected for processing (3)
ci-operator/config/openshift/ptp-operator/openshift-ptp-operator-release-5.0.yamlci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-commands.shci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- ci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-ref.yaml
- ci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-commands.sh
da6126d to
63d1aab
Compare
|
/pj-rehearse pull-ci-openshift-ptp-operator-release-5.0-tls-pqc-readiness |
There was a problem hiding this comment.
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 `@ci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-commands.sh`:
- Around line 544-546: Update the provisioning flow around TLS_SCANNER_CRB to
bind the scanner service account to a scanner-specific least-privilege
ClusterRole instead of cluster-admin. Immediately after provisioning, register a
guarded EXIT trap that revokes the RBAC binding and cleans up the scanner
namespace on every exit path, while preserving safe behavior if resources were
not created.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 4261c6d0-4ed0-43e4-b984-bfd1426971a0
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/ptp-operator/openshift-ptp-operator-release-5.0-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (3)
ci-operator/config/openshift/ptp-operator/openshift-ptp-operator-release-5.0.yamlci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-commands.shci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/telco5g/ptp/tests/telco5g-ptp-tests-ref.yaml
|
@jzding: your |
|
/test ci/prow/config |
|
/test config |
1 similar comment
|
/test config |
|
/pj-rehearse pull-ci-openshift-ptp-operator-release-5.0-tls-pqc-readiness |
|
@jzding: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-e2e-telco5g-ptp |
|
@jzding: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
37db75e to
980d9e7
Compare
|
/test generated-config |
|
/pj-rehearse pull-ci-openshift-ptp-operator-release-5.0-operator-e2e |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-telco5g-ptp-tls-scan |
|
@jzding: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-ptp-operator-release-5.0-tls-scan |
|
@jzding: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@jzding: job(s): periodic-ci-openshift-release-main-nightly-5.0-e2e-telco5g-ptp-tls-scan either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse periodic-ci-openshift-ptp-operator-release-5.0-tls-scan-periodic |
|
@jzding: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
jzding
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@jzding: This pull request references CNF-25916 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 "5.0.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. |
dfe6f59 to
8fe7bb6
Compare
|
/pj-rehearse periodic-ci-openshift-ptp-operator-release-5.0-tls-scan-periodic |
|
@jzding: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-ptp-operator-release-5.0-tls-scan |
|
@jzding: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-ptp-operator-main-tls-scan-periodic |
|
@jzding: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@jzding: |
openshift-ci
Bot
removed
the
approved
openshift-ci
Bot
added
the
approved
Add TLS compliance scanning to PTP operator CI per OCPSTRAT-2611. For each branch (4.22, 4.23, 5.0, 5.1, main), add a combined tls-scan test that covers both TLS 1.3 adherence (StrictAllComponents) and PQC readiness in a single cluster deploy: Presubmit (on-demand): - tls-scan: always_run false, optional true. Deploys PTP operator via ci-index, runs tls-13, waits for PTP operator to restart with the new TLS profile, then runs tls-scanner-run. The wait-ptp-tls-restart step is needed because the PTP operator's SecurityProfileWatcher detects the TLS profile change from the tls-13 step and self-terminates for OLM to restart it. The reconciler then re-renders the kube-rbac-proxy daemonset template with the new TLS min version and cipher suites. Without the wait, the scanner would scan endpoints still running with the old TLS configuration. Periodic (daily 03:00 UTC, one branch per weekday): - 4.22 Monday, 4.23 Tuesday, 5.0 Wednesday, 5.1 Thursday, main Friday. All tests use optional-operators-ci-aws workflow, scoped to openshift-ptp namespace, SCANNER_CPU=2, tls-scanner-tool from the official tls-scanner namespace. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jack Ding <jackding@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jzding The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
[REHEARSALNOTIFIER]
A total of 55 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@jzding: The following tests 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. |
2 participants
Summary
Add TLS compliance scanning to PTP operator CI per OCPSTRAT-2611.
Combined
tls-scantestA single test covers both TLS 1.3 adherence (
StrictAllComponents) and PQC readiness (PQC_CHECK=true) in one cluster deploy:tls-13— configures Modern TLS profile with StrictAllComponents adherencewait-ptp-tls-restart— waits for PTP operator to restart (theSecurityProfileWatcherdetects the profile change and self-terminates for OLM to restart) and the daemonset to re-roll with updated kube-rbac-proxy TLS configtls-scanner-run— scans openshift-ptp namespace with PQC check enabledPresubmit (on-demand)
tls-scan:always_run: false,optional: trueopenshift-ptpnamespace,SCANNER_CPU=2Periodic (daily 03:00 UTC, one branch per weekday)
Configuration
tls-scanner-toolbase image from the officialtls-scannernamespaceoptional-operators-ci-awsworkflow (PTP deploys fromci-index)operator:section withbundleci.Dockerfile, soci-indexis available for allScan results analysis
Rehearsal results show PTP's own endpoints are compliant:
The scanner also reports 14 non-compliant kube-rbac-proxy ports (9001, 9103, 9105, 9108) that are false positives — these belong to other OCP components (node-exporter, machine-config-daemon, network operator) sharing the same host network, since the linuxptp-daemon DaemonSet uses
hostNetwork: true. This is a known tls-scanner limitation tracked in openshift/tls-scanner#85.Outstanding issues in tls-scanner
tls-scanner-runcleanup trap crashes withOWNS_NAMESPACE: unbound variablewhen the scanner exits with code 1. Also fixes Failed pod handling and fallback wait logic. Fix submitted.hostNetwork: true.Changes
openshift-ptp-operator-release-4.22.yamltls-scanner-toolbase image +tls-scan+tls-scan-periodicwith wait stepopenshift-ptp-operator-release-4.23.yamlopenshift-ptp-operator-release-5.0.yamlopenshift-ptp-operator-release-5.1.yamlopenshift-ptp-operator-main.yamlTest plan
tls-scanran successfully — PTP endpoints compliant, false positives from hostNetwork identified🤖 Generated with Claude Code