From 5ae3be44d1cca408aa9565b6f6f0a56c2358f55e Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 9 Feb 2026 22:20:45 -0800 Subject: [PATCH] iceberg: persist namespace properties for create/get (#8276) * iceberg: persist namespace properties via s3tables metadata * iceberg: simplify namespace properties normalization * s3tables: broaden namespace properties round-trip test * adjust logs * adjust logs --- weed/s3api/iceberg/iceberg.go | 28 ++++----- .../iceberg_namespace_properties_test.go | 29 +++++++++ weed/s3api/s3tables/handler_namespace.go | 3 + weed/s3api/s3tables/types.go | 24 ++++---- weed/s3api/s3tables/utils.go | 7 ++- weed/s3api/s3tables/utils_namespace_test.go | 60 +++++++++++++++++++ 6 files changed, 124 insertions(+), 27 deletions(-) create mode 100644 weed/s3api/iceberg/iceberg_namespace_properties_test.go diff --git a/weed/s3api/iceberg/iceberg.go b/weed/s3api/iceberg/iceberg.go index 7fd2936d3..65b0e39f7 100644 --- a/weed/s3api/iceberg/iceberg.go +++ b/weed/s3api/iceberg/iceberg.go @@ -401,6 +401,13 @@ func parsePagination(r *http.Request) (pageToken string, pageSize int, err error return pageToken, parsedPageSize, nil } +func normalizeNamespaceProperties(properties map[string]string) map[string]string { + if properties == nil { + return map[string]string{} + } + return properties +} + // handleConfig returns catalog configuration. func (s *Server) handleConfig(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") @@ -482,36 +489,29 @@ func (s *Server) handleCreateNamespace(w http.ResponseWriter, r *http.Request) { createReq := &s3tables.CreateNamespaceRequest{ TableBucketARN: bucketARN, Namespace: req.Namespace, + Properties: normalizeNamespaceProperties(req.Properties), } var createResp s3tables.CreateNamespaceResponse err := s.filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { mgrClient := s3tables.NewManagerClient(client) - glog.Errorf("Iceberg: handleCreateNamespace calling Execute with identityName=%s", identityName) + glog.V(2).Infof("Iceberg: handleCreateNamespace calling Execute with identityName=%s", identityName) return s.tablesManager.Execute(r.Context(), mgrClient, "CreateNamespace", createReq, &createResp, identityName) }) if err != nil { - glog.Errorf("Iceberg: handleCreateNamespace error: %v", err) - if strings.Contains(err.Error(), "already exists") { writeError(w, http.StatusConflict, "AlreadyExistsException", err.Error()) return } - glog.Infof("Iceberg: CreateNamespace error: %v", err) + glog.Errorf("Iceberg: CreateNamespace error: %v", err) writeError(w, http.StatusInternalServerError, "InternalServerError", err.Error()) return } - // Standardize property initialization for consistency with GetNamespace - props := req.Properties - if props == nil { - props = make(map[string]string) - } - result := CreateNamespaceResponse{ - Namespace: req.Namespace, - Properties: props, + Namespace: Namespace(createResp.Namespace), + Properties: normalizeNamespaceProperties(createResp.Properties), } writeJSON(w, http.StatusOK, result) } @@ -554,8 +554,8 @@ func (s *Server) handleGetNamespace(w http.ResponseWriter, r *http.Request) { } result := GetNamespaceResponse{ - Namespace: namespace, - Properties: make(map[string]string), + Namespace: Namespace(getResp.Namespace), + Properties: normalizeNamespaceProperties(getResp.Properties), } writeJSON(w, http.StatusOK, result) } diff --git a/weed/s3api/iceberg/iceberg_namespace_properties_test.go b/weed/s3api/iceberg/iceberg_namespace_properties_test.go new file mode 100644 index 000000000..fa80dc342 --- /dev/null +++ b/weed/s3api/iceberg/iceberg_namespace_properties_test.go @@ -0,0 +1,29 @@ +package iceberg + +import "testing" + +func TestNormalizeNamespacePropertiesNil(t *testing.T) { + properties := normalizeNamespaceProperties(nil) + if properties == nil { + t.Fatalf("normalizeNamespaceProperties(nil) returned nil map") + } + if len(properties) != 0 { + t.Fatalf("normalizeNamespaceProperties(nil) length = %d, want 0", len(properties)) + } +} + +func TestNormalizeNamespacePropertiesReturnsInputWhenSet(t *testing.T) { + input := map[string]string{ + "owner": "analytics", + } + + properties := normalizeNamespaceProperties(input) + if properties["owner"] != "analytics" { + t.Fatalf("normalized properties value = %q, want %q", properties["owner"], "analytics") + } + + input["owner"] = "updated" + if properties["owner"] != "updated" { + t.Fatalf("normalizeNamespaceProperties should reuse the input map when non-nil") + } +} diff --git a/weed/s3api/s3tables/handler_namespace.go b/weed/s3api/s3tables/handler_namespace.go index b6a18d826..3610804f9 100644 --- a/weed/s3api/s3tables/handler_namespace.go +++ b/weed/s3api/s3tables/handler_namespace.go @@ -147,6 +147,7 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R Namespace: req.Namespace, CreatedAt: now, OwnerAccountID: bucketMetadata.OwnerAccountID, + Properties: req.Properties, } metadataBytes, err := json.Marshal(metadata) @@ -177,6 +178,7 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R resp := &CreateNamespaceResponse{ Namespace: req.Namespace, TableBucketARN: req.TableBucketARN, + Properties: req.Properties, } h.writeJSON(w, http.StatusOK, resp) @@ -265,6 +267,7 @@ func (h *S3TablesHandler) handleGetNamespace(w http.ResponseWriter, r *http.Requ Namespace: metadata.Namespace, CreatedAt: metadata.CreatedAt, OwnerAccountID: metadata.OwnerAccountID, + Properties: metadata.Properties, } h.writeJSON(w, http.StatusOK, resp) diff --git a/weed/s3api/s3tables/types.go b/weed/s3api/s3tables/types.go index 55c0f5f19..05fb661a8 100644 --- a/weed/s3api/s3tables/types.go +++ b/weed/s3api/s3tables/types.go @@ -77,19 +77,22 @@ type DeleteTableBucketPolicyRequest struct { // Namespace types type Namespace struct { - Namespace []string `json:"namespace"` - CreatedAt time.Time `json:"createdAt"` - OwnerAccountID string `json:"ownerAccountId"` + Namespace []string `json:"namespace"` + CreatedAt time.Time `json:"createdAt"` + OwnerAccountID string `json:"ownerAccountId"` + Properties map[string]string `json:"properties,omitempty"` } type CreateNamespaceRequest struct { - TableBucketARN string `json:"tableBucketARN"` - Namespace []string `json:"namespace"` + TableBucketARN string `json:"tableBucketARN"` + Namespace []string `json:"namespace"` + Properties map[string]string `json:"properties,omitempty"` } type CreateNamespaceResponse struct { - Namespace []string `json:"namespace"` - TableBucketARN string `json:"tableBucketARN"` + Namespace []string `json:"namespace"` + TableBucketARN string `json:"tableBucketARN"` + Properties map[string]string `json:"properties,omitempty"` } type GetNamespaceRequest struct { @@ -98,9 +101,10 @@ type GetNamespaceRequest struct { } type GetNamespaceResponse struct { - Namespace []string `json:"namespace"` - CreatedAt time.Time `json:"createdAt"` - OwnerAccountID string `json:"ownerAccountId"` + Namespace []string `json:"namespace"` + CreatedAt time.Time `json:"createdAt"` + OwnerAccountID string `json:"ownerAccountId"` + Properties map[string]string `json:"properties,omitempty"` } type ListNamespacesRequest struct { diff --git a/weed/s3api/s3tables/utils.go b/weed/s3api/s3tables/utils.go index a969cba3b..762e51cb1 100644 --- a/weed/s3api/s3tables/utils.go +++ b/weed/s3api/s3tables/utils.go @@ -128,9 +128,10 @@ type tableBucketMetadata struct { // namespaceMetadata stores metadata for a namespace type namespaceMetadata struct { - Namespace []string `json:"namespace"` - CreatedAt time.Time `json:"createdAt"` - OwnerAccountID string `json:"ownerAccountId"` + Namespace []string `json:"namespace"` + CreatedAt time.Time `json:"createdAt"` + OwnerAccountID string `json:"ownerAccountId"` + Properties map[string]string `json:"properties,omitempty"` } // tableMetadataInternal stores metadata for a table diff --git a/weed/s3api/s3tables/utils_namespace_test.go b/weed/s3api/s3tables/utils_namespace_test.go index 1081d024c..d0d4b2a54 100644 --- a/weed/s3api/s3tables/utils_namespace_test.go +++ b/weed/s3api/s3tables/utils_namespace_test.go @@ -1,6 +1,8 @@ package s3tables import ( + "encoding/json" + "reflect" "strings" "testing" ) @@ -124,3 +126,61 @@ func TestExpandNamespace(t *testing.T) { } } } + +func TestNamespaceMetadataPropertiesRoundTrip(t *testing.T) { + testCases := []struct { + name string + metadata namespaceMetadata + }{ + { + name: "with properties", + metadata: namespaceMetadata{ + Namespace: []string{"analytics"}, + Properties: map[string]string{"owner": "finance"}, + OwnerAccountID: "123456789012", + }, + }, + { + name: "nil properties", + metadata: namespaceMetadata{ + Namespace: []string{"analytics"}, + Properties: nil, + OwnerAccountID: "123456789012", + }, + }, + { + name: "empty properties", + metadata: namespaceMetadata{ + Namespace: []string{"analytics"}, + Properties: map[string]string{}, + OwnerAccountID: "123456789012", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + data, err := json.Marshal(tc.metadata) + if err != nil { + t.Fatalf("json.Marshal(metadata) returned error: %v", err) + } + + var decoded namespaceMetadata + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("json.Unmarshal(data) returned error: %v", err) + } + + // Due to `omitempty`, nil and empty maps are unmarshaled as nil. + if len(tc.metadata.Properties) == 0 { + if decoded.Properties != nil { + t.Fatalf("expected nil properties for empty/nil input, got %v", decoded.Properties) + } + return + } + + if !reflect.DeepEqual(decoded.Properties, tc.metadata.Properties) { + t.Fatalf("decoded.Properties = %v, want %v", decoded.Properties, tc.metadata.Properties) + } + }) + } +}