From 7d5b6dedbdb35ff450ac0f0476fe0c4cfc9984a6 Mon Sep 17 00:00:00 2001 From: Scott Friedman <3011922+scttfrdmn@users.noreply.github.com> Date: Sat, 26 Sep 2026 01:08:25 -0700 Subject: [PATCH] feat(#1273): the CloudWatch Logs tagging trio, and the ARN it takes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TagResource was not mis-validating its resourceArn — it was absent, along with UntagResource and ListTagsForResource, so the request was refused as an unknown action. The trio therefore arrives with the rule the issue was filed for: the operations take the unsuffixed log-group ARN and refuse the IAM-policy form with the trailing ":*". Substrate hands a caller the refused form itself. DescribeLogGroups reports both ARNs and the suffixed one travels under the shorter member name `arn`, so reaching for it is the natural mistake — and a fake that accepts any ARN keeps a tagging path green offline and then fails on every log group of a live deploy. The rule applies to the read as well as the two writes, because a convergence path reads before it writes: a ListTagsForResource that accepted the suffixed form would report a group untagged rather than naming the ARN to send. ValidationException / Invalid resourceArn is observed rather than published — no page for the three lists a ValidationException at all — but what is published supports refusing: resourceArn carries Pattern [\w+=/:,.@-], a class with no "*" in it, so the suffixed form violates the request model before any resource is resolved. Only the code and message text come from observation. CreateLogGroup's inline tags are now persisted and read back; they were decoded and discarded, which is the half of the convergence path that looked like it worked. The account and Region come from the ARN and never from the caller's context (#826), which is why the three handlers are the only ones in the plugin that take no request context at all. Closes #1273. Refs #1274. --- CHANGELOG.md | 29 ++ docs/services.md | 64 ++- emulator/cloudwatchlogs_plugin.go | 13 +- emulator/cloudwatchlogs_tags.go | 289 ++++++++++++++ emulator/cloudwatchlogs_tags_test.go | 567 +++++++++++++++++++++++++++ emulator/cloudwatchlogs_types.go | 9 + 6 files changed, 969 insertions(+), 2 deletions(-) create mode 100644 emulator/cloudwatchlogs_tags.go create mode 100644 emulator/cloudwatchlogs_tags_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 1df6b4e6..2712856c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,35 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `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. +- **CloudWatch Logs tagging: `TagResource`, `UntagResource` and `ListTagsForResource`, and the ARN + rule that decides which ARN they take** (#1273). The issue reports a validation refusal on + `TagResource`; the operation was absent, so the request was refused as an unknown action rather + than accepted with the wrong ARN, and the trio arrives with the rule because a rule on an + unroutable operation is untestable. The rule is that the operations take the **unsuffixed** + log-group ARN and refuse the IAM-policy form with the trailing `:*`. Substrate hands a caller the + refused form itself — `DescribeLogGroups` reports both, and the suffixed one travels under the + shorter member name `arn` — so reaching for it is the natural mistake, and a fake that accepts any + ARN keeps a tagging path green offline and then fails on every log group of a live deploy. The rule + is applied to the read as well as to the two writes: a convergence path reads before it writes, and + a `ListTagsForResource` that accepted the suffixed form would report a group untagged rather than + naming the ARN to send. The refusal is `ValidationException` / `Invalid resourceArn`, which is + **observed** behavior rather than published — none of the three pages lists a `ValidationException` + at all — though what is published supports refusing: `resourceArn` carries Pattern `[\w+=/:,.@-]*`, + a character class with no `*` in it, so the suffixed form violates the request model before any + resource is resolved. Only the code and the message text come from observation. A `:log-stream:` + suffix is refused the same way; a `destination:` ARN — the + other type all three pages publish as taggable — answers `ResourceNotFoundException`, because + substrate models no destinations, so it is the resource that is absent rather than the ARN that is + wrong. `CreateLogGroup`'s inline `tags` are now persisted and read back, having been decoded and + discarded before, which is the half of the convergence path that looked like it worked. `tags` is a + string-to-string map on both the request and the response, not the `{key, value}` array most + services use, and an untagged group answers `{"tags":{}}` rather than omitting the member. The + 50-tag ceiling is enforced at `TagResource` against the *merged* set, since a one-tag request + against a group holding fifty exceeds it while the request alone does not; `CreateLogGroup` is left + without it, because its page publishes no code for exceeding the `tags` maximum and + `TooManyTagsException` is published on `TagResource` alone (#671). The account and Region come from + the ARN and never from the caller's context — #826's rule, which is why the three handlers are the + only ones in the plugin that take no request context at all. ### Changed diff --git a/docs/services.md b/docs/services.md index 63c18b31..dd989ff4 100644 --- a/docs/services.md +++ b/docs/services.md @@ -14478,6 +14478,9 @@ KMS API requests: $0.03 per 10,000 requests. | PutLogEvents | Accepts up to 10,000 events per call | | GetLogEvents | Issues both `nextForwardToken` and `nextBackwardToken`; reads `startFromHead`; refuses a `nextToken` it did not issue | | FilterLogEvents | Substring match on `filterPattern`; reports `searchedLogStreams`; refuses a `nextToken` it did not issue | +| TagResource | Takes the **unsuffixed** log-group ARN — see below; refuses the 51st tag | +| UntagResource | Same ARN rule; an absent key and an empty `tagKeys` are both no-ops | +| ListTagsForResource | Same ARN rule; an untagged group answers `{"tags":{}}` | Lambda auto-creates `/aws/lambda/{name}` log groups. @@ -14494,7 +14497,9 @@ field read raised `KeyError`. All four now emit the API's member names. `DescribeLogGroups` reports **both** ARN forms the reference documents as distinct members: `logGroupArn` without a trailing `:*`, which is what a `logGroupIdentifier` input or a tagging API wants, and `arn` with it, which is -what an IAM policy wants for most actions. They differ only in that suffix. +what an IAM policy wants for most actions. They differ only in that suffix, and +which of the two the tagging operations accept is [a rule of its +own](#tagging-a-log-group-takes-the-unsuffixed-arn). A group with no retention policy omits `retentionInDays` entirely rather than reporting `0`, because the API has no value meaning "never" — the member's absence @@ -14520,6 +14525,63 @@ for the provenance, the one divergence the change deliberately leaves in place ( 24-hour token expiry, which is declined rather than deferred), and the argument that the token is validated before any listing is read. +### Tagging a log group takes the unsuffixed ARN + +The three tagging operations accept the **unsuffixed** log-group ARN and refuse the +IAM-policy form: + +``` +arn:aws:logs:us-west-2:123456789012:log-group:/aws/lambda/foray-gateway accepted +arn:aws:logs:us-west-2:123456789012:log-group:/aws/lambda/foray-gateway:* ValidationException: Invalid resourceArn +``` + +The trap is that substrate hands a caller the refused form itself: as the section +above records, `DescribeLogGroups` reports both, and the suffixed one travels under +the shorter member name `arn`. Reaching for it is the natural mistake, since it is +also the ARN an IAM policy wants, and a hand-written fake that accepts any ARN keeps +a tagging path green offline and then fails on every group of a live deploy — which +is the report [#1273](https://github.com/scttfrdmn/substrate/issues/1273) was filed +from. + +The same rule applies to all three operations, not to the write alone: a convergence +path reads before it writes, and a `ListTagsForResource` that accepted the suffixed +form would report a group as untagged rather than telling the caller which ARN to +send. + +**Provenance.** `ValidationException` / `Invalid resourceArn` is **observed** real-AWS +behaviour (us-west-2), not published: the pages for all three operations list +`InvalidParameterException`/400, `ResourceNotFoundException`/400 and +`ServiceUnavailableException`/500, plus `TooManyTagsException`/400 on `TagResource`, +and none of them lists a `ValidationException` at all. What *is* published supports +refusing — `resourceArn` carries Pattern `[\w+=/:,.@-]*`, a character class with no +`*` in it, so the suffixed form violates the request model before any resource is +resolved. Only the code and the message text come from observation. + +A `:log-stream:{name}` suffix is refused the same way, since a stream is not one of +the two types these operations accept. The other type that *is* accepted — +`destination:{name}` — answers `ResourceNotFoundException`, because substrate models +no destinations at all: the ARN names a taggable type and a resource that cannot +exist here, so it is the resource that is missing rather than the ARN that is wrong. + +Three further readings worth knowing: + +- `tags` is a **string-to-string map** on both the `TagResource` request and the + `ListTagsForResource` response, not the array of `{key, value}` objects most + services use. An untagged group answers `{"tags":{}}` rather than omitting the + member, so a caller comparing maps never has to tell nil from empty. +- `CreateLogGroup`'s inline `tags` are persisted and read back by + `ListTagsForResource`. They were decoded and discarded before #1273, so a group + created with tags inline reported none. +- The 50-tag ceiling is enforced at `TagResource` only, against the **merged** set — + a request of one tag against a group already holding fifty is refused with + `TooManyTagsException`. `CreateLogGroup` accepts more, because its page publishes + no error code for exceeding the `tags` map maximum and inventing one would assert a + refusal AWS documents nowhere ([#671](https://github.com/scttfrdmn/substrate/issues/671)). + +Note also what tagging does **not** reach: a log group is not resolvable through the +Resource Groups Tagging API, which has no `logs` arm in its ARN resolver, so +`GetResources` does not report one. + ### GetLogEvents pages by a pair of tokens `GetLogEvents` is the one Logs paginator whose response carries **two** tokens, and whose termination diff --git a/emulator/cloudwatchlogs_plugin.go b/emulator/cloudwatchlogs_plugin.go index 50da1349..d76652cc 100644 --- a/emulator/cloudwatchlogs_plugin.go +++ b/emulator/cloudwatchlogs_plugin.go @@ -14,7 +14,8 @@ import ( // CloudWatchLogsPlugin emulates the Amazon CloudWatch Logs JSON-protocol API. // It handles CreateLogGroup, DeleteLogGroup, DescribeLogGroups, // PutRetentionPolicy, DeleteRetentionPolicy, CreateLogStream, DeleteLogStream, -// DescribeLogStreams, PutLogEvents, GetLogEvents, and FilterLogEvents. +// DescribeLogStreams, PutLogEvents, GetLogEvents, FilterLogEvents, TagResource, +// UntagResource, and ListTagsForResource. type CloudWatchLogsPlugin struct { state StateManager logger Logger @@ -65,6 +66,12 @@ func (p *CloudWatchLogsPlugin) HandleRequest(ctx *RequestContext, req *AWSReques return p.getLogEvents(ctx, req) case "FilterLogEvents": return p.filterLogEvents(ctx, req) + case "TagResource": + return p.tagResource(req) + case "UntagResource": + return p.untagResource(req) + case "ListTagsForResource": + return p.listTagsForResource(req) default: return nil, unknownActionError(p.Name(), req.Operation) } @@ -95,11 +102,15 @@ func (p *CloudWatchLogsPlugin) createLogGroup(ctx *RequestContext, req *AWSReque return nil, &AWSError{Code: "ResourceAlreadyExistsException", Message: "Log group already exists: " + body.LogGroupName, HTTPStatus: http.StatusConflict} } + // The tags the request carries are persisted, so ListTagsForResource reports them. They were + // decoded and dropped before #1273 added the tagging trio: a group created with tags inline + // read back as untagged, which is the half of the convergence path that looked like it worked. lg := CWLogGroup{ LogGroupName: body.LogGroupName, ARN: cwLogGroupARN(ctx.Region, ctx.AccountID, body.LogGroupName), CreationTime: p.tc.Now().UnixMilli(), RetentionInDays: body.RetentionInDays, + Tags: body.Tags, } data, err := json.Marshal(lg) if err != nil { diff --git a/emulator/cloudwatchlogs_tags.go b/emulator/cloudwatchlogs_tags.go new file mode 100644 index 00000000..82361627 --- /dev/null +++ b/emulator/cloudwatchlogs_tags.go @@ -0,0 +1,289 @@ +package emulator + +// CloudWatch Logs tagging: TagResource, UntagResource, ListTagsForResource, and the one ARN rule +// that decides which ARN a caller may name. +// +// The issue this closes (#1273) reports a validation refusal on TagResource, but the operation was +// **absent**: the plugin routed eleven operations and none of the three tagging ones, so a request +// was refused as an unknown action rather than accepted with the wrong ARN. The trio and the ARN +// rule therefore land together — there is no half of this worth shipping alone, since a rule on an +// unroutable operation is untestable and an operation without the rule is the fake that caused the +// report. +// +// # The ARN a caller reuses is the wrong one +// +// A log group has two published ARN forms and they differ only in a suffix, which +// cloudwatchlogs_types.go already models: `logGroupArn` is bare and `arn` carries a trailing `:*`. +// The suffixed form is what an IAM policy wants, because it matches the group's streams, and it is +// what DescribeLogGroups reports under `arn` — so substrate itself hands a caller the value these +// three operations refuse. That is the whole trap: reusing the policy ARN for the API is a natural +// mistake and the emulator has to make it, or a consumer's tagging path stays green offline and +// fails on every group of a live deploy. +// +// # Provenance of the refusal +// +// The code and message are **observed**, not published. None of the three pages lists a +// ValidationException at all — their Errors are InvalidParameterException/400, +// ResourceNotFoundException/400, ServiceUnavailableException/500, plus TooManyTagsException/400 on +// TagResource — and the reporter saw `ValidationException: Invalid resourceArn` from us-west-2. +// +// What *is* published supports refusing: `resourceArn` carries Pattern `[\w+=/:,.@-]*`, a character +// class that contains no `*`, so the suffixed form violates the request model before any resource +// is resolved. So the refusal is derivable from the model and only its code and message text come +// from observation, which is the split docs/services.md labels and a release note reports under +// Provenance. +// +// # The taggable set, and the type substrate cannot hold +// +// All three pages name the same two types: `log-group:{name}` and `destination:{name}`. Substrate +// models no destinations — the plugin routes neither PutDestination nor DescribeDestinations — so a +// well-formed destination ARN names a type AWS publishes as taggable and a resource that cannot +// exist here. It answers ResourceNotFoundException, whose published gloss ("The specified resource +// does not exist.") is exactly true of it, rather than the ARN refusal: the ARN is not the thing +// that is wrong. +// +// # tags is a map, not an array of Tag +// +// Both the TagResource request and the ListTagsForResource response carry `tags` as a +// string-to-string map. That is not the shape most services use — Step Functions, ECS and ELB all +// take an array of {key, value} objects — and getting it wrong is the #528 failure mode for this +// service: a JSON-1.1 member that does not match the model parses to nothing rather than erroring, +// so an SDK would report a group with no tags and an HTTP 200. + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strings" +) + +// cwlMaxTagsPerResource is the number of tags a CloudWatch Logs resource may carry. +// +// All three pages state it in prose — "You can associate as many as 50 tags with a CloudWatch Logs +// resource" — and TagResource publishes TooManyTagsException for exceeding it. It is enforced at +// TagResource only; see [CloudWatchLogsPlugin.tagResource] for why CreateLogGroup does not. +const cwlMaxTagsPerResource = 50 + +// cwlLogGroupARNType is the resource-type segment of a taggable log-group ARN. +const cwlLogGroupARNType = "log-group:" + +// cwlDestinationARNType is the resource-type segment of a taggable destination ARN. Substrate +// stores no destination, so an ARN of this type resolves to nothing; see this file's preamble. +const cwlDestinationARNType = "destination:" + +// tagResource handles TagResource: it merges tags onto a log group. +// +// An existing key is replaced and a new one appended, which is the page's own description of the +// operation applied to a resource that already has tags. +// +// The 50-tag ceiling is checked against the *merged* set rather than the request, because a request +// of ten tags against a group already holding forty-five exceeds it while the request alone does +// not. CreateLogGroup performs no equivalent check: its page publishes no error code for exceeding +// the `tags` map maximum, and inventing one would assert a refusal AWS documents nowhere (#671), so +// the ceiling lives at the one operation whose page publishes the code for it. The asymmetry is +// deliberate and recorded rather than smoothed over. +func (p *CloudWatchLogsPlugin) tagResource(req *AWSRequest) (*AWSResponse, error) { + var body struct { + ResourceARN string `json:"resourceArn"` + Tags map[string]string `json:"tags"` + } + if err := json.Unmarshal(req.Body, &body); err != nil { + return nil, cwlInvalidBody() + } + // A nil map is an absent member and an empty one is `"tags": {}`. Only the first is a + // violation of Required: Yes; the page publishes no minimum entry count, so an empty map is + // a request that asks for nothing and gets it. + if body.Tags == nil { + return nil, cwlMissingMember("tags") + } + + lg, stateKey, err := p.resolveTaggedLogGroup(body.ResourceARN) + if err != nil { + return nil, err + } + + merged := mergeStringMap(lg.Tags, body.Tags, nil) + if len(merged) > cwlMaxTagsPerResource { + return nil, &AWSError{ + Code: "TooManyTagsException", + Message: fmt.Sprintf("Resource %s would have %d tags; a resource can have no more than %d.", + lg.LogGroupName, len(merged), cwlMaxTagsPerResource), + HTTPStatus: http.StatusBadRequest, + } + } + lg.Tags = merged + + if putErr := p.storeLogGroup(context.Background(), stateKey, lg, "tagResource"); putErr != nil { + return nil, putErr + } + return cwLogsJSONResponse(http.StatusOK, struct{}{}) +} + +// untagResource handles UntagResource: it removes the named keys from a log group. +// +// A key the group does not carry is not an error, and neither is an empty `tagKeys` — the member's +// Array Members constraint is "Minimum number of 0 items", so a list of none is a valid request, +// and the page publishes no code for a key that is absent. +func (p *CloudWatchLogsPlugin) untagResource(req *AWSRequest) (*AWSResponse, error) { + var body struct { + ResourceARN string `json:"resourceArn"` + TagKeys []string `json:"tagKeys"` + } + if err := json.Unmarshal(req.Body, &body); err != nil { + return nil, cwlInvalidBody() + } + if body.TagKeys == nil { + return nil, cwlMissingMember("tagKeys") + } + + lg, stateKey, err := p.resolveTaggedLogGroup(body.ResourceARN) + if err != nil { + return nil, err + } + + lg.Tags = mergeStringMap(lg.Tags, nil, body.TagKeys) + if putErr := p.storeLogGroup(context.Background(), stateKey, lg, "untagResource"); putErr != nil { + return nil, putErr + } + return cwLogsJSONResponse(http.StatusOK, struct{}{}) +} + +// listTagsForResource handles ListTagsForResource. +// +// A group with no tags answers `{"tags":{}}` rather than omitting the member: the response's only +// element is documented without a condition, and a caller converging tags reads the map and +// compares it, which an absent member turns into a nil-versus-empty distinction AWS never makes. +func (p *CloudWatchLogsPlugin) listTagsForResource(req *AWSRequest) (*AWSResponse, error) { + var body struct { + ResourceARN string `json:"resourceArn"` + } + if err := json.Unmarshal(req.Body, &body); err != nil { + return nil, cwlInvalidBody() + } + + lg, _, err := p.resolveTaggedLogGroup(body.ResourceARN) + if err != nil { + return nil, err + } + + tags := lg.Tags + if tags == nil { + tags = map[string]string{} + } + return cwLogsJSONResponse(http.StatusOK, struct { + Tags map[string]string `json:"tags"` + }{Tags: tags}) +} + +// resolveTaggedLogGroup parses a resourceArn and loads the log group it names, returning the group +// and the state key it is stored at. +// +// All three operations resolve through here, so none of them can accept an ARN another refuses — +// the property the issue asks for, since a convergence path reads before it writes and a read that +// accepted the suffixed form would report a group untagged rather than telling the caller its ARN +// was wrong. +// +// This is also why the three handlers are the only ones in the plugin that take no +// [RequestContext]: everything they need to key a record comes from the ARN, so there is no +// caller's account in scope for a later edit to reach for. Step Functions records the same +// arrangement for the same reason (stepfunctions_tags.go). +func (p *CloudWatchLogsPlugin) resolveTaggedLogGroup(resourceARN string) (CWLogGroup, string, error) { + accountID, region, name, arnErr := cwlParseLogGroupTagARN(resourceARN) + if arnErr != nil { + return CWLogGroup{}, "", arnErr + } + stateKey := cwLogGroupKey(accountID, region, name) + lg, err := p.loadLogGroup(context.Background(), stateKey, name, "resolveTaggedLogGroup") + if err != nil { + return CWLogGroup{}, "", err + } + return lg, stateKey, nil +} + +// cwlParseLogGroupTagARN parses a CloudWatch Logs resourceArn naming a log group. +// +// The account and Region come from the ARN and never from the caller's request context. That is the +// rule #826 established for SQS and DynamoDB and #845 applied across the tagging resolver: an ARN +// naming another account's log group must resolve that group or none, because resolving it against +// the caller's own account tags — or, in UntagResource's case, strips a tag from — a same-named +// group the caller never named. Taking it from the ARN makes the refusal fall out of the load: the +// key names a record that account does not have. +// +// The refusals, in the order they are decided: +// +// - An absent resourceArn is InvalidParameterException, the shape every other operation in this +// plugin uses for a missing required member. Its Length Constraints put the minimum at 1, so +// the empty string is not a candidate ARN to be judged as malformed. +// - Anything that is not a logs ARN of a taggable type — wrong service, too few segments, a +// resource type these operations do not accept, or a log-group ARN with anything after the +// name — is ValidationException / Invalid resourceArn. The trailing `:*` of the IAM-policy form +// and the `:log-stream:{name}` of a stream ARN both land here, which is the point of the rule. +// - A well-formed destination ARN resolves to no namespace substrate holds and is left to the +// caller as a not-found; see this file's preamble. +func cwlParseLogGroupTagARN(resourceARN string) (accountID, region, name string, err *AWSError) { + if resourceARN == "" { + return "", "", "", cwlMissingMember("resourceArn") + } + + // arn:aws:logs:{region}:{account}:{type}:{name} — six segments, the last of which keeps its + // own colons so a group name may contain them. + parts := strings.SplitN(resourceARN, ":", 6) + if len(parts) < 6 || parts[0] != "arn" || parts[2] != "logs" { + return "", "", "", cwlInvalidResourceARN() + } + region, accountID = parts[3], parts[4] + resource := parts[5] + + if strings.HasPrefix(resource, cwlDestinationARNType) { + // A destination is taggable at AWS and unrepresentable here: report the resource as + // absent rather than the ARN as invalid. + return "", "", "", cwLogsDestinationNotFound(strings.TrimPrefix(resource, cwlDestinationARNType)) + } + + groupName, ok := strings.CutPrefix(resource, cwlLogGroupARNType) + if !ok || groupName == "" { + return "", "", "", cwlInvalidResourceARN() + } + // A log group name cannot contain a colon — the published pattern is [\.\-_/#A-Za-z0-9]+ — + // so a remaining colon means the ARN carries a suffix past the name: the policy form's `:*`, + // or a `:log-stream:{name}` naming something these operations do not accept. + if strings.Contains(groupName, ":") { + return "", "", "", cwlInvalidResourceARN() + } + return accountID, region, groupName, nil +} + +// cwlInvalidResourceARN refuses a resourceArn that is not a taggable CloudWatch Logs ARN. +// +// The code and the message are both observed rather than published; see this file's preamble for +// the provenance and for the published pattern that makes the refusal derivable. +func cwlInvalidResourceARN() *AWSError { + return &AWSError{ + Code: "ValidationException", + Message: "Invalid resourceArn", + HTTPStatus: http.StatusBadRequest, + } +} + +// cwLogsDestinationNotFound refuses a request naming a destination, which substrate does not model. +// +// It names the destination for the reason [cwLogsGroupNotFound] names its group: the published +// gloss says nothing about which resource, and these operations accept two types. +func cwLogsDestinationNotFound(destinationName string) *AWSError { + return &AWSError{ + Code: "ResourceNotFoundException", + Message: "The specified destination does not exist: " + destinationName, + HTTPStatus: http.StatusBadRequest, + } +} + +// cwlMissingMember refuses a request that omits a required member, in the wording the plugin's +// other ten operations use for the same condition. +func cwlMissingMember(member string) *AWSError { + return &AWSError{ + Code: "InvalidParameterException", + Message: member + " is required", + HTTPStatus: http.StatusBadRequest, + } +} diff --git a/emulator/cloudwatchlogs_tags_test.go b/emulator/cloudwatchlogs_tags_test.go new file mode 100644 index 00000000..7555fb47 --- /dev/null +++ b/emulator/cloudwatchlogs_tags_test.go @@ -0,0 +1,567 @@ +package emulator_test + +// CloudWatch Logs tagging (#1273). The trio was absent, so these tests are the first to exercise +// it at all, and the rule they exist for is narrow: the ARN a caller most naturally reuses — the +// `:*`-suffixed policy form that DescribeLogGroups itself reports under `arn` — is the one the +// operations refuse. + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/scttfrdmn/substrate/emulator" +) + +const ( + // cwlTagGroup is the log group these tests tag. Named after a Lambda function because that is + // the case the issue describes: a group created explicitly so retention is set from the + // start, then tagged so a teardown sweep can find it. + cwlTagGroup = "/aws/lambda/foray-gateway" + + // cwlTagGroupARN is the form the API accepts — the bare ARN, no trailing ":*". + cwlTagGroupARN = "arn:aws:logs:us-east-1:123456789012:log-group:" + cwlTagGroup + + // cwlTagGroupPolicyARN is the IAM-policy form, which matches the group's streams and which + // these operations refuse. + cwlTagGroupPolicyARN = cwlTagGroupARN + ":*" +) + +// cwlCreateTaggedGroup creates cwlTagGroup, optionally with inline tags, and fails the test if the +// create is refused. +func cwlCreateTaggedGroup(t *testing.T, srv *emulator.Server, tags map[string]string) { + t.Helper() + body := map[string]any{"logGroupName": cwlTagGroup} + if tags != nil { + body["tags"] = tags + } + resp := cwLogsRequest(t, srv, "CreateLogGroup", body) + require.Equal(t, http.StatusOK, resp.StatusCode) +} + +// cwlListTags reads a group's tags through ListTagsForResource, requiring a 200. +func cwlListTags(t *testing.T, srv *emulator.Server, resourceARN string) map[string]string { + t.Helper() + resp := cwLogsRequest(t, srv, "ListTagsForResource", map[string]any{"resourceArn": resourceARN}) + require.Equal(t, http.StatusOK, resp.StatusCode) + var got struct { + Tags map[string]string `json:"tags"` + } + require.NoError(t, json.Unmarshal(cwLogsReadBody(t, resp), &got)) + return got.Tags +} + +// cwLogsRawRequest is cwLogsRequest over a body that is not a marshaled value — for the one case +// that needs to send bytes no marshal would produce. +func cwLogsRawRequest(t *testing.T, srv *emulator.Server, op string, body []byte) *http.Response { + t.Helper() + r := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body)) + r.Host = "logs.us-east-1.amazonaws.com" + r.Header.Set("Content-Type", "application/x-amz-json-1.1") + r.Header.Set("X-Amz-Target", "Logs_20140328."+op) + r.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIATEST1234567890/20240101/us-east-1/logs/aws4_request, SignedHeaders=host, Signature=fake") + + w := httptest.NewRecorder() + srv.ServeHTTP(w, r) + return w.Result() +} + +// cwlTag calls TagResource and hands back the response for the caller to judge. +func cwlTag(t *testing.T, srv *emulator.Server, resourceARN string, tags map[string]string) *http.Response { + t.Helper() + return cwLogsRequest(t, srv, "TagResource", map[string]any{"resourceArn": resourceARN, "tags": tags}) +} + +func TestCWLogsTags_ATaggedGroupReportsWhatItWasTagged(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, nil) + + assert.Empty(t, cwlListTags(t, srv, cwlTagGroupARN), + "a group that was never tagged reports no tags") + + resp := cwlTag(t, srv, cwlTagGroupARN, map[string]string{"Project": "foray", "Env": "dev"}) + require.Equal(t, http.StatusOK, resp.StatusCode) + + assert.Equal(t, map[string]string{"Project": "foray", "Env": "dev"}, cwlListTags(t, srv, cwlTagGroupARN)) +} + +// The response's `tags` is a JSON **object**, not an array of {key, value} pairs the way most +// services render tags. An SDK matches members against the model case-sensitively and a shape +// mismatch parses to nothing rather than erroring (#528), so the wire shape is asserted on the raw +// body and not only through a decode into the shape the test wants. +func TestCWLogsTags_TagsAreAMapOnTheWire(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, nil) + require.Equal(t, http.StatusOK, cwlTag(t, srv, cwlTagGroupARN, map[string]string{"Project": "foray"}).StatusCode) + + resp := cwLogsRequest(t, srv, "ListTagsForResource", map[string]any{"resourceArn": cwlTagGroupARN}) + require.Equal(t, http.StatusOK, resp.StatusCode) + assert.JSONEq(t, `{"tags":{"Project":"foray"}}`, string(cwLogsReadBody(t, resp))) +} + +// An untagged group answers an empty object rather than omitting the member: a caller converging +// tags compares the map it reads, and an absent member makes that a nil-versus-empty distinction +// the API never draws. +func TestCWLogsTags_AnUntaggedGroupAnswersAnEmptyMap(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, nil) + + resp := cwLogsRequest(t, srv, "ListTagsForResource", map[string]any{"resourceArn": cwlTagGroupARN}) + require.Equal(t, http.StatusOK, resp.StatusCode) + assert.JSONEq(t, `{"tags":{}}`, string(cwLogsReadBody(t, resp))) +} + +// The half of the convergence path that looked like it worked: CreateLogGroup decoded `tags` and +// dropped them, so a group created with tags inline read back untagged. +func TestCWLogsTags_TagsGivenAtCreateAreReadBack(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, map[string]string{"Project": "foray", "ManagedBy": "foray-deploy"}) + + assert.Equal(t, map[string]string{"Project": "foray", "ManagedBy": "foray-deploy"}, + cwlListTags(t, srv, cwlTagGroupARN)) +} + +func TestCWLogsTags_TagResourceReplacesAKeyAndAppendsANewOne(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, map[string]string{"Project": "foray", "Env": "dev"}) + + require.Equal(t, http.StatusOK, + cwlTag(t, srv, cwlTagGroupARN, map[string]string{"Env": "prod", "Owner": "platform"}).StatusCode) + + assert.Equal(t, map[string]string{"Project": "foray", "Env": "prod", "Owner": "platform"}, + cwlListTags(t, srv, cwlTagGroupARN)) +} + +func TestCWLogsTags_UntagResourceRemovesOnlyWhatItNames(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, map[string]string{"Project": "foray", "Env": "dev"}) + + cases := []struct { + name string + tagKeys []string + want map[string]string + }{ + { + // Array Members is "Minimum number of 0 items", so a list of none is a valid + // request that asks for nothing. + name: "an empty list removes nothing", + tagKeys: []string{}, + want: map[string]string{"Project": "foray", "Env": "dev"}, + }, + { + // The page publishes no code for a key the resource does not carry. + name: "a key the group does not carry is not an error", + tagKeys: []string{"NotThere"}, + want: map[string]string{"Project": "foray", "Env": "dev"}, + }, + { + name: "a key it does carry goes", + tagKeys: []string{"Env"}, + want: map[string]string{"Project": "foray"}, + }, + { + name: "the last key leaves an empty map, not a refusal", + tagKeys: []string{"Project"}, + want: map[string]string{}, + }, + } + // Sequential rather than independent: each case starts from the previous case's state, which + // is what makes the last one — untagging down to nothing — reachable. + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + resp := cwLogsRequest(t, srv, "UntagResource", map[string]any{ + "resourceArn": cwlTagGroupARN, + "tagKeys": tc.tagKeys, + }) + require.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, tc.want, cwlListTags(t, srv, cwlTagGroupARN)) + }) + } +} + +// The rule the issue was filed for, asserted on all three operations: a convergence path reads +// before it writes, so a read that accepted the suffixed form would report a group untagged rather +// than telling the caller which ARN to send. +func TestCWLogsTags_ThePolicyFormARNIsRefusedByEveryOperation(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, map[string]string{"Project": "foray"}) + + bodies := map[string]map[string]any{ + "TagResource": {"resourceArn": cwlTagGroupPolicyARN, "tags": map[string]string{"Env": "dev"}}, + "UntagResource": {"resourceArn": cwlTagGroupPolicyARN, "tagKeys": []string{"Project"}}, + "ListTagsForResource": {"resourceArn": cwlTagGroupPolicyARN}, + } + for op, body := range bodies { + t.Run(op, func(t *testing.T) { + resp := cwLogsRequest(t, srv, op, body) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + raw := cwLogsReadBody(t, resp) + assert.Equal(t, "ValidationException", cwLogsErrorTypeFrom(t, raw)) + assert.Contains(t, string(raw), "Invalid resourceArn") + }) + } + + // And the refusal changed nothing: the write that was refused did not half-apply. + assert.Equal(t, map[string]string{"Project": "foray"}, cwlListTags(t, srv, cwlTagGroupARN)) +} + +// The round trip that pins the trap: DescribeLogGroups reports both ARN forms, the tagging +// operations accept exactly one of them, and which is which is not guessable from the response. +func TestCWLogsTags_DescribeLogGroupsReportsBothFormsAndOnlyOneTags(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, nil) + + resp := cwLogsRequest(t, srv, "DescribeLogGroups", map[string]any{}) + require.Equal(t, http.StatusOK, resp.StatusCode) + var described struct { + LogGroups []struct { + LogGroupARN string `json:"logGroupArn"` + ARN string `json:"arn"` + } `json:"logGroups"` + } + require.NoError(t, json.Unmarshal(cwLogsReadBody(t, resp), &described)) + require.Len(t, described.LogGroups, 1) + require.Equal(t, cwlTagGroupARN, described.LogGroups[0].LogGroupARN) + require.Equal(t, cwlTagGroupPolicyARN, described.LogGroups[0].ARN) + + assert.Equal(t, http.StatusBadRequest, + cwlTag(t, srv, described.LogGroups[0].ARN, map[string]string{"Project": "foray"}).StatusCode, + "the `arn` member is the IAM-policy form and the API refuses it") + assert.Equal(t, http.StatusOK, + cwlTag(t, srv, described.LogGroups[0].LogGroupARN, map[string]string{"Project": "foray"}).StatusCode, + "the `logGroupArn` member is the form the API wants") +} + +func TestCWLogsTags_AnARNThatNamesNothingTaggableIsRefused(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, nil) + + cases := []struct { + name string + resourceARN string + wantCode string + }{ + { + // Length Constraints put the minimum at 1, so an empty value is an absent + // required member rather than a malformed ARN. + name: "an absent resourceArn is a missing member", + resourceARN: "", + wantCode: "InvalidParameterException", + }, + { + name: "a bare log group name is not an ARN", + resourceARN: cwlTagGroup, + wantCode: "ValidationException", + }, + { + name: "an ARN with too few segments", + resourceARN: "arn:aws:logs:us-east-1:123456789012", + wantCode: "ValidationException", + }, + { + name: "another service's ARN", + resourceARN: "arn:aws:lambda:us-east-1:123456789012:function:foray-gateway", + wantCode: "ValidationException", + }, + { + // A logs ARN of a type these operations do not accept. + name: "a log stream ARN", + resourceARN: cwlTagGroupARN + ":log-stream:2026/09/26/[$LATEST]abc", + wantCode: "ValidationException", + }, + { + name: "a logs ARN naming no resource type", + resourceARN: "arn:aws:logs:us-east-1:123456789012:query-definition-id", + wantCode: "ValidationException", + }, + { + name: "a log-group ARN with no name", + resourceARN: "arn:aws:logs:us-east-1:123456789012:log-group:", + wantCode: "ValidationException", + }, + { + // A destination is taggable at AWS and unrepresentable here, so the resource is + // reported absent rather than the ARN invalid — the ARN is not what is wrong. + name: "a destination ARN names a type substrate does not model", + resourceARN: "arn:aws:logs:us-east-1:123456789012:destination:foray-firehose", + wantCode: "ResourceNotFoundException", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + resp := cwLogsRequest(t, srv, "ListTagsForResource", map[string]any{"resourceArn": tc.resourceARN}) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode, + "this service reports every refusal at 400, with the code in the body's __type") + assert.Equal(t, tc.wantCode, cwLogsErrorType(t, resp)) + }) + } +} + +func TestCWLogsTags_AGroupThatDoesNotExistIsRefused(t *testing.T) { + srv := newCWLogsTestServer(t) + + resp := cwlTag(t, srv, cwlTagGroupARN, map[string]string{"Project": "foray"}) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + raw := cwLogsReadBody(t, resp) + assert.Equal(t, "ResourceNotFoundException", cwLogsErrorTypeFrom(t, raw)) + assert.Contains(t, string(raw), cwlTagGroup, "the message names the group, as the plugin's other not-founds do") +} + +// The #826 rule: the account and Region come from the ARN, so an ARN naming another account's or +// another Region's group resolves that group or none. Resolving it against the caller's own account +// would tag — and UntagResource would strip a tag from — a same-named group the caller never named. +func TestCWLogsTags_AForeignARNDoesNotReachTheCallersGroup(t *testing.T) { + srv, state := newCWLogsTestServerWithState(t) + cwlCreateTaggedGroup(t, srv, map[string]string{"Project": "foray"}) + + foreign := []struct { + name string + resourceARN string + }{ + {"another account", "arn:aws:logs:us-east-1:210987654321:log-group:" + cwlTagGroup}, + {"another Region", "arn:aws:logs:eu-west-1:123456789012:log-group:" + cwlTagGroup}, + } + for _, f := range foreign { + t.Run(f.name, func(t *testing.T) { + // The write direction first: an UntagResource that reached the caller's group + // would strip the tag a teardown sweep finds it by. + resp := cwLogsRequest(t, srv, "UntagResource", map[string]any{ + "resourceArn": f.resourceARN, + "tagKeys": []string{"Project"}, + }) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + assert.Equal(t, "ResourceNotFoundException", cwLogsErrorType(t, resp)) + + resp = cwlTag(t, srv, f.resourceARN, map[string]string{"Project": "someone-else"}) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + assert.Equal(t, "ResourceNotFoundException", cwLogsErrorType(t, resp)) + + assert.Equal(t, map[string]string{"Project": "foray"}, cwlListTags(t, srv, cwlTagGroupARN), + "the caller's own group is untouched") + }) + } + + // Nor did the refused writes leave a phantom record behind for the foreign keys — the failure + // #826 describes is a write that answers 200 against a key no group is stored at. + keys, err := state.List(t.Context(), "logs", "") + require.NoError(t, err) + assert.Equal(t, []string{ + "loggroup:123456789012/us-east-1" + "/" + cwlTagGroup, + "loggroup_names:123456789012/us-east-1", + }, keys) +} + +// TooManyTagsException is checked against the merged set, because a small request against an +// already-full group exceeds the ceiling while the request alone does not. +func TestCWLogsTags_TheFiftyFirstTagIsRefused(t *testing.T) { + srv := newCWLogsTestServer(t) + + fifty := make(map[string]string, 50) + for i := range 50 { + fifty[fmt.Sprintf("Key%02d", i)] = "v" + } + cwlCreateTaggedGroup(t, srv, fifty) + + resp := cwlTag(t, srv, cwlTagGroupARN, map[string]string{"OneTooMany": "v"}) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + assert.Equal(t, "TooManyTagsException", cwLogsErrorType(t, resp)) + assert.Len(t, cwlListTags(t, srv, cwlTagGroupARN), 50, "the refused request added nothing") + + // Replacing a value at the ceiling is not an increase, so it is allowed. + assert.Equal(t, http.StatusOK, cwlTag(t, srv, cwlTagGroupARN, map[string]string{"Key00": "w"}).StatusCode) + assert.Equal(t, "w", cwlListTags(t, srv, cwlTagGroupARN)["Key00"]) +} + +// CreateLogGroup carries no equivalent ceiling: its page publishes no code for exceeding the `tags` +// map maximum, and TooManyTagsException is published on TagResource alone, so refusing here would +// mean inventing a code (#671). The asymmetry is pinned rather than left to be read as an oversight. +func TestCWLogsTags_CreateLogGroupDoesNotEnforceTheTagCeiling(t *testing.T) { + srv := newCWLogsTestServer(t) + + tooMany := make(map[string]string, cwlTagCeilingProbe) + for i := range cwlTagCeilingProbe { + tooMany[fmt.Sprintf("Key%02d", i)] = "v" + } + cwlCreateTaggedGroup(t, srv, tooMany) + + assert.Len(t, cwlListTags(t, srv, cwlTagGroupARN), cwlTagCeilingProbe) +} + +// cwlTagCeilingProbe is one more tag than TagResource accepts. +const cwlTagCeilingProbe = 51 + +func TestCWLogsTags_ARequiredMemberIsRefusedBeforeTheARNIsResolved(t *testing.T) { + srv := newCWLogsTestServer(t) + + cases := []struct { + name string + op string + body map[string]any + }{ + // `tags` and `tagKeys` are both Required: Yes. An absent member is refused whether or + // not the ARN names anything, because there is no request to carry out. + {"TagResource without tags", "TagResource", map[string]any{"resourceArn": cwlTagGroupARN}}, + {"UntagResource without tagKeys", "UntagResource", map[string]any{"resourceArn": cwlTagGroupARN}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + resp := cwLogsRequest(t, srv, tc.op, tc.body) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + assert.Equal(t, "InvalidParameterException", cwLogsErrorType(t, resp)) + }) + } +} + +// An empty `tags` map is present, not absent, and the page publishes no minimum entry count — so it +// is a request that asks for nothing and gets it, which is a different answer from the one above. +func TestCWLogsTags_AnEmptyTagMapIsAcceptedAndChangesNothing(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, map[string]string{"Project": "foray"}) + + resp := cwlTag(t, srv, cwlTagGroupARN, map[string]string{}) + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, map[string]string{"Project": "foray"}, cwlListTags(t, srv, cwlTagGroupARN)) +} + +// Tags go with the group. A group re-created under the same name starts untagged, which is the +// observable difference between "the record was deleted" and "the name was reused". +func TestCWLogsTags_DeletingAGroupTakesItsTagsWithIt(t *testing.T) { + srv := newCWLogsTestServer(t) + cwlCreateTaggedGroup(t, srv, map[string]string{"Project": "foray"}) + + require.Equal(t, http.StatusOK, + cwLogsRequest(t, srv, "DeleteLogGroup", map[string]string{"logGroupName": cwlTagGroup}).StatusCode) + resp := cwLogsRequest(t, srv, "ListTagsForResource", map[string]any{"resourceArn": cwlTagGroupARN}) + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + assert.Equal(t, "ResourceNotFoundException", cwLogsErrorType(t, resp)) + + cwlCreateTaggedGroup(t, srv, nil) + assert.Empty(t, cwlListTags(t, srv, cwlTagGroupARN)) +} + +// DescribeLogGroups takes a *prefix*, and a caller that checks existence by calling it and treating +// any result as a match will read `/aws/lambda/foray-gateway` as existing when only +// `/aws/lambda/foray-gateway-v2` does. That is the service's own behavior and substrate reproduces +// it; the pin is here so the tagging change cannot be followed by a well-meaning edit turning the +// filter into an exact match, which would hide the trap rather than model it (#1273). +func TestCWLogsTags_DescribeLogGroupsMatchesAPrefixAndNotAName(t *testing.T) { + srv := newCWLogsTestServer(t) + resp := cwLogsRequest(t, srv, "CreateLogGroup", map[string]string{"logGroupName": cwlTagGroup + "-v2"}) + require.Equal(t, http.StatusOK, resp.StatusCode) + + resp = cwLogsRequest(t, srv, "DescribeLogGroups", map[string]string{"logGroupNamePrefix": cwlTagGroup}) + require.Equal(t, http.StatusOK, resp.StatusCode) + var described struct { + LogGroups []struct { + LogGroupName string `json:"logGroupName"` + } `json:"logGroups"` + } + require.NoError(t, json.Unmarshal(cwLogsReadBody(t, resp), &described)) + require.Len(t, described.LogGroups, 1) + assert.Equal(t, cwlTagGroup+"-v2", described.LogGroups[0].LogGroupName, + "the prefix matched a longer name, which is why existence cannot be checked this way") + + // And the group the prefix matched is not the one the tagging ARN names. + resp = cwLogsRequest(t, srv, "ListTagsForResource", map[string]any{"resourceArn": cwlTagGroupARN}) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + assert.Equal(t, "ResourceNotFoundException", cwLogsErrorType(t, resp)) +} + +// cwlFailingPutStateManager is a StateManager whose writes fail once armed, so a tagging +// operation's read-modify-*write* can be made to fail after the read has already succeeded. +type cwlFailingPutStateManager struct { + inner emulator.StateManager + fail bool +} + +func (m *cwlFailingPutStateManager) Get(ctx context.Context, namespace, key string) ([]byte, error) { + return m.inner.Get(ctx, namespace, key) +} + +func (m *cwlFailingPutStateManager) Put(ctx context.Context, namespace, key string, value []byte) error { + if m.fail { + return errCWLStoreFault + } + return m.inner.Put(ctx, namespace, key, value) +} + +func (m *cwlFailingPutStateManager) Delete(ctx context.Context, namespace, key string) error { + return m.inner.Delete(ctx, namespace, key) +} + +func (m *cwlFailingPutStateManager) List(ctx context.Context, namespace, prefix string) ([]string, error) { + return m.inner.List(ctx, namespace, prefix) +} + +// errCWLStoreFault is the store failure cwlFailingPutStateManager injects. +var errCWLStoreFault = errors.New("cwl store fault") + +// cwlPluginOn builds a logs server over a caller-supplied state manager, which +// newCWLogsTestServerWithState does not allow. +func cwlPluginOn(t *testing.T, state emulator.StateManager) *emulator.Server { + t.Helper() + cfg := emulator.DefaultConfig() + registry := emulator.NewPluginRegistry() + logger := emulator.NewDefaultLogger(slog.LevelInfo, false) + store := emulator.NewEventStore(cfg.EventStore.ToEventStoreConfig()) + tc := emulator.NewTimeController(time.Now()) + + plugin := &emulator.CloudWatchLogsPlugin{} + require.NoError(t, plugin.Initialize(t.Context(), emulator.PluginConfig{ + State: state, + Logger: logger, + Options: map[string]any{"time_controller": tc}, + })) + registry.Register(plugin) + return emulator.NewServer(*cfg, registry, store, state, tc, logger) +} + +// A write that fails in the store is not a refusal the API publishes: it is reported as a server +// error and carries no AWS error code, so a caller retrying on `TooManyTagsException` or giving up +// on `ValidationException` cannot mistake a broken store for either. +func TestCWLogsTags_AStoreFailureIsNotAPublishedRefusal(t *testing.T) { + state := &cwlFailingPutStateManager{inner: emulator.NewMemoryStateManager()} + srv := cwlPluginOn(t, state) + cwlCreateTaggedGroup(t, srv, map[string]string{"Project": "foray"}) + state.fail = true + + cases := []struct { + op string + body map[string]any + }{ + {"TagResource", map[string]any{"resourceArn": cwlTagGroupARN, "tags": map[string]string{"Env": "dev"}}}, + {"UntagResource", map[string]any{"resourceArn": cwlTagGroupARN, "tagKeys": []string{"Project"}}}, + } + for _, tc := range cases { + t.Run(tc.op, func(t *testing.T) { + resp := cwLogsRequest(t, srv, tc.op, tc.body) + assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) + assert.NotContains(t, string(cwLogsReadBody(t, resp)), "Exception", + "a store failure publishes no exception name for a caller to switch on") + }) + } +} + +// A body that is not JSON at all is refused before anything else, in the shape the plugin's other +// operations use. +func TestCWLogsTags_AnUnreadableBodyIsRefused(t *testing.T) { + srv := newCWLogsTestServer(t) + + for _, op := range []string{"TagResource", "UntagResource", "ListTagsForResource"} { + t.Run(op, func(t *testing.T) { + resp := cwLogsRawRequest(t, srv, op, []byte("{not json")) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + assert.Equal(t, "InvalidParameterException", cwLogsErrorType(t, resp)) + }) + } +} diff --git a/emulator/cloudwatchlogs_types.go b/emulator/cloudwatchlogs_types.go index 48d03f18..60d0a98f 100644 --- a/emulator/cloudwatchlogs_types.go +++ b/emulator/cloudwatchlogs_types.go @@ -18,6 +18,15 @@ type CWLogGroup struct { // RetentionInDays is the number of days to retain log events (0 = never expire). RetentionInDays int `json:"RetentionInDays,omitempty"` + + // Tags are the log group's tags, keyed by tag key. + // + // Written by CreateLogGroup's inline `tags` member and by TagResource, read by + // ListTagsForResource. PascalCase like every member above it, because this struct is the + // persisted encoding and not the wire — see the note below the type. No tags at all is a nil + // map and is omitted from the record rather than stored as an empty object; the read projects + // it back to `{}`, which is the shape the response publishes. + Tags map[string]string `json:"Tags,omitempty"` } // CWLogStream represents an emulated Amazon CloudWatch Logs log stream.