OSAC-794: add Slack notification step to step registry#79364
Conversation
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. |
openshift-ci
Bot
added
the
approved
|
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 selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds an OSAC CI Slack notification step: a step registry entry and metadata, a bash command script that reads a Vault-stored webhook and posts job completion info to Slack, and an OWNERS file assigning ChangesOSAC Project Slack Notification Step
sequenceDiagram
participant Script as osac-project-notify-commands.sh
participant Vault as Vault (/var/run/vault/osac-slack-webhook/url)
participant Prow as Prow (logs)
participant Slack as Slack Incoming Webhook
Script->>Vault: read WEBHOOK_URL
Script->>Prow: build logs URL from JOB_NAME and BUILD_ID
Script->>Slack: POST JSON (job, build, logs link) to WEBHOOK_URL
🎯 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)
Comment |
openshift-merge-bot
Bot
added
the
rehearsals-ack
![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.
…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
| 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 |
There was a problem hiding this comment.
I don't think we should send messages for presubmits, it will spam the channel
There was a problem hiding this comment.
We don't send messages for presubmits, it will be integrated only for periodics, but I'll remove it
omer-vishlitzky
force-pushed
the
osac-slack-notify
branch
from
e1cc7cf to
bf27f63
Compare
There was a problem hiding this comment.
Don't we want to exit early if the job is not periodic ?
There was a problem hiding this comment.
why the user care about build ID ?
There was a problem hiding this comment.
what about saying if it failed or succeeded ?
danmanor
left a comment
danmanor
left a comment
There was a problem hiding this comment.
Prow has a built in mechanism for doing that, see https://docs.ci.openshift.org/how-tos/notification/
…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
Add osac-project-notify step-registry ref that posts job results to Slack via incoming webhook. Not wired into any workflow yet.
omer-vishlitzky
force-pushed
the
osac-slack-notify
branch
from
bf27f63 to
f523499
Compare
…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
|
[REHEARSALNOTIFIER] Note: If this PR includes changes to step registry files ( Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danmanor, 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 |
…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
Add osac-project-notify step-registry ref that posts job results to Slack via incoming webhook. Not wired into any workflow yet.
3 participants
Summary
osac-project-notifystep-registry ref that posts job results to Slack via incoming webhookTest plan
Summary
This PR adds a Slack notification step to the OpenShift CI step registry for the OSAC project. It introduces a reusable step, osac-project-notify, which jobs can run to post a completion message and a link to Prow logs to Slack via an incoming webhook.
What changed (practical impact)
Files and behavior
ci-operator/step-registry/osac-project/notify/osac-project-notify-ref.yaml
ci-operator/step-registry/osac-project/notify/osac-project-notify-commands.sh
ci-operator/step-registry/osac-project/notify/OWNERS and osac-project-notify-ref.metadata.json
Integration & testing
Notes / review items