Automate GH JAR Upload to DevOps #24
Conversation
tsweckard
changed the title
.github/workflows/build.yml
Outdated
| git add resources/ | ||
| git commit -m "Update GraphHopper JAR from GitHub build - ${{ steps.jar-info.outputs.date }}" \ | ||
| -m "Built from commit: ${{ github.sha }}" \ | ||
| -m "Java version: 25" |
There was a problem hiding this comment.
question: sometimes when we merge from GH main, the Java version is updated. How were you thinking about handling that?
There was a problem hiding this comment.
I don't see a good way of dynamically updating this. I thought we could read it from a pom.xml file but it's only being updated in this build yml so we'll have to ensure that when we merge main, we update the values in this pipeline.
There was a problem hiding this comment.
I think we'll want some sort of documentation in order to store these steps so that we don't need to remember all of the processes. Then we can update the values as we're merging in main.
Since we're likely not merging our fork back in, what are your thoughts on adding some documentation in this repo? @payneBrandon
There was a problem hiding this comment.
Is that just needed to run the JAR file in our VM? It'd be fine to add in some documentation to it I think
There was a problem hiding this comment.
I think some documentation on keeping this build.yml updated, how to keep the secrets updated, and briefly how it works. Since we're updating it to build the jar automatically, we don't need to document how to compile and build it as much.
chansbtran
left a comment
chansbtran
left a comment
There was a problem hiding this comment.
LGTM! Just an open comment for Brandon regarding documentation.
payneBrandon
left a comment
payneBrandon
left a comment
There was a problem hiding this comment.
just a couple questions, all makes sense to me so far!
| JAVA_VERSION: 25 | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
There was a problem hiding this comment.
The push-to-azure job needs Java setup to run Maven commands. Add the actions/setup-java@v4 step with the appropriate Java version and distribution (e.g., java-version: 25, distribution: temurin) before the Package JAR step. Consider also adding Maven and node caching steps similar to the build job to improve performance.
| - name: Set up Java | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: ${{ env.JAVA_VERSION }} | |
| distribution: temurin | |
| cache: maven | |
| - name: Cache Maven repository | |
| uses: actions/cache@v4 | |
| with: | |
| path: ~/.m2/repository | |
| key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }} | |
| restore-keys: | | |
| ${{ runner.os }}-maven- |
| git clone https://[email protected]/$AZURE_ORG/$AZURE_PROJECT/_git/$AZURE_REPO azure-repo | ||
There was a problem hiding this comment.
The PAT token is embedded directly in the git clone URL. While GitHub Actions masks secrets in logs, the token will be stored in the .git/config file of the cloned repository. Although this is on a transient runner, consider using git credential helpers or configuring authentication after cloning to follow security best practices. Alternatively, you can add git remote set-url origin https://dev.azure.com/$AZURE_ORG/$AZURE_PROJECT/_git/$AZURE_REPO after cloning to remove the token from the remote URL.
| git clone https://[email protected]/$AZURE_ORG/$AZURE_PROJECT/_git/$AZURE_REPO azure-repo | |
| # Configure Git to use the PAT via an HTTP Authorization header instead of embedding it in the URL | |
| AUTH_HEADER=$(printf ":%s" "$AZURE_DEVOPS_PAT" | base64 | tr -d '\n') | |
| git config --global http.https://dev.azure.com/.extraheader "AUTHORIZATION: Basic $AUTH_HEADER" | |
| # Clone the Azure DevOps repository without embedding the PAT in the URL | |
| git clone https://dev.azure.com/$AZURE_ORG/$AZURE_PROJECT/_git/$AZURE_REPO azure-repo | |
| # Clean up the extra header configuration | |
| git config --global --unset-all http.https://dev.azure.com/.extraheader || true |
| - name: Get JAR info and set date | ||
| id: jar-info | ||
| run: | | ||
| JAR_FILE=$(ls web/target/*.jar | head -n 1) |
There was a problem hiding this comment.
The JAR_FILE path should be validated before attempting to use it. If no JAR file is found in web/target/, the variable will be empty and subsequent commands will fail in unclear ways. Add a check after line 59 to verify that JAR_FILE is not empty and the file exists, providing a clear error message if the package step failed to produce a JAR.
| JAR_FILE=$(ls web/target/*.jar | head -n 1) | |
| JAR_FILE=$(ls web/target/*.jar | head -n 1) | |
| if [ -z "$JAR_FILE" ] || [ ! -f "$JAR_FILE" ]; then | |
| echo "Error: No JAR file found in web/target/. Maven package step may have failed to produce a JAR." >&2 | |
| exit 1 | |
| fi |
Our current workflow after PR 21 get's merged requires manually downloading the JAR file built from the build pipeline, creating a new branch in the DevOps GraphHopper repository, copying the JAR over and submitting a PR.
This updates our build pipeline to do all of this for us by accessing the Azure DevOps repo, copying the built JAR and creating a PR with Chan and I as reviewers.