mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-20 14:17:07 +00:00
* 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.
172 lines
5.1 KiB
Go
172 lines
5.1 KiB
Go
package iceberg
|
|
|
|
import (
|
|
"encoding/json"
|
|
"fmt"
|
|
"net/http"
|
|
"os"
|
|
"strconv"
|
|
"strings"
|
|
|
|
"github.com/gorilla/mux"
|
|
"github.com/seaweedfs/seaweedfs/weed/glog"
|
|
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
|
"github.com/seaweedfs/seaweedfs/weed/s3api/s3tables"
|
|
)
|
|
|
|
// parseNamespace parses the namespace from path parameter.
|
|
// Iceberg uses unit separator (0x1F) for multi-level namespaces.
|
|
// Note: mux already decodes URL-encoded path parameters, so we only split by unit separator.
|
|
func parseNamespace(encoded string) []string {
|
|
if encoded == "" {
|
|
return nil
|
|
}
|
|
parts := strings.Split(encoded, "\x1F")
|
|
// Filter empty parts
|
|
result := make([]string, 0, len(parts))
|
|
for _, p := range parts {
|
|
if p != "" {
|
|
result = append(result, p)
|
|
}
|
|
}
|
|
return result
|
|
}
|
|
|
|
// encodeNamespace encodes namespace parts using the Iceberg REST protocol's
|
|
// unit separator (0x1F) convention. This is only appropriate for protocol-level
|
|
// encoding (e.g. URL path parameters), NOT for filesystem/S3 paths.
|
|
func encodeNamespace(parts []string) string {
|
|
return strings.Join(parts, "\x1F")
|
|
}
|
|
|
|
// flattenNamespacePath joins namespace parts with "." for use in S3 location
|
|
// and filer paths, matching the S3 Tables storage layer convention.
|
|
func flattenNamespacePath(parts []string) string {
|
|
return strings.Join(parts, ".")
|
|
}
|
|
|
|
func parseS3Location(location string) (bucketName, tablePath string, err error) {
|
|
if !strings.HasPrefix(location, "s3://") {
|
|
return "", "", fmt.Errorf("unsupported location: %s", location)
|
|
}
|
|
trimmed := strings.TrimPrefix(location, "s3://")
|
|
trimmed = strings.TrimSuffix(trimmed, "/")
|
|
if trimmed == "" {
|
|
return "", "", fmt.Errorf("invalid location: %s", location)
|
|
}
|
|
parts := strings.SplitN(trimmed, "/", 2)
|
|
bucketName = parts[0]
|
|
if bucketName == "" {
|
|
return "", "", fmt.Errorf("invalid location bucket: %s", location)
|
|
}
|
|
if len(parts) == 2 {
|
|
tablePath = parts[1]
|
|
}
|
|
return bucketName, tablePath, nil
|
|
}
|
|
|
|
func tableLocationFromMetadataLocation(metadataLocation string) string {
|
|
trimmed := strings.TrimSuffix(metadataLocation, "/")
|
|
if idx := strings.LastIndex(trimmed, "/metadata/"); idx != -1 {
|
|
return trimmed[:idx]
|
|
}
|
|
return trimmed
|
|
}
|
|
|
|
// writeJSON writes a JSON response.
|
|
func writeJSON(w http.ResponseWriter, status int, v interface{}) {
|
|
w.Header().Set("Content-Type", "application/json")
|
|
w.WriteHeader(status)
|
|
if v != nil {
|
|
data, err := json.Marshal(v)
|
|
if err != nil {
|
|
glog.Errorf("Iceberg: failed to encode response: %v", err)
|
|
return
|
|
}
|
|
w.Write(data)
|
|
}
|
|
}
|
|
|
|
// writeError writes an Iceberg error response.
|
|
func writeError(w http.ResponseWriter, status int, errType, message string) {
|
|
resp := ErrorResponse{
|
|
Error: ErrorModel{
|
|
Message: message,
|
|
Type: errType,
|
|
Code: status,
|
|
},
|
|
}
|
|
writeJSON(w, status, resp)
|
|
}
|
|
|
|
// 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
|
|
}
|
|
// Default bucket if no prefix - use "warehouse" for Iceberg
|
|
return "warehouse"
|
|
}
|
|
|
|
// buildTableBucketARN builds an ARN for a table bucket.
|
|
func buildTableBucketARN(bucketName string) string {
|
|
arn, _ := s3tables.BuildBucketARN(s3tables.DefaultRegion, s3_constants.AccountAdminId, bucketName)
|
|
return arn
|
|
}
|
|
|
|
const (
|
|
defaultListPageSize = 1000
|
|
maxListPageSize = 1000
|
|
)
|
|
|
|
func getPaginationQueryParam(r *http.Request, primary, fallback string) string {
|
|
if v := strings.TrimSpace(r.URL.Query().Get(primary)); v != "" {
|
|
return v
|
|
}
|
|
return strings.TrimSpace(r.URL.Query().Get(fallback))
|
|
}
|
|
|
|
func parsePagination(r *http.Request) (pageToken string, pageSize int, err error) {
|
|
pageToken = getPaginationQueryParam(r, "pageToken", "page-token")
|
|
pageSize = defaultListPageSize
|
|
|
|
pageSizeValue := getPaginationQueryParam(r, "pageSize", "page-size")
|
|
if pageSizeValue == "" {
|
|
return pageToken, pageSize, nil
|
|
}
|
|
|
|
parsedPageSize, parseErr := strconv.Atoi(pageSizeValue)
|
|
if parseErr != nil || parsedPageSize <= 0 {
|
|
return pageToken, 0, fmt.Errorf("invalid pageSize %q: must be a positive integer", pageSizeValue)
|
|
}
|
|
if parsedPageSize > maxListPageSize {
|
|
return pageToken, 0, fmt.Errorf("invalid pageSize %q: must be <= %d", pageSizeValue, maxListPageSize)
|
|
}
|
|
|
|
return pageToken, parsedPageSize, nil
|
|
}
|
|
|
|
func normalizeNamespaceProperties(properties map[string]string) map[string]string {
|
|
if properties == nil {
|
|
return map[string]string{}
|
|
}
|
|
return properties
|
|
}
|