Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/aws_cloudwatch_metric_alarm: Add period to metric_query block #29896

Merged
merged 6 commits into from
Mar 15, 2023
Merged

resource/aws_cloudwatch_metric_alarm: Add period to metric_query block #29896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4fa97ee
Cleanup
gdavison Mar 9, 2023
043a36f
Fixes config names
gdavison Mar 9, 2023
e920503
Cleanup
gdavison Mar 9, 2023
b41484e
Checks for actual values
gdavison Mar 9, 2023
3cce2a1
Adds `period` to `metric_query` and adds validation to `metric_query.…
gdavison Mar 10, 2023
a03bb8c
Adds CHANGELOG entry
gdavison Mar 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changelog/29896.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:enhancement
resource/aws_cloudwatch_metric_alarm: Add `period` parameter to `metric_query`
```

```release-note:enhancement
resource/aws_cloudwatch_metric_alarm: Add validation to `period` parameter of `metric_query.metric`
```
18 changes: 18 additions & 0 deletions internal/service/cloudwatch/metric_alarm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ func ResourceMetricAlarm() *schema.Resource {
"period": {
Type: schema.TypeInt,
Required: true,
ValidateFunc: validation.Any(
validation.IntInSlice([]int{1, 5, 10, 30}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation in https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_PutMetricAlarm.html suggests

Valid values are 10, 30, and any multiple of 60.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @henkka,the following line

validation.IntDivisibleBy(60)

covers the multiples of 60 case. It's good to have extra eyes review things, though 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, my point was that this should actually be validation.IntInSlice([]int{10, 30}) if we'd like to comply with what AWS allows currently

Copy link
Contributor Author

@gdavison gdavison Mar 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see what you're saying. The top-level period, at

terraform-provider-aws/internal/service/cloudwatch/metric_alarm.go

Lines 170 to 178 in 9bccc30

"period": {
Type: schema.TypeInt,
Optional: true,
ConflictsWith: []string{"metric_query"},
ValidateFunc: validation.Any(
validation.IntInSlice([]int{10, 30}),
validation.IntDivisibleBy(60),
),
},
, has valid values of 10, 30, and multiples of 60. The period of a metric_query, on the other hand, can be 1, 5, 10, 30, and multiples of 60 (https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_MetricDataQuery.html)

validation.IntDivisibleBy(60),
),
},
"stat": {
Type: schema.TypeString,
Expand All @@ -138,6 +142,14 @@ func ResourceMetricAlarm() *schema.Resource {
Type: schema.TypeString,
Optional: true,
},
"period": {
Type: schema.TypeInt,
Optional: true,
ValidateFunc: validation.Any(
validation.IntInSlice([]int{1, 5, 10, 30}),
validation.IntDivisibleBy(60),
),
},
"return_data": {
Type: schema.TypeBool,
Optional: true,
Expand Down Expand Up @@ -611,6 +623,9 @@ func flattenMetricAlarmMetrics(metrics []*cloudwatch.MetricDataQuery) []map[stri
metric := flattenMetricAlarmMetricsMetricStat(mq.MetricStat)
metricQuery["metric"] = []interface{}{metric}
}
if mq.Period != nil {
metricQuery["period"] = aws.Int64Value(mq.Period)
}
metricQueries = append(metricQueries, metricQuery)
}

Expand Down Expand Up @@ -655,6 +670,9 @@ func expandMetricAlarmMetrics(v *schema.Set) []*cloudwatch.MetricDataQuery {
if v := metricQueryResource["metric"]; v != nil && len(v.([]interface{})) > 0 {
metricQuery.MetricStat = expandMetricAlarmMetricsMetric(v.([]interface{}))
}
if v, ok := metricQueryResource["period"]; ok && v.(int) != 0 {
metricQuery.Period = aws.Int64(int64(v.(int)))
}
if v, ok := metricQueryResource["account_id"]; ok && v.(string) != "" {
metricQuery.AccountId = aws.String(v.(string))
}
Expand Down
Loading