Files
seaweedfs/weed/s3api/s3tables/handler_view_rename_test.go
T
Chris Lu ec37ef5aaa iceberg: add view rename, scan-report and snapshots=refs to the catalog (#10776)
* iceberg: add view rename, scan-report and snapshots=refs to the catalog

Three gaps against the REST spec that clients hit in normal use:

Views had no rename, though tables did and views are stored the same way,
so the move is the same catalog-only pointer move. Tables and views share
a namespace directory, so both renames now refuse the other kind instead
of moving it.

Engines POST a scan or commit report after planning; a 404 there turns
into an error line per query. Accept the report and discard it - the
catalog keeps no metrics store.

LoadTable ignored ?snapshots=refs and always returned the whole snapshot
history, which is what clients use the parameter to avoid on long-lived
tables.

* iceberg: authorize view rename against the view ARN, tighten the metrics endpoint

Review follow-ups:

The shared rename checked the source against a table ARN whatever the
kind, so a policy scoped to a view's own ARN never matched and one
written for a table ARN was evaluated for a view. The entry kind now
carries the ARN builder.

The metrics endpoint truncated a report at 1 MiB and then failed to parse
it, answering 400 for a query that had actually succeeded. Read one byte
past the limit to tell "fits" from "cut short", and discard an oversized
report instead of rejecting it. Empty bodies and reports without a
report-type are now rejected, which the REST schema requires.

?snapshots= is defined for LoadTable, so it no longer filters what
CreateTable echoes back.
2026-08-16 12:56:45 -07:00

135 lines
4.8 KiB
Go

package s3tables
import (
"context"
"encoding/json"
"path"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// seedView adds a view alongside the table the rename harness already creates.
func seedView(t *testing.T, fs *memFilerServer, name string) {
t.Helper()
viewMeta, err := json.Marshal(tableMetadataInternal{
Name: name,
Namespace: "ns",
Format: "ICEBERG",
OwnerAccountID: DefaultAccountID,
MetadataVersion: 1,
MetadataLocation: "s3://" + renameTestBucket + "/ns/" + name + "/metadata/v1.metadata.json",
})
require.NoError(t, err)
fs.putEntry(GetNamespacePath(renameTestBucket, "ns"), name, map[string][]byte{
ExtendedKeyMetadata: viewMeta,
ExtendedKeyMetadataVersion: []byte("1"),
ExtendedKeyEntryType: []byte(EntryTypeView),
})
viewPath := GetTablePath(renameTestBucket, "ns", name)
fs.putEntry(viewPath, "metadata", nil)
fs.putEntry(path.Join(viewPath, "metadata"), "v1.metadata.json", nil)
}
func runRenameView(t *testing.T, m *Manager, fs *memFilerServer, sourceName, destName string) error {
t.Helper()
return m.Execute(context.Background(), NewManagerClient(fs.client), "RenameView", &RenameTableRequest{
TableBucketARN: mustBucketARN(t),
SourceNamespace: []string{"ns"},
SourceName: sourceName,
DestNamespace: []string{"ns"},
DestName: destName,
}, nil, "")
}
func TestRenameViewMovesCatalogPointer(t *testing.T) {
fs, m := startRenameManager(t)
seedView(t, fs, "v")
require.NoError(t, runRenameView(t, m, fs, "v", "v2"))
dest := fs.getEntry(GetNamespacePath(renameTestBucket, "ns"), "v2")
require.NotNil(t, dest)
assert.Equal(t, EntryTypeView, entryType(dest.Extended), "destination must stay a view")
var moved tableMetadataInternal
require.NoError(t, json.Unmarshal(dest.Extended[ExtendedKeyMetadata], &moved))
assert.Equal(t, "v2", moved.Name)
assert.Equal(t, "s3://"+renameTestBucket+"/ns/v/metadata/v1.metadata.json", moved.MetadataLocation,
"rename is catalog-only, the metadata stays where it was written")
src := fs.getEntry(GetNamespacePath(renameTestBucket, "ns"), "v")
require.NotNil(t, src)
_, stillListed := src.Extended[ExtendedKeyMetadata]
assert.False(t, stillListed, "source name must stop resolving")
assert.NotNil(t, fs.getEntry(path.Join(GetTablePath(renameTestBucket, "ns", "v"), "metadata"), "v1.metadata.json"),
"the view's metadata file must survive")
}
// Tables and views share one namespace directory, so each rename must refuse
// the other kind rather than moving it.
func TestRenameViewRejectsTable(t *testing.T) {
fs, m := startRenameManager(t)
err := runRenameView(t, m, fs, "t", "t2")
require.Error(t, err)
var s3Err *S3TablesError
require.ErrorAs(t, err, &s3Err)
assert.Equal(t, ErrCodeNoSuchView, s3Err.Type)
}
func TestRenameTableRejectsView(t *testing.T) {
fs, m := startRenameManager(t)
seedView(t, fs, "v")
err := runRename(t, m, fs, &RenameTableRequest{
TableBucketARN: mustBucketARN(t),
SourceNamespace: []string{"ns"},
SourceName: "v",
DestNamespace: []string{"ns"},
DestName: "v2",
})
require.Error(t, err)
var s3Err *S3TablesError
require.ErrorAs(t, err, &s3Err)
assert.Equal(t, ErrCodeNoSuchTable, s3Err.Type)
}
// A policy scoped to the view ARN must authorize renaming that view. Checking
// the source against a table ARN would silently ignore it.
func TestRenameViewAuthorizesAgainstTheViewARN(t *testing.T) {
fs, m := startRenameManager(t)
m.SetTrusted(false)
m.SetDefaultAllow(false)
seedView(t, fs, "v")
const principal = "analyst"
viewARN := "arn:aws:s3tables:" + DefaultRegion + ":" + DefaultAccountID + ":bucket/" + renameTestBucket + "/view/ns/v"
viewPolicy := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"` + principal +
`","Action":"s3tables:RenameView","Resource":"` + viewARN + `"}]}`
view := fs.getEntry(GetNamespacePath(renameTestBucket, "ns"), "v")
require.NotNil(t, view)
view.Extended[ExtendedKeyPolicy] = []byte(viewPolicy)
// Landing in the namespace needs create permission there as well.
namespacePolicy := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"` + principal +
`","Action":"s3tables:CreateView","Resource":"` + mustBucketARN(t) + `"}]}`
namespace := fs.getEntry(GetTableBucketPath(renameTestBucket), "ns")
require.NotNil(t, namespace)
namespace.Extended[ExtendedKeyPolicy] = []byte(namespacePolicy)
err := m.Execute(context.Background(), NewManagerClient(fs.client), "RenameView", &RenameTableRequest{
TableBucketARN: mustBucketARN(t),
SourceNamespace: []string{"ns"},
SourceName: "v",
DestNamespace: []string{"ns"},
DestName: "v2",
}, nil, principal)
require.NoError(t, err)
assert.NotNil(t, fs.getEntry(GetNamespacePath(renameTestBucket, "ns"), "v2"))
}