From a8dd06acf8cdf6e046ce999a31a464e805ded2b4 Mon Sep 17 00:00:00 2001 From: Scott Friedman <3011922+scttfrdmn@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:46:20 -0700 Subject: [PATCH 1/2] feat(#1277): the CloudFront origin access control family, and a tagged create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An origin access control is what lets a distribution serve a private S3 bucket, and substrate routed none of it: /2020-05-31/origin-access-control was not a path the plugin knew, so the create reached the unknown-route refusal and a consumer keeping its bucket private could not run against the emulator at all. That is the first of the ten missing operations #1274's adopter found, and the one it named as blocking. The four operations land in emulator/cloudfront_oac.go, along with the one thing this service had not modelled: a version. DeleteOriginAccessControl takes an If-Match, so the record stores an ETag and the delete separates the three ways it can fail — an absent control is NoSuchOriginAccessControl/404, a missing version is InvalidIfMatchVersion/400 and a stale one is PreconditionFailed/412, because telling a caller that sent no version that its version was stale is a wrong answer. The four Required: Yes config members are validated, three against their published enums and case-sensitively: accepting "S3" would pass a request through substrate that AWS refuses. An account using no controls answers no Items element at all, which is what the page states and what a decoder cannot tell from an empty one, so the test asserts on the raw XML. CreateDistributionWithTags is the same path and verb as CreateDistribution with ?WithTags — a bare query key, so the routing tests for the key's presence; testing for a value would have sent a tagged create to the untagged handler and dropped the tags while answering 201. Its body is decoded strictly, unlike CreateDistribution's, for #883's reason. generateCloudFrontID converts to the request's IDMint rather than gaining a fourth crypto/rand caller, which crosses CloudFront off #856: a recorded stream that creates a control, creates a distribution, invalidates inside it and deletes the control with the ETag the create handed out replays with zero differences. 28 draw sites remain. OriginAccessControlInUse/409 and OriginAccessControlAlreadyExists/409 are deliberately absent and documented as such — the first needs a distribution's origins, which substrate records nowhere (#1271). Refs #1274. --- CHANGELOG.md | 42 ++ docs/services.md | 59 ++- docs/testing-guide.md | 4 +- emulator/cfn_resources_v23.go | 5 +- emulator/cloudfront_oac.go | 401 +++++++++++++++++++ emulator/cloudfront_oac_test.go | 681 ++++++++++++++++++++++++++++++++ emulator/cloudfront_plugin.go | 231 ++++++++--- emulator/cloudfront_tags.go | 23 +- emulator/ec2_types.go | 2 +- emulator/ids.go | 2 +- 10 files changed, 1389 insertions(+), 61 deletions(-) create mode 100644 emulator/cloudfront_oac.go create mode 100644 emulator/cloudfront_oac_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 696bf101..70faf82b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 behaves exactly as it did. The cost, stated in `docs/services.md`: an identifier is guessable from a request ID, which is already true of the request ID and is acceptable for a test emulator whose identifiers name nothing outside it. +- **CloudFront's origin access control family, and `CreateDistributionWithTags`** (#1277). An origin + access control is what lets a distribution serve a **private** S3 bucket — CloudFront signs the + origin request with SigV4 and the bucket policy trusts the distribution rather than the world — and + substrate routed none of it: `/2020-05-31/origin-access-control` was not a path the plugin knew, so + the create reached the unknown-route refusal and a consumer keeping its bucket private could not run + against the emulator at all. That was the first of ten missing operations an adopter found by + replacing ~1,100 lines of hand-written AWS fakes with substrate (#1274), and the one it named as + blocking. `CreateOriginAccessControl` answers 201 with the `ETag` and `Location` headers — neither is + in the API Reference's Response Syntax block, both are output members in the CLI and the SDKs, and + the ETag is what a later delete has to echo. The four `Required: Yes` config members are validated, + three of them against their published enums (`s3|mediastore|mediapackagev2|lambda`, + `never|always|no-override`, `sigv4`), case-sensitively: accepting `S3` would pass a request through + substrate that AWS refuses, which is the direction a consumer pays for with a failed live deploy. + `GetOriginAccessControl` answers the same document and the same version; `ListOriginAccessControls` + answers the account's controls whole, and an account using none answers **no `Items` element at + all**, which is what the page states and what a decoder cannot distinguish from an empty one — so + the test asserts on the raw XML. `DeleteOriginAccessControl` requires `If-Match` and separates the + three ways it can fail: an absent control is `NoSuchOriginAccessControl`/404, a *missing* version is + `InvalidIfMatchVersion`/400 and a stale one is `PreconditionFailed`/412, because telling a caller + that sent no version that its version was stale is a wrong answer. `CreateDistributionWithTags` is + the same path and verb as `CreateDistribution` with `?WithTags` — a bare query key, so the routing + tests for the key's *presence*; testing for a value would have sent a tagged create to the untagged + handler and dropped the tags while answering 201. Its body is decoded strictly, unlike + `CreateDistribution`'s: a caller that asked for tags must not be handed an untagged distribution and + a success, which is #883's argument applied to the create. Two published behaviors are deliberately + absent and documented as such in `docs/services.md`: `OriginAccessControlInUse`/409 needs to know a + distribution's origins and substrate records none (#1271 is where that changes), and + `OriginAccessControlAlreadyExists`/409 is published for a control "with the specified parameters" + without publishing which parameters, and a control carries no `CallerReference` to key a duplicate + on. `UpdateOriginAccessControl` is not implemented. ### Changed @@ -198,6 +228,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 receives an SQS message, subscribes to an SNS topic and creates an EFS file system with an access point — each later request naming what an earlier one minted — now replays with **zero** differences and `StateValid` true. +- **A replayed CloudFront create mints the identifiers its recording minted** (#856). One generator + produces every identifier this service publishes — a distribution ID, an invalidation ID, and now an + origin access control's ID and its ETag — so the whole service moved with #1277 rather than waiting + for its turn in the per-family tiering: adding a fourth `crypto/rand` caller to a list #856 is + actively shortening was the wrong direction. `generateCloudFrontID` now takes the request's `IDMint` + and has no error to return, the alphabet it maps onto is a named constant the three callers share, + and the mapping is byte-for-byte the one it performed before, so an ID recorded by an earlier + substrate is still the shape this one mints. A recorded stream that creates a control, reads it back, + creates a distribution, invalidates inside it and deletes the control with the ETag the create handed + out now replays with **zero** differences and `StateValid` true. That last step is what makes the + claim load-bearing rather than cosmetic: a re-minted ETag turns the recorded delete into a + `PreconditionFailed` against a control nothing had changed. 28 draw sites remain on `crypto/rand`. - **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 ba81f4b2..9a9a1631 100644 --- a/docs/services.md +++ b/docs/services.md @@ -2207,7 +2207,9 @@ Three kinds of value stay random, and one more is still migrating: CloudFormation's [stack and change-set ARNs](#stack-and-change-set-arns-are-deterministic), which predate this rule and are what generalising it was modelled on. - EC2, IAM, STS, SQS, SNS, Lambda, EFS, FSx, Transfer, ECS, Step Functions, EventBridge, - CloudWatch Logs and Service Quotas identifiers are derived today. The remaining services are + CloudWatch Logs, CloudFront and Service Quotas identifiers are derived today. A CloudFront + distribution, invalidation and origin access control all draw from one generator, so the three + moved together with the origin access control family (#1277). The remaining services are migrating one family at a time, tracked on #856; until a service moves, its identifiers are still drawn from `crypto/rand` and a replay of a stream creating one of its resources still diverges. @@ -16650,7 +16652,8 @@ Kinesis shard: $0.015 per shard-hour. PUT payload: $0.014 per million 25KB units | Operation | Notes | |-----------|-------| -| CreateDistribution | Distribution IDs: `E{13-char upper alphanum}` | +| CreateDistribution | Distribution IDs: `E{13-char upper alphanum}`, derived from the request ID (#1277) | +| CreateDistributionWithTags | Same path as `CreateDistribution` with `?WithTags`; body is a ``. A body substrate cannot decode is refused rather than creating an untagged distribution | | GetDistribution | | | GetDistributionConfig | Answers `DistributionConfig` members only, and two of its five required ones — see [A configuration is not a distribution](#a-configuration-is-not-a-distribution) | | UpdateDistribution | Shares the `/config` path with `GetDistributionConfig`, told apart by the verb | @@ -16662,6 +16665,10 @@ Kinesis shard: $0.015 per shard-hour. PUT payload: $0.014 per million 25KB units | TagResource | Body is a `` document; a body of another shape is refused rather than read as an empty tag set (#883) | | UntagResource | Body is a `` document. Removing a key the distribution does not carry succeeds — AWS documents no error for it, so that reading is substrate's (#883) | | ListTagsForResource | Reports the `` members sorted by key — see [A tag set read back out of a map](#a-tag-set-read-back-out-of-a-map) | +| CreateOriginAccessControl | 201 with the `ETag` and `Location` headers; the four required config members are validated against their published enums — see [The origin access control family](#the-origin-access-control-family) | +| GetOriginAccessControl | 200 with the `ETag` header; absent → `NoSuchOriginAccessControl` | +| ListOriginAccessControls | An account using no origin access controls answers **no `Items` element** | +| DeleteOriginAccessControl | 204. `If-Match` required: missing → `InvalidIfMatchVersion`, stale → `PreconditionFailed`. `OriginAccessControlInUse` is not answered — see below | All three tagging operations share the `POST`/`GET /2020-05-31/tagging` path and are told apart by the query string: `Operation=Tag`, `Operation=Untag`, and a `GET` carrying only `Resource`. A @@ -16678,6 +16685,49 @@ for why `GetResources` reports a distribution in `us-east-1` alone. All CloudFront resources are stored under `us-east-1` (global service). +### The origin access control family + +An origin access control is what lets a distribution read a private S3 bucket: CloudFront signs the +origin request with SigV4 and the bucket policy trusts the distribution rather than the world. The +four operations live on their own path — `POST`/`GET /2020-05-31/origin-access-control` for the +collection, `GET`/`DELETE …/origin-access-control/{Id}` for one member — and substrate routed none +of it before #1277, so a consumer keeping its bucket private could not run against the emulator at +all: the create reached the unknown-route refusal. + +`OriginAccessControlConfig` marks four members `Required: Yes`, and three of the four publish a +closed set of values: `OriginAccessControlOriginType` is `s3|mediastore|mediapackagev2|lambda`, +`SigningBehavior` is `never|always|no-override`, and `SigningProtocol` is `sigv4`. A missing member +or a value outside its enum is `InvalidArgument`/400, which is the code the operation publishes for +both. The comparison is case-sensitive: accepting `S3` would let a request through substrate that +AWS refuses, which is the direction a consumer pays for with a failed live deploy. + +The control carries an **ETag**, and it is the one version substrate models on this service. The +create answers it as a header alongside `Location`, the get answers it as a header, and +`DeleteOriginAccessControl` requires it in `If-Match`: a missing header is +`InvalidIfMatchVersion`/400 and a value that is not the current one is `PreconditionFailed`/412 — +two published codes for two different mistakes, so a caller that sent no version is not told its +version was stale. The `ETag` *shape* is substrate's: AWS publishes only that the value identifies +the current version, so substrate mints the same `E`-prefixed form it mints IDs in, from the same +per-request mint, which is what makes a replayed create hand out the version its recording did. +A quoted `If-Match` is accepted as well as a bare one, since HTTP ETags are conventionally quoted +and CloudFront's are not. + +`ListOriginAccessControls` answers the whole list, and an account using none answers **no `Items` +element at all** rather than an empty one — the page states exactly that, and it is the difference +between a caller's `len(Items) == 0` and a decode that has nothing to decode. `Marker`, `MaxItems` +and `NextMarker` are published members and are not rendered, for the reason `ListDistributions` +omits them: substrate holds no value for a page boundary it never draws. + +Two published behaviours are deliberately absent. **`OriginAccessControlInUse`/409** on delete needs +to know that some distribution names the control, and substrate records no origins — that is the gap +[A configuration is not a distribution](#a-configuration-is-not-a-distribution) describes, and +[#1271](https://github.com/scttfrdmn/substrate/issues/1271) is where a distribution starts recording +its configuration and the check becomes answerable. **`OriginAccessControlAlreadyExists`/409** on +create is published for a control "with the specified parameters", which parameters is not published, +and a control carries no `CallerReference` to key a duplicate on the way a distribution does; a +repeated create mints a second control, so a consumer converging by name should list first. +`UpdateOriginAccessControl` is not implemented. + ### A configuration is not a distribution `GetDistribution` returns a `Distribution` and `GetDistributionConfig` returns a @@ -16701,6 +16751,11 @@ shape from. An `Origins` needs `Items` and a `Quantity`; a `DefaultCacheBehavior subtree. Omitting a member substrate holds no value for is the honest answer; inventing one would assert a shape AWS has not published. +The unrecorded `Origins` is also what defers `OriginAccessControlInUse`/409: an origin access +control is referenced *from* an origin, so until a distribution records the origins it was created +with, a delete cannot tell an unused control from one a live distribution depends on. Both halves +land together in [#1271](https://github.com/scttfrdmn/substrate/issues/1271). + `API_GetDistributionConfig` publishes, on its `Id` parameter: *"The distribution's ID. If the ID is empty, an empty distribution configuration is returned."* An empty ID is reachable — the path `/2020-05-31/distribution//config` routes to the operation with an empty ID — and substrate answers diff --git a/docs/testing-guide.md b/docs/testing-guide.md index b013b99f..82ee1a73 100644 --- a/docs/testing-guide.md +++ b/docs/testing-guide.md @@ -336,8 +336,8 @@ all; before #856 every such stream diverged on its first create, which is why th tests above are built on caller-chosen bucket and key names instead. 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 and Service -Quotas are; the rest are migrating one family at a time, and until a service moves, a +Lambda, EFS, FSx, Transfer, ECS, Step Functions, EventBridge, CloudWatch Logs, CloudFront and +Service Quotas are; the rest are migrating one family at a time, and until a service moves, a replay of a stream creating one of its resources still diverges. And a recording made against an **unfrozen** clock can still diverge on a `state_hash_after` even when every identifier matches, because a handler reading the live clock stamps its record a few hundred diff --git a/emulator/cfn_resources_v23.go b/emulator/cfn_resources_v23.go index 15d3e23d..88d11b9a 100644 --- a/emulator/cfn_resources_v23.go +++ b/emulator/cfn_resources_v23.go @@ -123,7 +123,10 @@ const cfnOAIIDLen = 13 // cfnGeneratedName records for its own suffix: UpdateStack in substrate re-deploys the whole // template, so an ID minted from crypto/rand would change on every update and leak the identity // it replaced. It is derived rather than reused from generateCloudFrontID -// (cloudfront_plugin.go), which produces exactly this shape but reads crypto/rand. +// (cloudfront_plugin.go), which produces exactly this shape but draws from the *request's* mint: +// that makes a replayed CreateDistribution reproduce its ID (#856), and says nothing about an +// UpdateStack in the same run, which is a second request with a second request id and so a second +// mint. What this ID has to be stable across is redeployment, not replay. // // The two obvious deterministic helpers do not fit. cfnGeneratedName returns a hyphenated // {stack}-{logical}-{suffix}, which is not this shape. cfnNameSuffix is pinned to twelve base-36 diff --git a/emulator/cloudfront_oac.go b/emulator/cloudfront_oac.go new file mode 100644 index 00000000..e6b74e70 --- /dev/null +++ b/emulator/cloudfront_oac.go @@ -0,0 +1,401 @@ +package emulator + +// CloudFront origin access control: the family that lets a distribution read a private S3 +// bucket, and the one whose absence stops such a deploy at the first call. +// +// An origin access control (OAC) is a standalone CloudFront resource — created, read, listed and +// deleted on its own path, then named by an origin through OriginAccessControlId. Substrate +// routed none of it before #1277, so a consumer keeping its bucket private and serving it through +// a SigV4-signed origin could not run against the emulator at all: the create fell through +// [CloudFrontPlugin.HandleRequest]'s default arm as an unknown route. +// +// The whole family lives in one file rather than beside the distribution operations, because the +// two share nothing but the plugin: an OAC has no ARN, is not taggable, carries no status and +// does not transition. What it does carry is a **version**, and that is the one thing here the +// distribution operations do not model — DeleteOriginAccessControl takes an If-Match and answers +// PreconditionFailed for a stale one, so the record stores an ETag and the delete compares it. +// +// Two members of the published contract are deliberately absent, both recorded here rather than +// left to be discovered: +// +// - OriginAccessControlInUse/409, which DeleteOriginAccessControl publishes for an OAC a +// distribution still names. Answering it means knowing a distribution's origins, and +// substrate records none — that is the same gap docs/services.md describes under "A +// configuration is not a distribution", and it is why GetDistributionConfig answers two of +// its five required members. #1271 is where a distribution starts recording its +// configuration; the check becomes answerable there and is a follow-up on #1277 rather than +// a guess here. +// - OriginAccessControlAlreadyExists/409, which CreateOriginAccessControl publishes for one +// "with the specified parameters". Which parameters make two OACs the same is not published, +// and an OAC carries no CallerReference to key it on the way a distribution does, so +// substrate would be inventing the duplicate key. A create that repeats a name succeeds and +// mints a second OAC; a consumer converging by name should list first, which is what the +// operation exists for. + +import ( + "bytes" + "context" + "encoding/json" + "encoding/xml" + "fmt" + "net/http" + "strings" +) + +// CloudFront origin access control state-key prefixes: the record and the per-account index of +// its IDs. Declared beside the distribution and invalidation prefixes in cloudfront_tags.go for +// the reason that block records — one producer per key kind is what keeps [cfKeyIsTaggable]'s +// single positive test honest, and a prefix spelled inline here would be invisible to it. + +// CloudFrontOriginAccessControl holds persisted state for a CloudFront origin access control. +// +// Every field is a member the API publishes, plus the ETag. There is no AccountID: the record is +// keyed by the calling account and read back the same way, and no operation here resolves an OAC +// from an ARN, so an account on the record would be a field nothing reads — and a wire-visible +// bookkeeping member of exactly the kind #756 is removing. +type CloudFrontOriginAccessControl struct { + // ID is the unique identifier CloudFront assigns, in the same E-prefixed shape as a + // distribution ID. + ID string `json:"Id"` + + // Name identifies the origin access control. Required on create; up to 64 characters. + Name string `json:"Name"` + + // Description is the optional human-readable description. + Description string `json:"Description,omitempty"` + + // OriginType is OriginAccessControlOriginType: the kind of origin this control is for, + // one of s3, mediastore, mediapackagev2 or lambda. + OriginType string `json:"OriginAccessControlOriginType"` + + // SigningBehavior is which origin requests CloudFront signs: never, always or + // no-override. + SigningBehavior string `json:"SigningBehavior"` + + // SigningProtocol is the signing protocol; sigv4 is the only published value. + SigningProtocol string `json:"SigningProtocol"` + + // ETag is the version a caller must echo in If-Match to delete this control. + // + // AWS publishes no shape for it — only that it identifies "the current version" — so the + // E-prefixed form substrate mints is substrate's choice. What matters is that it is opaque + // to the caller and that the delete refuses a value that is not the current one. + ETag string `json:"ETag"` +} + +// cfOACKey returns the state key an origin access control is stored at. +// +// No Region component, for the reason [cfDistKey] has none: CloudFront is global. +func cfOACKey(accountID, oacID string) string { + return cfOACKeyPrefix + accountID + "/" + oacID +} + +// cfOACIDsKey returns the state index key for all origin access control IDs in an account. +func cfOACIDsKey(accountID string) string { + return cfOACIDsKeyPrefix + accountID +} + +// cfOACConfigWire is the OriginAccessControlConfig document, which is both the create request's +// body and a member of every response. +// +// One struct for both directions so the two cannot drift on an element name — a create that +// decodes SigningBehavior from one spelling and renders it under another would round-trip +// through substrate and fail against AWS. The member order is the reference's. +// +// XMLName is declared rather than left to the field tag so that decoding a create body *enforces* +// the root element: a body whose root is something else is a refusal rather than a config with +// every member empty, which would then fail the required-member checks with the wrong reason. +type cfOACConfigWire struct { + XMLName xml.Name `xml:"OriginAccessControlConfig"` + Description string `xml:"Description,omitempty"` + Name string `xml:"Name"` + OriginType string `xml:"OriginAccessControlOriginType"` + SigningBehavior string `xml:"SigningBehavior"` + SigningProtocol string `xml:"SigningProtocol"` +} + +// cfOACWire is the OriginAccessControl document CreateOriginAccessControl and +// GetOriginAccessControl both answer. +type cfOACWire struct { + XMLName xml.Name `xml:"OriginAccessControl"` + ID string `xml:"Id"` + Config cfOACConfigWire `xml:"OriginAccessControlConfig"` +} + +// cfOACSummaryWire is one OriginAccessControlSummary in a ListOriginAccessControls response. It +// is the config's members plus the Id, flattened — the summary is not a nested config. +type cfOACSummaryWire struct { + XMLName xml.Name `xml:"OriginAccessControlSummary"` + Description string `xml:"Description,omitempty"` + ID string `xml:"Id"` + Name string `xml:"Name"` + OriginType string `xml:"OriginAccessControlOriginType"` + SigningBehavior string `xml:"SigningBehavior"` + SigningProtocol string `xml:"SigningProtocol"` +} + +// cfOACListWire is the OriginAccessControlList document. +// +// Marker, MaxItems and NextMarker are published members and are not rendered, because substrate +// answers the list whole and so holds no value for them — the same reading [listDistributions] +// takes for DistributionList, and the same one docs/services.md states for the three +// DistributionConfig members substrate cannot answer. Inventing a MaxItems would assert a page +// size AWS publishes nowhere. +// +// Items is a pointer to a slice so that an account using no OACs answers **no Items element at +// all**, which is what the reference states: "If you're not using origin access controls for your +// AWS account, the ListOriginAccessControls operation doesn't return the Items element in the +// response." An empty slice would render the same absence by accident rather than by decision, +// and a later reader could "fix" it into an empty element; the pointer makes the absence +// deliberate and reviewable. +type cfOACListWire struct { + XMLName xml.Name `xml:"OriginAccessControlList"` + IsTruncated bool `xml:"IsTruncated"` + Items *[]cfOACSummaryWire `xml:"Items>OriginAccessControlSummary,omitempty"` + Quantity int `xml:"Quantity"` +} + +// createOriginAccessControl handles POST /2020-05-31/origin-access-control. +// +// The response carries the ETag and Location headers. Neither appears in the API Reference's +// Response Syntax block — which shows the XML body alone — but both are top-level members of the +// operation's output shape in the CLI and the SDKs, and the ETag is the value a caller must hold +// to delete the control later. Emitting them as headers is how a REST/XML output member that is +// not in the body reaches a caller. +func (p *CloudFrontPlugin) createOriginAccessControl(ctx *RequestContext, req *AWSRequest) (*AWSResponse, error) { + var cfg cfOACConfigWire + if err := xml.NewDecoder(bytes.NewReader(req.Body)).Decode(&cfg); err != nil { + return nil, cfInvalidOACArgument(fmt.Sprintf("the request body is not a valid document: %v", err)) + } + if err := cfValidateOACConfig(cfg); err != nil { + return nil, err + } + + oac := CloudFrontOriginAccessControl{ + ID: generateCloudFrontID(ctx.IDs), + Name: cfg.Name, + Description: cfg.Description, + OriginType: cfg.OriginType, + SigningBehavior: cfg.SigningBehavior, + SigningProtocol: cfg.SigningProtocol, + ETag: cfMintETag(ctx.IDs), + } + + if err := p.putOriginAccessControl(ctx.AccountID, oac); err != nil { + return nil, err + } + goCtx := context.Background() + updateStringIndex(goCtx, p.state, cloudfrontNamespace, cfOACIDsKey(ctx.AccountID), oac.ID) + + resp, err := cfOACResponse(http.StatusCreated, oac) + if err != nil { + return nil, err + } + resp.Headers["Location"] = cfOACLocation + oac.ID + return resp, nil +} + +// getOriginAccessControl handles GET /2020-05-31/origin-access-control/{Id}. +func (p *CloudFrontPlugin) getOriginAccessControl(ctx *RequestContext, oacID string) (*AWSResponse, error) { + oac, err := p.loadOriginAccessControl(ctx.AccountID, oacID) + if err != nil { + return nil, err + } + return cfOACResponse(http.StatusOK, oac) +} + +// cfOACResponse renders one control as the OriginAccessControl document with its version in the +// ETag header. +// +// Both the create and the get answer through here, so neither the body nor the header rule can +// drift: a get that answered a version the create never handed out would fail a caller's delete +// against a control nothing had changed. +func cfOACResponse(status int, oac CloudFrontOriginAccessControl) (*AWSResponse, error) { + resp, err := cloudfrontXMLResponse(status, cfOACWireFrom(oac)) + if err != nil { + return nil, err + } + resp.Headers["ETag"] = oac.ETag + return resp, nil +} + +// listOriginAccessControls handles GET /2020-05-31/origin-access-control. +// +// An unreadable record is skipped rather than failing the list, following [listDistributions]: +// one corrupt record must not make the account's other controls unfindable. +func (p *CloudFrontPlugin) listOriginAccessControls(ctx *RequestContext) (*AWSResponse, error) { + goCtx := context.Background() + ids, err := loadStringIndex(goCtx, p.state, cloudfrontNamespace, cfOACIDsKey(ctx.AccountID)) + if err != nil { + return nil, fmt.Errorf("cloudfront listOriginAccessControls loadIndex: %w", err) + } + + summaries := make([]cfOACSummaryWire, 0, len(ids)) + for _, id := range ids { + oac, loadErr := p.loadOriginAccessControl(ctx.AccountID, id) + if loadErr != nil { + continue + } + summaries = append(summaries, cfOACSummaryWire{ + Description: oac.Description, + ID: oac.ID, + Name: oac.Name, + OriginType: oac.OriginType, + SigningBehavior: oac.SigningBehavior, + SigningProtocol: oac.SigningProtocol, + }) + } + + list := cfOACListWire{Quantity: len(summaries)} + if len(summaries) > 0 { + list.Items = &summaries + } + return cloudfrontXMLResponse(http.StatusOK, list) +} + +// deleteOriginAccessControl handles DELETE /2020-05-31/origin-access-control/{Id}. +// +// The three refusals are three different failures and the reference gives each its own code: an +// absent control is NoSuchOriginAccessControl/404, a missing If-Match is +// InvalidIfMatchVersion/400 ("The If-Match version is missing or not valid"), and a version that +// is present but not the current one is PreconditionFailed/412. Answering the precondition +// failure for a *missing* header would tell a caller its version was stale when it never sent +// one. +// +// The absence is checked before the header, so a caller deleting an already-deleted control is +// told that rather than being asked for a version of something that is gone. +func (p *CloudFrontPlugin) deleteOriginAccessControl(ctx *RequestContext, req *AWSRequest, oacID string) (*AWSResponse, error) { + oac, err := p.loadOriginAccessControl(ctx.AccountID, oacID) + if err != nil { + return nil, err + } + + // A quoted value is tolerated: HTTP ETags are conventionally quoted, CloudFront's are not, + // and a caller that re-quotes the value substrate handed it means the version it was given. + ifMatch := strings.Trim(strings.TrimSpace(headerValueFold(req.Headers, "If-Match")), `"`) + if ifMatch == "" { + return nil, &AWSError{ + Code: "InvalidIfMatchVersion", + Message: "The If-Match version is missing or not valid for the resource.", + HTTPStatus: http.StatusBadRequest, + } + } + if ifMatch != oac.ETag { + return nil, &AWSError{ + Code: "PreconditionFailed", + Message: "The precondition in one or more of the request fields evaluated to false.", + HTTPStatus: http.StatusPreconditionFailed, + } + } + + goCtx := context.Background() + if err := p.state.Delete(goCtx, cloudfrontNamespace, cfOACKey(ctx.AccountID, oacID)); err != nil { + return nil, fmt.Errorf("cloudfront deleteOriginAccessControl state.Delete: %w", err) + } + removeFromStringIndex(goCtx, p.state, cloudfrontNamespace, cfOACIDsKey(ctx.AccountID), oacID) + + return &AWSResponse{StatusCode: http.StatusNoContent, Headers: map[string]string{}, Body: nil}, nil +} + +// cfOACWireFrom maps a stored control onto the OriginAccessControl document, for [cfOACResponse]. +func cfOACWireFrom(oac CloudFrontOriginAccessControl) cfOACWire { + return cfOACWire{ + ID: oac.ID, + Config: cfOACConfigWire{ + Description: oac.Description, + Name: oac.Name, + OriginType: oac.OriginType, + SigningBehavior: oac.SigningBehavior, + SigningProtocol: oac.SigningProtocol, + }, + } +} + +// cfValidateOACConfig refuses a config missing a required member or carrying a value outside a +// published enum. +// +// Four members are Required: Yes — Name, OriginAccessControlOriginType, SigningBehavior and +// SigningProtocol — and three of those four publish a closed set of valid values. InvalidArgument +// /400 is the code the operation publishes for both kinds of failure ("An argument is invalid"); +// the messages are substrate's, since the reference publishes codes and not message text, and +// each names the member so a caller learns which one in one round trip rather than in four. +// +// The enum values are compared exactly. AWS renders them lowercase and the SDKs send them that +// way; accepting "S3" would let a request through substrate that AWS refuses, which is the +// direction that costs a consumer a live deploy to discover. +func cfValidateOACConfig(cfg cfOACConfigWire) *AWSError { + if cfg.Name == "" { + return cfInvalidOACArgument("the OriginAccessControlConfig member Name is required") + } + switch cfg.OriginType { + case "s3", "mediastore", "mediapackagev2", "lambda": + case "": + return cfInvalidOACArgument("the OriginAccessControlConfig member OriginAccessControlOriginType is required") + default: + return cfInvalidOACArgument(fmt.Sprintf( + "OriginAccessControlOriginType %q is not one of s3, mediastore, mediapackagev2, lambda", cfg.OriginType)) + } + switch cfg.SigningBehavior { + case "never", "always", "no-override": + case "": + return cfInvalidOACArgument("the OriginAccessControlConfig member SigningBehavior is required") + default: + return cfInvalidOACArgument(fmt.Sprintf( + "SigningBehavior %q is not one of never, always, no-override", cfg.SigningBehavior)) + } + switch cfg.SigningProtocol { + case "sigv4": + case "": + return cfInvalidOACArgument("the OriginAccessControlConfig member SigningProtocol is required") + default: + return cfInvalidOACArgument(fmt.Sprintf("SigningProtocol %q is not sigv4", cfg.SigningProtocol)) + } + return nil +} + +// cfInvalidOACArgument reports that an origin access control request carries an invalid argument. +func cfInvalidOACArgument(reason string) *AWSError { + return &AWSError{ + Code: "InvalidArgument", + Message: reason, + HTTPStatus: http.StatusBadRequest, + } +} + +// putOriginAccessControl persists a control under the key the account and ID name. +func (p *CloudFrontPlugin) putOriginAccessControl(accountID string, oac CloudFrontOriginAccessControl) error { + data, err := json.Marshal(oac) + if err != nil { + return fmt.Errorf("cloudfront putOriginAccessControl marshal: %w", err) + } + if err := p.state.Put(context.Background(), cloudfrontNamespace, cfOACKey(accountID, oac.ID), data); err != nil { + return fmt.Errorf("cloudfront putOriginAccessControl state.Put: %w", err) + } + return nil +} + +// loadOriginAccessControl loads a control owned by accountID, answering +// NoSuchOriginAccessControl/404 if absent. +// +// The message is the description the reference publishes for the code, and it names no control — +// the same choice [CloudFrontPlugin.loadDistributionForAccount] records, for the same reason: an +// empty ID is reachable through the path /2020-05-31/origin-access-control/, so a message +// interpolating the ID would trail a bare colon. +func (p *CloudFrontPlugin) loadOriginAccessControl(accountID, oacID string) (CloudFrontOriginAccessControl, error) { + data, err := p.state.Get(context.Background(), cloudfrontNamespace, cfOACKey(accountID, oacID)) + if err != nil { + return CloudFrontOriginAccessControl{}, fmt.Errorf("cloudfront loadOriginAccessControl state.Get: %w", err) + } + if data == nil { + return CloudFrontOriginAccessControl{}, &AWSError{ + Code: "NoSuchOriginAccessControl", + Message: "The origin access control does not exist.", + HTTPStatus: http.StatusNotFound, + } + } + var oac CloudFrontOriginAccessControl + if err := json.Unmarshal(data, &oac); err != nil { + return CloudFrontOriginAccessControl{}, fmt.Errorf("cloudfront loadOriginAccessControl unmarshal: %w", err) + } + return oac, nil +} diff --git a/emulator/cloudfront_oac_test.go b/emulator/cloudfront_oac_test.go new file mode 100644 index 00000000..aa8f2abb --- /dev/null +++ b/emulator/cloudfront_oac_test.go @@ -0,0 +1,681 @@ +package emulator_test + +import ( + "context" + "encoding/xml" + "errors" + "io" + "log/slog" + "net/http" + "regexp" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/scttfrdmn/substrate/emulator" +) + +// cfOACConfigXML is a valid OriginAccessControlConfig: the private-S3-origin case, which is what +// every one of these operations exists to serve. +const cfOACConfigXML = `` + + `substrate test control` + + `substrate-oac` + + `s3` + + `always` + + `sigv4` + + `` + +// cfOACPath is the collection path the four operations are addressed on. +const cfOACPath = "/2020-05-31/origin-access-control" + +// cfOACIDPattern is the shape a CloudFront identifier is published in: E and thirteen uppercase +// alphanumerics. Anchored, so a longer ID that merely starts right fails. +var cfOACIDPattern = regexp.MustCompile(`^E[A-Z0-9]{13}$`) + +// cfOACDoc is the OriginAccessControl document the create and the get both answer. +type cfOACDoc struct { + XMLName xml.Name `xml:"OriginAccessControl"` + ID string `xml:"Id"` + Config struct { + Description string `xml:"Description"` + Name string `xml:"Name"` + OriginType string `xml:"OriginAccessControlOriginType"` + SigningBehavior string `xml:"SigningBehavior"` + SigningProtocol string `xml:"SigningProtocol"` + } `xml:"OriginAccessControlConfig"` +} + +// cfCreateOAC creates one origin access control and returns the decoded document and the response. +func cfCreateOAC(t *testing.T, p *emulator.CloudFrontPlugin, ctx *emulator.RequestContext, + body string, +) (cfOACDoc, *emulator.AWSResponse) { + t.Helper() + resp, err := p.HandleRequest(ctx, cfRequest(http.MethodPost, cfOACPath, nil, body)) + require.NoError(t, err) + require.Equal(t, http.StatusCreated, resp.StatusCode, "body=%s", resp.Body) + + var doc cfOACDoc + require.NoError(t, xml.Unmarshal(resp.Body, &doc), "body=%s", resp.Body) + return doc, resp +} + +// TestCloudFrontOAC_CreateThenGetAnswersTheConfigItWasGiven is the round trip the whole family +// exists for: a control created here is readable by the ID the create handed back, with the config +// members it was given and the version it was given. +// +// The ETag is asserted on both responses because it is the value a caller has to carry from the +// create to the delete — a get that answered a *different* version would make a converging +// consumer's delete fail its precondition against a control nothing had changed. +func TestCloudFrontOAC_CreateThenGetAnswersTheConfigItWasGiven(t *testing.T) { + p, ctx := setupCloudFrontPlugin(t) + + created, createResp := cfCreateOAC(t, p, ctx, cfOACConfigXML) + + assert.Regexp(t, cfOACIDPattern, created.ID, "an OAC ID has a distribution ID's shape") + assert.Equal(t, "substrate-oac", created.Config.Name) + assert.Equal(t, "substrate test control", created.Config.Description) + assert.Equal(t, "s3", created.Config.OriginType) + assert.Equal(t, "always", created.Config.SigningBehavior) + assert.Equal(t, "sigv4", created.Config.SigningProtocol) + + etag := createResp.Headers["ETag"] + assert.Regexp(t, cfOACIDPattern, etag, "the create answers a version in the ETag header") + assert.Equal(t, "https://cloudfront.amazonaws.com/2020-05-31/origin-access-control/"+created.ID, + createResp.Headers["Location"], "and the Location of the resource it made") + + getResp, err := p.HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath+"/"+created.ID, nil, "")) + require.NoError(t, err) + require.Equal(t, http.StatusOK, getResp.StatusCode, "body=%s", getResp.Body) + + var fetched cfOACDoc + require.NoError(t, xml.Unmarshal(getResp.Body, &fetched)) + assert.Equal(t, created, fetched, "the get answers the document the create answered") + assert.Equal(t, etag, getResp.Headers["ETag"], "and the same version") +} + +// TestCloudFrontOAC_AnUnusedAccountAnswersNoItemsElement pins the reference's own sentence: "If +// you're not using origin access controls for your AWS account, the ListOriginAccessControls +// operation doesn't return the Items element in the response." +// +// Asserted on the raw XML rather than on a decoded shape, because a decoder cannot tell an absent +// element from an empty one — which is exactly the distinction being claimed. +func TestCloudFrontOAC_AnUnusedAccountAnswersNoItemsElement(t *testing.T) { + p, ctx := setupCloudFrontPlugin(t) + + resp, err := p.HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath, nil, "")) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + + body := string(resp.Body) + assert.Contains(t, body, "") + assert.Contains(t, body, "0") + assert.NotContains(t, body, "", "an account using no OACs answers no Items element") +} + +// TestCloudFrontOAC_ListReportsWhatWasCreated covers the other side: once a control exists, the +// list carries a summary for it — flattened, not a nested config. +func TestCloudFrontOAC_ListReportsWhatWasCreated(t *testing.T) { + p, ctx := setupCloudFrontPlugin(t) + + first, _ := cfCreateOAC(t, p, ctx, cfOACConfigXML) + second, _ := cfCreateOAC(t, p, ctx, strings.Replace(cfOACConfigXML, + "substrate-oac", "substrate-oac-2", 1)) + require.NotEqual(t, first.ID, second.ID, "two creates mint two IDs") + + resp, err := p.HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath, nil, "")) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + + var list struct { + XMLName xml.Name `xml:"OriginAccessControlList"` + IsTruncated bool `xml:"IsTruncated"` + Quantity int `xml:"Quantity"` + Items []struct { + ID string `xml:"Id"` + Name string `xml:"Name"` + Description string `xml:"Description"` + OriginType string `xml:"OriginAccessControlOriginType"` + SigningBehavior string `xml:"SigningBehavior"` + SigningProtocol string `xml:"SigningProtocol"` + } `xml:"Items>OriginAccessControlSummary"` + } + require.NoError(t, xml.Unmarshal(resp.Body, &list), "body=%s", resp.Body) + + assert.False(t, list.IsTruncated, "the list is answered whole") + assert.Equal(t, 2, list.Quantity) + require.Len(t, list.Items, 2) + + byID := map[string]string{} + for _, item := range list.Items { + byID[item.ID] = item.Name + assert.Equal(t, "s3", item.OriginType, "a summary carries the config's members") + assert.Equal(t, "always", item.SigningBehavior) + assert.Equal(t, "sigv4", item.SigningProtocol) + assert.Equal(t, "substrate test control", item.Description) + } + assert.Equal(t, map[string]string{ + first.ID: "substrate-oac", second.ID: "substrate-oac-2", + }, byID) +} + +// TestCloudFrontOAC_AnInvalidConfigIsRefused walks the four Required: Yes members and the three +// published enums. Every refusal is InvalidArgument/400, which is the code the operation publishes +// for an invalid argument of any kind; the table exists to prove none of the seven is *accepted*, +// since a create that ignored SigningBehavior would answer 201 here and fail against AWS. +func TestCloudFrontOAC_AnInvalidConfigIsRefused(t *testing.T) { + tests := []struct { + name string + body string + }{ + { + name: "no Name", + body: strings.Replace(cfOACConfigXML, "substrate-oac", "", 1), + }, + { + name: "no OriginAccessControlOriginType", + body: strings.Replace(cfOACConfigXML, + "s3", "", 1), + }, + { + name: "no SigningBehavior", + body: strings.Replace(cfOACConfigXML, + "always", "", 1), + }, + { + name: "no SigningProtocol", + body: strings.Replace(cfOACConfigXML, + "sigv4", "", 1), + }, + { + name: "an origin type outside the enum", + body: strings.Replace(cfOACConfigXML, + "s3", + "S3", 1), + }, + { + name: "a signing behavior outside the enum", + body: strings.Replace(cfOACConfigXML, + "always", + "sometimes", 1), + }, + { + name: "a signing protocol outside the enum", + body: strings.Replace(cfOACConfigXML, + "sigv4", + "sigv2", 1), + }, + { + name: "a body of another shape entirely", + body: `wrong document`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + p, ctx := setupCloudFrontPlugin(t) + + resp, err := p.HandleRequest(ctx, cfRequest(http.MethodPost, cfOACPath, nil, tc.body)) + assert.Nil(t, resp) + + var awsErr *emulator.AWSError + require.ErrorAs(t, err, &awsErr) + assert.Equal(t, "InvalidArgument", awsErr.Code) + assert.Equal(t, http.StatusBadRequest, awsErr.HTTPStatus) + + // And nothing was recorded: a refused create must not leave a control behind. + listResp, listErr := p.HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath, nil, "")) + require.NoError(t, listErr) + assert.Contains(t, string(listResp.Body), "0") + }) + } +} + +// TestCloudFrontOAC_DeleteRequiresTheCurrentVersion is the precondition contract, and the reason +// the record stores an ETag at all. +// +// The three refusals are asserted separately because they are three different published codes for +// three different mistakes, and collapsing any two of them tells a caller something untrue: a +// missing If-Match reported as PreconditionFailed says a version was stale when none was sent, and +// a stale one reported as NoSuchOriginAccessControl says the control is gone when it is not. +func TestCloudFrontOAC_DeleteRequiresTheCurrentVersion(t *testing.T) { + p, ctx := setupCloudFrontPlugin(t) + + created, createResp := cfCreateOAC(t, p, ctx, cfOACConfigXML) + etag := createResp.Headers["ETag"] + path := cfOACPath + "/" + created.ID + + t.Run("an unknown id is NoSuchOriginAccessControl", func(t *testing.T) { + req := cfRequest(http.MethodDelete, cfOACPath+"/ENOSUCHCONTROL", nil, "") + req.Headers["If-Match"] = etag + resp, err := p.HandleRequest(ctx, req) + assert.Nil(t, resp) + + var awsErr *emulator.AWSError + require.ErrorAs(t, err, &awsErr) + assert.Equal(t, "NoSuchOriginAccessControl", awsErr.Code) + assert.Equal(t, http.StatusNotFound, awsErr.HTTPStatus) + }) + + t.Run("no If-Match is InvalidIfMatchVersion", func(t *testing.T) { + resp, err := p.HandleRequest(ctx, cfRequest(http.MethodDelete, path, nil, "")) + assert.Nil(t, resp) + + var awsErr *emulator.AWSError + require.ErrorAs(t, err, &awsErr) + assert.Equal(t, "InvalidIfMatchVersion", awsErr.Code) + assert.Equal(t, http.StatusBadRequest, awsErr.HTTPStatus) + }) + + t.Run("a stale If-Match is PreconditionFailed", func(t *testing.T) { + req := cfRequest(http.MethodDelete, path, nil, "") + req.Headers["If-Match"] = "EOUTOFDATE0000" + resp, err := p.HandleRequest(ctx, req) + assert.Nil(t, resp) + + var awsErr *emulator.AWSError + require.ErrorAs(t, err, &awsErr) + assert.Equal(t, "PreconditionFailed", awsErr.Code) + assert.Equal(t, http.StatusPreconditionFailed, awsErr.HTTPStatus) + }) + + t.Run("the current version deletes it", func(t *testing.T) { + // Quoted, and folded: the header reader is case-insensitive and a quoted ETag is + // tolerated, so a caller that re-quotes what substrate handed it still succeeds. + req := cfRequest(http.MethodDelete, path, nil, "") + req.Headers["if-match"] = `"` + etag + `"` + resp, err := p.HandleRequest(ctx, req) + require.NoError(t, err) + assert.Equal(t, http.StatusNoContent, resp.StatusCode) + assert.Empty(t, resp.Body, "the documented response has an empty body") + + getResp, getErr := p.HandleRequest(ctx, cfRequest(http.MethodGet, path, nil, "")) + assert.Nil(t, getResp) + var awsErr *emulator.AWSError + require.ErrorAs(t, getErr, &awsErr) + assert.Equal(t, "NoSuchOriginAccessControl", awsErr.Code) + + listResp, listErr := p.HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath, nil, "")) + require.NoError(t, listErr) + assert.NotContains(t, string(listResp.Body), created.ID, + "a deleted control leaves the account's index too") + }) +} + +// TestCloudFrontOAC_ControlsAreScopedToTheCallingAccount pins that the record is read back under +// the account that created it: another account's get answers the absence, not the control. +func TestCloudFrontOAC_ControlsAreScopedToTheCallingAccount(t *testing.T) { + p, ctx := setupCloudFrontPlugin(t) + created, _ := cfCreateOAC(t, p, ctx, cfOACConfigXML) + + other := &emulator.RequestContext{AccountID: "999988887777", Region: "us-east-1", RequestID: "req-2"} + + resp, err := p.HandleRequest(other, cfRequest(http.MethodGet, cfOACPath+"/"+created.ID, nil, "")) + assert.Nil(t, resp) + var awsErr *emulator.AWSError + require.ErrorAs(t, err, &awsErr) + assert.Equal(t, "NoSuchOriginAccessControl", awsErr.Code) + + listResp, listErr := p.HandleRequest(other, cfRequest(http.MethodGet, cfOACPath, nil, "")) + require.NoError(t, listErr) + assert.NotContains(t, string(listResp.Body), created.ID) +} + +// TestCloudFrontOAC_AnUnpublishedMethodIsRefused pins that the path family does not become a +// catch-all: a PUT on a control — which would be UpdateOriginAccessControl, an operation substrate +// does not implement — is refused rather than resolving to one of the four. +func TestCloudFrontOAC_AnUnpublishedMethodIsRefused(t *testing.T) { + p, ctx := setupCloudFrontPlugin(t) + created, _ := cfCreateOAC(t, p, ctx, cfOACConfigXML) + + resp, err := p.HandleRequest(ctx, + cfRequest(http.MethodPut, cfOACPath+"/"+created.ID, nil, cfOACConfigXML)) + assert.Nil(t, resp) + require.Error(t, err, "an unimplemented operation is refused, not silently routed") +} + +// TestCloudFront_CreateDistributionWithTagsTagsTheDistributionItCreates is the assertion a +// consumer's teardown verification makes: the tags sent with the create are readable through +// ListTagsForResource afterwards. +// +// One call, not two — which is why it is worth a test. The operation is documented as requiring +// both the CreateDistribution and the TagResource permission, and a substrate that routed +// `?WithTags` to the plain create would answer the same 201 while storing no tags at all. +func TestCloudFront_CreateDistributionWithTagsTagsTheDistributionItCreates(t *testing.T) { + p, ctx := setupCloudFrontPlugin(t) + + body := `` + + cfDistributionConfigXML + + `` + + `envtest` + + `ownersubstrate` + + `` + + `` + + resp, err := p.HandleRequest(ctx, cfRequest(http.MethodPost, "/2020-05-31/distribution", + map[string]string{"WithTags": "1"}, body)) + require.NoError(t, err) + require.Equal(t, http.StatusCreated, resp.StatusCode, "body=%s", resp.Body) + + var dist struct { + XMLName xml.Name `xml:"Distribution"` + ID string `xml:"Id"` + ARN string `xml:"ARN"` + } + require.NoError(t, xml.Unmarshal(resp.Body, &dist)) + require.NotEmpty(t, dist.ID, "the tagged create answers the same Distribution document") + + tagsResp, err := p.HandleRequest(ctx, cfRequest(http.MethodGet, "/2020-05-31/tagging", + map[string]string{"Resource": dist.ARN}, "")) + require.NoError(t, err) + require.Equal(t, http.StatusOK, tagsResp.StatusCode) + + var tags struct { + XMLName xml.Name `xml:"Tags"` + Items []struct { + Key string `xml:"Key"` + Value string `xml:"Value"` + } `xml:"Items>Tag"` + } + require.NoError(t, xml.Unmarshal(tagsResp.Body, &tags), "body=%s", tagsResp.Body) + + got := map[string]string{} + for _, tag := range tags.Items { + got[tag.Key] = tag.Value + } + assert.Equal(t, map[string]string{"env": "test", "owner": "substrate"}, got) +} + +// TestCloudFront_CreateDistributionWithTagsRefusesABodyItCannotRead is the other half of the same +// operation, and the one #883's argument applies to: a caller that asked for tags must not be +// answered 201 by a create that dropped them. +func TestCloudFront_CreateDistributionWithTagsRefusesABodyItCannotRead(t *testing.T) { + p, ctx := setupCloudFrontPlugin(t) + + resp, err := p.HandleRequest(ctx, cfRequest(http.MethodPost, "/2020-05-31/distribution", + map[string]string{"WithTags": "1"}, ``)) + assert.Nil(t, resp) + + var awsErr *emulator.AWSError + require.ErrorAs(t, err, &awsErr) + assert.Equal(t, "InvalidArgument", awsErr.Code) + assert.Equal(t, http.StatusBadRequest, awsErr.HTTPStatus) + + listResp, listErr := p.HandleRequest(ctx, + cfRequest(http.MethodGet, "/2020-05-31/distribution", nil, "")) + require.NoError(t, listErr) + assert.Contains(t, string(listResp.Body), "0", + "a refused tagged create leaves no distribution behind") +} + +// TestReplay_ACloudFrontCreateReplaysWithTheIdentifiersItMinted is #856's claim for CloudFront: +// the distribution ID, the invalidation ID and the origin access control's ID and ETag all come +// from the request's own mint, so a recorded stream replays byte-identically. +// +// The stream interlocks deliberately. The invalidation is created *inside* the recorded +// distribution and the delete names the recorded control's ETag, so a single re-minted value is +// not a cosmetic difference: the request naming it answers NoSuchDistribution or +// PreconditionFailed instead of succeeding. +func TestReplay_ACloudFrontCreateReplaysWithTheIdentifiersItMinted(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") + + // Frozen for the reason the EC2 and shared-mint replay tests freeze: a replay pins the clock + // to the recorded event's timestamp, and a handler stamping a record off a live clock + // diverges in a state hash for a reason that has nothing to do with an identifier. + ts.FreezeTime() + + var created cfOACDoc + createBody, createHeaders := cfReplayCall(t, ts, http.MethodPost, cfOACPath, cfOACConfigXML) + require.NoError(t, xml.Unmarshal(createBody, &created)) + require.NotEmpty(t, created.ID) + etag := createHeaders.Get("ETag") + require.NotEmpty(t, etag, "the delete below needs the version the create handed out") + + cfReplayCall(t, ts, http.MethodGet, cfOACPath+"/"+created.ID, "") + + var dist struct { + ID string `xml:"Id"` + } + distBody, _ := cfReplayCall(t, ts, http.MethodPost, "/2020-05-31/distribution", + cfDistributionConfigXML) + require.NoError(t, xml.Unmarshal(distBody, &dist)) + require.NotEmpty(t, dist.ID) + + // An invalidation inside the distribution the previous request minted. + cfReplayCall(t, ts, http.MethodPost, + "/2020-05-31/distribution/"+dist.ID+"/invalidation", "") + + // And a delete that names the version the create handed out. + cfReplayDelete(t, ts, cfOACPath+"/"+created.ID, etag) + + 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 creates") + assert.Equal(t, results.TotalEvents, results.SuccessEvents, + "every recorded request is re-executed and answers") + assert.Empty(t, results.Differences, + "a replayed CloudFront create mints the identifiers its recording minted: %s", + replayDifferenceSummary(results)) + assert.True(t, results.StateValid, + "and the state it reaches is the recorded state: %v", results.StateErrors) +} + +// TestCloudFrontOAC_AStoreFailureIsNotAPublishedRefusal covers the error paths every handler here +// carries, and the property they exist for: a state manager that cannot serve a read or a write is +// substrate's own failure, so it surfaces as a wrapped error — never as one of the operation's +// published codes. +// +// That distinction is the point. A caller told NoSuchOriginAccessControl by a broken store would +// conclude its control was deleted and move on; a caller told InvalidArgument would conclude its +// request was wrong and rewrite it. Both are wrong answers to "substrate could not read its state", +// and the assertion each case makes is that the error is not an [emulator.AWSError] at all. +func TestCloudFrontOAC_AStoreFailureIsNotAPublishedRefusal(t *testing.T) { + // The index key is "cfoac_ids:…" and a record key is "cfoac:…", so faulting on "cfoac:" + // reaches the records without disturbing the index a list has to read first. + const recordKey, indexKey = "cfoac:", "cfoac_ids:" + + tests := []struct { + name string + fault cfFaultStateManager + call func(t *testing.T, p *emulator.CloudFrontPlugin, ctx *emulator.RequestContext, oacID string) (*emulator.AWSResponse, error) + }{ + { + name: "a create whose write fails", + fault: cfFaultStateManager{failPut: recordKey}, + call: func(t *testing.T, p *emulator.CloudFrontPlugin, ctx *emulator.RequestContext, _ string) (*emulator.AWSResponse, error) { + t.Helper() + return p.HandleRequest(ctx, cfRequest(http.MethodPost, cfOACPath, nil, cfOACConfigXML)) + }, + }, + { + name: "a get whose read fails", + fault: cfFaultStateManager{failGet: recordKey}, + call: func(t *testing.T, p *emulator.CloudFrontPlugin, ctx *emulator.RequestContext, oacID string) (*emulator.AWSResponse, error) { + t.Helper() + return p.HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath+"/"+oacID, nil, "")) + }, + }, + { + name: "a get whose record does not decode", + fault: cfFaultStateManager{corruptGet: recordKey}, + call: func(t *testing.T, p *emulator.CloudFrontPlugin, ctx *emulator.RequestContext, oacID string) (*emulator.AWSResponse, error) { + t.Helper() + return p.HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath+"/"+oacID, nil, "")) + }, + }, + { + name: "a list whose index cannot be read", + fault: cfFaultStateManager{failGet: indexKey}, + call: func(t *testing.T, p *emulator.CloudFrontPlugin, ctx *emulator.RequestContext, _ string) (*emulator.AWSResponse, error) { + t.Helper() + return p.HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath, nil, "")) + }, + }, + { + name: "a delete whose removal fails", + fault: cfFaultStateManager{failDelete: recordKey}, + call: func(t *testing.T, p *emulator.CloudFrontPlugin, ctx *emulator.RequestContext, oacID string) (*emulator.AWSResponse, error) { + t.Helper() + req := cfRequest(http.MethodDelete, cfOACPath+"/"+oacID, nil, "") + req.Headers["If-Match"] = cfOACStoredETag(t, p, ctx, oacID) + return p.HandleRequest(ctx, req) + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + state := emulator.NewMemoryStateManager() + healthy := cfOACPluginOn(t, state) + ctx := &emulator.RequestContext{ + AccountID: "123456789012", Region: "us-east-1", RequestID: "req-1", + } + created, _ := cfCreateOAC(t, healthy, ctx, cfOACConfigXML) + + fault := tc.fault + fault.inner = state + resp, err := tc.call(t, cfOACPluginOn(t, &fault), ctx, created.ID) + + assert.Nil(t, resp) + require.Error(t, err) + var awsErr *emulator.AWSError + assert.NotErrorAs(t, err, &awsErr, + "a store failure is substrate's, not one of the operation's published codes") + }) + } +} + +// TestCloudFrontOAC_AnUnreadableRecordDoesNotHideTheRest is the one store fault the list answers +// *through* rather than failing on: one corrupt record must not make an account's other controls +// unfindable, which is the reading [listDistributions] already takes. +func TestCloudFrontOAC_AnUnreadableRecordDoesNotHideTheRest(t *testing.T) { + state := emulator.NewMemoryStateManager() + healthy := cfOACPluginOn(t, state) + ctx := &emulator.RequestContext{AccountID: "123456789012", Region: "us-east-1", RequestID: "req-1"} + created, _ := cfCreateOAC(t, healthy, ctx, cfOACConfigXML) + + broken := &cfFaultStateManager{inner: state, corruptGet: "cfoac:" + ctx.AccountID + "/" + created.ID} + resp, err := cfOACPluginOn(t, broken).HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath, nil, "")) + require.NoError(t, err, "the list answers despite the unreadable record") + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Contains(t, string(resp.Body), "0", + "and reports the records it could read, which here is none") +} + +// cfOACStoredETag reads back a control's current version through its own get. +func cfOACStoredETag(t *testing.T, p *emulator.CloudFrontPlugin, ctx *emulator.RequestContext, oacID string) string { + t.Helper() + resp, err := p.HandleRequest(ctx, cfRequest(http.MethodGet, cfOACPath+"/"+oacID, nil, "")) + require.NoError(t, err) + return resp.Headers["ETag"] +} + +// cfOACPluginOn builds a CloudFront plugin over the given state manager, so a test can serve one +// request from a healthy store and the next from a faulting one sharing the same records. +func cfOACPluginOn(t *testing.T, state emulator.StateManager) *emulator.CloudFrontPlugin { + t.Helper() + p := &emulator.CloudFrontPlugin{} + require.NoError(t, p.Initialize(t.Context(), emulator.PluginConfig{ + State: state, + Logger: emulator.NewDefaultLogger(slog.LevelError, false), + Options: map[string]any{"time_controller": emulator.NewTimeController(time.Now())}, + })) + return p +} + +// cfFaultStateManager is a StateManager that faults on keys containing a given substring, so each +// handler's error path can be reached without reaching for an unexported field. +// +// Keyed by substring rather than armed by call count because these handlers touch two key kinds — +// a record and the account's index — and a counter would fault whichever the implementation happened +// to read first, which is the kind of test that fails when a handler is reordered rather than when +// its behavior changes. +type cfFaultStateManager struct { + inner emulator.StateManager + failGet string + corruptGet string + failPut string + failDelete string +} + +func (m *cfFaultStateManager) Get(ctx context.Context, namespace, key string) ([]byte, error) { + if m.failGet != "" && strings.Contains(key, m.failGet) { + return nil, errCFFaultStore + } + if m.corruptGet != "" && strings.Contains(key, m.corruptGet) { + return []byte("{not json"), nil + } + return m.inner.Get(ctx, namespace, key) +} + +func (m *cfFaultStateManager) Put(ctx context.Context, namespace, key string, value []byte) error { + if m.failPut != "" && strings.Contains(key, m.failPut) { + return errCFFaultStore + } + return m.inner.Put(ctx, namespace, key, value) +} + +func (m *cfFaultStateManager) Delete(ctx context.Context, namespace, key string) error { + if m.failDelete != "" && strings.Contains(key, m.failDelete) { + return errCFFaultStore + } + return m.inner.Delete(ctx, namespace, key) +} + +func (m *cfFaultStateManager) List(ctx context.Context, namespace, prefix string) ([]string, error) { + return m.inner.List(ctx, namespace, prefix) +} + +// errCFFaultStore is what a faulting store returns: an error carrying no AWS code, so a handler +// that mistook it for a published refusal would be visible in the assertions above. +var errCFFaultStore = errors.New("cloudfront test store fault") + +// cfReplayCall issues one CloudFront REST/XML request against the test server and requires a 2xx, +// returning the body and the response headers. +func cfReplayCall(t *testing.T, ts *emulator.TestServer, method, path, body string, +) ([]byte, http.Header) { + t.Helper() + + var reader io.Reader + if body != "" { + reader = strings.NewReader(body) + } + req, err := http.NewRequestWithContext(t.Context(), method, ts.URL+path, reader) + require.NoError(t, err) + req.Host = "cloudfront.amazonaws.com" + if body != "" { + req.Header.Set("Content-Type", "application/xml") + } + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + out, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Less(t, resp.StatusCode, 300, "%s %s: %s", method, path, out) + return out, resp.Header +} + +// cfReplayDelete issues a DELETE carrying the If-Match version and requires the documented 204. +func cfReplayDelete(t *testing.T, ts *emulator.TestServer, path, etag string) { + t.Helper() + + req, err := http.NewRequestWithContext(t.Context(), http.MethodDelete, ts.URL+path, nil) + require.NoError(t, err) + req.Host = "cloudfront.amazonaws.com" + req.Header.Set("If-Match", etag) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + out, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusNoContent, resp.StatusCode, "DELETE %s: %s", path, out) +} diff --git a/emulator/cloudfront_plugin.go b/emulator/cloudfront_plugin.go index 70fdef1b..2960e1f3 100644 --- a/emulator/cloudfront_plugin.go +++ b/emulator/cloudfront_plugin.go @@ -3,7 +3,6 @@ package emulator import ( "bytes" "context" - "crypto/rand" "encoding/json" "encoding/xml" "fmt" @@ -43,29 +42,39 @@ func (p *CloudFrontPlugin) Shutdown(_ context.Context) error { return nil } // HandleRequest dispatches a CloudFront REST/XML request to the appropriate handler. // The operation is derived from the HTTP method and URL path. func (p *CloudFrontPlugin) HandleRequest(ctx *RequestContext, req *AWSRequest) (*AWSResponse, error) { - op, distID := parseCloudFrontOperation(requestMethod(req), req.Path, req.Params) + op, resourceID := parseCloudFrontOperation(requestMethod(req), req.Path, req.Params) // Handle GetInvalidation (op includes invID after colon). if strings.HasPrefix(op, "GetInvalidation:") { invID := strings.TrimPrefix(op, "GetInvalidation:") - return p.getInvalidation(ctx, distID, invID) + return p.getInvalidation(ctx, resourceID, invID) } switch op { case "CreateDistribution": return p.createDistribution(ctx, req) + case "CreateDistributionWithTags": + return p.createDistributionWithTags(ctx, req) case "GetDistribution": - return p.getDistribution(ctx, req, distID) + return p.getDistribution(ctx, req, resourceID) case "GetDistributionConfig": - return p.getDistributionConfig(ctx, req, distID) + return p.getDistributionConfig(ctx, req, resourceID) case "UpdateDistribution": - return p.updateDistribution(ctx, req, distID) + return p.updateDistribution(ctx, req, resourceID) case "DeleteDistribution": - return p.deleteDistribution(ctx, req, distID) + return p.deleteDistribution(ctx, req, resourceID) case "ListDistributions": return p.listDistributions(ctx, req) case "CreateInvalidation": - return p.createInvalidation(ctx, req, distID) + return p.createInvalidation(ctx, req, resourceID) case "ListInvalidations": - return p.listInvalidations(ctx, distID) + return p.listInvalidations(ctx, resourceID) + case "CreateOriginAccessControl": + return p.createOriginAccessControl(ctx, req) + case "GetOriginAccessControl": + return p.getOriginAccessControl(ctx, resourceID) + case "ListOriginAccessControls": + return p.listOriginAccessControls(ctx) + case "DeleteOriginAccessControl": + return p.deleteOriginAccessControl(ctx, req, resourceID) case "TagResource": return p.tagResource(ctx, req) case "UntagResource": @@ -78,8 +87,13 @@ func (p *CloudFrontPlugin) HandleRequest(ctx *RequestContext, req *AWSRequest) ( } // parseCloudFrontOperation derives the CloudFront operation name and optional -// distribution ID from the HTTP method, URL path, and query parameters. -func parseCloudFrontOperation(method, path string, params map[string]string) (op, distID string) { +// resource ID from the HTTP method, URL path, and query parameters. +// +// The second return is a distribution ID for every operation on /distribution and an origin +// access control ID for every operation on /origin-access-control; it is named resourceID rather +// than distID because the two path families now share it, and a name that says "distribution" +// would invite a caller to pass an OAC's ID into a distribution lookup. +func parseCloudFrontOperation(method, path string, params map[string]string) (op, resourceID string) { // Normalise path: strip trailing slash and leading "/2020-05-31". const apiVersion = "/2020-05-31" p2 := strings.TrimSuffix(path, "/") @@ -118,7 +132,37 @@ func parseCloudFrontOperation(method, path string, params map[string]string) (op } } + // The origin access control family, whose four operations are addressed by path shape + // alone: POST and GET on the collection, GET and DELETE on one member. + const oacPath = "/origin-access-control" + if p2 == oacPath || strings.HasPrefix(p2, oacPath+"/") { + oacID := strings.TrimPrefix(strings.TrimPrefix(p2, oacPath), "/") + switch { + case oacID == "" && method == http.MethodPost: + return "CreateOriginAccessControl", "" + case oacID == "" && method == http.MethodGet: + return "ListOriginAccessControls", "" + case method == http.MethodGet: + return "GetOriginAccessControl", oacID + case method == http.MethodDelete: + return "DeleteOriginAccessControl", oacID + } + // A method the family does not publish for this path — a PUT, which would be + // UpdateOriginAccessControl, an operation substrate does not implement — resolves to + // no operation and is refused by the default arm rather than falling through to the + // distribution parsing below. + return "", oacID + } + switch { + // CreateDistributionWithTags is the same method and path as CreateDistribution, + // distinguished only by the WithTags query parameter: the reference publishes it as + // "POST /2020-05-31/distribution?WithTags HTTP/1.1". The key carries no value, so the test + // is for its *presence* — the parser substitutes the sentinel "1" for a bare query key, + // and testing for a particular value would route a request AWS accepts to the untagged + // create, silently dropping the tags the caller sent in the same body. + case p2 == "/distribution" && method == http.MethodPost && cfHasWithTags(params): + return "CreateDistributionWithTags", "" case p2 == "/distribution" && method == http.MethodPost: return "CreateDistribution", "" case p2 == "/distribution" && method == http.MethodGet: @@ -177,38 +221,116 @@ func parseCloudFrontOperation(method, path string, params map[string]string) (op return "", id } +// cfHasWithTags reports whether a request's query string carries the WithTags key that selects +// CreateDistributionWithTags. +// +// Presence, not value: the key is published bare ("?WithTags"), and a bare key reaches a plugin as +// the sentinel value "1" (parser.go). The comparison folds case because the cost of the two +// directions is not symmetric — treating "withtags" as absent creates an untagged distribution from +// a body that asked for tags and reports 201, while treating it as present routes a request to a +// decoder that refuses anything but a DistributionConfigWithTags body. +func cfHasWithTags(params map[string]string) bool { + for key := range params { + if strings.EqualFold(key, "WithTags") { + return true + } + } + return false +} + // --- Distribution operations ------------------------------------------------ +// cfDistributionConfigBody is the part of a DistributionConfig substrate records: the two members +// CreateDistribution has always decoded. docs/services.md's "A configuration is not a +// distribution" section is the standing note on the three required members that are not here and +// on what that costs; #1271 is where the rest of the configuration starts being recorded. +// +// It is a named type rather than an anonymous struct because CreateDistributionWithTags decodes +// the same document one level down, inside DistributionConfigWithTags, and the two must decode it +// identically — a member the tagged create reads from a different element name would make the two +// creates record different distributions from the same body. +type cfDistributionConfigBody struct { + XMLName xml.Name `xml:"DistributionConfig"` + Comment string `xml:"Comment"` + Enabled string `xml:"Enabled"` +} + func (p *CloudFrontPlugin) createDistribution(ctx *RequestContext, req *AWSRequest) (*AWSResponse, error) { // Parse optional comment and enabled flag from XML body. - var xmlBody struct { - XMLName xml.Name `xml:"DistributionConfig"` - Comment string `xml:"Comment"` - Enabled string `xml:"Enabled"` - } + var xmlBody cfDistributionConfigBody if len(req.Body) > 0 { // Tolerate wrapper element names (CreateDistributionRequest, DistributionConfig). _ = xml.NewDecoder(bytes.NewReader(req.Body)).Decode(&xmlBody) } + return p.createDistributionFrom(ctx, xmlBody, nil) +} - distID, err := generateCloudFrontID() - if err != nil { - return nil, fmt.Errorf("cloudfront createDistribution generateID: %w", err) - } +// createDistributionWithTags handles POST /2020-05-31/distribution?WithTags. +// +// The body is a DistributionConfigWithTags wrapping the same DistributionConfig CreateDistribution +// takes and a Tags document in the shape TagResource takes. Both children are documented +// Required: Yes, and the operation is documented as requiring both the CreateDistribution and the +// TagResource permission — so it is one call doing the work of two, and here it is one decode +// feeding the two halves [CloudFrontPlugin.createDistributionFrom] already writes. +// +// Unlike CreateDistribution, the decode error is *not* discarded: a caller reaching this operation +// has asked for tags, and a body substrate cannot read would otherwise create an untagged +// distribution and report success — the failure mode #883 closed on the tagging path. The refusal +// is InvalidArgument/400, which the operation publishes alongside InvalidTagging/400. +func (p *CloudFrontPlugin) createDistributionWithTags(ctx *RequestContext, req *AWSRequest) (*AWSResponse, error) { + var body struct { + XMLName xml.Name `xml:"DistributionConfigWithTags"` + Config cfDistributionConfigBody `xml:"DistributionConfig"` + Tags struct { + Items []struct { + Key string `xml:"Key"` + Value string `xml:"Value"` + } `xml:"Items>Tag"` + } `xml:"Tags"` + } + if err := xml.NewDecoder(bytes.NewReader(req.Body)).Decode(&body); err != nil { + return nil, cfInvalidTagBody("DistributionConfigWithTags", err) + } + + tags := make(map[string]string, len(body.Tags.Items)) + for _, tag := range body.Tags.Items { + tags[tag.Key] = tag.Value + } + return p.createDistributionFrom(ctx, body.Config, tags) +} + +// createDistributionFrom records a distribution and answers the Distribution document both creates +// return. +// +// tags is nil for CreateDistribution and the decoded tag set for CreateDistributionWithTags. It is +// applied here rather than by a second call into the tagging path because the tags arrive with the +// create: writing the record and then tagging it would make a distribution observable untagged +// between the two writes, and would answer the create's 201 with the tagging's own errors still +// ahead of it. +func (p *CloudFrontPlugin) createDistributionFrom(ctx *RequestContext, xmlBody cfDistributionConfigBody, tags map[string]string) (*AWSResponse, error) { + distID := generateCloudFrontID(ctx.IDs) enabled := !strings.EqualFold(xmlBody.Enabled, "false") arn := fmt.Sprintf("arn:aws:cloudfront::%s:distribution/%s", ctx.AccountID, distID) domainName := distID + ".cloudfront.net" now := p.tc.Now() + if tags == nil { + tags = map[string]string{} + } dist := CloudFrontDistribution{ - ID: distID, - ARN: arn, - Status: "Deployed", - DomainName: domainName, - Comment: xmlBody.Comment, - Enabled: enabled, - Tags: map[string]string{}, + ID: distID, + ARN: arn, + Status: "Deployed", + DomainName: domainName, + Comment: xmlBody.Comment, + Enabled: enabled, + Tags: tags, + // EverTagged is set from the create's own tags, so that a distribution created with + // tags and then untagged to empty is still distinguishable from one that was never + // tagged — which is the distinction [taggingEverTagged] exists to preserve and which + // the Resource Groups Tagging API's GetResources reads. + EverTagged: taggingEverTagged(false, 0, len(tags)), CreatedTime: now, LastModifiedTime: now, AccountID: ctx.AccountID, @@ -403,12 +525,8 @@ func (p *CloudFrontPlugin) createInvalidation(ctx *RequestContext, _ *AWSRequest return nil, err } - invID, err := generateCloudFrontID() - if err != nil { - return nil, fmt.Errorf("cloudfront createInvalidation generateID: %w", err) - } // Use I prefix for invalidation IDs per CloudFront API convention. - invID = "I" + invID[1:] + invID := "I" + generateCloudFrontID(ctx.IDs)[1:] now := p.tc.Now().UTC() inv := CloudFrontInvalidation{ @@ -823,19 +941,42 @@ func (p *CloudFrontPlugin) marshalDistributionXML(dist CloudFrontDistribution) ( }) } -// generateCloudFrontID generates a CloudFront-style distribution identifier -// of the form E followed by 13 uppercase alphanumeric characters. -func generateCloudFrontID() (string, error) { - const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" - b := make([]byte, 13) - if _, err := rand.Read(b); err != nil { - return "", fmt.Errorf("generateCloudFrontID rand.Read: %w", err) - } - out := make([]byte, 13) - for i, ch := range b { - out[i] = chars[int(ch)%len(chars)] - } - return "E" + string(out), nil +// cfIDAlphabet is the alphabet every CloudFront identifier substrate mints draws from: the +// uppercase letters and the digits, as a CloudFront distribution ID is rendered. +// +// Named rather than written inline because three kinds of identifier draw from it — a +// distribution ID, an invalidation ID and an origin access control's ID and ETag — and #856's +// conversion turned the one function that held it into a mint call. A second copy would be a +// second alphabet the day someone corrected one of them. [cfnOAIIDChars] (cfn_resources_v23.go) +// is a deliberate third copy, derived for CloudFormation's own reasons and documented there. +const cfIDAlphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" + +// cfOACLocation is the prefix of the Location header CreateOriginAccessControl answers: the +// service endpoint and the operation's own path, which the new control's ID is appended to. +const cfOACLocation = "https://cloudfront.amazonaws.com/2020-05-31/origin-access-control/" + +// generateCloudFrontID mints a CloudFront-style identifier: E followed by 13 characters of +// [cfIDAlphabet]. +// +// It draws from the request's [IDMint] rather than from crypto/rand, so replaying a recorded +// CreateDistribution, CreateInvalidation or CreateOriginAccessControl mints the identifier the +// recording minted (#856). A nil or seedless mint still falls back to crypto/rand inside the +// mint, which is why there is no error to return: the byte source cannot fail in a way a caller +// here could act on, and the mapping — alphabet[b%len(alphabet)] — is the same one this function +// performed before the conversion, so an ID recorded by an earlier substrate is still the shape +// this one mints. +func generateCloudFrontID(m *IDMint) string { + return "E" + m.Chars(13, cfIDAlphabet) +} + +// cfMintETag mints the version identifier an origin access control carries in its ETag. +// +// AWS publishes no shape for a CloudFront ETag — the reference says only that it identifies the +// current version of a resource — so the rendering is substrate's: the same E-prefixed form as an +// ID, from the same mint, so that a replayed create reproduces the version its recording handed +// out and a recorded delete's If-Match still matches. +func cfMintETag(m *IDMint) string { + return generateCloudFrontID(m) } // cloudfrontXMLResponse serializes v to XML and returns an AWSResponse with diff --git a/emulator/cloudfront_tags.go b/emulator/cloudfront_tags.go index b8d9f0b3..22cbbce2 100644 --- a/emulator/cloudfront_tags.go +++ b/emulator/cloudfront_tags.go @@ -44,19 +44,22 @@ import ( // tagging. See this file's preamble for the types deliberately refused and why. const cfDistributionResourceType = "distribution" -// CloudFront state-key prefixes. The namespace holds four kinds: a distribution record, the -// per-account index of distribution IDs, an invalidation record and the per-distribution index of -// invalidation IDs. Only the distribution stores tags — the reference says so, in the sentence -// this file's preamble quotes — hence [cfKeyIsTaggable] in front of the merge. +// CloudFront state-key prefixes. The namespace holds six kinds: a distribution record, the +// per-account index of distribution IDs, an invalidation record, the per-distribution index of +// invalidation IDs, an origin access control record and the per-account index of origin access +// control IDs. Only the distribution stores tags — the reference says so, in the sentence this +// file's preamble quotes — hence [cfKeyIsTaggable] in front of the merge. // -// Every prefix is tested colon-terminated, because "cfdist" is a prefix of "cfdist_ids" and -// "cfinval" of "cfinval_ids". A bare-prefix test would report the distribution index taggable and -// merge a tags member into a JSON array of ID strings. +// Every prefix is tested colon-terminated, because "cfdist" is a prefix of "cfdist_ids", +// "cfinval" of "cfinval_ids" and "cfoac" of "cfoac_ids". A bare-prefix test would report the +// distribution index taggable and merge a tags member into a JSON array of ID strings. const ( cfDistKeyPrefix = "cfdist:" cfDistIDsKeyPrefix = "cfdist_ids:" cfInvalKeyPrefix = "cfinval:" cfInvalIDsKeyPrefix = "cfinval_ids:" + cfOACKeyPrefix = "cfoac:" + cfOACIDsKeyPrefix = "cfoac_ids:" ) // cfGlobalRegion is the Region AWS attributes a CloudFront distribution to when a per-Region @@ -92,9 +95,11 @@ func cfInvalIDsKey(accountID, distID string) string { // // Only the distribution does, and that is the reference's boundary rather than substrate's // convenience: "You can tag distributions, but you can't tag origin access identities or -// invalidations." The two index keys store no tags either, and no ARN addresses one. +// invalidations." An origin access control is outside the list of taggable resource types too — +// it publishes no tags member on create and no ARN at all — and the index keys store no tags +// either, nor does any ARN address one. // -// A single positive test rather than an enumeration of the three refusals, which is the safe +// A single positive test rather than an enumeration of the five refusals, which is the safe // direction: a key kind added later is refused by default, and refusing is the conservative // outcome — merging tags into a record that does not model them writes a member nothing reads and // reports success. diff --git a/emulator/ec2_types.go b/emulator/ec2_types.go index a4a16166..bac6ecc9 100644 --- a/emulator/ec2_types.go +++ b/emulator/ec2_types.go @@ -495,7 +495,7 @@ 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): 29 draw sites remain on crypto/rand, tiered by service family on the issue; +// TODO(#856): 28 draw sites remain on crypto/rand, tiered by service family on the issue; // delete this function when the last caller moves. func randomHex(n int) string { b := make([]byte, n) diff --git a/emulator/ids.go b/emulator/ids.go index 82442232..37df6b73 100644 --- a/emulator/ids.go +++ b/emulator/ids.go @@ -61,7 +61,7 @@ import ( // 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): 29 draw sites remain on crypto/rand, tiered by service family on the issue. +// TODO(#856): 28 draw sites remain on crypto/rand, tiered by service family on the issue. // IDMint mints the identifiers one request publishes, derived from that request's own id so // that replaying the request mints the same ones. From 419e62790b7aa69fe0b0d0792a97c609ebda3447 Mon Sep 17 00:00:00 2001 From: Scott Friedman <3011922+scttfrdmn@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:49:06 -0700 Subject: [PATCH 2/2] feat(#1277): a malformed If-Match is not a stale one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code's published description is "The If-Match version is missing or not valid", which is two cases, and the second was answering PreconditionFailed — telling a caller with a typo that its version was stale. Decidable only because the ETag rendering is substrate's own: a value outside the shape it mints was never handed out, so it cannot be a stale one. --- CHANGELOG.md | 11 +++++++--- docs/services.md | 24 +++++++++++++--------- emulator/cloudfront_oac.go | 36 ++++++++++++++++++++++++++------- emulator/cloudfront_oac_test.go | 16 +++++++++++++++ 4 files changed, 68 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70faf82b..1df6b4e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,9 +81,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 answers the account's controls whole, and an account using none answers **no `Items` element at all**, which is what the page states and what a decoder cannot distinguish from an empty one — so the test asserts on the raw XML. `DeleteOriginAccessControl` requires `If-Match` and separates the - three ways it can fail: an absent control is `NoSuchOriginAccessControl`/404, a *missing* version is - `InvalidIfMatchVersion`/400 and a stale one is `PreconditionFailed`/412, because telling a caller - that sent no version that its version was stale is a wrong answer. `CreateDistributionWithTags` is + ways it can fail: an absent control is `NoSuchOriginAccessControl`/404, a version that is *missing + or malformed* is `InvalidIfMatchVersion`/400 — the code's own published description is "missing or + not valid", which is two cases in one sentence — and a well-formed but stale one is + `PreconditionFailed`/412, because telling a caller that sent no version, or a typo, that its version + was stale is a wrong answer. Malformed is decidable only because the ETag rendering is substrate's + own: a value outside the shape substrate mints was never handed out here, so it cannot be a stale + one, and that is a statement about substrate's minting rather than about what CloudFront accepts. + `CreateDistributionWithTags` is the same path and verb as `CreateDistribution` with `?WithTags` — a bare query key, so the routing tests for the key's *presence*; testing for a value would have sent a tagged create to the untagged handler and dropped the tags while answering 201. Its body is decoded strictly, unlike diff --git a/docs/services.md b/docs/services.md index 9a9a1631..63c18b31 100644 --- a/docs/services.md +++ b/docs/services.md @@ -16668,7 +16668,7 @@ Kinesis shard: $0.015 per shard-hour. PUT payload: $0.014 per million 25KB units | CreateOriginAccessControl | 201 with the `ETag` and `Location` headers; the four required config members are validated against their published enums — see [The origin access control family](#the-origin-access-control-family) | | GetOriginAccessControl | 200 with the `ETag` header; absent → `NoSuchOriginAccessControl` | | ListOriginAccessControls | An account using no origin access controls answers **no `Items` element** | -| DeleteOriginAccessControl | 204. `If-Match` required: missing → `InvalidIfMatchVersion`, stale → `PreconditionFailed`. `OriginAccessControlInUse` is not answered — see below | +| DeleteOriginAccessControl | 204. `If-Match` required: missing or malformed → `InvalidIfMatchVersion`, well-formed but stale → `PreconditionFailed`. `OriginAccessControlInUse` is not answered — see below | All three tagging operations share the `POST`/`GET /2020-05-31/tagging` path and are told apart by the query string: `Operation=Tag`, `Operation=Untag`, and a `GET` carrying only `Resource`. A @@ -16703,14 +16703,20 @@ AWS refuses, which is the direction a consumer pays for with a failed live deplo The control carries an **ETag**, and it is the one version substrate models on this service. The create answers it as a header alongside `Location`, the get answers it as a header, and -`DeleteOriginAccessControl` requires it in `If-Match`: a missing header is -`InvalidIfMatchVersion`/400 and a value that is not the current one is `PreconditionFailed`/412 — -two published codes for two different mistakes, so a caller that sent no version is not told its -version was stale. The `ETag` *shape* is substrate's: AWS publishes only that the value identifies -the current version, so substrate mints the same `E`-prefixed form it mints IDs in, from the same -per-request mint, which is what makes a replayed create hand out the version its recording did. -A quoted `If-Match` is accepted as well as a bare one, since HTTP ETags are conventionally quoted -and CloudFront's are not. +`DeleteOriginAccessControl` requires it in `If-Match`, and the published description of +`InvalidIfMatchVersion` — *"The If-Match version is missing or not valid"* — is two cases in one +sentence: a **missing** header and a value that is **not a version substrate could have issued** are +both `InvalidIfMatchVersion`/400, while a well-formed version that is not the current one is +`PreconditionFailed`/412. Three published codes for three different mistakes, so a caller that sent +no version at all is not told its version was stale. + +The `ETag` *shape* is substrate's: AWS publishes only that the value identifies the current version, +so substrate mints the same `E`-prefixed form it mints IDs in, from the same per-request mint, which +is what makes a replayed create hand out the version its recording did. That is also what makes +"malformed" decidable at all — a value outside that shape was never handed out here, so it cannot be +a stale one — and it is a statement about substrate's own minting rather than a claim about what +CloudFront accepts. A quoted `If-Match` is accepted as well as a bare one, since HTTP ETags are +conventionally quoted and CloudFront's are not. `ListOriginAccessControls` answers the whole list, and an account using none answers **no `Items` element at all** rather than an empty one — the page states exactly that, and it is the difference diff --git a/emulator/cloudfront_oac.go b/emulator/cloudfront_oac.go index e6b74e70..c2114e28 100644 --- a/emulator/cloudfront_oac.go +++ b/emulator/cloudfront_oac.go @@ -255,12 +255,22 @@ func (p *CloudFrontPlugin) listOriginAccessControls(ctx *RequestContext) (*AWSRe // deleteOriginAccessControl handles DELETE /2020-05-31/origin-access-control/{Id}. // -// The three refusals are three different failures and the reference gives each its own code: an -// absent control is NoSuchOriginAccessControl/404, a missing If-Match is -// InvalidIfMatchVersion/400 ("The If-Match version is missing or not valid"), and a version that -// is present but not the current one is PreconditionFailed/412. Answering the precondition -// failure for a *missing* header would tell a caller its version was stale when it never sent -// one. +// The refusals are four different failures and the reference gives each its own code. An absent +// control is NoSuchOriginAccessControl/404. A missing or malformed If-Match is +// InvalidIfMatchVersion/400 — the reference's own description is "The If-Match version is missing +// or not valid", which is two cases in one sentence. A version that is well formed but not the +// current one is PreconditionFailed/412. +// +// The distinctions are all load-bearing in the same direction: answering the precondition failure +// for a *missing* header would tell a caller its version was stale when it never sent one, and +// answering it for a value substrate could not have issued would say the same of a typo. +// +// "Malformed" is decidable here only because the ETag rendering is substrate's own (see +// [CloudFrontOriginAccessControl]): a value that is not E followed by thirteen characters of +// [cfIDAlphabet] is not a version this emulator ever handed out, so it cannot be a stale one. The +// shape test is therefore a statement about substrate's own minting and not a claim about what +// CloudFront accepts; either way the request is refused, so the reading costs a caller nothing but +// the code it reads. // // The absence is checked before the header, so a caller deleting an already-deleted control is // told that rather than being asked for a version of something that is gone. @@ -273,7 +283,7 @@ func (p *CloudFrontPlugin) deleteOriginAccessControl(ctx *RequestContext, req *A // A quoted value is tolerated: HTTP ETags are conventionally quoted, CloudFront's are not, // and a caller that re-quotes the value substrate handed it means the version it was given. ifMatch := strings.Trim(strings.TrimSpace(headerValueFold(req.Headers, "If-Match")), `"`) - if ifMatch == "" { + if !cfIsMintedVersion(ifMatch) { return nil, &AWSError{ Code: "InvalidIfMatchVersion", Message: "The If-Match version is missing or not valid for the resource.", @@ -297,6 +307,18 @@ func (p *CloudFrontPlugin) deleteOriginAccessControl(ctx *RequestContext, req *A return &AWSResponse{StatusCode: http.StatusNoContent, Headers: map[string]string{}, Body: nil}, nil } +// cfIsMintedVersion reports whether a value has the shape [cfMintETag] produces: E followed by +// thirteen characters of [cfIDAlphabet]. An empty value is not one, which is what makes the single +// test cover both halves of "missing or not valid". +func cfIsMintedVersion(version string) bool { + if len(version) != 14 || version[0] != 'E' { + return false + } + return strings.IndexFunc(version[1:], func(r rune) bool { + return !strings.ContainsRune(cfIDAlphabet, r) + }) < 0 +} + // cfOACWireFrom maps a stored control onto the OriginAccessControl document, for [cfOACResponse]. func cfOACWireFrom(oac CloudFrontOriginAccessControl) cfOACWire { return cfOACWire{ diff --git a/emulator/cloudfront_oac_test.go b/emulator/cloudfront_oac_test.go index aa8f2abb..ba78c5c4 100644 --- a/emulator/cloudfront_oac_test.go +++ b/emulator/cloudfront_oac_test.go @@ -269,6 +269,22 @@ func TestCloudFrontOAC_DeleteRequiresTheCurrentVersion(t *testing.T) { assert.Equal(t, http.StatusBadRequest, awsErr.HTTPStatus) }) + t.Run("a malformed If-Match is InvalidIfMatchVersion", func(t *testing.T) { + // Not a version substrate could have issued, so it is "not valid" rather than stale — + // the other half of the code's published description. + for _, bad := range []string{"not-a-version", "E", "Etoolowercase0", "EWAYTOOLONGAVERSION"} { + req := cfRequest(http.MethodDelete, path, nil, "") + req.Headers["If-Match"] = bad + resp, err := p.HandleRequest(ctx, req) + assert.Nil(t, resp, "If-Match %q", bad) + + var awsErr *emulator.AWSError + require.ErrorAs(t, err, &awsErr, "If-Match %q", bad) + assert.Equal(t, "InvalidIfMatchVersion", awsErr.Code, "If-Match %q", bad) + assert.Equal(t, http.StatusBadRequest, awsErr.HTTPStatus) + } + }) + t.Run("a stale If-Match is PreconditionFailed", func(t *testing.T) { req := cfRequest(http.MethodDelete, path, nil, "") req.Headers["If-Match"] = "EOUTOFDATE0000"