-
Notifications
You must be signed in to change notification settings - Fork 133
IEP-1679 Update nightly build pipeline: JDK 17/21, Maven, and Azure signing #1386
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.
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
Conversation
📝 WalkthroughWalkthroughSwitched GitHub Actions nightly workflow to Temurin Java 21; removed local keystore-based JAR signing and disabled it in Maven; added Azure Key Vault + Jsign cloud signing steps for plugin JARs, regenerated P2 metadata, and cleaned up temporary artifacts and Azure session. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Actions Runner
participant Maven as Maven Build
participant Jsign as Jsign Tool
participant AzureAD as Azure AD (Service Principal)
participant KeyVault as Azure Key Vault
participant P2 as P2 Metadata Regenerator
Runner->>Maven: Build plugin JARs (with -Djarsigner.skip=true)
Runner->>Jsign: Download Jsign tool
Runner->>AzureAD: Authenticate using service principal
AzureAD-->>Runner: Return access token
Runner->>KeyVault: Request signing certificate/token (using access token)
KeyVault-->>Runner: Return signing token/cert reference
Runner->>Jsign: Sign JARs using KeyVault token/cert via Jsign
Jsign-->>Runner: Return signed JARs
Runner->>P2: Regenerate P2 metadata to reflect signed JARs
Runner->>Runner: Cleanup temporary artifacts and Azure session
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/nightly.yml:
- Around line 28-33: The "Download Jsign" step currently downloads jsign-7.4.jar
without integrity checks; update that step to fetch the official SHA256 value
(or include the known published SHA256 for jsign-7.4), compute the downloaded
file's SHA256 (e.g., with sha256sum or shasum -a 256) and compare it to the
expected value, and fail the job if the sums do not match; reference the step
name "Download Jsign" and the artifact "jsign-7.4.jar" so you add commands to
download/obtain the published checksum, verify it against the computed checksum,
and exit non‑zero on mismatch.
- Around line 17-22: Update the GitHub Actions step currently named "Set up JDK
21" that uses actions/setup-java@v2 to use actions/setup-java@v4 instead; locate
the step by its name and the uses: "actions/setup-java@v2" entry and change the
version to "@v4" so the workflow uses the non-deprecated runtime consistent with
other workflows in the repo.
| - name: Set up JDK 21 | ||
| uses: actions/setup-java@v2 | ||
| with: | ||
| java-version: '17' | ||
| java-version: '21' | ||
| distribution: 'temurin' | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/nightly.yml | head -30Repository: espressif/idf-eclipse-plugin
Length of output: 1090
🌐 Web query:
actions/setup-java v2 vs v4 deprecated node runtime
💡 Result:
Short answer: actions/setup-java@v2 is an older major that added custom-distribution support; actions/setup-java@v4 updates the action's internal Node.js runtime to Node 20 (v3 switched to Node 16, v4 → Node 20). The "Node.js 16 actions are deprecated" warning refers to the runtime the action code uses, not the Java version you install — upgrading to a Node‑20-compatible action release (e.g. setup-java@v4 or later) removes that warning. [1][2][3]
References
- actions/setup-java repo (V2 features and general docs). [1]
- actions/setup-java v4 release notes: Node runtime updated to 20 (v3 used Node 16). [2][3]
🏁 Script executed:
# Check for actionlint configuration in the repo
find . -name ".actionlintrc*" -o -name "actionlint*" 2>/dev/null | head -20Repository: espressif/idf-eclipse-plugin
Length of output: 54
🏁 Script executed:
# Search for other uses of setup-java in workflows
grep -r "setup-java" .github/workflows/ --include="*.yml"Repository: espressif/idf-eclipse-plugin
Length of output: 389
Upgrade actions/setup-java to v4 to avoid deprecated runtime.
actions/setup-java@v2 uses a deprecated Node runtime and will break when GitHub disables it. Other workflows in this repository already use v4. Please upgrade to v4.
🔧 Suggested change
- uses: actions/setup-java@v2
+ uses: actions/setup-java@v4
with:
java-version: '21'
distribution: 'temurin'📝 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.
| - name: Set up JDK 21 | |
| uses: actions/setup-java@v2 | |
| with: | |
| java-version: '17' | |
| java-version: '21' | |
| distribution: 'temurin' | |
| - name: Set up JDK 21 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: '21' | |
| distribution: 'temurin' |
🧰 Tools
🪛 actionlint (1.7.10)
18-18: the runner of "actions/setup-java@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In @.github/workflows/nightly.yml around lines 17 - 22, Update the GitHub
Actions step currently named "Set up JDK 21" that uses actions/setup-java@v2 to
use actions/setup-java@v4 instead; locate the step by its name and the uses:
"actions/setup-java@v2" entry and change the version to "@v4" so the workflow
uses the non-deprecated runtime consistent with other workflows in the repo.
| - name: Download Jsign | ||
| run: | | ||
| wget -q https://github.com/ebourg/jsign/releases/download/7.4/jsign-7.4.jar || \ | ||
| curl -L -o jsign-7.4.jar https://github.com/ebourg/jsign/releases/download/7.4/jsign-7.4.jar | ||
| echo "Downloaded Jsign 7.4" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat .github/workflows/nightly.ymlRepository: espressif/idf-eclipse-plugin
Length of output: 11069
🏁 Script executed:
# Search for any checksum or integrity verification in the repo related to jsign
rg -i "jsign|checksum|sha256|integrity" --type yamlRepository: espressif/idf-eclipse-plugin
Length of output: 54
🌐 Web query:
Jsign 7.4 release checksums GitHub ebourg
💡 Result:
The SHA-256 checksum for jsign 7.4 (jsign-7.4.jar) is:
2abf2ade9ea322acc2d60c24794eadc465ff9380938fca4c932d09e0b25f1c28. [1]
References:
- Chocolatey package verification notes for Jsign 7.4 (includes the SHA-256). [1]
- Jsign GitHub repository / project page (release 7.4 listed). [2]
- Maven Central listing for jsign-maven-plugin 7.4 (release artifacts with .sha1/.md5 available). [3]
Sources:
[1] Chocolatey — jsign package verification.
[2] ebourg/jsign GitHub repository.
[3] Maven Central — net/jsign/jsign-maven-plugin/7.4.
Add SHA256 checksum verification for Jsign download.
Downloading the signing tool without integrity verification is a supply-chain risk—a compromised release could inject malicious code into the artifact signing process. Add the published SHA256 checksum verification.
Suggested fix
- wget -q https://github.com/ebourg/jsign/releases/download/7.4/jsign-7.4.jar || \
- curl -L -o jsign-7.4.jar https://github.com/ebourg/jsign/releases/download/7.4/jsign-7.4.jar
- echo "Downloaded Jsign 7.4"
+ JAR_URL="https://github.com/ebourg/jsign/releases/download/7.4/jsign-7.4.jar"
+ JAR_SHA256="2abf2ade9ea322acc2d60c24794eadc465ff9380938fca4c932d09e0b25f1c28"
+ curl -fL -o jsign-7.4.jar "$JAR_URL"
+ echo "${JAR_SHA256} jsign-7.4.jar" | shasum -a 256 -c -
+ echo "Downloaded Jsign 7.4"📝 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.
| - name: Download Jsign | |
| run: | | |
| wget -q https://github.com/ebourg/jsign/releases/download/7.4/jsign-7.4.jar || \ | |
| curl -L -o jsign-7.4.jar https://github.com/ebourg/jsign/releases/download/7.4/jsign-7.4.jar | |
| echo "Downloaded Jsign 7.4" | |
| - name: Download Jsign | |
| run: | | |
| JAR_URL="https://github.com/ebourg/jsign/releases/download/7.4/jsign-7.4.jar" | |
| JAR_SHA256="2abf2ade9ea322acc2d60c24794eadc465ff9380938fca4c932d09e0b25f1c28" | |
| curl -fL -o jsign-7.4.jar "$JAR_URL" | |
| echo "${JAR_SHA256} jsign-7.4.jar" | shasum -a 256 -c - | |
| echo "Downloaded Jsign 7.4" | |
🤖 Prompt for AI Agents
In @.github/workflows/nightly.yml around lines 28 - 33, The "Download Jsign"
step currently downloads jsign-7.4.jar without integrity checks; update that
step to fetch the official SHA256 value (or include the known published SHA256
for jsign-7.4), compute the downloaded file's SHA256 (e.g., with sha256sum or
shasum -a 256) and compare it to the expected value, and fail the job if the
sums do not match; reference the step name "Download Jsign" and the artifact
"jsign-7.4.jar" so you add commands to download/obtain the published checksum,
verify it against the computed checksum, and exit non‑zero on mismatch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/nightly.yml:
- Line 23: Update the GitHub Actions checkout step to use the newer version
string by replacing the usage reference "actions/checkout@v2" with
"actions/checkout@v4"; locate the workflow step that contains the
actions/checkout invocation and update the version tag to v4 so the job runs on
the supported Node runtime.
- Around line 75-101: The signing loop uses the `find | while read -r file; do
... exit 1; done` pattern which runs the while in a subshell so `exit 1` doesn't
fail the main job; change the pipeline so the while loop runs in the main shell
(e.g., replace the pipe with process substitution so the loop reads from `<
<(find ...)`) and add `set -eo pipefail` at the top of the script; keep the
existing `jarsigner` invocation and its `$?` check (or replace with an immediate
conditional on the command) so any failure inside the `while` causes the main
script to exit non-zero.
♻️ Duplicate comments (2)
.github/workflows/nightly.yml (2)
25-30: Upgradeactions/setup-javato v4.This issue was already flagged in a previous review. Please upgrade to v4 to use the current Node.js runtime.
36-41: Add SHA256 checksum verification for Jsign download.This issue was already flagged in a previous review. Downloading a signing tool without integrity verification is a supply-chain risk.
| runs-on: macos-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade actions/checkout to v4 to avoid deprecated runtime.
actions/checkout@v2 uses a deprecated Node.js runtime. GitHub will eventually disable older Node runtimes, which will break this workflow.
🔧 Suggested fix
- - uses: actions/checkout@v2
+ - uses: actions/checkout@v4📝 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.
| - uses: actions/checkout@v2 | |
| - uses: actions/checkout@v4 |
🧰 Tools
🪛 actionlint (1.7.10)
23-23: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In @.github/workflows/nightly.yml at line 23, Update the GitHub Actions checkout
step to use the newer version string by replacing the usage reference
"actions/checkout@v2" with "actions/checkout@v4"; locate the workflow step that
contains the actions/checkout invocation and update the version tag to v4 so the
job runs on the supported Node runtime.
| find "$REPO_DIR" -type f -name "*.jar" | while read -r file; do | ||
| if [[ "$file" =~ (plugins|features)/com\.espressif\. ]]; then | ||
| echo "Signing: $file" | ||
|
|
||
| jarsigner \ | ||
| -J-cp -Jjsign-7.4.jar \ | ||
| -J--add-modules -Jjava.sql \ | ||
| -providerClass net.jsign.jca.JsignJcaProvider \ | ||
| -providerArg "${{ secrets.AZURE_KEYVAULT_URI }}" \ | ||
| -keystore NONE \ | ||
| -storetype AZUREKEYVAULT \ | ||
| -storepass "$AZURE_TOKEN" \ | ||
| -sigfile "$SIGFILE" \ | ||
| -digestalg SHA-256 \ | ||
| -tsa http://timestamp.digicert.com \ | ||
| $CERTCHAIN_ARG \ | ||
| -certs \ | ||
| -verbose \ | ||
| "$file" \ | ||
| "${{ secrets.AZURE_KEYVAULT_CERT_NAME }}" | ||
|
|
||
| if [ $? -ne 0 ]; then | ||
| echo "Failed to sign: $file" | ||
| exit 1 | ||
| fi | ||
| fi | ||
| done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: exit 1 in pipeline subshell won't fail the job.
The find | while read pattern runs the while loop in a subshell. When exit 1 is called on line 98, it only exits the subshell—not the main script. The step will report success even if signing fails.
Additionally, use set -eo pipefail at the start of the script to ensure pipeline failures propagate.
🐛 Proposed fix using process substitution
run: |
+ set -eo pipefail
+
# Setup Certificate Chain
CERTCHAIN_ARG=""
if [ -n "$CERT_CHAIN" ]; then
echo "$CERT_CHAIN" > certchain.p7b
CERTCHAIN_ARG="-certchain certchain.p7b"
fi
REPO_DIR="releng/com.espressif.idf.update/target/repository"
SIGFILE="ECLIPSE"
echo "Signing IDF plugin JARs in $REPO_DIR..."
+ SIGN_FAILED=0
# Find and sign specific Espressif JARs
- find "$REPO_DIR" -type f -name "*.jar" | while read -r file; do
+ while read -r file; do
if [[ "$file" =~ (plugins|features)/com\.espressif\. ]]; then
echo "Signing: $file"
- jarsigner \
+ if ! jarsigner \
-J-cp -Jjsign-7.4.jar \
-J--add-modules -Jjava.sql \
-providerClass net.jsign.jca.JsignJcaProvider \
-providerArg "${{ secrets.AZURE_KEYVAULT_URI }}" \
-keystore NONE \
-storetype AZUREKEYVAULT \
-storepass "$AZURE_TOKEN" \
-sigfile "$SIGFILE" \
-digestalg SHA-256 \
-tsa http://timestamp.digicert.com \
$CERTCHAIN_ARG \
-certs \
-verbose \
"$file" \
- "${{ secrets.AZURE_KEYVAULT_CERT_NAME }}"
-
- if [ $? -ne 0 ]; then
+ "${{ secrets.AZURE_KEYVAULT_CERT_NAME }}"; then
echo "Failed to sign: $file"
- exit 1
+ SIGN_FAILED=1
fi
fi
- done
+ done < <(find "$REPO_DIR" -type f -name "*.jar")
+
+ if [ "$SIGN_FAILED" -ne 0 ]; then
+ echo "One or more JAR files failed to sign"
+ exit 1
+ 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.
| find "$REPO_DIR" -type f -name "*.jar" | while read -r file; do | |
| if [[ "$file" =~ (plugins|features)/com\.espressif\. ]]; then | |
| echo "Signing: $file" | |
| jarsigner \ | |
| -J-cp -Jjsign-7.4.jar \ | |
| -J--add-modules -Jjava.sql \ | |
| -providerClass net.jsign.jca.JsignJcaProvider \ | |
| -providerArg "${{ secrets.AZURE_KEYVAULT_URI }}" \ | |
| -keystore NONE \ | |
| -storetype AZUREKEYVAULT \ | |
| -storepass "$AZURE_TOKEN" \ | |
| -sigfile "$SIGFILE" \ | |
| -digestalg SHA-256 \ | |
| -tsa http://timestamp.digicert.com \ | |
| $CERTCHAIN_ARG \ | |
| -certs \ | |
| -verbose \ | |
| "$file" \ | |
| "${{ secrets.AZURE_KEYVAULT_CERT_NAME }}" | |
| if [ $? -ne 0 ]; then | |
| echo "Failed to sign: $file" | |
| exit 1 | |
| fi | |
| fi | |
| done | |
| SIGN_FAILED=0 | |
| # Find and sign specific Espressif JARs | |
| while read -r file; do | |
| if [[ "$file" =~ (plugins|features)/com\.espressif\. ]]; then | |
| echo "Signing: $file" | |
| if ! jarsigner \ | |
| -J-cp -Jjsign-7.4.jar \ | |
| -J--add-modules -Jjava.sql \ | |
| -providerClass net.jsign.jca.JsignJcaProvider \ | |
| -providerArg "${{ secrets.AZURE_KEYVAULT_URI }}" \ | |
| -keystore NONE \ | |
| -storetype AZUREKEYVAULT \ | |
| -storepass "$AZURE_TOKEN" \ | |
| -sigfile "$SIGFILE" \ | |
| -digestalg SHA-256 \ | |
| -tsa http://timestamp.digicert.com \ | |
| $CERTCHAIN_ARG \ | |
| -certs \ | |
| -verbose \ | |
| "$file" \ | |
| "${{ secrets.AZURE_KEYVAULT_CERT_NAME }}"; then | |
| echo "Failed to sign: $file" | |
| SIGN_FAILED=1 | |
| fi | |
| fi | |
| done < <(find "$REPO_DIR" -type f -name "*.jar") | |
| if [ "$SIGN_FAILED" -ne 0 ]; then | |
| echo "One or more JAR files failed to sign" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In @.github/workflows/nightly.yml around lines 75 - 101, The signing loop uses
the `find | while read -r file; do ... exit 1; done` pattern which runs the
while in a subshell so `exit 1` doesn't fail the main job; change the pipeline
so the while loop runs in the main shell (e.g., replace the pipe with process
substitution so the loop reads from `< <(find ...)`) and add `set -eo pipefail`
at the top of the script; keep the existing `jarsigner` invocation and its `$?`
check (or replace with an immediate conditional on the command) so any failure
inside the `while` causes the main script to exit non-zero.
kolipakakondal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sigmaaa Thanks for the PR. Can you check if we can make use of this https://github.com/espressif/release-sign/ here or need improvements?
release-sign GH already implemented a github action for jar signing, probably we need to adopt the same for nightly and release builds.
Description
Updated nightly build pipeline: JDK 21, Maven, and Azure signing
Fixes # (IEP-1679)
Type of change
Please delete options that are not relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.