Add TLS scanner for External Secrets Operator - #82876
Conversation
|
/pj-rehearse |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: siddhibhor-56 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 |
|
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:
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)
WalkthroughThe configuration adds base images and an optional AWS workflow. The workflow deploys the External Secrets operator, validates resource reconciliation, creates TLS materials, enables the Bitwarden plugin, and runs TLS 1.3 checks. ChangesExternal Secrets TLS validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant AWSWorkflow
participant OperatorSDK
participant ExternalSecretsOperator
participant SecretStore
participant ExternalSecret
participant BitwardenPlugin
participant TLSScannerTool
AWSWorkflow->>OperatorSDK: Install operator and deploy operands
OperatorSDK->>ExternalSecretsOperator: Start operator components
AWSWorkflow->>SecretStore: Create test SecretStore
AWSWorkflow->>ExternalSecret: Create test ExternalSecret
ExternalSecret->>ExternalSecretsOperator: Reconcile test resource
ExternalSecretsOperator-->>AWSWorkflow: Report successful reconciliation
AWSWorkflow->>ExternalSecretsOperator: Create TLS secret and enable Bitwarden plugin
ExternalSecretsOperator->>BitwardenPlugin: Roll out Bitwarden SDK server
AWSWorkflow->>TLSScannerTool: Scan TLS-relevant services with TLS 1.3
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: 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/external-secrets-operator/openshift-external-secrets-operator-main.yaml`:
- Around line 314-325: Update the webhook verification block around WEBHOOK_SVC
and WEBHOOK_NS to fail with a nonzero status when the
ValidatingWebhookConfiguration, service, or endpoints cannot be discovered or
verified, instead of suppressing errors with true. Also update the
reconciliation readiness check around the Ready condition to fail when Ready
never becomes True, ensuring tls-scanner-run cannot succeed without exercising
both webhook and controller TLS paths.
🪄 Autofix
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: e068a18f-62ee-4e95-ab29-70cb421ae1e9
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/external-secrets-operator/openshift-external-secrets-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml
| echo "Verifying webhook TLS endpoint is live..." | ||
| WEBHOOK_SVC=$(oc get validatingwebhookconfigurations -l app=external-secrets -o jsonpath='{.items[0].webhooks[0].clientConfig.service.name}' 2>/dev/null || true) | ||
| WEBHOOK_NS=$(oc get validatingwebhookconfigurations -l app=external-secrets -o jsonpath='{.items[0].webhooks[0].clientConfig.service.namespace}' 2>/dev/null || true) | ||
| if [[ -n "${WEBHOOK_SVC}" && -n "${WEBHOOK_NS}" ]]; then | ||
| WEBHOOK_PORT=$(oc get validatingwebhookconfigurations -l app=external-secrets -o jsonpath='{.items[0].webhooks[0].clientConfig.service.port}' 2>/dev/null || echo "443") | ||
| echo "Found webhook service: ${WEBHOOK_NS}/${WEBHOOK_SVC}:${WEBHOOK_PORT}" | ||
| oc get svc -n "${WEBHOOK_NS}" "${WEBHOOK_SVC}" || true | ||
| oc get endpoints -n "${WEBHOOK_NS}" "${WEBHOOK_SVC}" || true | ||
| else | ||
| echo "Warning: could not discover ValidatingWebhookConfiguration with label app=external-secrets" | ||
| oc get validatingwebhookconfigurations -o name || true | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail when the TLS paths are not exercised.
The webhook checks ignore missing configuration, services, and endpoints. The reconciliation loop also exits successfully when Ready never becomes True.
The workflow can therefore run tls-scanner-run and pass without proving that the webhook and controller TLS paths were exercised. Return a nonzero status when either verification fails.
Proposed failure checks
if [[ -n "${WEBHOOK_SVC}" && -n "${WEBHOOK_NS}" ]]; then
WEBHOOK_PORT=$(oc get validatingwebhookconfigurations -l app=external-secrets -o jsonpath='{.items[0].webhooks[0].clientConfig.service.port}' 2>/dev/null || echo "443")
echo "Found webhook service: ${WEBHOOK_NS}/${WEBHOOK_SVC}:${WEBHOOK_PORT}"
- oc get svc -n "${WEBHOOK_NS}" "${WEBHOOK_SVC}" || true
- oc get endpoints -n "${WEBHOOK_NS}" "${WEBHOOK_SVC}" || true
+ oc get svc -n "${WEBHOOK_NS}" "${WEBHOOK_SVC}"
+ ENDPOINT_IPS=$(oc get endpoints -n "${WEBHOOK_NS}" "${WEBHOOK_SVC}" -o jsonpath='{.subsets[*].addresses[*].ip}')
+ [[ -n "${ENDPOINT_IPS}" ]] || {
+ echo "Webhook service has no ready endpoints" >&2
+ exit 1
+ }
else
- echo "Warning: could not discover ValidatingWebhookConfiguration with label app=external-secrets"
+ echo "Could not discover ValidatingWebhookConfiguration with label app=external-secrets" >&2
oc get validatingwebhookconfigurations -o name || true
+ exit 1
fi
...
done
+ if [[ "${STATUS}" != "True" ]]; then
+ oc describe externalsecret tls-scan-dummy -n tls-scanner-exercise || true
+ exit 1
+ fiAlso applies to: 363-372
🤖 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/external-secrets-operator/openshift-external-secrets-operator-main.yaml`
around lines 314 - 325, Update the webhook verification block around WEBHOOK_SVC
and WEBHOOK_NS to fail with a nonzero status when the
ValidatingWebhookConfiguration, service, or endpoints cannot be discovered or
verified, instead of suppressing errors with true. Also update the
reconciliation readiness check around the Ready condition to fail when Ready
never becomes True, ensuring tls-scanner-run cannot succeed without exercising
both webhook and controller TLS paths.
2f6ca1f to
8874ee8
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/external-secrets-operator/openshift-external-secrets-operator-main.yaml`:
- Around line 258-260: The tls-scanner job has been added to the configuration
in the openshift-external-secrets-operator-main.yaml file (with as:
tls-scanner), but the corresponding generated Prow job artifacts and metadata
have not been created. Run make update in the repository root to generate the
Prow job configurations for the tls-scanner job, then commit all generated files
and configuration changes together. Without these generated artifacts, Prow
cannot properly register the optional job after merge.
🪄 Autofix
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: fbb3f5c2-2dfc-4e7e-b204-a2f43f400365
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/external-secrets-operator/openshift-external-secrets-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml
|
/pj-rehearse |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
8874ee8 to
bfc1292
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/pj-rehearse |
|
@siddhibhor-56: 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/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml`:
- Around line 415-425: Update the oc patch payload in the Bitwarden plugin
configuration so TLS_SECRET_NAME is expanded by the shell before the request is
sent, while preserving valid JSON quoting and the existing secretRef structure.
Ensure ExternalSecretsConfig receives the actual secret name, such as
bitwarden-tls-cert, rather than the literal placeholder.
🪄 Autofix
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: 6276b220-9133-4e5e-ae65-59c64c117da9
📒 Files selected for processing (1)
ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml
|
/pj-rehearse |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/retest |
|
/pj-rehearse pull-ci-openshift-external-secrets-operator-main-e2e-operator-coverage |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@siddhibhor-56: requesting more than one rehearsal in one comment is not supported. If you would like to rehearse multiple specific jobs, please separate the job names by a space in a single command. |
1 similar comment
|
@siddhibhor-56: requesting more than one rehearsal in one comment is not supported. If you would like to rehearse multiple specific jobs, please separate the job names by a space in a single command. |
ce7436c to
561d99f
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@siddhibhor-56: 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. |
|
/pj-rehearse pull-ci-openshift-external-secrets-operator-main-tls-scanner |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
1 participant
Summary by CodeRabbit
Adds an optional, manually triggered AWS TLS scanner presubmit for
openshift/external-secrets-operatoronmaster.The workflow installs the operator, deploys operands, verifies webhook services, creates test resources, configures TLS materials and the Bitwarden plugin, and runs
tls-scanner-runagainst relevant namespaces.