OCPERT-368: Replace GitHub token with Github app for Jira Notificator#79905
OCPERT-368: Replace GitHub token with Github app for Jira Notificator#79905tomasdavidorg wants to merge 1 commit into
Conversation
rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
@tomasdavidorg: This pull request references OCPERT-368 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 sub-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: tomasdavidorg 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 |
WalkthroughThis PR updates the jira-notificator step script to provision GitHub App reader credentials from Vault instead of a GitHub token. The credential swap removes ChangesGitHub App Reader Credential Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 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)
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 |
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
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/step-registry/release-qe-tests/jira-notificator/release-qe-tests-jira-notificator-commands.sh`:
- Around line 8-9: The script currently sets github_app_reader_private_key to a
file path and exports GITHUB_APP_READER_PRIVATE_KEY with that path string;
instead read the actual private key contents from the file and export that
content. Replace the path-assignment/export pattern so that the variable
github_app_reader_private_key is used to read the file contents (preserving
newlines and avoiding word-splitting) and then export
GITHUB_APP_READER_PRIVATE_KEY with the file's content (not the path); ensure you
handle missing file errors and quoting when assigning in the script where
GITHUB_APP_READER_PRIVATE_KEY and github_app_reader_private_key are referenced.
- Around line 6-9: The secret reads/exports for
github_app_reader_id/GITHUB_APP_READER_ID and
github_app_reader_private_key/GITHUB_APP_READER_PRIVATE_KEY must be wrapped in a
saved-and-restored xtrace block: capture the current xtrace state (e.g., save
$(set +x; echo $-)), disable tracing with set +x, perform the cat/read and
export operations for the two variables, and then restore the original xtrace
state so tracing is returned to its prior setting; apply this around the code
that assigns github_app_reader_id, GITHUB_APP_READER_ID,
github_app_reader_private_key and GITHUB_APP_READER_PRIVATE_KEY.
🪄 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: 26ca1941-e75b-4e04-8c1a-6484ddceafb1
📒 Files selected for processing (1)
ci-operator/step-registry/release-qe-tests/jira-notificator/release-qe-tests-jira-notificator-commands.sh
| github_app_reader_id=$(cat "/var/run/vault/release-tests-token/github_app_reader_id") | ||
| export GITHUB_APP_READER_ID=$github_app_reader_id | ||
| github_app_reader_private_key="/var/run/vault/release-tests-token/github_app_reader_private_key" | ||
| export GITHUB_APP_READER_PRIVATE_KEY=$github_app_reader_private_key |
There was a problem hiding this comment.
Guard sensitive reads/exports with xtrace disable + restore.
This script handles Vault-backed secrets, but Line 6-9 do not save/restore xtrace state and force set +x during secret handling. Please wrap the sensitive block accordingly.
Suggested hardening
+had_xtrace=0
+case "$-" in
+ *x*) had_xtrace=1; set +x ;;
+esac
+
github_app_reader_id=$(cat "/var/run/vault/release-tests-token/github_app_reader_id")
-export GITHUB_APP_READER_ID=$github_app_reader_id
-github_app_reader_private_key="/var/run/vault/release-tests-token/github_app_reader_private_key"
-export GITHUB_APP_READER_PRIVATE_KEY=$github_app_reader_private_key
+export GITHUB_APP_READER_ID="$github_app_reader_id"
+github_app_reader_private_key=$(cat "/var/run/vault/release-tests-token/github_app_reader_private_key")
+export GITHUB_APP_READER_PRIVATE_KEY="$github_app_reader_private_key"
+
+if [[ $had_xtrace -eq 1 ]]; then
+ set -x
+fiAs per coding guidelines, "In step registry scripts handling sensitive data, temporarily disable tracing with set +x and save/restore previous tracing state to prevent credential leakage in logs."
🤖 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/release-qe-tests/jira-notificator/release-qe-tests-jira-notificator-commands.sh`
around lines 6 - 9, The secret reads/exports for
github_app_reader_id/GITHUB_APP_READER_ID and
github_app_reader_private_key/GITHUB_APP_READER_PRIVATE_KEY must be wrapped in a
saved-and-restored xtrace block: capture the current xtrace state (e.g., save
$(set +x; echo $-)), disable tracing with set +x, perform the cat/read and
export operations for the two variables, and then restore the original xtrace
state so tracing is returned to its prior setting; apply this around the code
that assigns github_app_reader_id, GITHUB_APP_READER_ID,
github_app_reader_private_key and GITHUB_APP_READER_PRIVATE_KEY.
| github_app_reader_private_key="/var/run/vault/release-tests-token/github_app_reader_private_key" | ||
| export GITHUB_APP_READER_PRIVATE_KEY=$github_app_reader_private_key |
There was a problem hiding this comment.
GITHUB_APP_READER_PRIVATE_KEY is exporting a file path, not the private key value.
Line 8 assigns the path string, while the PR objective says this credential should be read from Vault and exported. This can break auth if oarctl jira-notificator expects actual key content in GITHUB_APP_READER_PRIVATE_KEY.
Suggested fix
-github_app_reader_private_key="/var/run/vault/release-tests-token/github_app_reader_private_key"
-export GITHUB_APP_READER_PRIVATE_KEY=$github_app_reader_private_key
+github_app_reader_private_key=$(cat "/var/run/vault/release-tests-token/github_app_reader_private_key")
+export GITHUB_APP_READER_PRIVATE_KEY="$github_app_reader_private_key"🤖 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/release-qe-tests/jira-notificator/release-qe-tests-jira-notificator-commands.sh`
around lines 8 - 9, The script currently sets github_app_reader_private_key to a
file path and exports GITHUB_APP_READER_PRIVATE_KEY with that path string;
instead read the actual private key contents from the file and export that
content. Replace the path-assignment/export pattern so that the variable
github_app_reader_private_key is used to read the file contents (preserving
newlines and avoiding word-splitting) and then export
GITHUB_APP_READER_PRIVATE_KEY with the file's content (not the path); ensure you
handle missing file errors and quoting when assigning in the script where
GITHUB_APP_READER_PRIVATE_KEY and github_app_reader_private_key are referenced.
|
@tomasdavidorg: 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. |
|
Reopened in #79915 |
rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
https://redhat.atlassian.net/browse/OCPERT-368