Skip to content

./hack/post-release.sh omit bundle change #4184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wgahnagl wants to merge 1 commit into
base: master
Choose a base branch
from
+19 −4
Open

./hack/post-release.sh omit bundle change #4184

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions hack/pre-release.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ CVP=false
BASEIMAGE=false
DISTGIT=false
GITLAB=false
KEEP_BUNDLE_CHANGES=false

# codes for printing and resetting pink text
PINK='\033[95m'
Expand All @@ -30,6 +31,9 @@ usage() {
echo "optional flags: -g"
echo " bumps the gitlab versions"
echo -e " ${PINK}Requires connection to the RedHat VPN"
echo "optional flags: -k"
echo " keep all changes from 'make bundle' including RBAC regeneration"
echo " (default: only keep version-related changes, revert RBAC changes)"
printf "${START_COLOR}"
}

Expand Down Expand Up @@ -231,6 +235,16 @@ github_update() {
sed -i 's/REPLACE_IMAGE:latest/REPLACE_IMAGE/' bundle/manifests/windows-machine-config-operator.clusterserviceversion.yaml
sed -i "s/operator-sdk-v1.14.0+git/operator-sdk-$OPERATOR_SDK_VERSION+git/" bundle/manifests/windows-machine-config-operator.clusterserviceversion.yaml

# Revert unintended RBAC changes from make bundle unless -k flag is set
if [[ $KEEP_BUNDLE_CHANGES == "false" ]]; then
echo "Reverting RBAC changes from make bundle (use -k flag to keep all changes)"
git checkout HEAD -- bundle/manifests/prometheus-k8s_rbac.authorization.k8s.io_v1_role.yaml \
bundle/manifests/system-wicd-nodes_rbac.authorization.k8s.io_v1_clusterrole.yaml \
config/rbac/role.yaml 2>/dev/null || true
Comment on lines +239 to +243

@coderabbitai coderabbitai Bot Jun 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not suppress RBAC reversion failures.

At Line 243, 2>/dev/null || true hides checkout failures, so the script can continue while still carrying unintended RBAC changes (opposite of the default contract).

Suggested fix 
-  if [[ $KEEP_BUNDLE_CHANGES == "false" ]]; then
-    echo "Reverting RBAC changes from make bundle (use -k flag to keep all changes)"
-    git checkout HEAD -- bundle/manifests/prometheus-k8s_rbac.authorization.k8s.io_v1_role.yaml \
-                         bundle/manifests/system-wicd-nodes_rbac.authorization.k8s.io_v1_clusterrole.yaml \
-                         config/rbac/role.yaml 2>/dev/null || true
+  if [[ $KEEP_BUNDLE_CHANGES == "false" ]]; then
+    echo "Reverting RBAC changes from make bundle (use -k flag to keep all changes)"
+    for file in \
+      bundle/manifests/prometheus-k8s_rbac.authorization.k8s.io_v1_role.yaml \
+      bundle/manifests/system-wicd-nodes_rbac.authorization.k8s.io_v1_clusterrole.yaml \
+      config/rbac/role.yaml; do
+      if git ls-files --error-unmatch "$file" >/dev/null 2>&1; then
+        git checkout HEAD -- "$file"
+      fi
+    done
   else

As per coding guidelines, hack/**/* scripts must “Check for proper error handling.”

🤖 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/pre-release.sh` around lines 239 - 243, The script suppresses failures
when reverting RBAC bundle files by appending "2>/dev/null || true" to the git
checkout command (the block conditioned on KEEP_BUNDLE_CHANGES), so revert
errors are hidden and the script can continue with unintended RBAC changes;
remove the suppression and ensure failures are propagated by deleting
"2>/dev/null || true" and/or adding explicit error handling after the git
checkout (check the exit status and exit with a non-zero code or log and exit)
for the git checkout of
bundle/manifests/prometheus-k8s_rbac.authorization.k8s.io_v1_role.yaml,
bundle/manifests/system-wicd-nodes_rbac.authorization.k8s.io_v1_clusterrole.yaml
and config/rbac/role.yaml so the script fails when the revert fails.

Source: Coding guidelines

else
echo "Keeping all changes from make bundle including RBAC regeneration"
fi

commit_message="[$base_branch] Update version to $updated_version

This commit was generated by hack/pre-release.sh"
Expand Down Expand Up @@ -424,14 +438,15 @@ if [[ "$operator_sdk_current_version" != "$OPERATOR_SDK_VERSION" ]]; then
exit
fi

# This sets the CVP and baseimage flags
while getopts "bcdg" opt; do
case "$opt" in
# This sets the CVP, baseimage, and other flags
while getopts "bcdgk" opt; do
case "$opt" in
c) CVP=true;;
b) BASEIMAGE=true;;
d) DISTGIT=true;;
g) GITLAB=true;;
esac
k) KEEP_BUNDLE_CHANGES=true;;
esac
Comment on lines +442 to +449

@coderabbitai coderabbitai Bot Jun 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle unsupported flags explicitly in getopts.

There is no fallback branch for invalid options. At Line 443-449 this matches ShellCheck SC2220 and leads to unclear/partial execution on bad flags.

Suggested fix 
 while getopts "bcdgk" opt; do
   case "$opt" in
     c) CVP=true;;
     b) BASEIMAGE=true;;
     d) DISTGIT=true;;
     g) GITLAB=true;;
     k) KEEP_BUNDLE_CHANGES=true;;
+    \?)
+      echo "Invalid option: -$OPTARG"
+      usage
+      exit 2
+      ;;
   esac
 done

As per coding guidelines, **/*.sh must “Validate shellcheck compliance.”

📝 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.

Suggested change
while getopts "bcdgk" opt; do
case "$opt" in
c) CVP=true;;
b) BASEIMAGE=true;;
d) DISTGIT=true;;
g) GITLAB=true;;
esac
k) KEEP_BUNDLE_CHANGES=true;;
esac
while getopts "bcdgk" opt; do
case "$opt" in
c) CVP=true;;
b) BASEIMAGE=true;;
d) DISTGIT=true;;
g) GITLAB=true;;
k) KEEP_BUNDLE_CHANGES=true;;
\?)
echo "Invalid option: -$OPTARG"
usage
exit 2
;;
esac
done
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 443-449: Invalid flags are not handled. Add a *) case.

(SC2220)

🤖 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/pre-release.sh` around lines 442 - 449, The getopts option parsing lacks
a fallback for invalid flags, so update the case block that handles getopts (the
"while getopts \"bcdgk\" opt; do" and its case for "$opt") to add a default
branch (*) that prints a clear error/usage message and exits non‑zero; also
handle the ":" case if you want explicit "missing argument" detection for
options that take arguments. Ensure the new branch references the same variables
(opt) and uses the same script exit style as other failure paths so bad flags
cause an immediate non-zero exit instead of continuing execution.

Sources: Coding guidelines, Linters/SAST tools

done
shift $((OPTIND-1))

Expand Down