mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-18 21:26:56 +00:00
fix(iceberg): route catalog clients to the right bucket and vend S3 endpoint (#9109)
* fix(iceberg): route catalog clients to the right bucket and vend S3 endpoint DuckDB ATTACH 's3://<bucket>/' AS cat (TYPE 'ICEBERG', ...) was failing with "schema does not exist" because GET /v1/config ignored the warehouse query parameter and returned no overrides.prefix, so subsequent requests fell through to the hard-coded "warehouse" default bucket instead of the one the client attached. LoadTable also returned an empty config, forcing clients to discover the S3 endpoint out-of-band and producing 403s on direct iceberg_scan calls. - handleConfig now echoes overrides.prefix = bucket and defaults.warehouse when ?warehouse=s3://<bucket>/ is supplied. - getBucketFromPrefix honors a warehouse query parameter as a fallback for clients that skip the /v1/config handshake. - LoadTable responses advertise s3.endpoint and s3.path-style-access so clients can reach data files without separate configuration. Refs #9103 * address review feedback on iceberg S3 endpoint vending - deriveS3AdvertisedEndpoint is now a method on S3Options; honors externalUrl / S3_EXTERNAL_URL, switches to https when -s3.key.file is set, uses the https port when configured, and brackets IPv6 literals via util.JoinHostPort. - handleCreateTable returns s.buildFileIOConfig() in both its staged and final LoadTableResult branches so create and load flows see the same FileIO hints. - Add unit test coverage for the endpoint derivation scenarios. * address CI and review feedback for #9109 - DuckDB integration test now runs under its own newOAuthTestEnv (with a valid IAM config) so the OAuth2 client_credentials flow DuckDB requires actually works; the shared env has no registered credentials, which was the cause of the CI failure. Helper createTableWithToken was added to create tables via Bearer auth. - Tighten TestIssue9103_LoadTableDoesNotVendS3FileIOCredentials to also assert s3.path-style-access = "true", so a partial regression where the endpoint is vended but path-style is dropped still fails. - deriveS3AdvertisedEndpoint now logs a startup hint when it infers the host from os.Hostname because the bind IP is a wildcard, pointing operators at -s3.externalUrl / S3_EXTERNAL_URL for reverse-proxy deployments where the inferred name is not externally reachable. - handleConfig has a comment explaining that any sub-path in the warehouse URL is dropped because catalog routing is bucket-scoped. * fix(iceberg): make advertised S3 endpoint strictly opt-in; add region The wildcard-bind fallback to os.Hostname() in deriveS3AdvertisedEndpoint was hijacking correctly-configured clients: on the CI runner it produced http://runnervmrc6n4:<port>, which Spark (running in Docker) could not resolve, so Spark iceberg tests began failing after the endpoint started being vended in LoadTable responses. Change the rule so advertising is opt-in and never guesses a host that might not be routable: - -s3.externalUrl / S3_EXTERNAL_URL wins (covers reverse-proxy). - Otherwise, only an explicit, non-wildcard -s3.bindIp is used. - Wildcard / empty bind returns "" so no FileIO endpoint is vended and existing clients keep using their own configuration. buildFileIOConfig additionally vends s3.region (defaulting to the same value baked into table bucket ARNs) whenever it vends an endpoint, so DuckDB's attach does not fail with "No region was provided via the vended credentials" when the operator has opted in. The DuckDB issue-9103 integration test runs under an env with a wildcard bind, so it explicitly sets AWS_REGION in the docker run to pick up the same default. The HTTP-level LoadTable-vending test was dropped because its expectation is now conditional and already covered by unit tests in iceberg_issue_9103_test.go.
This commit is contained in:
@@ -0,0 +1,283 @@
|
||||
// Reproduction tests for https://github.com/seaweedfs/seaweedfs/issues/9103
|
||||
//
|
||||
// The issue reports two distinct failure modes when using DuckDB against the
|
||||
// SeaweedFS Iceberg REST catalog:
|
||||
//
|
||||
// 1. `ATTACH 's3://test/' AS cat (TYPE 'ICEBERG', ...); SELECT * FROM cat.ovirt.disk;`
|
||||
// fails with "Table with name 'ovirt.disk' does not exist because schema
|
||||
// 'ovirt' does not exist." The namespace "ovirt" does exist in the bucket.
|
||||
//
|
||||
// 2. `SELECT * FROM iceberg_scan('s3://test/ovirt/disk');` fails with HTTP 403
|
||||
// AccessDenied when DuckDB tries to glob `s3://test/ovirt/disk/metadata/v*`
|
||||
// because the LoadTable response does not vend S3 file-io credentials back
|
||||
// to the client.
|
||||
//
|
||||
// These tests reproduce the underlying catalog-level misbehavior at the REST
|
||||
// protocol layer so they run quickly and deterministically. A DuckDB-based
|
||||
// end-to-end reproduction is also included (gated on Docker availability).
|
||||
package catalog
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/test/testutil"
|
||||
)
|
||||
|
||||
// TestIssue9103_ConfigDoesNotVendWarehousePrefix reproduces failure mode #2.
|
||||
//
|
||||
// Per the Iceberg REST spec, a client attaching with warehouse=s3://<bucket>/
|
||||
// calls GET /v1/config?warehouse=s3://<bucket>/ and expects the server to
|
||||
// return an `overrides.prefix` that identifies the catalog namespace for
|
||||
// subsequent requests. Without it the client falls back to unprefixed paths
|
||||
// like /v1/namespaces, which on SeaweedFS resolve to the hard-coded default
|
||||
// bucket ("warehouse") and therefore do not list the user's namespaces.
|
||||
//
|
||||
// Currently the handler ignores the warehouse query parameter and returns
|
||||
// empty defaults/overrides. That is exactly what makes DuckDB's ATTACH flow
|
||||
// report `schema "ovirt" does not exist`.
|
||||
func TestIssue9103_ConfigDoesNotVendWarehousePrefix(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("Skipping integration test in short mode")
|
||||
}
|
||||
|
||||
env := sharedEnv
|
||||
bucketName := "warehouse-9103cfg-" + randomSuffix()
|
||||
createTableBucket(t, env, bucketName)
|
||||
|
||||
warehouse := fmt.Sprintf("s3://%s/", bucketName)
|
||||
u := fmt.Sprintf("%s/v1/config?warehouse=%s", env.IcebergURL(), url.QueryEscape(warehouse))
|
||||
|
||||
status, body, err := doIcebergJSONRequest(env, http.MethodGet, fmt.Sprintf("/v1/config?warehouse=%s", url.QueryEscape(warehouse)), nil)
|
||||
if err != nil {
|
||||
t.Fatalf("GET %s failed: %v", u, err)
|
||||
}
|
||||
if status != http.StatusOK {
|
||||
t.Fatalf("GET %s status = %d, want 200", u, status)
|
||||
}
|
||||
|
||||
overrides, _ := body["overrides"].(map[string]any)
|
||||
gotPrefix, _ := overrides["prefix"].(string)
|
||||
if gotPrefix != bucketName {
|
||||
t.Fatalf("GET /v1/config?warehouse=%s: overrides.prefix = %q, want %q (catalog must echo the warehouse's table bucket so clients like DuckDB know which /v1/{prefix}/namespaces to use)",
|
||||
warehouse, gotPrefix, bucketName)
|
||||
}
|
||||
}
|
||||
|
||||
// TestIssue9103_BareNamespacesListMissesNamespaceInAttachedBucket
|
||||
// demonstrates the downstream effect of the /v1/config bug.
|
||||
//
|
||||
// The client created the namespace "ovirt" inside bucket "test" (via
|
||||
// /v1/{bucket}/namespaces), but when it later issues GET /v1/namespaces (no
|
||||
// prefix, because /v1/config didn't give it one), that request resolves to
|
||||
// the default bucket and returns no namespaces. DuckDB surfaces this as
|
||||
// `schema "ovirt" does not exist`.
|
||||
func TestIssue9103_BareNamespacesListMissesNamespaceInAttachedBucket(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("Skipping integration test in short mode")
|
||||
}
|
||||
|
||||
env := sharedEnv
|
||||
bucketName := "warehouse-9103ns-" + randomSuffix()
|
||||
createTableBucket(t, env, bucketName)
|
||||
|
||||
namespace := "ovirt"
|
||||
status, _, err := doIcebergJSONRequest(env, http.MethodPost,
|
||||
icebergPath(bucketName, "/v1/namespaces"),
|
||||
map[string]any{"namespace": []string{namespace}})
|
||||
if err != nil {
|
||||
t.Fatalf("create namespace: %v", err)
|
||||
}
|
||||
if status != http.StatusOK && status != http.StatusConflict {
|
||||
t.Fatalf("create namespace status = %d, want 200 or 409", status)
|
||||
}
|
||||
|
||||
// Sanity: prefixed listing finds the namespace.
|
||||
status, prefixedBody, err := doIcebergJSONRequest(env, http.MethodGet,
|
||||
icebergPath(bucketName, "/v1/namespaces"), nil)
|
||||
if err != nil {
|
||||
t.Fatalf("list namespaces with prefix: %v", err)
|
||||
}
|
||||
if status != http.StatusOK {
|
||||
t.Fatalf("prefixed list status = %d, want 200", status)
|
||||
}
|
||||
if !containsNamespace(prefixedBody, namespace) {
|
||||
t.Fatalf("prefixed list missing namespace %q: %v", namespace, prefixedBody)
|
||||
}
|
||||
|
||||
// Simulate what a DuckDB-style client does when /v1/config did not vend a
|
||||
// prefix: it falls back to the unprefixed listing. It should still be
|
||||
// able to discover the namespace (e.g. via a warehouse query parameter),
|
||||
// but today it cannot, which is the user-visible bug.
|
||||
bareWarehouse := "s3://" + bucketName + "/"
|
||||
status, bareBody, err := doIcebergJSONRequest(env, http.MethodGet,
|
||||
fmt.Sprintf("/v1/namespaces?warehouse=%s", url.QueryEscape(bareWarehouse)), nil)
|
||||
if err != nil {
|
||||
t.Fatalf("bare list namespaces: %v", err)
|
||||
}
|
||||
switch {
|
||||
case status != http.StatusOK:
|
||||
t.Fatalf("GET /v1/namespaces?warehouse=%s status = %d, want 200 (server ignores the warehouse query parameter and falls back to the default %q bucket, causing DuckDB to see no schemas)",
|
||||
bareWarehouse, status, "warehouse")
|
||||
case !containsNamespace(bareBody, namespace):
|
||||
t.Fatalf("GET /v1/namespaces?warehouse=%s did not return %q: %v (server must honor the warehouse query parameter here or vend overrides.prefix from /v1/config so the client can route to the right bucket)",
|
||||
bareWarehouse, namespace, bareBody)
|
||||
}
|
||||
}
|
||||
|
||||
// TestIssue9103_DuckDBAttachCannotResolveNamespace is the end-to-end
|
||||
// reproduction of the issue using DuckDB in Docker, mirroring the exact
|
||||
// sequence of commands from the bug report. It runs under its own weed
|
||||
// mini with IAM configured so the OAuth2 client_credentials flow that
|
||||
// DuckDB's iceberg extension requires actually works (the shared env has
|
||||
// no credentials registered). Gated on Docker availability.
|
||||
func TestIssue9103_DuckDBAttachCannotResolveNamespace(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("Skipping integration test in short mode")
|
||||
}
|
||||
if !testutil.HasDocker() {
|
||||
t.Skip("Docker not available, skipping DuckDB integration reproduction")
|
||||
}
|
||||
|
||||
env := newOAuthTestEnv(t)
|
||||
defer env.cleanup(t)
|
||||
env.start(t)
|
||||
|
||||
bucketName := "test9103-" + randomSuffix()
|
||||
createTableBucketViaShell(t, env, bucketName)
|
||||
|
||||
namespace := "ovirt"
|
||||
tableName := "disk"
|
||||
|
||||
token := requestOAuthToken(t, env, env.accessKey, env.secretKey)
|
||||
createNamespaceWithToken(t, env, token, bucketName, namespace)
|
||||
createTableWithToken(t, env, token, bucketName, namespace, tableName)
|
||||
|
||||
sql := fmt.Sprintf(`
|
||||
INSTALL iceberg;
|
||||
LOAD iceberg;
|
||||
|
||||
CREATE SECRET test_berg (
|
||||
TYPE ICEBERG,
|
||||
ENDPOINT 'http://host.docker.internal:%d',
|
||||
SCOPE 's3://%s/',
|
||||
CLIENT_ID '%s',
|
||||
CLIENT_SECRET '%s'
|
||||
);
|
||||
|
||||
CREATE SECRET s3_berg (
|
||||
TYPE S3,
|
||||
KEY_ID '%s',
|
||||
SECRET '%s',
|
||||
ENDPOINT 'host.docker.internal:%d',
|
||||
URL_STYLE 'path',
|
||||
USE_SSL false,
|
||||
SCOPE 's3://%s/'
|
||||
);
|
||||
|
||||
ATTACH 's3://%s/' AS test_catalog (TYPE 'ICEBERG', secret test_berg);
|
||||
SELECT * FROM test_catalog.%s.%s LIMIT 0;
|
||||
`, env.icebergPort, bucketName,
|
||||
env.accessKey, env.secretKey,
|
||||
env.accessKey, env.secretKey, env.s3Port, bucketName,
|
||||
bucketName, namespace, tableName)
|
||||
|
||||
sqlFile := filepath.Join(env.dataDir, "issue_9103.sql")
|
||||
if err := os.WriteFile(sqlFile, []byte(sql), 0644); err != nil {
|
||||
t.Fatalf("write SQL: %v", err)
|
||||
}
|
||||
|
||||
cmd := exec.Command("docker", "run", "--rm",
|
||||
"-v", fmt.Sprintf("%s:/test", env.dataDir),
|
||||
"--add-host", "host.docker.internal:host-gateway",
|
||||
"-e", "AWS_REGION=us-east-1",
|
||||
"--entrypoint", "duckdb",
|
||||
"duckdb/duckdb:latest",
|
||||
"-init", "/test/issue_9103.sql",
|
||||
"-c", "SELECT 1",
|
||||
)
|
||||
|
||||
out, runErr := cmd.CombinedOutput()
|
||||
outStr := string(out)
|
||||
t.Logf("DuckDB output:\n%s", outStr)
|
||||
|
||||
if strings.Contains(outStr, "iceberg extension is not available") ||
|
||||
strings.Contains(outStr, "Failed to load") ||
|
||||
strings.Contains(outStr, "could not fetch extension") ||
|
||||
strings.Contains(outStr, "failed to download") {
|
||||
t.Skip("Iceberg extension not available in DuckDB Docker image")
|
||||
}
|
||||
|
||||
if runErr != nil {
|
||||
if strings.Contains(outStr, "does not exist because schema") &&
|
||||
strings.Contains(outStr, namespace) {
|
||||
t.Fatalf("reproduced issue #9103: DuckDB cannot see namespace %q after ATTACH 's3://%s/'; see output above",
|
||||
namespace, bucketName)
|
||||
}
|
||||
t.Fatalf("DuckDB run failed for an unexpected reason: %v", runErr)
|
||||
}
|
||||
}
|
||||
|
||||
// createTableWithToken creates an Iceberg table using an OAuth bearer token.
|
||||
func createTableWithToken(t *testing.T, env *oauthTestEnv, token, bucketName, namespace, tableName string) {
|
||||
t.Helper()
|
||||
|
||||
payload := map[string]any{
|
||||
"name": tableName,
|
||||
"schema": map[string]any{
|
||||
"type": "struct",
|
||||
"schema-id": 0,
|
||||
"fields": []map[string]any{
|
||||
{"id": 1, "name": "id", "required": true, "type": "long"},
|
||||
},
|
||||
},
|
||||
}
|
||||
body, err := json.Marshal(payload)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal create-table payload: %v", err)
|
||||
}
|
||||
|
||||
url := fmt.Sprintf("%s/v1/%s/namespaces/%s/tables", env.icebergURL(), bucketName, namespace)
|
||||
req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(body))
|
||||
if err != nil {
|
||||
t.Fatalf("create request: %v", err)
|
||||
}
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
t.Fatalf("create table: %v", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
respBody, _ := io.ReadAll(resp.Body)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("create table failed: status=%d body=%s", resp.StatusCode, respBody)
|
||||
}
|
||||
t.Logf("Created table %s.%s in bucket %s", namespace, tableName, bucketName)
|
||||
}
|
||||
|
||||
// containsNamespace checks whether an Iceberg REST ListNamespaces response
|
||||
// contains the given single-level namespace.
|
||||
func containsNamespace(body map[string]any, name string) bool {
|
||||
arr, _ := body["namespaces"].([]any)
|
||||
for _, item := range arr {
|
||||
parts, _ := item.([]any)
|
||||
if len(parts) == 1 {
|
||||
if s, ok := parts[0].(string); ok && s == name {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
@@ -518,6 +518,7 @@ func (s3opt *S3Options) startIcebergServer(s3ApiServer *s3api.S3ApiServer) {
|
||||
// Create Iceberg server using the S3ApiServer as filer client
|
||||
icebergServer := iceberg.NewServer(s3ApiServer, s3ApiServer)
|
||||
icebergServer.SetCredentialValidator(s3ApiServer)
|
||||
icebergServer.SetS3Endpoint(s3opt.deriveS3AdvertisedEndpoint())
|
||||
icebergServer.RegisterRoutes(icebergRouter)
|
||||
|
||||
listenAddress := fmt.Sprintf("%s:%d", *s3opt.bindIp, *s3opt.portIceberg)
|
||||
@@ -548,3 +549,43 @@ func (s3opt *S3Options) startIcebergServer(s3ApiServer *s3api.S3ApiServer) {
|
||||
glog.Fatalf("Iceberg REST Catalog Server Fail to serve: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// deriveS3AdvertisedEndpoint builds the S3 endpoint URL to advertise to
|
||||
// Iceberg catalog clients as part of LoadTable FileIO config. To avoid
|
||||
// hijacking correctly-configured clients (Spark/Trino/PyIceberg all bring
|
||||
// their own s3.endpoint), advertising is strictly opt-in and returns ""
|
||||
// whenever no reliable value is available:
|
||||
// - -s3.externalUrl / S3_EXTERNAL_URL wins and supports reverse-proxy
|
||||
// deployments.
|
||||
// - Otherwise the bind IP is used only when it is explicit and not a
|
||||
// wildcard (0.0.0.0 / ::), with the scheme picked from TLS config and
|
||||
// IPv6 literals bracketed via util.JoinHostPort.
|
||||
//
|
||||
// See issue #9103.
|
||||
func (s3opt *S3Options) deriveS3AdvertisedEndpoint() string {
|
||||
if ext := strings.TrimRight(s3opt.resolveExternalUrl(), "/"); ext != "" {
|
||||
return ext
|
||||
}
|
||||
|
||||
host := ""
|
||||
if s3opt.bindIp != nil {
|
||||
host = *s3opt.bindIp
|
||||
}
|
||||
switch host {
|
||||
case "", "0.0.0.0", "::", "[::]":
|
||||
return ""
|
||||
}
|
||||
|
||||
scheme := "http"
|
||||
port := 0
|
||||
if s3opt.port != nil {
|
||||
port = *s3opt.port
|
||||
}
|
||||
if s3opt.tlsPrivateKey != nil && *s3opt.tlsPrivateKey != "" {
|
||||
scheme = "https"
|
||||
if s3opt.portHttps != nil && *s3opt.portHttps > 0 {
|
||||
port = *s3opt.portHttps
|
||||
}
|
||||
}
|
||||
return fmt.Sprintf("%s://%s", scheme, util.JoinHostPort(host, port))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
package command
|
||||
|
||||
import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
func strp(s string) *string { return &s }
|
||||
func intp(i int) *int { return &i }
|
||||
|
||||
func TestDeriveS3AdvertisedEndpoint(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
opt S3Options
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "wildcard bind IP with no externalUrl does not advertise an endpoint",
|
||||
opt: S3Options{
|
||||
bindIp: strp("0.0.0.0"),
|
||||
port: intp(8333),
|
||||
portHttps: intp(0),
|
||||
tlsPrivateKey: strp(""),
|
||||
externalUrl: strp(""),
|
||||
},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "empty bind IP with no externalUrl does not advertise an endpoint",
|
||||
opt: S3Options{
|
||||
bindIp: strp(""),
|
||||
port: intp(8333),
|
||||
portHttps: intp(0),
|
||||
tlsPrivateKey: strp(""),
|
||||
externalUrl: strp(""),
|
||||
},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "explicit bind IP is kept",
|
||||
opt: S3Options{
|
||||
bindIp: strp("10.0.0.5"),
|
||||
port: intp(8333),
|
||||
portHttps: intp(0),
|
||||
tlsPrivateKey: strp(""),
|
||||
externalUrl: strp(""),
|
||||
},
|
||||
want: "http://10.0.0.5:8333",
|
||||
},
|
||||
{
|
||||
name: "IPv6 literals are bracketed",
|
||||
opt: S3Options{
|
||||
bindIp: strp("fe80::1"),
|
||||
port: intp(8333),
|
||||
portHttps: intp(0),
|
||||
tlsPrivateKey: strp(""),
|
||||
externalUrl: strp(""),
|
||||
},
|
||||
want: "http://[fe80::1]:8333",
|
||||
},
|
||||
{
|
||||
name: "TLS key + https port switches scheme and port",
|
||||
opt: S3Options{
|
||||
bindIp: strp("10.0.0.5"),
|
||||
port: intp(8333),
|
||||
portHttps: intp(8443),
|
||||
tlsPrivateKey: strp("/etc/seaweed/tls.key"),
|
||||
externalUrl: strp(""),
|
||||
},
|
||||
want: "https://10.0.0.5:8443",
|
||||
},
|
||||
{
|
||||
name: "TLS-only (no separate https port) still uses https on the HTTP port",
|
||||
opt: S3Options{
|
||||
bindIp: strp("10.0.0.5"),
|
||||
port: intp(8333),
|
||||
portHttps: intp(0),
|
||||
tlsPrivateKey: strp("/etc/seaweed/tls.key"),
|
||||
externalUrl: strp(""),
|
||||
},
|
||||
want: "https://10.0.0.5:8333",
|
||||
},
|
||||
{
|
||||
name: "externalUrl wins and trailing slash is stripped",
|
||||
opt: S3Options{
|
||||
bindIp: strp("10.0.0.5"),
|
||||
port: intp(8333),
|
||||
portHttps: intp(0),
|
||||
tlsPrivateKey: strp(""),
|
||||
externalUrl: strp("https://s3.example.com/"),
|
||||
},
|
||||
want: "https://s3.example.com",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := tc.opt.deriveS3AdvertisedEndpoint()
|
||||
if got != tc.want {
|
||||
t.Fatalf("deriveS3AdvertisedEndpoint() = %q, want %q", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -14,12 +14,29 @@ import (
|
||||
)
|
||||
|
||||
// handleConfig returns catalog configuration.
|
||||
//
|
||||
// When a client passes ?warehouse=s3://<bucket>/, the Iceberg REST spec
|
||||
// expects the server to echo a catalog identifier back as overrides.prefix
|
||||
// so subsequent calls use /v1/{prefix}/... and land on the right table
|
||||
// bucket. Without this, clients like DuckDB's ATTACH flow fall back to an
|
||||
// unprefixed path that resolves to the wrong bucket and report phantom
|
||||
// "schema does not exist" errors. See issue #9103.
|
||||
func (s *Server) handleConfig(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
config := CatalogConfig{
|
||||
Defaults: map[string]string{},
|
||||
Overrides: map[string]string{},
|
||||
}
|
||||
if warehouse := strings.TrimSpace(r.URL.Query().Get("warehouse")); warehouse != "" {
|
||||
// Only the bucket portion of the warehouse URL is meaningful today —
|
||||
// SeaweedFS table-bucket routing is bucket-scoped, so any sub-path
|
||||
// (e.g. s3://bucket/prefix/) is ignored here and clients that try to
|
||||
// scope a catalog under a sub-prefix will still land on the bucket.
|
||||
if bucket, _, err := parseS3Location(warehouse); err == nil && bucket != "" {
|
||||
config.Overrides["prefix"] = bucket
|
||||
config.Defaults["warehouse"] = warehouse
|
||||
}
|
||||
}
|
||||
if err := json.NewEncoder(w).Encode(config); err != nil {
|
||||
glog.Warningf("handleConfig: Failed to encode config: %v", err)
|
||||
}
|
||||
|
||||
@@ -190,7 +190,7 @@ func (s *Server) handleCreateTable(w http.ResponseWriter, r *http.Request) {
|
||||
if existsErr == nil {
|
||||
// Table already registered. Return the existing definition so CTAS/IF NOT
|
||||
// EXISTS flows see a stable response instead of a 409.
|
||||
result := buildLoadTableResult(existsResp, bucketName, namespace, tableName)
|
||||
result := s.buildLoadTableResult(existsResp, bucketName, namespace, tableName)
|
||||
writeJSON(w, http.StatusOK, result)
|
||||
return
|
||||
}
|
||||
@@ -214,7 +214,7 @@ func (s *Server) handleCreateTable(w http.ResponseWriter, r *http.Request) {
|
||||
result := LoadTableResult{
|
||||
MetadataLocation: metadataLocation,
|
||||
Metadata: metadata,
|
||||
Config: make(iceberg.Properties),
|
||||
Config: s.buildFileIOConfig(),
|
||||
}
|
||||
writeJSON(w, http.StatusOK, result)
|
||||
return
|
||||
@@ -262,7 +262,7 @@ func (s *Server) handleCreateTable(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusConflict, "AlreadyExistsException", err.Error())
|
||||
return
|
||||
}
|
||||
result := buildLoadTableResult(getResp, bucketName, namespace, tableName)
|
||||
result := s.buildLoadTableResult(getResp, bucketName, namespace, tableName)
|
||||
writeJSON(w, http.StatusOK, result)
|
||||
return
|
||||
}
|
||||
@@ -281,7 +281,7 @@ func (s *Server) handleCreateTable(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusConflict, "AlreadyExistsException", err.Error())
|
||||
return
|
||||
}
|
||||
result := buildLoadTableResult(getResp, bucketName, namespace, tableName)
|
||||
result := s.buildLoadTableResult(getResp, bucketName, namespace, tableName)
|
||||
writeJSON(w, http.StatusOK, result)
|
||||
return
|
||||
}
|
||||
@@ -302,7 +302,7 @@ func (s *Server) handleCreateTable(w http.ResponseWriter, r *http.Request) {
|
||||
result := LoadTableResult{
|
||||
MetadataLocation: finalLocation,
|
||||
Metadata: metadata,
|
||||
Config: make(iceberg.Properties),
|
||||
Config: s.buildFileIOConfig(),
|
||||
}
|
||||
writeJSON(w, http.StatusOK, result)
|
||||
}
|
||||
@@ -347,11 +347,11 @@ func (s *Server) handleLoadTable(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
result := buildLoadTableResult(getResp, bucketName, namespace, tableName)
|
||||
result := s.buildLoadTableResult(getResp, bucketName, namespace, tableName)
|
||||
writeJSON(w, http.StatusOK, result)
|
||||
}
|
||||
|
||||
func buildLoadTableResult(getResp s3tables.GetTableResponse, bucketName string, namespace []string, tableName string) LoadTableResult {
|
||||
func (s *Server) buildLoadTableResult(getResp s3tables.GetTableResponse, bucketName string, namespace []string, tableName string) LoadTableResult {
|
||||
location := tableLocationFromMetadataLocation(getResp.MetadataLocation)
|
||||
if location == "" {
|
||||
location = fmt.Sprintf("s3://%s/%s", bucketName, path.Join(flattenNamespacePath(namespace), tableName))
|
||||
@@ -385,10 +385,25 @@ func buildLoadTableResult(getResp s3tables.GetTableResponse, bucketName string,
|
||||
return LoadTableResult{
|
||||
MetadataLocation: getResp.MetadataLocation,
|
||||
Metadata: metadata,
|
||||
Config: make(iceberg.Properties),
|
||||
Config: s.buildFileIOConfig(),
|
||||
}
|
||||
}
|
||||
|
||||
// buildFileIOConfig returns the FileIO properties to advertise to catalog
|
||||
// clients so they can read the table's data files directly from S3 without
|
||||
// separately discovering the endpoint. The region defaults to the same
|
||||
// value baked into table bucket ARNs so clients like DuckDB that require
|
||||
// a region on attach don't need to be told it out-of-band. See issue #9103.
|
||||
func (s *Server) buildFileIOConfig() iceberg.Properties {
|
||||
config := make(iceberg.Properties)
|
||||
if s.s3Endpoint != "" {
|
||||
config["s3.endpoint"] = s.s3Endpoint
|
||||
config["s3.path-style-access"] = "true"
|
||||
config["s3.region"] = s3tables.DefaultRegion
|
||||
}
|
||||
return config
|
||||
}
|
||||
|
||||
// handleTableExists checks if a table exists.
|
||||
func (s *Server) handleTableExists(w http.ResponseWriter, r *http.Request) {
|
||||
vars := mux.Vars(r)
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
package iceberg
|
||||
|
||||
import (
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestGetBucketFromPrefix_WarehouseQueryFallback(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
url string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "warehouse query routes to its bucket when no prefix in path",
|
||||
url: "/v1/namespaces?warehouse=s3%3A%2F%2Fmyblkt%2F",
|
||||
want: "myblkt",
|
||||
},
|
||||
{
|
||||
name: "warehouse query with sub-path still picks the bucket",
|
||||
url: "/v1/namespaces?warehouse=s3%3A%2F%2Fanother%2Fextra",
|
||||
want: "another",
|
||||
},
|
||||
{
|
||||
name: "malformed warehouse value falls through to default",
|
||||
url: "/v1/namespaces?warehouse=not-a-url",
|
||||
want: "warehouse",
|
||||
},
|
||||
{
|
||||
name: "no warehouse and no prefix returns default",
|
||||
url: "/v1/namespaces",
|
||||
want: "warehouse",
|
||||
},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
r := httptest.NewRequest("GET", tc.url, nil)
|
||||
got := getBucketFromPrefix(r)
|
||||
if got != tc.want {
|
||||
t.Fatalf("getBucketFromPrefix(%q) = %q, want %q", tc.url, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildFileIOConfig(t *testing.T) {
|
||||
t.Run("no endpoint configured yields empty config", func(t *testing.T) {
|
||||
s := &Server{}
|
||||
got := s.buildFileIOConfig()
|
||||
if len(got) != 0 {
|
||||
t.Fatalf("buildFileIOConfig() = %v, want empty", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("endpoint is advertised with path-style-access and region", func(t *testing.T) {
|
||||
s := &Server{s3Endpoint: "http://seaweed.example:8333"}
|
||||
got := s.buildFileIOConfig()
|
||||
if got["s3.endpoint"] != "http://seaweed.example:8333" {
|
||||
t.Fatalf("s3.endpoint = %q, want %q", got["s3.endpoint"], "http://seaweed.example:8333")
|
||||
}
|
||||
if got["s3.path-style-access"] != "true" {
|
||||
t.Fatalf("s3.path-style-access = %q, want %q", got["s3.path-style-access"], "true")
|
||||
}
|
||||
if got["s3.region"] == "" {
|
||||
t.Fatalf("s3.region was empty, want a non-empty default so clients like DuckDB do not require AWS_REGION")
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -40,6 +40,7 @@ type Server struct {
|
||||
prefix string // optional prefix for routes
|
||||
authenticator S3Authenticator
|
||||
credentialValidator CredentialValidator
|
||||
s3Endpoint string // http(s):// URL advertised in LoadTable FileIO config
|
||||
}
|
||||
|
||||
// NewServer creates a new Iceberg REST Catalog server.
|
||||
@@ -58,6 +59,13 @@ func (s *Server) SetCredentialValidator(cv CredentialValidator) {
|
||||
s.credentialValidator = cv
|
||||
}
|
||||
|
||||
// SetS3Endpoint configures the S3 endpoint URL to vend to clients as part of
|
||||
// the LoadTable FileIO config, so they can read table data files directly
|
||||
// without separately discovering the S3 API address. See issue #9103.
|
||||
func (s *Server) SetS3Endpoint(endpoint string) {
|
||||
s.s3Endpoint = endpoint
|
||||
}
|
||||
|
||||
// RegisterRoutes registers Iceberg REST API routes on the provided router.
|
||||
func (s *Server) RegisterRoutes(router *mux.Router) {
|
||||
// Add middleware to log all requests/responses
|
||||
|
||||
@@ -101,11 +101,23 @@ func writeError(w http.ResponseWriter, status int, errType, message string) {
|
||||
|
||||
// getBucketFromPrefix extracts table bucket name from prefix parameter.
|
||||
// For now, we use the prefix as the table bucket name.
|
||||
//
|
||||
// The Iceberg REST spec lets clients identify a catalog either by embedding
|
||||
// its prefix in the URL (/v1/{prefix}/...) or by passing ?warehouse=s3://
|
||||
// <bucket>/ as a query parameter. Clients that skip the /v1/config handshake
|
||||
// (or ignore its overrides) still routinely send the warehouse parameter on
|
||||
// every request, so honor it as a fallback before the env-var default.
|
||||
// See issue #9103.
|
||||
func getBucketFromPrefix(r *http.Request) string {
|
||||
vars := mux.Vars(r)
|
||||
if prefix := vars["prefix"]; prefix != "" {
|
||||
return prefix
|
||||
}
|
||||
if warehouse := strings.TrimSpace(r.URL.Query().Get("warehouse")); warehouse != "" {
|
||||
if bucket, _, err := parseS3Location(warehouse); err == nil && bucket != "" {
|
||||
return bucket
|
||||
}
|
||||
}
|
||||
if bucket := os.Getenv("S3TABLES_DEFAULT_BUCKET"); bucket != "" {
|
||||
return bucket
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user