From 7a35d0569d3919e777e6641a451dc79f4b84454f Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Wed, 19 Sep 2018 15:16:35 -0700 Subject: [PATCH 1/3] Issue #2384 aws_cloudwatch_log_metric_filter: default_value is automatically set to 0 --- aws/resource_aws_cloudwatch_log_metric_filter.go | 12 +++++------- ...esource_aws_cloudwatch_log_metric_filter_test.go | 9 +++++++++ aws/structure.go | 13 ++++++++----- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/aws/resource_aws_cloudwatch_log_metric_filter.go b/aws/resource_aws_cloudwatch_log_metric_filter.go index 7abaec78f36..4a335c90cf6 100644 --- a/aws/resource_aws_cloudwatch_log_metric_filter.go +++ b/aws/resource_aws_cloudwatch_log_metric_filter.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "math" "strings" "github.com/hashicorp/terraform/helper/resource" @@ -73,6 +74,7 @@ func resourceAwsCloudWatchLogMetricFilter() *schema.Resource { "default_value": { Type: schema.TypeFloat, Optional: true, + Default: math.NaN(), }, }, }, @@ -92,14 +94,10 @@ func resourceAwsCloudWatchLogMetricFilterUpdate(d *schema.ResourceData, meta int transformations := d.Get("metric_transformation").([]interface{}) o := transformations[0].(map[string]interface{}) - metricsTransformations, err := expandCloudWachLogMetricTransformations(o) - if err != nil { - return err - } - input.MetricTransformations = metricsTransformations + input.MetricTransformations = expandCloudWatchLogMetricTransformations(o) log.Printf("[DEBUG] Creating/Updating CloudWatch Log Metric Filter: %s", input) - _, err = conn.PutMetricFilter(&input) + _, err := conn.PutMetricFilter(&input) if err != nil { return fmt.Errorf("Creating/Updating CloudWatch Log Metric Filter failed: %s", err) } @@ -130,7 +128,7 @@ func resourceAwsCloudWatchLogMetricFilterRead(d *schema.ResourceData, meta inter d.Set("name", mf.FilterName) d.Set("pattern", mf.FilterPattern) - d.Set("metric_transformation", flattenCloudWachLogMetricTransformations(mf.MetricTransformations)) + d.Set("metric_transformation", flattenCloudWatchLogMetricTransformations(mf.MetricTransformations)) return nil } diff --git a/aws/resource_aws_cloudwatch_log_metric_filter_test.go b/aws/resource_aws_cloudwatch_log_metric_filter_test.go index 8a955648d81..5084472401b 100644 --- a/aws/resource_aws_cloudwatch_log_metric_filter_test.go +++ b/aws/resource_aws_cloudwatch_log_metric_filter_test.go @@ -56,6 +56,7 @@ func TestAccAWSCloudWatchLogMetricFilter_basic(t *testing.T) { MetricName: aws.String("AccessDeniedCount"), MetricNamespace: aws.String("MyNamespace"), MetricValue: aws.String("2"), + DefaultValue: aws.Float64(1), }), ), }, @@ -109,6 +110,14 @@ func testAccCheckCloudWatchLogMetricFilterTransformation(mf *cloudwatchlogs.Metr *expected.MetricValue, *given.MetricValue) } + if (given.DefaultValue != nil) != (expected.DefaultValue != nil) { + return fmt.Errorf("Expected default value to be present: %t, received: %t", + expected.DefaultValue != nil, given.DefaultValue != nil) + } else if (given.DefaultValue != nil) && *given.DefaultValue != *expected.DefaultValue { + return fmt.Errorf("Expected metric value: %g, received: %g", + *expected.DefaultValue, *given.DefaultValue) + } + return nil } } diff --git a/aws/structure.go b/aws/structure.go index 266da93e6b1..bf4109d994e 100644 --- a/aws/structure.go +++ b/aws/structure.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + "math" "reflect" "regexp" "sort" @@ -1777,21 +1778,21 @@ func expandApiGatewayStageKeyOperations(d *schema.ResourceData) []*apigateway.Pa return operations } -func expandCloudWachLogMetricTransformations(m map[string]interface{}) ([]*cloudwatchlogs.MetricTransformation, error) { +func expandCloudWatchLogMetricTransformations(m map[string]interface{}) []*cloudwatchlogs.MetricTransformation { transformation := cloudwatchlogs.MetricTransformation{ MetricName: aws.String(m["name"].(string)), MetricNamespace: aws.String(m["namespace"].(string)), MetricValue: aws.String(m["value"].(string)), } - if m["default_value"] != "" { + if !math.IsNaN(m["default_value"].(float64)) { transformation.DefaultValue = aws.Float64(m["default_value"].(float64)) } - return []*cloudwatchlogs.MetricTransformation{&transformation}, nil + return []*cloudwatchlogs.MetricTransformation{&transformation} } -func flattenCloudWachLogMetricTransformations(ts []*cloudwatchlogs.MetricTransformation) []interface{} { +func flattenCloudWatchLogMetricTransformations(ts []*cloudwatchlogs.MetricTransformation) []interface{} { mts := make([]interface{}, 0) m := make(map[string]interface{}, 0) @@ -1799,7 +1800,9 @@ func flattenCloudWachLogMetricTransformations(ts []*cloudwatchlogs.MetricTransfo m["namespace"] = *ts[0].MetricNamespace m["value"] = *ts[0].MetricValue - if ts[0].DefaultValue != nil { + if ts[0].DefaultValue == nil { + m["default_value"] = math.NaN() + } else { m["default_value"] = *ts[0].DefaultValue } From 8a416666a41967957d64228fd77d4fc9b95ddabf Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Sat, 27 Oct 2018 00:54:06 -0700 Subject: [PATCH 2/3] use string type rather than NaN sentinel value --- aws/resource_aws_cloudwatch_log_metric_filter.go | 7 +++---- aws/structure.go | 8 ++++---- aws/validators.go | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/aws/resource_aws_cloudwatch_log_metric_filter.go b/aws/resource_aws_cloudwatch_log_metric_filter.go index 4a335c90cf6..92a361266a7 100644 --- a/aws/resource_aws_cloudwatch_log_metric_filter.go +++ b/aws/resource_aws_cloudwatch_log_metric_filter.go @@ -3,7 +3,6 @@ package aws import ( "fmt" "log" - "math" "strings" "github.com/hashicorp/terraform/helper/resource" @@ -72,9 +71,9 @@ func resourceAwsCloudWatchLogMetricFilter() *schema.Resource { ValidateFunc: validation.StringLenBetween(0, 100), }, "default_value": { - Type: schema.TypeFloat, - Optional: true, - Default: math.NaN(), + Type: schema.TypeString, + Optional: true, + ValidateFunc: validateTypeStringNullableFloat, }, }, }, diff --git a/aws/structure.go b/aws/structure.go index bf4109d994e..0f433779527 100644 --- a/aws/structure.go +++ b/aws/structure.go @@ -4,7 +4,6 @@ import ( "bytes" "encoding/json" "fmt" - "math" "reflect" "regexp" "sort" @@ -1785,8 +1784,9 @@ func expandCloudWatchLogMetricTransformations(m map[string]interface{}) []*cloud MetricValue: aws.String(m["value"].(string)), } - if !math.IsNaN(m["default_value"].(float64)) { - transformation.DefaultValue = aws.Float64(m["default_value"].(float64)) + if m["default_value"].(string) != "" { + value, _ := strconv.ParseFloat(m["default_value"].(string), 64) + transformation.DefaultValue = aws.Float64(value) } return []*cloudwatchlogs.MetricTransformation{&transformation} @@ -1801,7 +1801,7 @@ func flattenCloudWatchLogMetricTransformations(ts []*cloudwatchlogs.MetricTransf m["value"] = *ts[0].MetricValue if ts[0].DefaultValue == nil { - m["default_value"] = math.NaN() + m["default_value"] = "" } else { m["default_value"] = *ts[0].DefaultValue } diff --git a/aws/validators.go b/aws/validators.go index e18e230de68..002c0bef715 100644 --- a/aws/validators.go +++ b/aws/validators.go @@ -54,6 +54,22 @@ func validateTypeStringNullableBoolean(v interface{}, k string) (ws []string, es return } +// validateTypeStringNullableFloat provides custom error messaging for TypeString floats +// Some arguments require a floating point value or an unspecified, empty field. +func validateTypeStringNullableFloat(v interface{}, k string) (ws []string, es []error) { + value, ok := v.(string) + if !ok { + es = append(es, fmt.Errorf("expected type of %s to be string", k)) + return + } + + if _, err := strconv.ParseFloat(value, 64); err != nil { + es = append(es, fmt.Errorf("%s: cannot parse '%s' as float: %s", k, value, err)) + } + + return +} + func validateRdsIdentifier(v interface{}, k string) (ws []string, errors []error) { value := v.(string) if !regexp.MustCompile(`^[0-9a-z-]+$`).MatchString(value) { From 69103e898432b4f46fd5e0f0662d97f4ed68a326 Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Sat, 10 Nov 2018 21:17:59 -0800 Subject: [PATCH 3/3] add validateTypeStringNullableFloat unit tests; fix error --- aws/validators.go | 4 ++++ aws/validators_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/aws/validators.go b/aws/validators.go index 002c0bef715..bc2c029da0d 100644 --- a/aws/validators.go +++ b/aws/validators.go @@ -63,6 +63,10 @@ func validateTypeStringNullableFloat(v interface{}, k string) (ws []string, es [ return } + if value == "" { + return + } + if _, err := strconv.ParseFloat(value, 64); err != nil { es = append(es, fmt.Errorf("%s: cannot parse '%s' as float: %s", k, value, err)) } diff --git a/aws/validators_test.go b/aws/validators_test.go index 0b39b5a5464..36cf96a4519 100644 --- a/aws/validators_test.go +++ b/aws/validators_test.go @@ -129,6 +129,57 @@ func TestValidateTypeStringNullableBoolean(t *testing.T) { } } +func TestValidateTypeStringNullableFloat(t *testing.T) { + testCases := []struct { + val interface{} + expectedErr *regexp.Regexp + }{ + { + val: "", + }, + { + val: "0", + }, + { + val: "1", + }, + { + val: "42.0", + }, + { + val: "threeve", + expectedErr: regexp.MustCompile(`cannot parse`), + }, + } + + matchErr := func(errs []error, r *regexp.Regexp) bool { + // err must match one provided + for _, err := range errs { + if r.MatchString(err.Error()) { + return true + } + } + + return false + } + + for i, tc := range testCases { + _, errs := validateTypeStringNullableFloat(tc.val, "test_property") + + if len(errs) == 0 && tc.expectedErr == nil { + continue + } + + if len(errs) != 0 && tc.expectedErr == nil { + t.Fatalf("expected test case %d to produce no errors, got %v", i, errs) + } + + if !matchErr(errs, tc.expectedErr) { + t.Fatalf("expected test case %d to produce error matching \"%s\", got %v", i, tc.expectedErr, errs) + } + } +} + func TestValidateEcrRepositoryName(t *testing.T) { validNames := []string{ "nginx-web-app",