[Satyam-Trivedi-ZS]

Builds on #1645 (merged); rebased onto development, so the diff is now only this PR's changes.

Summary

• Fixes the Medium item in AWS Resource Groups Tagging API: some discovery walkers still abort when a resource vanishes mid-walk #1667. A resource deleted between a discovery walker's list call and its tag read no longer fails the whole Resource Groups Tagging API GetResources call (or Resource Explorer search). The walker skips that resource. Any other error still propagates.
• Covers the four walkers named in the issue (Glue, Kafka, Kinesis, GuardDuty), plus three more that had the same gap: OpenSearch, SageMaker and Redshift.

Changes

I checked every walker in providers/aws/rgt_discovery.go, providers/aws/sagemaker_discovery.go, providers/aws/rds_discovery.go and services/resourcediscovery/walkers.go. For each tag read, I called it on an ARN that no longer exists to see the error it actually returns:

Walker	Tag read on a deleted resource	Before	Fix
Kafka	ListTagsForResource returns NotFound	aborted the walk	goes through project
Kinesis	ListTagsForResource returns NotFound	aborted the walk	goes through project
OpenSearch (not in the issue)	ListTags returns NotFound	aborted the walk	goes through project
GuardDuty	GetDetector returns BadRequestException (cerrors.InvalidArgument), not NotFound	aborted the walk	see below
Glue	GetTags returns no error, because the tag store is keyed by ARN	never aborted, but reported the deleted job	GetJob first, then project
SageMaker (not in the issue)	ListTags returns NotFound	aborted the walk	skips NotFound
Redshift (not in the issue)	DescribeTags returns NotFound	aborted the walk	skips NotFound

• Paginated walkers (Glue, Kafka, Kinesis, GuardDuty) now collect every page with a small listAll helper and then go through project, the same path as the other walkers. Each has a named row function (kafkaRow, kinesisRow, glueJobRow, guardDutyRow, openSearchRow) so the tests can drive it directly.
• GuardDuty: GetDetector reports a missing detector as BadRequestException, which is what real GuardDuty returns ("The request is rejected because the input detectorId is not owned by the current account"), so cerrors.IsNotFound doesn't match it. I didn't change the provider's error, because that would change the wire contract. When GetDetector fails, the row re-lists detectors. If the detector is gone, it reports NotFound so project skips it. Otherwise the original error propagates. The re-list only runs on the error path, and there is at most one detector per account and Region.
• Glue: GetTags can't tell a deleted job apart, so glueJobRow calls GetJob first, which returns NotFound for a deleted job.
• SageMaker and Redshift: the per-item tag-read loops skip cerrors.IsNotFound and still return any other error.
• No other walker has the gap:
  • ECS, SFN, SSM, KMS, VPC Lattice, MemoryDB, Keyspaces, Route 53 Resolver, EventBridge and CloudTrail already go through project.
  • WAFv2, Config, Network Firewall, SESv2 and ACM take tags from the list result, so there is no second read.
  • In services/resourcediscovery, the S3 and DynamoDB walkers already skip NotFound. EKS discovery already skips NotFound. The other walkers take tags from the list result.
  • The Azure and GCP discovery adapters also take tags from the list result, so nothing needs mirroring.
• Scope: discovery only. No driver interface or wire change, so docs/coverage is unchanged.

Provider Coverage

• AWS
• Azure (not affected: its discovery adapters take tags from the list result)
• GCP (not affected, same reason)
• OCI

Checklist

• All tests pass (go test ./..., run as go test -p 2 in chunks of 30 packages)
• Linter passes: no new findings in touched packages (golangci-lint run --new-from-rev=origin/fix/aws-keyspaces-parity-1512 ./providers/aws/: 0 issues)
• Every provider the change applies to implements the same behavior
• Integration tests added to cloudemu_test.go (not needed: provider-internal discovery fix)
• Unit tests added to provider test files
• Regenerated docs (no interface or wiring change, so nothing to regenerate)

Test Plan

New tests in providers/aws/rgt_discovery_test.go:

• TestDiscoverySurvivesConcurrentDelete runs each walker 2000 times next to a create/delete loop, for Kafka, Kinesis, OpenSearch, GuardDuty, Glue, SageMaker and Redshift. The long-lived tagged resource must show up on every walk. Before the fix, under -race, it failed on Kafka (walk 28), Kinesis (walk 45), OpenSearch (walk 9), GuardDuty (walk 512, InvalidArgument: ... not owned by the current account), SageMaker (walk 20) and Redshift (walk 7). Glue passed, as expected. After the fix, all seven pass, also with -race -count=5.
• TestTaggedWalkersSkipResourceDeletedAfterList is deterministic. It lists, deletes one resource, then projects the stale list through each walker's row function (Kafka, Kinesis, OpenSearch, Glue, GuardDuty). Only the live, tagged resource comes back. The GuardDuty case also asserts that GetDetector on the deleted detector returns a non-NotFound error, which is the reason that walker needs the re-list.
• TestSageMakerDiscoverySkipsVanishedResource and TestRedshiftDiscoverySkipsVanishedCluster wrap the mock so one tag read fails. NotFound is skipped and another error propagates. Both NotFound cases fail without the fix.

Commands:

• go build ./...: ok.
• go test -race ./providers/aws/ ./services/resourcediscovery/... ./server/aws/... (all 76 server/aws packages, including resourcegroupstaggingapi and resourceexplorer2): ok.
• Full go test -p 2 -count=1 over all 657 packages in chunks of 30: no failures.
• golangci-lint run --allow-parallel-runners --new-from-rev=origin/fix/aws-keyspaces-parity-1512 ./providers/aws/: 0 issues, so no new findings in touched packages.

Against cloudemu serve -providers aws -aws-port 4613 (AWS CLI 1.44.87 / botocore 1.42.97, always with --endpoint-url http://127.0.0.1:4613):

• With the AWS CLI, I created a tagged (env=prod) Kinesis stream, MSK cluster, Glue job and OpenSearch domain, and get-resources --tag-filters Key=env,Values=prod listed all four.
• Then, for 25 seconds, I ran these in parallel against the same server:
  • 9 botocore workers creating and deleting Kinesis streams, MSK clusters, Glue jobs, OpenSearch domains and GuardDuty detectors.
  • 4 botocore workers calling GetResources in a loop. Each call also checked that the four long-lived resources were in the result.
  • A shell loop of 30 aws resourcegroupstaggingapi get-resources calls.
• This branch: 13,778 GetResources calls, 0 failed. 14,784 create/delete rounds. CLI loop 30 ok, 0 failed.
• Base (0be1e711), same run: 7 of 12,286 calls failed with InvalidParameterException: walkGeneric: NotFound: ..., for Kinesis streams, MSK clusters and OpenSearch domains. The 30-call CLI loop alone was too slow to hit the window. The GuardDuty case didn't trigger over HTTP in this run, so the in-process tests are the evidence for it.

Known limits:

• Discovery is still not a point-in-time snapshot. A resource deleted after its tag read is still reported, and one created after the list call is not. This is the same as the other walkers.
• The GuardDuty walker tells a deleted detector apart by re-listing, so an error for a detector that still exists propagates as before.
• Separate from discovery: Glue DeleteJob doesn't clear the job's entry in the ARN-keyed tag store, so a re-created job with the same name inherits the old tags. I haven't changed that here, because it is a Glue provider bug, not a discovery one.

Related Issues

Part of #1667

[NitinKumar004]

What I checked

• Scope: only the three commits after 0be1e711 (2a4e543, 6f7903e, 9a6db07). The Keyspaces commits belong to #1645.
• Walker coverage. I went through all 20 collectors in taggedDiscovery, plus sagemakerDiscovery, rdsDiscovery (RDS and Redshift), eksDiscovery and the services/resourcediscovery walkers. Every one that reads tags with a second call after its list call now skips a NotFound: Kafka, Kinesis, OpenSearch, Glue and GuardDuty through project, SageMaker and Redshift in their own loops. ECS, SFN, SSM, KMS, VPC Lattice, MemoryDB, Keyspaces, Resolver, EventBridge and CloudTrail already went through project. Network Firewall, SESv2, ACM, WAFv2 and Config take tags from the list result. S3, DynamoDB and EKS already skipped NotFound. On Azure, applyStorageAttrs and applyDatabaseAttrs make a second read, but BucketAttributes and TableAttributes never return an error, so nothing is missing there.
• Pagination. listAll stops on an empty token. The Kinesis adapter returns "" when HasMoreStreams is false, so the old !HasMore || NextToken == "" exit condition still holds.
• Only NotFound is skipped. Mutations:
  • Remove the skip in project: the Kafka, Kinesis, OpenSearch, GuardDuty and Glue churn cases fail, and so do the Keyspaces and project tests.
  • Make project skip every error: TestProjectSkipsVanishedResource fails.
  • Make SageMaker skip every error: the other_error case fails.
  • Remove the Redshift skip: the churn case and the not_found case fail.
  • Remove the GuardDuty re-list: the churn and deterministic GuardDuty cases fail.
  • Remove Glue's GetJob: the deterministic Glue case fails.
• GuardDuty. GetDetector returns BadRequest for a missing detector, which matches real GuardDuty. If the re-list itself fails, the original error is returned. NotFound is reported only when the detector is no longer listed. That logic is right.
• Gates on 9a6db07:
  • go build ./... ok. go vet ./providers/aws/ ok. gofmt is clean on the four touched files. go mod tidy and go generate ./... give no diff.
  • go test -race -count=3 on the new tests: ok. go test ok on providers/aws, server/aws/resourcegroupstaggingapi, server/aws/resourceexplorer2, services/resourcediscovery and the RGT tests in compat/aws.
  • golangci-lint 2.14.0 on providers/aws, run without --new-from-rev, reports 20 issues on the head and the same 20 on the merge base (0be1e71). Nothing is new. The only finding in a touched file is the existing gocyclo 11 on DiscoverDatabases, which this PR doesn't change.
• cloudemu serve, using the AWS CLI and SDK v2:
  • I created tagged (env=prod) Kinesis, MSK, Glue, OpenSearch, GuardDuty, SageMaker model and Redshift resources. get-resources --tag-filters env=prod lists all 7.
  • For 20 seconds, 12 SDK workers created and deleted Kinesis, MSK, Glue, OpenSearch, SageMaker and Redshift resources, while 4 workers called GetResources and checked that all 7 long-lived resources were in each result.
  • Head: 67,688 calls with 0 failures (123,600 churn rounds). A second run gave 63,715 calls with 0 failures. A parallel loop of 25 aws resourcegroupstaggingapi get-resources calls got 25/25 results with all 7 resources.
  • Base (0be1e71), same run: 1,768 of 57,010 calls failed, with walkGeneric: NotFound errors for Kinesis and SageMaker and walkRelationalDB: NotFound for Redshift.
• Terraform hashicorp/aws 6.66.0, private provider download (aws_kinesis_stream, aws_glue_job and aws_guardduty_detector, all tagged): apply, then plan exit 0. Then I changed the tags to env=staging, applied (3 changed in place) and plan exit 0. GetResources with env=staging lists all three. Destroy works, and GetResources then shows none of them.
• Commits and PR body have no attribution lines.

High

None.

Medium

None.

Low

• Nothing tests that a GuardDuty error propagates. If guardDutyRow turns every GetDetector error into NotFound, so a failing live detector is silently dropped, the whole suite still passes. Add a case that wraps GuardDuty so GetDetector fails with a non-NotFound error while the detector is still listed, and assert that discoverGuardDuty returns that error. TestSageMakerDiscoverySkipsVanishedResource already does this for SageMaker.
• A walker with a wrong ARN is only partly guarded. With the NotFound skip, a walker whose tag read misses its own resource now drops it silently instead of failing the call. The new tests catch this for the seven walkers in this PR: if I change the OpenSearch ARN to domain/x<name>, both new tests fail. compat/aws/rgt_full_coverage_compat_test.go covers KMS, SSM, ECS, SFN, Glue, Keyspaces, GuardDuty, ACM, SageMaker and Redshift. Nothing covers the other tag-read walkers: VPC Lattice, MemoryDB, Route 53 Resolver, EventBridge and CloudTrail. If I change VPC Lattice's tag read to s.ARN+"x", every test in providers/aws still passes. All five pass the ARN their list call returns, and I checked that each one shows up with its tags today, so nothing is broken now. A single table test that creates one tagged resource per collector and checks that each one comes back from taggedDiscovery.DiscoverResources would close the gap for all 20 collectors and for any new one.
• Pre-existing Glue bug, worth a ticket (the PR body already mentions part of it). Glue's tag store accepts any ARN, and DeleteJob doesn't clear it. In Terraform with skip_requesting_account_id = true, the provider reads and writes tags under an ARN with no account ID. Those tags survive a destroy, and the next apply shows a tag diff against the old value. Discovery itself is fine, because the walker uses the account-qualified ARN.
• Pre-existing, outside this PR's scope: a walker error reaches the wire as HTTP 400 InvalidParameterException: walkGeneric: ..., as seen on the base run. For an internal failure, real Resource Groups Tagging API returns InternalServiceException (500).

Further notes

• providers/aws/rgt_discovery.go:47: With project skipping NotFound, a collector whose tag read misses its own resource (a wrong ARN, for example) now drops rows silently. The new tests guard the seven walkers this PR touches, but nothing guards VPC Lattice, MemoryDB, Resolver, EventBridge or CloudTrail. If I change the VPC Lattice tag read to s.ARN+"x", every test in providers/aws still passes. A table test that creates one tagged resource per collector and checks it comes back from DiscoverResources would cover all of them, and any new one.

[NitinKumar004]

The re-list logic looks right, but no test covers the branch where the detector is still listed. If every GetDetector error becomes NotFound here, the suite still passes. Could you add a case that wraps GuardDuty so GetDetector returns a non-NotFound error for a detector that still exists, and assert that discoverGuardDuty returns that error?