fix(aws-keyspaces): tag existence and restrictions, name validation, missing-keyspace error, GetTable omissions

providers/aws/keyspaces/keyspaces.go

@@ -7,6 +7,7 @@ package keyspaces
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"strings"
 	"sync"
 
@@ -57,7 +58,7 @@ func New(opts *config.Options) *Mock {
 		opts:        opts,
 	}
 
-	for _, sys := range []string{"system", "system_schema", "system_multiregion_info"} {
+	for _, sys := range systemKeyspaceNames() {
 		m.keyspaces.Set(sys, ksdriver.Keyspace{
 			Name: sys, ARN: m.keyspaceARN(sys), ReplicationStrategy: ksdriver.SingleRegion,
 			ReplicationRegions: []string{opts.Region},
@@ -79,13 +80,51 @@ func tableKey(keyspace, table string) string { return keyspace + "/" + table }
 
 func typeKey(keyspace, name string) string { return keyspace + "/" + name }
 
+// maxNameLen is the documented maximum length of keyspace, table, and
+// user-defined type names (API reference KeyspaceName/TableName/TypeName:
+// "Maximum length of 48"; devguide quotas: "Max UDT name length 48").
+const maxNameLen = 48
+
+// keyspaceTableNamePattern is the documented KeyspaceName/TableName pattern
+// ([a-zA-Z0-9][a-zA-Z0-9_]{0,47}), anchored: alphanumerics and underscores
+// only, not starting with an underscore. A leading digit is allowed.
+var keyspaceTableNamePattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_]{0,47}$`)
+
+// validName validates a keyspace or table name against the documented
+// KeyspaceName/TableName constraints.
 func validName(kind, name string) error {
 	if name == "" {
 		return cerrors.Newf(cerrors.InvalidArgument, "%s name is required", kind)
 	}
 
+	if len(name) > maxNameLen {
+		return cerrors.Newf(cerrors.InvalidArgument,
+			"%s name %q must be at most %d characters", kind, name, maxNameLen)
+	}
+
+	if !keyspaceTableNamePattern.MatchString(name) {
+		return cerrors.Newf(cerrors.InvalidArgument,
+			"%s name %q must match pattern [a-zA-Z0-9][a-zA-Z0-9_]{0,47}", kind, name)
+	}
+
+	return nil
+}
+
+// validTypeName validates a user-defined type name. The API model gives
+// TypeName only a 1-48 length constraint (no pattern), so only emptiness,
+// length, and the '/' used in internal store keys are checked.
+func validTypeName(name string) error {
+	if name == "" {
+		return cerrors.New(cerrors.InvalidArgument, "type name is required")
+	}
+
+	if len(name) > maxNameLen {
+		return cerrors.Newf(cerrors.InvalidArgument,
+			"type name %q must be at most %d characters", name, maxNameLen)
+	}
+
 	if strings.Contains(name, "/") {
-		return cerrors.Newf(cerrors.InvalidArgument, "%s name %q must not contain '/'", kind, name)
+		return cerrors.Newf(cerrors.InvalidArgument, "type name %q must not contain '/'", name)
 	}
 
 	return nil
@@ -120,6 +159,10 @@ func (m *Mock) CreateKeyspace(_ context.Context, cfg ksdriver.CreateKeyspaceConf
 		return nil, err
 	}
 
+	if err := validateTags(cfg.Tags); err != nil {
+		return nil, err
+	}
+
 	strategy := cfg.ReplicationStrategy
 	if strategy == "" {
 		strategy = ksdriver.SingleRegion
@@ -244,13 +287,15 @@ func (m *Mock) DeleteKeyspace(_ context.Context, name string) error {
 	return nil
 }
 
+// systemKeyspaceNames lists the four read-only system keyspaces every account
+// has ("Amazon Keyspaces uses four system keyspaces", devguide
+// working-with-keyspaces.html).
+func systemKeyspaceNames() []string {
+	return []string{"system", "system_schema", "system_schema_mcs", "system_multiregion_info"}
+}
+
 func isSystemKeyspace(name string) bool {
-	switch name {
-	case "system", "system_schema", "system_multiregion_info":
-		return true
-	default:
-		return false
-	}
+	return containsStr(systemKeyspaceNames(), name)
 }
 
 func (m *Mock) keyspaceHasTables(keyspace string) bool {

providers/aws/keyspaces/keyspaces_test.go

@@ -69,8 +69,8 @@ func TestKeyspaceLifecycle(t *testing.T) {
 		t.Fatalf("ListKeyspaces: %v", err)
 	}
 
-	if len(list) != 4 {
-		t.Fatalf("expected 4 keyspaces, got %d", len(list))
+	if len(list) != 5 { // 4 system + app
+		t.Fatalf("expected 5 keyspaces, got %d", len(list))
 	}
 }
 
@@ -114,11 +114,12 @@ func TestTableLifecycleAndValidation(t *testing.T) {
 	ctx := context.Background()
 	mustKeyspace(t, m, "app")
 
-	// Missing keyspace rejected.
+	// Missing keyspace is ResourceNotFound (the documented "tried to access a
+	// keyspace ... that doesn't exist" error), not a validation failure.
 	if _, err := m.CreateTable(ctx, ksdriver.CreateTableConfig{
 		KeyspaceName: "ghost", Name: "t", SchemaDefinition: sampleSchema(),
-	}); !cerrors.IsInvalidArgument(err) {
-		t.Fatalf("table in missing keyspace: got %v, want InvalidArgument", err)
+	}); !cerrors.IsNotFound(err) {
+		t.Fatalf("table in missing keyspace: got %v, want NotFound", err)
 	}
 
 	// No partition key rejected.

providers/aws/keyspaces/names_test.go

@@ -0,0 +1,115 @@
+package keyspaces
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	cerrors "github.com/stackshy/cloudemu/v2/errors"
+	ksdriver "github.com/stackshy/cloudemu/v2/services/keyspaces/driver"
+)
+
+// Keyspace/table names follow the documented KeyspaceName/TableName shape:
+// length 1-48, pattern [a-zA-Z0-9][a-zA-Z0-9_]{0,47}.
+func TestNameValidation(t *testing.T) {
+	m := newTestMock()
+	ctx := context.Background()
+
+	maxName := strings.Repeat("a", maxNameLen)
+	tooLong := strings.Repeat("a", maxNameLen+1)
+
+	valid := []string{"a", "A1", "1abc", "9", "my_keyspace", maxName}
+	for _, n := range valid {
+		if _, err := m.CreateKeyspace(ctx, ksdriver.CreateKeyspaceConfig{Name: n}); err != nil {
+			t.Fatalf("CreateKeyspace(%q): unexpected error %v", n, err)
+		}
+	}
+
+	invalid := []string{"", tooLong, "_lead", "has-dash", "has.dot", "has space", "a/b", "ünicode"}
+	for _, n := range invalid {
+		if _, err := m.CreateKeyspace(ctx, ksdriver.CreateKeyspaceConfig{Name: n}); !cerrors.IsInvalidArgument(err) {
+			t.Fatalf("CreateKeyspace(%q): got %v, want InvalidArgument", n, err)
+		}
+
+		if _, err := m.CreateTable(ctx, ksdriver.CreateTableConfig{
+			KeyspaceName: "a", Name: n, SchemaDefinition: sampleSchema(),
+		}); !cerrors.IsInvalidArgument(err) {
+			t.Fatalf("CreateTable(%q): got %v, want InvalidArgument", n, err)
+		}
+	}
+
+	if _, err := m.CreateTable(ctx, ksdriver.CreateTableConfig{
+		KeyspaceName: "a", Name: "1t", SchemaDefinition: sampleSchema(),
+	}); err != nil {
+		t.Fatalf("CreateTable leading digit: %v", err)
+	}
+
+	if _, err := m.CreateTable(ctx, ksdriver.CreateTableConfig{
+		KeyspaceName: "a", Name: maxName, SchemaDefinition: sampleSchema(),
+	}); err != nil {
+		t.Fatalf("CreateTable 48-char name: %v", err)
+	}
+
+	if _, err := m.RestoreTable(ctx, ksdriver.RestoreTableConfig{
+		SourceKeyspace: "a", SourceTable: "1t", TargetKeyspace: "a", TargetTable: tooLong,
+		RestoreTimestamp: m.opts.Clock.Now().Add(-1),
+	}); !cerrors.IsInvalidArgument(err) {
+		t.Fatalf("RestoreTable too-long target: got %v, want InvalidArgument", err)
+	}
+
+	fields := []ksdriver.FieldDefinition{{Name: "f", Type: "text"}}
+	if _, err := m.CreateType(ctx, "a", tooLong, fields); !cerrors.IsInvalidArgument(err) {
+		t.Fatalf("CreateType too long: got %v, want InvalidArgument", err)
+	}
+
+	if _, err := m.CreateType(ctx, "a", maxName, fields); err != nil {
+		t.Fatalf("CreateType 48-char name: %v", err)
+	}
+}
+
+// A well-formed request naming a parent keyspace that doesn't exist is
+// ResourceNotFound for CreateTable, CreateType, and RestoreTable's target.
+func TestMissingParentKeyspaceIsNotFound(t *testing.T) {
+	m := newTestMock()
+	ctx := context.Background()
+	mustKeyspace(t, m, "app")
+
+	if _, err := m.CreateTable(ctx, ksdriver.CreateTableConfig{
+		KeyspaceName: "ghost", Name: "t", SchemaDefinition: sampleSchema(),
+	}); !cerrors.IsNotFound(err) {
+		t.Fatalf("CreateTable: got %v, want NotFound", err)
+	}
+
+	if _, err := m.CreateType(ctx, "ghost", "addr", []ksdriver.FieldDefinition{{Name: "f", Type: "text"}}); !cerrors.IsNotFound(err) {
+		t.Fatalf("CreateType: got %v, want NotFound", err)
+	}
+
+	if _, err := m.CreateTable(ctx, ksdriver.CreateTableConfig{
+		KeyspaceName: "app", Name: "src", SchemaDefinition: sampleSchema(),
+	}); err != nil {
+		t.Fatalf("CreateTable: %v", err)
+	}
+
+	if _, err := m.RestoreTable(ctx, ksdriver.RestoreTableConfig{
+		SourceKeyspace: "app", SourceTable: "src", TargetKeyspace: "ghost", TargetTable: "dst",
+		RestoreTimestamp: m.opts.Clock.Now(),
+	}); !cerrors.IsNotFound(err) {
+		t.Fatalf("RestoreTable: got %v, want NotFound", err)
+	}
+}
+
+// All four documented system keyspaces exist and none can be deleted.
+func TestSystemKeyspaces(t *testing.T) {
+	m := newTestMock()
+	ctx := context.Background()
+
+	for _, name := range []string{"system", "system_schema", "system_schema_mcs", "system_multiregion_info"} {
+		if _, err := m.GetKeyspace(ctx, name); err != nil {
+			t.Fatalf("GetKeyspace(%s): %v", name, err)
+		}
+
+		if err := m.DeleteKeyspace(ctx, name); !cerrors.IsInvalidArgument(err) {
+			t.Fatalf("DeleteKeyspace(%s): got %v, want InvalidArgument", name, err)
+		}
+	}
+}

providers/aws/keyspaces/snapshot.go

@@ -47,9 +47,11 @@ func (m *Mock) Snapshot(_ context.Context, _ bool) (json.RawMessage, error) {
 		*d.dst = b
 	}
 
+	// Copy the tag maps under the lock: json.Marshal runs after the unlock, and
+	// TagResource mutates the inner maps in place.
 	m.mu.RLock()
 	if len(m.tags) > 0 {
-		snap.Tags = m.tags
+		snap.Tags = copyTagMaps(m.tags)
 	}
 	m.mu.RUnlock()
 
@@ -86,9 +88,58 @@ func (m *Mock) Restore(_ context.Context, data json.RawMessage) error {
 
 	if snap.Tags != nil {
 		m.mu.Lock()
-		m.tags = snap.Tags
+		m.tags = m.canonicalTagsLocked(snap.Tags)
 		m.mu.Unlock()
 	}
 
 	return nil
 }
+
+// canonicalTagsLocked keys restored tags by the canonical ARN of the keyspace
+// or table they belong to. Tag reads and writes resolve to the canonical ARN,
+// but an older build stored tags under whatever ARN the caller passed (for
+// example the slash-less table form) and accepted ARNs of resources that don't
+// exist. A non-canonical entry for an existing resource is merged into the
+// canonical one, with the canonical entry's values winning on a key clash. An
+// entry that names no existing resource is dropped: no API can reach it, and
+// keeping it would hand its tags to a resource created later under that ARN.
+// The caller holds the write lock, and the stores are already restored.
+func (m *Mock) canonicalTagsLocked(in map[string]map[string]string) map[string]map[string]string {
+	out := make(map[string]map[string]string, len(in))
+
+	for arn, tags := range in {
+		if m.existingResourceARN(arn) == arn {
+			out[arn] = copyTags(tags)
+		}
+	}
+
+	for arn, tags := range in {
+		canonical := m.existingResourceARN(arn)
+		if canonical == "" || canonical == arn || arn+"/" != canonical {
+			continue
+		}
+
+		cur := out[canonical]
+		if cur == nil {
+			cur = make(map[string]string, len(tags))
+			out[canonical] = cur
+		}
+
+		for k, v := range tags {
+			if _, ok := cur[k]; !ok {
+				cur[k] = v
+			}
+		}
+	}
+
+	return out
+}
+
+func copyTagMaps(in map[string]map[string]string) map[string]map[string]string {
+	out := make(map[string]map[string]string, len(in))
+	for arn, tags := range in {
+		out[arn] = copyTags(tags)
+	}
+
+	return out
+}