Redesign Github workflows#59
Conversation
- Have a single test workflow for testing everything, including the API server. - Split the API server workflow into a build part and a deploy part. Get rid of test steps, since that's done by the test workflow. - Syntax check Ansible. - Make test and API server build workflows compatible with pull requests. - Enforce timeouts.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
abtreece
left a comment
abtreece
left a comment
There was a problem hiding this comment.
Solid consolidation — security posture is meaningfully better than what it replaces. A few inline notes.
| elif [[ "$GITHUB_EVENT_NAME" == "push" ]]; then | ||
| before_sha="$GITHUB_EVENT_BEFORE" | ||
| head_sha="$GITHUB_SHA" | ||
| if [[ "$before_sha" =~ ^0+$ ]] || ! git cat-file -e "$before_sha" 2>/dev/null; then |
There was a problem hiding this comment.
Could silently skip the build on force-push: git diff-tree -r "$head_sha" only sees the tip commit, so apiserver/ changes in any non-tip commit get missed. Consider failing open (changed=true) when before_sha is invalid — over-building is cheaper than missing a deploy.
| - name: Check that we're using system Ruby | ||
| run: test "$(which ruby)" = /usr/bin/ruby | ||
|
|
||
| - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 |
There was a problem hiding this comment.
Worth adding restore-keys so a Gemfile.lock bump doesn't blow the whole cache:
restore-keys: |
${{ runner.os }}-24.04-${{ runner.arch }}-gems-| echo "Build SHA: $BUILD_HEAD_SHA" | ||
| } > tag-message.txt | ||
|
|
||
| git tag -f -a apiserver-"$GITHUB_RUN_NUMBER" -F tag-message.txt "$BUILD_HEAD_SHA" |
There was a problem hiding this comment.
GITHUB_RUN_NUMBER is stable across re-runs, so a re-run of an already-published deploy will fail at git push origin apiserver-N (remote tag exists). Probably fine as a guardrail — but -f here rewrites the local tag before the push fails, so local and remote drift. Dropping -f would surface the conflict earlier.
3 participants
Uh oh!
There was an error while loading. Please reload this page.