OSAC-794: add Slack notification step to step registry#79364
OSAC-794: add Slack notification step to step registry#79364omer-vishlitzky wants to merge 1 commit into
Conversation
Add osac-project-notify step-registry ref that posts job results to Slack via incoming webhook. Not wired into any workflow yet.
openshift-ci-robot
added
the
jira/valid-reference
|
@omer-vishlitzky: This pull request references OSAC-865 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 task 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: omer-vishlitzky 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
WalkthroughThis PR introduces a new Slack notification step for OSAC CI pipelines. It registers a new step in the CI operator step registry, implements a bash script that reads webhook credentials from Vault and posts job completion notifications to Slack, and establishes ownership for the new step configuration. ChangesOSAC Project Slack Notification Step
🎯 1 (Trivial) | ⏱️ ~3 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)
Comment |
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
[REHEARSALNOTIFIER] Note: If this PR includes changes to step registry files ( |
![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/step-registry/osac-project/notify/osac-project-notify-commands.sh`:
- Around line 3-17: The script currently uses set -o nounset and set -o pipefail
but should use set -euo pipefail; replace those options accordingly, and wrap
the sensitive operations that read WEBHOOK_URL and call curl (the WEBHOOK_URL
assignment and the curl invocation) with tracing disabled (use set +x before and
restore previous errexit/tracing state after) to avoid leaking the webhook; also
add curl --fail and check its exit status (or let -e cause exit) so notification
failures are not silent—refer to the WEBHOOK_URL variable assignment and the
curl command to locate the changes.
🪄 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: 2913b073-472d-4ad1-bcd0-4bc8c5ef67c2
📒 Files selected for processing (4)
ci-operator/step-registry/osac-project/notify/OWNERSci-operator/step-registry/osac-project/notify/osac-project-notify-commands.shci-operator/step-registry/osac-project/notify/osac-project-notify-ref.metadata.jsonci-operator/step-registry/osac-project/notify/osac-project-notify-ref.yaml
| set -o nounset | ||
| set -o pipefail | ||
|
|
||
| WEBHOOK_URL="$(cat /var/run/vault/osac-slack-webhook/url)" | ||
| PROW_URL="https://prow.ci.openshift.org/view/gs/test-platform-results" | ||
|
|
||
| if [[ "${JOB_TYPE:-}" == "presubmit" ]]; then | ||
| JOB_URL="${PROW_URL}/pr-logs/pull/${REPO_OWNER}_${REPO_NAME}/${PULL_NUMBER}/${JOB_NAME}/${BUILD_ID}" | ||
| else | ||
| JOB_URL="${PROW_URL}/logs/${JOB_NAME}/${BUILD_ID}" | ||
| fi | ||
|
|
||
| curl -s -X POST -H 'Content-type: application/json' \ | ||
| --data "{\"text\":\"Job *${JOB_NAME}* #${BUILD_ID} completed.\n<${JOB_URL}|View logs>\"}" \ | ||
| "${WEBHOOK_URL}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -type f -name "*osac-project-notify-commands.sh"Repository: openshift/release
Length of output: 232
🏁 Script executed:
fd -t f "osac-project-notify-commands.sh"Repository: openshift/release
Length of output: 139
🏁 Script executed:
cat -n ci-operator/step-registry/osac-project/notify/osac-project-notify-commands.shRepository: openshift/release
Length of output: 742
🏁 Script executed:
find . -type f -name "*.md" | xargs grep -l "step-registry" | head -5Repository: openshift/release
Length of output: 285
🏁 Script executed:
find . -path "*/step-registry/*commands.sh" -type f | head -10 | xargs head -5Repository: openshift/release
Length of output: 1687
🏁 Script executed:
# Check how many step-registry scripts use set -euo/set -e instead of individual flags
find . -path "*/step-registry/*commands.sh" -type f | wc -l
echo "---"
find . -path "*/step-registry/*commands.sh" -type f -exec grep -l "set -euo\|set -e " {} \; | wc -l
echo "---"
# Check for set +x/set -x patterns around sensitive operations
find . -path "*/step-registry/*commands.sh" -type f -exec grep -l "set +x\|set -x" {} \; | head -10Repository: openshift/release
Length of output: 22016
🏁 Script executed:
# Check what pattern is standard in step-registry - look at recent examples
rg "set -euo\|set -e " ci-operator/step-registry/ -A1 -B1 | head -50Repository: openshift/release
Length of output: 43
🏁 Script executed:
# Check the specific file and check what set options are most common in step-registry
rg "^set -" ci-operator/step-registry/ -c | sort | uniq -c | sort -rn | head -20Repository: openshift/release
Length of output: 2149
🏁 Script executed:
# Let's look at actual set patterns in step-registry files
rg "^set -" ci-operator/step-registry/ --no-heading | head -50Repository: openshift/release
Length of output: 6491
🏁 Script executed:
# Look at examples of how other scripts handle sensitive data with set +x
rg -A5 "set \+x" ci-operator/step-registry/ | head -60Repository: openshift/release
Length of output: 6421
🏁 Script executed:
# Check if there are existing examples in step-registry of saving/restoring xtrace state with proper patterns
rg -B3 -A10 "xtrace_was_on|xtrace_state" ci-operator/step-registry/Repository: openshift/release
Length of output: 43
🏁 Script executed:
# Look for examples of how scripts handle curl and error checking
rg "curl.*-s" ci-operator/step-registry/ | head -20Repository: openshift/release
Length of output: 4354
Use set -euo pipefail and disable tracing around sensitive operations.
Lines 3-4 should consolidate to set -euo pipefail. Disable tracing with set +x around the webhook URL read and curl call to prevent credential leakage in CI logs. Additionally, add error checking to curl with --fail to prevent silent failures when the notification cannot be delivered.
Suggested changes
-set -o nounset
-set -o pipefail
+set -euo pipefail
+set +x
-WEBHOOK_URL="$(cat /var/run/vault/osac-slack-webhook/url)"
+WEBHOOK_URL="$(< /var/run/vault/osac-slack-webhook/url)"
PROW_URL="https://prow.ci.openshift.org/view/gs/test-platform-results"
+set -x
if [[ "${JOB_TYPE:-}" == "presubmit" ]]; then
JOB_URL="${PROW_URL}/pr-logs/pull/${REPO_OWNER}_${REPO_NAME}/${PULL_NUMBER}/${JOB_NAME}/${BUILD_ID}"
else
JOB_URL="${PROW_URL}/logs/${JOB_NAME}/${BUILD_ID}"
fi
-curl -s -X POST -H 'Content-type: application/json' \
+set +x
+curl --fail -s -X POST -H 'Content-type: application/json' \
--data "{\"text\":\"Job *${JOB_NAME}* #${BUILD_ID} completed.\n<${JOB_URL}|View logs>\"}" \
"${WEBHOOK_URL}"
+set -xPer coding guidelines, step-registry scripts should use set -euo pipefail and protect sensitive operations from being logged.
🤖 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/osac-project/notify/osac-project-notify-commands.sh`
around lines 3 - 17, The script currently uses set -o nounset and set -o
pipefail but should use set -euo pipefail; replace those options accordingly,
and wrap the sensitive operations that read WEBHOOK_URL and call curl (the
WEBHOOK_URL assignment and the curl invocation) with tracing disabled (use set
+x before and restore previous errexit/tracing state after) to avoid leaking the
webhook; also add curl --fail and check its exit status (or let -e cause exit)
so notification failures are not silent—refer to the WEBHOOK_URL variable
assignment and the curl command to locate the changes.
|
@omer-vishlitzky: 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. |
…tion Add a nightly periodic job that installs OSAC with ALL component images overridden to their latest CI-built versions, runs all vmaas tests, and sends results to Slack with pass/fail status and component versions. New step-registry components: - osac-project-installer-all-latest: installs OSAC with fulfillment-service, osac-operator, and osac-aap images all swapped to latest - osac-project-baremetal-test-all: runs make test-vmaas (all tests) - osac-project-ofcir-baremetal-nightly: dedicated nightly workflow - osac-project-notify: Slack webhook notification (from PR openshift#79364) Removes unused dead code: - osac-project-installer-component: single-image swap, replaced by all-latest - osac-project-ofcir-baremetal-component: workflow using installer-component
…tion Add a nightly periodic job that installs OSAC with ALL component images overridden to their latest CI-built versions, runs all vmaas tests, and sends results to Slack with pass/fail status and component versions. New step-registry components: - osac-project-installer-all-latest: installs OSAC with fulfillment-service, osac-operator, and osac-aap images all swapped to latest - osac-project-baremetal-test-all: runs make test-vmaas (all tests) - osac-project-ofcir-baremetal-nightly: dedicated nightly workflow - osac-project-notify: Slack webhook notification (from PR openshift#79364) Removes unused dead code: - osac-project-installer-component: single-image swap, replaced by all-latest - osac-project-ofcir-baremetal-component: workflow using installer-component
omer-vishlitzky
changed the title
|
@omer-vishlitzky: This pull request references OSAC-794 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 task 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. |
|
/jira refresh |
|
@omer-vishlitzky: This pull request references OSAC-794 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 task 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. |
…tion Add a nightly periodic job that installs OSAC with ALL component images overridden to their latest CI-built versions, runs all vmaas tests, and sends results to Slack with pass/fail status and component versions. New step-registry components: - osac-project-installer-all-latest: installs OSAC with fulfillment-service, osac-operator, and osac-aap images all swapped to latest - osac-project-baremetal-test-all: runs make test-vmaas (all tests) - osac-project-ofcir-baremetal-nightly: dedicated nightly workflow - osac-project-notify: Slack webhook notification (from PR openshift#79364) Removes unused dead code: - osac-project-installer-component: single-image swap, replaced by all-latest - osac-project-ofcir-baremetal-component: workflow using installer-component
2 participants
Summary
osac-project-notifystep-registry ref that posts job results to Slack via incoming webhookTest plan
Summary
This PR adds a new Slack notification capability to OpenShift CI's step registry. The change introduces a new step named
osac-project-notifythat enables CI jobs to post their completion status to Slack via an incoming webhook.Changes Made
A complete notification step has been added to the OSAC project's step registry:
Step Configuration (
osac-project-notify-ref.yaml): Defines a new CI step that runs asbest_effortwith a 5-minute timeout. The step executes a bash script to notify Slack, requests 100m CPU and 200Mi memory, and securely retrieves the webhook URL from a Vault-mounted credential (osac-slack-webhook). The step includes documentation explaining it sends the job result and a link to Prow logs.Notification Logic (
osac-project-notify-commands.sh): Implements the Slack notification by reading the webhook URL from Vault, determining the correct Prow logs URL based on the job type (distinguishing between presubmit pull logs and generic job logs), and posting a JSON message containing the job name, build ID, and logs link.Access Control: Both the OWNERS file and metadata configuration establish
osac-cicdas the approvers and reviewers for this new step.Integration Status
The new step is not yet integrated into any workflows. According to the PR description, integration with periodic jobs will be handled in a follow-up change.
Testing
The webhook has been tested manually and via rehearsal, with successful Slack message delivery confirmed.