NO-JIRA: Add optional TLS scanner CI job for cert-manager-operator - #82715
Conversation
Add a manually triggered tls-scanner presubmit on master that installs the operator, deploys the CertManager operand, and runs tls-scanner-run against the cert-manager namespaces.
|
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 cert-manager operator CI configuration adds a ChangesTLS scanner workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as tls-scanner workflow
participant Operator as cert-manager operator
participant TrustManager
participant Scanner as TLS scanner tests
Workflow->>Operator: Install bundle and wait for deployments
Workflow->>Operator: Enable TrustManager feature gate
Operator-->>Workflow: Report rollout readiness
Workflow->>TrustManager: Create TrustManager resource
TrustManager-->>Workflow: Report deployment availability
Workflow->>Scanner: Run TLS 1.3 and TLS scanner tests
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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/config/openshift/cert-manager-operator/openshift-cert-manager-operator-master.yaml`:
- Around line 468-469: The workflow references tls-scanner-run without
TLS_SCANNER_CLUSTER_LABEL, causing the scanner to use its privileged host-mode
configuration. Update this workflow to select a supported restricted scanner
mode for standard AWS clusters by configuring the required cluster label, or
remove the scanner reference until the shared step supports restricted
execution; do not enable host mode without documented approval.
- Around line 460-463: Update the operand-wait sequence after the “Waiting for
cert-manager operand deployments...” message to first wait for creation of the
cert-manager namespace and each deployment using oc wait --for=create, then
retain the existing --for=condition=Available checks for cert-manager,
cert-manager-webhook, and cert-manager-cainjector.
🪄 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: a60eeb49-e9b2-441e-827c-909c4e928439
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/cert-manager-operator/openshift-cert-manager-operator-master-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift/cert-manager-operator/openshift-cert-manager-operator-master.yaml
| - ref: tls-13 | ||
| - ref: tls-scanner-run |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not use the scanner’s default privileged host mode without an explicit security decision.
This workflow does not set TLS_SCANNER_CLUSTER_LABEL. The shared tls-scanner-run step therefore uses its default host-mode path, which creates a pod with privileged: true, hostNetwork: true, hostPID: true, and runAsUser: 0. It also grants cluster-admin to the scanner service account. OO_SECURITY_CONTEXT: restricted only affects operator installation; it does not restrict this scanner pod. (raw.githubusercontent.com)
Use a supported restricted scanner mode for standard AWS clusters, or update the shared step before adding this workflow. If host mode is required, document and approve the exception.
#!/usr/bin/env bash
set -euo pipefail
fd -t f -g 'tls-scanner-run-ref.yaml' -g 'tls-scanner-run-commands.sh' \
-x rg -n -C 4 \
'TLS_SCANNER_CLUSTER_LABEL|privileged: true|hostNetwork|hostPID|runAsUser|cluster-admin|allowPrivilegeEscalation|runAsNonRoot|capabilities'As per path instructions, this YAML review must flag privileged: true, hostPID, hostNetwork, and root execution.
🤖 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/cert-manager-operator/openshift-cert-manager-operator-master.yaml`
around lines 468 - 469, The workflow references tls-scanner-run without
TLS_SCANNER_CLUSTER_LABEL, causing the scanner to use its privileged host-mode
configuration. Update this workflow to select a supported restricted scanner
mode for standard AWS clusters by configuring the required cluster label, or
remove the scanner reference until the shared step supports restricted
execution; do not enable host mode without documented approval.
There was a problem hiding this comment.
Could you let us know, where are you suggesting the change to be made?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
fd -t f -g 'tls-scanner-run-ref.yaml' -g 'tls-scanner-run-commands.sh' \
-x sh -c '
echo "===== $1 ====="
rg -n -C 5 "TLS_SCANNER_CLUSTER_LABEL|privileged: true|hostNetwork|hostPID|cluster-admin" "$1"
' sh {}Length of output: 682
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
step_dir='ci-operator/step-registry/tls/scanner/run'
echo '===== files ====='
fd -t f . "$step_dir"
echo '===== relevant configuration ====='
fd -t f . "$step_dir" -x rg -n -C 5 \
'TLS_SCANNER_CLUSTER_LABEL|privileged: true|hostNetwork|hostPID|cluster-admin|restricted'Length of output: 7878
@bharath-b-rh, the workflow-local location is the env: block in ci-operator/config/openshift/cert-manager-operator/openshift-cert-manager-operator-master.yaml, next to SCAN_NAMESPACE.
For example, a HyperShift workflow would set:
env:
TLS_SCANNER_CLUSTER_LABEL: guestHowever, I verified the current tls-scanner-run step. This is not a valid fix for this standard AWS workflow:
- The step accepts only
managementandguest. - Those values select HyperShift-specific kubeconfigs.
- An empty value uses the default host-mode scanner.
- The default host-mode scanner uses
privileged,hostNetwork, andhostPID.
Therefore, this job cannot select a supported restricted mode only by editing its env: block. The valid options are:
- Document and obtain approval for the required host-mode exception, or
- Update the shared
tls-scanner-runstep to support restricted execution for standard AWS clusters before using it here.
I should have stated this limitation in the original comment.
🧠 Learnings used
Learnt from: dustman9000
Repo: openshift/release PR: 80929
File: ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__ocm-fvt-rosa-hcp-production.yaml:31-39
Timestamp: 2026-06-24T02:25:16.245Z
Learning: In the openshift/release repo, when a PR makes structural changes to CI operator config (e.g., adding/renaming tests, changing the pre/post steps, or adding allow_best_effort_post_steps), require that the contributor regenerated the downstream Prow job definitions/metadata by running exactly: `make registry-metadata && make ci-operator-config && make jobs`. If the contributor confirms those commands were run successfully and the relevant CI checks passed, then treat the regeneration requirement as satisfied for review purposes.
You are interacting with an AI system.
|
/pj-rehearse pull-ci-openshift-cert-manager-operator-master-tls-scanner |
|
@arun717: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
arun717
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@arun717: This pull request explicitly references no jira issue. 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. |
|
/pj-rehearse pull-ci-openshift-cert-manager-operator-master-tls-scanner |
|
@arun717: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-cert-manager-operator-master-tls-scanner |
|
@arun717: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Replace the redundant CertManager CR apply with enabling the TrustManager feature gate via subscription, waiting for operator rollout, creating the TrustManager CR, and waiting for the trust-manager deployment.
Add oc wait --for=create for the cert-manager namespace and operand deployments before the existing Available checks in the TLS scanner job.
|
/pj-rehearse pull-ci-openshift-cert-manager-operator-master-tls-scanner |
|
@arun717: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
operator-sdk run bundle creates a versioned Subscription, so stop hardcoding cert-manager-operator and patch the Subscription that exists.
|
/pj-rehearse pull-ci-openshift-cert-manager-operator-master-tls-scanner |
|
@arun717: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Match e2e: poll UNSUPPORTED_ADDON_FEATURES on the operator deployment instead of a CLI arg that is never set by OLM.
Preserve existing Subscription env when enabling the feature gate, apply a minimal TrustManager CR without defaultCAPackage, and wait for Ready before scanning.
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse pull-ci-openshift-cert-manager-operator-master-tls-scanner |
|
@arun717: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@arun717: 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. |
| SUB=$(oc -n cert-manager-operator get subscriptions.operators.coreos.com -o jsonpath='{.items[0].metadata.name}') | ||
| if [[ -z "${SUB}" ]]; then | ||
| echo "No Subscription found in cert-manager-operator namespace" | ||
| oc -n cert-manager-operator get subscriptions.operators.coreos.com -o yaml || true |
There was a problem hiding this comment.
nit: what is the use of the command here?
| exit 1 | ||
| fi | ||
| echo "Patching Subscription ${SUB} (preserve existing env, match e2e patchSubscriptionWithEnvVars)" | ||
| PATCH=$(oc -n cert-manager-operator get "subscription/${SUB}" -o json | python3 -c 'import json,sys; sub=json.load(sys.stdin); cfg=(sub.get("spec") or {}).get("config") or {}; env=[e for e in (cfg.get("env") or []) if e.get("name")!="UNSUPPORTED_ADDON_FEATURES"]; env.append({"name":"UNSUPPORTED_ADDON_FEATURES","value":"TrustManager=true"}); print(json.dumps({"spec":{"config":{"env":env}}}))') |
There was a problem hiding this comment.
nit: it's always to be generic, but here I think it is not required, instead we could just patch it, since the subscription is fesh created. No harm having it though.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arun717, bharath-b-rh 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 |
openshift-ci
Bot
added
the
approved
|
The rehearse is expected to fail, since there are few changes which are being addressed in openshift/cert-manager-operator#466. Proceeding with the merge here and the workflow will be tested on the mentioned PR. /pj-rehearse ack |
|
@bharath-b-rh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
fb13548
into
openshift:main
…penshift#82715) * Add optional TLS scanner CI job for cert-manager-operator Add a manually triggered tls-scanner presubmit on master that installs the operator, deploys the CertManager operand, and runs tls-scanner-run against the cert-manager namespaces. * Enable TrustManager before cert-manager-operator TLS scanner Replace the redundant CertManager CR apply with enabling the TrustManager feature gate via subscription, waiting for operator rollout, creating the TrustManager CR, and waiting for the trust-manager deployment. * Wait for cert-manager namespace and deployments to be created Add oc wait --for=create for the cert-manager namespace and operand deployments before the existing Available checks in the TLS scanner job. * Discover cert-manager Subscription name before TrustManager patch operator-sdk run bundle creates a versioned Subscription, so stop hardcoding cert-manager-operator and patch the Subscription that exists. * Wait for TrustManager feature gate via deployment env Match e2e: poll UNSUPPORTED_ADDON_FEATURES on the operator deployment instead of a CLI arg that is never set by OLM. * Align TrustManager enablement with e2e helpers Preserve existing Subscription env when enabling the feature gate, apply a minimal TrustManager CR without defaultCAPackage, and wait for Ready before scanning.
4 participants
Summary
tls-scannerpresubmit foropenshift/cert-manager-operatoronmaster.CertManageroperand, enables TLS 1.3 strict adherence, and runstls-scanner-runagainst thecert-manager-operatorandcert-managernamespaces.Test plan
/pj-rehearse listconfirmspull-ci-openshift-cert-manager-operator-master-tls-scanneris rehearsable/pj-rehearse pull-ci-openshift-cert-manager-operator-master-tls-scannersucceedsmaster,/test tls-scannercan trigger the job after this mergesSummary by CodeRabbit
tls-scannerpresubmit job foropenshift/cert-manager-operatoronmaster.UNSUPPORTED_ADDON_FEATURESon the operator deployment.TrustManagerresource.tls-scanner-runtests against thecert-manager-operatorandcert-managernamespaces.tls-scanner-toolbase image required by the workflow.