From 9688e8fa546c554019368055724a3c3a802bdc90 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 21 Nov 2023 12:11:27 -0800 Subject: [PATCH 1/8] Adds `state` parameter with migrator --- internal/service/events/rule.go | 31 +++- internal/service/events/rule_migrate.go | 94 ++++++++++++ internal/service/events/rule_test.go | 182 +++++++++++++++++++++--- 3 files changed, 281 insertions(+), 26 deletions(-) create mode 100644 internal/service/events/rule_migrate.go diff --git a/internal/service/events/rule.go b/internal/service/events/rule.go index 171f8a790feb..35c1aa8f7bd4 100644 --- a/internal/service/events/rule.go +++ b/internal/service/events/rule.go @@ -46,6 +46,15 @@ func ResourceRule() *schema.Resource { StateContext: schema.ImportStatePassthroughContext, }, + SchemaVersion: 1, + StateUpgraders: []schema.StateUpgrader{ + { + Type: resourceRuleV0().CoreConfigSchema().ImpliedType(), + Upgrade: resourceRuleUpgradeV0, + Version: 0, + }, + }, + Schema: map[string]*schema.Schema{ "arn": { Type: schema.TypeString, @@ -74,9 +83,10 @@ func ResourceRule() *schema.Resource { }, }, "is_enabled": { - Type: schema.TypeBool, - Optional: true, - Default: true, + Type: schema.TypeBool, + Optional: true, + Deprecated: `Use "state" instead`, + Default: true, }, "name": { Type: schema.TypeString, @@ -105,6 +115,20 @@ func ResourceRule() *schema.Resource { ValidateFunc: validation.StringLenBetween(0, 256), AtLeastOneOf: []string{"schedule_expression", "event_pattern"}, }, + "state": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringInSlice( + eventbridge.EventSourceState_Values(), + false, + ), + DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool { + if oldValue != "" && newValue == "" { + return true + } + return false + }, + }, names.AttrTags: tftags.TagsSchema(), names.AttrTagsAll: tftags.TagsSchemaComputed(), }, @@ -196,6 +220,7 @@ func resourceRuleRead(ctx context.Context, d *schema.ResourceData, meta interfac d.Set("event_pattern", pattern) } d.Set("is_enabled", aws.StringValue(output.State) == eventbridge.RuleStateEnabled) + d.Set("state", aws.StringValue(output.State)) d.Set("name", output.Name) d.Set("name_prefix", create.NamePrefixFromName(aws.StringValue(output.Name))) d.Set("role_arn", output.RoleArn) diff --git a/internal/service/events/rule_migrate.go b/internal/service/events/rule_migrate.go new file mode 100644 index 000000000000..75582280ff13 --- /dev/null +++ b/internal/service/events/rule_migrate.go @@ -0,0 +1,94 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package events + +import ( + "context" + + "github.com/aws/aws-sdk-go/service/eventbridge" + "github.com/hashicorp/terraform-plugin-log/tflog" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" + "github.com/hashicorp/terraform-provider-aws/internal/verify" + "github.com/hashicorp/terraform-provider-aws/names" +) + +func resourceRuleV0() *schema.Resource { + return &schema.Resource{ + Schema: map[string]*schema.Schema{ + "arn": { + Type: schema.TypeString, + Computed: true, + }, + "description": { + Type: schema.TypeString, + Optional: true, + }, + "event_bus_name": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Default: DefaultEventBusName, + }, + "event_pattern": { + Type: schema.TypeString, + Optional: true, + StateFunc: func(v interface{}) string { + json, _ := RuleEventPatternJSONDecoder(v.(string)) + return json + }, + }, + "is_enabled": { + Type: schema.TypeBool, + Optional: true, + }, + "name": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + "name_prefix": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + "role_arn": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: verify.ValidARN, + }, + "schedule_expression": { + Type: schema.TypeString, + Optional: true, + }, + "state": { + Type: schema.TypeString, + Optional: true, + }, + names.AttrTags: tftags.TagsSchema(), + names.AttrTagsAll: tftags.TagsSchemaComputed(), + }, + } +} + +func resourceRuleUpgradeV0(ctx context.Context, rawState map[string]any, meta any) (map[string]any, error) { + if rawState == nil { + rawState = map[string]any{} + } + + tflog.Debug(ctx, "Upgrading resource", map[string]any{ + "from_version": 0, + "to_version": 1, + }) + + if rawState["is_enabled"].(bool) { + rawState["state"] = eventbridge.RuleStateEnabled + } else { + rawState["state"] = eventbridge.RuleStateDisabled + } + + return rawState, nil +} diff --git a/internal/service/events/rule_test.go b/internal/service/events/rule_test.go index 4484f529502b..1564869d8ec5 100644 --- a/internal/service/events/rule_test.go +++ b/internal/service/events/rule_test.go @@ -15,6 +15,7 @@ import ( "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/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" @@ -83,7 +84,7 @@ func TestAccEventsRule_basic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_basic(rName1), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v1), acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "events", regexache.MustCompile(fmt.Sprintf(`rule/%s$`, rName1))), resource.TestCheckResourceAttr(resourceName, "name", rName1), @@ -95,6 +96,7 @@ func TestAccEventsRule_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "role_arn", ""), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "state", "ENABLED"), testAccCheckRuleEnabled(ctx, resourceName, "ENABLED"), ), }, @@ -111,7 +113,7 @@ func TestAccEventsRule_basic(t *testing.T) { }, { Config: testAccRuleConfig_basic(rName2), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v2), testAccCheckRuleRecreated(&v1, &v2), acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "events", regexache.MustCompile(fmt.Sprintf(`rule/%s$`, rName2))), @@ -121,12 +123,13 @@ func TestAccEventsRule_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "role_arn", ""), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "state", "ENABLED"), testAccCheckRuleEnabled(ctx, resourceName, "ENABLED"), ), }, { Config: testAccRuleConfig_defaultBusName(rName2), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v3), acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "events", regexache.MustCompile(fmt.Sprintf(`rule/%s$`, rName2))), testAccCheckRuleNotRecreated(&v2, &v3), @@ -155,7 +158,7 @@ func TestAccEventsRule_eventBusName(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_busName(rName1, busName1, "description 1"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v1), resource.TestCheckResourceAttr(resourceName, "name", rName1), resource.TestCheckResourceAttr(resourceName, "event_bus_name", busName1), @@ -169,7 +172,7 @@ func TestAccEventsRule_eventBusName(t *testing.T) { }, { Config: testAccRuleConfig_busName(rName1, busName1, "description 2"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v2), testAccCheckRuleNotRecreated(&v1, &v2), resource.TestCheckResourceAttr(resourceName, "name", rName1), @@ -178,7 +181,7 @@ func TestAccEventsRule_eventBusName(t *testing.T) { }, { Config: testAccRuleConfig_busName(rName2, busName2, "description 2"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v3), testAccCheckRuleRecreated(&v2, &v3), resource.TestCheckResourceAttr(resourceName, "name", rName2), @@ -205,7 +208,7 @@ func TestAccEventsRule_role(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_role(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttrPair(resourceName, "role_arn", iamRoleResourceName, "arn"), @@ -234,7 +237,7 @@ func TestAccEventsRule_description(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_description(rName, "description1"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v1), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "description", "description1"), @@ -247,7 +250,7 @@ func TestAccEventsRule_description(t *testing.T) { }, { Config: testAccRuleConfig_description(rName, "description2"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v2), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "description", "description2"), @@ -271,7 +274,7 @@ func TestAccEventsRule_pattern(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_pattern(rName, "{\"source\":[\"aws.ec2\"]}"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v1), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "schedule_expression", ""), @@ -285,7 +288,7 @@ func TestAccEventsRule_pattern(t *testing.T) { }, { Config: testAccRuleConfig_pattern(rName, "{\"source\":[\"aws.lambda\"]}"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v2), resource.TestCheckResourceAttr(resourceName, "name", rName), acctest.CheckResourceAttrEquivalentJSON(resourceName, "event_pattern", "{\"source\":[\"aws.lambda\"]}"), @@ -309,7 +312,7 @@ func TestAccEventsRule_patternJSONEncoder(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_patternJSONEncoder(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v1), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "schedule_expression", ""), @@ -334,7 +337,7 @@ func TestAccEventsRule_scheduleAndPattern(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_scheduleAndPattern(rName, "{\"source\":[\"aws.ec2\"]}"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "schedule_expression", "rate(1 hour)"), @@ -364,7 +367,7 @@ func TestAccEventsRule_namePrefix(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_namePrefix(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v), acctest.CheckResourceAttrNameFromPrefix(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "name_prefix", rName), @@ -392,7 +395,7 @@ func TestAccEventsRule_Name_generated(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_nameGenerated, - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v), acctest.CheckResourceAttrNameGenerated(resourceName, "name"), resource.TestCheckResourceAttr(resourceName, "name_prefix", "terraform-"), @@ -421,7 +424,7 @@ func TestAccEventsRule_tags(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_tags1(rName, "key1", "value1"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v1), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), @@ -434,7 +437,7 @@ func TestAccEventsRule_tags(t *testing.T) { }, { Config: testAccRuleConfig_tags2(rName, "key1", "value1updated", "key2", "value2"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v2), resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), @@ -443,7 +446,7 @@ func TestAccEventsRule_tags(t *testing.T) { }, { Config: testAccRuleConfig_tags1(rName, "key2", "value2"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v3), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), @@ -467,9 +470,10 @@ func TestAccEventsRule_isEnabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_isEnabled(rName, false), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v1), resource.TestCheckResourceAttr(resourceName, "is_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "state", "DISABLED"), testAccCheckRuleEnabled(ctx, resourceName, "DISABLED"), ), }, @@ -480,17 +484,67 @@ func TestAccEventsRule_isEnabled(t *testing.T) { }, { Config: testAccRuleConfig_isEnabled(rName, true), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v2), resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "state", "ENABLED"), testAccCheckRuleEnabled(ctx, resourceName, "ENABLED"), ), }, { Config: testAccRuleConfig_isEnabled(rName, false), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v3), resource.TestCheckResourceAttr(resourceName, "is_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "state", "DISABLED"), + testAccCheckRuleEnabled(ctx, resourceName, "DISABLED"), + ), + }, + }, + }) +} + +func TestAccEventsRule_state(t *testing.T) { + ctx := acctest.Context(t) + var v1, v2, v3 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_isEnabled(rName, false), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckRuleExists(ctx, resourceName, &v1), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "state", "DISABLED"), + testAccCheckRuleEnabled(ctx, resourceName, "DISABLED"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccRuleConfig_isEnabled(rName, true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckRuleExists(ctx, resourceName, &v2), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "state", "ENABLED"), + testAccCheckRuleEnabled(ctx, resourceName, "ENABLED"), + ), + }, + { + Config: testAccRuleConfig_isEnabled(rName, false), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckRuleExists(ctx, resourceName, &v3), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "state", "DISABLED"), testAccCheckRuleEnabled(ctx, resourceName, "DISABLED"), ), }, @@ -518,13 +572,14 @@ func TestAccEventsRule_partnerEventBus(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_partnerBus(rName, busName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v), acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "events", regexache.MustCompile(fmt.Sprintf(`rule/%s/%s$`, busName, rName))), resource.TestCheckResourceAttr(resourceName, "description", ""), resource.TestCheckResourceAttr(resourceName, "event_bus_name", busName), acctest.CheckResourceAttrEquivalentJSON(resourceName, "event_pattern", "{\"source\":[\"aws.ec2\"]}"), resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "state", "ENABLED"), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "role_arn", ""), resource.TestCheckResourceAttr(resourceName, "schedule_expression", ""), @@ -555,13 +610,14 @@ func TestAccEventsRule_eventBusARN(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccRuleConfig_busARN(rName, eventBusName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v), acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "events", regexache.MustCompile(fmt.Sprintf(`rule/%s/%s$`, eventBusName, rName))), resource.TestCheckResourceAttr(resourceName, "description", ""), resource.TestCheckResourceAttrPair(resourceName, "event_bus_name", "aws_cloudwatch_event_bus.test", "arn"), acctest.CheckResourceAttrEquivalentJSON(resourceName, "event_pattern", "{\"source\":[\"aws.ec2\"]}"), resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "state", "ENABLED"), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "role_arn", ""), resource.TestCheckResourceAttr(resourceName, "schedule_expression", ""), @@ -577,6 +633,86 @@ func TestAccEventsRule_eventBusARN(t *testing.T) { }) } +func TestAccEventsRule_migrateV0(t *testing.T) { + const resourceName = "aws_cloudwatch_event_rule.test" + + t.Parallel() + + testcases := map[string]struct { + config string + expectedIsEnabled string + expectedState string + }{ + "basic": { + config: testAccRuleConfig_basic(sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)), + expectedIsEnabled: "true", + expectedState: "ENABLED", + }, + + "enabled": { + config: testAccRuleConfig_isEnabled(sdkacctest.RandomWithPrefix(acctest.ResourcePrefix), true), + expectedIsEnabled: "true", + expectedState: "ENABLED", + }, + + "disabled": { + config: testAccRuleConfig_isEnabled(sdkacctest.RandomWithPrefix(acctest.ResourcePrefix), false), + expectedIsEnabled: "false", + expectedState: "DISABLED", + }, + } + + for name, testcase := range testcases { + testcase := testcase + + t.Run(name, func(t *testing.T) { + ctx := acctest.Context(t) + var v eventbridge.DescribeRuleOutput + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, eventbridge.EndpointsID), + CheckDestroy: testAccCheckRuleDestroy(ctx), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "aws": { + Source: "hashicorp/aws", + VersionConstraint: "5.26.0", + }, + }, + Config: testcase.config, + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckRuleExists(ctx, resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "is_enabled", testcase.expectedIsEnabled), + testAccCheckRuleEnabled(ctx, resourceName, testcase.expectedState), + ), + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: testcase.config, + PlanOnly: true, + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: testcase.config, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "is_enabled", testcase.expectedIsEnabled), + resource.TestCheckResourceAttr(resourceName, "state", testcase.expectedState), + testAccCheckRuleEnabled(ctx, resourceName, testcase.expectedState), + ), + }, + }, + }) + }) + } +} + func testAccCheckRuleExists(ctx context.Context, n string, v *eventbridge.DescribeRuleOutput) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] From 27e1c8107b4f4e9f78c720e1908aabbfd45f6399 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 21 Nov 2023 14:52:40 -0800 Subject: [PATCH 2/8] Allows setting `state` --- internal/service/events/rule.go | 33 ++++++++++++++---- internal/service/events/rule_test.go | 50 +++++++++++++++++++--------- 2 files changed, 61 insertions(+), 22 deletions(-) diff --git a/internal/service/events/rule.go b/internal/service/events/rule.go index 35c1aa8f7bd4..18d634fb207e 100644 --- a/internal/service/events/rule.go +++ b/internal/service/events/rule.go @@ -86,7 +86,11 @@ func ResourceRule() *schema.Resource { Type: schema.TypeBool, Optional: true, Deprecated: `Use "state" instead`, - Default: true, + DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool { + rawPlan := d.GetRawPlan() + rawIsEnabled := rawPlan.GetAttr("is_enabled") + return rawIsEnabled.IsKnown() && rawIsEnabled.IsNull() + }, }, "name": { Type: schema.TypeString, @@ -119,7 +123,7 @@ func ResourceRule() *schema.Resource { Type: schema.TypeString, Optional: true, ValidateFunc: validation.StringInSlice( - eventbridge.EventSourceState_Values(), + eventbridge.RuleState_Values(), false, ), DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool { @@ -219,7 +223,13 @@ func resourceRuleRead(ctx context.Context, d *schema.ResourceData, meta interfac } d.Set("event_pattern", pattern) } - d.Set("is_enabled", aws.StringValue(output.State) == eventbridge.RuleStateEnabled) + switch aws.StringValue(output.State) { + case eventbridge.RuleStateEnabled, + eventbridge.RuleStateEnabledWithAllCloudtrailManagementEvents: + d.Set("is_enabled", true) + default: + d.Set("is_enabled", false) + } d.Set("state", aws.StringValue(output.State)) d.Set("name", output.Name) d.Set("name_prefix", create.NamePrefixFromName(aws.StringValue(output.Name))) @@ -378,11 +388,20 @@ func expandPutRuleInput(d *schema.ResourceData, name string) *eventbridge.PutRul apiObject.ScheduleExpression = aws.String(v.(string)) } - state := eventbridge.RuleStateDisabled - if d.Get("is_enabled").(bool) { - state = eventbridge.RuleStateEnabled + rawConfig := d.GetRawConfig() + rawState := rawConfig.GetAttr("state") + if rawState.IsKnown() && !rawState.IsNull() { + apiObject.State = aws.String(rawState.AsString()) + } else { + rawIsEnabled := rawConfig.GetAttr("is_enabled") + if rawIsEnabled.IsKnown() && !rawIsEnabled.IsNull() { + if rawIsEnabled.True() { + apiObject.State = aws.String(eventbridge.RuleStateEnabled) + } else { + apiObject.State = aws.String(eventbridge.RuleStateDisabled) + } + } } - apiObject.State = aws.String(state) return apiObject } diff --git a/internal/service/events/rule_test.go b/internal/service/events/rule_test.go index 1564869d8ec5..ee54b7475e10 100644 --- a/internal/service/events/rule_test.go +++ b/internal/service/events/rule_test.go @@ -491,6 +491,11 @@ func TestAccEventsRule_isEnabled(t *testing.T) { testAccCheckRuleEnabled(ctx, resourceName, "ENABLED"), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, { Config: testAccRuleConfig_isEnabled(rName, false), Check: resource.ComposeAggregateTestCheckFunc( @@ -517,12 +522,12 @@ func TestAccEventsRule_state(t *testing.T) { CheckDestroy: testAccCheckRuleDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccRuleConfig_isEnabled(rName, false), + Config: testAccRuleConfig_state(rName, eventbridge.RuleStateDisabled), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v1), resource.TestCheckResourceAttr(resourceName, "is_enabled", "false"), - resource.TestCheckResourceAttr(resourceName, "state", "DISABLED"), - testAccCheckRuleEnabled(ctx, resourceName, "DISABLED"), + resource.TestCheckResourceAttr(resourceName, "state", eventbridge.RuleStateDisabled), + testAccCheckRuleEnabled(ctx, resourceName, eventbridge.RuleStateDisabled), ), }, { @@ -531,23 +536,33 @@ func TestAccEventsRule_state(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccRuleConfig_isEnabled(rName, true), + Config: testAccRuleConfig_state(rName, eventbridge.RuleStateEnabled), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v2), resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), - resource.TestCheckResourceAttr(resourceName, "state", "ENABLED"), - testAccCheckRuleEnabled(ctx, resourceName, "ENABLED"), + resource.TestCheckResourceAttr(resourceName, "state", eventbridge.RuleStateEnabled), + testAccCheckRuleEnabled(ctx, resourceName, eventbridge.RuleStateEnabled), ), }, { - Config: testAccRuleConfig_isEnabled(rName, false), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccRuleConfig_state(rName, eventbridge.RuleStateEnabledWithAllCloudtrailManagementEvents), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckRuleExists(ctx, resourceName, &v3), - resource.TestCheckResourceAttr(resourceName, "is_enabled", "false"), - resource.TestCheckResourceAttr(resourceName, "state", "DISABLED"), - testAccCheckRuleEnabled(ctx, resourceName, "DISABLED"), + resource.TestCheckResourceAttr(resourceName, "is_enabled", "true"), + resource.TestCheckResourceAttr(resourceName, "state", eventbridge.RuleStateEnabledWithAllCloudtrailManagementEvents), + testAccCheckRuleEnabled(ctx, resourceName, eventbridge.RuleStateEnabledWithAllCloudtrailManagementEvents), ), }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, }, }) } @@ -688,11 +703,6 @@ func TestAccEventsRule_migrateV0(t *testing.T) { testAccCheckRuleEnabled(ctx, resourceName, testcase.expectedState), ), }, - { - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - Config: testcase.config, - PlanOnly: true, - }, { ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, Config: testcase.config, @@ -926,6 +936,16 @@ resource "aws_cloudwatch_event_rule" "test" { `, rName, enabled) } +func testAccRuleConfig_state(rName, state string) string { + return fmt.Sprintf(` +resource "aws_cloudwatch_event_rule" "test" { + name = %[1]q + schedule_expression = "rate(1 hour)" + state = %[2]q +} +`, rName, state) +} + func testAccRuleConfig_namePrefix(namePrefix string) string { return fmt.Sprintf(` resource "aws_cloudwatch_event_rule" "test" { From 9efc6c100a4a470acd2345ea3942dedc15b974fe Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 21 Nov 2023 14:53:34 -0800 Subject: [PATCH 3/8] Adds test for equivalent changes when migrating --- internal/service/events/rule_test.go | 73 ++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/internal/service/events/rule_test.go b/internal/service/events/rule_test.go index ee54b7475e10..89788ae42b76 100644 --- a/internal/service/events/rule_test.go +++ b/internal/service/events/rule_test.go @@ -723,6 +723,79 @@ func TestAccEventsRule_migrateV0(t *testing.T) { } } +func TestAccEventsRule_migrateV0_Equivalent(t *testing.T) { + const resourceName = "aws_cloudwatch_event_rule.test" + + t.Parallel() + + testcases := map[string]struct { + enabled bool + state string + expectedIsEnabled string + expectedState string + }{ + "enabled": { + enabled: true, + state: eventbridge.RuleStateEnabled, + expectedIsEnabled: "true", + expectedState: eventbridge.RuleStateEnabled, + }, + + "disabled": { + enabled: false, + state: eventbridge.RuleStateDisabled, + expectedIsEnabled: "false", + expectedState: eventbridge.RuleStateDisabled, + }, + } + + for name, testcase := range testcases { + testcase := testcase + + t.Run(name, func(t *testing.T) { + ctx := acctest.Context(t) + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + var v eventbridge.DescribeRuleOutput + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, eventbridge.EndpointsID), + CheckDestroy: testAccCheckRuleDestroy(ctx), + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "aws": { + Source: "hashicorp/aws", + VersionConstraint: "5.26.0", + }, + }, + Config: testAccRuleConfig_isEnabled(rName, testcase.enabled), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckRuleExists(ctx, resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "is_enabled", testcase.expectedIsEnabled), + testAccCheckRuleEnabled(ctx, resourceName, testcase.expectedState), + ), + }, + { + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Config: testAccRuleConfig_state(rName, testcase.state), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "is_enabled", testcase.expectedIsEnabled), + resource.TestCheckResourceAttr(resourceName, "state", testcase.expectedState), + testAccCheckRuleEnabled(ctx, resourceName, testcase.expectedState), + ), + }, + }, + }) + }) + } +} + func testAccCheckRuleExists(ctx context.Context, n string, v *eventbridge.DescribeRuleOutput) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] From 10f5c747a646db5d7ae76ed6e1c69020519ecfc5 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 21 Nov 2023 15:11:04 -0800 Subject: [PATCH 4/8] Adds `ConflictsWith` --- internal/service/events/rule.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/service/events/rule.go b/internal/service/events/rule.go index 18d634fb207e..2bddc4da9910 100644 --- a/internal/service/events/rule.go +++ b/internal/service/events/rule.go @@ -91,6 +91,9 @@ func ResourceRule() *schema.Resource { rawIsEnabled := rawPlan.GetAttr("is_enabled") return rawIsEnabled.IsKnown() && rawIsEnabled.IsNull() }, + ConflictsWith: []string{ + "state", + }, }, "name": { Type: schema.TypeString, @@ -132,6 +135,9 @@ func ResourceRule() *schema.Resource { } return false }, + ConflictsWith: []string{ + "is_enabled", + }, }, names.AttrTags: tftags.TagsSchema(), names.AttrTagsAll: tftags.TagsSchemaComputed(), From 20b303dffc64f30555609f2b3010e86d3ed7801c Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 21 Nov 2023 15:11:29 -0800 Subject: [PATCH 5/8] Updates documentation --- website/docs/r/cloudwatch_event_rule.html.markdown | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/website/docs/r/cloudwatch_event_rule.html.markdown b/website/docs/r/cloudwatch_event_rule.html.markdown index b5b33c4f1061..7c7ea913e485 100644 --- a/website/docs/r/cloudwatch_event_rule.html.markdown +++ b/website/docs/r/cloudwatch_event_rule.html.markdown @@ -68,7 +68,15 @@ This resource supports the following arguments: * `event_pattern` - (Optional) The event pattern described a JSON object. At least one of `schedule_expression` or `event_pattern` is required. See full documentation of [Events and Event Patterns in EventBridge](https://docs.aws.amazon.com/eventbridge/latest/userguide/eventbridge-and-event-patterns.html) for details. **Note**: The event pattern size is 2048 by default but it is adjustable up to 4096 characters by submitting a service quota increase request. See [Amazon EventBridge quotas](https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-quota.html) for details. * `description` - (Optional) The description of the rule. * `role_arn` - (Optional) The Amazon Resource Name (ARN) associated with the role that is used for target invocation. -* `is_enabled` - (Optional) Whether the rule should be enabled (defaults to `true`). +* `is_enabled` - (Optional, **Deprecated** Use `state` instead) Whether the rule should be enabled. + Defaults to `true`. + Conflicts with `state`. +* `state` - (Optional) State of the rule. + Valid values are `DISABLED`, `ENABLED`, and `ENABLED_WITH_ALL_CLOUDTRAIL_MANAGEMENT_EVENTS`. + When state is `ENABLED`, the rule is enabled for all events except those delivered by CloudTrail. + To also enable the rule for events delivered by CloudTrail, set `state` to `ENABLED_WITH_ALL_CLOUDTRAIL_MANAGEMENT_EVENTS`. + Defaults to `ENABLED`. + Conflicts with `is_enabled`. * `tags` - (Optional) A map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. ## Attribute Reference From d11a8fdd914ecab085a4d5c6c4162d5184a131ea Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 21 Nov 2023 15:17:53 -0800 Subject: [PATCH 6/8] Update CHANGELOG --- .changelog/34510.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/34510.txt diff --git a/.changelog/34510.txt b/.changelog/34510.txt new file mode 100644 index 000000000000..b8ed9153fdd0 --- /dev/null +++ b/.changelog/34510.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_cloudwatch_event_rule: Add `state` parameter and deprecate `is_enabled` parameter +``` From ef07da1427b8786aa80d8193c8175aa50c50079d Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 21 Nov 2023 15:29:22 -0800 Subject: [PATCH 7/8] `semgrep` --- internal/service/events/rule.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/events/rule.go b/internal/service/events/rule.go index 2bddc4da9910..501a08393f69 100644 --- a/internal/service/events/rule.go +++ b/internal/service/events/rule.go @@ -236,7 +236,7 @@ func resourceRuleRead(ctx context.Context, d *schema.ResourceData, meta interfac default: d.Set("is_enabled", false) } - d.Set("state", aws.StringValue(output.State)) + d.Set("state", output.State) d.Set("name", output.Name) d.Set("name_prefix", create.NamePrefixFromName(aws.StringValue(output.Name))) d.Set("role_arn", output.RoleArn) From f2852adf00cc7281831f7e03a1b619460971c76a Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 21 Nov 2023 16:35:57 -0800 Subject: [PATCH 8/8] Linting fixes --- internal/service/events/rule_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/events/rule_test.go b/internal/service/events/rule_test.go index 89788ae42b76..54961a2976d2 100644 --- a/internal/service/events/rule_test.go +++ b/internal/service/events/rule_test.go @@ -677,7 +677,7 @@ func TestAccEventsRule_migrateV0(t *testing.T) { }, } - for name, testcase := range testcases { + for name, testcase := range testcases { //nolint:paralleltest testcase := testcase t.Run(name, func(t *testing.T) { @@ -749,7 +749,7 @@ func TestAccEventsRule_migrateV0_Equivalent(t *testing.T) { }, } - for name, testcase := range testcases { + for name, testcase := range testcases { //nolint:paralleltest testcase := testcase t.Run(name, func(t *testing.T) {