Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 145 additions & 4 deletions .github/workflows/go_app_pull_requests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ on:
description: "The duration before tests are stopped"
type: string
default: "10m"
DOCKER_CACHE_IMAGES:
description: "Comma-separated list of Docker images to pre-pull and cache for integration tests"
type: string
required: false
default: ""
secrets:
GH_CI_PAT:
description: 'Token password for GitHub auth'
Expand Down Expand Up @@ -130,7 +135,22 @@ jobs:
# Install go-junit-report to format test results.
- name: Install go-junit-report
run: go install github.com/jstemmer/go-junit-report/v2@v2.1.0
# Run unit test for evet Go module.
# Free up disk space before integration tests (removes ~25GB of unused tools)
- name: Free disk space
if: ${{ inputs.GO_TEST_INTEGRATION_ENABLED }}
run: |
echo "Disk space before cleanup:"
df -h

# Remove unnecessary tools and SDKs (preserve Go and Docker)
sudo rm -rf /usr/share/dotnet
sudo rm -rf /usr/local/lib/android
sudo rm -rf /opt/ghc
sudo rm -rf /opt/hostedtoolcache/CodeQL

echo "Disk space after cleanup:"
df -h
# Run unit test for every Go module.
- name: build coverage output directories
run: |
mkdir -p coverage/unit
Expand All @@ -156,8 +176,126 @@ jobs:
files: ./junit_report.xml
name: junit-report
token: ${{ secrets.CODECOV_TOKEN }}
# Get the digest of the latest integrations-testsuite image
- name: Get integrations-testsuite image digest
if: ${{ inputs.GO_TEST_INTEGRATION_ENABLED }}
id: image-digest
env:
ARTIFACT_REGISTRY: ${{ secrets.ARTIFACT_REGISTRY }}
run: |
IMAGE="${ARTIFACT_REGISTRY}/kochava/integrations-testsuite:latest"
# Get digest without pulling the full image (only manifest)
# Extract the actual image digest from the manifest
DIGEST=$(docker manifest inspect ${IMAGE} | jq -r '.manifests[0].digest // .config.digest' | sed 's/sha256://')
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docker manifest inspect command output is piped to jq and sed without error checking. If the manifest inspection fails (e.g., due to network issues or authentication problems), the pipeline might produce an empty or invalid digest, which would be written to GITHUB_OUTPUT. This could cause cache key issues or failures in subsequent steps.

Add error checking after the pipeline:

DIGEST=$(docker manifest inspect ${IMAGE} | jq -r '.manifests[0].digest // .config.digest' | sed 's/sha256://')
if [ -z "$DIGEST" ] || [ "$DIGEST" = "null" ]; then
  echo "Error: Failed to get image digest"
  exit 1
fi
echo "digest=${DIGEST}" >> $GITHUB_OUTPUT
Suggested change
DIGEST=$(docker manifest inspect ${IMAGE} | jq -r '.manifests[0].digest // .config.digest' | sed 's/sha256://')
DIGEST=$(docker manifest inspect ${IMAGE} | jq -r '.manifests[0].digest // .config.digest' | sed 's/sha256://')
if [ -z "$DIGEST" ] || [ "$DIGEST" = "null" ]; then
echo "Error: Failed to get image digest"
exit 1
fi

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The jq query .manifests[0].digest // .config.digest assumes the manifest has a specific structure, but for multi-architecture images it may select an arbitrary architecture's digest (the first manifest). For single-architecture images, the fallback to .config.digest may not provide the correct image digest.

Consider using docker buildx imagetools inspect --raw which provides a more reliable way to get the image digest, or specify the platform explicitly to ensure consistent cache keys across runs.

Suggested change
DIGEST=$(docker manifest inspect ${IMAGE} | jq -r '.manifests[0].digest // .config.digest' | sed 's/sha256://')
DIGEST=$(docker buildx imagetools inspect --raw ${IMAGE} | grep -m1 '^Digest:' | awk '{print $2}' | sed 's/sha256://')

Copilot uses AI. Check for mistakes.
echo "digest=${DIGEST}" >> $GITHUB_OUTPUT
Comment on lines +180 to +190
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docker manifest inspect command requires authentication when used with private registries, but this step runs before the Login to Artifact Registry step (lines 112-120) has authenticated. This will cause the command to fail with authentication errors for private images.

The authentication step should be moved before this step, or this step should be moved after the authentication step to ensure docker manifest inspect can access the private registry.

Copilot uses AI. Check for mistakes.
# Cache Docker images for integration tests
- name: Cache integrations-testsuite Docker image
if: ${{ inputs.GO_TEST_INTEGRATION_ENABLED }}
uses: actions/cache@v4
with:
path: /tmp/.integrations-testsuite-cache
key: docker-integrations-testsuite-${{ runner.os }}-${{ steps.image-digest.outputs.digest }}
restore-keys: |
docker-integrations-testsuite-${{ runner.os }}-
# Pre-pull integrations-testsuite image before running tests
- name: Load or pull integrations-testsuite Docker image
if: ${{ inputs.GO_TEST_INTEGRATION_ENABLED }}
env:
ARTIFACT_REGISTRY: ${{ secrets.ARTIFACT_REGISTRY }}
run: |
# Full image path with repository namespace
IMAGE="${ARTIFACT_REGISTRY}/kochava/integrations-testsuite:latest"

# Try to load from cache first
if [ -f /tmp/.integrations-testsuite-cache/image.tar ]; then
echo "Loading integrations-testsuite from cache..."
if docker load -i /tmp/.integrations-testsuite-cache/image.tar; then
echo "✓ Successfully loaded from cache"
else
echo "✗ Cache corrupted, pulling ${IMAGE}..."
docker pull ${IMAGE} || exit 1
mkdir -p /tmp/.integrations-testsuite-cache
docker save ${IMAGE} -o /tmp/.integrations-testsuite-cache/image.tar || echo "Warning: failed to cache image"
fi
else
echo "Cache miss, pulling ${IMAGE}..."
docker pull ${IMAGE} || exit 1

# Save for next time
mkdir -p /tmp/.integrations-testsuite-cache
docker save ${IMAGE} -o /tmp/.integrations-testsuite-cache/image.tar || echo "Warning: failed to cache image"
fi

# Verify image is available
if ! docker image inspect "${IMAGE}" >/dev/null 2>&1; then
echo "Error: integrations-testsuite image not found after load/pull"
exit 1
fi
echo "✅ integrations-testsuite image verified"
# Get hash of Docker images list for cache key
- name: Hash Docker images list
if: ${{ inputs.GO_TEST_INTEGRATION_ENABLED && inputs.DOCKER_CACHE_IMAGES != '' }}
id: images-hash
run: |
HASH=$(echo "${{ inputs.DOCKER_CACHE_IMAGES }}" | sha256sum | cut -d' ' -f1 | head -c 12)
echo "hash=${HASH}" >> $GITHUB_OUTPUT
# Calculate cache slot (invalidate every 20 runs)
CACHE_SLOT=$(( ${{ github.run_number }} - (${{ github.run_number }} % 20) ))
Comment on lines +242 to +243
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache key rotation strategy using github.run_number will not work correctly across different branches. Run numbers are global to the repository, not per-branch, which means cache keys will invalidate unpredictably when PRs from different branches are running.

Consider using a time-based rotation (e.g., ${{ github.run_number / 20 }} rounded down) or a date-based approach like $(date +%Y%m%d) to ensure more predictable cache behavior across branches.

Suggested change
# Calculate cache slot (invalidate every 20 runs)
CACHE_SLOT=$(( ${{ github.run_number }} - (${{ github.run_number }} % 20) ))
# Calculate cache slot (invalidate daily)
CACHE_SLOT=$(date +%Y%m%d)

Copilot uses AI. Check for mistakes.
echo "cache-slot=${CACHE_SLOT}" >> $GITHUB_OUTPUT
# Cache custom Docker images (specified by caller)
- name: Cache custom Docker images
if: ${{ inputs.GO_TEST_INTEGRATION_ENABLED && inputs.DOCKER_CACHE_IMAGES != '' }}
uses: actions/cache@v4
with:
path: /tmp/.custom-docker-cache
key: docker-custom-images-${{ runner.os }}-${{ steps.images-hash.outputs.hash }}-${{ steps.images-hash.outputs.cache-slot }}-v1
restore-keys: |
docker-custom-images-${{ runner.os }}-${{ steps.images-hash.outputs.hash }}-
# Pre-pull custom Docker images before running tests
- name: Load or pull custom Docker images
if: ${{ inputs.GO_TEST_INTEGRATION_ENABLED && inputs.DOCKER_CACHE_IMAGES != '' }}
env:
IMAGES_LIST: ${{ inputs.DOCKER_CACHE_IMAGES }}
run: |
echo "Processing custom Docker images: $IMAGES_LIST"

# Convert comma-separated string to array
IFS=',' read -ra IMAGES <<< "$IMAGES_LIST"

mkdir -p /tmp/.custom-docker-cache

for image in "${IMAGES[@]}"; do
# Trim whitespace
image=$(echo "$image" | xargs)

if [ -z "$image" ]; then
continue
fi

filename=$(echo "$image" | tr '/:' '_')
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filename sanitization using tr '/:' '_' is insufficient and could lead to collisions or filesystem issues. It doesn't handle other problematic characters like spaces, special characters, or very long names.

Consider using a more robust approach:

filename=$(echo "$image" | sed 's/[^a-zA-Z0-9._-]/_/g' | cut -c1-200)

Or use a hash-based approach for guaranteed uniqueness:

filename=$(echo "$image" | sha256sum | cut -d' ' -f1)
Suggested change
filename=$(echo "$image" | tr '/:' '_')
filename=$(echo "$image" | sha256sum | cut -d' ' -f1)

Copilot uses AI. Check for mistakes.

if [ -f "/tmp/.custom-docker-cache/${filename}.tar" ]; then
echo "Loading $image from cache..."
if docker load -i "/tmp/.custom-docker-cache/${filename}.tar"; then
echo "✓ Successfully loaded $image from cache"
else
echo "✗ Cache corrupted, pulling $image..."
docker pull "$image" || exit 1
docker save "$image" -o "/tmp/.custom-docker-cache/${filename}.tar" || echo "Warning: failed to cache $image"
fi
else
echo "Cache miss, pulling $image..."
docker pull "$image" || exit 1

# Save for next time
docker save "$image" -o "/tmp/.custom-docker-cache/${filename}.tar" || echo "Warning: failed to cache $image"
fi
done

echo "✅ Custom Docker images ready"
docker images
# relies on the tests themselves compiling the binary with `-cover` and setting GOCOVERDIR to /coverage/int
# to seperate out any integration tests
# to separate out any integration tests
- name: integration tests
if: ${{ inputs.GO_TEST_INTEGRATION_ENABLED }}
run: go test -v ${{ inputs.GO_TEST_INTEGRATION_TAGS }} -timeout ${{ inputs.GO_TEST_INTEGRATION_TIMEOUT }} ./...
Expand Down Expand Up @@ -230,14 +368,17 @@ jobs:
# Setup Docker builder to do build.
- name: Setup Docker Buildx
id: buildx
uses: docker/setup-buildx-action@v2
uses: docker/setup-buildx-action@v3

# Build the app.
- name: Build
uses: docker/build-push-action@v3
uses: docker/build-push-action@v5
with:
push: false
provenance: false
context: .
file: Dockerfile
platforms: linux/amd64
tags: ${{ steps.release.outputs.tags }}
cache-from: type=gha
cache-to: type=gha,mode=max