Open
Conversation
Signed-off-by: Shekhar Saxena <shekhar.saxena@ibm.com>
Signed-off-by: Shekhar Saxena <shekhar.saxena@ibm.com>
Reviewer's GuideAdds a Bash helper script to build and optionally push a Quarkus-based container image using the Maven wrapper and Jib extension, with configurable image name, tag, build, and push flags. Sequence diagram for build_and_push.sh image build and push flowsequenceDiagram
actor Developer
participant Script as build_and_push_sh
participant Maven as mvnw_Maven
participant QuarkusJib as Quarkus_Jib
participant Registry as Container_Registry
Developer->>Script: Execute with options (-i, -t, -b, -p)
Script->>Script: Parse arguments with getopts
Script->>Script: Resolve IMAGE_NAME and QUARKUS_PLATFORMS
Script->>Maven: ./mvnw clean package with Quarkus Jib properties
Maven->>QuarkusJib: Invoke container image build
alt QUARKUS_BUILD is true
QuarkusJib->>QuarkusJib: Build container image for platforms
else QUARKUS_BUILD is false
QuarkusJib->>QuarkusJib: Skip image build
end
alt QUARKUS_PUSH is true
QuarkusJib->>Registry: Push image IMAGE_NAME
else QUARKUS_PUSH is false
QuarkusJib->>QuarkusJib: Keep image local only
end
QuarkusJib-->>Maven: Build and push result
Maven-->>Script: mvnw exit status
Script-->>Developer: Exit with success or error code
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- Consider using
set -euo pipefailand quoting variables like"$IMAGE_NAME"and"$QUARKUS_PLATFORMS"to make the script more robust against unset variables and word-splitting issues. - It might be helpful to validate that
-band-ponly accepttrue/false(or a defined set of values) and fail fast with a clear error message when an invalid value is provided. - Rather than hardcoding
QUARKUS_PLATFORMSinside the script, consider allowing it to be overridden via an environment variable or CLI flag to make the script more flexible for different build targets.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using `set -euo pipefail` and quoting variables like `"$IMAGE_NAME"` and `"$QUARKUS_PLATFORMS"` to make the script more robust against unset variables and word-splitting issues.
- It might be helpful to validate that `-b` and `-p` only accept `true`/`false` (or a defined set of values) and fail fast with a clear error message when an invalid value is provided.
- Rather than hardcoding `QUARKUS_PLATFORMS` inside the script, consider allowing it to be overridden via an environment variable or CLI flag to make the script more flexible for different build targets.
## Individual Comments
### Comment 1
<location path="build_and_push.sh" line_range="1-2" />
<code_context>
+#!/bin/bash
+set -e
+
+# Usage function
</code_context>
<issue_to_address>
**suggestion:** Consider strengthening shell safety flags beyond `set -e`.
Since this script builds and pushes images, consider using a more defensive set of options like `set -euo pipefail` so unset variables and pipeline failures are caught explicitly. You may need to adjust any code that currently relies on undefined variables or non-critical pipeline steps succeeding.
Suggested implementation:
```
#!/bin/bash
set -euo pipefail
# Usage function
```
If the rest of the script uses any variables that may be unset (e.g., rely on them defaulting to empty), you may need to:
1. Initialize them explicitly before use, or
2. Use parameter expansion with defaults (e.g., `${VAR:-}`) where appropriate.
Also, if there are pipelines where a non-zero exit in an early command is acceptable, you may need to explicitly handle those cases (e.g., with `|| true` or restructuring the pipeline) now that `pipefail` is enabled.
</issue_to_address>
### Comment 2
<location path="build_and_push.sh" line_range="31-40" />
<code_context>
+IMAGE_NAME=""
+
+# Parse command line arguments
+while getopts "i:t:b:p:h" opt; do
+ case ${opt} in
+ i )
+ IMAGE_NAME=$OPTARG
+ ;;
+ t )
+ IMAGE_TAG=$OPTARG
+ ;;
+ b )
+ QUARKUS_BUILD=$OPTARG
+ ;;
+ p )
+ QUARKUS_PUSH=$OPTARG
+ ;;
+ h )
</code_context>
<issue_to_address>
**issue:** Validate `-b`/`-p` values to avoid surprising behavior from typos.
`-b` and `-p` are described as boolean flags, but any string is currently passed to `-Dquarkus.container-image.{build,push}` (e.g. `-b ture` would be accepted). Please restrict accepted values to `true|false` (case-normalized) and fail fast with a clear error on invalid input, or otherwise fall back to a safe default.
</issue_to_address>
### Comment 3
<location path="build_and_push.sh" line_range="13-21" />
<code_context>
+ echo " -t TAG Image tag (default: 0.1) - used only if -i is not provided"
+ echo " -b BUILD Build image true/false (default: true)"
+ echo " -p PUSH Push image true/false (default: true)"
+ echo " -h Show this help message"
+ echo ""
+ echo "Examples:"
+ echo " $0 -i quay.io/myuser/kruize-optimizer:1.0.0 -b true -p true"
+ echo " $0 -t 0.1_mvp -p false"
+ echo " $0 -b true -p true"
+ echo ""
+ echo "Note: Use either -i for full image name OR -t for tag with default registry/repo"
+ exit 1
+}
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use a success exit code when showing help via `-h`.
`usage` always exits with status 1, so a normal `-h` run is reported as an error by callers/CI. Consider distinguishing invalid usage from explicit help (e.g., accept a status argument or add a separate help function) so `-h` exits 0 while real parsing errors remain non‑zero.
Suggested implementation:
```
# Usage function
usage() {
# Optional first argument is the desired exit code (default: 1)
local exit_code="${1:-1}"
echo "Usage: $0 [OPTIONS]"
echo ""
echo "Options:"
echo " -i IMAGE_NAME Full image name (registry/repository:tag)"
echo " -t TAG Image tag (default: 0.1) - used only if -i is not provided"
echo " -b BUILD Build image true/false (default: true)"
echo " -p PUSH Push image true/false (default: true)"
echo " -h Show this help message"
exit "${exit_code}"
```
To fully implement the behavior you described, the argument parsing logic elsewhere in `build_and_push.sh` should be updated to:
1. Call `usage 0` when handling the `-h` option (explicit help).
2. Call `usage 1` (or omit the argument and rely on the default) for invalid options or other usage errors.
For example, in a `getopts` case block you’d want something conceptually like:
- `h) usage 0 ;;`
- `\?) usage 1 ;;`
Adjust these calls in the actual option-parsing code in this script.
</issue_to_address>
### Comment 4
<location path="build_and_push.sh" line_range="60" />
<code_context>
+ IMAGE_NAME="quay.io/kruize/optimizer:${IMAGE_TAG}"
+fi
+
+QUARKUS_PLATFORMS="linux/amd64,linux/arm64"
+
+echo "Building and pushing image: ${IMAGE_NAME}..."
</code_context>
<issue_to_address>
**suggestion:** Consider making target platforms configurable instead of hard-coded.
Since the list is fixed to `linux/amd64,linux/arm64`, consider reading the platforms from a flag or environment variable (with this as the default). That would let the same script work in local dev, CI with QEMU limits, or single-arch registries without needing manual edits.
Suggested implementation:
```
# Allow overriding target platforms via environment variable; default to linux/amd64,linux/arm64
: "${QUARKUS_PLATFORMS:=linux/amd64,linux/arm64}"
```
If you want platforms to be configurable via a command-line flag as well, you’ll need to:
1. Add a new option (e.g. `-p` or `--platforms`) in the argument parsing section to set `QUARKUS_PLATFORMS`.
2. Update the `usage` function to document the new flag and mention that `QUARKUS_PLATFORMS` can also be set via environment variable.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Shekhar Saxena <shekhar.saxena@ibm.com>
mbvreddy
approved these changes
Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description: Adding build_and_push.sh Script
Summary
Added the
build_and_push.shscript to support flexible command-line parameters for building and pushing container images, making it more versatile for different deployment scenarios.Parameters Added
-iquay.io/rh-ee-shesaxen/optimizer:0.1_mvp-t-iis not provided)0.1_mvp-btrue-ptrue-hUsage Examples
Use full image name
Use custom tag with default registry/repository
./build_and_push.sh -t 0.1_mvp -p falseUse all defaults
Build only (no push)
Summary by Sourcery
Build: