diff --git a/.github/actions/start-services/action.yml b/.github/actions/start-services/action.yml index bc8745b540..2cdf9255f8 100644 --- a/.github/actions/start-services/action.yml +++ b/.github/actions/start-services/action.yml @@ -121,24 +121,22 @@ runs: PERSISTENT_VOLUME_MOUNTS="test-volume-type:${test_volume_dir}" export PERSISTENT_VOLUME_MOUNTS - # The backgrounded warm-up pull in start-databases discards its exit - # status; this synchronous call is the actual guarantee (near-instant - # when the warm-up succeeded) so a rate-limited pull retries instead - # of failing the collector's 30s health window. The tag comes from - # the Makefile that runs the image. + # The warm-up pull in start-databases discards its exit status; this + # synchronous call is the guarantee (near-instant when warm). Tag + # comes from the Makefile that runs the image, so bumps can't strand it. OTEL_IMAGE=$(grep -oE 'otel/opentelemetry-collector-contrib:[0-9.]+' packages/otel-collector/Makefile | head -1) ./scripts/pull-retry.sh "$OTEL_IMAGE" - echo "Start otel-collector" - ./scripts/start-service.sh "OtelCollector" packages/otel-collector run ~/logs/otel-collector.log http://localhost:13133/healthz + # Two concurrent boot phases: otel + orchestrator (OTLP exporters + # retry until the collector is up), then API + client-proxy once the + # orchestrator is healthy — API must not start before it. + ./scripts/start-service.sh start "OtelCollector" packages/otel-collector run ~/logs/otel-collector.log + ./scripts/start-service.sh start "Orchestrator" packages/orchestrator run-debug ~/logs/orchestrator.log + ./scripts/start-service.sh wait "Orchestrator" ~/logs/orchestrator.log http://localhost:5008/health + ./scripts/start-service.sh wait "OtelCollector" ~/logs/otel-collector.log http://localhost:13133/healthz - - echo "Start orchestrator" - ./scripts/start-service.sh "Orchestrator" packages/orchestrator run-debug ~/logs/orchestrator.log http://localhost:5008/health - - echo "Start API" - STARTUP_TIMEOUT=60 ./scripts/start-service.sh "API" packages/api run ~/logs/api.log http://localhost:3000/health - - # Start client-proxy (needed for auto-resume tests) - ./scripts/start-service.sh "ClientProxy" packages/client-proxy run ~/logs/client-proxy.log http://localhost:3003 + ./scripts/start-service.sh start "API" packages/api run ~/logs/api.log + ./scripts/start-service.sh start "ClientProxy" packages/client-proxy run ~/logs/client-proxy.log + STARTUP_TIMEOUT=60 ./scripts/start-service.sh wait "API" ~/logs/api.log http://localhost:3000/health + ./scripts/start-service.sh wait "ClientProxy" ~/logs/client-proxy.log http://localhost:3003 shell: bash diff --git a/scripts/start-service.sh b/scripts/start-service.sh index b567dcc46d..4b9e932614 100755 --- a/scripts/start-service.sh +++ b/scripts/start-service.sh @@ -1,30 +1,66 @@ #!/bin/bash -# Default timeout, override with STARTUP_TIMEOUT env -TIMEOUT=${STARTUP_TIMEOUT:-30} +# Start a service and/or wait for its health endpoint. +# +# Modes: +# start-service.sh start +# Kick the service off in the background and return immediately. +# start-service.sh wait +# Poll the health URL until healthy (STARTUP_TIMEOUT, default 30s); +# on timeout print the log tail and fail. +# start-service.sh +# Legacy form: start + wait in one call. +# +# Splitting start from wait lets independent services boot concurrently: +# `start A; start B; wait A; wait B` boots B while A is being waited on. -if [ "$#" -ne 5 ]; then - echo "Usage: $0 " - exit 1 -fi +set -uo pipefail -NAME="$1" -MAKE_PATH="$2" -MAKE_COMMAND="$3" -LOG_FILE="$4" -HEALTH_URL="$5" +# Default timeout, override with STARTUP_TIMEOUT env +TIMEOUT=${STARTUP_TIMEOUT:-30} -echo "Starting $NAME..." -make -C "$MAKE_PATH" "$MAKE_COMMAND" 2>&1 | tee "$LOG_FILE" & +start() { + local name="$1" make_path="$2" make_command="$3" log_file="$4" + echo "Starting $name..." + make -C "$make_path" "$make_command" 2>&1 | tee "$log_file" & +} -echo "Waiting for $NAME to become healthy at $HEALTH_URL (timeout: $TIMEOUT seconds)..." -for ((i = 0; i < TIMEOUT; i++)); do - if curl -s -o /dev/null -w "%{http_code}" "$HEALTH_URL" | grep -q 200; then - echo "$NAME is healthy and running." - exit 0 - fi - sleep 1 -done +wait_healthy() { + local name="$1" log_file="$2" health_url="$3" + echo "Waiting for $name to become healthy at $health_url (timeout: $TIMEOUT seconds)..." + for ((i = 0; i < TIMEOUT; i++)); do + if curl -s -o /dev/null -w "%{http_code}" "$health_url" | grep -q 200; then + echo "$name is healthy and running." + return 0 + fi + sleep 1 + done + echo "$name failed to become healthy in time. Last log lines:" + tail -30 "$log_file" 2>/dev/null || true + return 1 +} -echo "$NAME failed to become healthy in time." -exit 1 \ No newline at end of file +case "${1:-}" in + start) + if [ "$#" -ne 5 ]; then + echo "Usage: $0 start " + exit 1 + fi + start "$2" "$3" "$4" "$5" + ;; + wait) + if [ "$#" -ne 4 ]; then + echo "Usage: $0 wait " + exit 1 + fi + wait_healthy "$2" "$3" "$4" + ;; + *) + if [ "$#" -ne 5 ]; then + echo "Usage: $0 " + exit 1 + fi + start "$1" "$2" "$3" "$4" + wait_healthy "$1" "$4" "$5" + ;; +esac diff --git a/tests/integration/Makefile b/tests/integration/Makefile index 728c8cebe9..23a29a354e 100644 --- a/tests/integration/Makefile +++ b/tests/integration/Makefile @@ -102,6 +102,7 @@ check-tests-allowlist: fi; \ done; \ echo "allow-list resolves; stale named and wildcard entries are rejected" + @./scripts/check-allowlist-completeness.sh # CI entrypoint: run one package shard (or the whole suite with the default # TEST_SHARD=all). TESTS_ONLY narrows the run to the allow-list within the diff --git a/tests/integration/internal/tests/api/sandboxes/sandbox_detail_test.go b/tests/integration/internal/tests/api/sandboxes/sandbox_detail_test.go index ed35b2f4a9..4ac217c823 100644 --- a/tests/integration/internal/tests/api/sandboxes/sandbox_detail_test.go +++ b/tests/integration/internal/tests/api/sandboxes/sandbox_detail_test.go @@ -34,6 +34,7 @@ func TestSandboxDetailRunning(t *testing.T) { assert.Equal(t, "base", *returnedSbx.Alias) } +// compression-tests:excluded subject is the detail API surface; autopause config is fixture func TestSandboxDetailReturnsLifecycleAndNetworkConfig(t *testing.T) { t.Parallel() c := setup.GetAPIClient() @@ -87,6 +88,7 @@ func TestSandboxDetailReturnsLifecycleAndNetworkConfig(t *testing.T) { assertDetail(t, api.Paused) } +// compression-tests:excluded subject is the detail API surface; pause is fixture func TestSandboxDetailPaused(t *testing.T) { t.Parallel() c := setup.GetAPIClient() @@ -106,6 +108,7 @@ func TestSandboxDetailPaused(t *testing.T) { assert.Equal(t, "base", *returnedSbx.Alias) } +// compression-tests:excluded subject is the detail API surface; pause is fixture func TestSandboxDetailPausingSandbox(t *testing.T) { t.Parallel() c := setup.GetAPIClient() diff --git a/tests/integration/internal/tests/api/sandboxes/sandbox_internet_test.go b/tests/integration/internal/tests/api/sandboxes/sandbox_internet_test.go index 884d9303ee..d4fe810118 100644 --- a/tests/integration/internal/tests/api/sandboxes/sandbox_internet_test.go +++ b/tests/integration/internal/tests/api/sandboxes/sandbox_internet_test.go @@ -50,6 +50,7 @@ func TestInternetAccess(t *testing.T) { } } +// compression-tests:excluded subject is internet egress behaviour, excluded per the allow-list criterion func TestInternetAccessResumedSbx(t *testing.T) { t.Parallel() diff --git a/tests/integration/internal/tests/api/sandboxes/sandbox_kill_test.go b/tests/integration/internal/tests/api/sandboxes/sandbox_kill_test.go index 58924fe7bb..4f4e246268 100644 --- a/tests/integration/internal/tests/api/sandboxes/sandbox_kill_test.go +++ b/tests/integration/internal/tests/api/sandboxes/sandbox_kill_test.go @@ -12,6 +12,7 @@ import ( "github.com/e2b-dev/infra/tests/integration/internal/utils" ) +// compression-tests:excluded subject is kill semantics; pause/resume set up the states to kill func TestSandboxKill(t *testing.T) { t.Parallel() c := setup.GetAPIClient() diff --git a/tests/integration/internal/tests/api/sandboxes/sandbox_list_test.go b/tests/integration/internal/tests/api/sandboxes/sandbox_list_test.go index b22d2a973c..7d32277087 100644 --- a/tests/integration/internal/tests/api/sandboxes/sandbox_list_test.go +++ b/tests/integration/internal/tests/api/sandboxes/sandbox_list_test.go @@ -127,6 +127,7 @@ func TestSandboxListRunning_NoMetadata(t *testing.T) { assert.Contains(t, sandboxIds, sandboxID) } +// compression-tests:excluded subject is the list API surface; pause is fixture func TestSandboxListPaused(t *testing.T) { t.Parallel() c := setup.GetAPIClient() @@ -162,6 +163,7 @@ func TestSandboxListPaused(t *testing.T) { assert.True(t, found) } +// compression-tests:excluded subject is the list API surface; pause is fixture func TestSandboxListPausing(t *testing.T) { t.Parallel() c := setup.GetAPIClient() @@ -218,6 +220,7 @@ func TestSandboxListPausing(t *testing.T) { require.NoError(t, err) } +// compression-tests:excluded subject is the list API surface; pause is fixture func TestSandboxListPaused_NoMetadata(t *testing.T) { t.Parallel() c := setup.GetAPIClient() @@ -393,6 +396,7 @@ func TestSandboxListPaginationRunningLargerLimit(t *testing.T) { //nolint:tparal }) } +// compression-tests:excluded subject is list pagination; pause is fixture func TestSandboxListPaginationPaused(t *testing.T) { t.Parallel() c := setup.GetAPIClient() @@ -443,6 +447,7 @@ func TestSandboxListPaginationPaused(t *testing.T) { assert.Empty(t, nextToken) } +// compression-tests:excluded subject is list pagination; pause is fixture func TestSandboxListPaginationRunningAndPaused(t *testing.T) { t.Parallel() c := setup.GetAPIClient() diff --git a/tests/integration/internal/tests/api/sandboxes/sandbox_network_out_test.go b/tests/integration/internal/tests/api/sandboxes/sandbox_network_out_test.go index 36229681c5..8cb1b83fe9 100644 --- a/tests/integration/internal/tests/api/sandboxes/sandbox_network_out_test.go +++ b/tests/integration/internal/tests/api/sandboxes/sandbox_network_out_test.go @@ -235,6 +235,7 @@ func TestEgressFirewallAllowAndBlockCombination(t *testing.T) { } // TestEgressFirewallPersistsAfterResume tests that network config persists after pause/resume +// compression-tests:excluded subject is egress firewall behaviour, excluded per the allow-list criterion func TestEgressFirewallPersistsAfterResume(t *testing.T) { t.Parallel() templateID := ensureNetworkTestTemplate(t) @@ -708,6 +709,7 @@ func TestEgressFirewallHTTPSByIPNoHostname(t *testing.T) { } // TestEgressFirewallDomainPersistsAfterResume tests that domain-based network config persists after pause/resume +// compression-tests:excluded subject is egress firewall behaviour, excluded per the allow-list criterion func TestEgressFirewallDomainPersistsAfterResume(t *testing.T) { t.Parallel() templateID := ensureNetworkTestTemplate(t) diff --git a/tests/integration/internal/tests/api/sandboxes/sandbox_network_update_test.go b/tests/integration/internal/tests/api/sandboxes/sandbox_network_update_test.go index c3fc9db11f..69b89da0c7 100644 --- a/tests/integration/internal/tests/api/sandboxes/sandbox_network_update_test.go +++ b/tests/integration/internal/tests/api/sandboxes/sandbox_network_update_test.go @@ -91,6 +91,7 @@ func verifyConnectivity( // TestUpdateNetworkConfig exercises all update scenarios using a single sandbox. // Subtests run sequentially — each PUT fully replaces the previous config. +// compression-tests:excluded subject is network config surface; pause/resume cycles the states func TestUpdateNetworkConfig(t *testing.T) { //nolint:tparallel // subtests are sequential t.Parallel() diff --git a/tests/integration/internal/tests/api/sandboxes/sandbox_timeout_test.go b/tests/integration/internal/tests/api/sandboxes/sandbox_timeout_test.go index c0bc7e93f9..aa3a3dab7d 100644 --- a/tests/integration/internal/tests/api/sandboxes/sandbox_timeout_test.go +++ b/tests/integration/internal/tests/api/sandboxes/sandbox_timeout_test.go @@ -82,6 +82,7 @@ func TestSandboxTimeout_NotFound(t *testing.T) { assert.Equal(t, http.StatusNotFound, timeoutResp.StatusCode()) } +// compression-tests:excluded subject is the timeout API surface; pause is fixture func TestSandboxSetTimeoutPausingSandbox(t *testing.T) { t.Parallel() c := setup.GetAPIClient() diff --git a/tests/integration/internal/tests/team_test.go b/tests/integration/internal/tests/team_test.go index 57612419ce..99c0b6adc5 100644 --- a/tests/integration/internal/tests/team_test.go +++ b/tests/integration/internal/tests/team_test.go @@ -58,6 +58,7 @@ UPDATE teams SET is_banned = $1 WHERE id = $2 // Mutate / Delete subtests use a synthetic sandbox ID because the // blocked-team check runs before the resource is resolved — we only // care that the request was (not) rejected by the blocked-team policy. +// compression-tests:excluded subject is team-blocking authz; the pause call targets a nonexistent sandbox func TestBlockedTeam(t *testing.T) { t.Parallel() ctx := t.Context() diff --git a/tests/integration/scripts/check-allowlist-completeness.sh b/tests/integration/scripts/check-allowlist-completeness.sh new file mode 100755 index 0000000000..43495008a0 --- /dev/null +++ b/tests/integration/scripts/check-allowlist-completeness.sh @@ -0,0 +1,145 @@ +#!/bin/bash + +# Guards the *missing* direction of scripts/compression-tests.tsv. +# +# select-tests.sh already rejects stale entries (a listed test that no longer +# exists). Nothing catches the opposite rot: a new test on the snapshot +# write/read path that nobody adds to the allow-list silently never runs +# under the compressed configs. This lint closes that hole: +# +# Every top-level test whose body touches a snapshot-lifecycle symbol +# (the SYMBOLS list below) must either appear in the allow-list or carry +# an explicit exclusion marker in the comment block directly above it: +# +# // compression-tests:excluded +# +# per the allow-list's own criterion: excluded is correct when the test's +# *subject* is API surface / egress / proxy routing and pause/resume is +# only a fixture; listed is correct when the subject is writing or +# reading back a snapshot. +# +# Packages the allow-list covers with "*" are exempt (new tests there are +# always selected). Top-level helper functions that touch the symbols must +# themselves be added to SYMBOLS, so tests hiding behind package-local +# wrappers stay visible to this lint. +# +# Known limitation: symbols called through cross-package helpers +# (internal/utils) are invisible here — add the helper's name to SYMBOLS +# when such a wrapper is introduced. + +set -uo pipefail + +cd "$(dirname "$0")/.." + +TSV="scripts/compression-tests.tsv" +TESTS_DIR="internal/tests" +MARKER="compression-tests:excluded" + +# The snapshot write/read surface as seen from the tests. Extend when a new +# lifecycle call or a package-local wrapper around one is introduced. +# NB: character classes ([(]) instead of \( — backslashes do not survive +# awk -v value processing portably. +# Any autopause call counts (variable args and line-broken calls included); +# the scan strips literal WithAutoPause(false)/WithAutoResume(false) from a +# line before matching, so only the explicit opt-out stays quiet. +SYMBOLS='PostSandboxesSandboxIDPauseWithResponse|PostSandboxesSandboxIDResumeWithResponse|PostSandboxesSandboxIDForkWithResponse|PostSandboxesSandboxIDSnapshotsWithResponse|WithAutoPause[(]|WithAutoResume[(]|FsFreeze|Fsfreeze|pauseFilesystemOnly[(]|pauseSandbox[(]|createSnapshotTemplate[(]|startSnapshotInBackground[(]|createSnapshotTemplateWithCleanup[(]' + +fail=0 + +# Packages fully covered by a "*" entry are exempt. +wildcard_pkgs=$(awk -F'\t' '!/^#/ && NF==2 && $2=="*" {print $1}' "$TSV") + +is_wildcarded() { + local pkg="$1" + while IFS= read -r w; do + [ "$pkg" = "$w" ] && return 0 + done <<<"$wildcard_pkgs" + return 1 +} + +is_listed() { + local pkg="$1" name="$2" + grep -qE "^${pkg}[[:space:]]+${name}\$" "$TSV" +} + +while IFS= read -r file; do + pkg=$(dirname "$file") + is_wildcarded "$pkg" && continue + + # Attribute symbol hits to the enclosing column-0 function; remember the + # comment block directly above each function for the exclusion marker. + hits=$(awk -v symre="$SYMBOLS" -v marker="$MARKER" ' + /^\/\// { cbuf = cbuf $0; next } + /^func / { + # Strip an optional receiver so methods attribute by name too. + fn = $0 + sub(/^func +/, "", fn) + sub(/^\([^)]*\) */, "", fn) + sub(/\(.*/, "", fn) + fline = FNR + excluded = (cbuf ~ marker) + cbuf = "" + } + # Column-0 close brace ends the function: symbols between functions + # must not be attributed to the previous one. + /^}/ { fn = "" } + { if (!/^func /) cbuf = "" } + { + probe = $0 + gsub(/WithAutoPause\(false\)|WithAutoResume\(false\)/, "", probe) + } + probe ~ symre && fn != "" && !reported[fn] { + reported[fn] = 1 + print fn "\t" fline "\t" (excluded ? "excluded" : "-") + } + ' "$file") + + [ -z "$hits" ] && continue + + while IFS=$'\t' read -r fn fline excluded; do + if [[ "$fn" != Test* ]]; then + # A package-local helper touches the snapshot surface: require it + # in SYMBOLS so tests calling it are not invisible to this lint. + # Exact whole-entry match ("name[(]"): a substring or prefix match + # would let a wrapper whose name is a prefix of an existing entry + # slip through as already listed. + if ! tr '|' '\n' <<<"$SYMBOLS" | grep -qxF "${fn}[(]"; then + echo "${file}:${fline}: helper ${fn} touches snapshot symbols; add '${fn}[(]' to SYMBOLS in $(basename "$0")" + fail=1 + fi + continue + fi + [ "$excluded" = "excluded" ] && continue + if ! is_listed "$pkg" "$fn"; then + echo "${file}:${fline}: ${fn} touches the snapshot path but is neither in ${TSV} nor marked '// ${MARKER} '" + fail=1 + fi + done <<<"$hits" +done < <(find "$TESTS_DIR" -name '*_test.go' | sort) + +# A marker on a listed test is a contradiction — one of the two must go. +while IFS= read -r file; do + pkg=$(dirname "$file") + awk -v marker="$MARKER" ' + $0 ~ "^// ?" marker { m = 1; next } + /^func Test/ && m { fn = $2; sub(/\(.*/, "", fn); print FNR "\t" fn } + { if (!/^\/\//) m = 0 } + ' "$file" | while IFS=$'\t' read -r fline fn; do + if is_listed "$pkg" "$fn"; then + echo "${file}:${fline}: ${fn} is both allow-listed and marked ${MARKER} — remove one" + exit 9 + fi + done + [ $? -eq 9 ] && fail=1 +done < <(grep -rl "$MARKER" "$TESTS_DIR" --include='*_test.go' 2>/dev/null) + +if [ "$fail" -ne 0 ]; then + echo "" + echo "Snapshot-path tests must run under the compressed configs or opt out" + echo "explicitly. Either add the test to ${TSV} (subject: writing or" + echo "reading back a snapshot) or put '// ${MARKER} ' directly" + echo "above it (subject: API surface / egress / routing; snapshot is fixture)." + exit 1 +fi + +echo "allow-list completeness: every snapshot-path test is listed or explicitly excluded" diff --git a/tests/integration/scripts/compression-tests.tsv b/tests/integration/scripts/compression-tests.tsv index e67ecbae7a..3efadce362 100644 --- a/tests/integration/scripts/compression-tests.tsv +++ b/tests/integration/scripts/compression-tests.tsv @@ -25,7 +25,7 @@ # compressed writer is agnostic to what a layer contains - a handful of builds # covers it, and 58 do not. # -# "*" selects every test in the package. shard-tests.sh fails if an entry no +# "*" selects every test in the package. select-tests.sh fails if an entry no # longer exists, so this list cannot rot into silently reduced coverage. # Snapshot/restore integrity suite - entirely on the pause/resume path. @@ -50,13 +50,18 @@ internal/tests/api/sandboxes TestSandboxPauseNonFound internal/tests/api/sandboxes TestSandboxResume internal/tests/api/sandboxes TestSandboxResumeUnknownSandbox internal/tests/api/sandboxes TestSandboxResumeWithSecuredEnvd +internal/tests/api/sandboxes TestSandboxResume_CrossTeamAccess_Paused +internal/tests/api/sandboxes TestSandboxResume_CrossTeamAccess_Running internal/tests/api/sandboxes TestSandboxResume_FilesystemOnlyReboots internal/tests/api/sandboxes TestSandboxConnect_FilesystemOnlyResumes +internal/tests/api/sandboxes TestSandboxConnect +internal/tests/api/sandboxes TestSandboxConnect_CrossTeamAccess_Paused internal/tests/api/sandboxes TestSandboxFork internal/tests/api/sandboxes TestSandboxFork_Multiple internal/tests/api/sandboxes TestSandboxFork_AlreadyPaused internal/tests/api/sandboxes TestSandboxFork_NotFound internal/tests/api/sandboxes TestSandboxFork_InvalidCount +internal/tests/api/sandboxes TestSandboxFork_CrossTeamAccess internal/tests/api/sandboxes TestSandboxRapidSnapshotForkChain internal/tests/api/sandboxes TestSandboxAutoPausePauseResume internal/tests/api/sandboxes TestSandboxAutoPauseResumePersisted @@ -82,6 +87,7 @@ internal/tests/api/templates TestTemplateBuildWithDifferentSourceImages # Guest filesystem state across freeze/snapshot/resume. internal/tests/envd TestFsFreezeThaw internal/tests/envd TestCACertRotationOnResume +internal/tests/envd TestCACertTrustedAfterFilesystemOnlyReboot internal/tests/envd TestSecureSandboxFilesystemOnlyResumeAuth internal/tests/envd TestAccessAuthorizedPathWithResumedSandboxWithValidAccessToken internal/tests/envd TestAccessAuthorizedPathWithResumedSandboxWithoutAccessToken