Repository navigation
Conversation
aws-sdk-go-v2/service/opensearch v1.85.1 (LeanerCloud#88) serializes requests through smithy-go's schema-based engine instead of generated per-operation serializers. Every existing test mocks provider.ServiceClient, so nothing checked the bytes PurchaseReservedInstanceOffering puts on the wire. Drive a confirmed cudly_aws_opensearch_ri_purchase call through the real provider client (awsprovider.NewOpenSearchClient) pointed at an httptest server via aws.Config.BaseEndpoint, with fixture responses and a transport that refuses any non-loopback host. Assert the route, the content type, the SigV4 scope (es, purchase region) and the JSON body: exactly InstanceCount, ReservationName and ReservedInstanceOfferingId, with InstanceCount an unquoted JSON number. Closes LeanerCloud#89 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
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.
Summary
Issue #89: after #88 bumped
aws-sdk-go-v2/service/opensearchfrom v1.52.3 to v1.85.1 (smithy-go v1.24.2 to v1.28.5), the OpenSearch client serializes requests through smithy-go's schema-based engine (service/opensearch/schemas) instead of generated per-operation serializers. Every existing test in this repo mocksprovider.ServiceClient, so nothing checked whatPurchaseReservedInstanceOfferingactually puts on the wire.This is a test-only change. No production code changes.
What the new test does
tools/aws_opensearch_wire_test.goaddsTestOpenSearchRIPurchaseWireEncoding:cudly_aws_opensearch_ri_purchasecall (dry_run=false,confirm=true,count=3) through the tool'shandle,ExecutePurchase(spend caps, USD cap, budget, idempotency token) and the real provider clientawsprovider.NewOpenSearchClient, which is the constructor the AWS provider'sGetServiceClientuses. That client goes throughpurchasecfg.NewConfigand the SDK's serializer exactly as in production.httptestserver throughaws.Config.BaseEndpoint, with static fixture credentials. The server answersDescribeReservedInstanceOfferings,DescribeReservedInstances,PurchaseReservedInstanceOffering, STSGetCallerIdentityandAddTagswith fixtures, and fails the test on any other request.loopbackOnlyTransportrefuses any request whose host is not the fake server, so a regression in endpoint resolution fails the test instead of reaching AWS. I probed this: withBaseEndpointremoved, the test fails withtest transport refused non-loopback request to es.us-east-1.amazonaws.com.POSTto/2021-01-01/opensearch/purchaseReservedInstanceOfferingContent-Typeisapplication/json/us-east-1/es/aws4_requestInstanceCount,ReservationNameandReservedInstanceOfferingIdInstanceCountis the unquoted JSON number3, checked on the raw JSON so a quoted"3"failsReservedInstanceOfferingIdis the fixture offeringReservationNameequals theopensearch-id-…name derived from the MCP idempotency token, recomputed in the testWhy here and not in cloud-commitments-go
The issue says this "may belong in cloud-commitments-go's opensearch provider". I put it here because this module's
go.modis the one that pinsservice/opensearch v1.85.1, and MVS means that is the serializer thecudly-mcpbinary actually ships. A test in cloud-commitments-go would pin whatever version that module resolves, which is a separate bump. The test is also end to end from the MCP tool arguments. A matching provider-level test in cloud-commitments-go would still be worthwhile, but I left it out of scope.Verification
All evidence is local unit/fixture evidence on macOS. There are no real AWS calls, and every response is a hand-written fixture shaped like the documented restJson1/awsQuery responses. Real-cloud coverage gap: this pins what the client sends, not what AWS accepts. No real
PurchaseReservedInstanceOfferingwas made or could be made.Tests ran with
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/nullbecause this machine's default AWS profile is invalid. The test itself uses static fixture credentials and needs no AWS config.Fail-before / pass-after (mutation probes)
There was no bug to fix, so there is no failing state on main to show. Instead I ran the test against mutated copies of the SDK and the provider, each swapped in through a throwaway
go.workreplace. None of these copies are in the diff.PurchaseReservedInstanceOfferingRequest.AddMember("InstanceCount", …)renamed to"instanceCount"inservice/opensearch@v1.85.1/schemaselements differ … {"instanceCount":3,…}s.WriteInt32(…InstanceCount…)changed tos.WriteString(…, fmt.Sprint(…))expected: "3" actual: "\"3\""(InstanceCount must be an unquoted integer)ReservationNamedropped from the purchase inputmissing required field … ReservationNameInstanceCount: aws.Int32(instanceCount - 1)expected: "3" actual: "2"service/opensearchreplaced with v1.52.3, the pre-#88 generated serializer.go list -depsconfirmsservice/opensearch/schemasis not linkedservice/opensearchv1.85.1, schema-based serializerThe test passes on both the old and the new serializer and fails on each mutation. So the wire encoding of this request is unchanged across #88, and a future drift will be caught.
Notes
t.Parallel(). It uses its ownhttptestserver and the package's existingTestMaindefaults (real-purchase gate on, generous caps, temp audit log). The only shared state it touches is the process budget ledger, and it reserves 3000 USD of the TestMain default. The budget tests that inspect or reset the ledger are non-parallel (t.Setenv), so they never overlap with it.fixture-account, so the fixture can never look like a 12-digit account ID. No.gitallowedchange was needed.AddTagsandDescribeReservedInstance*are served but not wire-asserted.Closes #89
Maintainer: please mirror the issue's triage labels onto this PR (I don't have label permissions on this repo):
priority/p2,severity/medium,type/chore,urgency/this-quarter,impact/many,effort/s. The issue does not carrytriaged.🤖 Generated with Claude Code