NO-ISSUE: Add LVMS branching automation script#2404
Conversation
Automates the 7-step release branching procedure for lvm-operator, covering GitHub branching, prodsec updates, Konflux release data, tekton config updates, and Konflux SA push secret verification. All steps are idempotent and can be re-run safely.
openshift-ci-robot
added
the
jira/valid-reference
|
@pacevedom: This pull request explicitly references no jira issue. 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. |
WalkthroughAdds ChangesRelease Branch Automation Script
🎯 3 (Moderate) | ⏱️ ~20 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
openshift-ci
Bot
added
the
size/XXL
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pacevedom 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: 3
🤖 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 `@hack/branch-release.sh`:
- Around line 507-515: The sed_replace function currently injects unescaped
pattern and replacement into sed which breaks when pattern/replacement contain
regex metacharacters (e.g., '.' in version strings); update sed_replace to
either escape regex/shell-special characters for both ${pattern} and
${replacement} before calling sed, or switch to a safer delimiter (e.g., use |
or a control character) in the sed substitution to avoid conflicts; ensure both
the DRY_RUN branch (log_dry_run message) and the actual sed invocation in
sed_replace use the same escaped/delimited form so the logged command matches
the executed command.
- Line 84: Update the help text string that currently reads "--step STEP
Run only a specific step (1-6)" to include step 7 (e.g. "1-7") so the usage
matches the documented Konflux SA secrets verification step; locate the
help/usage output in hack/branch-release.sh (the line containing the "--step
STEP" description) and change the range from 1-6 to 1-7.
- Around line 506-517: The sed_replace function is using non-portable sed -i
which fails on macOS; update sed_replace to detect platform or sed flavor (e.g.,
check uname or sed --version) and use a portable approach: for GNU sed keep sed
-i "s/.../.../g", for BSD/macOS use sed -i '' "s/.../.../g" or perform the edit
via a safe tempfile (mktemp) and mv to replace the original; ensure the DRY_RUN
branch prints the actual command that will be executed for the chosen method and
that any temporary backup file behavior is handled/removed so file contents are
identical after replacement.
🪄 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: 919b7197-ba61-4aec-a386-984f65e5d1f9
📒 Files selected for processing (1)
hack/branch-release.sh
| --konflux-dir PATH Path to konflux-release-data repo | ||
| --prodsec-dir PATH Path to product-definitions repo | ||
| --dry-run Show what would be done without making changes | ||
| --step STEP Run only a specific step (1-6) |
There was a problem hiding this comment.
Update help text to include step 7.
The help text states "Run only a specific step (1-6)" but step 7 (Konflux SA secrets verification) exists and is documented below at line 112.
📝 Proposed fix
- --step STEP Run only a specific step (1-6)
+ --step STEP Run only a specific step (1-7)📝 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.
| --step STEP Run only a specific step (1-6) | |
| --step STEP Run only a specific step (1-7) |
🤖 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 `@hack/branch-release.sh` at line 84, Update the help text string that
currently reads "--step STEP Run only a specific step (1-6)" to
include step 7 (e.g. "1-7") so the usage matches the documented Konflux SA
secrets verification step; locate the help/usage output in
hack/branch-release.sh (the line containing the "--step STEP" description) and
change the range from 1-6 to 1-7.
| # Perform sed replacement with backup for safety | ||
| sed_replace() { | ||
| local pattern="${1}" | ||
| local replacement="${2}" | ||
| local file="${3}" | ||
|
|
||
| if [[ "${DRY_RUN}" == "true" ]]; then | ||
| log_dry_run "sed -i 's/${pattern}/${replacement}/g' ${file}" | ||
| else | ||
| sed -i "s/${pattern}/${replacement}/g" "${file}" | ||
| fi | ||
| } |
There was a problem hiding this comment.
sed -i portability issue: fails on macOS.
The sed -i command at line 515 is not portable between Linux (GNU sed) and macOS (BSD sed). BSD sed requires an extension argument: sed -i '' ... or sed -i.bak ....
🔧 Proposed fix
sed_replace() {
local pattern="${1}"
local replacement="${2}"
local file="${3}"
if [[ "${DRY_RUN}" == "true" ]]; then
log_dry_run "sed -i 's/${pattern}/${replacement}/g' ${file}"
else
- sed -i "s/${pattern}/${replacement}/g" "${file}"
+ # Use a temporary file for portability across GNU and BSD sed
+ sed "s/${pattern}/${replacement}/g" "${file}" > "${file}.tmp" && mv "${file}.tmp" "${file}"
fi
}📝 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.
| # Perform sed replacement with backup for safety | |
| sed_replace() { | |
| local pattern="${1}" | |
| local replacement="${2}" | |
| local file="${3}" | |
| if [[ "${DRY_RUN}" == "true" ]]; then | |
| log_dry_run "sed -i 's/${pattern}/${replacement}/g' ${file}" | |
| else | |
| sed -i "s/${pattern}/${replacement}/g" "${file}" | |
| fi | |
| } | |
| # Perform sed replacement with backup for safety | |
| sed_replace() { | |
| local pattern="${1}" | |
| local replacement="${2}" | |
| local file="${3}" | |
| if [[ "${DRY_RUN}" == "true" ]]; then | |
| log_dry_run "sed -i 's/${pattern}/${replacement}/g' ${file}" | |
| else | |
| # Use a temporary file for portability across GNU and BSD sed | |
| sed "s/${pattern}/${replacement}/g" "${file}" > "${file}.tmp" && mv "${file}.tmp" "${file}" | |
| fi | |
| } |
🤖 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 `@hack/branch-release.sh` around lines 506 - 517, The sed_replace function is
using non-portable sed -i which fails on macOS; update sed_replace to detect
platform or sed flavor (e.g., check uname or sed --version) and use a portable
approach: for GNU sed keep sed -i "s/.../.../g", for BSD/macOS use sed -i ''
"s/.../.../g" or perform the edit via a safe tempfile (mktemp) and mv to replace
the original; ensure the DRY_RUN branch prints the actual command that will be
executed for the chosen method and that any temporary backup file behavior is
handled/removed so file contents are identical after replacement.
| sed_replace() { | ||
| local pattern="${1}" | ||
| local replacement="${2}" | ||
| local file="${3}" | ||
|
|
||
| if [[ "${DRY_RUN}" == "true" ]]; then | ||
| log_dry_run "sed -i 's/${pattern}/${replacement}/g' ${file}" | ||
| else | ||
| sed -i "s/${pattern}/${replacement}/g" "${file}" |
There was a problem hiding this comment.
sed_replace does not escape regex special characters.
The pattern and replacement parameters are passed directly to sed without escaping. If these contain regex metacharacters (e.g., ., *, [, ], /), the substitution may fail or produce unexpected results. Version strings contain . which matches any character in regex.
Consider escaping special characters or using a delimiter other than / that won't appear in the patterns. For example, use | as the delimiter: sed "s|${pattern}|${replacement}|g" which reduces conflicts, though full escaping would be more robust.
🤖 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 `@hack/branch-release.sh` around lines 507 - 515, The sed_replace function
currently injects unescaped pattern and replacement into sed which breaks when
pattern/replacement contain regex metacharacters (e.g., '.' in version strings);
update sed_replace to either escape regex/shell-special characters for both
${pattern} and ${replacement} before calling sed, or switch to a safer delimiter
(e.g., use | or a control character) in the sed substitution to avoid conflicts;
ensure both the DRY_RUN branch (log_dry_run message) and the actual sed
invocation in sed_replace use the same escaped/delimited form so the logged
command matches the executed command.
|
@pacevedom: The following tests failed, say
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. |
2 participants
Automates the 7-step release branching procedure for lvm-operator, covering GitHub branching, prodsec updates, Konflux release data, tekton config updates, and Konflux SA push secret verification.
All steps are idempotent and can be re-run safely.
Summary by CodeRabbit
New Features