Skip to content

Commit 5e2f1b7

Browse files
IAM-marcoclaudecursoragentadlerhurst
authored
chore: run spanner integration tests against a real test instance (#604) (#629)
## Summary The Cloud Spanner emulator only supports one transaction at a time, so concurrent `spanner_integration` tests are flaky (`Transaction ... aborted due to another transaction getting priority. The emulator only supports one transaction at a time`). This lets the integration suites run against a real, long-lived Spanner test instance, giving each run its own isolated database. Fixes #604. - New `internal/storage/database/dialect/spanner/testdb` helper: `Provision` creates a uniquely named database on an existing instance (via `ZITADEL_TEST_SPANNER_INSTANCE=projects/<p>/instances/<i>`, ADC auth) and drops it on teardown. Database ids are `itest_<run-id>_<random>`, always including a random suffix so the two test binaries in one CI run never collide. - Emulator path refactored to reuse the same `CreateDatabase` helper (emulator kept as the local default). - Both suites (repository + API integration) wired for the new mode. Precedence: `ZITADEL_TEST_SPANNER_INSTANCE` > `ZITADEL_TEST_SPANNER_URL` > emulator container > Postgres. - CI authenticates to Google Cloud with Workload Identity Federation on trusted runs and targets the shared instance; fork PRs fall back to the emulator. The auth + spanner steps run last in the job so their (currently expected) failure does not skip the unrelated build/journey/e2e steps. - `CONTRIBUTING.md` documents the new env var and precedence. > [!IMPORTANT] > **The `Authenticate to Google Cloud (Spanner test instance)` step is expected to FAIL on this PR** and will keep failing on trusted runs until the GCP infrastructure and `GCP_*` secrets are provisioned (see prerequisite below). This is intentional: a red check is a visible signal that the real test instance is not yet configured, rather than silently masking it. It runs last so every unrelated step still executes and validates. ## Validation - `gofmt -l` on all touched files: clean. - `go vet -tags spanner_integration ./internal/storage/database/dialect/spanner/... ./internal/storage/database/repository/ ./internal/storage/database/dbtest/ ./internal/api/integration_test/`: clean. - `go vet -tags postgres_integration ./internal/storage/database/repository/` (stub path): clean. - ci.yml validated as well-formed YAML. - Not run: the integration suite itself was not executed locally (no Docker for the emulator path / no GCP credentials for the instance path). Relying on CI's emulator run for now. ## Release notes / changeset - No changeset required — no shipped behavior changed (test-only Go code under the `spanner_integration` build tag, CI workflow, and docs). ## Notes **CI auth choice:** Workload Identity Federation (keyless) was chosen. The considered alternative was a service-account JSON key stored as a GitHub secret: simpler to set up, but a long-lived credential to manage and rotate. WIF avoids the standing secret. **One-time infra prerequisite (not in this PR).** Before trusted CI actually targets the real instance, ops must provision: 1. A Google Cloud test project with the Spanner API enabled. 2. One small Spanner instance (minimal processing units / regional config). 3. A service account with **Spanner Database Admin** (create/drop databases) + **Spanner Database User** (read/write). 4. A Workload Identity Federation pool + provider bound to `zitadel/nextgen`; grant the SA `roles/iam.workloadIdentityUser`. 5. GitHub Actions config: secrets `GCP_WORKLOAD_IDENTITY_PROVIDER` and `GCP_SERVICE_ACCOUNT`, and variable `SPANNER_TEST_INSTANCE = projects/<project>/instances/<instance>`. Until these exist, the auth step fails by design on trusted runs (see the note above). Because it runs last and the trailing artifact-upload steps use `always()`/`failure()`, no unrelated step is skipped or fails on its account. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Silvan <adlerhurst@users.noreply.github.com> Co-authored-by: Silvan <27845747+adlerhurst@users.noreply.github.com>
1 parent 7ea32f8 commit 5e2f1b7

6 files changed

Lines changed: 182 additions & 42 deletions

File tree

.github/workflows/ci.yml

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ jobs:
1818
# which inject DEPOT_CACHE_TOKEN independently of GitHub secret filtering.
1919
runs-on: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork && 'ubuntu-24.04' || 'depot-ubuntu-24.04-16' }}
2020
timeout-minutes: 120
21+
permissions:
22+
contents: read
23+
# Required to mint the OIDC token for Workload Identity Federation when
24+
# authenticating to the Spanner test instance (trusted runs only).
25+
id-token: write
2126
env:
2227
# Moon marks failed tasks by color alone, so a failed `moon ci` can end at
2328
# "Tasks: 1 failed" with no named target. The summary names them in plain
@@ -84,8 +89,25 @@ jobs:
8489
if: ${{ steps.ci-mode.outputs.mode == 'full' }}
8590
run: moon run server:test-postgres
8691

87-
- name: Run Go integration tests (spanner)
88-
if: ${{ steps.ci-mode.outputs.mode == 'full' }}
92+
# Auth when SPANNER_TEST_INSTANCE is set (trusted runs). Vars are safe in
93+
# if:; secrets stay only in with:. Unset var / forks → emulator step.
94+
# Auth hard-fails if the var is set but WIF is broken (no silent fallback).
95+
- name: Authenticate to Google Cloud (Spanner test instance)
96+
id: spanner-auth
97+
if: ${{ steps.ci-mode.outputs.mode == 'full' && (github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork) && vars.SPANNER_TEST_INSTANCE != '' }}
98+
uses: google-github-actions/auth@v2
99+
with:
100+
workload_identity_provider: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }}
101+
service_account: ${{ secrets.GCP_SERVICE_ACCOUNT }}
102+
103+
- name: Run Go integration tests (spanner emulator)
104+
if: ${{ steps.ci-mode.outputs.mode == 'full' && steps.spanner-auth.outcome != 'success' }}
105+
run: moon run server:test-spanner
106+
107+
- name: Run Go integration tests (spanner test instance)
108+
if: ${{ steps.ci-mode.outputs.mode == 'full' && steps.spanner-auth.outcome == 'success' }}
109+
env:
110+
ZITADEL_TEST_SPANNER_INSTANCE: ${{ vars.SPANNER_TEST_INSTANCE }}
89111
run: moon run server:test-spanner
90112

91113
- name: Build release snapshot without container

CONTRIBUTING.md

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,8 @@ Docker-outside-of-Docker setup. Run them with the same commands CI's
291291
# Postgres
292292
go test -v -tags postgres_integration -timeout=10m ./...
293293

294-
# Spanner
295-
go test -v -tags spanner_integration -timeout=10m ./...
294+
# Spanner (prefer the Moon task — see emulator note below)
295+
moon run server:test-spanner
296296
```
297297

298298
To run the integration tests against a database you manage instead of
@@ -302,6 +302,21 @@ these and connects to your database instead of starting a container, so
302302
`go test -tags … ./...` needs no Docker. Point it at a throwaway database —
303303
the suites run migrations that create the `zitadel_nextgen` schema.
304304

305+
The Spanner emulator only supports one transaction at a time, so concurrent
306+
integration tests are flaky against it. `moon run server:test-spanner`
307+
therefore passes `-parallel 1 -p 1` whenever `ZITADEL_TEST_SPANNER_INSTANCE`
308+
is unset (the default for local/OSS contributors). To run against a real,
309+
long-lived Spanner test instance instead, set `ZITADEL_TEST_SPANNER_INSTANCE`
310+
to an instance path (`projects/<project>/instances/<instance>`). The suites
311+
then provision a uniquely named database on that instance before the run and
312+
drop it afterwards, and the Moon task keeps normal go test parallelism.
313+
Authentication uses Application Default Credentials — locally run
314+
`gcloud auth application-default login`. CI authenticates via Workload
315+
Identity Federation when `SPANNER_TEST_INSTANCE` is set, and labels the job
316+
step as emulator vs test instance accordingly. Precedence when multiple are
317+
set: `ZITADEL_TEST_SPANNER_INSTANCE` > `ZITADEL_TEST_SPANNER_URL` > emulator
318+
container.
319+
305320
### Demo end-to-end suites
306321

307322
These tests start real servers and require a browser install, so they are opt-in

apps/server/moon.yml

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ stack: "backend"
66
fileGroups:
77
serverInputs:
88
- "/**/*.go"
9+
# Embedded goose migrations (//go:embed sql/*.sql); without these Moon can
10+
# cache-hit test/build after schema-only changes while Go would rebuild.
11+
- "/**/*.sql"
912
- "/go.mod"
1013
- "/go.sum"
1114
- "/api/**/*.yaml"
@@ -106,20 +109,20 @@ tasks:
106109
runInCI: true
107110

108111
test-spanner:
109-
command: "go"
110-
args:
111-
- "test"
112-
- "-v"
113-
- "-tags"
114-
- "spanner_integration"
115-
- "-timeout"
116-
- "10m"
117-
- "./..."
112+
# Serializes (-parallel 1 -p 1) when ZITADEL_TEST_SPANNER_INSTANCE is unset
113+
# so the single-transaction emulator stays reliable.
114+
script: |
115+
set -euo pipefail
116+
extra=()
117+
if [ -z "${ZITADEL_TEST_SPANNER_INSTANCE:-}" ]; then
118+
extra+=(-parallel 1 -p 1)
119+
fi
120+
go test -v -tags spanner_integration -timeout 10m "${extra[@]}" ./...
118121
inputs:
119122
- "@group(serverInputs)"
120123
options:
121124
runFromWorkspaceRoot: true
122-
# Invoked explicitly by the CI "Run Go integration tests (spanner)" step.
125+
# Invoked by the CI Spanner emulator / test-instance steps.
123126
# runInCI must be true, otherwise moon hides the task from `moon run` in CI
124127
# ("No tasks found"). It is not pulled into `moon ci :...:test` because that
125128
# selects the `test` task by name, not `test-spanner`.

internal/storage/v2/dbtest/spanner.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@ import (
1010
)
1111

1212
// Spanner returns a connected v2 pool for the Spanner integration tests.
13-
// If ZITADEL_TEST_SPANNER_URL is set, it connects to that database (no
14-
// Docker required); otherwise it starts a Cloud Spanner emulator
15-
// testcontainer. The returned stop function is always non-nil and safe to
16-
// defer.
13+
// Bring-up precedence is owned by testdb.SpannerDSN. The returned stop
14+
// function is always non-nil and safe to defer.
1715
func Spanner(ctx context.Context) (Pool, func(), error) {
1816
dsn, stop, err := testdb.SpannerDSN(ctx)
1917
if err != nil {
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
//go:build spanner_integration
2+
3+
package testdb
4+
5+
import (
6+
"context"
7+
"crypto/rand"
8+
"encoding/hex"
9+
"fmt"
10+
"log/slog"
11+
"os"
12+
"strings"
13+
14+
database_admin "cloud.google.com/go/spanner/admin/database/apiv1"
15+
"cloud.google.com/go/spanner/admin/database/apiv1/databasepb"
16+
"google.golang.org/api/option"
17+
)
18+
19+
// instanceEnv names a shared Spanner instance
20+
// (projects/<project>/instances/<instance>). When set, suites provision a
21+
// uniquely named database on that instance instead of starting the emulator.
22+
const instanceEnv = "ZITADEL_TEST_SPANNER_INSTANCE"
23+
24+
func provision(ctx context.Context) (string, func(), error) {
25+
project, instance, err := parseInstancePath(strings.TrimSpace(os.Getenv(instanceEnv)))
26+
if err != nil {
27+
return "", func() {}, err
28+
}
29+
30+
dbID := uniqueDatabaseID()
31+
if err := createDatabase(ctx, project, instance, dbID); err != nil {
32+
return "", func() {}, fmt.Errorf("unable to create Spanner test database %q: %w", dbID, err)
33+
}
34+
35+
drop := func() {
36+
if err := dropDatabase(context.Background(), project, instance, dbID); err != nil {
37+
slog.Error("unable to drop Spanner test database", "database", dbID, "err", err)
38+
}
39+
}
40+
41+
dsn := fmt.Sprintf("projects/%s/instances/%s/databases/%s", project, instance, dbID)
42+
slog.Info("provisioned Spanner test database", "dsn", dsn, "run_id", os.Getenv("GITHUB_RUN_ID"))
43+
return dsn, drop, nil
44+
}
45+
46+
func createDatabase(ctx context.Context, project, instance, dbID string, opts ...option.ClientOption) error {
47+
client, err := database_admin.NewDatabaseAdminClient(ctx, opts...)
48+
if err != nil {
49+
return fmt.Errorf("database admin client: %w", err)
50+
}
51+
defer client.Close()
52+
53+
op, err := client.CreateDatabase(ctx, &databasepb.CreateDatabaseRequest{
54+
Parent: fmt.Sprintf("projects/%s/instances/%s", project, instance),
55+
CreateStatement: "CREATE DATABASE `" + dbID + "`",
56+
})
57+
if err != nil {
58+
return fmt.Errorf("create database: %w", err)
59+
}
60+
if _, err = op.Wait(ctx); err != nil {
61+
return fmt.Errorf("wait for database: %w", err)
62+
}
63+
return nil
64+
}
65+
66+
func dropDatabase(ctx context.Context, project, instance, dbID string, opts ...option.ClientOption) error {
67+
client, err := database_admin.NewDatabaseAdminClient(ctx, opts...)
68+
if err != nil {
69+
return fmt.Errorf("database admin client: %w", err)
70+
}
71+
defer client.Close()
72+
73+
err = client.DropDatabase(ctx, &databasepb.DropDatabaseRequest{
74+
Database: fmt.Sprintf("projects/%s/instances/%s/databases/%s", project, instance, dbID),
75+
})
76+
if err != nil {
77+
return fmt.Errorf("drop database: %w", err)
78+
}
79+
return nil
80+
}
81+
82+
func parseInstancePath(path string) (project, instance string, err error) {
83+
parts := strings.Split(path, "/")
84+
if len(parts) != 4 || parts[0] != "projects" || parts[2] != "instances" || parts[1] == "" || parts[3] == "" {
85+
return "", "", fmt.Errorf("%s must be of the form projects/<project>/instances/<instance>, got %q", instanceEnv, path)
86+
}
87+
return parts[1], parts[3], nil
88+
}
89+
90+
// uniqueDatabaseID builds a Spanner database ID (2–30 chars, [a-z0-9_-], no
91+
// trailing hyphen). Entropy comes first so a 30-char clamp never drops
92+
// uniqueness when GITHUB_RUN_ID is long; a short run-id suffix keeps orphans
93+
// traceable when present.
94+
func uniqueDatabaseID() string {
95+
id := "itest_" + randomToken()
96+
if runID := os.Getenv("GITHUB_RUN_ID"); runID != "" {
97+
budget := 30 - len(id) - 1
98+
if budget > 0 {
99+
if len(runID) > budget {
100+
runID = runID[len(runID)-budget:]
101+
}
102+
id = id + "_" + runID
103+
}
104+
}
105+
return strings.TrimRight(id, "-")
106+
}
107+
108+
func randomToken() string {
109+
b := make([]byte, 4)
110+
if _, err := rand.Read(b); err != nil {
111+
panic("crypto/rand unavailable: " + err.Error())
112+
}
113+
return hex.EncodeToString(b)
114+
}

internal/storage/v2/testdb/spanner.go

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@ import (
77
"fmt"
88
"log/slog"
99
"os"
10+
"strings"
1011
"time"
1112

12-
database_admin "cloud.google.com/go/spanner/admin/database/apiv1"
13-
"cloud.google.com/go/spanner/admin/database/apiv1/databasepb"
1413
instance_admin "cloud.google.com/go/spanner/admin/instance/apiv1"
1514
"cloud.google.com/go/spanner/admin/instance/apiv1/instancepb"
1615
"github.com/testcontainers/testcontainers-go"
@@ -26,11 +25,17 @@ const (
2625
testDatabase = "test-database"
2726
)
2827

29-
// SpannerDSN starts the Cloud Spanner GoogleSQL emulator (unless
30-
// ZITADEL_TEST_SPANNER_URL is set), creates the test instance/database, sets
31-
// SPANNER_EMULATOR_HOST, and returns the database DSN plus a stop func that
32-
// clears the env var and terminates the container.
28+
// SpannerDSN returns a Spanner database DSN for integration tests. Precedence:
29+
// if ZITADEL_TEST_SPANNER_INSTANCE is set it provisions a fresh, uniquely named
30+
// database on that shared instance (the returned stop drops it); else if
31+
// ZITADEL_TEST_SPANNER_URL is set it returns that DSN; otherwise it starts a
32+
// Cloud Spanner emulator testcontainer, creates the test instance/database,
33+
// sets SPANNER_EMULATOR_HOST, and returns the database DSN plus a stop func
34+
// that clears the env var and terminates the container.
3335
func SpannerDSN(ctx context.Context) (string, func(), error) {
36+
if strings.TrimSpace(os.Getenv(instanceEnv)) != "" {
37+
return provision(ctx)
38+
}
3439
if url := os.Getenv("ZITADEL_TEST_SPANNER_URL"); url != "" {
3540
return url, func() {}, nil
3641
}
@@ -130,22 +135,5 @@ func tryCreateInstanceAndDatabase(ctx context.Context, opts []option.ClientOptio
130135
return fmt.Errorf("wait for instance: %w", err)
131136
}
132137

133-
dbClient, err := database_admin.NewDatabaseAdminClient(ctx, opts...)
134-
if err != nil {
135-
return fmt.Errorf("database admin client: %w", err)
136-
}
137-
defer dbClient.Close()
138-
139-
dbOp, err := dbClient.CreateDatabase(ctx, &databasepb.CreateDatabaseRequest{
140-
Parent: "projects/" + testProject + "/instances/" + testInstance,
141-
CreateStatement: "CREATE DATABASE `" + testDatabase + "`",
142-
})
143-
if err != nil {
144-
return fmt.Errorf("create database: %w", err)
145-
}
146-
if _, err = dbOp.Wait(ctx); err != nil {
147-
return fmt.Errorf("wait for database: %w", err)
148-
}
149-
150-
return nil
138+
return createDatabase(ctx, testProject, testInstance, testDatabase, opts...)
151139
}

0 commit comments

Comments
 (0)