Replace go-simple-asyncer with aws-sdk-go-v2 SQS - #789
Open
oskarhurst wants to merge 1 commit into
Open
Conversation
CVE-2020-8911 and CVE-2020-8912 are in aws-sdk-go v1 and were never patched on that line; the fix exists only in the v2 encryption client. The dependency was not ours directly -- it arrived through go-simple-asyncer, which pins aws-sdk-go v1.35.7 -- so no version bump could clear it. async_listener now sends to SQS through aws-sdk-go-v2 via a small local dispatch package. The ASYNC_ENGINE_* env contract is unchanged, including the "local/<url>" form used to target ElasticMQ, so no infrastructure config changes. The wire format is unchanged too: a plain SQS message whose body is the JSON payload, which is what the Python workers read via boto3 receive_messages. api simply drops the dependency: handlers.DoStatistics and models.DoStatistics were the only importers and neither was ever wired to a route. Only AWSSQS and the mock are implemented. AWSSNS, AWSLAMBDA and AMQP existed upstream but are unused in this repo's configuration; they now fall through to a no-op that logs loudly rather than failing silently. Dispatch calls are now bounded by a 30s timeout, which they previously had no way to express.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes
github.com/USACE/go-simple-asyncer, and with itaws-sdk-gov1, from both Gomodules.
Why this and not a version bump
CVE-2020-8911 and CVE-2020-8912 live in
aws-sdk-gov1 and were never patched on the v1line — the fix exists only in the v2 encryption client. So no bump closes them. The
dependency was never ours directly; it arrived through
go-simple-asyncer, which pinsaws-sdk-go v1.35.7. Removing that library is the only thing that clears these.What go-simple-asyncer was doing
It is a publish-side abstraction over SQS/SNS/Lambda/AMQP behind one interface:
Cumulus used it in exactly two places, and they are very different:
async_listenerNOTIFYoncumulus_new→CallAsync→ SQS → Python worker. Every geoprocess and packager job goes through it.apihandlers.DoStatistics/models.DoStatisticswere the only importers, and neither is wired to a route.main.gonever constructs anAsyncer.Changes
async_listener— newdispatchpackage (130 lines) sending viaaws-sdk-go-v2:dispatch.New(ctx, engine, target)keeps the exactASYNC_ENGINE_*/ASYNC_ENGINE_*_TARGETcontract, including thelocal/<queue-url>form that points atElasticMQ in
docker-compose. No infrastructure configuration changes.listener from starting — but it is now cached, where the old code called
GetQueueUrlonevery message.
api— deletesapi/handlers/statistics.goandapi/models/statistics.go. This is puredead-code removal; nothing referenced them. (For what it is worth,
models.DoStatisticshad ahardcoded
cwbi-data-developbucket and a 2020 MRMS S3 key, so it looks like a long-abandonedexperiment.)
The wire format is unchanged
This is the part worth checking, because it is what would break the workers. The consumers are
Python reading raw SQS:
No Celery, no framework envelope — just a JSON body. This PR sends exactly that:
SendMessage(QueueUrl, MessageBody=<payload>). Nothing on the Python side changes.Behaviour changes, stated plainly
1. Only
AWSSQSand the mock are implemented.AWSSNS,AWSLAMBDAandAMQPexistedupstream. Every
ASYNC_ENGINE_*value in this repo isAWSSQSorMOCK, so they lookedunused — but I could not check the deployed environment configuration, which lives outside
this repo. If any environment sets one of those three, this PR silently stops dispatching
there.
To make that failure loud rather than silent, an unimplemented engine now logs:
Please confirm against the infra config before merging. If one of them is genuinely in
use, say so and I will implement it rather than drop it.
2. Dispatches are now bounded by a 30s timeout. The old interface took no
context.Context, so a hung send leaked its goroutine for the life of the process. Bounding itis the main practical payoff of moving to v2.
Testing
Built and vetted locally with Go 1.26.7, using
GOOS=linux GOARCH=amd64 CGO_ENABLED=0tomatch both Dockerfiles:
go build ./...go vet ./...go mod verifyaws-sdk-gov1async_listenerapiThe
apivet finding ismodels/download.go:50:2: struct field StatusID repeats json tag "status_id", which reproduces on unmodifiedcwbi-devand is unrelated to this change.Not integration tested. This is the dispatch path for every geoprocess and packager job,
and
MockAsyncernever exercised any AWS code, so a compile is not sufficient evidence. Beforemerging, please run the compose stack and confirm a real
NOTIFYstill produces a queue messagea worker consumes — both
geoprocess-acquirablefileandnew-download. Gate oncwbi-test-build-push-listenerandcwbi-test-build-push-api.Closes
Both issues list
apiandasync_listener, andaws-sdk-gov1 is gone from both modules,so these are fully resolved.
Closes USACE/cumulus-issue-tracking#3, closes USACE/cumulus-issue-tracking#5
github.com/aws/aws-sdk-go(CVE-2020-8911)github.com/aws/aws-sdk-go(CVE-2020-8912)Worth noting for risk context: the vulnerable package is
service/s3/s3crypto, and onlyaws-sdk-go/awswas ever linked —s3cryptoappears nowhere in either module. So thepractical exposure was already nil; this removes the finding at the source rather than
carrying a permanent risk acceptance.