BrighterCommand / BrighterCommand/Brighter
S3 test buckets leak: S3LuggageUploadTests never deletes its bucket, and no sweep covers brightertestbucket-*
- Dominant language
- C#
- Stars
- 2.5k
- Forks
- 296
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 21
Description
## Summary
The S3 luggage-store tests create real buckets named `brightertestbucket-`. One of them never deletes its bucket, and `clean_failed_tests_aws_assets.sh` has no S3 handling at all — it sweeps SQS, SNS and EventBridge Scheduler only. Nothing, anywhere, reclaims these.
This is the same shape as the SNS/SQS leak fixed in #4298, on a resource type that fix did not cover.
## The definite leak
`tests/Paramore.Brighter.AWS.Tests/Transformers/When_uploading_luggage_to_S3.cs` (and its `AWS.V4.Tests` mirror):
```csharp
_bucketName = $"brightertestbucket-{Guid.NewGuid()}"; // ctor
...
await luggageStore.EnsureStoreExistsAsync(); // creates the bucket
...
await luggageStore.DeleteAsync(claim); // deletes the claim, not the bucket
```
The class has no `Dispose`/`DisposeAsync` and never calls `DeleteBucketAsync`. Every run of `aws-ci` leaves two buckets behind — one per project — pass or fail.
## The conditional leaks
These clean up on the happy path only:
- `When_wrapping_a_large_message.cs` and `When_unwrapping_a_large_message.cs` delete the bucket in `DisposeAsync`, so a throw in the constructor skips it.
- `When_validating_a_luggage_store_exists.cs` deletes inline as the last statement of the test, so any earlier failure skips it.
(`When_creating_luggagestore_missing_parameters.cs` calls `EnsureStoreExistsAsync` on a guid-named bucket, but expects a `ConfigurationException` that should be thrown before S3 is reached. I have not confirmed whether a bucket is ever created there — worth checking rather than assuming either way.)
## Suggested fix
Two parts, matching what #4298 did for topics and queues:
1. Give the fixtures a teardown that deletes the bucket by *name* — including `S3LuggageUploadTests`, which has none — so a test that fails while standing infrastructure up still cleans up.
2. Add an S3 pass to `clean_failed_tests_aws_assets.sh` matching `^brightertestbucket-` as a backstop for the runs that die before they can. Note `ListBuckets` returns `CreationDate`, so unlike SNS topics these can be age-guarded directly with `CLEANUP_MIN_AGE_SECONDS` — no first-seen stamp needed. Buckets must be emptied before they can be deleted.
The existing `aws-cleanup.yml` credentials would need `s3:ListAllMyBuckets`, `s3:ListBucket`, `s3:DeleteObject` and `s3:DeleteBucket`.
## Context
Raised in review of #4298 (https://github.com/BrighterCommand/Brighter/pull/4298#issuecomment-5573515915) and left out of that PR as out of scope.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01DEAvZsgdBRMKYJFix4KGj7
Contributor guide
Assessment
This issue has not been assessed yet.