Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Azure DevOps pipeline + helper scripts used by the Java Engineering Group to build/push OpenJDK container images, and to annotate previous images as EOL by digest, with added dry-run support for safer validation.
Changes:
- Refactors
build-image.shto accept CLI arguments (instead of env vars), adds dry-run logging, and emits the built image digest via--metadata-file. - Replaces tag-based “previous image” annotation with digest-based annotation using new pipeline templates.
- Updates
.devops/build.ymlto use the new templates, introducesdryrunandaz_packageparameters, and adjusts signing to use a container digest reference.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/image-annotation.sh | Switches from docker-pull/inspect to digest-based ORAS lifecycle annotation; adds dry-run mode and CLI args. |
| scripts/build-image.sh | Reworks argument passing, adds dry-run behavior, records container digest from build metadata. |
| .devops/templates/prepare-annotation.yml | New template to fetch a manifest and extract amd64/arm64 digests into pipeline variables. |
| .devops/templates/annotate-image.yml | New template to install ORAS and annotate prior amd64/arm64 digests, with optional dry-run. |
| .devops/build.yml | Pipeline refactor to use new templates/args, reduce defaults, and sign by digest reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
scripts/image-annotation.sh
Outdated
| while getopts "r:m:d" opt; do | ||
| case $opt in | ||
| r) | ||
| registry="$OPTARG" | ||
| ;; | ||
| m) | ||
| manifest="$OPTARG" | ||
| ;; | ||
| d) | ||
| debug=true | ||
| ;; | ||
| *) | ||
| echo "Invalid option: -$OPTARG" | ||
| exit 1 | ||
| ;; |
There was a problem hiding this comment.
| while getopts "r:m:d" opt; do | |
| case $opt in | |
| r) | |
| registry="$OPTARG" | |
| ;; | |
| m) | |
| manifest="$OPTARG" | |
| ;; | |
| d) | |
| debug=true | |
| ;; | |
| *) | |
| echo "Invalid option: -$OPTARG" | |
| exit 1 | |
| ;; | |
| while test $# -gt 0; do | |
| case "$1" in | |
| -r|--registry) | |
| shift | |
| registry="$1" | |
| ;; | |
| -m|--manifest) | |
| shift | |
| manifest="$1" | |
| ;; | |
| -d|--debug) | |
| shift | |
| debug=true | |
| ;; | |
| *) | |
| echo "Invalid option: $1" | |
| exit 1 | |
| ;; |
Is it cool if we add full-word options for flags? This suggestion is how I know to do it, but I am fine with any implementation if it achieves the same effect
There was a problem hiding this comment.
getopts does not support long versions of arguments. I'll have to switch to a different method.
.devops/templates/annotate-image.yml
Outdated
| -r ${{ parameters.registry }} | ||
| -m $(imageDigestAmd64) | ||
| -d | ||
| ${{ else }}: | ||
| arguments: > | ||
| -r ${{ parameters.registry }} | ||
| -m $(imageDigestAmd64) |
There was a problem hiding this comment.
| -r ${{ parameters.registry }} | |
| -m $(imageDigestAmd64) | |
| -d | |
| ${{ else }}: | |
| arguments: > | |
| -r ${{ parameters.registry }} | |
| -m $(imageDigestAmd64) | |
| --registry ${{ parameters.registry }} | |
| --manifest $(imageDigestAmd64) | |
| --debug | |
| ${{ else }}: | |
| arguments: > | |
| --registry ${{ parameters.registry }} | |
| --manifest $(imageDigestAmd64) |
If you implement my suggestion below, I would prefer this (to make the code more clear to future devs)
.devops/templates/annotate-image.yml
Outdated
| arguments: > | ||
| -r ${{ parameters.registry }} | ||
| -m $(imageDigestArm64) | ||
| -d | ||
| ${{ else }}: | ||
| arguments: > | ||
| -r ${{ parameters.registry }} | ||
| -m $(imageDigestArm64) No newline at end of file |
There was a problem hiding this comment.
| arguments: > | |
| -r ${{ parameters.registry }} | |
| -m $(imageDigestArm64) | |
| -d | |
| ${{ else }}: | |
| arguments: > | |
| -r ${{ parameters.registry }} | |
| -m $(imageDigestArm64) | |
| arguments: > | |
| --registry ${{ parameters.registry }} | |
| --manifest $(imageDigestArm64) | |
| --debug | |
| ${{ else }}: | |
| arguments: > | |
| --registry ${{ parameters.registry }} | |
| --manifest $(imageDigestArm64) |
Same here
|
PR LGTM! I will approve once my formatting suggestions are addressed :) |
This change request alters how the Java Engineering Group builds its OpenJDK docker images. With the intent that the process is more consistent and robust.