From 01bb3b30532f52145f410f6d16b48070a1447c56 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sat, 19 Sep 2026 03:29:51 -0700 Subject: [PATCH] s3api: add Snowflake s3compat API integration tests (#11394) * s3api: add Snowflake s3compat API integration tests Run the upstream snowflakedb/snowflake-s3compat-api-test-suite against a local SeaweedFS server in CI. test/s3/snowflake/run.sh starts weed server with S3 (-s3.autoCreateBucket=false so missing-bucket PUTs return NoSuchBucket), prepares the fixtures the suite needs (versioned bucket, deny-all-policy bucket, >1000-object prefix), clones the suite, patches it to path-style addressing, and runs mvn -Dtest=S3CompatApiTest. The suite also exposed that GetBucketLocation returned 404 NoSuchBucket for a malformed bucket name; validate the name first and return 400 InvalidBucketName like AWS. * test: harden snowflake s3compat runner per review - Pin the upstream suite to a tested commit (SUITE_REV) instead of the moving default branch - Bind the test server to loopback only - Require the AccessDenied error code when verifying the denied bucket - Fix README so go install runs in a subshell - checkout with persist-credentials: false - Make the concurrency group unique per PR, and widen path filters to the storage/operation/wdclient/cluster/pb packages the S3 stack uses * test: advertise loopback ip for snowflake test server -ip.bind 127.0.0.1 alone left the volume server advertising the host's primary address, so chunk uploads were refused. Also set -ip 127.0.0.1 and disable the Iceberg/Lance listeners so the harness is loopback-only and does not collide with other local services. --- .github/workflows/s3-snowflake-tests.yml | 106 +++++++++++++ test/s3/snowflake/README.md | 33 +++++ test/s3/snowflake/prepare.sh | 81 ++++++++++ test/s3/snowflake/run.sh | 140 ++++++++++++++++++ test/s3/snowflake/s3.json | 20 +++ weed/s3api/s3api_bucket_handlers.go | 5 + weed/s3api/s3api_bucket_handlers_misc_test.go | 18 +++ 7 files changed, 403 insertions(+) create mode 100644 .github/workflows/s3-snowflake-tests.yml create mode 100644 test/s3/snowflake/README.md create mode 100755 test/s3/snowflake/prepare.sh create mode 100755 test/s3/snowflake/run.sh create mode 100644 test/s3/snowflake/s3.json diff --git a/.github/workflows/s3-snowflake-tests.yml b/.github/workflows/s3-snowflake-tests.yml new file mode 100644 index 000000000..ec3ec6a32 --- /dev/null +++ b/.github/workflows/s3-snowflake-tests.yml @@ -0,0 +1,106 @@ +name: "Snowflake S3Compat API tests" + +on: + push: + branches: [ master ] + paths: + - 'weed/s3api/**' + - 'weed/filer/**' + - 'weed/server/**' + - 'weed/iam/**' + - 'weed/command/**' + - 'weed/storage/**' + - 'weed/operation/**' + - 'weed/wdclient/**' + - 'weed/cluster/**' + - 'weed/pb/**' + - 'test/s3/snowflake/**' + - 'go.mod' + - 'go.sum' + - '.github/workflows/s3-snowflake-tests.yml' + pull_request: + branches: [ master ] + paths: + - 'weed/s3api/**' + - 'weed/filer/**' + - 'weed/server/**' + - 'weed/iam/**' + - 'weed/command/**' + - 'weed/storage/**' + - 'weed/operation/**' + - 'weed/wdclient/**' + - 'weed/cluster/**' + - 'weed/pb/**' + - 'test/s3/snowflake/**' + - 'go.mod' + - 'go.sum' + - '.github/workflows/s3-snowflake-tests.yml' + +concurrency: + group: ${{ github.event.pull_request.number || github.ref }}/s3-snowflake-tests + cancel-in-progress: true + +permissions: + contents: read + +jobs: + snowflake-s3compat-tests: + name: Snowflake S3Compat API tests + runs-on: ubuntu-22.04 + timeout-minutes: 30 + env: + WORK_DIR: /tmp/seaweedfs-snowflake-tests + steps: + - name: Check out code + uses: actions/checkout@v7 + with: + persist-credentials: false + + - name: Set up Go + uses: actions/setup-go@v7 + with: + go-version-file: 'go.mod' + id: go + + - name: Set up Java + uses: actions/setup-java@v6 + with: + java-version: '17' + distribution: 'temurin' + cache: 'maven' + + - name: Install SeaweedFS + run: | + cd weed + go install -buildvcs=false + weed version + + - name: Run Snowflake S3Compat API tests + timeout-minutes: 20 + run: | + # Starts weed server, creates the buckets/objects the suite needs, + # clones the upstream suite, and runs mvn -Dtest=S3CompatApiTest. + bash test/s3/snowflake/run.sh + + - name: Show logs on failure + if: failure() + run: | + echo "=== SeaweedFS Server Log ===" + tail -200 "$WORK_DIR/weed.log" || echo "No server log" + echo "" + echo "=== Surefire results ===" + cat "$WORK_DIR"/snowflake-s3compat-api-test-suite/s3compatapi/target/surefire-reports/*.txt 2>/dev/null || echo "No surefire reports" + + - name: Upload test results + if: always() + uses: actions/upload-artifact@v7 + with: + name: snowflake-s3compat-surefire-reports + path: /tmp/seaweedfs-snowflake-tests/snowflake-s3compat-api-test-suite/s3compatapi/target/surefire-reports/ + retention-days: 14 + + - name: Cleanup + if: always() + run: | + pkill -9 -f "weed server" || true + rm -rf "$WORK_DIR" || true diff --git a/test/s3/snowflake/README.md b/test/s3/snowflake/README.md new file mode 100644 index 000000000..eeda9acc5 --- /dev/null +++ b/test/s3/snowflake/README.md @@ -0,0 +1,33 @@ +# Snowflake S3Compat API test suite + +Integration tests that run the upstream +[Snowflake s3compat API test suite](https://github.com/snowflakedb/snowflake-s3compat-api-test-suite) +against SeaweedFS. The suite covers `getBucketLocation`, `getObject` (including +range reads), `getObjectMetadata`, `putObject` (including a 5 GB upload), +`listObjectsV2` (including paged listing of >1000 objects), `deleteObject`, +`deleteObjects`, `copyObject`, and `generatePresignedUrl`. + +## Running locally + +Requires `weed` (or `WEED_BIN`), the `aws` CLI, `mvn`, and JDK 11+ on `PATH`. + +```sh +(cd weed && go install -buildvcs=false) # build weed first +bash test/s3/snowflake/run.sh +``` + +`run.sh` starts a `weed server` with S3 enabled, calls `prepare.sh` to create +the fixtures, clones the suite into a scratch dir, and runs +`mvn -Dtest=S3CompatApiTest`. Set `WORK_DIR` to keep the server log and suite +clone around, `SKIP_SERVER_START=1` with `ENDPOINT_URL` to run against an +already-running server, and see the top of `run.sh` for the other overrides. + +## Notes + +- The server runs with `-s3.autoCreateBucket=false` so PUTs to a missing bucket + return `NoSuchBucket` like AWS; the suite asserts this. +- The suite forces virtual-hosted-style bucket addressing, which requires + wildcard DNS that does not exist for a local endpoint. `run.sh` switches it + to path-style access with a `sed` patch. +- `NOT_ACCESSIBLE_BUCKET` is a real bucket carrying a deny-all bucket policy, + which is how the suite's `AccessDenied` negative tests are satisfied. diff --git a/test/s3/snowflake/prepare.sh b/test/s3/snowflake/prepare.sh new file mode 100755 index 000000000..030857b7c --- /dev/null +++ b/test/s3/snowflake/prepare.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# +# Prepares the fixtures required by the Snowflake s3compat API test suite +# (https://github.com/snowflakedb/snowflake-s3compat-api-test-suite) against a +# running SeaweedFS S3 endpoint: +# +# BUCKET_NAME_1 versioning-enabled bucket the suite writes to +# NOT_ACCESSIBLE_BUCKET bucket with a deny-all bucket policy; the suite +# expects 403 AccessDenied for every operation on it +# PREFIX_FOR_PAGE_LISTING prefix under BUCKET_NAME_1 holding more than 1000 +# objects (the suite asserts paged listing works) +# +# Requires: aws CLI on PATH, AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY set. + +set -euo pipefail + +ENDPOINT_URL="${ENDPOINT_URL:-http://127.0.0.1:8333}" +BUCKET_NAME_1="${BUCKET_NAME_1:-sf-snowflake-test}" +NOT_ACCESSIBLE_BUCKET="${NOT_ACCESSIBLE_BUCKET:-sf-denied-bucket}" +PREFIX_FOR_PAGE_LISTING="${PREFIX_FOR_PAGE_LISTING:-test-suite/page-listing/}" +PAGE_LISTING_TOTAL_SIZE="${PAGE_LISTING_TOTAL_SIZE:-1100}" + +aws="aws --endpoint-url $ENDPOINT_URL" + +echo "Creating test bucket $BUCKET_NAME_1 with versioning enabled" +$aws s3api create-bucket --bucket "$BUCKET_NAME_1" +$aws s3api put-bucket-versioning --bucket "$BUCKET_NAME_1" --versioning-configuration Status=Enabled +$aws s3api get-bucket-versioning --bucket "$BUCKET_NAME_1" + +echo "Creating not-accessible bucket $NOT_ACCESSIBLE_BUCKET with a deny-all bucket policy" +$aws s3api create-bucket --bucket "$NOT_ACCESSIBLE_BUCKET" +POLICY_FILE="$(mktemp)" +cat > "$POLICY_FILE" <&1)"; then + echo "ERROR: expected AccessDenied on $NOT_ACCESSIBLE_BUCKET, got success" >&2 + exit 1 +elif ! echo "$OUT" | grep -q "AccessDenied"; then + echo "ERROR: expected AccessDenied on $NOT_ACCESSIBLE_BUCKET, got: $OUT" >&2 + exit 1 +fi +echo "Verified $NOT_ACCESSIBLE_BUCKET denies access" + +if [ "$PAGE_LISTING_TOTAL_SIZE" -le 1000 ]; then + echo "ERROR: PAGE_LISTING_TOTAL_SIZE must be > 1000, got $PAGE_LISTING_TOTAL_SIZE" >&2 + exit 1 +fi + +echo "Uploading $PAGE_LISTING_TOTAL_SIZE objects to s3://$BUCKET_NAME_1/$PREFIX_FOR_PAGE_LISTING" +WORKDIR="$(mktemp -d)" +for i in $(seq 1 "$PAGE_LISTING_TOTAL_SIZE"); do + echo "object-$i" > "$WORKDIR/file_$(printf %05d "$i").txt" +done +$aws s3 sync "$WORKDIR" "s3://$BUCKET_NAME_1/$PREFIX_FOR_PAGE_LISTING" --quiet +rm -rf "$WORKDIR" + +COUNT=$($aws s3 ls "s3://$BUCKET_NAME_1/$PREFIX_FOR_PAGE_LISTING" | wc -l | tr -d ' ') +if [ "$COUNT" != "$PAGE_LISTING_TOTAL_SIZE" ]; then + echo "ERROR: expected $PAGE_LISTING_TOTAL_SIZE objects under $PREFIX_FOR_PAGE_LISTING, found $COUNT" >&2 + exit 1 +fi +echo "Verified $COUNT objects under s3://$BUCKET_NAME_1/$PREFIX_FOR_PAGE_LISTING" +echo "Fixtures ready." diff --git a/test/s3/snowflake/run.sh b/test/s3/snowflake/run.sh new file mode 100755 index 000000000..667c4d0bd --- /dev/null +++ b/test/s3/snowflake/run.sh @@ -0,0 +1,140 @@ +#!/usr/bin/env bash +# +# Runs the Snowflake s3compat API test suite +# (https://github.com/snowflakedb/snowflake-s3compat-api-test-suite) against a +# locally-built SeaweedFS server. +# +# Required on PATH: weed (or WEED_BIN), aws, mvn, java, git. +# +# Env overrides: +# WEED_BIN path to the weed binary (default: weed) +# WORK_DIR scratch dir for data + suite clone (default: mktemp -d, +# removed on success, kept on failure) +# MASTER_PORT master http port (default: 9333) +# VOLUME_PORT volume http port (default: 8080) +# FILER_PORT filer http port (default: 8888) +# S3_PORT s3 endpoint port (default: 8333) +# METRICS_PORT metrics http port (default: 9324) +# SKIP_SERVER_START if set, do not start weed; prepare and run the suite +# against ENDPOINT_URL +# SUITE_REPO git url of the test suite (default: upstream) +# SUITE_REV suite commit to check out (default: pinned SHA) +# +# The suite env vars (BUCKET_NAME_1, PREFIX_FOR_PAGE_LISTING, +# PAGE_LISTING_TOTAL_SIZE, NOT_ACCESSIBLE_BUCKET) default to the same values +# prepare.sh uses. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +WEED_BIN="${WEED_BIN:-weed}" +MASTER_PORT="${MASTER_PORT:-9333}" +VOLUME_PORT="${VOLUME_PORT:-8080}" +FILER_PORT="${FILER_PORT:-8888}" +S3_PORT="${S3_PORT:-8333}" +METRICS_PORT="${METRICS_PORT:-9324}" +ENDPOINT_URL="${ENDPOINT_URL:-http://127.0.0.1:$S3_PORT}" +SUITE_REPO="${SUITE_REPO:-https://github.com/snowflakedb/snowflake-s3compat-api-test-suite.git}" +# Pinned upstream revision verified against SeaweedFS; bump deliberately. +SUITE_REV="${SUITE_REV:-8ae535b35fff0d8a72e21bba4e51281ac991cab9}" +WORK_DIR_CREATED="" +if [ -z "${WORK_DIR:-}" ]; then + WORK_DIR="$(mktemp -d)" + WORK_DIR_CREATED=1 +fi + +export BUCKET_NAME_1="${BUCKET_NAME_1:-sf-snowflake-test}" +export NOT_ACCESSIBLE_BUCKET="${NOT_ACCESSIBLE_BUCKET:-sf-denied-bucket}" +export PREFIX_FOR_PAGE_LISTING="${PREFIX_FOR_PAGE_LISTING:-test-suite/page-listing/}" +export PAGE_LISTING_TOTAL_SIZE="${PAGE_LISTING_TOTAL_SIZE:-1100}" +export ENDPOINT_URL + +# The suite reads credentials from these variables. +export S3COMPAT_ACCESS_KEY="${S3COMPAT_ACCESS_KEY:-snowflake_compat_access}" +export S3COMPAT_SECRET_KEY="${S3COMPAT_SECRET_KEY:-snowflake_compat_secret}" +export AWS_ACCESS_KEY_ID="$S3COMPAT_ACCESS_KEY" +export AWS_SECRET_ACCESS_KEY="$S3COMPAT_SECRET_KEY" + +WEED_PID="" +cleanup() { + status=$? + if [ -n "$WEED_PID" ]; then + kill "$WEED_PID" 2>/dev/null || true + sleep 2 + kill -9 "$WEED_PID" 2>/dev/null || true + fi + if [ -n "$WORK_DIR_CREATED" ]; then + if [ "$status" -eq 0 ]; then + rm -rf "$WORK_DIR" + else + echo "Work dir kept for debugging: $WORK_DIR" >&2 + fi + fi +} +trap cleanup EXIT + +wait_for_url() { + local url="$1" name="$2" + for i in $(seq 1 30); do + if curl -s "$url" > /dev/null 2>&1; then + echo "$name is ready" + return 0 + fi + echo "Waiting for $name... ($i/30)" + sleep 2 + done + echo "ERROR: $name did not become ready" >&2 + return 1 +} + +if [ -z "${SKIP_SERVER_START:-}" ]; then + WEED_DATA_DIR="$WORK_DIR/data" + mkdir -p "$WEED_DATA_DIR" + + echo "Starting SeaweedFS (data dir: $WEED_DATA_DIR)" + "$WEED_BIN" server -filer -filer.maxMB=64 -s3 -ip 127.0.0.1 -ip.bind 127.0.0.1 \ + -dir="$WEED_DATA_DIR" \ + -master.raftHashicorp -master.electionTimeout 1s -master.volumeSizeLimitMB=5000 \ + -volume.max=4 -volume.preStopSeconds=1 \ + -master.peers=none \ + -master.port="$MASTER_PORT" -volume.port="$VOLUME_PORT" -filer.port="$FILER_PORT" -s3.port="$S3_PORT" \ + -metricsPort="$METRICS_PORT" \ + -s3.allowDeleteBucketNotEmpty=true \ + -s3.autoCreateBucket=false \ + -s3.port.iceberg=0 -s3.port.lance=0 \ + -s3.config="$SCRIPT_DIR/s3.json" \ + > "$WORK_DIR/weed.log" 2>&1 & + WEED_PID=$! + + wait_for_url "http://127.0.0.1:$MASTER_PORT/cluster/status" "Master server" + wait_for_url "http://127.0.0.1:$VOLUME_PORT/status" "Volume server" + wait_for_url "http://127.0.0.1:$FILER_PORT/" "Filer" + wait_for_url "$ENDPOINT_URL/" "S3 API" + echo "All SeaweedFS components are ready" +fi + +"$SCRIPT_DIR/prepare.sh" + +SUITE_DIR="$WORK_DIR/snowflake-s3compat-api-test-suite" +if [ ! -d "$SUITE_DIR" ]; then + git init -q "$SUITE_DIR" + git -C "$SUITE_DIR" remote add origin "$SUITE_REPO" + git -C "$SUITE_DIR" fetch -q --depth 1 origin "$SUITE_REV" + git -C "$SUITE_DIR" checkout -q FETCH_HEAD +fi + +# The suite forces virtual-hosted style bucket addressing, which needs wildcard +# DNS (.) that does not exist for a local endpoint. Switch it +# to path-style access, which SeaweedFS supports. +STORAGE_CLIENT="$SUITE_DIR/s3compatapi/src/main/java/com/snowflake/s3compatapitestsuite/compatapi/S3CompatStorageClient.java" +sed -i.bak 's/setPathStyleAccess(false)/setPathStyleAccess(true)/' "$STORAGE_CLIENT" +rm -f "$STORAGE_CLIENT.bak" +grep -q 'setPathStyleAccess(true)' "$STORAGE_CLIENT" + +cd "$SUITE_DIR/s3compatapi" + +END_POINT="$ENDPOINT_URL" \ +REGION_1=us-east-1 \ +REGION_2=us-west-2 \ +mvn -B test -Dtest=S3CompatApiTest diff --git a/test/s3/snowflake/s3.json b/test/s3/snowflake/s3.json new file mode 100644 index 000000000..0f52e9ab4 --- /dev/null +++ b/test/s3/snowflake/s3.json @@ -0,0 +1,20 @@ +{ + "identities": [ + { + "name": "snowflake_compat_admin", + "credentials": [ + { + "accessKey": "snowflake_compat_access", + "secretKey": "snowflake_compat_secret" + } + ], + "actions": [ + "Admin", + "Read", + "List", + "Tagging", + "Write" + ] + } + ] +} diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index a087c22e0..2e970a936 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -1251,6 +1251,11 @@ func (s3a *S3ApiServer) DeleteBucketLifecycleHandler(w http.ResponseWriter, r *h func (s3a *S3ApiServer) GetBucketLocationHandler(w http.ResponseWriter, r *http.Request) { bucket, _ := s3_constants.GetBucketAndObject(r) + if err := s3bucket.VerifyS3BucketName(bucket); err != nil { + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidBucketName) + return + } + if err := s3a.checkBucket(r, bucket); err != s3err.ErrNone { s3err.WriteErrorResponse(w, r, err) return diff --git a/weed/s3api/s3api_bucket_handlers_misc_test.go b/weed/s3api/s3api_bucket_handlers_misc_test.go index 90ab5ec52..2e704d860 100644 --- a/weed/s3api/s3api_bucket_handlers_misc_test.go +++ b/weed/s3api/s3api_bucket_handlers_misc_test.go @@ -227,6 +227,24 @@ func TestGetBucketLogging(t *testing.T) { } } +func TestGetBucketLocationInvalidBucketName(t *testing.T) { + // AWS answers 400 InvalidBucketName for a malformed bucket name rather than + // the 404 NoSuchBucket an unknown-but-valid name gets. + s3a := &S3ApiServer{} + req := httptest.NewRequest(http.MethodGet, "/invalid%20bucket%20name?location=", nil) + req = mux.SetURLVars(req, map[string]string{"bucket": "invalid bucket name"}) + rec := httptest.NewRecorder() + + s3a.GetBucketLocationHandler(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusBadRequest, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "InvalidBucketName") { + t.Fatalf("body = %s, want InvalidBucketName", rec.Body.String()) + } +} + func TestHandleAutoCreateBucketDisabled(t *testing.T) { s3a := &S3ApiServer{option: &S3ApiServerOption{}} req := newBucketRequest(http.MethodPut, "test-bucket", "", "")