USHIFT-7211: MicroShift Manage Versions & Releases: Fix credentials for common versions update#81440
USHIFT-7211: MicroShift Manage Versions & Releases: Fix credentials for common versions update#81440pmtk wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pmtk 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 |
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe generated release script now exports ChangesMicroshift release script
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh`:
- Around line 8-12: The rehearse dry-run handling in the
manage-versions-releases script was disabled, so `DRY_RUN` never gets set for
rehearse jobs. Restore the `JOB_NAME == rehearse*` guard in
`openshift-microshift-manage-versions-releases-commands.sh` so the step sets
`DRY_RUN="--dry-run"` only for rehearse runs; keep the existing `DRY_RUN`
variable and its use in the `publish`/`--create-pr` flow intact.
🪄 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: 01ff677d-6f88-46b0-ac3f-41538bcc85a0
📒 Files selected for processing (1)
ci-operator/step-registry/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh
| DRY_RUN="" | ||
| if [[ "$JOB_NAME" == rehearse* ]]; then | ||
| echo "INFO: \$JOB_NAME starts with rehearse - running in DRY RUN mode" | ||
| DRY_RUN="--dry-run" | ||
| fi | ||
| #if [[ "$JOB_NAME" == rehearse* ]]; then | ||
| # echo "INFO: \$JOB_NAME starts with rehearse - running in DRY RUN mode" | ||
| # DRY_RUN="--dry-run" | ||
| #fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='ci-operator/step-registry/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh'
sibling='ci-operator/step-registry/openshift/microshift/publish-release-notes/openshift-microshift-publish-release-notes-commands.sh'
echo "== $file =="
cat -n "$file" | sed -n '1,140p'
echo
echo "== $sibling =="
cat -n "$sibling" | sed -n '1,120p'
echo
echo "== DRY_RUN references =="
rg -n 'DRY_RUN|rehearse\*|dry-run|create-pr|publish ' "$file" "$sibling"Repository: openshift/release
Length of output: 7278
Restore rehearse dry-run handling
DRY_RUN is now always empty, so rehearse jobs will take the real publish and --create-pr paths instead of dry-run mode. Keep the JOB_NAME == rehearse* guard here unless this step never runs under rehearses.
🤖 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/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh`
around lines 8 - 12, The rehearse dry-run handling in the
manage-versions-releases script was disabled, so `DRY_RUN` never gets set for
rehearse jobs. Restore the `JOB_NAME == rehearse*` guard in
`openshift-microshift-manage-versions-releases-commands.sh` so the step sets
`DRY_RUN="--dry-run"` only for rehearse runs; keep the existing `DRY_RUN`
variable and its use in the `publish`/`--create-pr` flow intact.
|
/pj-rehearse periodic-ci-openshift-microshift-release-4.22-periodics-update-versions-releases |
|
@pmtk: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@pmtk: This pull request references USHIFT-7211 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 bug 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. |
7a6b2f3 to
b08c834
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ci-operator/step-registry/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh (1)
58-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate credential variable for the same key file.
CLIENT_KEY(Line 58) andKEY(Line 71) are both set to/tmp/key.pem. If both are truly required by different downstream consumers (gen_gh_releases.shvs.generate_common_versions.py), a short comment clarifying why two separate env vars point at the same file would help avoid confusion for future maintainers.Also applies to: 71-71
🤖 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/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh` at line 58, The script sets both CLIENT_KEY and KEY to the same key file, which is confusing without context. In openshift-microshift-manage-versions-releases-commands.sh, keep the two env vars only if they are required by different consumers like gen_gh_releases.sh and generate_common_versions.py, and add a brief inline comment near the CLIENT_KEY/KEY assignments explaining that they intentionally reference the same /tmp/key.pem for separate downstream tools.
🤖 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/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh`:
- Around line 56-59: Credential loading in the manage-versions-releases command
is happening while shell tracing is enabled, so the APP_ID assignment can leak
secret values into CI logs. In
openshift-microshift-manage-versions-releases-commands.sh, wrap the APP_ID and
CLIENT_KEY export/loading block with tracing disabled and then restore tracing
afterward, keeping the rest of the script’s set -euo pipefail behavior intact.
Use the existing shell setup in the script to scope the sensitive operations
only, and ensure no credential-derived values are echoed or traced.
---
Nitpick comments:
In
`@ci-operator/step-registry/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh`:
- Line 58: The script sets both CLIENT_KEY and KEY to the same key file, which
is confusing without context. In
openshift-microshift-manage-versions-releases-commands.sh, keep the two env vars
only if they are required by different consumers like gen_gh_releases.sh and
generate_common_versions.py, and add a brief inline comment near the
CLIENT_KEY/KEY assignments explaining that they intentionally reference the same
/tmp/key.pem for separate downstream tools.
🪄 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: 8d6ccd7a-a917-4bb6-a408-66198c8c0bbd
📒 Files selected for processing (1)
ci-operator/step-registry/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@pmtk: 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. |
Summary by CodeRabbit
This change updates the OpenShift CI step that runs MicroShift’s
manage-versions/releasesworkflow.In
ci-operator/step-registry/openshift/microshift/manage-versions-releases/openshift-microshift-manage-versions-releases-commands.sh(the script that generates/tmp/run.sh), the job now loads and exports the GitHub App credentials (APP_IDandCLIENT_KEY) immediately after querying/tmp/releases.json. That ensures the publishing step (gen_gh_releases.sh ... publish ... --input /tmp/releases.json) has credentials available when the job proceeds to print/consumereleases.jsonand run the release publication logic.