diff --git a/providers/aws/cloudwatch/cloudwatch.go b/providers/aws/cloudwatch/cloudwatch.go
index 79ac744a4..db2b671fd 100644
--- a/providers/aws/cloudwatch/cloudwatch.go
+++ b/providers/aws/cloudwatch/cloudwatch.go
@@ -178,6 +178,14 @@ func (m *Mock) PutMetricData(ctx context.Context, data []driver.MetricDatum) err
//
//nolint:gocritic // hugeParam: interface method signature cannot be changed.
func (m *Mock) GetMetricData(_ context.Context, input driver.GetMetricInput) (*driver.MetricDataResult, error) {
+ // Zero keeps meaning the one-minute default. Any other value follows the
+ // AWS Period rule, so it cannot overflow the bucket step.
+ if input.Period != 0 {
+ if err := m.ValidatePeriod("Period", input.Period); err != nil {
+ return nil, err
+ }
+ }
+
return m.readMetric(&input), nil
}
@@ -200,7 +208,7 @@ func (m *Mock) readMetric(input *driver.GetMetricInput) *driver.MetricDataResult
period := input.Period
if period <= 0 {
- period = 60
+ period = defaultPeriodSeconds
}
return buildMetricResult(filtered, input.StartTime, period, input.Stat)
@@ -272,20 +280,26 @@ func buildMetricResult(filtered []driver.MetricDatum, startTime time.Time, perio
// unit reads back as None, like on AWS.
result.Unit = alarmeval.EffectiveUnit(unitOf(filtered))
- periodDur := time.Duration(period) * time.Second
+ // Count in whole seconds, so a long period cannot overflow a Duration.
+ step := int64(period)
+ if step <= 0 {
+ return result
+ }
+
+ bucket := func(ts time.Time) int64 { return int64(ts.Sub(startTime)/time.Second) / step }
for from := 0; from < len(filtered); {
- idx := filtered[from].Timestamp.Sub(startTime) / periodDur
+ idx := bucket(filtered[from].Timestamp)
to := from + 1
- for to < len(filtered) && filtered[to].Timestamp.Sub(startTime)/periodDur == idx {
+ for to < len(filtered) && bucket(filtered[to].Timestamp) == idx {
to++
}
// A period where an extended statistic is not available, such as a
// percentile over negative values, is left out as on AWS.
if v, ok := alarmeval.StatValue(filtered[from:to], stat); ok {
- result.Timestamps = append(result.Timestamps, startTime.Add(idx*periodDur))
+ result.Timestamps = append(result.Timestamps, startTime.Add(time.Duration(idx*step)*time.Second))
result.Values = append(result.Values, v)
}
diff --git a/providers/aws/cloudwatch/period.go b/providers/aws/cloudwatch/period.go
new file mode 100644
index 000000000..93558517f
--- /dev/null
+++ b/providers/aws/cloudwatch/period.go
@@ -0,0 +1,52 @@
+package cloudwatch
+
+import (
+ "math"
+ "slices"
+
+ "github.com/stackshy/cloudemu/v2/errors"
+)
+
+// Period rules from the MetricStat, MetricDataQuery and GetMetricStatistics
+// API references. Period is an Integer of at least 1. A regular resolution
+// period is a multiple of 60, and a high-resolution metric also allows 1, 5,
+// 10, 20 and 30.
+const (
+ defaultPeriodSeconds = 60
+ maxPeriodSeconds = math.MaxInt32
+ periodSecondsInMinute = 60
+)
+
+// validPeriod reports whether p is 1, 5, 10, 20, 30 or a multiple of 60 that
+// fits the API's Integer type.
+func validPeriod(p int) bool {
+ switch {
+ case p < 1 || p > maxPeriodSeconds:
+ return false
+ case p%periodSecondsInMinute == 0:
+ return true
+ default:
+ periods := highResolutionPeriods()
+
+ return slices.Contains(periods[:], p)
+ }
+}
+
+// highResolutionPeriods are the periods under a minute that a high-resolution
+// metric allows.
+func highResolutionPeriods() [5]int {
+ return [5]int{1, 5, 10, 20, 30}
+}
+
+// ValidatePeriod returns an InvalidArgument error when period is not a valid
+// metric period. param names the request field in the message, such as
+// "MetricDataQueries.member.1.MetricStat.Period".
+func (*Mock) ValidatePeriod(param string, period int) error {
+ if validPeriod(period) {
+ return nil
+ }
+
+ return errors.Newf(errors.InvalidArgument,
+ "The value %d for parameter %s is not a valid period. Valid periods are 1, 5, 10, 20, 30, 60 and any multiple of 60.",
+ period, param)
+}
diff --git a/providers/aws/cloudwatch/period_test.go b/providers/aws/cloudwatch/period_test.go
new file mode 100644
index 000000000..345c050ac
--- /dev/null
+++ b/providers/aws/cloudwatch/period_test.go
@@ -0,0 +1,84 @@
+package cloudwatch
+
+import (
+ "context"
+ "strings"
+ "testing"
+ "time"
+
+ cerrors "github.com/stackshy/cloudemu/v2/errors"
+ "github.com/stackshy/cloudemu/v2/services/monitoring/driver"
+)
+
+func TestValidatePeriod(t *testing.T) {
+ m := newTestMock()
+
+ for _, p := range []int{1, 5, 10, 20, 30, 60, 120, 300, 3600, 86400, 2147483640} {
+ if err := m.ValidatePeriod("Period", p); err != nil {
+ t.Errorf("period %d: unexpected error %v", p, err)
+ }
+ }
+
+ for _, p := range []int{0, -1, -60, 2, 15, 45, 61, 90, 2147483700, 540431955284459520} {
+ err := m.ValidatePeriod("MetricDataQueries.member.1.MetricStat.Period", p)
+ if !cerrors.IsInvalidArgument(err) {
+ t.Fatalf("period %d: want InvalidArgument, got %v", p, err)
+ }
+
+ if !strings.Contains(cerrors.Message(err), "parameter MetricDataQueries.member.1.MetricStat.Period is not a valid period") {
+ t.Errorf("period %d: message should name the parameter: %s", p, cerrors.Message(err))
+ }
+ }
+}
+
+// TestGetMetricDataRejectsBadPeriod covers a Period that used to overflow the
+// bucket step to zero (divide by zero) or to a negative value.
+func TestGetMetricDataRejectsBadPeriod(t *testing.T) {
+ m := newTestMock()
+ ctx := context.Background()
+ now := m.opts.Clock.Now()
+
+ requireNoError(t, m.PutMetricData(ctx, []driver.MetricDatum{
+ {Namespace: "App", MetricName: "Latency", Value: 5, Timestamp: now.Add(-time.Minute)},
+ }))
+
+ in := driver.GetMetricInput{
+ Namespace: "App", MetricName: "Latency", StartTime: now.Add(-time.Hour), EndTime: now, Stat: "Sum",
+ }
+
+ for _, p := range []int{-60, 61, 540431955284459520, 9223372036854775800} {
+ in.Period = p
+
+ if _, err := m.GetMetricData(ctx, in); !cerrors.IsInvalidArgument(err) {
+ t.Fatalf("period %d: want InvalidArgument, got %v", p, err)
+ }
+ }
+
+ in.Period = 0
+
+ res, err := m.GetMetricData(ctx, in)
+ requireNoError(t, err)
+
+ if len(res.Values) != 1 || res.Values[0] != 5 {
+ t.Fatalf("zero period should default to a minute: %+v", res)
+ }
+}
+
+// TestBuildMetricResultLongPeriod checks that a period longer than a
+// Duration can hold still buckets every point into one period.
+func TestBuildMetricResultLongPeriod(t *testing.T) {
+ start := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
+ data := []driver.MetricDatum{
+ {Value: 1, Timestamp: start.Add(time.Hour)},
+ {Value: 2, Timestamp: start.Add(48 * time.Hour)},
+ }
+
+ res := buildMetricResult(data, start, 540431955284459520, "Sum")
+ if len(res.Values) != 1 || res.Values[0] != 3 || !res.Timestamps[0].Equal(start) {
+ t.Fatalf("want one bucket of 3 at start, got %+v", res)
+ }
+
+ if res := buildMetricResult(data, start, 0, "Sum"); len(res.Values) != 0 {
+ t.Fatalf("a zero step should return no points, got %+v", res)
+ }
+}
diff --git a/server/aws/cloudwatch/core_alarm_statistics.go b/server/aws/cloudwatch/core_alarm_statistics.go
index 59ff4dae0..007cc2275 100644
--- a/server/aws/cloudwatch/core_alarm_statistics.go
+++ b/server/aws/cloudwatch/core_alarm_statistics.go
@@ -67,7 +67,19 @@ func validateAlarmPeriods(cfg *mondriver.AlarmConfig) error {
return newWireError(errValidation, "DatapointsToAlarm must be less than or equal to EvaluationPeriods.")
}
- if len(cfg.Metrics) > 0 || cfg.Period == 0 {
+ // validateQueryShape checks the MetricStat periods of a metric math
+ // alarm. Each one is held to the same lookback cap.
+ if len(cfg.Metrics) > 0 {
+ for _, p := range alarmPeriods(cfg) {
+ if err := validateAlarmLookback(p, cfg.EvaluationPeriods); err != nil {
+ return err
+ }
+ }
+
+ return nil
+ }
+
+ if cfg.Period == 0 {
return nil
}
@@ -79,15 +91,20 @@ func validateAlarmPeriods(cfg *mondriver.AlarmConfig) error {
}
// validateAlarmLookback checks EvaluationPeriods * Period against the day or
-// week cap that applies to period p.
+// week cap that applies to period p. It divides instead of multiplying, so a
+// huge Period or EvaluationPeriods cannot overflow past the cap.
func validateAlarmLookback(p, evaluationPeriods int) error {
- lookback := p * max(evaluationPeriods, 1)
+ if p <= 0 {
+ return nil
+ }
+
+ n := max(evaluationPeriods, 1)
switch {
- case p < hourPeriod && lookback > maxAlarmLookbackDay:
+ case p < hourPeriod && n > maxAlarmLookbackDay/p:
return newWireError(errValidation, "Metrics cannot be checked across more than a day "+
"(EvaluationPeriods * Period must be <= 86400)")
- case p >= hourPeriod && lookback > maxAlarmLookbackWeek:
+ case p >= hourPeriod && n > maxAlarmLookbackWeek/p:
return newWireError(errValidation, "Metrics cannot be checked across more than a week "+
"(EvaluationPeriods * Period must be <= 604800)")
}
diff --git a/server/aws/cloudwatch/core_metric_data.go b/server/aws/cloudwatch/core_metric_data.go
index a1d8ed7f7..ae0cdc0f1 100644
--- a/server/aws/cloudwatch/core_metric_data.go
+++ b/server/aws/cloudwatch/core_metric_data.go
@@ -3,8 +3,10 @@ package cloudwatch
import (
"context"
"sort"
+ "strconv"
"time"
+ cerrors "github.com/stackshy/cloudemu/v2/errors"
mondriver "github.com/stackshy/cloudemu/v2/services/monitoring/driver"
"github.com/stackshy/cloudemu/v2/services/monitoring/metricmath"
)
@@ -57,6 +59,10 @@ func (h *Handler) getMetricDataCore(ctx context.Context, in *getMetricDataInput)
return getMetricDataResult{}, err
}
+ if err := h.validateQueryPeriods(in.MetricDataQueries); err != nil {
+ return getMetricDataResult{}, err
+ }
+
queries := toDriverQueries(in.MetricDataQueries)
eval := h.metricDataEvaluator(ctx, queries, timeOrZero(in.StartTime), timeOrZero(in.EndTime))
rows := make([]metricDataRow, 0, len(in.MetricDataQueries))
@@ -71,7 +77,7 @@ func (h *Handler) getMetricDataCore(ctx context.Context, in *getMetricDataInput)
qrows, err := metricDataRows(eval, q, descending)
if err != nil {
- return getMetricDataResult{}, err
+ return getMetricDataResult{}, invalidParameterError(err)
}
rows = append(rows, qrows...)
@@ -82,6 +88,49 @@ func (h *Handler) getMetricDataCore(ctx context.Context, in *getMetricDataInput)
return getMetricDataResult{Rows: page, NextToken: next}, nil
}
+// periodValidator is the AWS-local capability that checks a metric Period.
+// param names the request field in the error message.
+type periodValidator interface {
+ ValidatePeriod(param string, period int) error
+}
+
+// validateQueryPeriods checks the MetricStat Period and the optional Period
+// of every query, including ones that return no data.
+func (h *Handler) validateQueryPeriods(queries []metricDataQueryCBR) error {
+ v, ok := h.monitoring.(periodValidator)
+ if !ok {
+ return nil
+ }
+
+ for i := range queries {
+ member := "MetricDataQueries.member." + strconv.Itoa(i+1)
+
+ if ms := queries[i].MetricStat; ms != nil {
+ if err := v.ValidatePeriod(member+".MetricStat.Period", ms.Period); err != nil {
+ return invalidParameterError(err)
+ }
+ }
+
+ if p := queries[i].Period; p != 0 {
+ if err := v.ValidatePeriod(member+".Period", p); err != nil {
+ return invalidParameterError(err)
+ }
+ }
+ }
+
+ return nil
+}
+
+// invalidParameterError maps a driver InvalidArgument error to the
+// InvalidParameterValue wire error and passes any other error through.
+func invalidParameterError(err error) error {
+ if cerrors.IsInvalidArgument(err) {
+ return newWireError(errInvalidParameterValue, cerrors.Message(err))
+ }
+
+ return err
+}
+
// bandExclusionSource is the AWS-local capability that returns the excluded
// training ranges of the detector behind a band.
type bandExclusionSource interface {
diff --git a/server/aws/cloudwatch/core_metrics.go b/server/aws/cloudwatch/core_metrics.go
index 5c3d69227..6ac0ed6d2 100644
--- a/server/aws/cloudwatch/core_metrics.go
+++ b/server/aws/cloudwatch/core_metrics.go
@@ -306,6 +306,13 @@ func (h *Handler) getMetricStatisticsCore(
return getMetricStatisticsResult{}, err
}
+ // Period is required, so zero is out of range here.
+ if v, ok := h.monitoring.(periodValidator); ok {
+ if err := v.ValidatePeriod("Period", in.Period); err != nil {
+ return getMetricStatisticsResult{}, invalidParameterError(err)
+ }
+ }
+
// Callers often ask for several statistics at once and expect all of them
// on each datapoint.
stats := requestedStats(in)
@@ -335,7 +342,7 @@ func (h *Handler) getMetricStatisticsCore(
res, err := h.monitoring.GetMetricData(ctx, q)
if err != nil {
- return getMetricStatisticsResult{}, err
+ return getMetricStatisticsResult{}, invalidParameterError(err)
}
acc.add(res, stat, unit)
diff --git a/server/aws/cloudwatch/metric_period_validation_test.go b/server/aws/cloudwatch/metric_period_validation_test.go
new file mode 100644
index 000000000..412ef9f11
--- /dev/null
+++ b/server/aws/cloudwatch/metric_period_validation_test.go
@@ -0,0 +1,193 @@
+package cloudwatch_test
+
+import (
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "testing"
+ "time"
+
+ "github.com/stackshy/cloudemu/v2/config"
+ cwprovider "github.com/stackshy/cloudemu/v2/providers/aws/cloudwatch"
+ cwserver "github.com/stackshy/cloudemu/v2/server/aws/cloudwatch"
+)
+
+// periodRequestTimeout bounds each request, so a Period that sends the server
+// into an endless loop fails the test instead of hanging it.
+const periodRequestTimeout = 10 * time.Second
+
+// newPeriodPoster is newQueryPoster with a client timeout. It stores one
+// datum of App/Latency so GetMetricData has points to bucket.
+func newPeriodPoster(t *testing.T) func(url.Values) (int, string) {
+ t.Helper()
+
+ ts := httptest.NewServer(cwserver.New(cwprovider.New(config.NewOptions())))
+ t.Cleanup(ts.Close)
+
+ client := &http.Client{Timeout: periodRequestTimeout}
+
+ post := func(form url.Values) (int, string) {
+ req, _ := http.NewRequest(http.MethodPost, ts.URL+"/", strings.NewReader(form.Encode()))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ req.Header.Set("Authorization", monitoringAuth)
+
+ resp, err := client.Do(req)
+ if err != nil {
+ t.Fatalf("post %s: %v", form.Get("Action"), err)
+ }
+ defer resp.Body.Close()
+
+ b, _ := io.ReadAll(resp.Body)
+
+ return resp.StatusCode, string(b)
+ }
+
+ if code, body := post(url.Values{
+ "Action": {"PutMetricData"}, "Namespace": {"App"},
+ "MetricData.member.1.MetricName": {"Latency"}, "MetricData.member.1.Value": {"5"},
+ "MetricData.member.1.Timestamp": {time.Now().Add(-10 * time.Minute).UTC().Format(time.RFC3339)},
+ }); code != http.StatusOK {
+ t.Fatalf("PutMetricData: code=%d body=%s", code, body)
+ }
+
+ return post
+}
+
+func periodMetricDataForm(statPeriod, exprPeriod string) url.Values {
+ form := url.Values{
+ "Action": {"GetMetricData"},
+ "StartTime": {time.Now().Add(-time.Hour).UTC().Format(time.RFC3339)},
+ "EndTime": {time.Now().UTC().Format(time.RFC3339)},
+ "MetricDataQueries.member.1.Id": {"m1"},
+ "MetricDataQueries.member.1.MetricStat.Metric.Namespace": {"App"},
+ "MetricDataQueries.member.1.MetricStat.Metric.MetricName": {"Latency"},
+ "MetricDataQueries.member.1.MetricStat.Period": {statPeriod},
+ "MetricDataQueries.member.1.MetricStat.Stat": {"Sum"},
+ }
+
+ if exprPeriod != "" {
+ form.Set("MetricDataQueries.member.2.Id", "e1")
+ form.Set("MetricDataQueries.member.2.Expression", "m1 * 2")
+ form.Set("MetricDataQueries.member.2.Period", exprPeriod)
+ }
+
+ return form
+}
+
+// TestGetMetricDataPeriodValidation checks the Period rule of MetricStat and
+// MetricDataQuery: at least 1, and 1, 5, 10, 20, 30, 60 or a multiple of 60.
+// A huge Period used to overflow the bucket step and crash or hang the server.
+func TestGetMetricDataPeriodValidation(t *testing.T) {
+ post := newPeriodPoster(t)
+
+ bad := []struct {
+ name, statPeriod, exprPeriod, param string
+ }{
+ {"huge multiple of 60 overflowing to a zero step", "540431955284459520", "", "MetricDataQueries.member.1.MetricStat.Period"},
+ {"huge near int64 max", "9223372036854775800", "", "MetricDataQueries.member.1.MetricStat.Period"},
+ {"above int32", "2147483700", "", "MetricDataQueries.member.1.MetricStat.Period"},
+ {"zero", "0", "", "MetricDataQueries.member.1.MetricStat.Period"},
+ {"negative", "-60", "", "MetricDataQueries.member.1.MetricStat.Period"},
+ {"not a multiple of 60", "61", "", "MetricDataQueries.member.1.MetricStat.Period"},
+ {"high resolution value not allowed", "15", "", "MetricDataQueries.member.1.MetricStat.Period"},
+ {"negative expression period", "60", "-60", "MetricDataQueries.member.2.Period"},
+ {"huge expression period", "60", "540431955284459520", "MetricDataQueries.member.2.Period"},
+ }
+
+ for _, tc := range bad {
+ t.Run(tc.name, func(t *testing.T) {
+ code, body := post(periodMetricDataForm(tc.statPeriod, tc.exprPeriod))
+ if code != http.StatusBadRequest || !strings.Contains(body, "InvalidParameterValue") {
+ t.Fatalf("want 400 InvalidParameterValue, got code=%d body=%s", code, body)
+ }
+
+ if !strings.Contains(body, "parameter "+tc.param+" is not a valid period") {
+ t.Fatalf("message should name %s: %s", tc.param, body)
+ }
+ })
+ }
+
+ for _, p := range []string{"1", "5", "10", "20", "30", "60", "300", "86400"} {
+ t.Run("valid "+p, func(t *testing.T) {
+ code, body := post(periodMetricDataForm(p, p))
+ if code != http.StatusOK {
+ t.Fatalf("period %s: code=%d body=%s", p, code, body)
+ }
+
+ if !strings.Contains(body, "m1") || !strings.Contains(body, "5") {
+ t.Fatalf("period %s: want the stored datum back: %s", p, body)
+ }
+ })
+ }
+}
+
+// TestGetMetricStatisticsPeriodValidation applies the same Period rule to
+// GetMetricStatistics, where Period is required.
+func TestGetMetricStatisticsPeriodValidation(t *testing.T) {
+ post := newPeriodPoster(t)
+
+ form := func(period string) url.Values {
+ return url.Values{
+ "Action": {"GetMetricStatistics"}, "Namespace": {"App"}, "MetricName": {"Latency"},
+ "StartTime": {time.Now().Add(-time.Hour).UTC().Format(time.RFC3339)},
+ "EndTime": {time.Now().UTC().Format(time.RFC3339)},
+ "Period": {period},
+ "Statistics.member.1": {"Sum"},
+ }
+ }
+
+ for _, p := range []string{"0", "-60", "61", "540431955284459520"} {
+ t.Run("invalid "+p, func(t *testing.T) {
+ code, body := post(form(p))
+ if code != http.StatusBadRequest || !strings.Contains(body, "InvalidParameterValue") ||
+ !strings.Contains(body, "parameter Period is not a valid period") {
+ t.Fatalf("want 400 InvalidParameterValue, got code=%d body=%s", code, body)
+ }
+ })
+ }
+
+ code, body := post(form("300"))
+ if code != http.StatusOK || !strings.Contains(body, "5") {
+ t.Fatalf("valid period: code=%d body=%s", code, body)
+ }
+}
+
+// TestPutMetricAlarmMathPeriodLookback checks that a metric math alarm's
+// MetricStat Period obeys the EvaluationPeriods * Period cap, so a huge
+// Period cannot reach the evaluator.
+func TestPutMetricAlarmMathPeriodLookback(t *testing.T) {
+ post := newPeriodPoster(t)
+
+ form := func(period, evalPeriods string) url.Values {
+ return url.Values{
+ "Action": {"PutMetricAlarm"}, "AlarmName": {"math"}, "ComparisonOperator": {"GreaterThanThreshold"},
+ "EvaluationPeriods": {evalPeriods}, "Threshold": {"1"},
+ "Metrics.member.1.Id": {"m1"},
+ "Metrics.member.1.ReturnData": {"false"},
+ "Metrics.member.1.MetricStat.Metric.Namespace": {"App"},
+ "Metrics.member.1.MetricStat.Metric.MetricName": {"Latency"},
+ "Metrics.member.1.MetricStat.Period": {period},
+ "Metrics.member.1.MetricStat.Stat": {"Sum"},
+ "Metrics.member.2.Id": {"e1"},
+ "Metrics.member.2.Expression": {"m1 * 2"},
+ "Metrics.member.2.ReturnData": {"true"},
+ }
+ }
+
+ for _, tc := range []struct{ period, evalPeriods string }{
+ {"540431955284459520", "1"},
+ {"60", "1537228672809129302"},
+ {"3600", "169"},
+ } {
+ code, body := post(form(tc.period, tc.evalPeriods))
+ if code != http.StatusBadRequest || !strings.Contains(body, "ValidationError") {
+ t.Fatalf("period %s x %s: want 400 ValidationError, got code=%d body=%s", tc.period, tc.evalPeriods, code, body)
+ }
+ }
+
+ if code, body := post(form("3600", "168")); code != http.StatusOK {
+ t.Fatalf("a week of hourly periods: code=%d body=%s", code, body)
+ }
+}