From 83aa83eaeabce38f375b6dc66fecdd32d3680fea Mon Sep 17 00:00:00 2001 From: Sharon Nam Date: Wed, 21 Jun 2023 10:34:01 -0700 Subject: [PATCH 1/3] Replace NormalizeJsonString with local helper func --- internal/service/events/rule.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/internal/service/events/rule.go b/internal/service/events/rule.go index 6a6c4370662e..df25ef6b686a 100644 --- a/internal/service/events/rule.go +++ b/internal/service/events/rule.go @@ -1,7 +1,9 @@ package events import ( + "bytes" "context" + "encoding/json" "fmt" "log" "time" @@ -65,7 +67,7 @@ func ResourceRule() *schema.Resource { ValidateFunc: validateEventPatternValue(), AtLeastOneOf: []string{"schedule_expression", "event_pattern"}, StateFunc: func(v interface{}) string { - json, _ := structure.NormalizeJsonString(v.(string)) + json, _ := jsonMarshal(v.(string)) return json }, }, @@ -109,6 +111,30 @@ func ResourceRule() *schema.Resource { } } +func jsonMarshal(jsonString interface{}) (string, error) { + var j interface{} + + if jsonString == nil || jsonString.(string) == "" { + return "", nil + } + + s := jsonString.(string) + + err := json.Unmarshal([]byte(s), &j) + if err != nil { + return s, err + } + + b, _ := json.Marshal(j) + + if bytes.Contains(b, []byte("\\u003c")) || bytes.Contains(b, []byte("\\u003e")) || bytes.Contains(b, []byte("\\u0026")) { + b = bytes.Replace(b, []byte("\\u003c"), []byte("<"), -1) + b = bytes.Replace(b, []byte("\\u003e"), []byte(">"), -1) + b = bytes.Replace(b, []byte("\\u0026"), []byte("&"), -1) + } + return string(b[:]), nil +} + func resourceRuleCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).EventsConn(ctx) From 58015df29397502e7e4732cffe5c4775226c10ac Mon Sep 17 00:00:00 2001 From: Sharon Nam Date: Mon, 26 Jun 2023 16:46:01 -0700 Subject: [PATCH 2/3] Add unit test --- internal/service/events/rule.go | 61 +++++++++++++++------------- internal/service/events/rule_test.go | 36 ++++++++++++++++ 2 files changed, 68 insertions(+), 29 deletions(-) diff --git a/internal/service/events/rule.go b/internal/service/events/rule.go index df25ef6b686a..89d5dd35dbcc 100644 --- a/internal/service/events/rule.go +++ b/internal/service/events/rule.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" @@ -67,7 +66,7 @@ func ResourceRule() *schema.Resource { ValidateFunc: validateEventPatternValue(), AtLeastOneOf: []string{"schedule_expression", "event_pattern"}, StateFunc: func(v interface{}) string { - json, _ := jsonMarshal(v.(string)) + json, _ := RuleEventPatternJSONDecoder(v.(string)) return json }, }, @@ -111,30 +110,6 @@ func ResourceRule() *schema.Resource { } } -func jsonMarshal(jsonString interface{}) (string, error) { - var j interface{} - - if jsonString == nil || jsonString.(string) == "" { - return "", nil - } - - s := jsonString.(string) - - err := json.Unmarshal([]byte(s), &j) - if err != nil { - return s, err - } - - b, _ := json.Marshal(j) - - if bytes.Contains(b, []byte("\\u003c")) || bytes.Contains(b, []byte("\\u003e")) || bytes.Contains(b, []byte("\\u0026")) { - b = bytes.Replace(b, []byte("\\u003c"), []byte("<"), -1) - b = bytes.Replace(b, []byte("\\u003e"), []byte(">"), -1) - b = bytes.Replace(b, []byte("\\u0026"), []byte("&"), -1) - } - return string(b[:]), nil -} - func resourceRuleCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).EventsConn(ctx) @@ -211,7 +186,7 @@ func resourceRuleRead(ctx context.Context, d *schema.ResourceData, meta interfac d.Set("description", output.Description) d.Set("event_bus_name", eventBusName) // Use event bus name from resource ID as API response may collapse any ARN. if output.EventPattern != nil { - pattern, err := structure.NormalizeJsonString(aws.StringValue(output.EventPattern)) + pattern, err := RuleEventPatternJSONDecoder(aws.StringValue(output.EventPattern)) if err != nil { return sdkdiag.AppendErrorf(diags, "event pattern contains an invalid JSON: %s", err) } @@ -321,6 +296,34 @@ func FindRuleByTwoPartKey(ctx context.Context, conn *eventbridge.EventBridge, ev return output, nil } +// Decodes unicode translation of <,>,& +func RuleEventPatternJSONDecoder(jsonString interface{}) (string, error) { + var j interface{} + + if jsonString == nil || jsonString.(string) == "" { + return "", nil + } + + s := jsonString.(string) + + err := json.Unmarshal([]byte(s), &j) + if err != nil { + return s, err + } + + b, err := json.Marshal(j) + if err != nil { + return "", err + } + + if bytes.Contains(b, []byte("\\u003c")) || bytes.Contains(b, []byte("\\u003e")) || bytes.Contains(b, []byte("\\u0026")) { + b = bytes.Replace(b, []byte("\\u003c"), []byte("<"), -1) + b = bytes.Replace(b, []byte("\\u003e"), []byte(">"), -1) + b = bytes.Replace(b, []byte("\\u0026"), []byte("&"), -1) + } + return string(b[:]), nil +} + func expandPutRuleInput(d *schema.ResourceData, name string) *eventbridge.PutRuleInput { apiObject := &eventbridge.PutRuleInput{ Name: aws.String(name), @@ -335,7 +338,7 @@ func expandPutRuleInput(d *schema.ResourceData, name string) *eventbridge.PutRul } if v, ok := d.GetOk("event_pattern"); ok { - json, _ := structure.NormalizeJsonString(v) + json, _ := RuleEventPatternJSONDecoder(v.(string)) apiObject.EventPattern = aws.String(json) } @@ -358,7 +361,7 @@ func expandPutRuleInput(d *schema.ResourceData, name string) *eventbridge.PutRul func validateEventPatternValue() schema.SchemaValidateFunc { return func(v interface{}, k string) (ws []string, errors []error) { - json, err := structure.NormalizeJsonString(v) + json, err := RuleEventPatternJSONDecoder(v.(string)) if err != nil { errors = append(errors, fmt.Errorf("%q contains an invalid JSON: %w", k, err)) diff --git a/internal/service/events/rule_test.go b/internal/service/events/rule_test.go index 6383dff823ce..9e36a8f2f58c 100644 --- a/internal/service/events/rule_test.go +++ b/internal/service/events/rule_test.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/eventbridge" + "github.com/google/go-cmp/cmp" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" @@ -29,6 +30,41 @@ func testAccErrorCheckSkip(t *testing.T) resource.ErrorCheckFunc { ) } +func TestRuleEventPatternJSONDecoder(t *testing.T) { + t.Parallel() + + type testCase struct { + input string + expected string + } + tests := map[string]testCase{ + "lessThanGreaterThan": { + input: `{"detail": {"count": [ { "numeric": [ "\u003e", 0, "\u003c", 5 ] } ]}}`, + expected: `{"detail": {"count": [ { "numeric": [ ">", 0, "<", 5 ] } ]}}`, + }, + "ampersand": { + input: `{"detail": {"count": [ { "numeric": [ "\u0026", 0, "\u0026", 5 ] } ]}}`, + expected: `{"detail": {"count": [ { "numeric": [ "&", 0, "&", 5 ] } ]}}`, + }, + } + + for name, test := range tests { + name, test := name, test + t.Run(name, func(t *testing.T) { + t.Parallel() + + got, err := tfevents.RuleEventPatternJSONDecoder(test.input) + if err != nil { + t.Fatal(err) + } + + if diff := cmp.Diff(got, test.expected); diff != "" { + t.Errorf("unexpected diff (+wanted, -got): %s", diff) + } + }) + } +} + func TestAccEventsRule_basic(t *testing.T) { ctx := acctest.Context(t) var v1, v2, v3 eventbridge.DescribeRuleOutput From 4acc68c177492f7bf5893b8b2a060f39ca8c571f Mon Sep 17 00:00:00 2001 From: Sharon Nam Date: Wed, 28 Jun 2023 13:46:20 -0700 Subject: [PATCH 3/3] Add acceptance test --- internal/service/events/rule.go | 2 +- internal/service/events/rule_test.go | 42 +++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/internal/service/events/rule.go b/internal/service/events/rule.go index 89d5dd35dbcc..3740591cb3a5 100644 --- a/internal/service/events/rule.go +++ b/internal/service/events/rule.go @@ -296,7 +296,7 @@ func FindRuleByTwoPartKey(ctx context.Context, conn *eventbridge.EventBridge, ev return output, nil } -// Decodes unicode translation of <,>,& +// RuleEventPatternJSONDecoder decodes unicode translation of <,>,& func RuleEventPatternJSONDecoder(jsonString interface{}) (string, error) { var j interface{} diff --git a/internal/service/events/rule_test.go b/internal/service/events/rule_test.go index 9e36a8f2f58c..bd3486a5565a 100644 --- a/internal/service/events/rule_test.go +++ b/internal/service/events/rule_test.go @@ -39,12 +39,12 @@ func TestRuleEventPatternJSONDecoder(t *testing.T) { } tests := map[string]testCase{ "lessThanGreaterThan": { - input: `{"detail": {"count": [ { "numeric": [ "\u003e", 0, "\u003c", 5 ] } ]}}`, - expected: `{"detail": {"count": [ { "numeric": [ ">", 0, "<", 5 ] } ]}}`, + input: `{"detail":{"count":[{"numeric":["\u003e",0,"\u003c",5]}]}}`, + expected: `{"detail":{"count":[{"numeric":[">",0,"<",5]}]}}`, }, "ampersand": { - input: `{"detail": {"count": [ { "numeric": [ "\u0026", 0, "\u0026", 5 ] } ]}}`, - expected: `{"detail": {"count": [ { "numeric": [ "&", 0, "&", 5 ] } ]}}`, + input: `{"detail":{"count":[{"numeric":["\u0026",0,"\u0026",5]}]}}`, + expected: `{"detail":{"count":[{"numeric":["&",0,"&",5]}]}}`, }, } @@ -292,6 +292,31 @@ func TestAccEventsRule_pattern(t *testing.T) { }) } +func TestAccEventsRule_patternJSONEncoder(t *testing.T) { + ctx := acctest.Context(t) + var v1 eventbridge.DescribeRuleOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_cloudwatch_event_rule.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, eventbridge.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckRuleDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccRuleConfig_patternJSONEncoder(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckRuleExists(ctx, resourceName, &v1), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "schedule_expression", ""), + acctest.CheckResourceAttrEquivalentJSON(resourceName, "event_pattern", `{"detail":{"count":[{"numeric":[">",0,"<",5]}]}}`), + ), + }, + }, + }) +} + func TestAccEventsRule_scheduleAndPattern(t *testing.T) { ctx := acctest.Context(t) var v eventbridge.DescribeRuleOutput @@ -721,6 +746,15 @@ PATTERN `, rName, pattern) } +func testAccRuleConfig_patternJSONEncoder(rName string) string { + return fmt.Sprintf(` +resource "aws_cloudwatch_event_rule" "test" { + name = %[1]q + event_pattern = jsonencode({ "detail" : { "count" : [{ "numeric" : [">", 0, "<", 5] }] } }) +} +`, rName) +} + func testAccRuleConfig_scheduleAndPattern(rName, pattern string) string { return fmt.Sprintf(` resource "aws_cloudwatch_event_rule" "test" {