Skip to content

Commit 9bcb12a

Browse files
committed
[patch] Harden PPB retry and image delivery
1 parent 8d7a039 commit 9bcb12a

8 files changed

Lines changed: 357 additions & 25 deletions

File tree

.github/workflows/goreleaser.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,19 @@ jobs:
5353
args: release --clean
5454
env:
5555
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
56+
57+
# Keep the release run open until both registries have published the tag.
58+
# This covers the normal release dispatch and a manually pushed tag without
59+
# racing a second tag-triggered image workflow.
60+
publish-images:
61+
if: github.ref_type == 'tag'
62+
needs: [test, goreleaser]
63+
permissions:
64+
contents: read
65+
id-token: write
66+
packages: write
67+
uses: ./.github/workflows/lint-test-build.yml
68+
secrets:
69+
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
70+
GCLOUD_OIDC_POOL: ${{ secrets.GCLOUD_OIDC_POOL }}
71+
GSA: ${{ secrets.GSA }}

.github/workflows/lint-test-build.yml

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
name: build-push
22
on:
3+
workflow_call:
4+
secrets:
5+
CODECOV_TOKEN:
6+
required: false
7+
GCLOUD_OIDC_POOL:
8+
required: false
9+
GSA:
10+
required: false
311
pull_request:
412
push:
513
branches:
614
- main
7-
tags:
8-
- "*"
915
workflow_dispatch:
1016

1117
permissions:
@@ -48,21 +54,24 @@ jobs:
4854
env:
4955
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
5056

51-
publish-ghcr:
57+
# PPB is public in GHCR and also runs on Cloud Run. The shared workflow
58+
# builds and scans each native image once, then publishes the same digest to
59+
# both registries.
60+
publish:
5261
if: github.event_name != 'pull_request'
5362
needs: [lint-test]
54-
uses: libops/.github/.github/workflows/build-push.yaml@3bdb895c3fd05490b0c6a1341e630ac554842468 # main
55-
permissions:
56-
contents: read
57-
packages: write
58-
59-
publish-gar:
60-
if: github.event_name != 'pull_request'
61-
needs: [lint-test]
62-
uses: libops/.github/.github/workflows/build-push.yaml@3bdb895c3fd05490b0c6a1341e630ac554842468 # main
63+
uses: libops/.github/.github/workflows/build-push.yaml@a86300fb8020d0f7141bb9f833d89b5dbd7aa4d7 # guarded-signed-image-publisher
6364
with:
64-
docker-registry: us-docker.pkg.dev/libops-images/public
65+
ref: ${{ github.sha }}
66+
expected-main-sha: ${{ github.ref == 'refs/heads/main' && github.sha || '' }}
67+
additional-gar-registry: us-docker.pkg.dev/libops-images/public
68+
scan: true
69+
sign: true
70+
certificate-identity: https://github.com/libops/.github/.github/workflows/build-push.yaml@a86300fb8020d0f7141bb9f833d89b5dbd7aa4d7
6571
permissions:
6672
contents: read
73+
packages: write
6774
id-token: write
68-
secrets: inherit
75+
secrets:
76+
GCLOUD_OIDC_POOL: ${{ secrets.GCLOUD_OIDC_POOL }}
77+
GSA: ${{ secrets.GSA }}

Dockerfile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
FROM ghcr.io/libops/base:main@sha256:389706a359c6ba0a4ffb9c3d21c0d909a1c2e5e0c4ab79cb5779129267207049 AS builder
1+
FROM ghcr.io/libops/go:1.26.5@sha256:ea764e85e42a243217c621891123b3fda9374674c29d59785414fc6b15815b3d AS builder
22

33
SHELL ["/bin/ash", "-o", "pipefail", "-ex", "-c"]
44

@@ -14,7 +14,7 @@ COPY pkg ./pkg
1414
RUN --mount=type=cache,target=/root/.cache/go-build \
1515
CGO_ENABLED=0 go build -ldflags="-s -w" -o /app/binary .
1616

17-
FROM ghcr.io/libops/base:main@sha256:389706a359c6ba0a4ffb9c3d21c0d909a1c2e5e0c4ab79cb5779129267207049
17+
FROM ghcr.io/libops/base:3.2.2.0@sha256:0c6bab5444d4758bb977a42c1e5199cba733c7776e1aa006f8021108a785c547
1818

1919
COPY --from=builder /app/binary /app/binary
2020

ci/workflow_contract_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package ci
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"regexp"
7+
"runtime"
8+
"strings"
9+
"testing"
10+
)
11+
12+
const sharedPublisherSHA = "a86300fb8020d0f7141bb9f833d89b5dbd7aa4d7"
13+
14+
func repositoryRoot(t *testing.T) string {
15+
t.Helper()
16+
_, current, _, ok := runtime.Caller(0)
17+
if !ok {
18+
t.Fatal("resolve test source path")
19+
}
20+
return filepath.Dir(filepath.Dir(current))
21+
}
22+
23+
func readRepositoryFile(t *testing.T, path ...string) string {
24+
t.Helper()
25+
content, err := os.ReadFile(filepath.Join(append([]string{repositoryRoot(t)}, path...)...))
26+
if err != nil {
27+
t.Fatalf("read %s: %v", filepath.Join(path...), err)
28+
}
29+
return string(content)
30+
}
31+
32+
func TestImagePublicationUsesGuardedSharedContract(t *testing.T) {
33+
workflow := readRepositoryFile(t, ".github", "workflows", "lint-test-build.yml")
34+
for _, required := range []string{
35+
"libops/.github/.github/workflows/build-push.yaml@" + sharedPublisherSHA,
36+
"additional-gar-registry: us-docker.pkg.dev/libops-images/public",
37+
"expected-main-sha:",
38+
"scan: true",
39+
"sign: true",
40+
"certificate-identity: https://github.com/libops/.github/.github/workflows/build-push.yaml@" + sharedPublisherSHA,
41+
"GCLOUD_OIDC_POOL: ${{ secrets.GCLOUD_OIDC_POOL }}",
42+
"GSA: ${{ secrets.GSA }}",
43+
} {
44+
if !strings.Contains(workflow, required) {
45+
t.Errorf("image workflow must contain %q", required)
46+
}
47+
}
48+
if strings.Contains(workflow, "secrets: inherit") {
49+
t.Fatal("image workflow must map only its required registry secrets")
50+
}
51+
}
52+
53+
func TestReleaseImageCallMapsOnlyRegistrySecrets(t *testing.T) {
54+
workflow := readRepositoryFile(t, ".github", "workflows", "goreleaser.yml")
55+
if strings.Contains(workflow, "secrets: inherit") {
56+
t.Fatal("release workflow must not pass every repository secret")
57+
}
58+
for _, required := range []string{
59+
"if: github.ref_type == 'tag'",
60+
"uses: ./.github/workflows/lint-test-build.yml",
61+
"CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}",
62+
"GCLOUD_OIDC_POOL: ${{ secrets.GCLOUD_OIDC_POOL }}",
63+
"GSA: ${{ secrets.GSA }}",
64+
} {
65+
if !strings.Contains(workflow, required) {
66+
t.Errorf("release workflow must contain %q", required)
67+
}
68+
}
69+
}
70+
71+
func TestDockerfilePinsPublishedBuildkitImages(t *testing.T) {
72+
dockerfile := readRepositoryFile(t, "Dockerfile")
73+
for _, pattern := range []string{
74+
`FROM ghcr\.io/libops/go:[^\s]+@sha256:[0-9a-f]{64} AS builder`,
75+
`FROM ghcr\.io/libops/base:[0-9][^\s]*@sha256:[0-9a-f]{64}`,
76+
} {
77+
if !regexp.MustCompile(pattern).MatchString(dockerfile) {
78+
t.Errorf("Dockerfile must match %q", pattern)
79+
}
80+
}
81+
if strings.Contains(dockerfile, "ghcr.io/libops/base:main") {
82+
t.Fatal("runtime base must use a released version tag")
83+
}
84+
}

main.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,13 @@ func newHandler(c *config.Config, backend http.Handler) http.Handler {
148148
// requests can then be cancelled cleanly during disconnect or shutdown.
149149
powerCtx, powerCancel := context.WithTimeout(r.Context(), time.Duration(c.PowerOnTimeout)*time.Second)
150150
err = c.Machine.PowerOnWithCooldown(powerCtx, c.PowerOnCooldown)
151+
powerTimedOut := powerCtx.Err() == context.DeadlineExceeded && r.Context().Err() == nil
151152
powerCancel()
152153
if err != nil {
153154
slog.Error("Power-on attempt failed", "err", err)
154-
w.Header().Set("Retry-After", "5")
155+
if powerTimedOut {
156+
w.Header().Set("Retry-After", "5")
157+
}
155158
http.Error(w, "Backend not available", http.StatusServiceUnavailable)
156159
return
157160
}

main_test.go

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
"github.com/libops/ppb/pkg/machine"
1717
)
1818

19-
func TestHandlerPowerFailureReturnsRetryableUnavailable(t *testing.T) {
19+
func TestHandlerPermanentPowerFailureOmitsRetryAfter(t *testing.T) {
2020
t.Parallel()
2121

2222
_, allowed, err := net.ParseCIDR("127.0.0.1/32")
@@ -38,14 +38,53 @@ func TestHandlerPowerFailureReturnsRetryableUnavailable(t *testing.T) {
3838

3939
handler.ServeHTTP(recorder, request)
4040

41+
if recorder.Code != http.StatusServiceUnavailable {
42+
t.Fatalf("status = %d, want %d", recorder.Code, http.StatusServiceUnavailable)
43+
}
44+
if got := recorder.Header().Get("Retry-After"); got != "" {
45+
t.Fatalf("Retry-After = %q, want omitted for a permanent power failure", got)
46+
}
47+
if backendCalled {
48+
t.Fatal("backend handler ran after power-on failure")
49+
}
50+
}
51+
52+
func TestHandlerPowerTimeoutReturnsRetryableUnavailable(t *testing.T) {
53+
t.Parallel()
54+
55+
_, allowed, err := net.ParseCIDR("127.0.0.1/32")
56+
if err != nil {
57+
t.Fatal(err)
58+
}
59+
machine := machine.NewGceMachine()
60+
if err := machine.Lock.Acquire(context.Background(), 1); err != nil {
61+
t.Fatal(err)
62+
}
63+
defer machine.Lock.Release(1)
64+
65+
backendCalled := false
66+
handler := newHandler(&config.Config{
67+
AllowedIps: []config.IPNet{{IPNet: allowed}},
68+
PowerOnCooldown: 30,
69+
PowerOnTimeout: 1,
70+
Machine: machine,
71+
}, http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
72+
backendCalled = true
73+
}))
74+
request := httptest.NewRequest(http.MethodPost, "http://example.test/", strings.NewReader("must-not-be-dispatched"))
75+
request.RemoteAddr = "127.0.0.1:12345"
76+
recorder := httptest.NewRecorder()
77+
78+
handler.ServeHTTP(recorder, request)
79+
4180
if recorder.Code != http.StatusServiceUnavailable {
4281
t.Fatalf("status = %d, want %d", recorder.Code, http.StatusServiceUnavailable)
4382
}
4483
if got := recorder.Header().Get("Retry-After"); got != "5" {
4584
t.Fatalf("Retry-After = %q, want 5", got)
4685
}
4786
if backendCalled {
48-
t.Fatal("backend handler ran after power-on failure")
87+
t.Fatal("backend handler ran after power-on timeout")
4988
}
5089
}
5190

0 commit comments

Comments
 (0)