telcov10n/dast: Mount GCS credential for RapidAST#79302
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds GCS credential handling for RapidAST DAST runs: pre-flight key check on the operator step host, create a Kubernetes Secret from the key, inject GCS settings into the RapidAST ConfigMap, and mount the secret into the RapidAST pod. ChangesGCS Integration for DAST Testing
sequenceDiagram
participant Operator as Operator step host
participant K8sAPI as Kubernetes API
participant Config as RapidAST ConfigMap
participant Pod as RapidAST Pod
Operator->>Operator: read GCS key file (`GCS_KEY_ON_STEP`)
Operator->>K8sAPI: create Secret `rapidast-gcs-credentials`
Operator->>Config: update ConfigMap with googleCloudStorage.keyFile
K8sAPI->>Pod: schedule pod with secret-backed volume `gcs-sa`
Pod->>Pod: mount secret at /var/run/secrets/gcs and run RapidAST
Estimated code review effort 🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: oblau 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
![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
🧹 Nitpick comments (2)
ci-operator/step-registry/telcov10n/functional/dast/tests/telcov10n-functional-dast-tests-commands.sh (2)
86-88: 💤 Low valueRestrict the GCS key file permissions on the mounted volume.
A service-account key is sensitive credential material; the secret volume should be projected with restrictive file mode. Kubernetes' default
defaultModefor secret volumes is0644, leaving the key world-readable inside the container. SettingdefaultMode: 0400(or0440) is a low-cost hardening step that pairs well with thereadOnly: trueyou already have on the mount.🔒 Proposed change
- name: gcs-sa secret: secretName: rapidast-gcs-credentials + defaultMode: 0400Also applies to: 102-104
🤖 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/step-registry/telcov10n/functional/dast/tests/telcov10n-functional-dast-tests-commands.sh` around lines 86 - 88, The secret volume for the GCS service account (volume name "gcs-sa" with secretName "rapidast-gcs-credentials") should set a restrictive file mode; update the Secret projection to include defaultMode: 0400 (or 0440) so the mounted key file is not world-readable, and apply the same defaultMode change to the other identical secret volume definition later in the file (the second "gcs-sa"/rapidast-gcs-credentials mount that currently uses readOnly: true).
43-46: ⚡ Quick winThe RapiDAST schema is correct as written; consider parameterizing the GCS bucket and directory.
The RapiDAST schema (from the official repository) confirms that
keyFile(camelCase),bucketName, anddirectoryare the correct field names underconfig.googleCloudStorage, matching your code at lines 44–46. No schema mismatch exists.However,
bucketName: secaut-bucketanddirectory: "telco"are hardcoded. Promote these to step environment variables (with sensible defaults) so downstream teams can retarget the bucket without forking this file.🤖 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/step-registry/telcov10n/functional/dast/tests/telcov10n-functional-dast-tests-commands.sh` around lines 43 - 46, The googleCloudStorage block currently hardcodes bucketName and directory; change them to read from step environment variables (e.g., RAPI_DAST_GCS_BUCKET with default "secaut-bucket" and RAPI_DAST_GCS_DIR with default "telco") while keeping keyFile configured from ${GCS_KEY_ON_POD}; update the config.googleCloudStorage.bucketName and config.googleCloudStorage.directory references to use those env vars so downstream consumers can override the target GCS bucket and directory without forking.
🤖 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/telcov10n/functional/dast/tests/telcov10n-functional-dast-tests-commands.sh`:
- Around line 18-24: Enable strict failure handling and make the secret creation
idempotent: add "set -euo pipefail" (or set -o errexit) at the top of the script
so any failing oc command bubbles up, and replace the oc create secret generic
rapidast-gcs-credentials --from-file="${GCS_KEY_NAME}=${GCS_KEY_ON_STEP}"
invocation with the dry-run/apply pattern (e.g. generate the secret YAML with oc
create secret ... --dry-run -o yaml and pipe to oc apply -f -) so the secret is
reconciled if it already exists instead of failing on AlreadyExists; keep the
existing pre-flight check of GCS_KEY_ON_STEP and ensure other oc calls (project
creation, configmap, pod apply) also fail fast under the new errexit behavior.
---
Nitpick comments:
In
`@ci-operator/step-registry/telcov10n/functional/dast/tests/telcov10n-functional-dast-tests-commands.sh`:
- Around line 86-88: The secret volume for the GCS service account (volume name
"gcs-sa" with secretName "rapidast-gcs-credentials") should set a restrictive
file mode; update the Secret projection to include defaultMode: 0400 (or 0440)
so the mounted key file is not world-readable, and apply the same defaultMode
change to the other identical secret volume definition later in the file (the
second "gcs-sa"/rapidast-gcs-credentials mount that currently uses readOnly:
true).
- Around line 43-46: The googleCloudStorage block currently hardcodes bucketName
and directory; change them to read from step environment variables (e.g.,
RAPI_DAST_GCS_BUCKET with default "secaut-bucket" and RAPI_DAST_GCS_DIR with
default "telco") while keeping keyFile configured from ${GCS_KEY_ON_POD}; update
the config.googleCloudStorage.bucketName and config.googleCloudStorage.directory
references to use those env vars so downstream consumers can override the target
GCS bucket and directory without forking.
🪄 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: a994ff70-0d8a-4782-8e37-7109d96029a1
📒 Files selected for processing (2)
ci-operator/step-registry/telcov10n/functional/dast/tests/telcov10n-functional-dast-tests-commands.shci-operator/step-registry/telcov10n/functional/dast/tests/telcov10n-functional-dast-tests-ref.yaml
| # Copy key from this step → Secret on the test cluster so RapidAST pods can mount it | ||
| if [[ ! -r "${GCS_KEY_ON_STEP}" ]]; then | ||
| echo "ERROR: GCS key not found at ${GCS_KEY_ON_STEP} (check Vault sync)" | ||
| exit 1 | ||
| fi | ||
|
|
||
| oc create secret generic rapidast-gcs-credentials --from-file="${GCS_KEY_NAME}=${GCS_KEY_ON_STEP}" -n dast |
There was a problem hiding this comment.
Make secret creation idempotent and fail fast on errors.
Two related concerns on this new block:
oc create secret genericis not idempotent — if this step is retried (or run against a cluster where thedastproject + secret already exist) the command returns a non-zero exit code on "AlreadyExists". Because the script only setsnounset/pipefail(noerrexit), the failure is silently swallowed and the loop proceeds to create pods that may mount a stale secret. Prefer thedry-run | applypattern so the secret is reconciled regardless of prior state.- The pre-flight check correctly exits when the key file is missing, but every subsequent
occall (project creation, secret creation, configmap apply, pod apply) lacks the same explicit guarding. Consider enablingset -o errexitat the top of the file so genuine failures bubble up rather than being papered over by the per-podoc waitlater.
🛠️ Proposed change for idempotency
-oc create secret generic rapidast-gcs-credentials --from-file="${GCS_KEY_NAME}=${GCS_KEY_ON_STEP}" -n dast
+oc create secret generic rapidast-gcs-credentials \
+ --from-file="${GCS_KEY_NAME}=${GCS_KEY_ON_STEP}" \
+ -n dast --dry-run=client -o yaml | oc apply -f -As per coding guidelines: "Step registry step definitions … with the command script using set -euo pipefail as default".
📝 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.
| # Copy key from this step → Secret on the test cluster so RapidAST pods can mount it | |
| if [[ ! -r "${GCS_KEY_ON_STEP}" ]]; then | |
| echo "ERROR: GCS key not found at ${GCS_KEY_ON_STEP} (check Vault sync)" | |
| exit 1 | |
| fi | |
| oc create secret generic rapidast-gcs-credentials --from-file="${GCS_KEY_NAME}=${GCS_KEY_ON_STEP}" -n dast | |
| # Copy key from this step → Secret on the test cluster so RapidAST pods can mount it | |
| if [[ ! -r "${GCS_KEY_ON_STEP}" ]]; then | |
| echo "ERROR: GCS key not found at ${GCS_KEY_ON_STEP} (check Vault sync)" | |
| exit 1 | |
| fi | |
| oc create secret generic rapidast-gcs-credentials \ | |
| --from-file="${GCS_KEY_NAME}=${GCS_KEY_ON_STEP}" \ | |
| -n dast --dry-run=client -o yaml | oc apply -f - |
🤖 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/step-registry/telcov10n/functional/dast/tests/telcov10n-functional-dast-tests-commands.sh`
around lines 18 - 24, Enable strict failure handling and make the secret
creation idempotent: add "set -euo pipefail" (or set -o errexit) at the top of
the script so any failing oc command bubbles up, and replace the oc create
secret generic rapidast-gcs-credentials
--from-file="${GCS_KEY_NAME}=${GCS_KEY_ON_STEP}" invocation with the
dry-run/apply pattern (e.g. generate the secret YAML with oc create secret ...
--dry-run -o yaml and pipe to oc apply -f -) so the secret is reconciled if it
already exists instead of failing on AlreadyExists; keep the existing pre-flight
check of GCS_KEY_ON_STEP and ensure other oc calls (project creation, configmap,
pod apply) also fail fast under the new errexit behavior.
oblau
force-pushed
the
enable-dast-reporting
branch
from
d6bedba to
714a6b4
Compare
Signed-off-by: oblau <oblau@redhat.com>
oblau
force-pushed
the
enable-dast-reporting
branch
from
714a6b4 to
dd67b72
Compare
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-telco-operators-dast-4.21-telco-dast-operators-ci |
|
@oblau: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-telco-operators-dast-4.21-telco-dast-operators-ci |
|
@oblau: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@oblau: all tests passed! 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
Mount Vault-synced GCS service account into the DAST step and forward it to the RapidAST pod so scan results can upload to secaut-bucket.
Summary by CodeRabbit
New Features
Bug Fixes / Reliability