From cbb95ba26cf04fe8c7c2289d1b35336d775d940f Mon Sep 17 00:00:00 2001 From: Aniket Shukla Date: Tue, 29 Sep 2026 17:12:52 +0530 Subject: [PATCH 1/9] fix(cloudwatch): retain metric-math alarm query definitions (#4594) * fix(cloudwatch): retain metric-math alarm query definitions * test(cloudwatch): use shared XML parser for alarm assertions (cherry picked from commit 1d646c6fea70a8d4bfeea4d0ca6d8fc390ea6170) --- .../test/CloudWatchMetricMathAlarmTest.java | 117 ++++++++++ docs/services/cloudwatch.md | 12 + .../cloudwatch/metrics/AlarmEvaluator.java | 3 +- .../metrics/CloudWatchMetricsJsonHandler.java | 45 +++- .../CloudWatchMetricsQueryHandler.java | 101 +++++++- .../metrics/model/AlarmMetricDataQuery.java | 46 ++++ .../cloudwatch/metrics/model/MetricAlarm.java | 13 +- .../CloudWatchMetricMathAlarmTest.java | 218 ++++++++++++++++++ 8 files changed, 529 insertions(+), 26 deletions(-) create mode 100644 compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchMetricMathAlarmTest.java create mode 100644 src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/AlarmMetricDataQuery.java create mode 100644 src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricMathAlarmTest.java diff --git a/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchMetricMathAlarmTest.java b/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchMetricMathAlarmTest.java new file mode 100644 index 0000000..d32f9d5 --- /dev/null +++ b/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchMetricMathAlarmTest.java @@ -0,0 +1,117 @@ +package com.floci.test; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.services.cloudwatch.CloudWatchClient; +import software.amazon.awssdk.services.cloudwatch.model.ComparisonOperator; +import software.amazon.awssdk.services.cloudwatch.model.Dimension; +import software.amazon.awssdk.services.cloudwatch.model.Metric; +import software.amazon.awssdk.services.cloudwatch.model.MetricAlarm; +import software.amazon.awssdk.services.cloudwatch.model.MetricDataQuery; +import software.amazon.awssdk.services.cloudwatch.model.MetricStat; +import software.amazon.awssdk.services.cloudwatch.model.StandardUnit; +import software.amazon.awssdk.services.cloudwatch.model.Statistic; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +@DisplayName("CloudWatch - Metric-math alarm definitions") +class CloudWatchMetricMathAlarmTest { + + private CloudWatchClient cloudWatch; + private String alarmName; + + @BeforeEach + void setup() { + cloudWatch = TestFixtures.cloudWatchClient(); + alarmName = TestFixtures.uniqueName("sdk-metric-math-alarm"); + } + + @AfterEach + void cleanup() { + try { + cloudWatch.deleteAlarms(request -> request.alarmNames(alarmName)); + } finally { + cloudWatch.close(); + } + } + + @Test + void metricsRoundTripWithoutInventingSingleMetricFields() { + List metrics = metrics(); + putMathAlarm(metrics); + assertMathAlarm(metrics); + } + + @Test + void updatesReplaceQueriesAndSwitchBackToASingleMetric() { + putSingleMetricAlarm(300); + List original = metrics(); + putMathAlarm(original); + assertMathAlarm(original); + + List replacement = List.of( + original.get(0).toBuilder().expression("errors / requests").label("Ratio").build(), + original.get(1), original.get(2)); + putMathAlarm(replacement); + assertMathAlarm(replacement); + + putSingleMetricAlarm(120); + MetricAlarm alarm = describe(); + assertThat(alarm.hasMetrics()).isFalse(); + assertThat(alarm.period()).isEqualTo(120); + assertThat(alarm.metricName()).isEqualTo("5xx"); + assertThat(alarm.namespace()).isEqualTo("AWS/ApiGateway"); + } + + private void putMathAlarm(List metrics) { + cloudWatch.putMetricAlarm(request -> request.alarmName(alarmName) + .comparisonOperator(ComparisonOperator.GREATER_THAN_THRESHOLD) + .evaluationPeriods(2).threshold(5.0).metrics(metrics)); + } + + private void putSingleMetricAlarm(int period) { + cloudWatch.putMetricAlarm(request -> request.alarmName(alarmName) + .comparisonOperator(ComparisonOperator.GREATER_THAN_THRESHOLD) + .evaluationPeriods(2).threshold(5.0).namespace("AWS/ApiGateway") + .metricName("5xx").statistic(Statistic.SUM).period(period)); + } + + private MetricAlarm describe() { + List alarms = cloudWatch.describeAlarms(request -> request.alarmNames(alarmName)).metricAlarms(); + assertThat(alarms).hasSize(1); + return alarms.get(0); + } + + private void assertMathAlarm(List metrics) { + MetricAlarm alarm = describe(); + assertThat(alarm.metrics()).containsExactlyElementsOf(metrics); + assertThat(alarm.period()).isNull(); + assertThat(alarm.metricName()).isNull(); + assertThat(alarm.namespace()).isNull(); + assertThat(alarm.statistic()).isNull(); + assertThat(alarm.extendedStatistic()).isNull(); + assertThat(alarm.unit()).isNull(); + assertThat(alarm.hasDimensions()).isFalse(); + assertThat(alarm.evaluationPeriods()).isEqualTo(2); + assertThat(alarm.threshold()).isEqualTo(5.0); + } + + private static List metrics() { + return List.of( + MetricDataQuery.builder().id("rate").expression("100 * errors / requests") + .returnData(true).period(300).label("Error & requests").build(), + MetricDataQuery.builder().id("errors").returnData(false).accountId("123456789012") + .metricStat(MetricStat.builder().period(300).stat("Sum").unit(StandardUnit.COUNT) + .metric(Metric.builder().namespace("AWS/ApiGateway").metricName("5xx") + .dimensions(Dimension.builder().name("ApiName").value("api<&>").build()) + .build()).build()).build(), + MetricDataQuery.builder().id("requests").returnData(false) + .metricStat(MetricStat.builder().period(300).stat("Sum") + .metric(Metric.builder().namespace("AWS/ApiGateway").metricName("Count").build()) + .build()).build()); + } +} diff --git a/docs/services/cloudwatch.md b/docs/services/cloudwatch.md index e9e1b61..121e9c5 100644 --- a/docs/services/cloudwatch.md +++ b/docs/services/cloudwatch.md @@ -331,6 +331,18 @@ aws logs put-retention-policy \ Metric streams are stored as definitions with their `running` or `stopped` state. Floci never delivers metrics to the Firehose delivery stream a metric stream names. +### Metric-math alarm definitions + +`PutMetricAlarm` stores the `Metrics` query definitions and `DescribeAlarms` returns them over +both JSON and Query protocols. Expressions, labels, `ReturnData`, account IDs, query periods, +and nested metric statistics (including dimensions and units) survive storage reloads. +Metric-math alarm responses omit the single-metric fields, including top-level `Period`. +Updating an alarm replaces its metric definition, so changing between `Metrics` and a single +metric does not retain the previous definition. + +This is management-plane support: the alarm evaluator currently evaluates only single-metric +alarms, not metric-math expressions. `SetAlarmState` remains available for manual transitions. + ### Examples ```bash diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmEvaluator.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmEvaluator.java index f7b0eeb..335ef51 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmEvaluator.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmEvaluator.java @@ -275,9 +275,10 @@ public class AlarmEvaluator { private static boolean isEvaluable(MetricAlarm alarm) { return alarm.getRegion() != null + && alarm.getMetrics().isEmpty() && alarm.getNamespace() != null && !alarm.getNamespace().isBlank() && alarm.getMetricName() != null && !alarm.getMetricName().isBlank() - && alarm.getPeriod() > 0 + && alarm.getPeriod() != null && alarm.getPeriod() > 0 && alarm.getEvaluationPeriods() > 0 && alarm.getComparisonOperator() != null; } diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java index d0edf31..5aa036d 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java @@ -12,6 +12,7 @@ import io.github.hectorvent.floci.services.cloudwatch.metricstreams.CloudWatchMe import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStream; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStreamFilter; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStreamStatisticsConfiguration; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.AlarmMetricDataQuery; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Dimension; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricDatum; @@ -156,7 +157,13 @@ public class CloudWatchMetricsJsonHandler { alarm.setMetricName(request.path("MetricName").asText(null)); alarm.setNamespace(request.path("Namespace").asText(null)); alarm.setStatistic(request.path("Statistic").asText(null)); - alarm.setPeriod(request.path("Period").asInt(60)); + JsonNode metrics = request.path("Metrics"); + if (metrics.isArray()) { + for (JsonNode query : metrics) { + alarm.getMetrics().add(objectMapper.convertValue(query, AlarmMetricDataQuery.class)); + } + } + alarm.setPeriod(alarm.getMetrics().isEmpty() ? request.path("Period").asInt(60) : null); alarm.setUnit(request.path("Unit").asText(null)); alarm.setEvaluationPeriods(request.path("EvaluationPeriods").asInt(1)); alarm.setDatapointsToAlarm(request.hasNonNull("DatapointsToAlarm") @@ -214,17 +221,31 @@ public class CloudWatchMetricsJsonHandler { a.getOkActions().forEach(okActions::add); ArrayNode insufficientDataActions = node.putArray("InsufficientDataActions"); a.getInsufficientDataActions().forEach(insufficientDataActions::add); - if (a.getMetricName() != null) node.put("MetricName", a.getMetricName()); - if (a.getNamespace() != null) node.put("Namespace", a.getNamespace()); - if (a.getStatistic() != null) node.put("Statistic", a.getStatistic()); - ArrayNode dimensions = node.putArray("Dimensions"); - a.getDimensions().forEach(d -> { - ObjectNode dimNode = dimensions.addObject(); - dimNode.put("Name", d.name()); - dimNode.put("Value", d.value()); - }); - node.put("Period", a.getPeriod()); - if (a.getUnit() != null) node.put("Unit", a.getUnit()); + if (!a.getMetrics().isEmpty()) { + node.set("Metrics", objectMapper.valueToTree(a.getMetrics())); + } else { + if (a.getMetricName() != null) { + node.put("MetricName", a.getMetricName()); + } + if (a.getNamespace() != null) { + node.put("Namespace", a.getNamespace()); + } + if (a.getStatistic() != null) { + node.put("Statistic", a.getStatistic()); + } + ArrayNode dimensions = node.putArray("Dimensions"); + a.getDimensions().forEach(d -> { + ObjectNode dimNode = dimensions.addObject(); + dimNode.put("Name", d.name()); + dimNode.put("Value", d.value()); + }); + if (a.getPeriod() != null) { + node.put("Period", a.getPeriod()); + } + if (a.getUnit() != null) { + node.put("Unit", a.getUnit()); + } + } node.put("EvaluationPeriods", a.getEvaluationPeriods()); if (a.getDatapointsToAlarm() != null) { node.put("DatapointsToAlarm", a.getDatapointsToAlarm()); diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java index 61584f1..6d67684 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java @@ -11,6 +11,7 @@ import io.github.hectorvent.floci.services.cloudwatch.metricstreams.CloudWatchMe import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStream; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStreamFilter; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStreamStatisticsConfiguration; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.AlarmMetricDataQuery; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Dimension; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricDatum; @@ -660,7 +661,8 @@ public class CloudWatchMetricsQueryHandler { a.setMetricName(params.getFirst("MetricName")); a.setNamespace(params.getFirst("Namespace")); a.setStatistic(params.getFirst("Statistic")); - a.setPeriod(parseIntParam(params, "Period", 60)); + a.setMetrics(parseAlarmMetricQueries(params)); + a.setPeriod(a.getMetrics().isEmpty() ? parseIntParam(params, "Period", 60) : null); a.setUnit(params.getFirst("Unit")); a.setEvaluationPeriods(parseIntParam(params, "EvaluationPeriods", 1)); String datapointsToAlarm = params.getFirst("DatapointsToAlarm"); @@ -709,6 +711,50 @@ public class CloudWatchMetricsQueryHandler { return a; } + private List parseAlarmMetricQueries(MultivaluedMap params) { + List queries = new ArrayList<>(); + for (int index = 1; ; index++) { + String prefix = "Metrics.member." + index; + String id = params.getFirst(prefix + ".Id"); + if (id == null) { + break; + } + AlarmMetricDataQuery.MetricStat metricStat = null; + String metricPrefix = prefix + ".MetricStat.Metric"; + if (params.getFirst(metricPrefix + ".Namespace") != null) { + List dimensions = null; + for (int dimensionIndex = 1; ; dimensionIndex++) { + String dimensionPrefix = metricPrefix + ".Dimensions.member." + dimensionIndex; + String name = params.getFirst(dimensionPrefix + ".Name"); + if (name == null) { + break; + } + if (dimensions == null) { + dimensions = new ArrayList<>(); + } + dimensions.add(new AlarmMetricDataQuery.MetricDimension( + name, params.getFirst(dimensionPrefix + ".Value"))); + } + AlarmMetricDataQuery.Metric metric = new AlarmMetricDataQuery.Metric( + params.getFirst(metricPrefix + ".Namespace"), + params.getFirst(metricPrefix + ".MetricName"), dimensions); + metricStat = new AlarmMetricDataQuery.MetricStat(metric, + optionalInteger(params.getFirst(prefix + ".MetricStat.Period")), + params.getFirst(prefix + ".MetricStat.Stat"), params.getFirst(prefix + ".MetricStat.Unit")); + } + String returnData = params.getFirst(prefix + ".ReturnData"); + queries.add(new AlarmMetricDataQuery(id, params.getFirst(prefix + ".Expression"), + params.getFirst(prefix + ".Label"), returnData == null ? null : Boolean.valueOf(returnData), + optionalInteger(params.getFirst(prefix + ".Period")), + params.getFirst(prefix + ".AccountId"), metricStat)); + } + return queries; + } + + private static Integer optionalInteger(String value) { + return value == null ? null : Integer.valueOf(value); + } + private void toAlarmXml(XmlBuilder xml, MetricAlarm a) { xml.start("member") .elem("AlarmName", a.getAlarmName()) @@ -726,21 +772,25 @@ public class CloudWatchMetricsQueryHandler { xml.start("InsufficientDataActions"); a.getInsufficientDataActions().forEach(act -> xml.elem("member", act)); xml.end("InsufficientDataActions"); - xml.start("Dimensions"); - for (Dimension d : a.getDimensions()) { - xml.start("member").elem("Name", d.name()).elem("Value", d.value()).end("member"); + if (!a.getMetrics().isEmpty()) { + toAlarmMetricsXml(xml, a.getMetrics()); + } else { + xml.start("Dimensions"); + for (Dimension d : a.getDimensions()) { + xml.start("member").elem("Name", d.name()).elem("Value", d.value()).end("member"); + } + xml.end("Dimensions") + .elem("MetricName", a.getMetricName()) + .elem("Namespace", a.getNamespace()) + .elem("Statistic", a.getStatistic()) + .elem("Period", a.getPeriod() == null ? null : a.getPeriod().toString()) + .elem("Unit", a.getUnit()); } - xml.end("Dimensions"); xml.elem("StateValue", a.getStateValue()) .elem("StateReason", a.getStateReason()) .elem("StateReasonData", a.getStateReasonData()) .elem("StateUpdatedTimestamp", Instant.ofEpochSecond(a.getStateUpdatedTimestamp()).toString()) - .elem("MetricName", a.getMetricName()) - .elem("Namespace", a.getNamespace()) - .elem("Statistic", a.getStatistic()) - .elem("Period", String.valueOf(a.getPeriod())) - .elem("Unit", a.getUnit()) .elem("EvaluationPeriods", String.valueOf(a.getEvaluationPeriods())) .elem("Threshold", String.valueOf(a.getThreshold())) .elem("ComparisonOperator", a.getComparisonOperator()) @@ -753,6 +803,37 @@ public class CloudWatchMetricsQueryHandler { xml.end("member"); } + private void toAlarmMetricsXml(XmlBuilder xml, List metrics) { + xml.start("Metrics"); + for (AlarmMetricDataQuery query : metrics) { + xml.start("member").elem("Id", query.id()).elem("Expression", query.expression()) + .elem("Label", query.label()).elem("AccountId", query.accountId()) + .elem("Period", query.period() == null ? null : query.period().toString()) + .elem("ReturnData", query.returnData() == null ? null : query.returnData().toString()); + AlarmMetricDataQuery.MetricStat stat = query.metricStat(); + if (stat != null) { + xml.start("MetricStat"); + AlarmMetricDataQuery.Metric metric = stat.metric(); + if (metric != null) { + xml.start("Metric").elem("Namespace", metric.namespace()).elem("MetricName", metric.metricName()); + if (metric.dimensions() != null) { + xml.start("Dimensions"); + for (AlarmMetricDataQuery.MetricDimension dimension : metric.dimensions()) { + xml.start("member").elem("Name", dimension.name()).elem("Value", dimension.value()) + .end("member"); + } + xml.end("Dimensions"); + } + xml.end("Metric"); + } + xml.elem("Period", stat.period() == null ? null : stat.period().toString()) + .elem("Stat", stat.stat()).elem("Unit", stat.unit()).end("MetricStat"); + } + xml.end("member"); + } + xml.end("Metrics"); + } + private Instant parseInstant(String value) { if (value == null || value.isBlank()) { return null; diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/AlarmMetricDataQuery.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/AlarmMetricDataQuery.java new file mode 100644 index 0000000..effc9f6 --- /dev/null +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/AlarmMetricDataQuery.java @@ -0,0 +1,46 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics.model; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import io.quarkus.runtime.annotations.RegisterForReflection; + +import java.util.List; + +/** The persisted AWS wire shape of a metric-math alarm query, including optional members. */ +@RegisterForReflection +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonInclude(JsonInclude.Include.NON_NULL) +public record AlarmMetricDataQuery( + @JsonProperty("Id") String id, + @JsonProperty("Expression") String expression, + @JsonProperty("Label") String label, + @JsonProperty("ReturnData") Boolean returnData, + @JsonProperty("Period") Integer period, + @JsonProperty("AccountId") String accountId, + @JsonProperty("MetricStat") MetricStat metricStat) { + + @RegisterForReflection + @JsonIgnoreProperties(ignoreUnknown = true) + @JsonInclude(JsonInclude.Include.NON_NULL) + public record MetricStat( + @JsonProperty("Metric") Metric metric, + @JsonProperty("Period") Integer period, + @JsonProperty("Stat") String stat, + @JsonProperty("Unit") String unit) { } + + @RegisterForReflection + @JsonIgnoreProperties(ignoreUnknown = true) + @JsonInclude(JsonInclude.Include.NON_NULL) + public record Metric( + @JsonProperty("Namespace") String namespace, + @JsonProperty("MetricName") String metricName, + @JsonProperty("Dimensions") List dimensions) { } + + @RegisterForReflection + @JsonIgnoreProperties(ignoreUnknown = true) + @JsonInclude(JsonInclude.Include.NON_NULL) + public record MetricDimension( + @JsonProperty("Name") String name, + @JsonProperty("Value") String value) { } +} diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/MetricAlarm.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/MetricAlarm.java index e23c518..7347be3 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/MetricAlarm.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/MetricAlarm.java @@ -28,7 +28,9 @@ public class MetricAlarm { private String namespace; private String statistic; private List dimensions = new ArrayList<>(); - private int period; + /** Single-metric period; absent for alarms whose periods are defined in Metrics. */ + private Integer period; + private List metrics = new ArrayList<>(); private String unit; private int evaluationPeriods; /** @@ -100,8 +102,13 @@ public class MetricAlarm { public List getDimensions() { return dimensions; } public void setDimensions(List dimensions) { this.dimensions = dimensions; } - public int getPeriod() { return period; } - public void setPeriod(int period) { this.period = period; } + public Integer getPeriod() { return period; } + public void setPeriod(Integer period) { this.period = period; } + + public List getMetrics() { return metrics; } + public void setMetrics(List metrics) { + this.metrics = metrics == null ? new ArrayList<>() : new ArrayList<>(metrics); + } public String getUnit() { return unit; } public void setUnit(String unit) { this.unit = unit; } diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricMathAlarmTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricMathAlarmTest.java new file mode 100644 index 0000000..e7b16a9 --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricMathAlarmTest.java @@ -0,0 +1,218 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import io.github.hectorvent.floci.core.common.RegionResolver; +import io.github.hectorvent.floci.core.common.XmlParser; +import io.github.hectorvent.floci.core.storage.InMemoryStorage; +import io.github.hectorvent.floci.core.storage.PersistentStorage; +import io.github.hectorvent.floci.core.storage.StorageBackend; +import io.github.hectorvent.floci.services.cloudwatch.dashboards.CloudWatchDashboardsService; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; +import jakarta.ws.rs.core.MultivaluedHashMap; +import jakarta.ws.rs.core.MultivaluedMap; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.w3c.dom.Document; + +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathFactory; +import java.nio.file.Path; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +class CloudWatchMetricMathAlarmTest { + + private static final String REGION = "us-east-1"; + private static final String ALARM_NAME = "metric-math-round-trip"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final String XML_ALARM = "/DescribeAlarmsResponse/DescribeAlarmsResult/MetricAlarms/member"; + + private CloudWatchMetricsJsonHandler jsonHandler; + private CloudWatchMetricsQueryHandler queryHandler; + + @BeforeEach + void setup() { + initialize(new InMemoryStorage<>()); + } + + private void initialize(StorageBackend alarmStore) { + RegionResolver resolver = new RegionResolver(REGION, "000000000000"); + CloudWatchMetricsService service = new CloudWatchMetricsService(new InMemoryStorage<>(), alarmStore, resolver); + CloudWatchDashboardsService dashboards = new CloudWatchDashboardsService(new InMemoryStorage<>(), resolver); + jsonHandler = new CloudWatchMetricsJsonHandler(service, dashboards, null, MAPPER); + queryHandler = new CloudWatchMetricsQueryHandler(service, dashboards, null); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void metricMathQueriesRoundTripThroughBothProtocols(boolean queryProtocol) throws Exception { + ObjectNode request = metricMathRequest(); + put(request, queryProtocol); + assertMetricMathAlarm(request); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void metricMathQueriesSurvivePersistentStorageReload(boolean queryProtocol, @TempDir Path directory) + throws Exception { + Path storageFile = directory.resolve("alarms.json"); + TypeReference> type = new TypeReference<>() { }; + initialize(new PersistentStorage<>(storageFile, type)); + ObjectNode request = metricMathRequest(); + put(request, queryProtocol); + + PersistentStorage reloaded = new PersistentStorage<>(storageFile, type); + reloaded.load(); + assertEquals(1, reloaded.keys().size(), "the stored alarm must deserialize after a restart"); + initialize(reloaded); + assertMetricMathAlarm(request); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void switchingBetweenSingleMetricAndMetricMathReplacesThePreviousDefinition(boolean queryProtocol) + throws Exception { + ObjectNode singleMetric = singleMetricRequest(); + put(singleMetric, queryProtocol); + assertSingleMetricAlarm(300); + + ObjectNode math = metricMathRequest(); + put(math, !queryProtocol); + assertMetricMathAlarm(math); + + singleMetric.put("Period", 120); + put(singleMetric, queryProtocol); + assertSingleMetricAlarm(120); + } + + @Test + void existingSingleMetricDefaultPeriodIsPreserved() throws Exception { + ObjectNode singleMetric = singleMetricRequest(); + singleMetric.remove("Period"); + put(singleMetric, false); + assertSingleMetricAlarm(60); + } + + private void put(ObjectNode request, boolean queryProtocol) { + if (queryProtocol) { + assertEquals(200, queryHandler.handle("PutMetricAlarm", toQuery(request), REGION).getStatus()); + } else { + assertEquals(200, jsonHandler.handle("PutMetricAlarm", request, REGION).getStatus()); + } + } + + private JsonNode describeJson() { + ObjectNode request = MAPPER.createObjectNode(); + request.putArray("AlarmNames").add(ALARM_NAME); + JsonNode response = (JsonNode) jsonHandler.handle("DescribeAlarms", request, REGION).getEntity(); + assertEquals(1, response.path("MetricAlarms").size()); + return response.path("MetricAlarms").get(0); + } + + private Document describeXml() throws Exception { + MultivaluedMap params = new MultivaluedHashMap<>(); + params.putSingle("AlarmNames.member.1", ALARM_NAME); + String xml = (String) queryHandler.handle("DescribeAlarms", params, REGION).getEntity(); + return XmlParser.parseDocument(xml); + } + + private void assertMetricMathAlarm(ObjectNode request) throws Exception { + JsonNode alarm = describeJson(); + assertEquals(request.path("Metrics"), alarm.path("Metrics")); + Document document = describeXml(); + XPath xpath = XPathFactory.newInstance().newXPath(); + for (String field : new String[]{"Period", "MetricName", "Namespace", "Statistic", "ExtendedStatistic", + "Unit", "Dimensions"}) { + assertFalse(alarm.has(field), "metric-math JSON must not contain " + field); + assertEquals("0", xpath.evaluate("count(" + XML_ALARM + "/" + field + ")", document), + "metric-math XML must not contain " + field); + } + assertEquals("3", xpath.evaluate("count(" + XML_ALARM + "/Metrics/member)", document)); + assertEquals("5", xpath.evaluate(XML_ALARM + "/Threshold", document).replace(".0", "")); + assertEquals("2", xpath.evaluate(XML_ALARM + "/EvaluationPeriods", document)); + assertEquals(5.0, alarm.path("Threshold").asDouble()); + assertEquals(2, alarm.path("EvaluationPeriods").asInt()); + assertXmlMatchesJson(request.path("Metrics"), XML_ALARM + "/Metrics", document, xpath); + } + + private void assertSingleMetricAlarm(int period) throws Exception { + JsonNode alarm = describeJson(); + assertEquals(period, alarm.path("Period").asInt()); + assertEquals("AWS/ApiGateway", alarm.path("Namespace").asText()); + assertEquals("5xx", alarm.path("MetricName").asText()); + assertEquals("Sum", alarm.path("Statistic").asText()); + assertFalse(alarm.has("Metrics")); + Document document = describeXml(); + XPath xpath = XPathFactory.newInstance().newXPath(); + assertEquals(Integer.toString(period), xpath.evaluate(XML_ALARM + "/Period", document)); + assertEquals("0", xpath.evaluate("count(" + XML_ALARM + "/Metrics)", document)); + } + + private static void assertXmlMatchesJson(JsonNode value, String path, Document document, XPath xpath) + throws Exception { + if (value.isArray()) { + assertEquals(Integer.toString(value.size()), xpath.evaluate("count(" + path + "/member)", document)); + for (int index = 0; index < value.size(); index++) { + assertXmlMatchesJson(value.get(index), path + "/member[" + (index + 1) + "]", document, xpath); + } + } else if (value.isObject()) { + for (Map.Entry field : value.properties()) { + assertXmlMatchesJson(field.getValue(), path + "/" + field.getKey(), document, xpath); + } + } else { + assertEquals(value.asText(), xpath.evaluate(path, document), path); + } + } + + private static MultivaluedMap toQuery(JsonNode value) { + MultivaluedMap params = new MultivaluedHashMap<>(); + flatten(value, "", params); + return params; + } + + private static void flatten(JsonNode value, String prefix, MultivaluedMap params) { + if (value.isArray()) { + for (int index = 0; index < value.size(); index++) { + flatten(value.get(index), prefix + ".member." + (index + 1), params); + } + } else if (value.isObject()) { + for (Map.Entry field : value.properties()) { + flatten(field.getValue(), prefix.isEmpty() ? field.getKey() : prefix + "." + field.getKey(), params); + } + } else { + params.putSingle(prefix, value.asText()); + } + } + + private static ObjectNode metricMathRequest() throws Exception { + return (ObjectNode) MAPPER.readTree(""" + {"AlarmName":"metric-math-round-trip","EvaluationPeriods":2,"Threshold":5, + "ComparisonOperator":"GreaterThanThreshold","Metrics":[ + {"Id":"rate","Expression":"100 * errors / requests","ReturnData":true, + "Period":300,"Label":"Error & requests"}, + {"Id":"errors","ReturnData":false,"AccountId":"123456789012","Label":"Errors", + "MetricStat":{"Metric":{"Namespace":"AWS/ApiGateway","MetricName":"5xx", + "Dimensions":[{"Name":"ApiName","Value":"api<&>"},{"Name":"Stage","Value":"test"}]}, + "Period":300,"Stat":"Sum","Unit":"Count"}}, + {"Id":"requests","ReturnData":false,"MetricStat":{ + "Metric":{"Namespace":"AWS/ApiGateway","MetricName":"Count"},"Period":300,"Stat":"Sum"}} + ]} + """); + } + + private static ObjectNode singleMetricRequest() throws Exception { + return (ObjectNode) MAPPER.readTree(""" + {"AlarmName":"metric-math-round-trip","Namespace":"AWS/ApiGateway","MetricName":"5xx", + "Statistic":"Sum","Period":300,"EvaluationPeriods":2,"Threshold":5, + "ComparisonOperator":"GreaterThanThreshold"} + """); + } +} -- 2.52.0 From ccdf32335d6586791a06fd4acd88fd4706140ef8 Mon Sep 17 00:00:00 2001 From: Allen Sanborn Date: Thu, 1 Oct 2026 12:21:47 -0500 Subject: [PATCH 2/9] fix(cloudwatch): raise ResourceNotFoundException when a tag operation names a resource that does not exist (#4783) * fix(cloudwatch): raise ResourceNotFoundException when a tag operation names a resource that does not exist TagResource, UntagResource and ListTagsForResource answered 200 for any ARN, so tagging a typo'd or deleted alarm, dashboard or metric stream told the caller the tag had been applied. AWS raises ResourceNotFoundException. All three operations now resolve the ARN first and report a 404 when nothing answers to it. The fix sits in the three services rather than in the handlers, because CloudWatch Metrics serves these operations over both Query and JSON 1.1 and the two handlers route to the same service methods. A handler-level check would have left the other protocol still answering 200. What the 200 was hiding is worth naming, because "return an error instead of 200" understates it. The write path resolved the ARN and then did nothing with the miss: tagResource and untagResource each ended in an ifPresent, so a tag against an unknown ARN was discarded, and ListTagsForResource answered the empty map. So the ARN was neither rejected nor recorded. Nothing was stored against a phantom resource, which is the one mercy here, but a caller that wrote a tag and read back an empty list had no way to tell "this resource has no tags" from "this resource does not exist". That is why ListTagsForResource is included rather than left alone. It is the one operation whose answer changes for a caller doing a speculative list, and the one whose leniency made the silent write survive a round trip. No test or caller in the repository depended on the empty-map answer for an unknown ARN: the only internal caller is CloudWatchDashboardCfnProvisioner.reconcileTags, and both of its call sites pass an ARN that putDashboard has just returned. The error code is ResourceNotFoundException, not the shorter ResourceNotFound that GetDashboard, DeleteDashboards and SetAlarmState answer with. Those are two separate shapes in the CloudWatch model, mapped to two exception classes by the SDK, and the model declares the longer one on exactly these three operations. CloudWatch Logs needed no change. Its own TagResource, UntagResource and ListTagsForResource already resolve the log group and raise ResourceNotFoundException, which the new test pins so the two halves of CloudWatch tagging cannot diverge in the other direction. * fix(cloudwatch): resolve a dashboard by either the regionless or the region-ful form of its ARN Dashboards are a global resource and AWS documents their ARN without a region, arn:aws:cloudwatch:::dashboard/, on both the TagResource and the ListTagsForResource pages. Floci mints a region-ful one. The dashboard lookup now accepts both shapes deliberately, so a caller holding either resolves. Accepting both is what stops the preceding commit regressing a caller that does the documented thing. Until the tag operations raised on a miss, the divergence was invisible on this path: the documented ARN matched no stored dashboard, and the answer was a silent 200 with an empty tag map. Turning a miss into ResourceNotFoundException would have turned that into a hard 404 for a dashboard that exists. Only an absent region stands in for the dashboard's own, so an ARN naming some other region is still a miss and does not resolve to a same-named dashboard here. What Floci mints is not changed. Clients may already hold the ARNs it has handed out, so bringing the minted shape into line with AWS alters values already in circulation and belongs in its own change; it is tracked separately. The tolerant lookup is all that is needed to keep this path correct, and it is strictly more permissive than before, so it cannot break a caller that works today. Alarms and metric streams were checked rather than assumed: both mint region-ful ARNs and AWS documents both region-ful, so the question is dashboards only. Also reworded the not-found message on the alarm path to name the ARN that did not resolve instead of asserting it was an alarm. Both tag handlers route every ARN that is not a dashboard and not a metric stream to that path, including kinds AWS considers taggable and Floci does not serve, a Contributor Insights insight-rule ARN being the one AWS documents. Reporting that no resource matches the ARN is true of those; "alarm not found" was not. No Contributor Insights support is added here. * test(cloudwatch): assert a refused tag call left the real resource untagged The not-found tests tagged the unknown ARN env=prod and then re-tagged the real resource env=dev before reading it back, so a failed call that had written through to the real resource would have been overwritten and the assertion would still pass. Read the real resource's tags first; they must be empty. * test(cloudwatch): import matchers and use configureAwsContentTypes Import Matchers.not and java.util.Map in the new tests instead of writing them inline, register the x-amz-json content types through RestAssuredJsonUtils.configureAwsContentTypes() in place of registerParser and the hand-built encoder config, and drop DeleteDashboards from the requireDashboard Javadoc: only GetDashboard declares ResourceNotFound. (cherry picked from commit 0c8d1848541392ff91991121d96586db564cb3f8) --- .../CloudWatchDashboardsService.java | 73 +++- .../metrics/CloudWatchMetricsService.java | 48 ++- .../CloudWatchMetricStreamsService.java | 27 +- ...atchTagUnknownResourceIntegrationTest.java | 349 ++++++++++++++++++ .../CloudWatchDashboardsServiceTest.java | 81 +++- .../metrics/CloudWatchMetricsTagsTest.java | 29 ++ .../CloudWatchMetricStreamsServiceTest.java | 25 ++ 7 files changed, 585 insertions(+), 47 deletions(-) create mode 100644 src/test/java/io/github/hectorvent/floci/services/cloudwatch/CloudWatchTagUnknownResourceIntegrationTest.java 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)); + } } -- 2.52.0 From 2e0a438d705c69df9da7683404a747758021131d Mon Sep 17 00:00:00 2001 From: mrjusepelo Date: Sun, 18 Oct 2026 10:15:34 -0500 Subject: [PATCH 3/9] fix(cloudwatch): publish SNS actions on alarm state transitions (#5918) (cherry picked from commit f5811c3a0735be634b6122503237330826e0ba4a) --- docs/services/cloudwatch.md | 8 ++ .../cloudwatch/metrics/AlarmSnsNotifier.java | 112 +++++++++++++++ .../metrics/CloudWatchMetricsService.java | 20 ++- .../metrics/AlarmSnsNotifierTest.java | 127 ++++++++++++++++++ 4 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java create mode 100644 src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifierTest.java diff --git a/docs/services/cloudwatch.md b/docs/services/cloudwatch.md index 121e9c5..34b16ef 100644 --- a/docs/services/cloudwatch.md +++ b/docs/services/cloudwatch.md @@ -331,6 +331,14 @@ aws logs put-retention-policy \ Metric streams are stored as definitions with their `running` or `stopped` state. Floci never delivers metrics to the Firehose delivery stream a metric stream names. +### SNS alarm actions + +State transitions publish to same-account standard SNS topics in `AlarmActions`, `OKActions`, or +`InsufficientDataActions`, including transitions from `SetAlarmState`. Unchanged states and +`ActionsEnabled=false` suppress notifications. Scaling action retries remain independent. +Failures are logged per target without blocking other targets. Notifications include a +single-metric `Trigger`; durable retries, FIFO topics, and metric-math payloads are not covered. + ### Metric-math alarm definitions `PutMetricAlarm` stores the `Metrics` query definitions and `DescribeAlarms` returns them over diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java new file mode 100644 index 0000000..a3b9f35 --- /dev/null +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java @@ -0,0 +1,112 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.github.hectorvent.floci.core.common.AwsArnUtils; +import io.github.hectorvent.floci.core.common.AwsPartition; +import io.github.hectorvent.floci.core.common.AwsPartitions; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; +import io.github.hectorvent.floci.services.sns.SnsService; +import jakarta.enterprise.context.ApplicationScoped; +import org.jboss.logging.Logger; + +import java.time.Instant; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +/** Publishes SNS actions once per state transition, independently of scaling action retries. */ +@ApplicationScoped +public class AlarmSnsNotifier { + + private static final Logger LOG = Logger.getLogger(AlarmSnsNotifier.class); + private static final DateTimeFormatter TIMESTAMP_FORMAT = DateTimeFormatter + .ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ", Locale.ROOT).withZone(ZoneOffset.UTC); + private final SnsService snsService; + private final ObjectMapper objectMapper; + + public AlarmSnsNotifier(SnsService snsService, ObjectMapper objectMapper) { + this.snsService = snsService; + this.objectMapper = objectMapper; + } + + public void notifyTransition(MetricAlarm alarm, String oldState) { + if (!alarm.isActionsEnabled() || alarm.getStateValue().equals(oldState)) { + return; + } + List actions = switch (alarm.getStateValue()) { + case "ALARM" -> alarm.getAlarmActions(); + case "OK" -> alarm.getOkActions(); + case "INSUFFICIENT_DATA" -> alarm.getInsufficientDataActions(); + default -> List.of(); + }; + if (actions == null) { + return; + } + for (String action : actions) { + if (!AwsArnUtils.isArnFor(action, "sns")) { + continue; + } + try { + AwsArnUtils.Arn topic = AwsArnUtils.parse(action); + String subject = alarm.getStateValue() + ": \"" + alarm.getAlarmName() + "\""; + if (subject.length() > 100) { + subject = subject.substring(0, 100); + } + snsService.publish(action, null, message(alarm, oldState), subject, topic.region()); + } catch (Exception e) { + LOG.warnv(e, "Unable to publish alarm {0} state {1} to SNS action {2}", + alarm.getAlarmName(), alarm.getStateValue(), action); + } + } + } + + private String message(MetricAlarm alarm, String oldState) throws JsonProcessingException { + AwsArnUtils.Arn arn = AwsArnUtils.parse(alarm.getAlarmArn()); + Map payload = new LinkedHashMap<>(); + payload.put("AlarmName", alarm.getAlarmName()); + payload.put("AlarmDescription", alarm.getAlarmDescription()); + payload.put("AWSAccountId", arn.accountId()); + payload.put("AlarmConfigurationUpdatedTimestamp", timestamp(alarm.getAlarmConfigurationUpdatedTimestamp())); + payload.put("NewStateValue", alarm.getStateValue()); + payload.put("NewStateReason", alarm.getStateReason()); + payload.put("StateChangeTime", timestamp(alarm.getStateUpdatedTimestamp())); + String regionName = AwsPartitions.forRegion(arn.region()).stream() + .flatMap(partition -> partition.regions().stream()) + .filter(region -> region.id().equals(arn.region())) + .map(AwsPartition.Region::description).findFirst().orElse(arn.region()); + payload.put("Region", regionName); + payload.put("AlarmArn", alarm.getAlarmArn()); + payload.put("OldStateValue", oldState); + payload.put("OKActions", alarm.getOkActions()); + payload.put("AlarmActions", alarm.getAlarmActions()); + payload.put("InsufficientDataActions", alarm.getInsufficientDataActions()); + Map trigger = new LinkedHashMap<>(); + trigger.put("MetricName", alarm.getMetricName()); + trigger.put("Namespace", alarm.getNamespace()); + trigger.put("StatisticType", "Statistic"); + trigger.put("Statistic", alarm.getStatistic() != null + ? alarm.getStatistic().toUpperCase(Locale.ROOT) : null); + trigger.put("Unit", alarm.getUnit()); + trigger.put("Dimensions", alarm.getDimensions()); + trigger.put("Period", alarm.getPeriod()); + trigger.put("EvaluationPeriods", alarm.getEvaluationPeriods()); + if (alarm.getDatapointsToAlarm() != null) { + trigger.put("DatapointsToAlarm", alarm.getDatapointsToAlarm()); + } + trigger.put("ComparisonOperator", alarm.getComparisonOperator()); + trigger.put("Threshold", alarm.getThreshold()); + trigger.put("TreatMissingData", alarm.getTreatMissingData()); + trigger.put("EvaluateLowSampleCountPercentile", alarm.getEvaluateLowSampleCountPercentile() != null + ? alarm.getEvaluateLowSampleCountPercentile() : ""); + payload.put("Trigger", trigger); + return objectMapper.writeValueAsString(payload); + } + + private String timestamp(long epochSeconds) { + return TIMESTAMP_FORMAT.format(Instant.ofEpochSecond(epochSeconds)); + } +} 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 6ea32a1..d7c35e2 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 @@ -30,22 +30,36 @@ public class CloudWatchMetricsService { private final StorageBackend metricStore; private final StorageBackend alarmStore; private final RegionResolver regionResolver; + private final AlarmSnsNotifier alarmSnsNotifier; + + public CloudWatchMetricsService(StorageFactory storageFactory, RegionResolver regionResolver) { + this(storageFactory, regionResolver, null); + } @Inject - public CloudWatchMetricsService(StorageFactory storageFactory, RegionResolver regionResolver) { + public CloudWatchMetricsService(StorageFactory storageFactory, RegionResolver regionResolver, + AlarmSnsNotifier alarmSnsNotifier) { this.metricStore = storageFactory.create("cloudwatchmetrics", "cwmetrics.json", new TypeReference>() {}); this.alarmStore = storageFactory.create("cloudwatchmetrics", "cwalarms.json", new TypeReference>() {}); this.regionResolver = regionResolver; + this.alarmSnsNotifier = alarmSnsNotifier; } CloudWatchMetricsService(StorageBackend metricStore, StorageBackend alarmStore, RegionResolver regionResolver) { + this(metricStore, alarmStore, regionResolver, null); + } + + CloudWatchMetricsService(StorageBackend metricStore, + StorageBackend alarmStore, + RegionResolver regionResolver, AlarmSnsNotifier alarmSnsNotifier) { this.metricStore = metricStore; this.alarmStore = alarmStore; this.regionResolver = regionResolver; + this.alarmSnsNotifier = alarmSnsNotifier; } public void putMetricData(String namespace, List datums, String region) { @@ -323,6 +337,7 @@ public class CloudWatchMetricsService { MetricAlarm alarm = alarmStore.get(key) .orElseThrow(() -> new AwsException("ResourceNotFound", "Alarm not found: " + alarmName, 404)); + String oldState = alarm.getStateValue(); alarm.setStateValue(stateValue); alarm.setStateReason(stateReason); alarm.setStateReasonData(stateReasonData); @@ -330,6 +345,9 @@ public class CloudWatchMetricsService { alarmStore.put(key, alarm); LOG.infov("SetAlarmState: {0} -> {1}", alarmName, stateValue); + if (alarmSnsNotifier != null) { + alarmSnsNotifier.notifyTransition(alarm, oldState); + } } /** diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifierTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifierTest.java new file mode 100644 index 0000000..8a638b6 --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifierTest.java @@ -0,0 +1,127 @@ +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.RegionResolver; +import io.github.hectorvent.floci.core.storage.InMemoryStorage; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; +import io.github.hectorvent.floci.services.sns.SnsService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import java.time.Instant; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +class AlarmSnsNotifierTest { + private static final String REGION = "us-east-1"; + private static final String TOPIC = "arn:aws:sns:us-east-1:000000000000:alarms"; + private final ObjectMapper mapper = new ObjectMapper(); + private SnsService sns; + private CloudWatchMetricsService service; + private MetricAlarm alarm; + + @BeforeEach + void setUp() { + sns = mock(SnsService.class); + service = new CloudWatchMetricsService(new InMemoryStorage<>(), new InMemoryStorage<>(), + new RegionResolver(REGION, "000000000000"), new AlarmSnsNotifier(sns, mapper)); + alarm = new MetricAlarm(); + alarm.setAlarmName("test-alarm"); + alarm.setActionsEnabled(true); + alarm.setAlarmActions(List.of(TOPIC)); + alarm.setOkActions(List.of(TOPIC)); + alarm.setInsufficientDataActions(List.of(TOPIC)); + alarm.setEvaluationPeriods(3); + alarm.setStatistic("Average"); + alarm.setMetricName("Errors"); + alarm.setNamespace("Local/PoC"); + service.putMetricAlarm(alarm, REGION); + } + + @Test + void publishesAwsPayloadForEachTransitionButNotRepeatedState() throws Exception { + service.setAlarmState("test-alarm", "ALARM", "breached", null, REGION); + service.setAlarmState("test-alarm", "ALARM", "still breached", null, REGION); + service.setAlarmState("test-alarm", "OK", "recovered", null, REGION); + service.setAlarmState("test-alarm", "INSUFFICIENT_DATA", "missing", null, REGION); + ArgumentCaptor messages = ArgumentCaptor.forClass(String.class); + verify(sns, times(3)).publish(eq(TOPIC), isNull(), messages.capture(), anyString(), eq(REGION)); + JsonNode first = mapper.readTree(messages.getAllValues().getFirst()); + assertEquals("test-alarm", first.path("AlarmName").asText()); + assertEquals("INSUFFICIENT_DATA", first.path("OldStateValue").asText()); + assertEquals("ALARM", first.path("NewStateValue").asText()); + assertEquals("000000000000", first.path("AWSAccountId").asText()); + assertEquals(alarm.getAlarmArn(), first.path("AlarmArn").asText()); + assertEquals("Errors", first.path("Trigger").path("MetricName").asText()); + assertFalse(first.path("Trigger").has("DatapointsToAlarm")); + assertEquals("AVERAGE", first.path("Trigger").path("Statistic").asText()); + assertEquals("", first.path("Trigger").path("EvaluateLowSampleCountPercentile").asText()); + assertTrue(first.path("Trigger").path("EvaluateLowSampleCountPercentile").isTextual()); + assertNull(alarm.getDatapointsToAlarm()); + assertEquals("OK", mapper.readTree(messages.getAllValues().get(2)).path("OldStateValue").asText()); + } + + @Test + void formatsBothTimestampsWithMillisAndUtcOffset() throws Exception { + alarm.setAlarmConfigurationUpdatedTimestamp(1716280510L); + service.setAlarmState("test-alarm", "ALARM", "breached", null, REGION); + ArgumentCaptor message = ArgumentCaptor.forClass(String.class); + verify(sns).publish(eq(TOPIC), isNull(), message.capture(), anyString(), eq(REGION)); + JsonNode payload = mapper.readTree(message.getValue()); + assertEquals("2024-05-21T08:35:10.000+0000", + payload.path("AlarmConfigurationUpdatedTimestamp").asText()); + String stateChangeTime = payload.path("StateChangeTime").asText(); + assertTrue(stateChangeTime + .matches("[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\\.[0-9]{3}\\+0000")); + assertEquals(alarm.getStateUpdatedTimestamp(), + Instant.parse(stateChangeTime.replace("+0000", "Z")).getEpochSecond()); + } + + @Test + void preservesExplicitDatapointsToAlarmInPayload() throws Exception { + alarm.setDatapointsToAlarm(2); + alarm.setStatistic("Maximum"); + alarm.setEvaluateLowSampleCountPercentile("ignore"); + service.setAlarmState("test-alarm", "ALARM", "breached", null, REGION); + ArgumentCaptor message = ArgumentCaptor.forClass(String.class); + verify(sns).publish(eq(TOPIC), isNull(), message.capture(), anyString(), eq(REGION)); + assertEquals(2, mapper.readTree(message.getValue()).path("Trigger").path("DatapointsToAlarm").intValue()); + JsonNode trigger = mapper.readTree(message.getValue()).path("Trigger"); + assertEquals("MAXIMUM", trigger.path("Statistic").asText()); + assertEquals("ignore", trigger.path("EvaluateLowSampleCountPercentile").asText()); + assertEquals(2, alarm.getDatapointsToAlarm()); + } + + @Test + void disabledActionsDoNotPublish() { + alarm.setActionsEnabled(false); + service.setAlarmState("test-alarm", "ALARM", "breached", null, REGION); + verifyNoInteractions(sns); + assertEquals("ALARM", alarm.getStateValue()); + } + + @Test + void failedTargetDoesNotPreventOtherTargetsOrStateChange() { + String missing = "arn:aws:sns:us-east-1:000000000000:missing"; + alarm.setAlarmActions(List.of(missing, "arn:aws:autoscaling:us-east-1:000000000000:scalingPolicy:x", TOPIC)); + doThrow(new IllegalStateException("missing topic")) + .when(sns).publish(eq(missing), isNull(), anyString(), anyString(), eq(REGION)); + assertDoesNotThrow(() -> service.setAlarmState("test-alarm", "ALARM", "breached", null, REGION)); + verify(sns).publish(eq(TOPIC), isNull(), anyString(), anyString(), eq(REGION)); + assertEquals("ALARM", alarm.getStateValue()); + } + + @Test + void usesTopicRegionAndPreservesChinaPartition() { + String topic = "arn:aws-cn:sns:cn-north-1:000000000000:alarms"; + alarm.setAlarmArn("arn:aws-cn:cloudwatch:cn-north-1:000000000000:alarm:test-alarm"); + alarm.setAlarmActions(List.of(topic)); + service.setAlarmState("test-alarm", "ALARM", "breached", null, REGION); + verify(sns).publish(eq(topic), isNull(), contains("arn:aws-cn:cloudwatch:"), anyString(), eq("cn-north-1")); + } +} -- 2.52.0 From 21625c5a1b003a1958815e5fa58210e8ee4c0d71 Mon Sep 17 00:00:00 2001 From: Mohith Date: Tue, 20 Oct 2026 17:49:42 +0530 Subject: [PATCH 4/9] fix(cloudwatch): store datums sent in PutMetricData EntityMetricData (#6070) The CloudWatch Agent sends PutMetricData with an empty top-level MetricData and every datum nested under EntityMetricData[].MetricData. Both the JSON 1.0 handler (also reached by Smithy RPC v2 CBOR) and the Query handler read only the top-level list, so the agent's metrics were silently dropped. Both handlers now append each EntityMetricData entry's datums to the top-level ones and store them under the request Namespace, with the same parsing as top-level datums. Entity and StrictEntityValidation are accepted and ignored. The per-request INFO logs of the raw request and parsed datums are lowered to a DEBUG count. Fixes #6066 (cherry picked from commit 03f57303969561b049c3ebba38f1fd9d2b68df4c) --- .../java/com/floci/test/CloudWatchTest.java | 48 ++++++ .../metrics/CloudWatchMetricsJsonHandler.java | 7 +- .../CloudWatchMetricsQueryHandler.java | 44 +++-- ...etricsEntityMetricDataIntegrationTest.java | 156 ++++++++++++++++++ .../CloudWatchMetricsJsonHandlerTest.java | 59 +++++++ .../CloudWatchMetricsQueryHandlerTest.java | 58 +++++++ 6 files changed, 358 insertions(+), 14 deletions(-) create mode 100644 src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java diff --git a/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java b/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java index 8f4ecca..e48b58a 100644 --- a/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java +++ b/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java @@ -6,6 +6,7 @@ import software.amazon.awssdk.services.cloudwatch.model.*; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.Map; import static org.assertj.core.api.Assertions.*; @@ -185,4 +186,51 @@ class CloudWatchTest { ); assertThat(noMatch.datapoints()).isEmpty(); } + + @Test + @DisplayName("PutMetricData stores datums nested under EntityMetricData") + void testPutMetricDataWithEntityMetricData() { + String namespace = "JavaTestEntityMetricData"; + Instant now = Instant.now(); + + cloudWatchClient.putMetricData(request -> request + .namespace(namespace) + .metricData(datum -> datum + .metricName("TopLevel") + .value(1.0) + .timestamp(now)) + .entityMetricData(entityMetricData -> entityMetricData + .entity(entity -> entity + .keyAttributes(Map.of( + "Type", "AWS::Resource", + "ResourceType", "AWS::EC2::Instance", + "Identifier", "i-0123456789abcdef0"))) + .metricData(MetricDatum.builder() + .metricName("mem_used_percent") + .value(6.5) + .unit(StandardUnit.PERCENT) + .storageResolution(60) + .timestamp(now) + .dimensions(Dimension.builder().name("host").value("h1").build()) + .build())) + .strictEntityValidation(false) + ); + + ListMetricsResponse listed = cloudWatchClient.listMetrics(request -> request.namespace(namespace)); + assertThat(listed.metrics()) + .extracting(Metric::metricName) + .containsExactlyInAnyOrder("TopLevel", "mem_used_percent"); + + GetMetricStatisticsResponse stats = cloudWatchClient.getMetricStatistics(request -> request + .namespace(namespace) + .metricName("mem_used_percent") + .dimensions(Dimension.builder().name("host").value("h1").build()) + .startTime(now.minus(1, ChronoUnit.HOURS)) + .endTime(now.plus(1, ChronoUnit.MINUTES)) + .period(3600) + .statistics(Statistic.SUM) + ); + assertThat(stats.datapoints()).hasSize(1); + assertThat(stats.datapoints().get(0).sum()).isEqualTo(6.5); + } } diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java index 5aa036d..acba84d 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java @@ -82,10 +82,11 @@ public class CloudWatchMetricsJsonHandler { private Response handlePutMetricData(JsonNode request, String region) { String namespace = request.path("Namespace").asText(); - LOG.infov("JSON PutMetricData raw: {0}", request); List datums = parseMetricDataJson(request.path("MetricData")); - LOG.infov("JSON PutMetricData parsed {0} datums, sums={1}", datums.size(), - datums.stream().map(d -> d.getMetricName() + "=" + d.getSum()).toList()); + for (JsonNode entityMetricData : request.path("EntityMetricData")) { + datums.addAll(parseMetricDataJson(entityMetricData.path("MetricData"))); + } + LOG.debugv("JSON PutMetricData parsed {0} datums for namespace {1}", datums.size(), namespace); metricsService.putMetricData(namespace, datums, region); return Response.ok(objectMapper.createObjectNode()).build(); } diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java index 6d67684..4865b89 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java @@ -26,11 +26,16 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import org.jboss.logging.Logger; +import java.util.SortedSet; +import java.util.TreeSet; +import java.util.regex.Matcher; +import java.util.regex.Pattern; @ApplicationScoped public class CloudWatchMetricsQueryHandler { private static final Logger LOG = Logger.getLogger(CloudWatchMetricsQueryHandler.class); + private static final Pattern ENTITY_METRIC_DATA_MEMBER = Pattern.compile("EntityMetricData\\.member\\.(\\d{1,9})\\."); private CloudWatchMetricsService metricsService; private final CloudWatchDashboardsService dashboardsService; @@ -591,42 +596,59 @@ public class CloudWatchMetricsQueryHandler { // ──────────────────────────── Parsing Helpers ──────────────────────────── private List parseMetricData(MultivaluedMap params) { - LOG.infov("Query PutMetricData params: {0}", params); + List datums = parseMetricData(params, "MetricData"); + // An empty EntityMetricData entry serializes to no keys, so its index is absent + // while later entries keep theirs: iterate the indexes present, not 1..first gap. + SortedSet entityIndexes = new TreeSet<>(); + for (String key : params.keySet()) { + Matcher matcher = ENTITY_METRIC_DATA_MEMBER.matcher(key); + if (matcher.lookingAt()) { + entityIndexes.add(Integer.parseInt(matcher.group(1))); + } + } + for (int index : entityIndexes) { + datums.addAll(parseMetricData(params, "EntityMetricData.member." + index + ".MetricData")); + } + LOG.debugv("Query PutMetricData parsed {0} datums", datums.size()); + return datums; + } + + private List parseMetricData(MultivaluedMap params, String listPrefix) { List datums = new ArrayList<>(); for (int i = 1; ; i++) { - String metricName = params.getFirst("MetricData.member." + i + ".MetricName"); + String metricName = params.getFirst(listPrefix + ".member." + i + ".MetricName"); if (metricName == null) { break; } MetricDatum datum = new MetricDatum(); datum.setMetricName(metricName); - datum.setUnit(params.getFirst("MetricData.member." + i + ".Unit")); + datum.setUnit(params.getFirst(listPrefix + ".member." + i + ".Unit")); - String valueStr = params.getFirst("MetricData.member." + i + ".Value"); + String valueStr = params.getFirst(listPrefix + ".member." + i + ".Value"); if (valueStr != null) { datum.setValue(parseDouble(valueStr, 0)); } - String tsStr = params.getFirst("MetricData.member." + i + ".Timestamp"); + String tsStr = params.getFirst(listPrefix + ".member." + i + ".Timestamp"); if (tsStr != null) { Instant ts = parseInstant(tsStr); datum.setTimestamp(ts != null ? ts.getEpochSecond() : 0); } // StatisticValues - String sc = params.getFirst("MetricData.member." + i + ".StatisticValues.SampleCount"); + String sc = params.getFirst(listPrefix + ".member." + i + ".StatisticValues.SampleCount"); if (sc != null) { datum.setSampleCount(parseDouble(sc, 0)); - datum.setSum(parseDouble(params.getFirst("MetricData.member." + i + ".StatisticValues.Sum"), 0)); - datum.setMinimum(parseDouble(params.getFirst("MetricData.member." + i + ".StatisticValues.Minimum"), 0)); - datum.setMaximum(parseDouble(params.getFirst("MetricData.member." + i + ".StatisticValues.Maximum"), 0)); + datum.setSum(parseDouble(params.getFirst(listPrefix + ".member." + i + ".StatisticValues.Sum"), 0)); + datum.setMinimum(parseDouble(params.getFirst(listPrefix + ".member." + i + ".StatisticValues.Minimum"), 0)); + datum.setMaximum(parseDouble(params.getFirst(listPrefix + ".member." + i + ".StatisticValues.Maximum"), 0)); } // Dimensions List dims = new ArrayList<>(); for (int j = 1; ; j++) { - String dimName = params.getFirst("MetricData.member." + i + ".Dimensions.member." + j + ".Name"); - String dimValue = params.getFirst("MetricData.member." + i + ".Dimensions.member." + j + ".Value"); + String dimName = params.getFirst(listPrefix + ".member." + i + ".Dimensions.member." + j + ".Name"); + String dimValue = params.getFirst(listPrefix + ".member." + i + ".Dimensions.member." + j + ".Value"); if (dimName == null) { break; } diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java new file mode 100644 index 0000000..890bceb --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java @@ -0,0 +1,156 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics; + +import io.quarkus.test.junit.QuarkusTest; +import io.restassured.RestAssured; +import io.restassured.config.EncoderConfig; +import io.restassured.http.ContentType; +import io.restassured.parsing.Parser; +import io.restassured.specification.RequestSpecification; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.time.Instant; + +import static io.restassured.RestAssured.config; +import static io.restassured.RestAssured.given; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; + +/** + * PutMetricData datums nested under EntityMetricData, as the CloudWatch Agent sends them, + * must be stored under the request namespace on every protocol path. floci-io/floci#6066. + */ +@QuarkusTest +class CloudWatchMetricsEntityMetricDataIntegrationTest { + + private static final String CW_AUTH = + "AWS4-HMAC-SHA256 Credential=test/20261020/us-east-1/monitoring/aws4_request"; + private static final String JSON_1_0 = "application/x-amz-json-1.0"; + + @BeforeAll + static void registerJsonParser() { + RestAssured.registerParser(JSON_1_0, Parser.JSON); + } + + private static RequestSpecification json(String operation) { + return given() + .header("Authorization", CW_AUTH) + .contentType(JSON_1_0) + .config(config().encoderConfig(EncoderConfig.encoderConfig() + .encodeContentTypeAs(JSON_1_0, ContentType.TEXT))) + .header("X-Amz-Target", "GraniteServiceVersion20100801." + operation); + } + + private static RequestSpecification query(String action) { + return given() + .header("Authorization", CW_AUTH) + .contentType("application/x-www-form-urlencoded") + .formParam("Action", action) + .formParam("Version", "2010-08-01"); + } + + @Test + void jsonEntityMetricDataOnlyIsListedAndAggregated() { + long now = Instant.now().getEpochSecond(); + json("PutMetricData") + .body(""" + {"Namespace":"CWAgent/Json","MetricData":[], + "EntityMetricData":[{ + "Entity":{"KeyAttributes":{"Type":"AWS::Resource", + "ResourceType":"AWS::EC2::Instance","Identifier":"i-0123456789abcdef0"}, + "Attributes":{}}, + "MetricData":[ + {"MetricName":"mem_used_percent","Value":6.5,"Unit":"Percent", + "StorageResolution":60,"Timestamp":%d, + "Dimensions":[{"Name":"host","Value":"h1"}]}, + {"MetricName":"disk_used_percent","Value":0.75,"Unit":"Percent", + "StorageResolution":60,"Timestamp":%d, + "Dimensions":[{"Name":"host","Value":"h1"}]}]}], + "StrictEntityValidation":false} + """.formatted(now, now)) + .when().post("/") + .then().statusCode(200); + + json("ListMetrics") + .body("{\"Namespace\":\"CWAgent/Json\"}") + .when().post("/") + .then() + .statusCode(200) + .body("Metrics.MetricName", containsInAnyOrder("mem_used_percent", "disk_used_percent")); + + json("GetMetricStatistics") + .body(""" + {"Namespace":"CWAgent/Json","MetricName":"mem_used_percent", + "Dimensions":[{"Name":"host","Value":"h1"}], + "StartTime":%d,"EndTime":%d,"Period":60,"Statistics":["Sum"]} + """.formatted(now - 120, now + 120)) + .when().post("/") + .then() + .statusCode(200) + .body("Datapoints.Sum", hasItem(6.5f)); + } + + @Test + void jsonTopLevelAndEntityMetricDataAreBothStored() { + json("PutMetricData") + .body(""" + {"Namespace":"CWAgent/JsonMixed", + "MetricData":[{"MetricName":"TopLevel","Value":1}], + "EntityMetricData":[{"Entity":{"KeyAttributes":{"Type":"Service", + "Name":"checkout","Environment":"prod"}}, + "MetricData":[{"MetricName":"FromEntity","Value":2}]}]} + """) + .when().post("/") + .then().statusCode(200); + + json("ListMetrics") + .body("{\"Namespace\":\"CWAgent/JsonMixed\"}") + .when().post("/") + .then() + .statusCode(200) + .body("Metrics.MetricName", containsInAnyOrder("TopLevel", "FromEntity")); + } + + @Test + void queryTopLevelAndEntityMetricDataAreListedAndAggregated() { + Instant now = Instant.now(); + query("PutMetricData") + .formParam("Namespace", "CWAgent/Query") + .formParam("MetricData.member.1.MetricName", "TopLevel") + .formParam("MetricData.member.1.Value", "1") + .formParam("EntityMetricData.member.1.Entity.KeyAttributes.entry.1.key", "Type") + .formParam("EntityMetricData.member.1.Entity.KeyAttributes.entry.1.value", "AWS::Resource") + .formParam("EntityMetricData.member.1.MetricData.member.1.MetricName", "mem_used_percent") + .formParam("EntityMetricData.member.1.MetricData.member.1.Value", "6.5") + .formParam("EntityMetricData.member.1.MetricData.member.1.Timestamp", now.toString()) + .formParam("EntityMetricData.member.1.MetricData.member.1.Dimensions.member.1.Name", "host") + .formParam("EntityMetricData.member.1.MetricData.member.1.Dimensions.member.1.Value", "h1") + .formParam("StrictEntityValidation", "false") + .when().post("/") + .then().statusCode(200); + + query("ListMetrics") + .formParam("Namespace", "CWAgent/Query") + .when().post("/") + .then() + .statusCode(200) + .body("ListMetricsResponse.ListMetricsResult.Metrics.member.MetricName", + containsInAnyOrder("TopLevel", "mem_used_percent")); + + query("GetMetricStatistics") + .formParam("Namespace", "CWAgent/Query") + .formParam("MetricName", "mem_used_percent") + .formParam("Dimensions.member.1.Name", "host") + .formParam("Dimensions.member.1.Value", "h1") + .formParam("StartTime", now.minusSeconds(120).toString()) + .formParam("EndTime", now.plusSeconds(120).toString()) + .formParam("Period", "60") + .formParam("Statistics.member.1", "Sum") + .when().post("/") + .then() + .statusCode(200) + .body("GetMetricStatisticsResponse.GetMetricStatisticsResult.Datapoints.member.Sum", + equalTo("6.5")); + } +} diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java index 89628e6..9b57294 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java @@ -17,6 +17,12 @@ import java.math.BigDecimal; import java.time.Instant; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Handler-level tests for CloudWatchMetricsJsonHandler. @@ -146,6 +152,59 @@ class CloudWatchMetricsJsonHandlerTest { "metric stored with 24h-ago timestamp must not appear in a 20-second window around now"); } + // The CloudWatch Agent sends an empty top-level MetricData and nests every datum under + // EntityMetricData, floci-io/floci#6066. + @Test + void putMetricData_entityMetricDataOnly_storesNestedDatumsUnderTheRequestNamespace() { + ObjectNode req = MAPPER.createObjectNode(); + req.put("Namespace", "CWAgent"); + req.putArray("MetricData"); + ObjectNode entityMetricData = req.putArray("EntityMetricData").addObject(); + ObjectNode entity = entityMetricData.putObject("Entity"); + entity.putObject("KeyAttributes") + .put("Type", "AWS::Resource") + .put("ResourceType", "AWS::EC2::Instance") + .put("Identifier", "i-0123456789abcdef0"); + entity.putObject("Attributes"); + ObjectNode datum = entityMetricData.putArray("MetricData").addObject(); + datum.put("MetricName", "mem_used_percent"); + datum.put("Value", 6.43); + datum.put("Unit", "Percent"); + datum.put("StorageResolution", 60); + datum.put("Timestamp", sdkTimestamp(EPOCH_NOW)); + datum.putArray("Dimensions").addObject().put("Name", "host").put("Value", "h1"); + req.put("StrictEntityValidation", false); + + assertEquals(200, handler.handle("PutMetricData", req, REGION).getStatus()); + + ObjectNode stats = getStats("CWAgent", "mem_used_percent", "host", "h1", + EPOCH_NOW.minusSeconds(60), EPOCH_NOW.plusSeconds(60), 60); + assertEquals(1, stats.get("Datapoints").size()); + assertEquals(6.43, stats.get("Datapoints").get(0).get("Sum").asDouble()); + } + + @Test + void putMetricData_topLevelAndEntityMetricData_storesBoth() { + ObjectNode req = MAPPER.createObjectNode(); + req.put("Namespace", "Mixed"); + req.putArray("MetricData").addObject() + .put("MetricName", "TopLevel").put("Value", 1.0); + ArrayNode entityMetricData = req.putArray("EntityMetricData"); + entityMetricData.addObject().putArray("MetricData").addObject() + .put("MetricName", "FirstEntity").put("Value", 2.0); + entityMetricData.addObject().putArray("MetricData").addObject() + .put("MetricName", "SecondEntity").put("Value", 3.0); + + assertEquals(200, handler.handle("PutMetricData", req, REGION).getStatus()); + + ObjectNode listReq = MAPPER.createObjectNode(); + listReq.put("Namespace", "Mixed"); + ObjectNode listed = (ObjectNode) handler.handle("ListMetrics", listReq, REGION).getEntity(); + List names = new ArrayList<>(); + listed.get("Metrics").forEach(m -> names.add(m.get("MetricName").asText())); + assertEquals(List.of("FirstEntity", "SecondEntity", "TopLevel"), names.stream().sorted().toList()); + } + @Test void setAlarmState_updatesFieldsCorrectly() { putMetric("NS", "M", "type", "current", 100.0, EPOCH_NOW); diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java index 6f465c2..94e7e3f 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java @@ -1,6 +1,8 @@ package io.github.hectorvent.floci.services.cloudwatch.metrics; import io.github.hectorvent.floci.services.cloudwatch.dashboards.CloudWatchDashboardsService; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricDatum; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.CloudWatchMetricStreamsService; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; import jakarta.ws.rs.core.MultivaluedHashMap; @@ -17,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -123,4 +126,59 @@ class CloudWatchMetricsQueryHandlerTest { anyInt(), any(), unitCaptor.capture(), any()); assertEquals("Count", unitCaptor.getValue()); } + + // The Query serialization of EntityMetricData nests each datum two lists deep; + // floci-io/floci#6066 is the JSON-side report of the same dropped datums. + @Test + void putMetricDataStoresTopLevelAndEntityMetricDataUnderTheRequestNamespace() { + MultivaluedMap params = new MultivaluedHashMap<>(); + params.putSingle("Namespace", "CWAgent"); + params.putSingle("MetricData.member.1.MetricName", "TopLevel"); + params.putSingle("MetricData.member.1.Value", "1"); + params.putSingle("EntityMetricData.member.1.Entity.KeyAttributes.entry.1.key", "Type"); + params.putSingle("EntityMetricData.member.1.Entity.KeyAttributes.entry.1.value", "AWS::Resource"); + params.putSingle("EntityMetricData.member.1.MetricData.member.1.MetricName", "mem_used_percent"); + params.putSingle("EntityMetricData.member.1.MetricData.member.1.Value", "6.5"); + params.putSingle("EntityMetricData.member.1.MetricData.member.1.Unit", "Percent"); + params.putSingle("EntityMetricData.member.1.MetricData.member.1.Dimensions.member.1.Name", "host"); + params.putSingle("EntityMetricData.member.1.MetricData.member.1.Dimensions.member.1.Value", "h1"); + params.putSingle("EntityMetricData.member.2.MetricData.member.1.MetricName", "disk_used_percent"); + params.putSingle("EntityMetricData.member.2.MetricData.member.1.Value", "0.75"); + params.putSingle("StrictEntityValidation", "false"); + + handler.handle("PutMetricData", params, REGION); + + @SuppressWarnings("unchecked") + ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); + verify(metricsService).putMetricData(eq("CWAgent"), captor.capture(), eq(REGION)); + List datums = captor.getValue(); + assertEquals(List.of("TopLevel", "mem_used_percent", "disk_used_percent"), + datums.stream().map(MetricDatum::getMetricName).toList()); + MetricDatum entityDatum = datums.get(1); + assertEquals(6.5, entityDatum.getValue()); + assertEquals("Percent", entityDatum.getUnit()); + assertEquals("host", entityDatum.getDimensions().get(0).name()); + assertEquals("h1", entityDatum.getDimensions().get(0).value()); + } + + @Test + void putMetricDataStoresEntityMetricDataAfterAnEmptyEntityEntry() { + MultivaluedMap params = new MultivaluedHashMap<>(); + params.putSingle("Namespace", "CWAgent"); + params.putSingle("EntityMetricData.member.2.MetricData.member.1.MetricName", "M"); + params.putSingle("EntityMetricData.member.2.MetricData.member.1.Value", "1"); + params.putSingle("EntityMetricData.member.10.MetricData.member.1.MetricName", "N"); + params.putSingle("EntityMetricData.member.10.MetricData.member.1.Value", "2"); + params.putSingle("StrictEntityValidation", "false"); + + handler.handle("PutMetricData", params, REGION); + + @SuppressWarnings("unchecked") + ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); + verify(metricsService).putMetricData(eq("CWAgent"), captor.capture(), eq(REGION)); + List datums = captor.getValue(); + assertEquals(List.of("M", "N"), datums.stream().map(MetricDatum::getMetricName).toList()); + assertEquals(1.0, datums.get(0).getValue()); + assertEquals(2.0, datums.get(1).getValue()); + } } -- 2.52.0 From e90978ae9472376bc1c2ec03e475fa2a793319a7 Mon Sep 17 00:00:00 2001 From: Zakaria Date: Sat, 24 Oct 2026 22:19:09 -0400 Subject: [PATCH 5/9] fix(cloudwatch): name the alarm's region without the partition catalog The SNS alarm notification from upstream (#5918) looks up the region's display name in AwsPartitions, which upstream added in a partition rework not taken here. A table of the commercial regions' names gives the same Region field. --- .../cloudwatch/metrics/AlarmSnsNotifier.java | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java index a3b9f35..41332ef 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java @@ -3,8 +3,6 @@ package io.github.hectorvent.floci.services.cloudwatch.metrics; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import io.github.hectorvent.floci.core.common.AwsArnUtils; -import io.github.hectorvent.floci.core.common.AwsPartition; -import io.github.hectorvent.floci.core.common.AwsPartitions; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; import io.github.hectorvent.floci.services.sns.SnsService; import jakarta.enterprise.context.ApplicationScoped; @@ -22,6 +20,38 @@ import java.util.Map; @ApplicationScoped public class AlarmSnsNotifier { + /** The names CloudWatch writes in the alarm message's Region field; other regions use their ID. */ + private static final Map REGION_NAMES = Map.ofEntries( + Map.entry("us-east-1", "US East (N. Virginia)"), + Map.entry("us-east-2", "US East (Ohio)"), + Map.entry("us-west-1", "US West (N. California)"), + Map.entry("us-west-2", "US West (Oregon)"), + Map.entry("ca-central-1", "Canada (Central)"), + Map.entry("ca-west-1", "Canada West (Calgary)"), + Map.entry("sa-east-1", "South America (Sao Paulo)"), + Map.entry("eu-west-1", "EU (Ireland)"), + Map.entry("eu-west-2", "EU (London)"), + Map.entry("eu-west-3", "EU (Paris)"), + Map.entry("eu-central-1", "EU (Frankfurt)"), + Map.entry("eu-central-2", "EU (Zurich)"), + Map.entry("eu-north-1", "EU (Stockholm)"), + Map.entry("eu-south-1", "EU (Milan)"), + Map.entry("eu-south-2", "EU (Spain)"), + Map.entry("me-south-1", "Middle East (Bahrain)"), + Map.entry("me-central-1", "Middle East (UAE)"), + Map.entry("il-central-1", "Israel (Tel Aviv)"), + Map.entry("af-south-1", "Africa (Cape Town)"), + Map.entry("ap-east-1", "Asia Pacific (Hong Kong)"), + Map.entry("ap-south-1", "Asia Pacific (Mumbai)"), + Map.entry("ap-south-2", "Asia Pacific (Hyderabad)"), + Map.entry("ap-northeast-1", "Asia Pacific (Tokyo)"), + Map.entry("ap-northeast-2", "Asia Pacific (Seoul)"), + Map.entry("ap-northeast-3", "Asia Pacific (Osaka)"), + Map.entry("ap-southeast-1", "Asia Pacific (Singapore)"), + Map.entry("ap-southeast-2", "Asia Pacific (Sydney)"), + Map.entry("ap-southeast-3", "Asia Pacific (Jakarta)"), + Map.entry("ap-southeast-4", "Asia Pacific (Melbourne)")); + private static final Logger LOG = Logger.getLogger(AlarmSnsNotifier.class); private static final DateTimeFormatter TIMESTAMP_FORMAT = DateTimeFormatter .ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ", Locale.ROOT).withZone(ZoneOffset.UTC); @@ -74,11 +104,7 @@ public class AlarmSnsNotifier { payload.put("NewStateValue", alarm.getStateValue()); payload.put("NewStateReason", alarm.getStateReason()); payload.put("StateChangeTime", timestamp(alarm.getStateUpdatedTimestamp())); - String regionName = AwsPartitions.forRegion(arn.region()).stream() - .flatMap(partition -> partition.regions().stream()) - .filter(region -> region.id().equals(arn.region())) - .map(AwsPartition.Region::description).findFirst().orElse(arn.region()); - payload.put("Region", regionName); + payload.put("Region", REGION_NAMES.getOrDefault(arn.region(), arn.region())); payload.put("AlarmArn", alarm.getAlarmArn()); payload.put("OldStateValue", oldState); payload.put("OKActions", alarm.getOkActions()); -- 2.52.0 From 24314f44327d464e9f8db2505d8e6c0487b0b5d9 Mon Sep 17 00:00:00 2001 From: Mohith Date: Tue, 20 Oct 2026 19:37:57 +0530 Subject: [PATCH 6/9] fix(cloudwatch): reject PutMetricData datums without a MetricName (#6072) * fix(cloudwatch): reject PutMetricData datums without a MetricName MetricName is the one required member of MetricDatum. The JSON handler stored a nameless datum under an empty name, and the Query handler treated a member without MetricName as the end of the list, silently dropping it and every later datum while still storing the earlier ones. Both handlers now reject a missing or empty MetricName in top-level MetricData and in EntityMetricData[].MetricData with MissingParameter (400), the error PutMetricData declares for a missing required input, naming the offending member. A whitespace name is still accepted, since AWS constrains MetricName only to 1 to 255 characters. Every datum is validated before any is stored, so a rejected request leaves no state behind. On Query, a member index with no keys at all still ends that MetricData list. * fix(cloudwatch): return MissingRequiredParameterException on JSON and CBOR CloudWatch is awsQueryCompatible. MissingParameter is the awsQueryError code of the MissingRequiredParameterException shape, so it belongs in the Query and the x-amzn-query-error header, while a JSON 1.0 or CBOR body's __type names the shape. With the code in __type the AWS SDK v2 raised a generic CloudWatchException instead of the modelled one. A package-private CloudWatchMetricsException keeps the Query code as its error code and overrides jsonType() with the shape name, the same split SwfException makes. The shared query-code table in AwsException is left alone because SQS reports MissingParameter as its JSON type. (cherry picked from commit b8d87df05413ea8c4f9497fa05f388c19d4124ec) --- .../java/com/floci/test/CloudWatchTest.java | 16 +++++ .../metrics/CloudWatchMetricsException.java | 34 +++++++++++ .../metrics/CloudWatchMetricsJsonHandler.java | 17 ++++-- .../CloudWatchMetricsQueryHandler.java | 10 ++- ...borCloudWatchTimestampIntegrationTest.java | 25 ++++++++ ...etricsEntityMetricDataIntegrationTest.java | 47 ++++++++++++++ .../CloudWatchMetricsJsonHandlerTest.java | 57 +++++++++++++++++ .../CloudWatchMetricsQueryHandlerTest.java | 61 +++++++++++++++++++ 8 files changed, 260 insertions(+), 7 deletions(-) create mode 100644 src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsException.java diff --git a/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java b/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java index e48b58a..66dc0e9 100644 --- a/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java +++ b/compatibility-tests/sdk-test-java/src/test/java/com/floci/test/CloudWatchTest.java @@ -233,4 +233,20 @@ class CloudWatchTest { assertThat(stats.datapoints()).hasSize(1); assertThat(stats.datapoints().get(0).sum()).isEqualTo(6.5); } + + @Test + @DisplayName("PutMetricData without MetricName throws MissingRequiredParameterException") + void testPutMetricDataWithoutMetricNameIsRejected() { + String namespace = "JavaTestMissingMetricName"; + + assertThatThrownBy(() -> cloudWatchClient.putMetricData(request -> request + .namespace(namespace) + .entityMetricData(entityMetricData -> entityMetricData + .metricData(MetricDatum.builder().value(1.0).build())))) + .isInstanceOf(MissingRequiredParameterException.class) + .satisfies(e -> assertThat(((MissingRequiredParameterException) e).awsErrorDetails().errorCode()) + .isEqualTo("MissingParameter")); + + assertThat(cloudWatchClient.listMetrics(request -> request.namespace(namespace)).metrics()).isEmpty(); + } } diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsException.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsException.java new file mode 100644 index 0000000..1ea39ed --- /dev/null +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsException.java @@ -0,0 +1,34 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics; + +import io.github.hectorvent.floci.core.common.AwsException; + +/** + * A CloudWatch error whose Query code and JSON shape name differ. + * + *

CloudWatch is awsQueryCompatible: the Query {@code } and the + * {@code x-amzn-query-error} header carry the {@code awsQueryError} code + * ({@code MissingParameter}), while a JSON 1.0 or CBOR body's {@code __type} names the + * model shape ({@code MissingRequiredParameterException}). The AWS SDK v2 picks the typed + * exception from {@code __type}, so the code alone yields a generic {@code CloudWatchException}. + * Keeping {@link #getErrorCode()} as the Query code and overriding {@link #jsonType()} + * reproduces both surfaces. + */ +class CloudWatchMetricsException extends AwsException { + + private final String jsonType; + + private CloudWatchMetricsException(String errorCode, String jsonType, String message, int httpStatus) { + super(errorCode, message, httpStatus); + this.jsonType = jsonType; + } + + static CloudWatchMetricsException missingParameter(String parameter) { + return new CloudWatchMetricsException("MissingParameter", "MissingRequiredParameterException", + "The parameter " + parameter + " is required.", 400); + } + + @Override + public String jsonType() { + return jsonType; + } +} diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java index acba84d..2aef32e 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java @@ -82,9 +82,12 @@ public class CloudWatchMetricsJsonHandler { private Response handlePutMetricData(JsonNode request, String region) { String namespace = request.path("Namespace").asText(); - List datums = parseMetricDataJson(request.path("MetricData")); + List datums = parseMetricDataJson(request.path("MetricData"), "MetricData"); + int entityIndex = 0; for (JsonNode entityMetricData : request.path("EntityMetricData")) { - datums.addAll(parseMetricDataJson(entityMetricData.path("MetricData"))); + entityIndex++; + datums.addAll(parseMetricDataJson(entityMetricData.path("MetricData"), + "EntityMetricData.member." + entityIndex + ".MetricData")); } LOG.debugv("JSON PutMetricData parsed {0} datums for namespace {1}", datums.size(), namespace); metricsService.putMetricData(namespace, datums, region); @@ -608,12 +611,18 @@ public class CloudWatchMetricsJsonHandler { return configurations; } - private List parseMetricDataJson(JsonNode node) { + private List parseMetricDataJson(JsonNode node, String listPath) { List datums = new ArrayList<>(); if (!node.isArray()) return datums; + int index = 0; for (JsonNode item : node) { + index++; + JsonNode metricName = item.path("MetricName"); + if (!metricName.isTextual() || metricName.asText().isEmpty()) { + throw CloudWatchMetricsException.missingParameter(listPath + ".member." + index + ".MetricName"); + } MetricDatum datum = new MetricDatum(); - datum.setMetricName(item.path("MetricName").asText()); + datum.setMetricName(metricName.asText()); datum.setValue(item.path("Value").asDouble(0)); datum.setUnit(item.path("Unit").asText(null)); JsonNode ts = item.path("Timestamp"); diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java index 4865b89..d3a0542 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java @@ -616,9 +616,13 @@ public class CloudWatchMetricsQueryHandler { private List parseMetricData(MultivaluedMap params, String listPrefix) { List datums = new ArrayList<>(); for (int i = 1; ; i++) { - String metricName = params.getFirst(listPrefix + ".member." + i + ".MetricName"); - if (metricName == null) { - break; + String memberPrefix = listPrefix + ".member." + i + "."; + String metricName = params.getFirst(memberPrefix + "MetricName"); + if (metricName == null || metricName.isEmpty()) { + if (metricName == null && params.keySet().stream().noneMatch(key -> key.startsWith(memberPrefix))) { + break; + } + throw CloudWatchMetricsException.missingParameter(memberPrefix + "MetricName"); } MetricDatum datum = new MetricDatum(); datum.setMetricName(metricName); diff --git a/src/test/java/io/github/hectorvent/floci/core/common/AwsJsonCborCloudWatchTimestampIntegrationTest.java b/src/test/java/io/github/hectorvent/floci/core/common/AwsJsonCborCloudWatchTimestampIntegrationTest.java index 3cbf6c9..fbdc848 100644 --- a/src/test/java/io/github/hectorvent/floci/core/common/AwsJsonCborCloudWatchTimestampIntegrationTest.java +++ b/src/test/java/io/github/hectorvent/floci/core/common/AwsJsonCborCloudWatchTimestampIntegrationTest.java @@ -6,11 +6,13 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.dataformat.cbor.CBORFactory; import io.quarkus.test.junit.QuarkusTest; +import io.restassured.response.Response; import org.junit.jupiter.api.Test; import java.time.Instant; import static io.restassured.RestAssured.given; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -82,6 +84,29 @@ class AwsJsonCborCloudWatchTimestampIntegrationTest { + driftSeconds + "s): " + datapoint); } + // CloudWatch is awsQueryCompatible: the header keeps the Query code while the body names + // the model shape, which is what the AWS SDK v2 uses to pick the typed exception. + @Test + void putMetricDataWithoutMetricNameReturnsTheModelledExceptionShape() throws Exception { + ObjectNode putRequest = JSON.createObjectNode(); + putRequest.put("Namespace", "CborRpcV2MissingName-" + System.nanoTime()); + putRequest.putArray("MetricData").addObject().put("Value", 1.0); + + Response response = given() + .contentType("application/cbor") + .accept("application/cbor") + .header("smithy-protocol", "rpc-v2-cbor") + .body(AwsJsonCborController.nodeToSmithyCbor(putRequest)) + .when().post("/service/GraniteServiceVersion20100801/operation/PutMetricData"); + + assertEquals(400, response.statusCode()); + assertEquals("MissingParameter;Sender", response.header("x-amzn-query-error")); + JsonNode body = CBOR.readTree(response.asByteArray()); + assertEquals("MissingRequiredParameterException", body.path("__type").asText(), body.toString()); + assertEquals("The parameter MetricData.member.1.MetricName is required.", + body.path("message").asText(), body.toString()); + } + private static JsonNode rpcV2Call(String action, ObjectNode body) throws Exception { byte[] responseCbor = given() .contentType("application/cbor") diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java index 890bceb..27d81e0 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsEntityMetricDataIntegrationTest.java @@ -14,6 +14,7 @@ import java.time.Instant; import static io.restassured.RestAssured.config; import static io.restassured.RestAssured.given; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; @@ -153,4 +154,50 @@ class CloudWatchMetricsEntityMetricDataIntegrationTest { .body("GetMetricStatisticsResponse.GetMetricStatisticsResult.Datapoints.member.Sum", equalTo("6.5")); } + + @Test + void jsonNestedDatumWithoutMetricNameIsRejectedAndNothingIsStored() { + json("PutMetricData") + .body(""" + {"Namespace":"CWAgent/JsonInvalid", + "MetricData":[{"MetricName":"TopLevel","Value":1}], + "EntityMetricData":[{"MetricData":[{"Value":2}]}]} + """) + .when().post("/") + .then() + .statusCode(400) + .header("x-amzn-query-error", equalTo("MissingParameter;Sender")) + .body("__type", equalTo("MissingRequiredParameterException")) + .body("message", equalTo( + "The parameter EntityMetricData.member.1.MetricData.member.1.MetricName is required.")); + + json("ListMetrics") + .body("{\"Namespace\":\"CWAgent/JsonInvalid\"}") + .when().post("/") + .then() + .statusCode(200) + .body("Metrics", empty()); + } + + @Test + void queryMemberWithoutMetricNameIsRejectedAndNothingIsStored() { + query("PutMetricData") + .formParam("Namespace", "CWAgent/QueryInvalid") + .formParam("MetricData.member.1.Value", "1") + .formParam("MetricData.member.2.MetricName", "Valid") + .formParam("MetricData.member.2.Value", "2") + .when().post("/") + .then() + .statusCode(400) + .body("ErrorResponse.Error.Code", equalTo("MissingParameter")) + .body("ErrorResponse.Error.Message", + equalTo("The parameter MetricData.member.1.MetricName is required.")); + + query("ListMetrics") + .formParam("Namespace", "CWAgent/QueryInvalid") + .when().post("/") + .then() + .statusCode(200) + .body("ListMetricsResponse.ListMetricsResult.Metrics.member.size()", equalTo(0)); + } } diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java index 9b57294..2d3a2a2 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java @@ -8,6 +8,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; +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; @@ -22,6 +23,7 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -205,6 +207,61 @@ class CloudWatchMetricsJsonHandlerTest { assertEquals(List.of("FirstEntity", "SecondEntity", "TopLevel"), names.stream().sorted().toList()); } + @Test + void putMetricData_entityDatumWithoutMetricName_rejectsTheWholeRequest() { + ObjectNode req = MAPPER.createObjectNode(); + req.put("Namespace", "Invalid"); + req.putArray("MetricData").addObject() + .put("MetricName", "TopLevel").put("Value", 1.0); + ArrayNode nested = req.putArray("EntityMetricData").addObject().putArray("MetricData"); + nested.addObject().put("MetricName", "Valid").put("Value", 2.0); + nested.addObject().put("Value", 3.0); + + AwsException e = assertThrows(AwsException.class, + () -> handler.handle("PutMetricData", req, REGION)); + assertEquals("MissingParameter", e.getErrorCode()); + assertEquals("MissingRequiredParameterException", e.jsonType()); + assertEquals(400, e.getHttpStatus()); + assertEquals("The parameter EntityMetricData.member.1.MetricData.member.2.MetricName is required.", + e.getMessage()); + assertEquals(0, listMetricNames("Invalid").size()); + } + + @Test + void putMetricData_topLevelDatumWithEmptyMetricName_rejectsTheWholeRequest() { + ObjectNode req = MAPPER.createObjectNode(); + req.put("Namespace", "InvalidTop"); + ArrayNode data = req.putArray("MetricData"); + data.addObject().put("MetricName", "Valid").put("Value", 1.0); + data.addObject().put("MetricName", "").put("Value", 2.0); + + AwsException e = assertThrows(AwsException.class, + () -> handler.handle("PutMetricData", req, REGION)); + assertEquals("MissingParameter", e.getErrorCode()); + assertEquals("The parameter MetricData.member.2.MetricName is required.", e.getMessage()); + assertEquals(0, listMetricNames("InvalidTop").size()); + } + + // AWS constrains MetricName to 1 to 255 characters with no pattern, so whitespace is a name. + @Test + void putMetricData_whitespaceMetricName_isStored() { + ObjectNode req = MAPPER.createObjectNode(); + req.put("Namespace", "WhitespaceName"); + req.putArray("MetricData").addObject().put("MetricName", " ").put("Value", 1.0); + + assertEquals(200, handler.handle("PutMetricData", req, REGION).getStatus()); + assertEquals(List.of(" "), listMetricNames("WhitespaceName")); + } + + private List listMetricNames(String namespace) { + ObjectNode listReq = MAPPER.createObjectNode(); + listReq.put("Namespace", namespace); + ObjectNode listed = (ObjectNode) handler.handle("ListMetrics", listReq, REGION).getEntity(); + List names = new ArrayList<>(); + listed.get("Metrics").forEach(m -> names.add(m.get("MetricName").asText())); + return names; + } + @Test void setAlarmState_updatesFieldsCorrectly() { putMetric("NS", "M", "type", "current", 100.0, EPOCH_NOW); diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java index 94e7e3f..881de60 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java @@ -1,5 +1,6 @@ package io.github.hectorvent.floci.services.cloudwatch.metrics; +import io.github.hectorvent.floci.core.common.AwsException; import io.github.hectorvent.floci.services.cloudwatch.dashboards.CloudWatchDashboardsService; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricDatum; @@ -14,6 +15,7 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.mockito.ArgumentMatchers.any; @@ -21,6 +23,7 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -181,4 +184,62 @@ class CloudWatchMetricsQueryHandlerTest { assertEquals(1.0, datums.get(0).getValue()); assertEquals(2.0, datums.get(1).getValue()); } + + @Test + void putMetricDataRejectsAMemberWithoutMetricNameBeforeStoringAnything() { + MultivaluedMap params = new MultivaluedHashMap<>(); + params.putSingle("Namespace", "NS"); + params.putSingle("MetricData.member.1.Value", "1"); + params.putSingle("MetricData.member.2.MetricName", "Valid"); + params.putSingle("MetricData.member.2.Value", "2"); + + AwsException e = assertThrows(AwsException.class, + () -> handler.handle("PutMetricData", params, REGION)); + assertEquals("MissingParameter", e.getErrorCode()); + assertEquals("MissingRequiredParameterException", e.jsonType()); + assertEquals(400, e.getHttpStatus()); + assertEquals("The parameter MetricData.member.1.MetricName is required.", e.getMessage()); + verify(metricsService, never()).putMetricData(any(), any(), any()); + } + + @Test + void putMetricDataRejectsANestedMemberWithoutMetricNameBeforeStoringAnything() { + MultivaluedMap params = new MultivaluedHashMap<>(); + params.putSingle("Namespace", "NS"); + params.putSingle("MetricData.member.1.MetricName", "TopLevel"); + params.putSingle("MetricData.member.1.Value", "1"); + params.putSingle("EntityMetricData.member.1.MetricData.member.1.Unit", "Percent"); + params.putSingle("EntityMetricData.member.1.MetricData.member.2.MetricName", "Valid"); + + AwsException e = assertThrows(AwsException.class, + () -> handler.handle("PutMetricData", params, REGION)); + assertEquals("The parameter EntityMetricData.member.1.MetricData.member.1.MetricName is required.", + e.getMessage()); + verify(metricsService, never()).putMetricData(any(), any(), any()); + } + + @Test + void putMetricDataRejectsAnEmptyMetricNameButAcceptsWhitespace() { + MultivaluedMap empty = new MultivaluedHashMap<>(); + empty.putSingle("Namespace", "NS"); + empty.putSingle("MetricData.member.1.MetricName", ""); + empty.putSingle("MetricData.member.1.Value", "1"); + + AwsException e = assertThrows(AwsException.class, + () -> handler.handle("PutMetricData", empty, REGION)); + assertEquals("The parameter MetricData.member.1.MetricName is required.", e.getMessage()); + verify(metricsService, never()).putMetricData(any(), any(), any()); + + MultivaluedMap whitespace = new MultivaluedHashMap<>(); + whitespace.putSingle("Namespace", "NS"); + whitespace.putSingle("MetricData.member.1.MetricName", " "); + whitespace.putSingle("MetricData.member.1.Value", "1"); + + handler.handle("PutMetricData", whitespace, REGION); + + @SuppressWarnings("unchecked") + ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); + verify(metricsService).putMetricData(eq("NS"), captor.capture(), eq(REGION)); + assertEquals(List.of(" "), captor.getValue().stream().map(MetricDatum::getMetricName).toList()); + } } -- 2.52.0 From 83aafc032560209f114aed5abb921f1f871a7d27 Mon Sep 17 00:00:00 2001 From: Stuart Lang Date: Sat, 24 Oct 2026 00:06:44 +0100 Subject: [PATCH 7/9] fix(cloudwatch): return empty CompositeAlarms and LogAlarms from DescribeAlarms (#6363) * fix(cloudwatch): return empty CompositeAlarms and LogAlarms from DescribeAlarms AWS sends every alarm collection even when it is empty. Floci only sent MetricAlarms, so the AWS CLI's text output printed a leading None line for each missing paginator result key. Closes #6362 * test(cloudwatch): check empty alarm collections in sdk-test-python instead of bats Drop the new AWS CLI bats file and assert the empty CompositeAlarms and LogAlarms lists in the existing describe_alarms test, as boto3 leaves an absent list out of the response. (cherry picked from commit 42f5019231b446c727d6c94def9d1445263de2d7) --- .../sdk-test-python/tests/test_cloudwatch.py | 2 ++ .../metrics/CloudWatchMetricsJsonHandler.java | 2 ++ .../metrics/CloudWatchMetricsQueryHandler.java | 2 ++ .../CloudWatchMetricsJsonHandlerTest.java | 16 ++++++++++++++++ .../CloudWatchMetricsQueryHandlerTest.java | 11 +++++++++++ 5 files changed, 33 insertions(+) diff --git a/compatibility-tests/sdk-test-python/tests/test_cloudwatch.py b/compatibility-tests/sdk-test-python/tests/test_cloudwatch.py index 9a674cf..6baa173 100644 --- a/compatibility-tests/sdk-test-python/tests/test_cloudwatch.py +++ b/compatibility-tests/sdk-test-python/tests/test_cloudwatch.py @@ -203,6 +203,8 @@ class TestCloudWatchAlarms: alarm = next(a for a in alarms if a["AlarmName"] == alarm_name) assert alarm["StateValue"] == "INSUFFICIENT_DATA" + assert response["CompositeAlarms"] == [] + assert response["LogAlarms"] == [] finally: cloudwatch_client.delete_alarms(AlarmNames=[alarm_name]) diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java index 2aef32e..35c0307 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java @@ -263,6 +263,8 @@ public class CloudWatchMetricsJsonHandler { if (a.getStateReasonData() != null) node.put("StateReasonData", a.getStateReasonData()); node.put("StateUpdatedTimestamp", a.getStateUpdatedTimestamp()); } + response.putArray("CompositeAlarms"); + response.putArray("LogAlarms"); return Response.ok(response).build(); } diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java index d3a0542..7b55d0e 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java @@ -259,6 +259,8 @@ public class CloudWatchMetricsQueryHandler { toAlarmXml(xml, a); } xml.end("MetricAlarms"); + xml.start("CompositeAlarms").end("CompositeAlarms"); + xml.start("LogAlarms").end("LogAlarms"); return Response.ok(AwsQueryResponse.envelope("DescribeAlarms", null, xml.build())).build(); } diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java index 2d3a2a2..cc81e98 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandlerTest.java @@ -350,6 +350,22 @@ class CloudWatchMetricsJsonHandlerTest { assertEquals("Count", alarm.get("Unit").asText()); } + // AWS sends every alarm collection even when it is empty. The CLI's text output writes a + // None line for each paginator result key missing from the page (floci-io/floci#6362). + @Test + void describeAlarms_returnsEmptyCompositeAndLogAlarmCollections() { + handler.handle("PutMetricAlarm", alarmRequest("MetricOnly"), REGION); + + ObjectNode response = (ObjectNode) handler.handle("DescribeAlarms", MAPPER.createObjectNode(), REGION) + .getEntity(); + + assertEquals(1, response.get("MetricAlarms").size()); + assertTrue(response.get("CompositeAlarms").isArray()); + assertTrue(response.get("CompositeAlarms").isEmpty()); + assertTrue(response.get("LogAlarms").isArray()); + assertTrue(response.get("LogAlarms").isEmpty()); + } + // An alarm the caller never gave an M for reports no DatapointsToAlarm at all. AWS omits the // member in that case, and echoing EvaluationPeriods instead reads as a change to any client // that re-plans against its own configuration, which is what floci-io/floci#3660 hit. diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java index 881de60..27869c6 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandlerTest.java @@ -242,4 +242,15 @@ class CloudWatchMetricsQueryHandlerTest { verify(metricsService).putMetricData(eq("NS"), captor.capture(), eq(REGION)); assertEquals(List.of(" "), captor.getValue().stream().map(MetricDatum::getMetricName).toList()); } + + @Test + void describeAlarmsReturnsEmptyCompositeAndLogAlarmCollections() { + when(metricsService.describeAlarms(any(), any(), eq(REGION))).thenReturn(List.of()); + + String xml = (String) handler.handle("DescribeAlarms", new MultivaluedHashMap<>(), REGION).getEntity(); + + assertTrue(xml.contains(""), xml); + assertTrue(xml.contains(""), xml); + assertTrue(xml.contains(""), xml); + } } -- 2.52.0 From fa3327bf71b922b4d03515efc7d967ee0934d0b6 Mon Sep 17 00:00:00 2001 From: Andre Kurait Date: Sat, 24 Oct 2026 16:54:21 -0500 Subject: [PATCH 8/9] feat(cloudwatch): support composite alarm lifecycle (#6408) (cherry picked from commit 922e36a472341861e97d1b5639656f3c4a3978d8) --- docs/services/cloudformation.md | 2 +- docs/services/cloudwatch.md | 6 + ...loudWatchCompositeAlarmCfnProvisioner.java | 183 +++++++++++ .../cloudwatch/metrics/AlarmSnsNotifier.java | 25 +- .../metrics/CloudWatchMetricsJsonHandler.java | 98 ++++-- .../CloudWatchMetricsQueryHandler.java | 140 +++++---- .../metrics/CloudWatchMetricsService.java | 290 +++++++++++++++--- .../metrics/CompositeAlarmRule.java | 184 +++++++++++ .../cloudwatch/metrics/model/Alarm.java | 87 ++++++ .../metrics/model/CompositeAlarm.java | 14 + .../cloudwatch/metrics/model/MetricAlarm.java | 71 +---- .../cloudformation/CfnProvisionerFixture.java | 2 + ...ormationCompositeAlarmIntegrationTest.java | 190 ++++++++++++ ...WatchCompositeAlarmCfnProvisionerTest.java | 89 ++++++ .../metrics/AlarmSnsNotifierTest.java | 43 +++ .../CloudWatchAlarmQueryHandlerTest.java | 45 +++ .../CloudWatchMetricMathAlarmTest.java | 8 +- .../CompositeAlarmIntegrationTest.java | 160 ++++++++++ .../metrics/CompositeAlarmServiceTest.java | 274 +++++++++++++++++ .../supported-resource-types.tsv | 1 + tools/docs/cfn_resource_types.yaml | 3 + 21 files changed, 1721 insertions(+), 194 deletions(-) create mode 100644 src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/CloudWatchCompositeAlarmCfnProvisioner.java create mode 100644 src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmRule.java create mode 100644 src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/Alarm.java create mode 100644 src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/CompositeAlarm.java create mode 100644 src/test/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationCompositeAlarmIntegrationTest.java create mode 100644 src/test/java/io/github/hectorvent/floci/services/cloudformation/provisioners/CloudWatchCompositeAlarmCfnProvisionerTest.java create mode 100644 src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchAlarmQueryHandlerTest.java create mode 100644 src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmIntegrationTest.java create mode 100644 src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmServiceTest.java diff --git a/docs/services/cloudformation.md b/docs/services/cloudformation.md index d09a4eb..3b73d07 100644 --- a/docs/services/cloudformation.md +++ b/docs/services/cloudformation.md @@ -160,7 +160,7 @@ cross-resource references. | Kinesis Data Firehose | `DeliveryStream` | | IoT Core | `DomainConfiguration` (`ServerCertificates` resolves to a JSON string), `Policy` (deleted after detaching it from its principals; on AWS the delete fails with `DeleteConflictException` while the policy is attached), `Thing`, `TopicRule` | | CloudFront | `CachePolicy`, `Distribution`, `OriginAccessControl`, `OriginRequestPolicy`, `ResponseHeadersPolicy` | -| CloudWatch | `Alarm`, `Dashboard` | +| CloudWatch | `Alarm`, `CompositeAlarm`, `Dashboard`. Composite alarms have backing definitions with name Ref and Arn GetAtt, rollback, and replacement cleanup. Composite actions are stored only; suppression properties are rejected. | | CloudWatch Logs | `LogGroup`, `LogStream`, `MetricFilter` | | WAFv2 | `WebACL` | | Config | `ConfigRule` | diff --git a/docs/services/cloudwatch.md b/docs/services/cloudwatch.md index 34b16ef..eda278a 100644 --- a/docs/services/cloudwatch.md +++ b/docs/services/cloudwatch.md @@ -311,6 +311,7 @@ aws logs put-retention-policy \ | `GetMetricStatistics` | Get metric statistics (Average, Sum, etc.) | | `GetMetricData` | Query metrics with math expressions | | `PutMetricAlarm` | Create a metric alarm | +| `PutCompositeAlarm` | Create or update a composite alarm rule | | `DescribeAlarms` | List alarms | | `DeleteAlarms` | Delete alarms | | `SetAlarmState` | Manually set alarm state | @@ -331,6 +332,11 @@ aws logs put-retention-policy \ Metric streams are stored as definitions with their `running` or `stopped` state. Floci never delivers metrics to the Firehose delivery stream a metric stream names. +Composite alarms support boolean rules over alarm states. Request `AlarmTypes` containing +`CompositeAlarm` when describing them; the default describes metric alarms. Composite +action lists are stored but not executed. Action suppression is not implemented and is +rejected with `ValidationError`. + ### SNS alarm actions State transitions publish to same-account standard SNS topics in `AlarmActions`, `OKActions`, or diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/CloudWatchCompositeAlarmCfnProvisioner.java b/src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/CloudWatchCompositeAlarmCfnProvisioner.java new file mode 100644 index 0000000..616334a --- /dev/null +++ b/src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/CloudWatchCompositeAlarmCfnProvisioner.java @@ -0,0 +1,183 @@ +package io.github.hectorvent.floci.services.cloudformation.provisioners; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import io.github.hectorvent.floci.core.common.AwsException; +import io.github.hectorvent.floci.services.cloudformation.model.StackResource; +import io.github.hectorvent.floci.services.cloudwatch.metrics.CloudWatchMetricsService; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.CompositeAlarm; +import jakarta.enterprise.context.ApplicationScoped; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; + +@ApplicationScoped +public class CloudWatchCompositeAlarmCfnProvisioner implements CfnResourceProvisioner { + private static final String TYPE = "AWS::CloudWatch::CompositeAlarm"; + private static final String SNAPSHOT = "FlociCompositeAlarmSnapshot"; + private static final String TAG_KEYS = "FlociCompositeAlarmTagKeys"; + private static final String EXPLICIT_NAME = "FlociCompositeAlarmExplicitName"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + private final CloudWatchMetricsService service; + + public CloudWatchCompositeAlarmCfnProvisioner(CloudWatchMetricsService service) { + this.service = service; + } + + @Override + public Set resourceTypes() { + return Set.of(TYPE); + } + + @Override + public void provision(StackResource resource, JsonNode props, ProvisionContext ctx) { + for (String unsupported : List.of("ActionsSuppressor", "ActionsSuppressorWaitPeriod", + "ActionsSuppressorExtensionPeriod")) { + if (props.has(unsupported)) { + throw new AwsException("ValidationError", "Composite action suppression is not supported", 400); + } + } + if (resource.getAttributes().containsKey(SNAPSHOT)) { + throw new AwsException("ResourceConflictException", "Alarm update cleanup is still pending", 400); + } + String explicit = ctx.resolveOptional(props, "AlarmName"); + boolean removedName = explicit == null && "true".equals(resource.getAttributes().get(EXPLICIT_NAME)); + String name = removedName ? ctx.generatePhysicalName(resource.getLogicalId(), 255, false) + : ctx.stablePhysicalName(explicit, resource.getLogicalId(), 255, false); + CompositeAlarm alarm = new CompositeAlarm(); + alarm.setAlarmName(name); + alarm.setAlarmDescription(ctx.resolveOptional(props, "AlarmDescription")); + alarm.setAlarmRule(ctx.resolveOptional(props, "AlarmRule")); + alarm.setActionsEnabled(!"false".equals(ctx.resolveOptional(props, "ActionsEnabled"))); + alarm.setTags(ctx.resolveTags(props, "Tags")); + actions(props, "AlarmActions", ctx, alarm.getAlarmActions()); + actions(props, "OKActions", ctx, alarm.getOkActions()); + actions(props, "InsufficientDataActions", ctx, alarm.getInsufficientDataActions()); + Map desiredTags = Map.copyOf(alarm.getTags()); + Map priorAttributes = Map.copyOf(resource.getAttributes()); + boolean inPlace = ctx.reusesPriorEntity(name); + String updateSnapshot = null; + if (inPlace) { + ObjectNode snapshot = MAPPER.createObjectNode().put("region", ctx.region()).put("name", name); + List previous = service.describeCompositeAlarms(List.of(name), null, ctx.region()); + if (!previous.isEmpty()) { + snapshot.set("alarm", MAPPER.valueToTree(previous.getFirst())); + } + snapshot.set("attributes", MAPPER.valueToTree(priorAttributes)); + Set changedKeys = new TreeSet<>(desiredTags.keySet()); + changedKeys.addAll(tagKeys(resource.getAttributes().get(TAG_KEYS))); + snapshot.set("changedTagKeys", MAPPER.valueToTree(changedKeys)); + updateSnapshot = snapshot.toString(); + } + service.putCompositeAlarm(alarm, ctx.region(), inPlace); + if (inPlace) { + resource.getAttributes().put(SNAPSHOT, updateSnapshot); + reconcileTags(alarm.getAlarmArn(), tagKeys(priorAttributes.get(TAG_KEYS)), desiredTags, ctx.region()); + } + resource.getAttributes().put(TAG_KEYS, MAPPER.valueToTree(desiredTags.keySet()).toString()); + resource.setPhysicalId(name); + resource.getAttributes().put("Arn", alarm.getAlarmArn()); + resource.getAttributes().put(EXPLICIT_NAME, Boolean.toString(explicit != null)); + ReplacementCleanup.record(resource, ctx, priorAttributes); + } + + private static void actions(JsonNode props, String field, ProvisionContext ctx, List target) { + JsonNode values = ctx.engine().resolveNode(props.path(field)); + if (values != null && values.isArray()) { + values.forEach(value -> target.add(value.asText())); + } + } + + private static Set tagKeys(String json) { + Set keys = new TreeSet<>(); + if (json != null) { + try { + MAPPER.readTree(json).forEach(value -> keys.add(value.asText())); + } catch (JsonProcessingException error) { + throw new IllegalStateException("Cannot read composite alarm tag ownership", error); + } + } + return keys; + } + + private void reconcileTags(String arn, Set owned, Map desired, String region) { + List removed = owned.stream().filter(key -> !desired.containsKey(key)).toList(); + if (!removed.isEmpty()) { + service.untagResource(arn, removed, region); + } + if (!desired.isEmpty()) { + service.tagResource(arn, desired, region); + } + } + + @Override + public void delete(String type, String physicalId, String region) { + service.deleteAlarms(List.of(physicalId), region); + } + + @Override + public boolean retainsFailedUpdateState(StackResource resource) { + return resource.getAttributes().containsKey(SNAPSHOT); + } + + @Override + public boolean rollbackUpdate(StackResource resource) { + if (ReplacementCleanup.rollback(resource, this::delete)) { + return true; + } + String raw = resource.getAttributes().get(SNAPSHOT); + if (raw == null) { + return false; + } + try { + JsonNode snapshot = MAPPER.readTree(raw); + String region = snapshot.path("region").asText(); + if (snapshot.has("alarm")) { + CompositeAlarm original = MAPPER.treeToValue(snapshot.get("alarm"), CompositeAlarm.class); + Map originalTags = Map.copyOf(original.getTags()); + service.putCompositeAlarm(original, region); + Set changed = tagKeys(snapshot.path("changedTagKeys").toString()); + reconcileTags(original.getAlarmArn(), changed, originalTags.entrySet().stream() + .filter(entry -> changed.contains(entry.getKey())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)), region); + } else { + delete(TYPE, snapshot.path("name").asText(), region); + } + resource.getAttributes().clear(); + snapshot.path("attributes").fields().forEachRemaining(entry -> + resource.getAttributes().put(entry.getKey(), entry.getValue().asText())); + } catch (JsonProcessingException error) { + throw new IllegalStateException("Cannot read composite alarm rollback state", error); + } + return true; + } + + @Override + public boolean hasReplacementUpdate(StackResource resource) { + return ReplacementCleanup.hasReplacement(resource); + } + + @Override + public String updateCleanupPhysicalId(StackResource resource) { + return ReplacementCleanup.cleanupPhysicalId(resource); + } + + @Override + public UpdateCleanupResult completeUpdate(StackResource resource) { + if (resource.getAttributes().containsKey(SNAPSHOT) && !ReplacementCleanup.hasReplacement(resource)) { + return new UpdateCleanupResult(true, true, null, 0, null); + } + return ReplacementCleanup.complete(resource, this::delete); + } + + @Override + public void clearUpdate(StackResource resource) { + resource.getAttributes().remove(SNAPSHOT); + ReplacementCleanup.clear(resource); + } +} diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java index 41332ef..c1a5463 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifier.java @@ -11,6 +11,7 @@ import org.jboss.logging.Logger; import java.time.Instant; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; +import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; @@ -63,9 +64,11 @@ public class AlarmSnsNotifier { this.objectMapper = objectMapper; } - public void notifyTransition(MetricAlarm alarm, String oldState) { + record Notification(String topicArn, String message, String subject, String region) {} + + List prepareTransition(MetricAlarm alarm, String oldState) { if (!alarm.isActionsEnabled() || alarm.getStateValue().equals(oldState)) { - return; + return List.of(); } List actions = switch (alarm.getStateValue()) { case "ALARM" -> alarm.getAlarmActions(); @@ -74,8 +77,9 @@ public class AlarmSnsNotifier { default -> List.of(); }; if (actions == null) { - return; + return List.of(); } + List notifications = new ArrayList<>(); for (String action : actions) { if (!AwsArnUtils.isArnFor(action, "sns")) { continue; @@ -86,12 +90,25 @@ public class AlarmSnsNotifier { if (subject.length() > 100) { subject = subject.substring(0, 100); } - snsService.publish(action, null, message(alarm, oldState), subject, topic.region()); + notifications.add(new Notification(action, message(alarm, oldState), subject, topic.region())); } catch (Exception e) { LOG.warnv(e, "Unable to publish alarm {0} state {1} to SNS action {2}", alarm.getAlarmName(), alarm.getStateValue(), action); } } + return notifications; + } + + void publish(List notifications) { + for (Notification notification : notifications) { + try { + snsService.publish(notification.topicArn(), null, notification.message(), + notification.subject(), notification.region()); + } catch (Exception error) { + LOG.warnv(error, "Unable to publish alarm transition {0} to SNS action {1}", + notification.subject(), notification.topicArn()); + } + } } private String message(MetricAlarm alarm, String oldState) throws JsonProcessingException { diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java index 35c0307..0920b95 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsJsonHandler.java @@ -5,9 +5,16 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import io.github.hectorvent.floci.core.common.AwsErrorResponse; +import io.github.hectorvent.floci.core.common.AwsException; import io.github.hectorvent.floci.core.common.PaginatedResult; import io.github.hectorvent.floci.services.cloudwatch.dashboards.CloudWatchDashboardsService; import io.github.hectorvent.floci.services.cloudwatch.dashboards.model.Dashboard; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Alarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.AlarmMetricDataQuery; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.CompositeAlarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Dimension; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricDatum; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.CloudWatchMetricStreamsService; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStream; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStreamFilter; @@ -57,6 +64,7 @@ public class CloudWatchMetricsJsonHandler { case "ListMetrics" -> handleListMetrics(request, region); case "GetMetricStatistics" -> handleGetMetricStatistics(request, region); case "PutMetricAlarm" -> handlePutMetricAlarm(request, region); + case "PutCompositeAlarm" -> handlePutCompositeAlarm(request, region); case "DescribeAlarms" -> handleDescribeAlarms(request, region); case "DeleteAlarms" -> handleDeleteAlarms(request, region); case "SetAlarmState" -> handleSetAlarmState(request, region); @@ -156,8 +164,7 @@ public class CloudWatchMetricsJsonHandler { private Response handlePutMetricAlarm(JsonNode request, String region) { MetricAlarm alarm = new MetricAlarm(); - alarm.setAlarmName(request.path("AlarmName").asText()); - alarm.setAlarmDescription(request.path("AlarmDescription").asText(null)); + parseAlarmFields(request, alarm); alarm.setMetricName(request.path("MetricName").asText(null)); alarm.setNamespace(request.path("Namespace").asText(null)); alarm.setStatistic(request.path("Statistic").asText(null)); @@ -175,9 +182,33 @@ public class CloudWatchMetricsJsonHandler { alarm.setThreshold(request.path("Threshold").asDouble(0)); alarm.setComparisonOperator(request.path("ComparisonOperator").asText(null)); alarm.setTreatMissingData(request.path("TreatMissingData").asText(null)); - alarm.setActionsEnabled(request.path("ActionsEnabled").asBoolean(true)); alarm.setDimensions(parseDimensionsJson(request.path("Dimensions"))); + metricsService.putMetricAlarm(alarm, region); + return Response.ok(objectMapper.createObjectNode()).build(); + } + + private Response handlePutCompositeAlarm(JsonNode request, String region) { + rejectSuppressor(request); + CompositeAlarm alarm = new CompositeAlarm(); + parseAlarmFields(request, alarm); + alarm.setAlarmRule(request.path("AlarmRule").asText(null)); + metricsService.putCompositeAlarm(alarm, region); + return Response.ok(objectMapper.createObjectNode()).build(); + } + + private static void rejectSuppressor(JsonNode request) { + if (request.has("ActionsSuppressor") || request.has("ActionsSuppressorWaitPeriod") + || request.has("ActionsSuppressorExtensionPeriod")) { + throw new AwsException("ValidationError", "Composite action suppression is not supported", 400); + } + } + + private void parseAlarmFields(JsonNode request, Alarm alarm) { + alarm.setAlarmName(request.path("AlarmName").asText()); + alarm.setAlarmDescription(request.path("AlarmDescription").asText(null)); + alarm.setActionsEnabled(request.path("ActionsEnabled").asBoolean(true)); + JsonNode alarmActions = request.path("AlarmActions"); if (alarmActions.isArray()) { alarmActions.forEach(a -> alarm.getAlarmActions().add(a.asText())); @@ -198,8 +229,6 @@ public class CloudWatchMetricsJsonHandler { alarm.setTags(tags); } - metricsService.putMetricAlarm(alarm, region); - return Response.ok(objectMapper.createObjectNode()).build(); } private Response handleDescribeAlarms(JsonNode request, String region) { @@ -210,21 +239,17 @@ public class CloudWatchMetricsJsonHandler { } String prefix = request.has("AlarmNamePrefix") ? request.path("AlarmNamePrefix").asText() : null; - List alarms = metricsService.describeAlarms(alarmNames, prefix, region); + List types = new ArrayList<>(); + request.path("AlarmTypes").forEach(value -> types.add(value.asText())); + metricsService.validateAlarmTypes(types); + List alarms = types.isEmpty() || types.contains("MetricAlarm") + ? metricsService.describeAlarms(alarmNames, prefix, region) : List.of(); ObjectNode response = objectMapper.createObjectNode(); ArrayNode arr = response.putArray("MetricAlarms"); for (MetricAlarm a : alarms) { ObjectNode node = arr.addObject(); - node.put("AlarmName", a.getAlarmName()); - if (a.getAlarmArn() != null) node.put("AlarmArn", a.getAlarmArn()); - if (a.getAlarmDescription() != null) node.put("AlarmDescription", a.getAlarmDescription()); - ArrayNode alarmActions = node.putArray("AlarmActions"); - a.getAlarmActions().forEach(alarmActions::add); - ArrayNode okActions = node.putArray("OKActions"); - a.getOkActions().forEach(okActions::add); - ArrayNode insufficientDataActions = node.putArray("InsufficientDataActions"); - a.getInsufficientDataActions().forEach(insufficientDataActions::add); + describeAlarmFields(node, a); if (!a.getMetrics().isEmpty()) { node.set("Metrics", objectMapper.valueToTree(a.getMetrics())); } else { @@ -257,17 +282,48 @@ public class CloudWatchMetricsJsonHandler { node.put("Threshold", a.getThreshold()); if (a.getComparisonOperator() != null) node.put("ComparisonOperator", a.getComparisonOperator()); if (a.getTreatMissingData() != null) node.put("TreatMissingData", a.getTreatMissingData()); - node.put("ActionsEnabled", a.isActionsEnabled()); - if (a.getStateValue() != null) node.put("StateValue", a.getStateValue()); - if (a.getStateReason() != null) node.put("StateReason", a.getStateReason()); - if (a.getStateReasonData() != null) node.put("StateReasonData", a.getStateReasonData()); - node.put("StateUpdatedTimestamp", a.getStateUpdatedTimestamp()); + + } + ArrayNode composites = response.putArray("CompositeAlarms"); + if (types.contains("CompositeAlarm")) { + for (CompositeAlarm alarm : metricsService.describeCompositeAlarms(alarmNames, prefix, region)) { + ObjectNode node = composites.addObject(); + describeAlarmFields(node, alarm); + node.put("AlarmRule", alarm.getAlarmRule()); + } } - response.putArray("CompositeAlarms"); response.putArray("LogAlarms"); return Response.ok(response).build(); } + private void describeAlarmFields(ObjectNode node, Alarm a) { + node.put("AlarmName", a.getAlarmName()); + if (a.getAlarmArn() != null) { + node.put("AlarmArn", a.getAlarmArn()); + } + if (a.getAlarmDescription() != null) { + node.put("AlarmDescription", a.getAlarmDescription()); + } + ArrayNode alarmActions = node.putArray("AlarmActions"); + a.getAlarmActions().forEach(alarmActions::add); + ArrayNode okActions = node.putArray("OKActions"); + a.getOkActions().forEach(okActions::add); + ArrayNode insufficientDataActions = node.putArray("InsufficientDataActions"); + a.getInsufficientDataActions().forEach(insufficientDataActions::add); + node.put("ActionsEnabled", a.isActionsEnabled()); + if (a.getStateValue() != null) { + node.put("StateValue", a.getStateValue()); + } + if (a.getStateReason() != null) { + node.put("StateReason", a.getStateReason()); + } + if (a.getStateReasonData() != null) { + node.put("StateReasonData", a.getStateReasonData()); + } + node.put("StateUpdatedTimestamp", a.getStateUpdatedTimestamp()); + node.put("AlarmConfigurationUpdatedTimestamp", a.getAlarmConfigurationUpdatedTimestamp()); + } + private Response handleDeleteAlarms(JsonNode request, String region) { List alarmNames = new ArrayList<>(); JsonNode namesNode = request.path("AlarmNames"); diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java index 7b55d0e..5ad8ff3 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricsQueryHandler.java @@ -1,5 +1,6 @@ package io.github.hectorvent.floci.services.cloudwatch.metrics; +import io.github.hectorvent.floci.core.common.AwsException; import io.github.hectorvent.floci.core.common.AwsNamespaces; import io.github.hectorvent.floci.core.common.AwsQueryResponse; import io.github.hectorvent.floci.core.common.PaginatedResult; @@ -7,6 +8,12 @@ import io.github.hectorvent.floci.core.common.Pagination; import io.github.hectorvent.floci.core.common.XmlBuilder; import io.github.hectorvent.floci.services.cloudwatch.dashboards.CloudWatchDashboardsService; import io.github.hectorvent.floci.services.cloudwatch.dashboards.model.Dashboard; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Alarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.AlarmMetricDataQuery; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.CompositeAlarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Dimension; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricDatum; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.CloudWatchMetricStreamsService; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStream; import io.github.hectorvent.floci.services.cloudwatch.metricstreams.model.MetricStreamFilter; @@ -58,6 +65,7 @@ public class CloudWatchMetricsQueryHandler { case "GetMetricStatistics" -> handleGetMetricStatistics(params, region); case "GetMetricData" -> handleGetMetricData(params, region); case "PutMetricAlarm" -> handlePutMetricAlarm(params, region); + case "PutCompositeAlarm" -> handlePutCompositeAlarm(params, region); case "DescribeAlarms" -> handleDescribeAlarms(params, region); case "DeleteAlarms" -> handleDeleteAlarms(params, region); case "SetAlarmState" -> handleSetAlarmState(params, region); @@ -159,7 +167,8 @@ public class CloudWatchMetricsQueryHandler { xml.elem("Unit", dp.unit()).end("member"); } xml.end("Datapoints"); - return Response.ok(AwsQueryResponse.envelope("GetMetricStatistics", AwsNamespaces.CW, xml.build())).build(); + return Response.ok(AwsQueryResponse.envelope("GetMetricStatistics", + AwsNamespaces.CW, xml.build())).build(); } private Response handleGetMetricData(MultivaluedMap params, String region) { @@ -243,6 +252,26 @@ public class CloudWatchMetricsQueryHandler { return Response.ok(AwsQueryResponse.envelopeNoResult("PutMetricAlarm", null)).build(); } + private Response handlePutCompositeAlarm(MultivaluedMap params, String region) { + if (params.containsKey("ActionsSuppressor") || params.containsKey("ActionsSuppressorWaitPeriod") + || params.containsKey("ActionsSuppressorExtensionPeriod")) { + throw new AwsException("ValidationError", "Composite action suppression is not supported", 400); + } + CompositeAlarm alarm = new CompositeAlarm(); + parseAlarmFields(params, alarm); + alarm.setAlarmRule(params.getFirst("AlarmRule")); + metricsService.putCompositeAlarm(alarm, region); + return Response.ok(AwsQueryResponse.envelopeNoResult("PutCompositeAlarm", null)).build(); + } + + private static List alarmMembers(MultivaluedMap params, String key) { + List values = new ArrayList<>(); + for (int i = 1; params.getFirst(key + ".member." + i) != null; i++) { + values.add(params.getFirst(key + ".member." + i)); + } + return values; + } + private Response handleDescribeAlarms(MultivaluedMap params, String region) { List alarmNames = new ArrayList<>(); for (int i = 1; ; i++) { @@ -252,15 +281,24 @@ public class CloudWatchMetricsQueryHandler { } String prefix = params.getFirst("AlarmNamePrefix"); - List alarms = metricsService.describeAlarms(alarmNames, prefix, region); + List types = alarmMembers(params, "AlarmTypes"); + metricsService.validateAlarmTypes(types); + List alarms = types.isEmpty() || types.contains("MetricAlarm") + ? metricsService.describeAlarms(alarmNames, prefix, region) : List.of(); var xml = new XmlBuilder().start("MetricAlarms"); for (MetricAlarm a : alarms) { toAlarmXml(xml, a); } - xml.end("MetricAlarms"); - xml.start("CompositeAlarms").end("CompositeAlarms"); - xml.start("LogAlarms").end("LogAlarms"); + xml.end("MetricAlarms").start("CompositeAlarms"); + if (types.contains("CompositeAlarm")) { + for (CompositeAlarm alarm : metricsService.describeCompositeAlarms(alarmNames, prefix, region)) { + xml.start("member"); + describeAlarmFields(xml, alarm); + xml.elem("AlarmRule", alarm.getAlarmRule()).end("member"); + } + } + xml.end("CompositeAlarms").start("LogAlarms").end("LogAlarms"); return Response.ok(AwsQueryResponse.envelope("DescribeAlarms", null, xml.build())).build(); } @@ -682,10 +720,7 @@ public class CloudWatchMetricsQueryHandler { private MetricAlarm parseAlarm(MultivaluedMap params) { MetricAlarm a = new MetricAlarm(); - a.setAlarmName(params.getFirst("AlarmName")); - a.setAlarmDescription(params.getFirst("AlarmDescription")); - String actionsEnabled = params.getFirst("ActionsEnabled"); - a.setActionsEnabled(actionsEnabled == null || Boolean.parseBoolean(actionsEnabled)); + parseAlarmFields(params, a); a.setMetricName(params.getFirst("MetricName")); a.setNamespace(params.getFirst("Namespace")); a.setStatistic(params.getFirst("Statistic")); @@ -710,35 +745,24 @@ public class CloudWatchMetricsQueryHandler { } a.setDimensions(dims); - // Actions - for (int i = 1; ; i++) { - String act = params.getFirst("OKActions.member." + i); - if (act == null) break; - a.getOkActions().add(act); - } - for (int i = 1; ; i++) { - String act = params.getFirst("AlarmActions.member." + i); - if (act == null) break; - a.getAlarmActions().add(act); - } - for (int i = 1; ; i++) { - String act = params.getFirst("InsufficientDataActions.member." + i); - if (act == null) break; - a.getInsufficientDataActions().add(act); - } - - // Tags - Map tags = new LinkedHashMap<>(); - for (int i = 1; ; i++) { - String key = params.getFirst("Tags.member." + i + ".Key"); - if (key == null) break; - tags.put(key, params.getFirst("Tags.member." + i + ".Value")); - } - a.setTags(tags); - return a; } + private void parseAlarmFields(MultivaluedMap params, Alarm alarm) { + alarm.setAlarmName(params.getFirst("AlarmName")); + alarm.setAlarmDescription(params.getFirst("AlarmDescription")); + String actionsEnabled = params.getFirst("ActionsEnabled"); + alarm.setActionsEnabled(actionsEnabled == null || Boolean.parseBoolean(actionsEnabled)); + alarm.setAlarmActions(alarmMembers(params, "AlarmActions")); + alarm.setOkActions(alarmMembers(params, "OKActions")); + alarm.setInsufficientDataActions(alarmMembers(params, "InsufficientDataActions")); + Map tags = new LinkedHashMap<>(); + for (int i = 1; params.getFirst("Tags.member." + i + ".Key") != null; i++) { + tags.put(params.getFirst("Tags.member." + i + ".Key"), params.getFirst("Tags.member." + i + ".Value")); + } + alarm.setTags(tags); + } + private List parseAlarmMetricQueries(MultivaluedMap params) { List queries = new ArrayList<>(); for (int index = 1; ; index++) { @@ -784,22 +808,8 @@ public class CloudWatchMetricsQueryHandler { } private void toAlarmXml(XmlBuilder xml, MetricAlarm a) { - xml.start("member") - .elem("AlarmName", a.getAlarmName()) - .elem("AlarmArn", a.getAlarmArn()) - .elem("AlarmDescription", a.getAlarmDescription()); - xml.start("AlarmActions"); - a.getAlarmActions().forEach(act -> xml.elem("member", act)); - xml.end("AlarmActions") - .elem("AlarmConfigurationUpdatedTimestamp", Instant.ofEpochSecond(a.getAlarmConfigurationUpdatedTimestamp()).toString()) - .elem("ActionsEnabled", String.valueOf(a.isActionsEnabled())); - - xml.start("OKActions"); - a.getOkActions().forEach(act -> xml.elem("member", act)); - xml.end("OKActions"); - xml.start("InsufficientDataActions"); - a.getInsufficientDataActions().forEach(act -> xml.elem("member", act)); - xml.end("InsufficientDataActions"); + xml.start("member"); + describeAlarmFields(xml, a); if (!a.getMetrics().isEmpty()) { toAlarmMetricsXml(xml, a.getMetrics()); } else { @@ -815,11 +825,7 @@ public class CloudWatchMetricsQueryHandler { .elem("Unit", a.getUnit()); } - xml.elem("StateValue", a.getStateValue()) - .elem("StateReason", a.getStateReason()) - .elem("StateReasonData", a.getStateReasonData()) - .elem("StateUpdatedTimestamp", Instant.ofEpochSecond(a.getStateUpdatedTimestamp()).toString()) - .elem("EvaluationPeriods", String.valueOf(a.getEvaluationPeriods())) + xml.elem("EvaluationPeriods", String.valueOf(a.getEvaluationPeriods())) .elem("Threshold", String.valueOf(a.getThreshold())) .elem("ComparisonOperator", a.getComparisonOperator()) .elem("TreatMissingData", a.getTreatMissingData()); @@ -831,6 +837,28 @@ public class CloudWatchMetricsQueryHandler { xml.end("member"); } + private void describeAlarmFields(XmlBuilder xml, Alarm a) { + xml.elem("AlarmName", a.getAlarmName()) + .elem("AlarmArn", a.getAlarmArn()) + .elem("AlarmDescription", a.getAlarmDescription()); + xml.start("AlarmActions"); + a.getAlarmActions().forEach(act -> xml.elem("member", act)); + xml.end("AlarmActions") + .elem("AlarmConfigurationUpdatedTimestamp", Instant.ofEpochSecond(a.getAlarmConfigurationUpdatedTimestamp()).toString()) + .elem("ActionsEnabled", String.valueOf(a.isActionsEnabled())); + + xml.start("OKActions"); + a.getOkActions().forEach(act -> xml.elem("member", act)); + xml.end("OKActions"); + xml.start("InsufficientDataActions"); + a.getInsufficientDataActions().forEach(act -> xml.elem("member", act)); + xml.end("InsufficientDataActions"); + xml.elem("StateValue", a.getStateValue()) + .elem("StateReason", a.getStateReason()) + .elem("StateReasonData", a.getStateReasonData()) + .elem("StateUpdatedTimestamp", Instant.ofEpochSecond(a.getStateUpdatedTimestamp()).toString()); + } + private void toAlarmMetricsXml(XmlBuilder xml, List metrics) { xml.start("Metrics"); for (AlarmMetricDataQuery query : metrics) { 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 d7c35e2..74e3d18 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 @@ -1,10 +1,15 @@ package io.github.hectorvent.floci.services.cloudwatch.metrics; +import com.fasterxml.jackson.core.type.TypeReference; +import io.github.hectorvent.floci.core.common.AwsArnUtils; import io.github.hectorvent.floci.core.common.AwsException; import io.github.hectorvent.floci.core.common.RegionResolver; +import io.github.hectorvent.floci.core.common.SmithyValidationErrors; import io.github.hectorvent.floci.core.storage.AccountAwareStorageBackend; import io.github.hectorvent.floci.core.storage.StorageBackend; import io.github.hectorvent.floci.core.storage.StorageFactory; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Alarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.CompositeAlarm; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Dimension; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricDatum; @@ -14,11 +19,16 @@ import jakarta.inject.Inject; import org.jboss.logging.Logger; import java.time.Instant; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; @@ -27,8 +37,10 @@ public class CloudWatchMetricsService { private static final Logger LOG = Logger.getLogger(CloudWatchMetricsService.class); + private static final List ALARM_TYPES = List.of("CompositeAlarm", "MetricAlarm", "LogAlarm"); + private final StorageBackend metricStore; - private final StorageBackend alarmStore; + private final StorageBackend alarmStore; private final RegionResolver regionResolver; private final AlarmSnsNotifier alarmSnsNotifier; @@ -42,19 +54,19 @@ public class CloudWatchMetricsService { this.metricStore = storageFactory.create("cloudwatchmetrics", "cwmetrics.json", new TypeReference>() {}); this.alarmStore = storageFactory.create("cloudwatchmetrics", "cwalarms.json", - new TypeReference>() {}); + new TypeReference>() {}); this.regionResolver = regionResolver; this.alarmSnsNotifier = alarmSnsNotifier; } CloudWatchMetricsService(StorageBackend metricStore, - StorageBackend alarmStore, + StorageBackend alarmStore, RegionResolver regionResolver) { this(metricStore, alarmStore, regionResolver, null); } CloudWatchMetricsService(StorageBackend metricStore, - StorageBackend alarmStore, + StorageBackend alarmStore, RegionResolver regionResolver, AlarmSnsNotifier alarmSnsNotifier) { this.metricStore = metricStore; this.alarmStore = alarmStore; @@ -296,57 +308,253 @@ public class CloudWatchMetricsService { }; } - public void putMetricAlarm(MetricAlarm alarm, String region) { + public synchronized void putMetricAlarm(MetricAlarm alarm, String region) { + putAlarm(alarm, region); + evaluateComposites(region, Set.of(alarm.getAlarmName()), null); + } + + public void putCompositeAlarm(CompositeAlarm alarm, String region) { + putCompositeAlarm(alarm, region, true); + } + + public synchronized void putCompositeAlarm(CompositeAlarm alarm, String region, boolean replaceExisting) { + if (!replaceExisting && alarmStore.get(region + "::" + alarm.getAlarmName()).isPresent()) { + throw new AwsException("AlreadyExists", "Alarm already exists: " + alarm.getAlarmName(), 400); + } + Set references = alarmReferences(alarm, region); + if (references.size() > 100) { + throw new AwsException("LimitExceeded", "AlarmRule references more than 100 child alarms", 400); + } + List existing = describeCompositeAlarms(null, null, region); + for (String reference : references) { + long parents = existing.stream().filter(parent -> !parent.getAlarmName().equals(alarm.getAlarmName())) + .filter(parent -> alarmReferences(parent, region).contains(reference)) + .count(); + if (parents >= 150) { + throw new AwsException("LimitExceeded", "An alarm can be referenced by at most 150 composite alarms", 400); + } + } + boolean created = alarmStore.get(region + "::" + alarm.getAlarmName()).isEmpty(); + putAlarm(alarm, region); + if (created) { + evaluateComposites(region, Set.of(), alarm.getAlarmName()); + } + } + + private Set alarmReferences(CompositeAlarm alarm, String region) { + return CompositeAlarmRule.parse(alarm.getAlarmRule()).references().stream() + .map(reference -> AwsArnUtils.isArn(reference) ? reference + : regionResolver.buildArn("cloudwatch", region, "alarm:" + reference)) + .collect(Collectors.toSet()); + } + + private void putAlarm(Alarm alarm, String region) { + String name = alarm.getAlarmName(); + if (name == null || name.isBlank() || name.length() > 255) { + throw SmithyValidationErrors.validationError(name == null ? SmithyValidationErrors.notNull("alarmName") + : "Value '" + name + "' at 'alarmName' failed to satisfy constraint: Member must have length " + + (name.length() > 255 ? "less than or equal to 255" : "greater than or equal to 1")); + } + Alarm previous = alarmStore.get(region + "::" + name).orElse(null); + if (previous != null) { + if (!previous.getClass().equals(alarm.getClass())) { + throw new AwsException("ValidationError", "An alarm of another type already has this name", 400); + } + if (alarm instanceof CompositeAlarm) { + alarm.setStateValue(previous.getStateValue()); + alarm.setStateReason(previous.getStateReason()); + alarm.setStateReasonData(previous.getStateReasonData()); + alarm.setStateUpdatedTimestamp(previous.getStateUpdatedTimestamp()); + alarm.setTags(new HashMap<>(previous.getTags())); + } + } if (alarm.getAlarmArn() == null) { - alarm.setAlarmArn(regionResolver.buildArn("cloudwatch", region, "alarm:" + alarm.getAlarmName())); + alarm.setAlarmArn(regionResolver.buildArn("cloudwatch", region, "alarm:" + name)); } alarm.setRegion(region); alarm.setAlarmConfigurationUpdatedTimestamp(Instant.now().getEpochSecond()); - alarmStore.put(region + "::" + alarm.getAlarmName(), alarm); - LOG.infov("PutMetricAlarm: {0} in {1}", alarm.getAlarmName(), region); + alarmStore.put(region + "::" + name, alarm); + LOG.infov("Put {0}: {1} in {2}", alarm.getClass().getSimpleName(), name, region); } - /** Every stored alarm, across all regions. Used by the background {@link AlarmEvaluator} - * tick, which has no per-request region to scope a lookup to. */ + /** Metric alarms evaluated by the existing metric evaluator. */ public List allAlarms() { - return alarmStore.scan(k -> true); + return alarmStore.scan(k -> true).stream().filter(MetricAlarm.class::isInstance) + .map(MetricAlarm.class::cast).toList(); } - public List describeAlarms(List alarmNames, String alarmNamePrefix, String region) { - String prefix = region + "::"; - List all = alarmStore.scan(k -> k.startsWith(prefix)); - - if (alarmNames != null && !alarmNames.isEmpty()) { - return all.stream().filter(a -> alarmNames.contains(a.getAlarmName())).toList(); - } - if (alarmNamePrefix != null && !alarmNamePrefix.isBlank()) { - return all.stream().filter(a -> a.getAlarmName().startsWith(alarmNamePrefix)).toList(); - } - return all; + public List describeAlarms(List names, String prefix, String region) { + return matchingAlarms(names, prefix, region).stream().filter(MetricAlarm.class::isInstance) + .map(MetricAlarm.class::cast).toList(); } - public void deleteAlarms(List alarmNames, String region) { + public void validateAlarmTypes(List types) { + if (!ALARM_TYPES.containsAll(types)) { + throw SmithyValidationErrors.validationError( + SmithyValidationErrors.notAllInEnum(types, "alarmTypes", ALARM_TYPES)); + } + } + + public List describeCompositeAlarms(List names, String prefix, String region) { + return matchingAlarms(names, prefix, region).stream().filter(CompositeAlarm.class::isInstance) + .map(CompositeAlarm.class::cast).toList(); + } + + private List matchingAlarms(List names, String prefix, String region) { + return alarmStore.scan(k -> k.startsWith(region + "::")).stream() + .filter(a -> names == null || names.isEmpty() || names.contains(a.getAlarmName())) + .filter(a -> prefix == null || a.getAlarmName().startsWith(prefix)).toList(); + } + + private Map alarmIndex(String region) { + Map alarms = new HashMap<>(); + for (Alarm alarm : alarmStore.scan(k -> k.startsWith(region + "::"))) { + alarms.put(alarm.getAlarmName(), alarm); + alarms.put(alarm.getAlarmArn(), alarm); + } + return alarms; + } + + private void evaluateComposites(String region, Set changedAlarmNames, String newAlarm) { + if (changedAlarmNames.isEmpty() && newAlarm == null) { + return; + } + Map alarms = alarmIndex(region); + Set changed = new HashSet<>(changedAlarmNames); + for (String name : changedAlarmNames) { + changed.add(regionResolver.buildArn("cloudwatch", region, "alarm:" + name)); + } + Map candidates = new HashMap<>(); + for (Alarm alarm : new HashSet<>(alarms.values())) { + if (alarm instanceof CompositeAlarm composite && !changedAlarmNames.contains(alarm.getAlarmName())) { + candidates.put(alarm.getAlarmName(), CompositeAlarmRule.parse(composite.getAlarmRule())); + } + } + Set affected = new HashSet<>(changed); + Map pending = new HashMap<>(); + boolean discovered; + do { + discovered = false; + Iterator> iterator = candidates.entrySet().iterator(); + while (iterator.hasNext()) { + Map.Entry next = iterator.next(); + if (next.getKey().equals(newAlarm) || next.getValue().references().stream().anyMatch(affected::contains)) { + pending.put(next.getKey(), next.getValue()); + affected.add(next.getKey()); + affected.add(alarms.get(next.getKey()).getAlarmArn()); + iterator.remove(); + discovered = true; + } + } + } while (discovered); + Set cycles = pending.keySet().stream() + .filter(name -> isCyclic(name, alarms)).collect(Collectors.toSet()); + boolean progress; + do { + progress = false; + Iterator> iterator = pending.entrySet().iterator(); + while (iterator.hasNext()) { + Map.Entry next = iterator.next(); + boolean blocked = next.getValue().references().stream().map(alarms::get) + .anyMatch(child -> child != null && pending.containsKey(child.getAlarmName())); + if (blocked || cycles.contains(next.getKey())) { + continue; + } + if (!next.getKey().equals(newAlarm) + && next.getValue().references().stream().noneMatch(changed::contains)) { + iterator.remove(); + progress = true; + continue; + } + Alarm alarm = alarms.get(next.getKey()); + boolean triggered = next.getValue().evaluate(reference -> { + Alarm child = alarms.get(reference); + return child == null ? null : child.getStateValue(); + }); + String state = triggered ? "ALARM" : "OK"; + if (!state.equals(alarm.getStateValue())) { + alarm.setStateValue(state); + alarm.setStateReason("AlarmRule evaluated to " + state); + alarm.setStateReasonData(null); + alarm.setStateUpdatedTimestamp(Instant.now().getEpochSecond()); + alarmStore.put(region + "::" + alarm.getAlarmName(), alarm); + changed.add(alarm.getAlarmName()); + changed.add(alarm.getAlarmArn()); + } + iterator.remove(); + progress = true; + } + } while (progress && !pending.isEmpty()); + if (!pending.isEmpty()) { + LOG.debugv("Composite alarm evaluation stopped at cyclic dependencies: {0}", pending.keySet()); + } + } + + public synchronized void deleteAlarms(List alarmNames, String region) { + Map alarms = alarmIndex(region); + long composites = alarmNames.stream().map(alarms::get).filter(CompositeAlarm.class::isInstance).count(); + if (composites > 1) { + throw new AwsException("ValidationError", "DeleteAlarms accepts at most one composite alarm", 400); + } for (String name : alarmNames) { + if (isCyclic(name, alarms)) { + throw new AwsException("ValidationError", "Break the composite alarm cycle before deleting " + name, 400); + } + } + Set deletedNames = new HashSet<>(); + for (String name : alarmNames) { + if (alarmStore.get(region + "::" + name).isPresent()) { + deletedNames.add(name); + } alarmStore.delete(region + "::" + name); } + evaluateComposites(region, deletedNames, null); LOG.infov("Deleted alarms: {0} in {1}", alarmNames, region); } + private boolean isCyclic(String name, Map alarms) { + Set visited = new HashSet<>(); + ArrayDeque pending = new ArrayDeque<>(); + pending.add(name); + while (!pending.isEmpty()) { + Alarm next = alarms.get(pending.remove()); + if (next instanceof CompositeAlarm composite && visited.add(next.getAlarmName())) { + for (String reference : CompositeAlarmRule.parse(composite.getAlarmRule()).references()) { + Alarm child = alarms.get(reference); + if (child != null && child.getAlarmName().equals(name)) { + return true; + } + if (child != null) { + pending.add(child.getAlarmName()); + } + } + } + } + return false; + } + public void setAlarmState(String alarmName, String stateValue, String stateReason, String stateReasonData, String region) { - String key = region + "::" + alarmName; - MetricAlarm alarm = alarmStore.get(key) - .orElseThrow(() -> new AwsException("ResourceNotFound", "Alarm not found: " + alarmName, 404)); + List notifications = List.of(); + synchronized (this) { + String key = region + "::" + alarmName; + Alarm alarm = alarmStore.get(key) + .orElseThrow(() -> new AwsException("ResourceNotFound", "Alarm not found: " + alarmName, 404)); - String oldState = alarm.getStateValue(); - alarm.setStateValue(stateValue); - alarm.setStateReason(stateReason); - alarm.setStateReasonData(stateReasonData); - alarm.setStateUpdatedTimestamp(Instant.now().getEpochSecond()); + String oldState = alarm.getStateValue(); + alarm.setStateValue(stateValue); + alarm.setStateReason(stateReason); + alarm.setStateReasonData(stateReasonData); + alarm.setStateUpdatedTimestamp(Instant.now().getEpochSecond()); - alarmStore.put(key, alarm); - LOG.infov("SetAlarmState: {0} -> {1}", alarmName, stateValue); + alarmStore.put(key, alarm); + evaluateComposites(region, Set.of(alarmName), null); + LOG.infov("SetAlarmState: {0} -> {1}", alarmName, stateValue); + if (alarmSnsNotifier != null && alarm instanceof MetricAlarm metricAlarm) { + notifications = alarmSnsNotifier.prepareTransition(metricAlarm, oldState); + } + } if (alarmSnsNotifier != null) { - alarmSnsNotifier.notifyTransition(alarm, oldState); + alarmSnsNotifier.publish(notifications); } } @@ -364,7 +572,7 @@ public class CloudWatchMetricsService { * {@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) { + private Alarm requireAlarm(String resourceArn, String region) { return alarmStore.scan(k -> k.startsWith(region + "::")) .stream() .filter(a -> a.getAlarmArn() != null && a.getAlarmArn().equals(resourceArn)) @@ -373,18 +581,18 @@ public class CloudWatchMetricsService { "No CloudWatch resource matches the ARN " + resourceArn + ".", 404)); } - public Map listTagsForResource(String resourceArn, String region) { - return requireAlarm(resourceArn, region).getTags(); + public synchronized Map listTagsForResource(String resourceArn, String region) { + return new HashMap<>(requireAlarm(resourceArn, region).getTags()); } - public void tagResource(String resourceArn, Map tags, String region) { - MetricAlarm alarm = requireAlarm(resourceArn, region); + public synchronized void tagResource(String resourceArn, Map tags, String region) { + Alarm alarm = requireAlarm(resourceArn, region); alarm.getTags().putAll(tags); alarmStore.put(region + "::" + alarm.getAlarmName(), alarm); } - public void untagResource(String resourceArn, List tagKeys, String region) { - MetricAlarm alarm = requireAlarm(resourceArn, region); + public synchronized void untagResource(String resourceArn, List tagKeys, String region) { + Alarm alarm = requireAlarm(resourceArn, region); tagKeys.forEach(alarm.getTags()::remove); alarmStore.put(region + "::" + alarm.getAlarmName(), alarm); } diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmRule.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmRule.java new file mode 100644 index 0000000..8d2d97e --- /dev/null +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmRule.java @@ -0,0 +1,184 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics; + +import io.github.hectorvent.floci.core.common.AwsException; +import io.github.hectorvent.floci.core.common.SmithyValidationErrors; + +import java.util.HashSet; +import java.util.Set; +import java.util.function.Function; + +/** CloudWatch's boolean alarm language, parsed completely before any alarm is stored. */ +final class CompositeAlarmRule { + private final String input; + private final Set references = new HashSet<>(); + private int offset; + private int elements; + + private CompositeAlarmRule(String input) { + this.input = input; + } + + static Rule parse(String input) { + if (input == null || input.isBlank() || input.length() > 10240) { + throw SmithyValidationErrors.validationError(input == null ? SmithyValidationErrors.notNull("alarmRule") + : "Value '" + input + "' at 'alarmRule' failed to satisfy constraint: Member must have length " + + (input.length() > 10240 ? "less than or equal to 10240" : "greater than or equal to 1")); + } + CompositeAlarmRule parser = new CompositeAlarmRule(input); + Expression expression = parser.or(); + parser.whitespace(); + if (parser.offset != input.length()) { + throw invalid("Invalid AlarmRule"); + } + return new Rule(expression, Set.copyOf(parser.references)); + } + + private Expression or() { + Expression left = and(); + while (word("OR")) { + Expression prior = left; + Expression right = and(); + left = states -> prior.evaluate(states) || right.evaluate(states); + } + return left; + } + + private Expression and() { + Expression left = atom(); + while (word("AND")) { + Expression prior = left; + Expression right = atom(); + left = states -> prior.evaluate(states) && right.evaluate(states); + } + return left; + } + + private Expression atom() { + boolean negate = false; + while (word("NOT")) { + negate = !negate; + } + Expression operand = primary(); + return negate ? states -> !operand.evaluate(states) : operand; + } + + private Expression primary() { + element(); + if (character('(')) { + Expression nested = or(); + require(')'); + element(); + return nested; + } + if (word("TRUE")) { + return states -> true; + } + if (word("FALSE")) { + return states -> false; + } + String state; + if (word("ALARM")) { + state = "ALARM"; + } else if (word("OK")) { + state = "OK"; + } else if (word("INSUFFICIENT_DATA")) { + state = "INSUFFICIENT_DATA"; + } else { + throw invalid("Expected an alarm state, boolean, NOT, or parenthesis"); + } + require('('); + whitespace(); + String name; + if (character('"')) { + StringBuilder quoted = new StringBuilder(); + boolean closed = false; + while (offset < input.length()) { + char next = input.charAt(offset++); + if (next == '"') { + closed = true; + break; + } + if (next == '\\') { + if (offset == input.length()) { + throw invalid("Unterminated quoted alarm name"); + } + next = input.charAt(offset++); + if (next != '\\' && next != '"') { + throw invalid("Invalid quoted alarm name escape"); + } + } + quoted.append(next); + } + if (!closed) { + throw invalid("Unterminated quoted alarm name"); + } + name = quoted.toString(); + } else { + int start = offset; + while (offset < input.length() && input.charAt(offset) != ')') { + offset++; + } + name = input.substring(start, offset).trim(); + } + if (name.isEmpty()) { + throw invalid("An alarm name or ARN is required"); + } + require(')'); + references.add(name); + return states -> state.equals(states.apply(name)); + } + + private void element() { + if (++elements > 500) { + throw invalid("AlarmRule exceeds 500 child alarms, booleans, and parentheses"); + } + } + + private boolean word(String word) { + whitespace(); + if (!input.startsWith(word, offset)) { + return false; + } + int end = offset + word.length(); + if (end < input.length() && (Character.isLetterOrDigit(input.charAt(end)) || input.charAt(end) == '_')) { + return false; + } + offset = end; + return true; + } + + private boolean character(char expected) { + whitespace(); + if (offset < input.length() && input.charAt(offset) == expected) { + offset++; + return true; + } + return false; + } + + private void require(char expected) { + if (!character(expected)) { + throw invalid("Expected '" + expected + "' in AlarmRule"); + } + } + + private void whitespace() { + while (offset < input.length() && Character.isWhitespace(input.charAt(offset))) { + offset++; + } + } + + private static AwsException invalid(String message) { + return new AwsException("ValidationError", message, 400); + } + + record Rule(Expression expression, Set references) { + boolean evaluate(Function states) { + return expression.evaluate(states); + } + } + + interface Expression { + boolean evaluate(Function states); + } +} diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/Alarm.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/Alarm.java new file mode 100644 index 0000000..ce81d6c --- /dev/null +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/Alarm.java @@ -0,0 +1,87 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics.model; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import io.quarkus.runtime.annotations.RegisterForReflection; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +@RegisterForReflection +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, + property = "alarmType", defaultImpl = MetricAlarm.class) +@JsonSubTypes({@JsonSubTypes.Type(value = MetricAlarm.class, name = "MetricAlarm"), + @JsonSubTypes.Type(value = CompositeAlarm.class, name = "CompositeAlarm")}) +public abstract class Alarm { + private String alarmName; + private String alarmArn; + private String alarmDescription; + private long alarmConfigurationUpdatedTimestamp; + private boolean actionsEnabled; + private List okActions = new ArrayList<>(); + private List alarmActions = new ArrayList<>(); + private List insufficientDataActions = new ArrayList<>(); + private String stateValue = "INSUFFICIENT_DATA"; + private String stateReason = "Unchecked"; + private String stateReasonData; + private long stateUpdatedTimestamp; + private Map tags = new HashMap<>(); + private String region; + + + protected Alarm() { + long now = Instant.now().getEpochSecond(); + this.alarmConfigurationUpdatedTimestamp = now; + this.stateUpdatedTimestamp = now; + } + + public abstract String getAlarmType(); + + public String getAlarmName() { return alarmName; } + public void setAlarmName(String alarmName) { this.alarmName = alarmName; } + + public String getAlarmArn() { return alarmArn; } + public void setAlarmArn(String alarmArn) { this.alarmArn = alarmArn; } + + public String getAlarmDescription() { return alarmDescription; } + public void setAlarmDescription(String alarmDescription) { this.alarmDescription = alarmDescription; } + + public long getAlarmConfigurationUpdatedTimestamp() { return alarmConfigurationUpdatedTimestamp; } + public void setAlarmConfigurationUpdatedTimestamp(long timestamp) { this.alarmConfigurationUpdatedTimestamp = timestamp; } + + public boolean isActionsEnabled() { return actionsEnabled; } + public void setActionsEnabled(boolean actionsEnabled) { this.actionsEnabled = actionsEnabled; } + + public List getOkActions() { return okActions; } + public void setOkActions(List okActions) { this.okActions = okActions; } + + public List getAlarmActions() { return alarmActions; } + public void setAlarmActions(List alarmActions) { this.alarmActions = alarmActions; } + + public List getInsufficientDataActions() { return insufficientDataActions; } + public void setInsufficientDataActions(List insufficientDataActions) { this.insufficientDataActions = insufficientDataActions; } + + public String getStateValue() { return stateValue; } + public void setStateValue(String stateValue) { this.stateValue = stateValue; } + + public String getStateReason() { return stateReason; } + public void setStateReason(String stateReason) { this.stateReason = stateReason; } + + public String getStateReasonData() { return stateReasonData; } + public void setStateReasonData(String stateReasonData) { this.stateReasonData = stateReasonData; } + + public long getStateUpdatedTimestamp() { return stateUpdatedTimestamp; } + public void setStateUpdatedTimestamp(long stateUpdatedTimestamp) { this.stateUpdatedTimestamp = stateUpdatedTimestamp; } + + public Map getTags() { return tags; } + public void setTags(Map tags) { this.tags = tags; } + + + public String getRegion() { return region; } + public void setRegion(String region) { this.region = region; } +} diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/CompositeAlarm.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/CompositeAlarm.java new file mode 100644 index 0000000..c426e31 --- /dev/null +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/CompositeAlarm.java @@ -0,0 +1,14 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics.model; + +import io.quarkus.runtime.annotations.RegisterForReflection; + +@RegisterForReflection +public class CompositeAlarm extends Alarm { + private String alarmRule; + + @Override + public String getAlarmType() { return "CompositeAlarm"; } + + public String getAlarmRule() { return alarmRule; } + public void setAlarmRule(String alarmRule) { this.alarmRule = alarmRule; } +} diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/MetricAlarm.java b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/MetricAlarm.java index 7347be3..0701772 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/MetricAlarm.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudwatch/metrics/model/MetricAlarm.java @@ -3,27 +3,15 @@ package io.github.hectorvent.floci.services.cloudwatch.metrics.model; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import io.quarkus.runtime.annotations.RegisterForReflection; -import java.time.Instant; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; @RegisterForReflection @JsonIgnoreProperties(ignoreUnknown = true) -public class MetricAlarm { - private String alarmName; - private String alarmArn; - private String alarmDescription; - private long alarmConfigurationUpdatedTimestamp; - private boolean actionsEnabled; - private List okActions = new ArrayList<>(); - private List alarmActions = new ArrayList<>(); - private List insufficientDataActions = new ArrayList<>(); - private String stateValue = "INSUFFICIENT_DATA"; - private String stateReason = "Unchecked"; - private String stateReasonData; - private long stateUpdatedTimestamp; +public class MetricAlarm extends Alarm { + @Override + public String getAlarmType() { return "MetricAlarm"; } + private String metricName; private String namespace; private String statistic; @@ -45,50 +33,6 @@ public class MetricAlarm { private String comparisonOperator; private String treatMissingData = "missing"; private String evaluateLowSampleCountPercentile; - private Map tags = new HashMap<>(); - private String region; - - public MetricAlarm() { - long now = Instant.now().getEpochSecond(); - this.alarmConfigurationUpdatedTimestamp = now; - this.stateUpdatedTimestamp = now; - } - - public String getAlarmName() { return alarmName; } - public void setAlarmName(String alarmName) { this.alarmName = alarmName; } - - public String getAlarmArn() { return alarmArn; } - public void setAlarmArn(String alarmArn) { this.alarmArn = alarmArn; } - - public String getAlarmDescription() { return alarmDescription; } - public void setAlarmDescription(String alarmDescription) { this.alarmDescription = alarmDescription; } - - public long getAlarmConfigurationUpdatedTimestamp() { return alarmConfigurationUpdatedTimestamp; } - public void setAlarmConfigurationUpdatedTimestamp(long timestamp) { this.alarmConfigurationUpdatedTimestamp = timestamp; } - - public boolean isActionsEnabled() { return actionsEnabled; } - public void setActionsEnabled(boolean actionsEnabled) { this.actionsEnabled = actionsEnabled; } - - public List getOkActions() { return okActions; } - public void setOkActions(List okActions) { this.okActions = okActions; } - - public List getAlarmActions() { return alarmActions; } - public void setAlarmActions(List alarmActions) { this.alarmActions = alarmActions; } - - public List getInsufficientDataActions() { return insufficientDataActions; } - public void setInsufficientDataActions(List insufficientDataActions) { this.insufficientDataActions = insufficientDataActions; } - - public String getStateValue() { return stateValue; } - public void setStateValue(String stateValue) { this.stateValue = stateValue; } - - public String getStateReason() { return stateReason; } - public void setStateReason(String stateReason) { this.stateReason = stateReason; } - - public String getStateReasonData() { return stateReasonData; } - public void setStateReasonData(String stateReasonData) { this.stateReasonData = stateReasonData; } - - public long getStateUpdatedTimestamp() { return stateUpdatedTimestamp; } - public void setStateUpdatedTimestamp(long stateUpdatedTimestamp) { this.stateUpdatedTimestamp = stateUpdatedTimestamp; } public String getMetricName() { return metricName; } public void setMetricName(String metricName) { this.metricName = metricName; } @@ -131,11 +75,4 @@ public class MetricAlarm { public String getEvaluateLowSampleCountPercentile() { return evaluateLowSampleCountPercentile; } public void setEvaluateLowSampleCountPercentile(String percentile) { this.evaluateLowSampleCountPercentile = percentile; } - public Map getTags() { return tags; } - public void setTags(Map tags) { this.tags = tags; } - - /** Not part of the AWS shape; set by {@code putMetricAlarm} so a background evaluator - * that only holds the alarm object still knows which region's metric data to query. */ - public String getRegion() { return region; } - public void setRegion(String region) { this.region = region; } } diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudformation/CfnProvisionerFixture.java b/src/test/java/io/github/hectorvent/floci/services/cloudformation/CfnProvisionerFixture.java index 9a01129..b131756 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudformation/CfnProvisionerFixture.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudformation/CfnProvisionerFixture.java @@ -45,6 +45,7 @@ import io.github.hectorvent.floci.services.cloudformation.provisioners.AppSyncCf import io.github.hectorvent.floci.services.cloudformation.provisioners.CdkMetadataCfnProvisioner; import io.github.hectorvent.floci.services.cloudformation.provisioners.CloudTrailCfnProvisioner; import io.github.hectorvent.floci.services.cloudformation.provisioners.CloudWatchCfnProvisioner; +import io.github.hectorvent.floci.services.cloudformation.provisioners.CloudWatchCompositeAlarmCfnProvisioner; import io.github.hectorvent.floci.services.cloudformation.provisioners.CloudWatchDashboardCfnProvisioner; import io.github.hectorvent.floci.services.cloudformation.provisioners.CognitoCfnProvisioner; import io.github.hectorvent.floci.services.cloudformation.provisioners.CognitoUserPoolGroupCfnProvisioner; @@ -300,6 +301,7 @@ final class CfnProvisionerFixture { } if (cloudWatchMetricsService != null) { discovered.add(new CloudWatchCfnProvisioner(cloudWatchMetricsService)); + discovered.add(new CloudWatchCompositeAlarmCfnProvisioner(cloudWatchMetricsService)); } if (cloudWatchDashboardsService != null) { discovered.add(new CloudWatchDashboardCfnProvisioner(cloudWatchDashboardsService)); diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationCompositeAlarmIntegrationTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationCompositeAlarmIntegrationTest.java new file mode 100644 index 0000000..42d8805 --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationCompositeAlarmIntegrationTest.java @@ -0,0 +1,190 @@ +package io.github.hectorvent.floci.services.cloudformation; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.github.hectorvent.floci.testing.CfnStackWaits; +import io.github.hectorvent.floci.testing.PartitionCleanup; +import io.github.hectorvent.floci.testing.RestAssuredJsonUtils; +import io.quarkus.test.common.http.TestHTTPResource; +import io.quarkus.test.junit.QuarkusTest; +import io.restassured.response.Response; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.net.URI; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static io.restassured.RestAssured.given; +import static org.junit.jupiter.api.Assertions.*; + +@QuarkusTest +class CloudFormationCompositeAlarmIntegrationTest { + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @TestHTTPResource + URI baseUri; + + @RegisterExtension + final PartitionCleanup cleanup = new PartitionCleanup(); + + @BeforeAll + static void configureRestAssured() { + RestAssuredJsonUtils.configureAwsContentTypes(); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void failedUpdateRestoresRuleTagsAndIdentity(boolean rename) throws Exception { + String stack = unique("composite-rollback"); + String name = stack + "-alarm"; + String next = rename ? name + "-new" : name; + deploy("CreateStack", stack, template(name, "TRUE", Map.of("team", "old", "drop", "restore"), false, false)); + assertEquals("CREATE_COMPLETE", CfnStackWaits.awaitTerminal(stack).status()); + String arn = alarm(name).get("AlarmArn").toString(); + Response described = given().contentType("application/x-www-form-urlencoded") + .formParam("Action", "DescribeStacks").formParam("StackName", stack).post("/"); + assertEquals(name, described.xmlPath().getString( + "DescribeStacksResponse.DescribeStacksResult.Stacks.member.Outputs.member.find { it.OutputKey == 'Name' }.OutputValue")); + assertEquals(arn, described.xmlPath().getString( + "DescribeStacksResponse.DescribeStacksResult.Stacks.member.Outputs.member.find { it.OutputKey == 'Arn' }.OutputValue")); + cw("TagResource", Map.of("ResourceARN", arn, "Tags", List.of(Map.of("Key", "external", "Value", "keep")))) + .then().statusCode(200); + deploy("UpdateStack", stack, template(next, "FALSE", Map.of("team", "new", "added", "remove"), true, false)); + assertEquals("UPDATE_ROLLBACK_COMPLETE", CfnStackWaits.awaitTerminal(stack).status()); + assertEquals("TRUE", alarm(name).get("AlarmRule")); + List> tags = cw("ListTagsForResource", Map.of("ResourceARN", arn)) + .then().statusCode(200).extract().jsonPath().getList("Tags"); + assertEquals(Map.of("team", "old", "drop", "restore", "external", "keep"), + tags.stream().collect(Collectors.toMap(t -> t.get("Key"), t -> t.get("Value")))); + if (rename) { + assertMissing(next); + } + deleteStack(stack); + assertMissing(name); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void replacementHonorsRetentionAndDeletesItsCurrentBackingAlarm(boolean retain) throws Exception { + String stack = unique("composite-replace"); + String original = stack + "-old"; + String next = stack + "-new"; + deploy("CreateStack", stack, template(original, "TRUE", Map.of(), false, retain)); + assertEquals("CREATE_COMPLETE", CfnStackWaits.awaitTerminal(stack).status()); + deploy("UpdateStack", stack, template(next, "FALSE", Map.of(), false, retain)); + assertEquals("UPDATE_COMPLETE", CfnStackWaits.awaitTerminal(stack).status()); + assertEquals("FALSE", alarm(next).get("AlarmRule")); + if (retain) { + assertEquals("TRUE", alarm(original).get("AlarmRule")); + } else { + assertMissing(original); + } + deleteStack(stack); + assertMissing(next); + if (retain) { + assertEquals("TRUE", alarm(original).get("AlarmRule")); + cw("DeleteAlarms", Map.of("AlarmNames", List.of(original))).then().statusCode(200); + } + } + + @Test + void committedInPlaceUpdatesAllowAnotherUpdateAndReplacement() throws Exception { + String stack = unique("composite-sequence"); + String name = stack + "-alarm"; + String replacement = name + "-new"; + deploy("CreateStack", stack, template(name, "TRUE", Map.of("team", "initial"), false, false)); + assertEquals("CREATE_COMPLETE", CfnStackWaits.awaitTerminal(stack).status()); + for (String rule : List.of("FALSE", "TRUE")) { + deploy("UpdateStack", stack, template(name, rule, Map.of("team", rule), false, false)); + assertEquals("UPDATE_COMPLETE", CfnStackWaits.awaitTerminal(stack).status()); + assertEquals(rule, alarm(name).get("AlarmRule")); + } + deploy("UpdateStack", stack, template(replacement, "FALSE", Map.of(), false, false)); + assertEquals("UPDATE_COMPLETE", CfnStackWaits.awaitTerminal(stack).status()); + assertMissing(name); + assertEquals("FALSE", alarm(replacement).get("AlarmRule")); + deleteStack(stack); + assertMissing(replacement); + } + + @Test + void createCollisionCannotOverwriteOrDeleteAnUnmanagedAlarm() throws Exception { + String stack = unique("composite-foreign"); + String name = stack + "-alarm"; + cw("PutCompositeAlarm", Map.of("AlarmName", name, "AlarmRule", "TRUE")).then().statusCode(200); + deploy("CreateStack", stack, template(name, "FALSE", Map.of(), false, false)); + assertEquals("ROLLBACK_COMPLETE", CfnStackWaits.awaitTerminal(stack).status()); + assertEquals("TRUE", alarm(name).get("AlarmRule")); + deleteStack(stack); + assertEquals("TRUE", alarm(name).get("AlarmRule")); + cw("DeleteAlarms", Map.of("AlarmNames", List.of(name))).then().statusCode(200); + } + + private String template(String name, String rule, Map tags, boolean fail, boolean retain) + throws JsonProcessingException { + cleanup.register(() -> cw("DeleteAlarms", Map.of("AlarmNames", List.of(name))).then().statusCode(200)); + Map resources = new LinkedHashMap<>(); + resources.put("Alarm", Map.of("Type", "AWS::CloudWatch::CompositeAlarm", + "UpdateReplacePolicy", retain ? "Retain" : "Delete", + "Properties", Map.of("AlarmName", name, "AlarmRule", rule, + "Tags", tags.entrySet().stream().map(e -> Map.of("Key", e.getKey(), "Value", e.getValue())).toList()))); + if (fail) { + resources.put("Failure", Map.of("Type", "AWS::SecretsManager::Secret", "DependsOn", "Alarm", + "Properties", Map.of("SecretString", "explicit", "GenerateSecretString", Map.of("PasswordLength", 32)))); + } + return MAPPER.writeValueAsString(Map.of("Resources", resources, "Outputs", Map.of( + "Name", Map.of("Value", Map.of("Ref", "Alarm")), + "Arn", Map.of("Value", Map.of("Fn::GetAtt", List.of("Alarm", "Arn")))))); + } + + private void deploy(String action, String stack, String template) { + if ("CreateStack".equals(action)) { + cleanup.register(() -> given().port(baseUri.getPort()) + .contentType("application/x-www-form-urlencoded") + .formParam("Action", "DeleteStack").formParam("StackName", stack) + .post("/").then().statusCode(200)); + } + given().contentType("application/x-www-form-urlencoded").formParam("Action", action) + .formParam("StackName", stack).formParam("TemplateBody", template).post("/").then().statusCode(200); + } + + private Response cw(String operation, Map body) { + if ("PutCompositeAlarm".equals(operation)) { + String name = body.get("AlarmName").toString(); + cleanup.register(() -> cw("DeleteAlarms", Map.of("AlarmNames", List.of(name))).then().statusCode(200)); + } + return given().port(baseUri.getPort()).contentType("application/x-amz-json-1.0") + .header("X-Amz-Target", "GraniteServiceVersion20100801." + operation) + .body(MAPPER.valueToTree(body).toString()).post("/"); + } + + private Map alarm(String name) throws JsonProcessingException { + List> alarms = cw("DescribeAlarms", Map.of("AlarmNames", List.of(name), + "AlarmTypes", List.of("CompositeAlarm"))).then().statusCode(200) + .extract().jsonPath().getList("CompositeAlarms"); + assertEquals(1, alarms.size(), name); + return alarms.getFirst(); + } + + private void assertMissing(String name) throws JsonProcessingException { + assertEquals(List.of(), cw("DescribeAlarms", Map.of("AlarmNames", List.of(name), + "AlarmTypes", List.of("CompositeAlarm"))).then().statusCode(200) + .extract().jsonPath().getList("CompositeAlarms")); + } + + private void deleteStack(String stack) { + given().contentType("application/x-www-form-urlencoded").formParam("Action", "DeleteStack") + .formParam("StackName", stack).post("/").then().statusCode(200); + CfnStackWaits.awaitStackDeleted(stack); + } + + private static String unique(String prefix) { + return prefix + "-" + Long.toString(System.nanoTime(), 36); + } +} diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudformation/provisioners/CloudWatchCompositeAlarmCfnProvisionerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudformation/provisioners/CloudWatchCompositeAlarmCfnProvisionerTest.java new file mode 100644 index 0000000..c1a208a --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/cloudformation/provisioners/CloudWatchCompositeAlarmCfnProvisionerTest.java @@ -0,0 +1,89 @@ +package io.github.hectorvent.floci.services.cloudformation.provisioners; + +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.services.cloudformation.CloudFormationTemplateEngine; +import io.github.hectorvent.floci.services.cloudformation.model.StackResource; +import io.github.hectorvent.floci.services.cloudwatch.metrics.CloudWatchMetricsService; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +class CloudWatchCompositeAlarmCfnProvisionerTest { + @Test + void rejectedBackingUpdateDoesNotArmRollbackToDeleteAnUnmanagedReplacement() throws Exception { + CloudWatchMetricsService service = mock(CloudWatchMetricsService.class); + CloudWatchCompositeAlarmCfnProvisioner provisioner = new CloudWatchCompositeAlarmCfnProvisioner(service); + CloudFormationTemplateEngine engine = mock(CloudFormationTemplateEngine.class); + when(engine.resolve(any())).thenAnswer(call -> { + JsonNode node = call.getArgument(0); + return node == null || node.isMissingNode() ? null : node.asText(); + }); + when(engine.resolveNode(any())).thenAnswer(call -> call.getArgument(0)); + StackResource resource = new StackResource(); + resource.setLogicalId("Alarm"); + resource.setResourceType("AWS::CloudWatch::CompositeAlarm"); + resource.setPhysicalId("kept"); + resource.getAttributes().put("Arn", "prior-arn"); + AwsException collision = new AwsException("ValidationError", "A metric alarm now has this name", 400); + doThrow(collision).when(service).putCompositeAlarm(any(), eq("us-east-1"), eq(true)); + ProvisionContext context = new ProvisionContext(engine, "us-east-1", "000000000000", "stack", "kept"); + JsonNode props = new ObjectMapper().readTree("{\"AlarmName\":\"kept\",\"AlarmRule\":\"TRUE\"}"); + assertSame(collision, assertThrows(AwsException.class, () -> provisioner.provision(resource, props, context))); + assertFalse(provisioner.retainsFailedUpdateState(resource)); + assertFalse(provisioner.rollbackUpdate(resource)); + assertEquals("kept", resource.getPhysicalId()); + assertEquals(Map.of("Arn", "prior-arn"), resource.getAttributes()); + verify(service, never()).deleteAlarms(any(), anyString()); + } + + @Test + void inPlaceUpdateRetriesPreviouslyDisplacedAlarmCleanup() throws Exception { + CloudWatchMetricsService service = mock(CloudWatchMetricsService.class); + CloudWatchCompositeAlarmCfnProvisioner provisioner = new CloudWatchCompositeAlarmCfnProvisioner(service); + CloudFormationTemplateEngine engine = mock(CloudFormationTemplateEngine.class); + when(engine.resolve(any())).thenAnswer(call -> { + JsonNode node = call.getArgument(0); + return node == null || node.isMissingNode() ? null : node.asText(); + }); + when(engine.resolveNode(any())).thenAnswer(call -> call.getArgument(0)); + StackResource resource = new StackResource(); + resource.setLogicalId("Alarm"); + resource.setResourceType("AWS::CloudWatch::CompositeAlarm"); + resource.setPhysicalId("current"); + ReplacementCleanup.record(resource, + new ProvisionContext(engine, "us-east-1", "000000000000", "stack", "displaced"), Map.of()); + doThrow(new IllegalStateException("delete unavailable")) + .doThrow(new IllegalStateException("delete still unavailable")).doNothing() + .when(service).deleteAlarms(List.of("displaced"), "us-east-1"); + assertFalse(provisioner.completeUpdate(resource).complete()); + provisioner.clearUpdate(resource); + assertEquals(Set.of("displaced"), ReplacementCleanup.owedPhysicalIds(resource)); + + ProvisionContext context = new ProvisionContext(engine, "us-east-1", "000000000000", "stack", "current"); + JsonNode props = new ObjectMapper().readTree("{\"AlarmName\":\"current\",\"AlarmRule\":\"TRUE\"}"); + provisioner.provision(resource, props, context); + assertTrue(provisioner.retainsFailedUpdateState(resource)); + UpdateCleanupResult retry = provisioner.completeUpdate(resource); + assertFalse(retry.complete()); + assertEquals(2, retry.attempts()); + assertEquals("displaced", retry.previousPhysicalId()); + assertEquals("delete still unavailable", retry.failureReason()); + assertEquals(Set.of("displaced"), ReplacementCleanup.owedPhysicalIds(resource)); + assertTrue(provisioner.completeUpdate(resource).complete()); + provisioner.clearUpdate(resource); + assertFalse(provisioner.retainsFailedUpdateState(resource)); + assertTrue(ReplacementCleanup.owedPhysicalIds(resource).isEmpty()); + assertEquals("current", resource.getPhysicalId()); + verify(service, times(3)).deleteAlarms(List.of("displaced"), "us-east-1"); + verify(service, never()).deleteAlarms(List.of("current"), "us-east-1"); + } + +} diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifierTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifierTest.java index 8a638b6..f6ebcad 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifierTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/AlarmSnsNotifierTest.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import io.github.hectorvent.floci.core.common.RegionResolver; import io.github.hectorvent.floci.core.storage.InMemoryStorage; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.CompositeAlarm; import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; import io.github.hectorvent.floci.services.sns.SnsService; import org.junit.jupiter.api.BeforeEach; @@ -12,6 +13,11 @@ import org.mockito.ArgumentCaptor; import java.time.Instant; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.*; @@ -124,4 +130,41 @@ class AlarmSnsNotifierTest { service.setAlarmState("test-alarm", "ALARM", "breached", null, REGION); verify(sns).publish(eq(topic), isNull(), contains("arn:aws-cn:cloudwatch:"), anyString(), eq("cn-north-1")); } + + @Test + void blockedNotificationDoesNotHoldTheAlarmMutationLock() throws Exception { + CountDownLatch publishing = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + doAnswer(call -> { + if (call.getArgument(3).startsWith("ALARM:")) { + publishing.countDown(); + assertTrue(release.await(5, TimeUnit.SECONDS)); + } + return "message-id"; + }).when(sns).publish(eq(TOPIC), isNull(), anyString(), anyString(), eq(REGION)); + + try (ExecutorService executor = Executors.newFixedThreadPool(2)) { + Future transition = executor.submit(() -> + service.setAlarmState("test-alarm", "ALARM", "breached", null, REGION)); + try { + assertTrue(publishing.await(2, TimeUnit.SECONDS)); + Future unrelated = executor.submit(() -> { + CompositeAlarm composite = new CompositeAlarm(); + composite.setAlarmName("unrelated"); + composite.setAlarmRule("FALSE"); + service.putCompositeAlarm(composite, REGION); + service.setAlarmState("test-alarm", "OK", "recovered", null, REGION); + }); + unrelated.get(2, TimeUnit.SECONDS); + assertEquals("OK", alarm.getStateValue()); + } finally { + release.countDown(); + } + transition.get(2, TimeUnit.SECONDS); + } + ArgumentCaptor messages = ArgumentCaptor.forClass(String.class); + verify(sns, times(2)).publish(eq(TOPIC), isNull(), messages.capture(), anyString(), eq(REGION)); + assertEquals("ALARM", mapper.readTree(messages.getAllValues().getFirst()).path("NewStateValue").asText()); + assertEquals("OK", mapper.readTree(messages.getAllValues().getLast()).path("NewStateValue").asText()); + } } diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchAlarmQueryHandlerTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchAlarmQueryHandlerTest.java new file mode 100644 index 0000000..0612237 --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchAlarmQueryHandlerTest.java @@ -0,0 +1,45 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics; + +import io.github.hectorvent.floci.core.common.RegionResolver; +import io.github.hectorvent.floci.core.storage.InMemoryStorage; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Alarm; +import jakarta.ws.rs.core.MultivaluedHashMap; +import jakarta.ws.rs.core.MultivaluedMap; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.*; + +class CloudWatchAlarmQueryHandlerTest { + @ParameterizedTest + @ValueSource(strings = {"PutMetricAlarm", "PutCompositeAlarm"}) + void sharedFieldsReachBothAlarmTypes(String operation) { + String region = "us-east-1"; + CloudWatchMetricsService service = new CloudWatchMetricsService(new InMemoryStorage<>(), + new InMemoryStorage<>(), new RegionResolver(region, "123456789012")); + CloudWatchMetricsQueryHandler handler = new CloudWatchMetricsQueryHandler(service, null, null); + MultivaluedMap request = new MultivaluedHashMap<>(); + request.putSingle("AlarmName", "shared"); + request.putSingle("AlarmRule", "FALSE"); + request.putSingle("AlarmDescription", "shared fields"); + request.putSingle("ActionsEnabled", "false"); + request.putSingle("AlarmActions.member.1", "alarm-action"); + request.putSingle("OKActions.member.1", "ok-action"); + request.putSingle("InsufficientDataActions.member.1", "missing-action"); + request.putSingle("Tags.member.1.Key", "team"); + request.putSingle("Tags.member.1.Value", "metrics"); + assertEquals(200, handler.handle(operation, request, region).getStatus()); + Alarm alarm = "PutMetricAlarm".equals(operation) + ? service.describeAlarms(List.of("shared"), null, region).getFirst() + : service.describeCompositeAlarms(List.of("shared"), null, region).getFirst(); + assertEquals("shared fields", alarm.getAlarmDescription()); + assertFalse(alarm.isActionsEnabled()); + assertEquals(List.of("alarm-action"), alarm.getAlarmActions()); + assertEquals(List.of("ok-action"), alarm.getOkActions()); + assertEquals(List.of("missing-action"), alarm.getInsufficientDataActions()); + assertEquals(Map.of("team", "metrics"), alarm.getTags()); + } +} diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricMathAlarmTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricMathAlarmTest.java index e7b16a9..c540198 100644 --- a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricMathAlarmTest.java +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CloudWatchMetricMathAlarmTest.java @@ -10,7 +10,7 @@ import io.github.hectorvent.floci.core.storage.InMemoryStorage; import io.github.hectorvent.floci.core.storage.PersistentStorage; import io.github.hectorvent.floci.core.storage.StorageBackend; import io.github.hectorvent.floci.services.cloudwatch.dashboards.CloudWatchDashboardsService; -import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.Alarm; import jakarta.ws.rs.core.MultivaluedHashMap; import jakarta.ws.rs.core.MultivaluedMap; import org.junit.jupiter.api.BeforeEach; @@ -43,7 +43,7 @@ class CloudWatchMetricMathAlarmTest { initialize(new InMemoryStorage<>()); } - private void initialize(StorageBackend alarmStore) { + private void initialize(StorageBackend alarmStore) { RegionResolver resolver = new RegionResolver(REGION, "000000000000"); CloudWatchMetricsService service = new CloudWatchMetricsService(new InMemoryStorage<>(), alarmStore, resolver); CloudWatchDashboardsService dashboards = new CloudWatchDashboardsService(new InMemoryStorage<>(), resolver); @@ -64,12 +64,12 @@ class CloudWatchMetricMathAlarmTest { void metricMathQueriesSurvivePersistentStorageReload(boolean queryProtocol, @TempDir Path directory) throws Exception { Path storageFile = directory.resolve("alarms.json"); - TypeReference> type = new TypeReference<>() { }; + TypeReference> type = new TypeReference<>() { }; initialize(new PersistentStorage<>(storageFile, type)); ObjectNode request = metricMathRequest(); put(request, queryProtocol); - PersistentStorage reloaded = new PersistentStorage<>(storageFile, type); + PersistentStorage reloaded = new PersistentStorage<>(storageFile, type); reloaded.load(); assertEquals(1, reloaded.keys().size(), "the stored alarm must deserialize after a restart"); initialize(reloaded); diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmIntegrationTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmIntegrationTest.java new file mode 100644 index 0000000..d30ad43 --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmIntegrationTest.java @@ -0,0 +1,160 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.github.hectorvent.floci.testing.PartitionCleanup; +import io.github.hectorvent.floci.testing.RestAssuredJsonUtils; +import io.quarkus.test.common.http.TestHTTPResource; +import io.quarkus.test.junit.QuarkusTest; +import io.restassured.response.Response; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.net.URI; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import static io.restassured.RestAssured.given; +import static org.junit.jupiter.api.Assertions.*; + +@QuarkusTest +class CompositeAlarmIntegrationTest { + private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final String SCOPE = + "AWS4-HMAC-SHA256 Credential=test/20261010/cn-north-1/monitoring/aws4_request"; + + @TestHTTPResource + URI baseUri; + + @RegisterExtension + final PartitionCleanup cleanup = new PartitionCleanup(); + + @BeforeAll + static void configureRestAssured() { + RestAssuredJsonUtils.configureAwsContentTypes(); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void bothProtocolsCreateDescribeEvaluateAndDeleteRealAlarms(boolean json) throws Exception { + String prefix = "wire-composite-" + Long.toString(System.nanoTime(), 36); + String source = prefix + "-source"; + String aggregate = prefix + "-aggregate"; + call(json, "PutCompositeAlarm", Map.of("AlarmName", source, "AlarmRule", "TRUE")).then().statusCode(200); + call(json, "PutCompositeAlarm", Map.of("AlarmName", aggregate, "AlarmRule", "ALARM(\"" + source + "\")", + "ActionsEnabled", false, "AlarmDescription", "combined")).then().statusCode(200); + Response result = call(json, "DescribeAlarms", Map.of("AlarmNames", List.of(aggregate), + "AlarmTypes", List.of("CompositeAlarm"))); + result.then().statusCode(200); + if (json) { + assertEquals(List.of(), result.jsonPath().getList("LogAlarms")); + assertEquals(1, result.jsonPath().getList("CompositeAlarms").size()); + assertEquals("ALARM", result.jsonPath().getString("CompositeAlarms[0].StateValue")); + assertEquals("arn:aws-cn:cloudwatch:cn-north-1:000000000000:alarm:" + aggregate, + result.jsonPath().getString("CompositeAlarms[0].AlarmArn")); + assertFalse(result.jsonPath().getBoolean("CompositeAlarms[0].ActionsEnabled")); + } else { + assertTrue(result.asString().contains("")); + assertEquals("ALARM", result.xmlPath().getString( + "DescribeAlarmsResponse.DescribeAlarmsResult.CompositeAlarms.member.StateValue")); + assertTrue(result.asString().contains("arn:aws-cn:cloudwatch:cn-north-1:000000000000:alarm:" + aggregate)); + assertEquals("false", result.xmlPath().getString( + "DescribeAlarmsResponse.DescribeAlarmsResult.CompositeAlarms.member.ActionsEnabled")); + } + assertFalse(call(json, "DescribeAlarms", Map.of("AlarmNames", List.of(aggregate))) + .then().statusCode(200).extract().asString().contains(aggregate)); + call(json, "SetAlarmState", Map.of("AlarmName", source, "StateValue", "OK", + "StateReason", "recovery")).then().statusCode(200); + Response recovered = call(json, "DescribeAlarms", Map.of("AlarmNames", List.of(aggregate), + "AlarmTypes", List.of("CompositeAlarm"))); + assertEquals("OK", json ? recovered.jsonPath().getString("CompositeAlarms[0].StateValue") + : recovered.xmlPath().getString( + "DescribeAlarmsResponse.DescribeAlarmsResult.CompositeAlarms.member.StateValue")); + for (String name : List.of(aggregate, source)) { + call(json, "DeleteAlarms", Map.of("AlarmNames", List.of(name))).then().statusCode(200); + } + Response empty = call(json, "DescribeAlarms", Map.of("AlarmNamePrefix", prefix, + "AlarmTypes", List.of("CompositeAlarm"))); + assertFalse(empty.asString().contains(aggregate)); + assertFalse(empty.asString().contains(source)); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void invalidRuleAndUnsupportedSuppressionCannotCreateAnAlarm(boolean json) throws Exception { + String name = "invalid-composite-" + Long.toString(System.nanoTime(), 36); + call(json, "PutCompositeAlarm", Map.of("AlarmName", name, "AlarmRule", "TRUE AND")) + .then().statusCode(400); + call(json, "PutCompositeAlarm", Map.of("AlarmName", name, "AlarmRule", "TRUE", + "ActionsSuppressor", "other")).then().statusCode(400); + assertFalse(call(json, "DescribeAlarms", Map.of("AlarmNames", List.of(name), + "AlarmTypes", List.of("CompositeAlarm"))).asString().contains(name)); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void alarmTypeValidationAcceptsLogAlarmsAndUsesSmithyErrors(boolean json) throws Exception { + call(json, "DescribeAlarms", Map.of("AlarmTypes", List.of("LogAlarm"))).then().statusCode(200); + Response error = call(json, "DescribeAlarms", Map.of("AlarmTypes", List.of("Unknown"))); + error.then().statusCode(400); + assertEquals("1 validation error detected: Value '[Unknown]' at 'alarmTypes' failed to satisfy constraint: " + + "Member must satisfy constraint: [Member must satisfy enum value set: " + + "[CompositeAlarm, MetricAlarm, LogAlarm]]", errorMessage(json, error)); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void lengthErrorsUseSmithyMessages(boolean json) throws Exception { + for (String field : List.of("AlarmName", "AlarmRule")) { + int maximum = "AlarmName".equals(field) ? 255 : 10240; + String path = "AlarmName".equals(field) ? "alarmName" : "alarmRule"; + for (String value : List.of("", "x".repeat(maximum + 1))) { + Map request = new LinkedHashMap<>(Map.of("AlarmName", "length-validation", "AlarmRule", "TRUE")); + request.put(field, value); + Response error = call(json, "PutCompositeAlarm", request); + error.then().statusCode(400); + assertEquals("1 validation error detected: Value '" + value + "' at '" + path + + "' failed to satisfy constraint: Member must have length " + + (value.isEmpty() ? "greater than or equal to 1" : "less than or equal to " + maximum), + errorMessage(json, error)); + } + } + } + + private String errorMessage(boolean json, Response response) { + return json ? response.jsonPath().getString("message") : response.xmlPath().getString("ErrorResponse.Error.Message"); + } + + private Response call(boolean json, String operation, Map body) + throws JsonProcessingException { + if ("PutCompositeAlarm".equals(operation)) { + String name = body.get("AlarmName").toString(); + cleanup.register(() -> given().port(baseUri.getPort()).contentType("application/x-amz-json-1.0") + .header("Authorization", SCOPE) + .header("X-Amz-Target", "GraniteServiceVersion20100801.DeleteAlarms") + .body(MAPPER.valueToTree(Map.of("AlarmNames", List.of(name))).toString()) + .post("/").then().statusCode(200)); + } + if (json) { + return given().contentType("application/x-amz-json-1.0").header("Authorization", SCOPE) + .header("X-Amz-Target", "GraniteServiceVersion20100801." + operation) + .body(MAPPER.writeValueAsString(body)).post("/"); + } + Map form = new LinkedHashMap<>(); + form.put("Action", operation); + body.forEach((key, value) -> { + if (value instanceof List values) { + for (int index = 0; index < values.size(); index++) { + form.put(key + ".member." + (index + 1), values.get(index)); + } + } else { + form.put(key, value); + } + }); + return given().contentType("application/x-www-form-urlencoded").header("Authorization", SCOPE) + .formParams(form).post("/"); + } +} diff --git a/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmServiceTest.java b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmServiceTest.java new file mode 100644 index 0000000..42797f9 --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/services/cloudwatch/metrics/CompositeAlarmServiceTest.java @@ -0,0 +1,274 @@ +package io.github.hectorvent.floci.services.cloudwatch.metrics; + +import com.fasterxml.jackson.core.type.TypeReference; +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.metrics.model.Alarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.CompositeAlarm; +import io.github.hectorvent.floci.services.cloudwatch.metrics.model.MetricAlarm; +import io.github.hectorvent.floci.testing.ThreadStates; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.junit.jupiter.api.Assertions.*; + +class CompositeAlarmServiceTest { + private static final String REGION = "cn-north-1"; + private final CloudWatchMetricsService service = new CloudWatchMetricsService( + new InMemoryStorage<>(), new InMemoryStorage<>(), new RegionResolver(REGION, "123456789012")); + + @Test + void metricTransitionsPropagateThroughNamesArnsAndNestedComposites() { + MetricAlarm metric = new MetricAlarm(); + metric.setAlarmName("latency"); + service.putMetricAlarm(metric, REGION); + put("combined", "ALARM(\"" + metric.getAlarmArn() + "\") AND NOT ALARM(\"maintenance\")"); + put("outer", "ALARM(combined) OR (TRUE AND FALSE)"); + assertEquals("OK", state("outer")); + service.setAlarmState("latency", "ALARM", "high latency", null, REGION); + assertEquals("ALARM", state("combined")); + assertEquals("ALARM", state("outer")); + service.setAlarmState("latency", "OK", "recovered", null, REGION); + assertEquals("OK", state("outer")); + assertEquals(List.of(metric), service.describeAlarms(null, null, REGION)); + assertTrue(service.describeCompositeAlarms(null, null, "us-east-1").isEmpty()); + assertTrue(service.describeCompositeAlarms(List.of("combined"), null, REGION) + .getFirst().getAlarmArn().startsWith("arn:aws-cn:cloudwatch:")); + } + + @Test + void updatesPreserveStateAndTagsUntilTheNextDependencyChange() { + put("source", "FALSE"); + CompositeAlarm alarm = put("aggregate", "ALARM(source)"); + service.tagResource(alarm.getAlarmArn(), Map.of("owner", "retained"), REGION); + service.setAlarmState("aggregate", "ALARM", "manual", null, REGION); + CompositeAlarm replacement = put("aggregate", "OK(source)"); + assertEquals("ALARM", replacement.getStateValue()); + assertEquals(Map.of("owner", "retained"), replacement.getTags()); + service.setAlarmState("source", "ALARM", "dependency changed", null, REGION); + assertEquals("OK", state("aggregate")); + } + + @Test + void unrelatedWritesPreserveManualCompositeStatesAndParentStates() { + put("source", "FALSE"); + put("aggregate", "ALARM(source)"); + put("parent", "ALARM(aggregate)"); + service.setAlarmState("aggregate", "ALARM", "manual", null, REGION); + assertEquals("ALARM", state("parent")); + put("unrelated", "TRUE"); + assertEquals("ALARM", state("aggregate")); + assertEquals("ALARM", state("parent")); + MetricAlarm metric = new MetricAlarm(); + metric.setAlarmName("unrelated-metric"); + service.putMetricAlarm(metric, REGION); + assertEquals("ALARM", state("aggregate")); + service.setAlarmState("unrelated-metric", "ALARM", "unrelated", null, REGION); + assertEquals("ALARM", state("aggregate")); + service.deleteAlarms(List.of("unrelated"), REGION); + assertEquals("ALARM", state("aggregate")); + assertEquals("ALARM", state("parent")); + service.setAlarmState("source", "ALARM", "child changed", null, REGION); + service.setAlarmState("source", "OK", "child recovered", null, REGION); + assertEquals("OK", state("aggregate")); + assertEquals("OK", state("parent")); + } + + @Test + void unchangedChildStateDoesNotResetManuallySetParent() { + put("source", "FALSE"); + put("child", "ALARM(source) AND FALSE"); + put("parent", "ALARM(child)"); + service.setAlarmState("parent", "ALARM", "manual", null, REGION); + service.setAlarmState("source", "ALARM", "child condition remains false", null, REGION); + assertEquals("OK", state("child")); + assertEquals("ALARM", state("parent")); + } + + @Test + void creationAndDeletionReevaluateOnlyReferencingComposites() { + MetricAlarm metric = new MetricAlarm(); + metric.setAlarmName("late"); + put("watcher", "INSUFFICIENT_DATA(late)"); + put("outer", "ALARM(watcher)"); + assertEquals("OK", state("watcher")); + service.putMetricAlarm(metric, REGION); + assertEquals("ALARM", state("watcher")); + assertEquals("ALARM", state("outer")); + service.deleteAlarms(List.of("late"), REGION); + assertEquals("OK", state("watcher")); + assertEquals("OK", state("outer")); + } + + @Test + void tagReadIsAnIndependentSnapshotOfTheOwnedAlarm() { + CompositeAlarm alarm = put("tag-snapshot", "TRUE"); + service.tagResource(alarm.getAlarmArn(), Map.of("team", "old"), REGION); + Map read = service.listTagsForResource(alarm.getAlarmArn(), REGION); + service.tagResource(alarm.getAlarmArn(), Map.of("team", "new"), REGION); + assertEquals(Map.of("team", "old"), read); + read.put("injected", "unowned"); + assertEquals(Map.of("team", "new"), service.listTagsForResource(alarm.getAlarmArn(), REGION)); + } + + @Test + void cyclesStopEvaluationEvenBehindShortCircuitAndMustBeBrokenBeforeDelete() { + put("a", "FALSE AND ALARM(b)"); + put("b", "ALARM(a)"); + service.setAlarmState("b", "ALARM", "manual", null, REGION); + assertThrows(AwsException.class, () -> service.deleteAlarms(List.of("a"), REGION)); + assertEquals(2, service.describeCompositeAlarms(null, null, REGION).size()); + put("a", "FALSE"); + service.deleteAlarms(List.of("b"), REGION); + assertEquals("OK", state("a")); + service.deleteAlarms(List.of("a"), REGION); + assertTrue(service.describeCompositeAlarms(null, null, REGION).isEmpty()); + } + + @Test + void manuallyChangingOneMemberDoesNotResumeACyclicDependency() { + put("a", "ALARM(b)"); + put("b", "ALARM(a)"); + assertEquals("OK", state("a")); + service.setAlarmState("b", "ALARM", "manual", null, REGION); + assertEquals("OK", state("a")); + put("b", "TRUE"); + service.setAlarmState("b", "ALARM", "cycle broken", null, REGION); + assertEquals("ALARM", state("a")); + } + + @Test + void parentQuotaTreatsAChildNameAndItsArnAsTheSameAlarm() { + CompositeAlarm child = put("child", "FALSE"); + for (int index = 0; index < 150; index++) { + put("parent-" + index, "ALARM(child)"); + } + assertThrows(AwsException.class, () -> put("overflow", "ALARM(\"" + child.getAlarmArn() + "\")")); + service.deleteAlarms(List.of("parent-0"), REGION); + assertEquals("OK", put("replacement", "ALARM(\"" + child.getAlarmArn() + "\")").getStateValue()); + } + + @ParameterizedTest + @ValueSource(strings = {"", "ALARM()", "TRUE junk", "TRUE AND", "ALARM(\"unterminated)", + "(TRUE", "ALARM(x) XOR FALSE", "ALARM(\"x\\z\")"}) + void invalidRulesDoNotReplaceAnExistingAlarm(String invalid) { + put("saved", "TRUE"); + assertThrows(AwsException.class, () -> put("saved", invalid)); + assertEquals("TRUE", service.describeCompositeAlarms(List.of("saved"), null, REGION).getFirst().getAlarmRule()); + assertEquals("ALARM", state("saved")); + } + + @Test + void quotasAndAtomicCreatePreventExcessReferencesAndForeignNameReplacement() { + String many = IntStream.range(0, 101).mapToObj(i -> "ALARM(a" + i + ")") + .collect(Collectors.joining(" OR ")); + assertThrows(AwsException.class, () -> put("too-many", many)); + put("owned", "TRUE"); + CompositeAlarm foreign = new CompositeAlarm(); + foreign.setAlarmName("owned"); + foreign.setAlarmRule("FALSE"); + assertThrows(AwsException.class, () -> service.putCompositeAlarm(foreign, REGION, false)); + assertEquals("ALARM", state("owned")); + assertThrows(AwsException.class, () -> put("deep", "(".repeat(250) + "TRUE" + ")".repeat(250))); + assertEquals("ALARM", put("many-not", "NOT ".repeat(502) + "TRUE").getStateValue()); + assertThrows(AwsException.class, () -> put("elements", "FALSE OR ".repeat(500) + "TRUE")); + } + + @Test + void mixedPersistenceLoadsLegacyMetricsAndRoundTripsComposites() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + TypeReference> type = new TypeReference<>() {}; + Map legacy = mapper.readValue( + "{\"legacy\":{\"alarmName\":\"old\",\"metricName\":\"CPU\",\"stateValue\":\"OK\"}}", type); + assertInstanceOf(MetricAlarm.class, legacy.get("legacy")); + CompositeAlarm alarm = put("new", "FALSE"); + Map restored = mapper.readValue(mapper.writeValueAsBytes(Map.of("new", alarm)), type); + assertEquals("FALSE", assertInstanceOf(CompositeAlarm.class, restored.get("new")).getAlarmRule()); + assertEquals("OK", restored.get("new").getStateValue()); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void concurrentTagChangesPreserveTheUpdatedDefinition(boolean remove) throws Exception { + CountDownLatch updating = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + InMemoryStorage storage = new InMemoryStorage<>() { + @Override + public void put(String key, Alarm value) { + if (value instanceof CompositeAlarm composite && "TRUE".equals(composite.getAlarmRule())) { + updating.countDown(); + try { + assertTrue(release.await(5, TimeUnit.SECONDS)); + } catch (InterruptedException error) { + Thread.currentThread().interrupt(); + throw new AssertionError(error); + } + } + super.put(key, value); + } + }; + CloudWatchMetricsService concurrent = new CloudWatchMetricsService(new InMemoryStorage<>(), storage, + new RegionResolver(REGION, "123456789012")); + CompositeAlarm original = new CompositeAlarm(); + original.setAlarmName("concurrent"); + original.setAlarmRule("FALSE"); + original.setTags(new HashMap<>(Map.of("team", "old"))); + concurrent.putCompositeAlarm(original, REGION); + CompositeAlarm replacement = new CompositeAlarm(); + replacement.setAlarmName("concurrent"); + replacement.setAlarmRule("TRUE"); + AtomicReference tagThread = new AtomicReference<>(); + try (ExecutorService executor = Executors.newFixedThreadPool(2)) { + Future update = executor.submit(() -> concurrent.putCompositeAlarm(replacement, REGION)); + Future tags; + try { + assertTrue(updating.await(2, TimeUnit.SECONDS)); + tags = executor.submit(() -> { + tagThread.set(Thread.currentThread()); + if (remove) { + concurrent.untagResource(original.getAlarmArn(), List.of("team"), REGION); + } else { + concurrent.tagResource(original.getAlarmArn(), Map.of("team", "new"), REGION); + } + }); + ThreadStates.awaitBlockedOrDone(tagThread, tags); + assertFalse(tags.isDone(), "tag mutation must share the definition update lock"); + } finally { + release.countDown(); + } + update.get(2, TimeUnit.SECONDS); + tags.get(2, TimeUnit.SECONDS); + } + CompositeAlarm result = concurrent.describeCompositeAlarms(List.of("concurrent"), null, REGION).getFirst(); + assertEquals("TRUE", result.getAlarmRule()); + assertEquals(remove ? Map.of() : Map.of("team", "new"), result.getTags()); + } + + private CompositeAlarm put(String name, String rule) { + CompositeAlarm alarm = new CompositeAlarm(); + alarm.setAlarmName(name); + alarm.setAlarmRule(rule); + alarm.setActionsEnabled(true); + service.putCompositeAlarm(alarm, REGION); + return alarm; + } + + private String state(String name) { + return service.describeCompositeAlarms(List.of(name), null, REGION).getFirst().getStateValue(); + } +} diff --git a/src/test/resources/cloudformation/supported-resource-types.tsv b/src/test/resources/cloudformation/supported-resource-types.tsv index 48280ea..6d47e3f 100644 --- a/src/test/resources/cloudformation/supported-resource-types.tsv +++ b/src/test/resources/cloudformation/supported-resource-types.tsv @@ -41,6 +41,7 @@ AWS::CloudFront::OriginRequestPolicy CloudFrontCfnProvisioner AWS::CloudFront::ResponseHeadersPolicy CloudFrontCfnProvisioner AWS::CloudTrail::Trail CloudTrailCfnProvisioner AWS::CloudWatch::Alarm CloudWatchCfnProvisioner +AWS::CloudWatch::CompositeAlarm CloudWatchCompositeAlarmCfnProvisioner AWS::CloudWatch::Dashboard CloudWatchDashboardCfnProvisioner AWS::CodeBuild::Project CodeBuildCfnProvisioner AWS::CodePipeline::CustomActionType CodePipelineCfnProvisioner diff --git a/tools/docs/cfn_resource_types.yaml b/tools/docs/cfn_resource_types.yaml index ba37eef..1055d74 100644 --- a/tools/docs/cfn_resource_types.yaml +++ b/tools/docs/cfn_resource_types.yaml @@ -169,6 +169,9 @@ notes: # Trailing prose appended to a row after its type list, for behaviour that belongs to the service # rather than to one type. row_notes: + AWS::CloudWatch: >- + Composite alarms have backing definitions with name Ref and Arn GetAtt, rollback, and + replacement cleanup. Composite actions are stored only; suppression properties are rejected. AWS::Lambda: >- Inline `ZipFile` packages include the `cfn-response` (Node.js) / `cfnresponse` (Python) module AWS injects for that code path, so Solutions-style custom-resource handlers work. -- 2.52.0 From a9df81a2945b116dea42a3a632ed4f1b98205034 Mon Sep 17 00:00:00 2001 From: Zakaria Date: Sat, 24 Oct 2026 22:23:29 -0400 Subject: [PATCH 9/9] chore: add the upstream helpers the composite alarm change builds on Composite alarms from upstream (#6408) use SmithyValidationErrors for AWS's validation messages, and their tests use the CfnStackWaits, PartitionCleanup and ThreadStates test helpers and ReplacementCleanup.owedPhysicalIds. These are copied from upstream as they were at that commit; none brings in other behaviour. --- .../core/common/SmithyValidationErrors.java | 97 +++++++++ .../provisioners/ReplacementCleanup.java | 18 ++ .../floci/testing/CfnStackWaits.java | 199 ++++++++++++++++++ .../floci/testing/PartitionCleanup.java | 47 +++++ .../floci/testing/ThreadStates.java | 93 ++++++++ 5 files changed, 454 insertions(+) create mode 100644 src/main/java/io/github/hectorvent/floci/core/common/SmithyValidationErrors.java create mode 100644 src/test/java/io/github/hectorvent/floci/testing/CfnStackWaits.java create mode 100644 src/test/java/io/github/hectorvent/floci/testing/PartitionCleanup.java create mode 100644 src/test/java/io/github/hectorvent/floci/testing/ThreadStates.java diff --git a/src/main/java/io/github/hectorvent/floci/core/common/SmithyValidationErrors.java b/src/main/java/io/github/hectorvent/floci/core/common/SmithyValidationErrors.java new file mode 100644 index 0000000..bb33c6c --- /dev/null +++ b/src/main/java/io/github/hectorvent/floci/core/common/SmithyValidationErrors.java @@ -0,0 +1,97 @@ +package io.github.hectorvent.floci.core.common; + +import java.util.List; + +/** + * The message AWS's Smithy validation layer answers when a request breaks its model's constraints: + * "1 validation error detected: a" or "N validation errors detected: a; b", each violation reading + * "Value at 'path' failed to satisfy constraint: ...". The wording is Smithy's, shared by every + * service built on it; only the error code differs by protocol. + */ +public final class SmithyValidationErrors { + + private SmithyValidationErrors() {} + + /** + * The message for one or more violations. An empty list is the caller's bug, so it fails rather + * than answering "0 validation errors"; check {@code isEmpty()} before throwing. + */ + public static String message(List violations) { + if (violations.isEmpty()) { + throw new IllegalArgumentException("no violations to report"); + } + String count = violations.size() == 1 + ? "1 validation error detected: " + : violations.size() + " validation errors detected: "; + return count + String.join("; ", violations); + } + + /** The violation for a required member the request leaves out. */ + public static String notNull(String path) { + return "Value null at '" + path + "' failed to satisfy constraint: Member must not be null"; + } + + /** The 400 error carrying {@link #message} under the protocol's validation error code. */ + public static AwsException exception(String errorCode, List violations) { + return new AwsException(errorCode, message(violations), 400); + } + + public static AwsException exception(String errorCode, String violation) { + return exception(errorCode, List.of(violation)); + } + + /** The code most JSON services use, on both the awsJson and the restJson protocols. */ + public static AwsException validationException(List violations) { + return exception("ValidationException", violations); + } + + public static AwsException validationException(String violation) { + return validationException(List.of(violation)); + } + + /** The awsQuery protocol's code. */ + public static AwsException validationError(List violations) { + return exception("ValidationError", violations); + } + + public static AwsException validationError(String violation) { + return validationError(List.of(violation)); + } + + /** The code of REST JSON services that model their validation failure as BadRequestException. */ + public static AwsException badRequest(List violations) { + return exception("BadRequestException", violations); + } + + public static AwsException badRequest(String violation) { + return badRequest(List.of(violation)); + } + + /** The violation for a number outside {@code min} to {@code max}, or {@code null} for one inside. */ + public static String outOfRange(String path, long value, long min, long max) { + if (value < min) { + return "Value '" + value + "' at '" + path + "' failed to satisfy constraint: " + + "Member must have value greater than or equal to " + min; + } + if (value > max) { + return "Value '" + value + "' at '" + path + "' failed to satisfy constraint: " + + "Member must have value less than or equal to " + max; + } + return null; + } + + /** The violation for a value outside its enum, listing the allowed values in the model's order. */ + public static String notInEnum(String value, String path, List allowed) { + return "Value '" + value + "' at '" + path + "' failed to satisfy constraint: " + + "Member must satisfy enum value set: " + allowed; + } + + /** + * The violation for a list holding a value outside its members' enum. The list is checked as a + * whole, so the whole list is the offending value and the enum constraint is nested. + */ + public static String notAllInEnum(List values, String path, List allowed) { + return "Value '" + values + "' at '" + path + "' failed to satisfy constraint: " + + "Member must satisfy constraint: [Member must satisfy enum value set: " + allowed + "]"; + } +} diff --git a/src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/ReplacementCleanup.java b/src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/ReplacementCleanup.java index 9ab3340..d9c1ff2 100644 --- a/src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/ReplacementCleanup.java +++ b/src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/ReplacementCleanup.java @@ -9,9 +9,11 @@ import io.github.hectorvent.floci.services.cloudformation.model.StackResource; import org.jboss.logging.Logger; import java.util.ArrayList; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; /** * The replacement lifecycle for provisioners whose replacing update simply creates the new entity @@ -121,6 +123,22 @@ public final class ReplacementCleanup { return cleanup != null && cleanup.path("displaced").size() > 0; } + /** The physical IDs of replaced resources this resource still has to delete. */ + static Set owedPhysicalIds(StackResource r) { + Set owed = new HashSet<>(); + ObjectNode cleanup = read(r); + if (cleanup == null) { + return owed; + } + for (JsonNode entry : cleanup.path("displaced")) { + String physicalId = entry.path("physicalId").asText(null); + if (physicalId != null) { + owed.add(physicalId); + } + } + return owed; + } + /** * The entity the cleanup is working on, or null when none is owed or {@code UpdateReplacePolicy: * Retain} keeps it. The engine announces one physical id per resource, so a resource owing more diff --git a/src/test/java/io/github/hectorvent/floci/testing/CfnStackWaits.java b/src/test/java/io/github/hectorvent/floci/testing/CfnStackWaits.java new file mode 100644 index 0000000..ff2bea8 --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/testing/CfnStackWaits.java @@ -0,0 +1,199 @@ +package io.github.hectorvent.floci.testing; + +import io.restassured.path.xml.XmlPath; +import io.restassured.path.xml.element.Node; +import io.restassured.response.Response; +import io.restassured.specification.RequestSpecification; + +import java.util.List; +import java.util.Set; + +import static io.restassured.RestAssured.given; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * DeleteStack answers before the stack is gone. The deletion runs on an executor, so a test + * that looks at the stack's resources right after the answer races it. + */ +public final class CfnStackWaits { + + private CfnStackWaits() { + } + + /** What DescribeStacks reported: the status and, when present, the StackStatusReason. */ + public record StackState(String status, String reason) { + } + + private static final Set TERMINAL = Set.of( + "CREATE_COMPLETE", "CREATE_FAILED", "ROLLBACK_COMPLETE", "ROLLBACK_FAILED", + "UPDATE_COMPLETE", "UPDATE_FAILED", "UPDATE_ROLLBACK_COMPLETE", "UPDATE_ROLLBACK_FAILED", + "DELETE_COMPLETE", "DELETE_FAILED"); + + private static final Set OPERATION_STARTS = Set.of("CREATE_IN_PROGRESS", "UPDATE_IN_PROGRESS"); + + /** + * Waits until DescribeStacks reports a terminal status for a create or update and returns + * it, so the caller decides what counts as success. A DescribeStacks that is not 200 (the + * stack does not exist) fails at once with the error body rather than polling the clock out; + * ten seconds without a terminal status fails too. + */ + public static StackState awaitTerminal(String stackName) { + return awaitTerminal(stackName, null); + } + + public static StackState awaitTerminal(String stackName, String authorization) { + long deadline = System.currentTimeMillis() + 10_000; + StackState last = new StackState(null, ""); + while (System.currentTimeMillis() < deadline) { + RequestSpecification request = given() + .contentType("application/x-www-form-urlencoded") + .formParam("Action", "DescribeStacks") + .formParam("StackName", stackName); + if (authorization != null) { + request.header("Authorization", authorization); + } + Response response = request.when().post("/"); + if (response.statusCode() != 200) { + fail("DescribeStacks " + stackName + " answered " + response.statusCode() + ": " + response.asString()); + } + XmlPath xml = response.xmlPath(); + String status = xml.getString("**.find { it.name() == 'StackStatus' }"); + String reason = xml.getString("**.find { it.name() == 'StackStatusReason' }"); + last = new StackState(status == null || status.isEmpty() ? null : status, reason == null ? "" : reason); + if (last.status() != null && TERMINAL.contains(last.status())) { + return last; + } + try { + Thread.sleep(50); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError("Interrupted while waiting for stack " + stackName, e); + } + } + fail("Stack " + stackName + " reached no terminal status within ten seconds, last: " + last); + return last; + } + + /** + * Waits until DescribeStacks reports {@code expectedStatus} and returns that state. Any other + * terminal status fails at once with its reason, as do a DescribeStacks that is not 200 and ten + * seconds without the expected status. + */ + public static StackState awaitStatus(String stackName, String expectedStatus) { + long deadline = System.currentTimeMillis() + 10_000; + StackState last = new StackState(null, ""); + while (System.currentTimeMillis() < deadline) { + Response response = given() + .contentType("application/x-www-form-urlencoded") + .formParam("Action", "DescribeStacks") + .formParam("StackName", stackName) + .when().post("/"); + if (response.statusCode() != 200) { + fail("DescribeStacks " + stackName + " answered " + response.statusCode() + ": " + response.asString()); + } + XmlPath xml = response.xmlPath(); + String status = xml.getString("**.find { it.name() == 'StackStatus' }"); + String reason = xml.getString("**.find { it.name() == 'StackStatusReason' }"); + last = new StackState(status == null || status.isEmpty() ? null : status, reason == null ? "" : reason); + if (expectedStatus.equals(last.status())) { + return last; + } + if (last.status() != null && TERMINAL.contains(last.status())) { + fail("Stack " + stackName + " reached " + last.status() + " instead of " + expectedStatus + + ": " + last.reason()); + } + try { + Thread.sleep(50); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError("Interrupted while waiting for stack " + stackName, e); + } + } + fail("Stack " + stackName + " did not reach " + expectedStatus + " within ten seconds, last: " + last); + return last; + } + + /** + * The reason on the first resource event that failed a create or update in the stack's latest + * operation, the one its last stack-level CREATE_IN_PROGRESS or UPDATE_IN_PROGRESS began. The + * stack's own reason only names the failed resource, as on AWS, so the failure's message is read + * here. Fails when DescribeStackEvents is not 200 or no resource failed in that operation. + */ + public static String failedResourceReason(String stackName) { + return failedResourceReason(stackName, null); + } + + public static String failedResourceReason(String stackName, String authorization) { + RequestSpecification request = given() + .contentType("application/x-www-form-urlencoded") + .formParam("Action", "DescribeStackEvents") + .formParam("StackName", stackName); + if (authorization != null) { + request.header("Authorization", authorization); + } + Response response = request.when().post("/"); + if (response.statusCode() != 200) { + fail("DescribeStackEvents " + stackName + " answered " + response.statusCode() + ": " + response.asString()); + } + List newestFirst = response.xmlPath().getList("**.findAll { it.name() == 'member' }", Node.class); + String reason = null; + for (Node event : newestFirst) { + String status = text(event, "ResourceStatus"); + if (text(event, "StackName").equals(text(event, "LogicalResourceId")) + && text(event, "StackId").equals(text(event, "PhysicalResourceId"))) { + if (OPERATION_STARTS.contains(status)) { + break; + } + } else if ("CREATE_FAILED".equals(status) || "UPDATE_FAILED".equals(status)) { + reason = text(event, "ResourceStatusReason"); + } + } + if (reason == null) { + fail("Stack " + stackName + " has no failed resource event in its latest operation: " + response.asString()); + } + return reason; + } + + private static String text(Node event, String name) { + Node child = event.getNode(name); + return child == null ? "" : child.value(); + } + + /** + * Waits until DescribeStacks answers 400, which is how a missing stack is reported. Fails on + * DELETE_FAILED or after ten seconds. + */ + public static void awaitStackDeleted(String stackName) { + awaitStackDeleted(stackName, null); + } + + public static void awaitStackDeleted(String stackName, String authorization) { + long deadline = System.currentTimeMillis() + 10_000; + String body = ""; + while (System.currentTimeMillis() < deadline) { + RequestSpecification request = given() + .contentType("application/x-www-form-urlencoded") + .formParam("Action", "DescribeStacks") + .formParam("StackName", stackName); + if (authorization != null) { + request.header("Authorization", authorization); + } + Response response = request.when().post("/"); + body = response.asString(); + assertThat(body, not(containsString("DELETE_FAILED"))); + if (response.statusCode() == 400) { + return; + } + try { + Thread.sleep(50); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError("Interrupted while waiting for stack " + stackName + " to be deleted", e); + } + } + fail("Stack " + stackName + " was not deleted within ten seconds: " + body); + } +} diff --git a/src/test/java/io/github/hectorvent/floci/testing/PartitionCleanup.java b/src/test/java/io/github/hectorvent/floci/testing/PartitionCleanup.java new file mode 100644 index 0000000..e6a169b --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/testing/PartitionCleanup.java @@ -0,0 +1,47 @@ +package io.github.hectorvent.floci.testing; + +import org.junit.jupiter.api.extension.AfterEachCallback; +import org.junit.jupiter.api.extension.ExtensionContext; + +import java.util.ArrayDeque; +import java.util.Deque; + +/** + * Deletes what a test created, in reverse order, after each test. The {@code @QuarkusTest} + * emulator is shared by every test class, so a resource left behind outlives the class that made + * it: one under {@code cn-north-1} shows up in every later cross-region listing (ResourceExplorer + * scans all resources), and an IAM user or an S3 bucket shows up in the next class to list either + * one unscoped. A test that leaves such a resource registers each create here. + * + * 

{@code
+ * @RegisterExtension
+ * final PartitionCleanup cleanup = new PartitionCleanup();
+ * ...
+ * createTable(region, name);
+ * cleanup.register(() -> deleteTable(region, name));
+ * }
+ * + * A failing cleanup never fails the test: it is reported on stderr so the leak is visible and + * the assertion that actually failed stays the headline. + */ +public final class PartitionCleanup implements AfterEachCallback { + + private final Deque actions = new ArrayDeque<>(); + + public void register(Runnable action) { + actions.push(action); + } + + @Override + public void afterEach(ExtensionContext context) { + while (!actions.isEmpty()) { + Runnable action = actions.pop(); + try { + action.run(); + } catch (RuntimeException | AssertionError e) { + System.err.println("PartitionCleanup: cleanup step failed after " + + context.getDisplayName() + ": " + e); + } + } + } +} diff --git a/src/test/java/io/github/hectorvent/floci/testing/ThreadStates.java b/src/test/java/io/github/hectorvent/floci/testing/ThreadStates.java new file mode 100644 index 0000000..1acdf9e --- /dev/null +++ b/src/test/java/io/github/hectorvent/floci/testing/ThreadStates.java @@ -0,0 +1,93 @@ +package io.github.hectorvent.floci.testing; + +import java.time.Duration; +import java.util.Collections; +import java.util.EnumSet; +import java.util.Set; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.LockSupport; +import java.util.function.BooleanSupplier; +import java.util.function.Supplier; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** Waiting for a thread to reach a lock, for tests that force two requests to overlap. */ +public final class ThreadStates { + + /** Every state a thread is in once it has stopped running: parked in any way, or ended. */ + public static final Set PARKED_OR_DONE = Collections.unmodifiableSet(EnumSet.of( + Thread.State.BLOCKED, Thread.State.WAITING, Thread.State.TIMED_WAITING, Thread.State.TERMINATED)); + + private ThreadStates() {} + + /** + * Returns once {@code thread} is blocked entering a monitor or parked on a lock, or {@code task} has + * finished, whichever comes first. Code that serializes correctly leaves the second request blocked + * behind the first; code that does not lets it run to the end, which the caller's assertions + * then catch. Polls every 5 ms and gives up after five seconds rather than hang a broken test. + */ + public static void awaitBlockedOrDone(AtomicReference thread, Future task) + throws InterruptedException { + awaitBlockedOrDone(thread::get, task::isDone, 5); + } + + /** + * Returns once {@code thread} is blocked entering a monitor, parked on a lock, or is no longer alive. + * Same wait as the {@link Future} overload, for a test that owns the {@link Thread} directly; gives up + * after ten seconds and fails, since a thread still running unblocked by then is not overlapping the + * way the test needs. + */ + public static void awaitBlockedOrDone(Thread thread) throws InterruptedException { + assertTrue(awaitBlockedOrDone(() -> thread, () -> !thread.isAlive(), 10), + "worker blocked or finished before timeout"); + } + + private static boolean awaitBlockedOrDone(Supplier thread, BooleanSupplier done, long timeoutSeconds) + throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(timeoutSeconds); + while (System.nanoTime() < deadline && !done.getAsBoolean()) { + if (isBlocked(thread.get())) { + return true; + } + Thread.sleep(5); + } + return done.getAsBoolean() || isBlocked(thread.get()); + } + + private static boolean isBlocked(Thread thread) { + if (thread == null) { + return false; + } + Thread.State state = thread.getState(); + return state == Thread.State.BLOCKED || state == Thread.State.WAITING; + } + + /** + * Waits until {@code thread} is parked in any way (blocked on a monitor, waiting or timed + * waiting) or has ended, and returns the state it reached. Fails after ten seconds. + */ + public static Thread.State awaitParkedOrDone(Thread thread) { + return awaitState(thread, PARKED_OR_DONE, Duration.ofSeconds(10), "the thread"); + } + + /** + * Waits until {@code thread} is in one of {@code states} and returns the one it reached, checking + * every millisecond. Fails naming {@code what} and the state it was left in once {@code timeout} + * passes, so a thread that never gets there fails the test instead of hanging it. + */ + public static Thread.State awaitState(Thread thread, Set states, Duration timeout, String what) { + long deadline = System.nanoTime() + timeout.toNanos(); + Thread.State state = thread.getState(); + while (!states.contains(state)) { + if (System.nanoTime() - deadline > 0) { + fail(what + " never reached " + states + ", it was " + state); + } + LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(1)); + state = thread.getState(); + } + return state; + } +} -- 2.52.0