From abe2ea9e8bff9c99991e11cff04fe6b20573e2b6 Mon Sep 17 00:00:00 2001 From: Scott Friedman <3011922+scttfrdmn@users.noreply.github.com> Date: Sat, 26 Sep 2026 08:40:59 -0700 Subject: [PATCH] fix(#856): the internal dispatch paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every earlier tier of #856 migrated a generator. Counting generators answers the wrong question: a plugin whose minters are all migrated still publishes random identifiers if it is called with nothing to derive from. Substrate has four internal dispatch sites — a RequestContext built in-process rather than parsed off the wire — and the CloudFormation deployer is one of them, so every identifier of every resource in every CloudFormation stack was undirected randomness while the draw-site count read two. dispatch now seeds a mint from its request id, and WithDeployerMint derives that request id from the request that asked for the deployment. The two fix different failures: the first makes a replayed internal event reproduce, the second makes a replayed CreateStack — which re-runs the whole deployment — reproduce too. Reverting either alone gives 7 and 6 differences respectively against the new replay test, each reported as a successful 200. Falling out of the same seam: a StackDriftDetectionId is no longer substrate's internal req-… string, deploySSMAssociation is no longer the deployer's last randomHex caller, and an API Gateway proxy integration carries a derived mint into the Lambda invocation it dispatches. The Lambda ESM poller's two dispatches stay mintless, with an in-code note, because a wall-clock ticker records nothing for a replay to derive from (#1292). Refs #856. --- CHANGELOG.md | 24 ++++++ docs/services.md | 46 +++++++++- docs/testing-guide.md | 7 ++ emulator/apigateway_plugin.go | 25 +++++- emulator/cfn_deployer.go | 81 +++++++++++++++++- emulator/client.go | 10 ++- emulator/cloudformation_plugin.go | 3 +- emulator/ec2_types.go | 4 +- emulator/export_test.go | 11 +++ emulator/ids.go | 12 ++- emulator/ids_test.go | 134 ++++++++++++++++++++++++++++++ emulator/lambda_plugin.go | 10 +++ 12 files changed, 351 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e30fefb..05571aa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -414,6 +414,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 observed from the example rather than required; it is documented beside the `backup-plan` vs. `plan` segment gap (#1181) rather than folded into the derivation, since it is a rendering question and not one about where the bytes come from. +- **Every identifier of every resource in every CloudFormation stack was random** (#856). This is the + part of #856 that counting draw sites could not find: `StackDeployer` holds no `crypto/rand` call of + its own, it turns each resource into an *internal* EC2, IAM or S3 request — and those requests were + built with a fresh request ID and no mint, so every plugin they reached took `IDMint`'s seedless + fallback however carefully its own minters had been migrated. A replayed `DescribeStackResources` + reported four different `PhysicalResourceId` values in a **200**, and `CreateStack`'s + `state_hash_after` never matched because that hash covers all of them. Two changes were needed and + they fix different failures: an internal request now carries `IDs: NewIDMint(requestID)`, which makes + a replayed internal event reproduce, and the internal request ID is itself derived from the stack + request's ID through the new `WithDeployerMint` option, which makes a replayed `CreateStack` — which + re-runs the whole deployment — reproduce too. Reverting either half alone was measured: seven + differences without the mint, six without the derived request ID. The sequence is reproducible because + a deploy already orders resources by type priority and then logical ID, never by map iteration. The + same dispatch fix reaches an API Gateway proxy integration, which invokes a Lambda through an internal + request. A Lambda event-source-mapping poll is deliberately left on the fallback and now says so in + the code: its dispatches come from a wall-clock ticker and are recorded nowhere, so there is no + recorded ID to derive from (#1292). +- **A `StackDriftDetectionId` was substrate's internal request ID, verbatim** (#856). `DetectStackDrift` + answered the `req-…` string substrate stamps on a request, a shape no AWS reference describes, in a + response element a consumer hands straight back to `DescribeStackDriftDetectionStatus`. + `StackDriftDetectionId` publishes a **maximum length of 36** and no pattern, and the page's own sample + is `2f2b2d60-df86-11e7-bea1-500c2example` — so it is now the UUID shape within that bound, derived from + the request ID. A recorded detection replays with the ID it recorded, where before the poll that + followed it resolved to nothing. - **A stream recorded under a seed replays under the same seed** (#1140). Every seedable outcome in substrate is written through a control-plane endpoint, and only the AWS path recorded anything — so a seed never entered the event stream. A replay opens by resetting the whole `StateManager`, and a diff --git a/docs/services.md b/docs/services.md index 5842d4c9..e4d2bdb0 100644 --- a/docs/services.md +++ b/docs/services.md @@ -2318,6 +2318,36 @@ a third — so the counter is what keeps a plan from being its own version. AWS' shows the UUID in *upper* case where substrate renders lower; that is an observed difference the model does not require, tracked with the [resource segment](#arn-shapes) rather than with the derivation. +**A request that dispatches another request has to hand the mint along with it.** Counting draw +sites finds the places that call `crypto/rand`; it cannot find a plugin that derives its +identifiers correctly and is *called* with nothing to derive from. CloudFormation's deployer is +that case: deploying a stack turns each resource into an internal EC2, IAM or S3 request, and those +requests were built with a fresh request ID and no mint, so every plugin they reached took the +seedless fallback. The consequence is larger than the count suggested — **every identifier of every +resource in every CloudFormation stack** was undirected randomness, while the deployer itself +contains no draw site at all. A stack's `vpc-…`, its subnet, its security group and its role were +all unreproducible, and a `ValidateState` replay of the outer `CreateStack` reported a +`state_hash_after` mismatch because that hash covers all of them. + +Two separate things had to change, and each fixes a different failure. An internal request carries a +mint seeded from its own ID, which is what makes a replayed *internal* event reproduce. And the +internal ID is itself derived from the stack request's ID, which is what makes a replayed +`CreateStack` reproduce — replaying it re-runs the whole deployment, so the internal IDs must come +out the same way twice. The sequence is reproducible because a deploy orders resources by type +priority and then by logical ID, never by map iteration. Both halves are load-bearing and were +measured that way: dropping the mint produces seven differences in a four-resource stack, dropping +the derived request ID produces six. Notably the replay reported every event as a **success** — a +200 describing four resources that were not the recorded ones, which is the silent-wrong-answer +class rather than a visible failure. + +Removing substrate's internal request ID from that path also removed it from an AWS response. +`DetectStackDrift` answered a `StackDriftDetectionId` that was literally substrate's `req-…` string, +which no AWS reference describes; it is now UUID-shaped within the published maximum length of 36. +The same dispatch fix applies to an API Gateway proxy integration, which invokes a Lambda through an +internal request. A Lambda event-source-mapping poll is deliberately left random: its dispatches come +from a wall-clock ticker and are recorded nowhere, so there is no recorded ID to derive from and a +seed there would be no more reproducible than the fallback ([#1292](https://github.com/scttfrdmn/substrate/issues/1292)). + An ECR image digest is minted rather than computed from the manifest, so it is reproducible across a replay but is not the SHA-256 of the image it names, and two pushes of identical manifest bytes store two images where AWS stores one. [#1283](https://github.com/scttfrdmn/substrate/issues/1283) @@ -2351,7 +2381,7 @@ the most recent one deletes. A replay of either call reproduces the handle that | ExecuteChangeSet | Applies the change and its tags, and consumes the set | | ListChangeSets | Pending change sets for a stack | | DeleteChangeSet | Discards a pending set; deleting an absent set succeeds | -| DetectStackDrift | Returns a `StackDriftDetectionId` | +| DetectStackDrift | Returns a `StackDriftDetectionId` — UUID-shaped within the published maximum length of 36, [derived from the request ID](#an-identifier-a-replay-mints-is-the-one-it-recorded) (#856). It used to be substrate's internal `req-…` string, a shape no AWS reference describes | | DescribeStackDriftDetectionStatus | Resolves that ID to a completed detection | | DescribeStackResourceDrifts | Per-resource drift; honours `StackResourceDriftStatusFilters.member.N` | | ListExports | Every exported output value in the caller's account and Region, in one page | @@ -3489,6 +3519,14 @@ This was substrate's first derived identifier and is the one the general rule wa modelled on; see [An identifier a replay mints is the one it recorded](#an-identifier-a-replay-mints-is-the-one-it-recorded). +The **physical** IDs of the resources a stack deploys are derived too, and were +not until #856's dispatch fix: the deployer turns each resource into an internal +request, and those requests carried no mint, so a replayed stack reported +different `PhysicalResourceId` values and a different `state_hash_after` for +`CreateStack`. The internal request IDs now derive from the stack request's own +ID, in the order a deploy already uses — type priority, then logical ID — so a +replayed `CreateStack` re-runs the deployment and arrives at the same IDs. + ### Cost CloudFormation operations are free. The resources a template deploys are costed @@ -12951,7 +12989,7 @@ Secrets Manager API calls: $0.05 per 10,000 API calls. | RemoveTagsFromResource | Removes only the named keys | | ListTagsForResource | Reports `TagList` sorted by key; an empty list, never `null` | | LabelParameterVersion | Accepted; always reports `ParameterVersion: 1` | -| SendCommand | Run Command; records the intent — substrate does not execute the command. The `CommandId` is the published fixed length of 36, [derived from the request ID](#derived-identifiers) ([#856](https://github.com/scttfrdmn/substrate/issues/856)) | +| SendCommand | Run Command; records the intent — substrate does not execute the command. The `CommandId` is the published fixed length of 36, [derived from the request ID](#an-identifier-a-replay-mints-is-the-one-it-recorded) ([#856](https://github.com/scttfrdmn/substrate/issues/856)) | | GetCommandInvocation | | | DescribeInstanceInformation | | @@ -19533,7 +19571,7 @@ The published example differs in one more way the table does not show: AWS's sam `arn:aws:backup:us-east-1:123456789012:plan:8F81F553-3A74-4A3F-B93D-B3360DC80C50`, an **uppercase** UUID, where Substrate renders lowercase. `BackupPlanId` publishes no pattern and no length, so this is observed from the example rather than required by the model, and it is a rendering question rather than -a derivation one — the plan ID is [derived from the request ID](#derived-identifiers) either way. +a derivation one — the plan ID is [derived from the request ID](#an-identifier-a-replay-mints-is-the-one-it-recorded) either way. ### Cost @@ -19567,7 +19605,7 @@ running. |-----------|-------| | InvokeModel | `POST /model/{modelId}/invoke`. Answers a [seeded response body](#seeding-a-model-response) verbatim, or a canned Claude Messages body naming the requested model. Nothing but the model ID is read — not the body, not `accept` or `contentType`, and [not the guardrail headers](#invokemodel-reads-nothing-but-the-model-id) | | ApplyGuardrail | `POST /guardrail/{guardrailIdentifier}/version/{guardrailVersion}/apply`. [`NONE` or `GUARDRAIL_INTERVENED`, decided by a blocklist](#how-a-guardrail-decides); the version is discarded | -| CreateModelInvocationJob | `POST /model-invocation-job`. Answers `{"jobArn"}`, exactly the published shape, and records the job as `Submitted` — the first state the page documents, so a batch job is deliberately not terminal at birth. None of the five members marked `Required: Yes` is checked. The job ID is twelve characters of `[a-z0-9]`, the published `jobArn` pattern, [derived from the request ID](#derived-identifiers) ([#856](https://github.com/scttfrdmn/substrate/issues/856)) | +| CreateModelInvocationJob | `POST /model-invocation-job`. Answers `{"jobArn"}`, exactly the published shape, and records the job as `Submitted` — the first state the page documents, so a batch job is deliberately not terminal at birth. None of the five members marked `Required: Yes` is checked. The job ID is twelve characters of `[a-z0-9]`, the published `jobArn` pattern, [derived from the request ID](#an-identifier-a-replay-mints-is-the-one-it-recorded) ([#856](https://github.com/scttfrdmn/substrate/issues/856)) | | GetModelInvocationJob | `GET /model-invocation-job/{jobIdentifier}`. Returns the stored record whole, so `accountID` and `region` reach the wire ([#756](https://github.com/scttfrdmn/substrate/issues/756)), and reports a [seeded status](#seeding-a-batch-job-status) if one is set | | ListModelInvocationJobs | `GET /model-invocation-jobs`. `invocationJobSummaries` of five members each; the seeded status is applied here too, so a poll on either operation agrees. Every published query filter is ignored and no `nextToken` is emitted | | StopModelInvocationJob | `POST /model-invocation-job/{jobIdentifier}/stop`. [Stops a job in any state and skips `Stopping`](#stopping-a-batch-job-is-immediate) | diff --git a/docs/testing-guide.md b/docs/testing-guide.md index fd3c2dc7..d54079cf 100644 --- a/docs/testing-guide.md +++ b/docs/testing-guide.md @@ -335,6 +335,13 @@ makes `Differences` empty — and `StateValid` true, with `WithRecordedStateHash all; before #856 every such stream diverged on its first create, which is why the #1140 tests above are built on caller-chosen bucket and key names instead. +**That now includes a CloudFormation stack.** A deploy turns each resource into an internal +request, and those requests used to carry no mint, so every physical resource ID in every +stack was random even though the deployer holds no draw site of its own — a replayed +`DescribeStackResources` reported different IDs, in a 200, and `CreateStack`'s +`state_hash_after` never matched. A stack of a handful of resources now replays with zero +differences and `StateValid` true. + Two caveats. **Not every service's identifiers are derived yet.** EC2, IAM, STS, SQS, SNS, Lambda, EFS, FSx, Transfer, ECS, Step Functions, EventBridge, CloudWatch Logs, CloudFront, Service Quotas, API Gateway (v1 and v2), AppSync, Batch, EMR Serverless, ECR, ELB, Route 53, diff --git a/emulator/apigateway_plugin.go b/emulator/apigateway_plugin.go index fccff38f..00a8e9c0 100644 --- a/emulator/apigateway_plugin.go +++ b/emulator/apigateway_plugin.go @@ -1628,11 +1628,18 @@ func (p *APIGatewayProxyPlugin) HandleRequest(reqCtx *RequestContext, req *AWSRe Headers: map[string]string{"Content-Type": "application/json"}, Body: eventJSON, } + // Derived from the request that reached the integration, so the invocation this + // dispatches mints what it minted in the recording (#856). A proxy integration is + // one of the tree's internal dispatch paths: the context is built here rather than + // parsed off the wire, so nothing gave it a mint and every identifier the invoked + // function's plugin published fell back to crypto/rand. + invokeRequestID := apigwInternalRequestID(reqCtx.IDs) invokeCtx := &RequestContext{ - RequestID: generateRequestID(), + RequestID: invokeRequestID, AccountID: reqCtx.AccountID, Region: reqCtx.Region, Timestamp: reqCtx.Timestamp, + IDs: NewIDMint(invokeRequestID), Metadata: make(map[string]interface{}), } invokeResp, invokeErr := p.registry.RouteRequest(invokeCtx, invokeReq) @@ -1816,6 +1823,22 @@ func buildV1ProxyEvent(req *AWSRequest, apiID, stage, resourcePath string) ([]by return json.Marshal(event) } +// apigwInternalRequestID returns the request id for the Lambda invocation a proxy +// integration dispatches, derived from m when it derives. +// +// Separate from generateRequestID because the value seeds the invocation's own mint: +// a wall-clock id makes every identifier the invoked function's plugin publishes +// unreproducible, which is the gap #856's tier 8 closes on the two internal dispatch +// sites that have a recorded request behind them — this one and the CloudFormation +// deployer's. A mint that does not derive — an in-process caller with no request behind +// it — falls back to the wall clock, which is what this always did. +func apigwInternalRequestID(m *IDMint) string { + if !m.Derived() { + return generateRequestID() + } + return "req-apigw-" + m.Hex(12) +} + // buildV2ProxyEvent constructs a v2 (HTTP API) proxy event JSON payload. func buildV2ProxyEvent(req *AWSRequest, apiID, stage, resourcePath string) ([]byte, error) { rawQS := "" diff --git a/emulator/cfn_deployer.go b/emulator/cfn_deployer.go index 0bc6fb9e..8e03336a 100644 --- a/emulator/cfn_deployer.go +++ b/emulator/cfn_deployer.go @@ -1005,6 +1005,12 @@ type StackDeployer struct { // nil to authorize nothing. Taken from PluginConfig.Options["auth_controller"] // the same way event_store and cost_controller are. auth *AuthController + + // mint derives the request id of every request this deployment dispatches, and + // so — transitively — every identifier of every resource in the stack. Nil + // means the deployment's internal request ids are drawn from the wall clock, + // which is what they all were before #856's tier 8. See WithDeployerMint. + mint *IDMint } // StackDeployerOption configures optional behavior of a [StackDeployer]. @@ -1030,6 +1036,34 @@ func WithDeployerIdentity(accountID, region string) StackDeployerOption { } } +// WithDeployerMint derives this deployment's internal request ids from m, which is +// the mint of the request that asked for the deployment. +// +// A stack's resources are created by requests substrate builds in this package and +// dispatches through the registry, and each of those requests mints its resource's +// identifier from its own request id. Those ids came from generateRequestID, which +// is the wall clock, so **every identifier in every CloudFormation stack was +// undirected randomness** — which is the part of #856 the draw-site count never +// showed, because the deployer contains no `rand.Read` call of its own. +// +// Deriving them fixes two different failures at once. A replayed *internal* event +// reproduces its recording because the recorded request id is the seed, which is +// true of any recorded event (see replayEvent). And a replayed outer `CreateStack` +// — which re-runs the whole deployment, since the create is itself a recorded +// event — reproduces it too, because the re-run derives the same sequence of +// internal ids from the same outer request. The second is why the option exists: +// without it the two halves of a replayed CloudFormation stream disagreed with each +// other, not merely with the recording. +// +// The sequence is reproducible because [StackDeployer.DeployWithOptions] sorts its +// resources by type priority and then by logical id before dispatching any of them. +// A deployment is single-goroutine, so the mint's ordinal advances in that order. +func WithDeployerMint(m *IDMint) StackDeployerOption { + return func(d *StackDeployer) { + d.mint = m + } +} + // cfnAttribution is who a stack's resource calls are made as: a service role if // the stack has one, otherwise the identity that created it. // @@ -2023,7 +2057,19 @@ func (d *StackDeployer) StartStackDriftDetection(ctx context.Context, stackName return "", cfnErrf(ErrCFNStackNotFound, "cfn StartStackDriftDetection: stack %q not found", stackName) } - detectionID := generateRequestID() + // Minted from the deployment's mint, so a replayed DetectStackDrift returns the id + // the recording returned and the recorded DescribeStackDriftDetectionStatus that + // names it is answered rather than refused (#856). It was generateRequestID, which + // is the wall clock — and which also put substrate's internal `req-…` request-id + // format into an AWS response element. + // + // `API_DetectStackDrift` publishes `StackDriftDetectionId` as a String with a + // **maximum length of 36** and no pattern, and its sample response is + // `2f2b2d60-df86-11e7-bea1-500c2example` — a UUID shape. 36 is exactly what + // [IDMint.HexUUID] renders, so the published bound is met at its limit and the value + // looks like one CloudFormation would issue. Reading AWS's example is the provenance + // #671 allows; inventing a bound from a sibling operation is not. + detectionID := d.mint.HexUUID() // Persist the in-progress record before running detection. inProgress := &CFNDriftDetectionStatus{ StackDriftDetectionID: detectionID, @@ -4255,6 +4301,12 @@ func (d *StackDeployer) deploySSMParameter( } // deploySSMAssociation is a stub for SSM::Association resources. +// +// The association id is minted from the deployment's own mint rather than dispatched +// for, because there is no request to dispatch: SSM's plugin models Run Command and +// Parameter Store, not State Manager, so this resource has no service behind it and +// the physical id is the whole of what a template can observe. It was the last +// [randomHex] caller in the deployer (#856). func (d *StackDeployer) deploySSMAssociation( _ context.Context, logicalID string, @@ -4262,7 +4314,7 @@ func (d *StackDeployer) deploySSMAssociation( _ string, _ *cfnContext, ) (DeployedResource, float64, error) { - assocID := randomHex(16) + assocID := d.mint.Hex(16) return DeployedResource{ LogicalID: logicalID, Type: "AWS::SSM::Association", @@ -4582,6 +4634,21 @@ func extractXMLField(b []byte, name string) string { return s[start : start+end] } +// internalRequestID returns the request id for the next request this deployment +// dispatches. +// +// Derived from the deployment's mint when it has one, so the sequence reproduces on +// a re-run of the same template (see [WithDeployerMint]); drawn from the wall clock +// otherwise, which is what every in-process dispatch did before. The `req-` prefix +// and the width match generateRequestID, because this value reaches the event log +// and `/v1/debug/events` beside ids that came from there. +func (d *StackDeployer) internalRequestID() string { + if !d.mint.Derived() { + return generateRequestID() + } + return "req-cfn-" + d.mint.Hex(12) +} + // dispatch performs in-process request routing, records the event, and returns // the response, the estimated cost, and any routing error. func (d *StackDeployer) dispatch( @@ -4589,11 +4656,19 @@ func (d *StackDeployer) dispatch( req *AWSRequest, streamID string, ) (*AWSResponse, float64, error) { + requestID := d.internalRequestID() reqCtx := &RequestContext{ - RequestID: generateRequestID(), + RequestID: requestID, AccountID: d.identity.accountID, Region: d.identity.region, Timestamp: d.tc.Now(), + // Seeded from the request id above, which is what makes the resource this + // request creates carry the identifier it carried in the recording (#856). + // Before this the deployer dispatched with a nil mint, so every plugin it + // reached fell back to crypto/rand — a replayed stack re-created every one + // of its resources under a new identifier, and ValidateState reported a + // state_hash mismatch for the create that was the right answer. + IDs: NewIDMint(requestID), Principal: d.attribution.resolve(), Metadata: map[string]interface{}{"stream_id": streamID}, } diff --git a/emulator/client.go b/emulator/client.go index 31ce9cb5..a52a10ac 100644 --- a/emulator/client.go +++ b/emulator/client.go @@ -131,7 +131,15 @@ func (b *Client) Deploy(ctx context.Context, cfn string, intent Intent) (*Deploy // threaded — substrate's defaults are the right answer here rather than a // placeholder for one. An in-process caller that does need another partition // can pass WithDeployerIdentity to NewStackDeployer directly. - deployer := NewStackDeployer(b.registry, b.store, b.state, b.tc, b.logger, b.costs) + // + // The mint is seeded from a fresh request id rather than left nil, so the + // deployment's resource identifiers are derived from one seed instead of drawn + // one at a time (#856). The seed is random because an in-process Deploy is not a + // recorded AWS request — there is no outer event for a replay to re-run — and + // that is enough: each dispatched request records the derived id it used, and a + // replay of *those* events reuses it. + deployer := NewStackDeployer(b.registry, b.store, b.state, b.tc, b.logger, b.costs, + WithDeployerMint(NewIDMint(generateRequestID()))) streamID := fmt.Sprintf("deploy-%d", b.tc.Now().UnixNano()) result, err := deployer.Deploy(ctx, cfn, streamID, nil) diff --git a/emulator/cloudformation_plugin.go b/emulator/cloudformation_plugin.go index 82622005..9b275f8f 100644 --- a/emulator/cloudformation_plugin.go +++ b/emulator/cloudformation_plugin.go @@ -75,7 +75,8 @@ func (p *CloudFormationPlugin) deployerFor(reqCtx *RequestContext, att cfnAttrib return NewStackDeployer(p.registry, p.store, p.state, p.tc, p.logger, p.costs, WithDeployerIdentity(reqCtx.AccountID, reqCtx.Region), WithDeployerAttribution(att.roleARN, att.creator), - WithDeployerAuth(p.auth)) + WithDeployerAuth(p.auth), + WithDeployerMint(reqCtx.IDs)) } // Name returns the service name "cloudformation". diff --git a/emulator/ec2_types.go b/emulator/ec2_types.go index a129f70a..a8bc2526 100644 --- a/emulator/ec2_types.go +++ b/emulator/ec2_types.go @@ -495,8 +495,8 @@ func generateAssociationID(m *IDMint) string { // flag day: a caller moves by taking a mint and calling [IDMint.Hex] with the same width. // EC2's own ids no longer come through here. // -// TODO(#856): 2 draw sites remain on crypto/rand — this one and [IDMint.bytes]'s own fallback; -// delete this function when the last caller moves. +// TODO(#856): one caller remains — the AIPA… instance profile id minted inside a describe, +// which is a fidelity bug of its own (#1291). Delete this function when it moves. func randomHex(n int) string { b := make([]byte, n) if _, err := rand.Read(b); err != nil { diff --git a/emulator/export_test.go b/emulator/export_test.go index 0c709c5d..690844c6 100644 --- a/emulator/export_test.go +++ b/emulator/export_test.go @@ -34,6 +34,17 @@ func GenerateIAMIDForTest(seed, prefix string) string { return generateIAMID(NewIDMint(seed), prefix) } +// APIGatewayInternalRequestIDForTest wraps apigwInternalRequestID for external tests, +// minting from a fresh [IDMint] over seed. +// +// Exported because the seedless branch is not reachable through the proxy integration: a +// request that arrives over the wire always carries a mint, so only a hand-built +// [RequestContext] — the shape a plugin unit test constructs — reaches the fallback, and +// #856's tier 8 turns on that branch existing rather than deriving from an empty seed. +func APIGatewayInternalRequestIDForTest(seed string) string { + return apigwInternalRequestID(NewIDMint(seed)) +} + // IAMUserARNForTest wraps iamUserARN for external tests. func IAMUserARNForTest(accountID, path, name string) string { return iamUserARN(accountID, path, name) } diff --git a/emulator/ids.go b/emulator/ids.go index 6edd39d8..011610c9 100644 --- a/emulator/ids.go +++ b/emulator/ids.go @@ -55,15 +55,19 @@ import ( // // The request id itself, which is the seed (#866 settled that it is wall-clock and recorded // rather than derived). Substrate's own bookkeeping ids — an event id, a snapshot id, a -// replay id — which no AWS call observes. EC2 key-pair *material*, which needs a +// replay id — which no AWS call observes. A Lambda ESM poll's dispatches, which are driven by +// a wall-clock ticker and recorded nowhere, so there is nothing for a replay to derive from +// (#1292). EC2 key-pair *material*, which needs a // deterministic reader into the key generator rather than a string; a replayed CreateKeyPair // still diverges on the key and its fingerprint (see the #856 checklist). And everything // already derived from its inputs: a public IP from its instance id, a secret's ARN from its // name, CloudFormation's stack UUIDs from account and region. // -// TODO(#856): 2 draw sites remain on crypto/rand — randomHex, whose one caller is the -// CloudFormation deployer, and bytes's own seedless fallback, which goes when the deployer -// threads a request id through. +// TODO(#856): one draw site remains on crypto/rand: randomHex, whose last caller mints an +// EC2 instance profile's AIPA… id inside a *describe* (#1291). bytes's own seedless fallback +// stays: a nil or seedless mint is what a hand-built RequestContext has, which is how every +// plugin unit test — substrate's and a consumer's — constructs one, and generateRequestID +// draws through it because the request id is the seed and is deliberately random. // IDMint mints the identifiers one request publishes, derived from that request's own id so // that replaying the request mints the same ones. diff --git a/emulator/ids_test.go b/emulator/ids_test.go index c7c70f7f..94155a05 100644 --- a/emulator/ids_test.go +++ b/emulator/ids_test.go @@ -2276,3 +2276,137 @@ const ( idsBackupHost = "backup.us-east-1.amazonaws.com" idsBedrockHost = "bedrock.us-east-1.amazonaws.com" ) + +// Tier 8 of #856: the internal dispatch paths — CloudFormation's deployer, and an API Gateway proxy +// integration's Lambda invoke. +// +// The seven tiers above each moved a family of generators. This one moves no generator at all: it +// gives a *mint* to the request contexts substrate builds in-process rather than parsing off the wire, +// which is how every identifier of every resource in every CloudFormation stack came to be undirected +// randomness while the draw-site count read two. A plugin reached through `StackDeployer.dispatch` was +// handed a context with a nil `IDs`, so it took `IDMint`'s seedless fallback — the count never saw it, +// because the deployer contains no `rand.Read` of its own. +// +// Two changes, fixing two different failures. Setting `IDs` on the dispatched context makes a replayed +// *internal* event reproduce its recording, the property every recorded event already had. Deriving +// the internal request *id* from the stack's own request makes a replayed outer `CreateStack` +// reproduce it too — a create is itself a recorded event, so replaying one re-runs the whole +// deployment, and without this the two halves of a replayed CloudFormation stream disagreed with each +// other rather than merely with the recording. + +// TestReplay_ACloudFormationStackReplaysWithTheIdentifiersItCreated is the assertion #856 was opened +// to be able to make, and the one the tiers above could not: a *stack* replays identically. +// +// The template is four resources across three services and the deployer's own stub, chosen so that +// every kind of minted value in a deployment is present — a VPC and a subnet minted by EC2's plugin +// through a dispatched request, a security group whose id a later describe addresses, and an +// `AWS::SSM::Association` physical id the deployer mints itself with no service behind it. The outer +// `CreateStack` event's `state_hash_after` covers all four at once, which is what makes this a single +// assertion rather than four. +func TestReplay_ACloudFormationStackReplaysWithTheIdentifiersItCreated(t *testing.T) { + t.Parallel() + ts := emulator.StartTestServer(t, + emulator.WithRecordedBodies(), emulator.WithRecordedStateHashes()) + require.True(t, ts.Store().RecordsStateHashes(), "precondition: state_hash_after is compared") + ts.FreezeTime() + + idsQueryCall(t, ts, idsCFNHost, "2010-05-15", map[string]string{ + "Action": "CreateStack", + "StackName": "ids-tier8-stack", + "TemplateBody": idsTier8Template, + }) + resources := string(idsQueryCall(t, ts, idsCFNHost, "2010-05-15", map[string]string{ + "Action": "DescribeStackResources", + "StackName": "ids-tier8-stack", + })) + assert.Contains(t, resources, "vpc-", "the stack created a VPC: %s", resources) + assert.Contains(t, resources, "subnet-", "and a subnet") + assert.Contains(t, resources, "sg-", "and a security group") + + results, err := replayEngineFor(ts, emulator.ReplayConfig{ValidateState: true}). + Replay(t.Context(), replayStreamID) + require.NoError(t, err) + + assert.Positive(t, results.TotalEvents, "the stream has to contain the stack's own requests") + assert.Equal(t, results.TotalEvents, results.SuccessEvents, + "every recorded request — the create and each resource call it dispatched — answers") + assert.Empty(t, results.Differences, + "a stack's resources replay with the identifiers they were created with: %s", + replayDifferenceSummary(results)) + assert.True(t, results.StateValid, + "and the state the deployment reaches is the recorded state: %v", results.StateErrors) +} + +// TestIDs_ADriftDetectionIDIsDerivedAndUUIDShaped covers the one identifier the deployer answers +// directly rather than through a dispatched request. +// +// It is addressed — `DescribeStackDriftDetectionStatus` takes it — and it was `generateRequestID`, +// which put substrate's internal `req-…` format into an AWS response element as well as making the +// value unreproducible. `API_DetectStackDrift` publishes a maximum length of 36 and no pattern, and +// its sample response is a UUID, so 36 hex-and-hyphen characters is the published bound met at its +// limit in the shape AWS's own example shows. +func TestIDs_ADriftDetectionIDIsDerivedAndUUIDShaped(t *testing.T) { + t.Parallel() + ts := emulator.StartTestServer(t) + ts.FreezeTime() + + idsQueryCall(t, ts, idsCFNHost, "2010-05-15", map[string]string{ + "Action": "CreateStack", + "StackName": "ids-tier8-drift", + "TemplateBody": idsTier8Template, + }) + var detected struct { + ID string `xml:"DetectStackDriftResult>StackDriftDetectionId"` + } + require.NoError(t, xml.Unmarshal(idsQueryCall(t, ts, idsCFNHost, "2010-05-15", map[string]string{ + "Action": "DetectStackDrift", + "StackName": "ids-tier8-drift", + }), &detected)) + + idsRequireHexUUID(t, detected.ID, "a drift detection id") + require.Len(t, detected.ID, 36, "the published maximum length, met exactly: %q", detected.ID) + assert.NotContains(t, detected.ID, "req-", + "substrate's internal request-id format does not reach an AWS response element: %q", + detected.ID) + + // Addressed, which is why deriving it matters: the status read takes the id back. + status := string(idsQueryCall(t, ts, idsCFNHost, "2010-05-15", map[string]string{ + "Action": "DescribeStackDriftDetectionStatus", + "StackDriftDetectionId": detected.ID, + })) + assert.Contains(t, status, detected.ID, "and answers for it: %s", status) +} + +// A proxy integration's internal request id is the other half of the same seam, and its seedless +// branch is only reachable from a hand-built RequestContext — the shape every plugin unit test +// builds — so it is asserted directly rather than through the integration. +func TestIDs_AProxyIntegrationInvokeIDDerivesFromTheRequestThatReachedIt(t *testing.T) { + t.Parallel() + + first := emulator.APIGatewayInternalRequestIDForTest("req-tier8-seed") + assert.Equal(t, first, emulator.APIGatewayInternalRequestIDForTest("req-tier8-seed"), + "one request id mints one invoke id, which is what makes the invocation replay") + assert.Regexp(t, `^req-apigw-[0-9a-f]{24}$`, first, "and it is hex over the seed: %q", first) + assert.NotEqual(t, first, emulator.APIGatewayInternalRequestIDForTest("req-tier8-other"), + "two requests do not share an invoke id") + + // No request behind the caller: the wall-clock id, which is what this path always used. + fallback := emulator.APIGatewayInternalRequestIDForTest("") + assert.NotContains(t, fallback, "req-apigw-", + "a seedless mint cannot derive, so it must not claim to: %q", fallback) + assert.NotEqual(t, fallback, emulator.APIGatewayInternalRequestIDForTest(""), + "and two seedless draws must not collide onto one invocation") +} + +// idsTier8Template is four resources over three services plus the deployer's own stub, sorted by the +// deployer into VPC, then subnet and security group, then the association — the order the mint's +// ordinal advances in, and the reason a re-run reproduces it. +const idsTier8Template = `{"Resources":{` + + `"Net":{"Type":"AWS::EC2::VPC","Properties":{"CidrBlock":"10.8.0.0/16"}},` + + `"Sub":{"Type":"AWS::EC2::Subnet","Properties":{"VpcId":{"Ref":"Net"},"CidrBlock":"10.8.1.0/24"}},` + + `"Sg":{"Type":"AWS::EC2::SecurityGroup","Properties":{"GroupDescription":"ids tier 8",` + + `"VpcId":{"Ref":"Net"}}},` + + `"Assoc":{"Type":"AWS::SSM::Association","Properties":{"Name":"AWS-RunShellScript"}}}}` + +// The host CloudFormation's query protocol is addressed at. +const idsCFNHost = "cloudformation.us-east-1.amazonaws.com" diff --git a/emulator/lambda_plugin.go b/emulator/lambda_plugin.go index a53933f3..2ea718cc 100644 --- a/emulator/lambda_plugin.go +++ b/emulator/lambda_plugin.go @@ -1617,6 +1617,14 @@ func (p *LambdaPlugin) pollAndInvoke(esm ESMConfig) { Path: "/", Body: nil, } + // No mint, deliberately. Substrate has four internal dispatch sites — a context built + // here rather than parsed off the wire — and #856's tier 8 gave a derived mint to the + // two that had something to derive from: the CloudFormation deployer's and an API + // Gateway proxy integration's. The other two are this poller's receive and its invoke + // below. An ESM poll is driven by a wall-clock ticker and is recorded nowhere, so + // there is no recorded request id for a replay to derive from and a seed here would be + // exactly as random as the nil mint it replaced. What this path needs is a simulated + // clock and a recorded dispatch, which is #1292. rxCtx := &RequestContext{ RequestID: generateRequestID(), AccountID: acct, @@ -1689,6 +1697,8 @@ func (p *LambdaPlugin) pollAndInvoke(esm ESMConfig) { Headers: map[string]string{"Content-Type": "application/json"}, Body: eventJSON, } + // Mintless for the same reason as the receive above (#1292): the identifier this + // invocation's plugin would publish has no recorded request id behind it. invokeCtx := &RequestContext{ RequestID: generateRequestID(), AccountID: fnAcct,