Skip to content

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

Merged
NitinKumar004 merged 4 commits into
stackshy:developmentfrom
Satyam-Trivedi-ZS:fix/aws-keyspaces-parity-1512
Oct 23, 2026
Merged

NitinKumar004 merged 4 commits into
stackshy:developmentfrom
Satyam-Trivedi-ZS:fix/aws-keyspaces-parity-1512

Conversation

@Satyam-Trivedi-ZS

@Satyam-Trivedi-ZS Satyam-Trivedi-ZS commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes the AWS Keyspaces parity gaps listed in #1512, checked against the Keyspaces API reference, the developer guide and the botocore keyspaces/2022-02-10 model:

  • Tagging operations now require an existing keyspace or table, and tags follow the documented restrictions.
  • Keyspace and table names follow the documented 48-character pattern.
  • A missing parent keyspace returns ResourceNotFoundException.
  • GetTable no longer sends readCapacityUnits/writeCapacityUnits = 0 for PAY_PER_REQUEST, or viewType: "" when CDC is off.

Issue coverage (#1512)

Item Status Evidence
(Medium) Tag/Untag/ListTags on a nonexistent resource ARN succeed Fixed All three operations list ResourceNotFoundException (TagResource, UntagResource, ListTagsForResource). The ARN must match arn:(aws[a-zA-Z0-9-]*):cassandra:.+.*, length 20–1000; anything else is now ValidationException. A well-formed ARN that names no existing keyspace or table, or names another region/account, is ResourceNotFoundException. The trailing / is optional: the docs show table ARNs without it (example), and both forms resolve to the same resource.
(Medium) Names starting with a digit accepted Not a real gap (related gap fixed) The documented KeyspaceName/TableName pattern is [a-zA-Z0-9][a-zA-Z0-9_]{0,47} (CreateKeyspace, CreateTable, botocore model), so a leading digit is valid and is still accepted. The console rules say the same: "alphanumeric characters and underscore" (getting started). The real gap was that validName only rejected "" and /. Names such as bad-name, a.b, has space and _lead now return ValidationException.
(Medium) Names longer than 48 chars accepted Fixed KeyspaceName, TableName and TypeName all have max length 48, and the quotas page also lists "Max UDT name length 48". The limit is enforced on CreateKeyspace, CreateTable, the RestoreTable target table, and CreateType. TypeName has no pattern in the model, so type names get the length check only.
(Low) Missing parent keyspace returns ValidationException, not ResourceNotFoundException Fixed (docs-based) CreateTable, CreateType and RestoreTable list ResourceNotFoundException, defined as "The operation tried to access a keyspace, table, or type that doesn't exist" (ResourceNotFoundException). ValidationException means "invalid or malformed request". CreateTable, CreateType and the RestoreTable target keyspace now return ResourceNotFoundException, matching what GetType, ListTables and ListTypes already did. I have no captured real-cloud response for this case; it rests on the reference text.
(Low) PAY_PER_REQUEST capacitySpecification emits RCU/WCU 0 Fixed In CapacitySpecificationSummary, RCU/WCU are optional with "Valid Range: Minimum value of 1". The documented get-table output for an on-demand table shows only throughputMode (devguide). RCU/WCU are now emitted only for PROVISIONED tables.
(Low) cdcSpecification.viewType "" emitted when CDC is disabled Fixed viewType is an optional enum (NEW_IMAGE, OLD_IMAGE, KEYS_ONLY, NEW_AND_OLD_IMAGES), so "" is never a valid value. The encoder now drops an unset viewType.
(Low) DeleteKeyspace on a system keyspace: error code unconfirmed Rejection confirmed; error code unverifiable (kept ValidationException) The system keyspaces page says "You cannot modify or delete system keyspaces", so rejecting the delete is right. No doc says which error code is returned, so ValidationException is unchanged. The same page lists four system keyspaces, including system_schema_mcs, which cloudemu was missing. It is now seeded and also can't be deleted.

Tag restrictions added alongside the existence check

From Restrictions for using tags in Amazon Keyspaces and the model (TagKey 1–128, TagValue 1–256, TagList 1–60). These apply on TagResource and on the tags of CreateKeyspace, CreateTable and RestoreTable (tagsOverride):

Input Result
Key with the aws: prefix (case-insensitive) ValidationException ("AWS-assigned tag names ... have the aws: prefix, which you can't assign")
Empty key or empty value ValidationException ("In Amazon Keyspaces the value cannot be empty or null")
Key over 128 or value over 256 Unicode characters ValidationException
Characters outside letters, white space, numbers and + - = . _ : / ValidationException
More than 60 tags in one request, or an empty tag list on Tag/UntagResource ValidationException (the TagList bounds)

Not enforced: the guide's "maximum number of tags per resource is 50". No doc says which error a 51st tag returns (ValidationException vs ServiceQuotaExceededException), so I left it alone rather than guess.

Changes

  • providers/aws/keyspaces/keyspaces.go:
    • validName enforces the anchored documented pattern and the 48-character maximum.
    • New validTypeName enforces length 1–48.
    • systemKeyspaceNames() returns all four system keyspaces and is used for both seeding and the delete guard.
  • providers/aws/keyspaces/tags.go:
    • resolveTaggableLocked validates the ARN, maps it to the canonical ARN of an existing keyspace or table (trailing / optional), and is used by Tag, Untag and ListTags.
    • validateTags applies the tag restrictions.
  • providers/aws/keyspaces/tables.go, udt.go: a missing parent or target keyspace now returns cerrors.NotFound; tags are validated on CreateTable and RestoreTable.
  • server/aws/keyspaces/types.go: toWireCapacity emits RCU/WCU only for PROVISIONED.
  • server/aws/keyspaces/wirejson.go: an unset cdcSpecification.viewType is dropped (the rule is scoped to that member, not to empty strings in general). lowerCamelKeys is split into a helper, which keeps gocyclo quiet.
  • server/aws/keyspaces/tags.go, keyspaces.go, tables.go: the TagList 60-item bound on the raw wire list.

Added after review:

  • providers/aws/keyspaces/tags.go: the provider also enforces the 60-item TagList bound (TagResource, UntagResource, CreateKeyspace, CreateTable, RestoreTable), so the Go library and serve agree.
  • providers/aws/rgt_discovery.go: project skips a resource whose tag read returns NotFound (deleted between list and tag read) instead of aborting DiscoverResources for every service. This covers Keyspaces and the other single-list walkers that use project.
  • providers/aws/keyspaces/snapshot.go: Snapshot deep-copies the tag maps under the lock (fixes a DATA RACE with a concurrent TagResource). Restore keys tags by canonical ARN (see the release note).

The business rules stay in the provider; the handler only repeats the list bound on the raw wire list (which can repeat a key). No driver interface or operation changed, so go generate produces no diff. No new state: tags were already snapshotted, and system keyspaces are seeded in New.

Release note

  • Restoring a snapshot from an older build: tags stored under a non-canonical ARN of an existing keyspace or table (for example the slash-less table ARN) are moved onto the canonical ARN, with the canonical entry winning on a key clash. Tag entries for ARNs that name no existing keyspace or table are dropped, since no API can reach them and they would otherwise attach to a resource created later under that ARN.
  • Tagging a keyspace or table that doesn't exist now returns ResourceNotFoundException instead of succeeding.

Behaviour change worth noting: ListKeyspaces on a fresh emulator now returns 4 system keyspaces instead of 3. The two existing tests that counted them are updated.

Provider Coverage

  • AWS
  • Azure (N/A: Keyspaces is AWS-only)
  • GCP (N/A)
  • OCI (N/A)

Checklist

  • All tests pass (go test -p 2 ./...)
  • Linter passes (golangci-lint run --timeout=9m ./...). Not claimed: development already has pre-existing findings. On the touched packages, golangci-lint run --new-from-rev=upstream/development ./providers/aws/keyspaces/... ./server/aws/keyspaces/... reports no new findings in touched packages (0 issues). After the review fixes, the same run over those packages plus ./providers/aws/ also reports 0 issues.
  • Every provider the change applies to implements the same behavior (AWS only)
  • Integration tests added to cloudemu_test.go (wire tests use the real aws-sdk-go-v2 client instead)
  • Unit tests added to provider test files
  • Regenerated docs (N/A: no interface or operation changes; go generate would produce no diff)

Test Plan

Provider tests (providers/aws/keyspaces/names_test.go, tags_test.go, snapshot_test.go):

  • TestNameValidation
  • TestMissingParentKeyspaceIsNotFound
  • TestSystemKeyspaces
  • TestTagsRequireExistingResource
  • TestTagRestrictions
  • TestTagListBound (added after review)
  • TestTagARNForms
  • TestSnapshotConcurrentWithTagResource (added after review; run with -race)
  • TestRestoreCanonicalizesLegacyTagARNs (added after review)

RGT discovery tests (providers/aws/rgt_discovery_test.go, added after review):

  • TestProjectSkipsVanishedResource (deterministic)
  • TestKeyspacesDiscoverySurvivesConcurrentDelete (create/delete loop next to discoverKeyspaces; failed at walk 53 before the fix)

SDK wire tests with the real aws-sdk-go-v2/service/keyspaces client (server/aws/keyspaces/tags_test.go, tables_test.go):

  • TestSDKTagsOnMissingResource
  • TestSDKTagRestrictionsAndSystemKeyspace
  • TestSDKNameValidationAndMissingKeyspace
  • TestGetTableOmitsUnsetFields, which inspects the raw JSON body

Fail before, pass after. I checked out the upstream/development versions of the non-test files and ran the new tests:

  • The provider test package does not build on the base, because the tests reference the new name and tag limit constants.
  • Every new SDK test fails there, for example TagResource: got <nil>, want ResourceNotFoundException and ppr capacitySpecification has readCapacityUnits: map[readCapacityUnits:0 ...].
  • All of them pass on this branch.

Checks:

  • go build ./...: OK.
  • go vet on both touched packages: OK.
  • go test -p 2 ./...: no failures.
  • go test on providers/aws/keyspaces, server/aws/keyspaces, providers/aws, compat/aws and persist/... after the final lint refactor: OK.

Real-user check. I ran cloudemu serve -providers aws -aws-port 4602 in the background and drove it with AWS CLI v1 (botocore 1.42), --endpoint-url http://127.0.0.1:4602 --region us-east-1, with a timeout on every call. Afterwards I killed the server and confirmed nothing was still listening on 4602.

# names
create-keyspace --keyspace-name 1app                          -> 200, arn .../keyspace/1app/   (leading digit allowed)
create-keyspace --keyspace-name <49 x a>                      -> ValidationException: must be at most 48 characters
create-keyspace --keyspace-name <48 x a>                      -> 200
create-keyspace --keyspace-name bad-name                      -> ValidationException: must match pattern [a-zA-Z0-9][a-zA-Z0-9_]{0,47}
create-keyspace --keyspace-name _lead                         -> ValidationException (pattern)
create-table --table-name <49 x t>                            -> ValidationException
create-type --type-name <49 x t>                              -> ValidationException
# missing parent keyspace
create-table --keyspace-name ghost ...                        -> ResourceNotFoundException
create-type --keyspace-name ghost ...                         -> ResourceNotFoundException
# capacity / cdc
get-table 1app/2ppr (on-demand)  -> capacitySpecification {"throughputMode":"PAY_PER_REQUEST"}, cdcSpecification {"status":"DISABLED"}
get-table 1app/prov (PROVISIONED 5/7) -> readCapacityUnits 5, writeCapacityUnits 7
raw POST KeyspacesService.GetTable -> body has no readCapacityUnits/writeCapacityUnits/viewType keys
# tags: existence / ARN
tag-resource / untag-resource / list-tags-for-resource on .../keyspace/ghost/   -> ResourceNotFoundException (x3)
list-tags-for-resource .../keyspace/1app/table/ghost/                           -> ResourceNotFoundException
list-tags-for-resource arn:aws:cassandra:us-west-2:...:/keyspace/1app/          -> ResourceNotFoundException
tag-resource --resource-arn arn:aws:dynamodb:...                                -> ValidationException
# tags: restrictions
tag-resource --tags key=aws:owner,value=v / key=AWS:owner,value=v -> ValidationException (reserved prefix)
tag-resource --tags 'key=k*,value=v'                              -> ValidationException (characters)
create-keyspace --tags key=aws:x,value=v                          -> ValidationException
tag-resource with 61 tags                                         -> ValidationException: tags must contain at most 60 items
tag-resource --tags key=k,value=      -> rejected client-side by the CLI; raw POST with "value":"" -> 400 ValidationException
raw POST TagResource with "tags":[]   -> 400 ValidationException
# tags: happy path
tag-resource 1app 'team name'='data + ml', env=prod -> 200; list -> both tags (sorted)
tag-resource .../keyspace/1app/table/prov (no trailing slash) -> 200; list via .../table/prov/ -> t=1
list-tags-for-resource --max-results 1 -> 1 tag + nextToken
untag-resource env -> 200; list -> only 'team name'
create-keyspace tagged --tags env=dev -> list shows env=dev
delete-table prov -> list-tags on its ARN -> ResourceNotFoundException
# system keyspaces
list-keyspaces -> includes system, system_multiregion_info, system_schema, system_schema_mcs
delete-keyspace system_schema_mcs / system -> ValidationException (cannot be deleted)

Real-user check of the review fixes (same setup, AWS CLI v1 against serve on 4602):

tag-resource with 61 tags -> ValidationException: tags must contain at most 60 items; 60 tags -> 200
resourcegroupstaggingapi get-resources x40 while 8 parallel workers create/delete keyspaces -> 40 ok, 0 failed
GET /_cloudemu/snapshot, add a slash-less table ARN entry {env:old, team:data} and a ghost keyspace entry, POST it back -> 200
list-tags-for-resource on the canonical table ARN -> env=prod, team=data (canonical value kept, legacy key merged)
create-keyspace ghost; list-tags-for-resource -> [] (stale entry not inherited)
delete-table; list-tags-for-resource -> ResourceNotFoundException

Over HTTP I could not hit the list/tag-read window on the pre-fix binary either, so the race is covered by the in-process tests above.

Not run:

  • Terraform is not installed in my environment.
  • hashicorp/aws has aws_keyspaces_keyspace and aws_keyspaces_table, which would exercise tagging and capacity. That is worth a reviewer run.

Related Issues

Part of #1512

Follow-ups noticed (not changed here)

  • The botocore model declares ResourceNotFoundException as httpStatusCode: 404, but the HTML API reference says HTTP 400. cloudemu uses 400. SDKs dispatch on the error code, so typed errors match either way.
  • The API reference says DeleteKeyspace "deletes a keyspace and all of its tables", but cloudemu rejects deleting a non-empty keyspace with ConflictException.
  • When CDC is enabled, viewType is neither stored nor echoed; the documented default is NEW_AND_OLD_IMAGES.
  • The per-resource limit of 50 tags is not enforced; see above.
  • From the review (pre-existing, same on the base): GetTable always emits clientSideTimestamps {status: DISABLED} and ttl {status: DISABLED}, which makes Terraform plan a replacement when the config sets neither; hashicorp/aws 6.69.x speaks rpc-v2-cbor to /service/KeyspacesService/operation/..., which serve doesn't route yet.
  • The paginated tagged-discovery walkers (Glue, Kafka, Kinesis, GuardDuty) don't use project and still abort on a NotFound tag read. They need the same skip.

…missing-keyspace error, GetTable omissions

- TagResource/UntagResource/ListTagsForResource validate the ARN shape
  (ValidationException) and require an existing keyspace or table
  (ResourceNotFoundException); the trailing "/" is optional.
- Tags follow the documented restrictions on every path that accepts
  them: key 1-128 and value 1-256 characters, allowed character set,
  reserved "aws:" prefix, and at most 60 tags per request.
- Keyspace/table names enforce the documented 1-48 char pattern
  [a-zA-Z0-9][a-zA-Z0-9_]{0,47} (a leading digit is allowed); UDT names
  get the documented 48-char limit.
- CreateTable/CreateType/RestoreTable on a missing parent keyspace return
  ResourceNotFoundException instead of ValidationException.
- GetTable omits readCapacityUnits/writeCapacityUnits for
  PAY_PER_REQUEST and an unset cdcSpecification.viewType.
- Seed the documented fourth system keyspace, system_schema_mcs, which
  likewise cannot be deleted.

Part of stackshy#1512

@NitinKumar004 NitinKumar004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I checked

  • Every item in #1512 against the Keyspaces API reference and the tagging restrictions page. The leading-digit item: I agree it is not a real gap, since the documented KeyspaceName/TableName pattern is [a-zA-Z0-9][a-zA-Z0-9_]{0,47}. The tag character set, the 128/256 lengths, the non-empty value and the aws: prefix all match the TaggingRestrictions page word for word.
  • Layering: the rules live in the provider. The handler only adds the TagList bound. No driver or interface change, and go generate gives no diff.
  • Locking: TagResource, UntagResource and ListTagsForResource resolve the ARN and read or write m.tags under one m.mu critical section. DeleteKeyspace and DeleteTable drop the tag entry under the same lock, so a tag call can't race a delete inside the provider.
  • Gates on the PR head: go build ./... ok. go vet ok on the touched packages. go test -race ok on providers/aws/keyspaces and server/aws/keyspaces. go test ok on providers/aws, server/aws, compat/aws, persist and services/cost. gofmt is clean, go mod tidy gives no diff and go generate gives no diff. golangci-lint 2.14.0 on the two touched packages, run without --new-from-rev, reports 0 issues. The merge base reports 1 (goconst), so the PR adds nothing and fixes one.
  • AWS CLI against cloudemu serve: a leading-digit name is accepted. A 49-character name and bad-name get ValidationException. CreateTable in a missing keyspace gets ResourceNotFoundException. An on-demand GetTable returns only throughputMode. Tagging a missing keyspace gets ResourceNotFoundException. The slash-less table ARN tags the same resource as the slash form. aws: keys, @ and a non-cassandra ARN all get ValidationException. system_schema_mcs is listed and can't be deleted. After DeleteTable, ListTags gets ResourceNotFoundException. Resource Groups Tagging API TagResources on a missing keyspace returns a FailedResourcesMap entry, and on an existing one it succeeds and GetResources shows the tag.
  • Terraform hashicorp/aws 6.66.0 (aws_keyspaces_keyspace plus a PAY_PER_REQUEST table and a PROVISIONED table, all tagged): apply, then plan exit 0, then update the tags and the RCU, then plan exit 0, then a second update, then plan exit 0, then destroy. After destroy, ListTags gets ResourceNotFoundException. Plan only reaches 0 when the config sets client_side_timestamps and ttl explicitly. See the Low notes.

High

None.

Medium

Keyspaces discovery in Resource Groups Tagging API now fails while a keyspace is being deleted. taggedDiscovery.discoverKeyspaces (providers/aws/rgt_discovery.go:193) calls ListKeyspaces and then, in a separate call, ListTagsForResource(k.ARN) for each keyspace. project returns on the first error. Before this PR, ListTagsForResource on an ARN that had just gone away returned an empty list. Now it returns NotFound. That NotFound aborts DiscoverResources for every service, not just Keyspaces, because the collector loop returns the error too. So any GetResources call that overlaps a DeleteKeyspace fails as a whole. Tests and CI jobs that share one serve instance will hit this. I reproduced it with a small test that loops CreateKeyspace/DeleteKeyspace in one goroutine and calls discoverKeyspaces in another. On this branch 2991 of 3000 calls failed with NotFound: resource ".../keyspace/ks1/" not found. On the merge base none failed. The existence check is correct for the public API. The fix belongs in the walker: skip a keyspace whose ListTags returns cerrors.IsNotFound, the same as a resource that was never listed. Another option is a provider method that returns keyspaces together with their tags under a single lock. Please add a regression test for it.

Low

  • The 60-item TagList bound is only enforced in the wire handler (server/aws/keyspaces/tags.go:82). A Go library caller can still pass 61 tags to TagResource, CreateKeyspace or CreateTable. A map can't hold duplicate keys, so the provider can apply the same len(tags) > 60 check. That keeps the library and serve in agreement.
  • omitEmptyEnum (server/aws/keyspaces/wirejson.go:41) matches the key name ViewType anywhere in any response. That is fine today, but the doc comment describes a general rule. Either make the comment say it covers only cdcSpecification.viewType, or make the encoder skip zero-valued string enums in general.
  • Snapshot compatibility: tags saved by an older build under a non-canonical ARN, such as the slash-less table form, are restored but can no longer be reached or deleted, because every read now goes through the canonical ARN. This is minor, but a release note would help.
  • Pre-existing, not introduced here, but worth a ticket. (1) Snapshot copies the m.tags map reference under RLock and marshals it after unlocking. A concurrent TagResource then races with it: -race reports a DATA RACE between snapshot.go:56 and tags.go:29. (2) GetTable always emits clientSideTimestamps {status: DISABLED} and ttl {status: DISABLED}. With a table that sets neither, Terraform 6.66.0 plans a forced replacement on every run. (3) hashicorp/aws 6.69.x sends Keyspaces requests as smithy-protocol: rpc-v2-cbor to /service/KeyspacesService/operation/..., and serve answers UnknownOperationException, so the current provider can't create a keyspace at all. All three behave the same on the merge base.
  • The test file names parity_test.go and parity_sdk_test.go are generic. STRUCTURE.md prefers feature names, for example tags_test.go and names_test.go.

defer m.mu.RUnlock()

tags := m.tags[arn]
canonical, err := m.resolveTaggableLocked(arn)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now returns NotFound for an ARN that was deleted a moment ago. That is right for the API, but taggedDiscovery.discoverKeyspaces (providers/aws/rgt_discovery.go:193) lists keyspaces and then calls this per keyspace in a separate call. If a DeleteKeyspace lands in between, the NotFound aborts the whole RGT GetResources call for every service. I reproduced it with a create/delete loop running next to discoverKeyspaces: 2991 of 3000 calls failed on this branch and 0 failed on the base. Can the walker skip cerrors.IsNotFound here, and can you add a regression test?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a8e8329. project in providers/aws/rgt_discovery.go now skips an item whose tag read returns cerrors.IsNotFound, the same way the resourcediscovery walkers handle the S3 and DynamoDB race. Other errors still propagate. I put the skip in project rather than only in discoverKeyspaces, so every single-list walker that uses it gets it.

Regression tests are in providers/aws/rgt_discovery_test.go:

  • TestProjectSkipsVanishedResource is deterministic: NotFound is skipped and any other error propagates.
  • TestKeyspacesDiscoverySurvivesConcurrentDelete is your create/delete loop next to discoverKeyspaces. Before the fix it failed at walk 53; it now passes 2000 walks under -race.


// rejectTagListTooLong writes a ValidationException and returns true when a
// request's tag list exceeds the documented TagList size.
func rejectTagListTooLong(w http.ResponseWriter, n int) bool {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only serve callers get this TagList bound. The Go library path (TagResource, CreateKeyspace, CreateTable, RestoreTable) still accepts 61 or more tags. Could validateTags in the provider check len(tags) > 60 as well, so both surfaces agree?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0be1e71. validateTags now rejects more than 60 tags, so TagResource, CreateKeyspace, CreateTable and RestoreTable all get the bound. UntagResource checks len(keys) > 60 as well. I kept the handler check because the raw wire list can repeat a key and be longer than the map the provider sees. Test: TestTagListBound (61 is rejected on all five ops, 60 is accepted).

// a table without a CDC stream), so an empty value is dropped rather than sent
// as "".
func omitEmptyEnum(key string, val any) bool {
return key == "ViewType" && val == ""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches ViewType at any depth in any response, while the comment above describes a general rule for unset string enums. Either make the comment say it covers only cdcSpecification.viewType, or make the encoder skip zero-valued string enums in general.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scoped the comment in 0be1e71. In the Keyspaces SDK, ViewType only appears on CdcSpecification and CdcSpecificationSummary, so the comment now says it covers only cdcSpecification.viewType. I did not make it a general rule: by that point the decoded map has no Go types, so a general rule would also drop non-enum strings that are legitimately empty.

A resource deleted between a service's list call and its tag read makes
the tag read return NotFound. project returned that error, which aborted
DiscoverResources for every service. Keyspaces hit it once
ListTagsForResource started rejecting unknown ARNs. Skip the vanished
resource instead, as the resourcediscovery walkers already do; other
errors still propagate.
…e tag ARNs on restore

Snapshot handed json.Marshal the live tag maps after releasing the lock,
so a concurrent TagResource raced it. Deep-copy them under the lock.

Restore now keys tags by the canonical ARN of an existing keyspace or
table. Tags an older build stored under the slash-less ARN form are
merged in (the canonical entry wins on a clash), and entries for ARNs
that name no resource are dropped, so they can't leak onto a resource
created later under that ARN.
TagResource, UntagResource, CreateKeyspace, CreateTable and RestoreTable
now reject more than 60 tags on the library path too, matching the
wire handler. Scope the omitEmptyEnum comment to
cdcSpecification.viewType, and rename the parity test files to feature
names (names_test.go, tags_test.go, tables_test.go).
@Satyam-Trivedi-ZS

Copy link
Copy Markdown
Collaborator Author

Thanks for the thorough review, @NitinKumar004. I pushed three commits on top of 7cedcbe. Here is each finding:

Medium: RGT discovery fails while a keyspace is being deleted
Fixed in a8e8329. project in providers/aws/rgt_discovery.go now skips a resource whose tag read returns NotFound, the same as a resource that was never listed. Other errors still propagate. This matches how the resourcediscovery walkers handle the S3 and DynamoDB race, and it covers every single-list walker that uses project, not only Keyspaces.

  • Tests: TestProjectSkipsVanishedResource (deterministic) and TestKeyspacesDiscoverySurvivesConcurrentDelete (your create/delete loop). The loop test failed at walk 53 before the fix and passes 2000 walks under -race now.
  • AWS CLI: 40 get-resources calls while 8 parallel workers created and deleted keyspaces: 40 ok, 0 failed.

Low: the 60-tag bound only applies in the handler
Fixed in 0be1e71. The provider's validateTags rejects more than 60 tags, so TagResource, CreateKeyspace, CreateTable and RestoreTable all get the bound, and UntagResource checks its key list too. The handler check stays because the raw wire list can repeat a key.

  • Test: TestTagListBound.

Low: omitEmptyEnum comment is too general
Fixed in 0be1e71. The comment is now scoped to cdcSpecification.viewType, which is the only place ViewType appears in the Keyspaces SDK. I did not generalise it, because the decoded map has no Go types and a general rule would drop legitimately empty strings.

Low: snapshot compatibility for non-canonical ARNs
Fixed in d637012, and there is a release note in the PR description. On restore, tags are now keyed by the canonical ARN:

  • A slash-less entry for an existing keyspace or table is merged in. If the same key appears in both, the canonical entry's value wins.
  • An entry for an ARN that names no resource is dropped. Otherwise a resource created later under that ARN would inherit its tags.
  • Test: TestRestoreCanonicalizesLegacyTagARNs. I also checked it end to end: I patched a GET /_cloudemu/snapshot dump with a slash-less entry and a ghost entry and POSTed it back. The tags were reachable on the canonical ARN, the new ghost keyspace had no tags, and DeleteTable cleared them.

Low: generic test file names
Fixed in 0be1e71. The files are now providers/aws/keyspaces/names_test.go and tags_test.go, and server/aws/keyspaces/tags_test.go and tables_test.go.

Pre-existing (1): Snapshot data race
Fixed in d637012. Snapshot now deep-copies the tag maps under the RLock.

  • Test: TestSnapshotConcurrentWithTagResource. It reports a DATA RACE without the fix and is clean with it.

Pre-existing (2): GetTable always emits clientSideTimestamps/ttl DISABLED
Follow-up, not changed here. I confirmed it in server/aws/keyspaces/tables.go (toWireTable). The fix means tracking whether each setting was ever set, which is more than a review fix.

Pre-existing (3): hashicorp/aws 6.69.x sends rpc-v2-cbor requests
Follow-up. serve needs a Smithy RPCv2 CBOR route for Keyspaces, which is a protocol feature of its own.

Other follow-ups I noticed:

  • The paginated tagged-discovery walkers (Glue, Kafka, Kinesis, GuardDuty) don't use project, so they still abort on a NotFound tag read. They need the same skip.
  • The documented limit of 50 tags per resource is still not enforced. No doc says which error code it returns.

Checks:

  • go build ./...: ok.
  • go test -race on providers/aws/keyspaces/..., server/aws/keyspaces/..., providers/aws/ and persist/...: ok.
  • Full go test -p 2 ./..., run in chunks: no failures.
  • golangci-lint --new-from-rev=upstream/development on the touched packages: 0 issues.

@NitinKumar004 NitinKumar004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I checked

  • The three new commits (a8e8329, d637012, 0be1e71) against each point from the earlier review.
  • Gates on 0be1e71: go build ./... ok. go vet ok on providers/aws, providers/aws/keyspaces and server/aws/keyspaces. go test -race ok on providers/aws/keyspaces and server/aws/keyspaces, and on the discovery tests in providers/aws. go test ok on providers/aws, server/aws, persist and compat/aws. gofmt is clean on the touched files, go mod tidy and go generate ./... give no diff. golangci-lint on providers/aws, providers/aws/keyspaces and server/aws/keyspaces, run without --new-from-rev, reports 20 issues on the head and 21 on the merge base. The difference is the system goconst in keyspaces.go, which this PR removes. Nothing new, and project stays under the gocyclo limit.
  • I disabled the new NotFound skip in project and reran the discovery tests. TestKeyspacesDiscoverySurvivesConcurrentDelete failed at walk 9 with NotFound: resource ".../keyspace/churn/" not found. With the skip in place it passes, so the test does guard the fix.
  • AWS CLI against cloudemu serve: 61 tags on TagResource, CreateKeyspace and UntagResource all get ValidationException: tags must contain at most 60 items. 60 tags succeed and ListTags returns 60. The checks from the first round still hold: names, missing keyspace, on-demand GetTable showing only throughputMode, ResourceNotFoundException for tags on missing resources, aws: keys, a non-cassandra ARN, the slash-less table ARN, system_schema_mcs, and ListTags after DeleteTable.
  • RGT under churn: I ran 300 GetResources calls while 6 workers created and deleted keyspaces (2400 creates and deletes in total). All 300 returned 200.
  • Snapshot: I tagged a table, took a snapshot through GET /_cloudemu/snapshot, added a slash-less entry for that table (env=old, team=data) and a .../keyspace/ghost/ entry, and posted it back. ListTags on the canonical table ARN returned env=prod, team=data, so the canonical value wins and the legacy key merges in. A newly created ghost keyspace had no tags.
  • Terraform hashicorp/aws 6.66.0 with a private provider download (keyspace, PAY_PER_REQUEST table, PROVISIONED table, all tagged): apply, then plan exit 0. I then changed the tags and the RCU, applied (3 changed in place), and plan exited 0 again. GetTable showed RCU 9 and the new tags. Destroy worked, and ListTags on the keyspace afterwards returned ResourceNotFoundException. As before, the config sets client_side_timestamps and ttl explicitly.
  • Commits and PR body have no attribution lines.

Earlier findings

  • Medium, RGT discovery fails during a keyspace delete: fixed in a8e8329. project (providers/aws/rgt_discovery.go:436) now skips an item whose tag read returns NotFound and still returns any other error. It helps every single-list walker that uses project, not just Keyspaces. Evidence: the mutation run and the churn run above.
  • Low, 60-tag bound only in the handler: fixed in 0be1e71. validateTags checks len(tags) > 60, and UntagResource checks its key list. The handler check stays for the raw wire list, which can repeat a key. That is reasonable.
  • Low, omitEmptyEnum matching ViewType at any depth: fixed by narrowing the comment. In the Keyspaces SDK, ViewType exists only on the two CDC shapes, so matching the key name is safe. I agree with not making it a general rule.
  • Low, tags under old slash-less ARNs unreachable after restore: fixed in d637012 by canonicalTagsLocked, and the PR body has a release note. It runs after the stores load, so it sees the restored keyspaces and tables. It also drops entries for ARNs that name no resource, which stops a later resource with the same name from picking up stale tags.
  • Pre-existing Snapshot vs TagResource race: fixed in d637012. Snapshot deep-copies the tag maps under the RLock, and TestSnapshotConcurrentWithTagResource is clean under -race.
  • Pre-existing GetTable ttl / clientSideTimestamps drift: still open here. It is carried by #1680 (e419c3b).
  • Pre-existing rpc-v2-cbor in hashicorp/aws 6.69: still open here. It is carried by #1680 (9ae06eb, 3d7697d).
  • Generic test file names: fixed. The files are now names_test.go, tags_test.go and tables_test.go.

High

None.

Medium

None.

Low

  • The paginated tagged-discovery walkers (Glue, Kafka, Kinesis, GuardDuty), plus the RDS and SageMaker discovery paths, still abort on a NotFound tag read. That is not a regression from this PR, and #1681 covers it, so no change is needed here.
  • RGT GetResources with --resource-type-filters cassandra returns the system keyspaces with no tags. There are four now, since system_schema_mcs was added. Real Resource Groups Tagging API would not show AWS-owned system keyspaces. The base has the same behaviour with three, so a follow-up is enough. A possible fix is to skip isSystemKeyspace names in discoverKeyspaces.
  • Because of the NotFound skip in project, a walker that builds a wrong ARN for its tag read would now lose its resources silently, where before the whole call failed. Nothing does this today, since every walker passes the ARN its list call returns. Keep it in mind if a walker later builds ARNs by hand.


for i := range items {
r, err := fn(ctx, items[i])
if cerrors.IsNotFound(err) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I disabled this skip and TestKeyspacesDiscoverySurvivesConcurrentDelete failed at walk 9 with NotFound on churn. With it in place, 300 GetResources calls against serve while 6 workers created and deleted keyspaces all returned 200. The paginated walkers (Glue, Kafka, Kinesis, GuardDuty) still return early on NotFound, but #1681 covers those.

// 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"}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, and the same on the base: RGT GetResources with --resource-type-filters cassandra lists all four system keyspaces with no tags. Real RGT doesn't show AWS-owned system keyspaces. A follow-up could skip isSystemKeyspace names in discoverKeyspaces.

@NitinKumar004
NitinKumar004 merged commit 057abf7 into stackshy:development Oct 23, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants