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.

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

Add Tag Support to Resource Group Resource #10640

Merged
merged 11 commits into from
Nov 19, 2019
21 changes: 20 additions & 1 deletion aws/resource_aws_resourcegroups_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/aws/aws-sdk-go/service/resourcegroups"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

func resourceAwsResourceGroupsGroup() *schema.Resource {
Expand Down Expand Up @@ -61,6 +62,7 @@ func resourceAwsResourceGroupsGroup() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},
"tags": tagsSchema(),
},
}
}
Expand All @@ -81,6 +83,7 @@ func resourceAwsResourceGroupsGroupCreate(d *schema.ResourceData, meta interface
Description: aws.String(d.Get("description").(string)),
Name: aws.String(d.Get("name").(string)),
ResourceQuery: extractResourceGroupResourceQuery(d.Get("resource_query").([]interface{})),
Tags: keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().ResourcegroupsTags(),
}

res, err := conn.CreateGroup(&input)
Expand Down Expand Up @@ -110,9 +113,10 @@ func resourceAwsResourceGroupsGroupRead(d *schema.ResourceData, meta interface{}
return fmt.Errorf("error reading resource group (%s): %s", d.Id(), err)
}

arn := aws.StringValue(g.Group.GroupArn)
d.Set("name", aws.StringValue(g.Group.Name))
d.Set("description", aws.StringValue(g.Group.Description))
d.Set("arn", aws.StringValue(g.Group.GroupArn))
d.Set("arn", arn)

q, err := conn.GetGroupQuery(&resourcegroups.GetGroupQueryInput{
GroupName: aws.String(d.Id()),
Expand All @@ -129,6 +133,14 @@ func resourceAwsResourceGroupsGroupRead(d *schema.ResourceData, meta interface{}
return fmt.Errorf("error setting resource_query: %s", err)
}

tags, err := keyvaluetags.ResourcegroupsListTags(conn, arn)
if err != nil {
return fmt.Errorf("error listing tags for resource (%s): %s", arn, err)
}
if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil {
return fmt.Errorf("error setting tags: %s", err)
}

return nil
}

Expand Down Expand Up @@ -159,6 +171,13 @@ func resourceAwsResourceGroupsGroupUpdate(d *schema.ResourceData, meta interface
}
}

if d.HasChange("tags") {
o, n := d.GetChange("tags")
if err := keyvaluetags.ResourcegroupsUpdateTags(conn, d.Get("arn").(string), o, n); err != nil {
return fmt.Errorf("error updating tags: %s", err)
}
}

return resourceAwsResourceGroupsGroupRead(d, meta)
}

Expand Down
136 changes: 115 additions & 21 deletions aws/resource_aws_resourcegroups_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,13 @@ import (
)

func TestAccAWSResourceGroup_basic(t *testing.T) {
var v resourcegroups.Group
resourceName := "aws_resourcegroups_group.test"
n := fmt.Sprintf("test-group-%d", acctest.RandInt())

desc1 := "Hello World"
desc2 := "Foo Bar"

query1 := `{
"ResourceTypeFilters": [
"AWS::EC2::Instance"
],
"TagFilters": [
{
"Key": "Stage",
"Values": ["Test"]
}
]
}
`

query2 := `{
"ResourceTypeFilters": [
"AWS::EC2::Instance"
Expand All @@ -50,12 +38,12 @@ func TestAccAWSResourceGroup_basic(t *testing.T) {
CheckDestroy: testAccCheckAWSResourceGroupDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSResourceGroupConfig_basic(n, desc1, query1),
Config: testAccAWSResourceGroupConfig_basic(n, desc1, testAccAWSResourceGroupConfigQuery),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSResourceGroupExists(resourceName),
testAccCheckAWSResourceGroupExists(resourceName, &v),
resource.TestCheckResourceAttr(resourceName, "name", n),
resource.TestCheckResourceAttr(resourceName, "description", desc1),
resource.TestCheckResourceAttr(resourceName, "resource_query.0.query", query1+"\n"),
resource.TestCheckResourceAttr(resourceName, "resource_query.0.query", testAccAWSResourceGroupConfigQuery+"\n"),
resource.TestCheckResourceAttrSet(resourceName, "arn"),
),
},
Expand All @@ -75,7 +63,52 @@ func TestAccAWSResourceGroup_basic(t *testing.T) {
})
}

func testAccCheckAWSResourceGroupExists(n string) resource.TestCheckFunc {
func TestAccAWSResourceGroup_tags(t *testing.T) {
var v resourcegroups.Group
resourceName := "aws_resourcegroups_group.test"
n := fmt.Sprintf("test-group-%d", acctest.RandInt())
desc1 := "Hello World"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This acceptance testing does not need to verify anything with this attribute, so it should omitted or hardcoded in the test configurations.


resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSResourceGroupDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSResourceGroupConfigTags1(n, desc1, testAccAWSResourceGroupConfigQuery, "key1", "value1"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSResourceGroupExists(resourceName, &v),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAWSResourceGroupConfigTags2(n, desc1, testAccAWSResourceGroupConfigQuery, "key1", "value1updated", "key2", "value2"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSResourceGroupExists(resourceName, &v),
resource.TestCheckResourceAttr(resourceName, "tags.%", "2"),
resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
),
},
{
Config: testAccAWSResourceGroupConfigTags1(n, desc1, testAccAWSResourceGroupConfigQuery, "key2", "value2"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSResourceGroupExists(resourceName, &v),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"),
),
},
},
})
}

func testAccCheckAWSResourceGroupExists(n string, v *resourcegroups.Group) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
Expand All @@ -88,17 +121,26 @@ func testAccCheckAWSResourceGroupExists(n string) resource.TestCheckFunc {

conn := testAccProvider.Meta().(*AWSClient).resourcegroupsconn

_, err := conn.GetGroup(&resourcegroups.GetGroupInput{
resp, err := conn.GetGroup(&resourcegroups.GetGroupInput{
GroupName: aws.String(rs.Primary.ID),
})

return err
if err != nil {
return err
}

if *resp.Group.Name == rs.Primary.ID {
*v = *resp.Group
return nil
}

return fmt.Errorf("Resource Group (%s) not found", rs.Primary.ID)
}
}

func testAccCheckAWSResourceGroupDestroy(s *terraform.State) error {
for _, rs := range s.RootModule().Resources {
if rs.Type != "aws_waf_rule_group" {
if rs.Type != "aws_resourcegroups_group" {
continue
}

Expand All @@ -124,7 +166,20 @@ func testAccCheckAWSResourceGroupDestroy(s *terraform.State) error {
return nil
}

func testAccAWSResourceGroupConfig_basic(rName string, desc string, query string) string {
const testAccAWSResourceGroupConfigQuery = `{
"ResourceTypeFilters": [
"AWS::EC2::Instance"
],
"TagFilters": [
{
"Key": "Stage",
"Values": ["Test"]
}
]
}
`

func testAccAWSResourceGroupConfig_basic(rName, desc, query string) string {
return fmt.Sprintf(`
resource "aws_resourcegroups_group" "test" {
name = "%s"
Expand All @@ -138,3 +193,42 @@ JSON
}
`, rName, desc, query)
}

func testAccAWSResourceGroupConfigTags1(rName, desc, query, tag1Key, tag1Value string) string {
return fmt.Sprintf(`
resource "aws_resourcegroups_group" "test" {
name = "%s"
description = "%s"

resource_query {
query = <<JSON
%s
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We prefer to hardcode values like these into the test configurations so they can more easily be copy-pasted for manual Terraform CLI usage.

JSON
}

tags = {
%q = %q
}
}
`, rName, desc, query, tag1Key, tag1Value)
}

func testAccAWSResourceGroupConfigTags2(rName, desc, query, tag1Key, tag1Value, tag2Key, tag2Value string) string {
return fmt.Sprintf(`
resource "aws_resourcegroups_group" "test" {
name = "%s"
description = "%s"

resource_query {
query = <<JSON
%s
JSON
}

tags = {
%q = %q
%q = %q
}
}
`, rName, desc, query, tag1Key, tag1Value, tag2Key, tag2Value)
}
1 change: 1 addition & 0 deletions website/docs/r/resourcegroups_group.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ The following arguments are supported:
* `name` - (Required) The resource group's name. A resource group name can have a maximum of 127 characters, including letters, numbers, hyphens, dots, and underscores. The name cannot start with `AWS` or `aws`.
* `description` - (Optional) A description of the resource group.
* `resource_query` - (Required) A `resource_query` block. Resource queries are documented below.
* `tags` - (Optional) Key-value mapping of resource tags

An `resource_query` block supports the following arguments:

Expand Down