Merge pull request #30242 from codablock/resourcegroups-no-conflict

r/aws_resourcegroups_group: Make resource_query and configuration not conflict
hashicorp · Jun 29, 2023 · 5c61579 · 5c61579
2 parents f9e9bbd + 2b83189
commit 5c61579
Show file tree

Hide file tree

Showing 11 changed files with 317 additions and 130 deletions.
diff --git a/.changelog/30242.txt b/.changelog/30242.txt
@@ -0,0 +1,7 @@
+```release-note:enhancement
+resource/aws_resourcegroups_group: `resource_query` no longer conflicts with `configuration`
+```
+
+```release-note:bug
+resource/aws_resourcegroups_resource: Fix crash when resource Create fails
+```
diff --git a/internal/service/resourcegroups/group.go b/internal/service/resourcegroups/group.go
@@ -44,9 +44,8 @@ func ResourceGroup() *schema.Resource {
 				Computed: true,
 			},
 			"configuration": {
-				Type:          schema.TypeSet,
-				Optional:      true,
-				ConflictsWith: []string{"resource_query"},
+				Type:     schema.TypeSet,
+				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"parameters": {
@@ -85,11 +84,10 @@ func ResourceGroup() *schema.Resource {
 				ForceNew: true,
 			},
 			"resource_query": {
-				Type:          schema.TypeList,
-				Optional:      true,
-				MinItems:      1,
-				MaxItems:      1,
-				ConflictsWith: []string{"configuration"},
+				Type:     schema.TypeList,
+				Optional: true,
+				MinItems: 1,
+				MaxItems: 1,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"query": {
@@ -178,32 +176,37 @@ func resourceGroupRead(ctx context.Context, d *schema.ResourceData, meta interfa
 		GroupName: aws.String(d.Id()),
 	})
 
-	isConfigurationGroup := false
+	hasQuery := true
 	if err != nil {
 		if tfawserr.ErrCodeEquals(err, resourcegroups.ErrCodeBadRequestException) {
 			// Attempting to get the query on a configuration group returns BadRequestException.
-			isConfigurationGroup = true
+			hasQuery = false
 		} else {
 			return diag.Errorf("reading Resource Groups Group (%s) resource query: %s", d.Id(), err)
 		}
 	}
 
-	if !isConfigurationGroup {
+	groupCfg, err := findGroupConfigurationByGroupName(ctx, conn, d.Id())
+
+	hasConfiguration := true
+	if err != nil {
+		if tfawserr.ErrCodeEquals(err, resourcegroups.ErrCodeBadRequestException) {
+			// Attempting to get configuration on a query group returns BadRequestException.
+			hasConfiguration = false
+		} else {
+			return diag.Errorf("reading Resource Groups Group (%s) configuration: %s", d.Id(), err)
+		}
+	}
+
+	if hasQuery {
 		resultQuery := map[string]interface{}{}
 		resultQuery["query"] = aws.StringValue(q.GroupQuery.ResourceQuery.Query)
 		resultQuery["type"] = aws.StringValue(q.GroupQuery.ResourceQuery.Type)
 		if err := d.Set("resource_query", []map[string]interface{}{resultQuery}); err != nil {
 			return diag.Errorf("setting resource_query: %s", err)
 		}
 	}
-
-	if isConfigurationGroup {
-		groupCfg, err := findGroupConfigurationByGroupName(ctx, conn, d.Id())
-
-		if err != nil {
-			return diag.Errorf("reading Resource Groups Group (%s) configuration: %s", d.Id(), err)
-		}
-
+	if hasConfiguration {
 		if err := d.Set("configuration", flattenResourceGroupConfigurationItems(groupCfg.Configuration)); err != nil {
 			return diag.Errorf("setting configuration: %s", err)
 		}

diff --git a/internal/service/resourcegroups/group_test.go b/internal/service/resourcegroups/group_test.go
@@ -223,6 +223,40 @@ func TestAccResourceGroupsGroup_configurationParametersOptional(t *testing.T) {
 	})
 }
 
+func TestAccResourceGroupsGroup_resourceQueryAndConfiguration(t *testing.T) {
+	ctx := acctest.Context(t)
+	var v resourcegroups.Group
+	resourceName := "aws_resourcegroups_group.test"
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+
+	configType := "AWS::NetworkFirewall::RuleGroup"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, resourcegroups.EndpointsID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckResourceGroupDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccGroupConfig_resourceQueryAndConfiguration(rName, testAccResourceGroupQueryConfig, configType),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckResourceGroupExists(ctx, resourceName, &v),
+					resource.TestCheckResourceAttr(resourceName, "name", rName),
+					resource.TestCheckResourceAttrSet(resourceName, "arn"),
+					resource.TestCheckResourceAttr(resourceName, "resource_query.0.query", testAccResourceGroupQueryConfig+"\n"),
+					resource.TestCheckResourceAttr(resourceName, "configuration.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "configuration.0.type", configType),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func testAccCheckResourceGroupExists(ctx context.Context, n string, v *resourcegroups.Group) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -425,3 +459,22 @@ resource "aws_resourcegroups_group" "test" {
 }
 `, rName, configType1, configType2)
 }
+
+func testAccGroupConfig_resourceQueryAndConfiguration(rName, query, configType string) string {
+	return fmt.Sprintf(`
+resource "aws_resourcegroups_group" "test" {
+  name = %[1]q
+
+  resource_query {
+    query = <<JSON
+%[2]s
+JSON
+
+  }
+
+  configuration {
+    type = %[3]q
+  }
+}
+`, rName, query, configType)
+}