diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/dashboards/CloudWatchDashboardsService.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/dashboards/CloudWatchDashboardsService.java index 976a43f..a0f4005 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/dashboards/CloudWatchDashboardsService.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/dashboards/CloudWatchDashboardsService.java @@ -179,29 +179,82 @@ public class CloudWatchDashboardsService { } public Map listTagsForResource(String resourceArn, String region) { - return findByArn(resourceArn, region).map(Dashboard::getTags).orElse(Map.of()); + return requireDashboard(resourceArn, region).getTags(); } public void tagResource(String resourceArn, Map tags, String region) { - findByArn(resourceArn, region).ifPresent(dashboard -> { - dashboard.getTags().putAll(tags); - dashboardStore.put(key(region, dashboard.getDashboardName()), dashboard); - }); + Dashboard dashboard = requireDashboard(resourceArn, region); + dashboard.getTags().putAll(tags); + dashboardStore.put(key(region, dashboard.getDashboardName()), dashboard); } public void untagResource(String resourceArn, List tagKeys, String region) { - findByArn(resourceArn, region).ifPresent(dashboard -> { - tagKeys.forEach(dashboard.getTags()::remove); - dashboardStore.put(key(region, dashboard.getDashboardName()), dashboard); - }); + Dashboard dashboard = requireDashboard(resourceArn, region); + tagKeys.forEach(dashboard.getTags()::remove); + dashboardStore.put(key(region, dashboard.getDashboardName()), dashboard); + } + + /** + * Resolves the dashboard an ARN names, or reports that nothing does. The tag operations + * declare {@code ResourceNotFoundException}, which is a different shape from the + * {@code ResourceNotFound} that {@code GetDashboard} declares, + * so this is not the {@link #notFound(String)} used by the rest of this service. + */ + private Dashboard requireDashboard(String resourceArn, String region) { + return findByArn(resourceArn, region).orElseThrow(() -> + new AwsException("ResourceNotFoundException", + "Dashboard does not exist: " + resourceArn, 404)); } private java.util.Optional findByArn(String resourceArn, String region) { return dashboardStore.scan(k -> k.startsWith(region + "::")).stream() - .filter(d -> d.getDashboardArn() != null && d.getDashboardArn().equals(resourceArn)) + .filter(d -> namesDashboard(resourceArn, d.getDashboardArn())) .findFirst(); } + /** + * Whether {@code candidate} is an ARN for the dashboard whose own ARN is {@code stored}. + * + *

Both the region-ful and the regionless form resolve, deliberately, because the two + * disagree here and a caller may hold either. Dashboards are a global resource and AWS + * documents their ARN without a region on both the TagResource and the ListTagsForResource + * pages, {@code arn:aws:cloudwatch:::dashboard/}, while Floci + * mints a region-ful one. That divergence used to be invisible on this path: an ARN + * matching nothing was a silent success answering an empty tag map. Now that it is an + * error, a caller sending the documented ARN for a dashboard that exists would be told the + * dashboard does not, so the lookup accepts both shapes. + * + *

What Floci mints is left alone here on purpose. Clients may already hold the ARNs it + * has handed out, so bringing the minted shape into line with AWS is a separate change + * from making the lookup tolerant, and only the second one is needed to keep this path + * correct. + * + *

Only an absent region is accepted in place of the dashboard's own, so an ARN naming + * some other region is still a miss rather than resolving to a same-named dashboard + * elsewhere. + */ + private static boolean namesDashboard(String candidate, String stored) { + if (candidate == null || stored == null) { + return false; + } + return candidate.equals(stored) || candidate.equals(withoutRegion(stored)); + } + + /** + * Blanks the region field of an ARN, turning + * {@code arn:aws:cloudwatch:us-east-1:123:dashboard/x} into + * {@code arn:aws:cloudwatch::123:dashboard/x}. An ARN with fewer than six fields is + * returned unchanged, since there is no region field to blank. + */ + private static String withoutRegion(String arn) { + String[] fields = arn.split(":", 6); + if (fields.length < 6) { + return arn; + } + fields[3] = ""; + return String.join(":", fields); + } + private static String key(String region, String dashboardName) { return region + "::" + dashboardName; } diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsService.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsService.java index 38c4a24..6ea32a1 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsService.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsService.java @@ -332,35 +332,43 @@ public class CloudWatchMetricsService { LOG.infov("SetAlarmState: {0} -> {1}", alarmName, stateValue); } - public Map listTagsForResource(String resourceArn, String region) { + /** + * Resolves the alarm an ARN names, or reports that nothing does. + * + *

CloudWatch's three tag operations each declare {@code ResourceNotFoundException}, + * which is a different shape from the {@code ResourceNotFound} that {@code SetAlarmState} + * and {@code GetDashboard} declare; the SDK maps the two codes to two exception classes, + * so the tag path uses the longer one rather than the code used elsewhere in this service. + * + *

The message names the ARN rather than asserting it was an alarm, because both tag + * handlers route every ARN that is not a dashboard and not a metric stream here. That + * includes kinds AWS considers taggable and Floci does not serve, a Contributor Insights + * {@code insight-rule/} ARN being the one AWS documents; reporting that no resource + * matches the ARN is true of those, where "alarm not found" would not be. + */ + private MetricAlarm requireAlarm(String resourceArn, String region) { return alarmStore.scan(k -> k.startsWith(region + "::")) .stream() - .filter(a -> resourceArn.equals(a.getAlarmArn())) + .filter(a -> a.getAlarmArn() != null && a.getAlarmArn().equals(resourceArn)) .findFirst() - .map(MetricAlarm::getTags) - .orElse(Map.of()); + .orElseThrow(() -> new AwsException("ResourceNotFoundException", + "No CloudWatch resource matches the ARN " + resourceArn + ".", 404)); + } + + public Map listTagsForResource(String resourceArn, String region) { + return requireAlarm(resourceArn, region).getTags(); } public void tagResource(String resourceArn, Map tags, String region) { - alarmStore.scan(k -> k.startsWith(region + "::")) - .stream() - .filter(a -> resourceArn.equals(a.getAlarmArn())) - .findFirst() - .ifPresent(alarm -> { - alarm.getTags().putAll(tags); - alarmStore.put(region + "::" + alarm.getAlarmName(), alarm); - }); + MetricAlarm alarm = requireAlarm(resourceArn, region); + alarm.getTags().putAll(tags); + alarmStore.put(region + "::" + alarm.getAlarmName(), alarm); } public void untagResource(String resourceArn, List tagKeys, String region) { - alarmStore.scan(k -> k.startsWith(region + "::")) - .stream() - .filter(a -> resourceArn.equals(a.getAlarmArn())) - .findFirst() - .ifPresent(alarm -> { - tagKeys.forEach(alarm.getTags()::remove); - alarmStore.put(region + "::" + alarm.getAlarmName(), alarm); - }); + MetricAlarm alarm = requireAlarm(resourceArn, region); + tagKeys.forEach(alarm.getTags()::remove); + alarmStore.put(region + "::" + alarm.getAlarmName(), alarm); } // ──────────────────────────── Helpers ──────────────────────────── diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metricstreams/CloudWatchMetricStreamsService.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metricstreams/CloudWatchMetricStreamsService.java index 4f3ba3c..020ff70 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metricstreams/CloudWatchMetricStreamsService.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metricstreams/CloudWatchMetricStreamsService.java @@ -196,21 +196,30 @@ public class CloudWatchMetricStreamsService { } public Map listTagsForResource(String resourceArn, String region) { - return findByArn(resourceArn, region).map(MetricStream::getTags).orElse(Map.of()); + return requireStream(resourceArn, region).getTags(); } public void tagResource(String resourceArn, Map tags, String region) { - findByArn(resourceArn, region).ifPresent(stream -> { - stream.getTags().putAll(tags); - streamStore.put(key(region, stream.getName()), stream); - }); + MetricStream stream = requireStream(resourceArn, region); + stream.getTags().putAll(tags); + streamStore.put(key(region, stream.getName()), stream); } public void untagResource(String resourceArn, List tagKeys, String region) { - findByArn(resourceArn, region).ifPresent(stream -> { - tagKeys.forEach(stream.getTags()::remove); - streamStore.put(key(region, stream.getName()), stream); - }); + MetricStream stream = requireStream(resourceArn, region); + tagKeys.forEach(stream.getTags()::remove); + streamStore.put(key(region, stream.getName()), stream); + } + + /** + * Resolves the stream an ARN names, or reports that nothing does. The tag operations declare + * {@code ResourceNotFoundException}, the same code {@code GetMetricStream} already answers + * with, so an unknown ARN here reads the same way an unknown name does. + */ + private MetricStream requireStream(String resourceArn, String region) { + return findByArn(resourceArn, region).orElseThrow(() -> + new AwsException("ResourceNotFoundException", + "Metric stream " + resourceArn + " does not exist.", 404)); } private Optional findByArn(String resourceArn, String region) { diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/CloudWatchTagUnknownResourceIntegrationTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/CloudWatchTagUnknownResourceIntegrationTest.java new file mode 100644 index 0000000..8963e06 --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/CloudWatchTagUnknownResourceIntegrationTest.java @@ -0,0 +1,349 @@ +package io.github.hectorvent.floci.services.cloudwatch; + +import io.github.hectorvent.floci.testing.RestAssuredJsonUtils; +import io.quarkus.test.junit.QuarkusTest; +import io.restassured.specification.RequestSpecification; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static io.restassured.RestAssured.given; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; + +/** + * Tagging a CloudWatch resource whose ARN names nothing has to fail, on every one of the three + * tag operations and for every taggable kind CloudWatch owns. + * + *

Each kind is checked twice: once on an ARN that resolves, so the operation is known to work + * at all, and once on the same ARN with the resource name swapped for one that was never created. + * The two ARNs are byte-identical in shape, so "the ARN was malformed" cannot explain the second + * answer. Without that pairing a test asserting only the failure would also pass against a build + * whose tag operations were broken outright. + * + *

The failure to look for is {@code ResourceNotFoundException}. CloudWatch's model declares + * that code, not the shorter {@code ResourceNotFound} that {@code GetDashboard} and + * {@code SetAlarmState} answer with, for {@code TagResource}, {@code UntagResource} and + * {@code ListTagsForResource}; the SDK maps the two to different exception classes, so a caller + * catching the documented one would not see the other. + */ +@QuarkusTest +class CloudWatchTagUnknownResourceIntegrationTest { + + private static final String CW_SCOPE = + "AWS4-HMAC-SHA256 Credential=test/20260205/us-east-1/monitoring/aws4_request"; + private static final String LOGS_SCOPE = + "AWS4-HMAC-SHA256 Credential=test/20260205/us-east-1/logs/aws4_request"; + + private static final String JSON_1_0 = "application/x-amz-json-1.0"; + private static final String JSON_1_1 = "application/x-amz-json-1.1"; + + private static final String PREFIX = "arn:aws:cloudwatch:us-east-1:000000000000:"; + private static final String FIREHOSE_ARN = + "arn:aws:firehose:us-east-1:000000000000:deliverystream/metrics"; + private static final String ROLE_ARN = "arn:aws:iam::000000000000:role/metric-stream-role"; + + @BeforeAll + static void configureRestAssured() { + RestAssuredJsonUtils.configureAwsContentTypes(); + } + + private static RequestSpecification query(String action) { + return given() + .contentType("application/x-www-form-urlencoded") + .header("Authorization", CW_SCOPE) + .formParam("Action", action); + } + + private static RequestSpecification json(String contentType, String scope, String target, String body) { + return given() + .contentType(contentType) + .header("Authorization", scope) + .header("X-Amz-Target", target) + .body(body); + } + + private static RequestSpecification metricsJson(String target, String body) { + return json(JSON_1_0, CW_SCOPE, "GraniteServiceVersion20100801." + target, body); + } + + private static RequestSpecification logsJson(String target, String body) { + return json(JSON_1_1, LOGS_SCOPE, "Logs_20140328." + target, body); + } + + /** The three Query tag operations against one ARN, asserting the same outcome for each. */ + private static void assertQueryTagOpsNotFound(String arn) { + query("TagResource") + .formParam("ResourceARN", arn) + .formParam("Tags.member.1.Key", "env") + .formParam("Tags.member.1.Value", "prod") + .when() + .post("/") + .then() + .statusCode(404) + .body("ErrorResponse.Error.Code", equalTo("ResourceNotFoundException")); + + query("UntagResource") + .formParam("ResourceARN", arn) + .formParam("TagKeys.member.1", "env") + .when() + .post("/") + .then() + .statusCode(404) + .body("ErrorResponse.Error.Code", equalTo("ResourceNotFoundException")); + + query("ListTagsForResource") + .formParam("ResourceARN", arn) + .when() + .post("/") + .then() + .statusCode(404) + .body("ErrorResponse.Error.Code", equalTo("ResourceNotFoundException")); + } + + /** The same three operations against an ARN that does resolve, read back through a list. */ + private static void assertQueryTagOpsSucceed(String arn) { + query("TagResource") + .formParam("ResourceARN", arn) + .formParam("Tags.member.1.Key", "env") + .formParam("Tags.member.1.Value", "prod") + .when() + .post("/") + .then() + .statusCode(200); + + query("ListTagsForResource") + .formParam("ResourceARN", arn) + .when() + .post("/") + .then() + .statusCode(200) + .body("ListTagsForResourceResponse.ListTagsForResourceResult.Tags.member.Key", + equalTo("env")) + .body("ListTagsForResourceResponse.ListTagsForResourceResult.Tags.member.Value", + equalTo("prod")); + + query("UntagResource") + .formParam("ResourceARN", arn) + .formParam("TagKeys.member.1", "env") + .when() + .post("/") + .then() + .statusCode(200); + + query("ListTagsForResource") + .formParam("ResourceARN", arn) + .when() + .post("/") + .then() + .statusCode(200) + .body(not(containsString("env"))); + } + + @Test + void alarmTagOperationsRejectAnArnThatNamesNoAlarm() { + query("PutMetricAlarm") + .formParam("AlarmName", "tag-unknown-alarm") + .formParam("MetricName", "CPUUtilization") + .formParam("Namespace", "AWS/EC2") + .formParam("ComparisonOperator", "GreaterThanThreshold") + .formParam("EvaluationPeriods", "1") + .formParam("Period", "60") + .formParam("Statistic", "Average") + .formParam("Threshold", "80") + .when() + .post("/") + .then() + .statusCode(200); + + assertQueryTagOpsSucceed(PREFIX + "alarm:tag-unknown-alarm"); + assertQueryTagOpsNotFound(PREFIX + "alarm:tag-unknown-alarm-nosuch"); + } + + @Test + void dashboardTagOperationsRejectAnArnThatNamesNoDashboard() { + query("PutDashboard") + .formParam("DashboardName", "tag-unknown-dashboard") + .formParam("DashboardBody", "{\"widgets\":[]}") + .when() + .post("/") + .then() + .statusCode(200); + + assertQueryTagOpsSucceed(PREFIX + "dashboard/tag-unknown-dashboard"); + assertQueryTagOpsNotFound(PREFIX + "dashboard/tag-unknown-dashboard-nosuch"); + } + + @Test + void metricStreamTagOperationsRejectAnArnThatNamesNoStream() { + query("PutMetricStream") + .formParam("Name", "tag-unknown-stream") + .formParam("FirehoseArn", FIREHOSE_ARN) + .formParam("RoleArn", ROLE_ARN) + .formParam("OutputFormat", "json") + .formParam("IncludeFilters.member.1.Namespace", "AWS/EC2") + .when() + .post("/") + .then() + .statusCode(200); + + assertQueryTagOpsSucceed(PREFIX + "metric-stream/tag-unknown-stream"); + assertQueryTagOpsNotFound(PREFIX + "metric-stream/tag-unknown-stream-nosuch"); + } + + /** + * The dashboard ARN AWS documents carries no region, and the one Floci mints does. A + * caller sending the documented form for a dashboard that exists must not be told the + * dashboard does not exist, which is what raising on a miss would otherwise have made of + * a shape that used to fall through to an empty tag map. + */ + @Test + void theRegionlessDashboardArnAwsDocumentsAlsoResolves() { + query("PutDashboard") + .formParam("DashboardName", "tag-regionless-dashboard") + .formParam("DashboardBody", "{\"widgets\":[]}") + .when() + .post("/") + .then() + .statusCode(200); + + assertQueryTagOpsSucceed("arn:aws:cloudwatch::000000000000:dashboard/tag-regionless-dashboard"); + + // Still only an absent region: another region's ARN remains a miss. + assertQueryTagOpsNotFound( + "arn:aws:cloudwatch:eu-west-1:000000000000:dashboard/tag-regionless-dashboard"); + } + + /** + * Both handlers route every ARN that is not a dashboard and not a metric stream to the + * alarm path, so a Contributor Insights ARN, which AWS documents as taggable and Floci + * does not serve, lands there. Reporting it as not found is right; calling it an alarm + * that was not found is not, so the message names the ARN instead. + */ + @Test + void anArnOfAKindFlociDoesNotServeIsReportedAgainstTheArn() { + query("TagResource") + .formParam("ResourceARN", PREFIX + "insight-rule/no-such-rule") + .formParam("Tags.member.1.Key", "env") + .formParam("Tags.member.1.Value", "prod") + .when() + .post("/") + .then() + .statusCode(404) + .body("ErrorResponse.Error.Code", equalTo("ResourceNotFoundException")) + .body("ErrorResponse.Error.Message", + containsString("insight-rule/no-such-rule")) + .body("ErrorResponse.Error.Message", + not(containsString("Alarm"))); + } + + /** + * CloudWatch Metrics serves the same three operations over JSON as well as Query, and + * AGENTS.md requires the two handlers not to drift. A fix applied to only one of them + * would leave the CLI and SDK v3 route still answering 200. + */ + @Test + void theJsonProtocolRejectsTheSameArns() { + metricsJson("PutMetricAlarm", """ + {"AlarmName": "tag-unknown-json-alarm", "MetricName": "CPUUtilization", + "Namespace": "AWS/EC2", "ComparisonOperator": "GreaterThanThreshold", + "EvaluationPeriods": 1, "Period": 60, "Statistic": "Average", "Threshold": 80}""") + .when() + .post("/") + .then() + .statusCode(200); + + String real = PREFIX + "alarm:tag-unknown-json-alarm"; + String ghost = PREFIX + "alarm:tag-unknown-json-alarm-nosuch"; + + metricsJson("TagResource", + "{\"ResourceARN\": \"" + real + "\", \"Tags\": [{\"Key\": \"env\", \"Value\": \"prod\"}]}") + .when() + .post("/") + .then() + .statusCode(200); + + metricsJson("ListTagsForResource", "{\"ResourceARN\": \"" + real + "\"}") + .when() + .post("/") + .then() + .statusCode(200) + .body("Tags[0].Key", equalTo("env")) + .body("Tags[0].Value", equalTo("prod")); + + metricsJson("TagResource", + "{\"ResourceARN\": \"" + ghost + "\", \"Tags\": [{\"Key\": \"env\", \"Value\": \"prod\"}]}") + .when() + .post("/") + .then() + .statusCode(404) + .body("__type", containsString("ResourceNotFoundException")); + + metricsJson("UntagResource", "{\"ResourceARN\": \"" + ghost + "\", \"TagKeys\": [\"env\"]}") + .when() + .post("/") + .then() + .statusCode(404) + .body("__type", containsString("ResourceNotFoundException")); + + metricsJson("ListTagsForResource", "{\"ResourceARN\": \"" + ghost + "\"}") + .when() + .post("/") + .then() + .statusCode(404) + .body("__type", containsString("ResourceNotFoundException")); + } + + /** + * CloudWatch Logs serves its own TagResource, UntagResource and ListTagsForResource over + * a separate handler, and already rejects an ARN that names no log group. Pinning it keeps + * the two halves of CloudWatch's tagging from diverging again in the other direction. + */ + @Test + void logGroupTagOperationsRejectAnArnThatNamesNoGroup() { + String real = "arn:aws:logs:us-east-1:000000000000:log-group:tag-unknown-group:*"; + String ghost = "arn:aws:logs:us-east-1:000000000000:log-group:tag-unknown-group-nosuch:*"; + + logsJson("CreateLogGroup", "{\"logGroupName\": \"tag-unknown-group\"}") + .when() + .post("/") + .then() + .statusCode(200); + + logsJson("TagResource", + "{\"resourceArn\": \"" + real + "\", \"tags\": {\"env\": \"prod\"}}") + .when() + .post("/") + .then() + .statusCode(200); + + logsJson("ListTagsForResource", "{\"resourceArn\": \"" + real + "\"}") + .when() + .post("/") + .then() + .statusCode(200) + .body("tags.env", equalTo("prod")); + + logsJson("TagResource", + "{\"resourceArn\": \"" + ghost + "\", \"tags\": {\"env\": \"prod\"}}") + .when() + .post("/") + .then() + .statusCode(400) + .body("__type", containsString("ResourceNotFoundException")); + + logsJson("UntagResource", "{\"resourceArn\": \"" + ghost + "\", \"tagKeys\": [\"env\"]}") + .when() + .post("/") + .then() + .statusCode(400) + .body("__type", containsString("ResourceNotFoundException")); + + logsJson("ListTagsForResource", "{\"resourceArn\": \"" + ghost + "\"}") + .when() + .post("/") + .then() + .statusCode(400) + .body("__type", containsString("ResourceNotFoundException")); + } +} diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/dashboards/CloudWatchDashboardsServiceTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/dashboards/CloudWatchDashboardsServiceTest.java index be2c481..fc21abd 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/dashboards/CloudWatchDashboardsServiceTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/dashboards/CloudWatchDashboardsServiceTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.List; +import java.util.Map; import static org.junit.jupiter.api.Assertions.*; @@ -116,10 +117,10 @@ class CloudWatchDashboardsServiceTest { @Test void tagsGivenOnCreateAreReadableByArn() { - service.putDashboard("ops", BODY, java.util.Map.of("team", "platform"), REGION); + service.putDashboard("ops", BODY, Map.of("team", "platform"), REGION); String arn = service.getDashboard("ops", REGION).getDashboardArn(); - assertEquals(java.util.Map.of("team", "platform"), service.listTagsForResource(arn, REGION)); + assertEquals(Map.of("team", "platform"), service.listTagsForResource(arn, REGION)); assertTrue(CloudWatchDashboardsService.isDashboardArn(arn)); } @@ -129,12 +130,12 @@ class CloudWatchDashboardsServiceTest { */ @Test void tagsOnAReplacingPutDoNotOverwriteTheExistingOnes() { - service.putDashboard("ops", BODY, java.util.Map.of("team", "platform"), REGION); + service.putDashboard("ops", BODY, Map.of("team", "platform"), REGION); String arn = service.getDashboard("ops", REGION).getDashboardArn(); - service.putDashboard("ops", BODY, java.util.Map.of("team", "someone-else"), REGION); + service.putDashboard("ops", BODY, Map.of("team", "someone-else"), REGION); - assertEquals(java.util.Map.of("team", "platform"), service.listTagsForResource(arn, REGION)); + assertEquals(Map.of("team", "platform"), service.listTagsForResource(arn, REGION)); } @Test @@ -142,11 +143,75 @@ class CloudWatchDashboardsServiceTest { service.putDashboard("ops", BODY, REGION); String arn = service.getDashboard("ops", REGION).getDashboardArn(); - service.tagResource(arn, java.util.Map.of("env", "dev"), REGION); - assertEquals(java.util.Map.of("env", "dev"), service.listTagsForResource(arn, REGION)); + service.tagResource(arn, Map.of("env", "dev"), REGION); + assertEquals(Map.of("env", "dev"), service.listTagsForResource(arn, REGION)); service.untagResource(arn, List.of("env"), REGION); - assertEquals(java.util.Map.of(), service.listTagsForResource(arn, REGION)); + assertEquals(Map.of(), service.listTagsForResource(arn, REGION)); + } + + /** + * AWS documents the dashboard ARN without a region and Floci mints one with a region, so + * both forms have to resolve. Before the tag operations raised on a miss this divergence + * was invisible here: the documented ARN matched nothing and got an empty tag map back. + */ + @Test + void bothTheRegionlessAndTheRegionfulDashboardArnResolve() { + service.putDashboard("ops", BODY, REGION); + String regionful = service.getDashboard("ops", REGION).getDashboardArn(); + String regionless = "arn:aws:cloudwatch::000000000000:dashboard/ops"; + + assertEquals("arn:aws:cloudwatch:us-east-1:000000000000:dashboard/ops", regionful); + assertTrue(CloudWatchDashboardsService.isDashboardArn(regionless)); + + service.tagResource(regionless, Map.of("env", "dev"), REGION); + // Read back through the other form: one dashboard, reachable either way. + assertEquals(Map.of("env", "dev"), service.listTagsForResource(regionful, REGION)); + assertEquals(Map.of("env", "dev"), service.listTagsForResource(regionless, REGION)); + + service.untagResource(regionless, List.of("env"), REGION); + assertEquals(Map.of(), service.listTagsForResource(regionful, REGION)); + } + + /** + * Only an absent region stands in for the dashboard's own. An ARN naming a different + * region must not resolve to the same-named dashboard in this one. + */ + @Test + void anArnNamingAnotherRegionDoesNotResolve() { + service.putDashboard("ops", BODY, REGION); + String elsewhere = "arn:aws:cloudwatch:eu-west-1:000000000000:dashboard/ops"; + + AwsException e = assertThrows(AwsException.class, + () -> service.listTagsForResource(elsewhere, REGION)); + assertEquals("ResourceNotFoundException", e.getErrorCode()); + } + + /** + * An ARN naming no dashboard is an error on all three operations, not a silent no-op. The + * ARN used here differs from a working one only in the dashboard name, so the outcome + * cannot be explained by the ARN failing to parse as a dashboard ARN. + */ + @Test + void tagOperationsRejectAnArnThatNamesNoDashboard() { + service.putDashboard("ops", BODY, REGION); + String arn = service.getDashboard("ops", REGION).getDashboardArn(); + String ghost = arn.replace("dashboard/ops", "dashboard/nosuch"); + assertTrue(CloudWatchDashboardsService.isDashboardArn(ghost)); + + for (Runnable call : List.of( + () -> service.tagResource(ghost, Map.of("env", "prod"), REGION), + () -> service.untagResource(ghost, List.of("env"), REGION), + () -> service.listTagsForResource(ghost, REGION))) { + AwsException e = assertThrows(AwsException.class, call::run); + assertEquals("ResourceNotFoundException", e.getErrorCode()); + assertEquals(404, e.getHttpStatus()); + } + + // The failures wrote nothing: the real dashboard still carries only what it was given. + assertEquals(Map.of(), service.listTagsForResource(arn, REGION)); + service.tagResource(arn, Map.of("env", "dev"), REGION); + assertEquals(Map.of("env", "dev"), service.listTagsForResource(arn, REGION)); } @Test diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsTagsTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsTagsTest.java index 4112eb7..db5770a 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsTagsTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsTagsTest.java @@ -2,6 +2,7 @@ package io.github.hectorvent.floci.services.cloudwatch.metrics; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import io.github.hectorvent.floci.core.common.AwsException; import io.github.hectorvent.floci.core.common.RegionResolver; import io.github.hectorvent.floci.core.storage.InMemoryStorage; import io.github.hectorvent.floci.services.cloudwatch.dashboards.CloudWatchDashboardsService; @@ -83,6 +84,34 @@ class CloudWatchMetricsTagsTest { assertNull(tags.get("team")); } + /** + * An ARN naming no alarm is an error on all three operations, not a silent no-op. The ARN + * used here differs from a working one only in the alarm name, so the outcome cannot be + * explained by the ARN failing to parse. + */ + @Test + void tagOperationsRejectAnArnThatNamesNoAlarm() { + MetricAlarm alarm = new MetricAlarm(); + alarm.setAlarmName("test-alarm"); + alarm.setAlarmArn("arn:aws:cloudwatch:us-east-1:000000000000:alarm:test-alarm"); + service.putMetricAlarm(alarm, REGION); + String ghost = "arn:aws:cloudwatch:us-east-1:000000000000:alarm:nosuch"; + + for (Runnable call : List.of( + () -> service.tagResource(ghost, Map.of("env", "prod"), REGION), + () -> service.untagResource(ghost, List.of("env"), REGION), + () -> service.listTagsForResource(ghost, REGION))) { + AwsException e = assertThrows(AwsException.class, call::run); + assertEquals("ResourceNotFoundException", e.getErrorCode()); + assertEquals(404, e.getHttpStatus()); + } + + // The failures wrote nothing: the real alarm still carries only what it was given. + assertEquals(Map.of(), service.listTagsForResource(alarm.getAlarmArn(), REGION)); + service.tagResource(alarm.getAlarmArn(), Map.of("env", "dev"), REGION); + assertEquals(Map.of("env", "dev"), service.listTagsForResource(alarm.getAlarmArn(), REGION)); + } + @Test void tagsViaPutMetricAlarmQuery() { MultivaluedMap params = new MultivaluedHashMap<>(); diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metricstreams/CloudWatchMetricStreamsServiceTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metricstreams/CloudWatchMetricStreamsServiceTest.java index 6648f46..66b8beb 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metricstreams/CloudWatchMetricStreamsServiceTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metricstreams/CloudWatchMetricStreamsServiceTest.java @@ -232,4 +232,29 @@ class CloudWatchMetricStreamsServiceTest { service.untagResource(arn, List.of("team"), REGION); assertFalse(service.listTagsForResource(arn, REGION).containsKey("team")); } + + /** + * An ARN naming no stream is an error on all three operations, not a silent no-op. The ARN + * used here differs from a working one only in the stream name, so the outcome cannot be + * explained by the ARN failing to parse as a metric stream ARN. + */ + @Test + void tagOperationsRejectAnArnThatNamesNoStream() { + String arn = service.putMetricStream(stream("ops"), REGION).getArn(); + String ghost = arn.replace("metric-stream/ops", "metric-stream/nosuch"); + assertTrue(CloudWatchMetricStreamsService.isMetricStreamArn(ghost)); + + for (Runnable call : List.of( + () -> service.tagResource(ghost, Map.of("env", "prod"), REGION), + () -> service.untagResource(ghost, List.of("env"), REGION), + () -> service.listTagsForResource(ghost, REGION))) { + AwsException e = assertThrows(AwsException.class, call::run); + assertEquals("ResourceNotFoundException", e.getErrorCode()); + assertEquals(404, e.getHttpStatus()); + } + + // The failures wrote nothing: the real stream still carries only what it was given. + service.tagResource(arn, Map.of("team", "platform"), REGION); + assertEquals(Map.of("team", "platform"), service.listTagsForResource(arn, REGION)); + } }