fix(aws-cloudwatch): validate metric Period on GetMetricData and GetMetricStatistics

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)
 		}
 

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)
+}

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)
+	}
+}

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)")
 	}

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 {

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)