New resource: aws cloudwatch query definition #17899

tlkamp · 2021-03-03T15:39:14Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSCloudWatchQueryDefinition'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSCloudWatchQueryDefinition -timeout 120m
=== RUN   TestAccAWSCloudWatchQueryDefinition_basic
=== PAUSE TestAccAWSCloudWatchQueryDefinition_basic
=== RUN   TestAccAWSCloudWatchQueryDefinition_disappears
=== PAUSE TestAccAWSCloudWatchQueryDefinition_disappears
=== RUN   TestAccAWSCloudWatchQueryDefinition_update
=== PAUSE TestAccAWSCloudWatchQueryDefinition_update
=== CONT  TestAccAWSCloudWatchQueryDefinition_basic
=== CONT  TestAccAWSCloudWatchQueryDefinition_update
=== CONT  TestAccAWSCloudWatchQueryDefinition_disappears
--- PASS: TestAccAWSCloudWatchQueryDefinition_disappears (22.34s)
--- PASS: TestAccAWSCloudWatchQueryDefinition_basic (27.95s)
--- PASS: TestAccAWSCloudWatchQueryDefinition_update (44.00s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       46.095s

…nition resource resolves hashicorp#13223

…ead with multiple results

… read

…h convention

…y name

…efinitions

…iable names, comments

…functions

… new resource

github-actions

Welcome @tlkamp 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

…ition

gdavison

Thanks for the PR, @tlkamp. I've made a number of updates to the PR, based on my comments included in the review.

Acceptance test results in Commercial partition

--- PASS: TestAccAWSCloudWatchQueryDefinition_disappears (44.26s)
--- PASS: TestAccAWSCloudWatchQueryDefinition_basic (56.71s)
--- PASS: TestAccAWSCloudWatchQueryDefinition_Rename (97.73s)
--- PASS: TestAccAWSCloudWatchQueryDefinition_LogGroups (100.81s)

Acceptance test results in GovCloud

--- PASS: TestAccAWSCloudWatchQueryDefinition_disappears (38.51s)
--- PASS: TestAccAWSCloudWatchQueryDefinition_basic (52.08s)
--- PASS: TestAccAWSCloudWatchQueryDefinition_Rename (75.55s)
--- PASS: TestAccAWSCloudWatchQueryDefinition_LogGroups (78.67s)

gdavison · 2021-03-29T21:00:59Z

aws/resource_aws_cloudwatch_query_definition.go

+				Type:     schema.TypeList,
+				Optional: true,
+				Elem: &schema.Schema{
+					Type: schema.TypeString,


Log group names have a particular format, so we can add ValidateFunc: validateLogGroupName so that validation can be done at plan time

gdavison · 2021-03-29T21:02:27Z

aws/resource_aws_cloudwatch_query_definition.go

+				Type:     schema.TypeString,
+				Required: true,
+			},
+			"query": {


To more closely match the API, we should call this query_string

gdavison · 2021-03-29T21:02:41Z

aws/resource_aws_cloudwatch_query_definition.go

+				Type:     schema.TypeString,
+				Computed: true,
+			},
+			"log_groups": {


To more closely match the API, we should call this log_group_names

gdavison · 2021-03-29T21:08:45Z

aws/resource_aws_cloudwatch_query_definition.go

+	if err := d.Set("query_definition_id", aws.StringValue(r.QueryDefinitionId)); err != nil {
+		return diag.FromErr(err)
+	}


Checking for errors on Set() is only necessary when setting TypeList and TypeMap values

Suggested change

if err := d.Set("query_definition_id", aws.StringValue(r.QueryDefinitionId)); err != nil {

return diag.FromErr(err)

}

d.Set("query_definition_id", aws.StringValue(r.QueryDefinitionId))

gdavison · 2021-03-29T21:14:06Z

aws/resource_aws_cloudwatch_query_definition.go

+	logGroups := d.Get("log_groups").([]interface{})
+	var lgs []*string
+
+	for _, group := range logGroups {
+		l := group.(string)
+		lgs = append(lgs, aws.String(l))
+	}
+
+	query := d.Get("query").(string)
+	return &cloudwatchlogs.PutQueryDefinitionInput{
+		Name:          aws.String(name),
+		LogGroupNames: lgs,
+		QueryString:   aws.String(query),
+	}


The function expandStringList() can handle the conversion from []interface{} to []*string

Suggested change

logGroups := d.Get("log_groups").([]interface{})

var lgs []*string

for _, group := range logGroups {

l := group.(string)

lgs = append(lgs, aws.String(l))

}

query := d.Get("query").(string)

return &cloudwatchlogs.PutQueryDefinitionInput{

Name: aws.String(name),

LogGroupNames: lgs,

QueryString: aws.String(query),

}

logGroups := d.Get("log_groups").([]interface{})

query := d.Get("query").(string)

return &cloudwatchlogs.PutQueryDefinitionInput{

Name: aws.String(name),

LogGroupNames: expandStringList(logGroups),

QueryString: aws.String(query),

}

gdavison · 2021-03-29T21:36:45Z

aws/resource_aws_cloudwatch_query_definition_test.go

+				Config: testAccAWSCloudWatchQueryDefinitionConfig(queryName, ident),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSCloudWatchQueryDefinitionExists(resourceName, queryName),
+					testAccCheckAWSCloudWatchQueryDefinitionDisappears(resourceName),


We now have a generic function testAccCheckResourceDisappears() that can be used

Suggested change

testAccCheckAWSCloudWatchQueryDefinitionDisappears(resourceName),

testAccCheckResourceDisappears(testAccProvider, resourceAwsCloudWatchQueryDefinition(), resourceName),

gdavison · 2021-03-29T21:42:56Z

aws/resource_aws_cloudwatch_query_definition_test.go

+
+func testAccAWSCloudWatchQueryDefinitionConfig(name, ident string) string {
+	return fmt.Sprintf(`
+resource "aws_cloudwatch_query_definition" "%s" {


Typically, for simplicity, we name the resource "test" in all cases

Suggested change

resource "aws_cloudwatch_query_definition" "%s" {

resource "aws_cloudwatch_query_definition" "test" {

gdavison · 2021-03-29T21:45:20Z

aws/resource_aws_cloudwatch_query_definition_test.go

+		CheckDestroy:      testAccCheckAWSCloudWatchQueryDefinitionDestroy(resourceName, queryName),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSCloudWatchQueryDefinitionConfig(queryName, ident),


For <resource>_basic test cases, we typically don't include optional fields. In this case, we would have a testAccAWSCloudWatchQueryDefinitionConfig_Basic function that doesn't define a value for log_groups and the test would confirm that no log groups were set.

We would also want another test case that validates that log_groups is set when there are values in the parameter.

gdavison · 2021-03-29T21:48:08Z

aws/resource_aws_cloudwatch_query_definition_test.go

+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"last_modified"},


This can be removed, since the field last_modified is not defined on aws_cloudwatch_query_definition

gdavison · 2021-03-29T22:26:55Z

aws/resource_aws_cloudwatch_query_definition.go

+		Importer: &schema.ResourceImporter{
+			StateContext: resourceAwsCloudWatchQueryDefinitionImport,
+		},
+		SchemaVersion: 1,


SchemaVersion should not be set for new resources

…uery_definition

tlkamp · 2021-03-31T14:54:21Z

@gdavison thanks for the review! My apologies on missing your comments until recently.

ghost · 2021-04-01T22:54:12Z

This has been released in version 3.35.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2021-04-30T17:10:37Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

Traci Kamp added 19 commits February 25, 2021 16:50

feat(aws_cloudwatch_query_definition): adds aws_cloudwatch_query_defi…

4c258f6

…nition resource resolves hashicorp#13223

chore(.gitignore): ignore the binary

51161fb

refactor(aws_cloudwatch_query_definition): handle the disappears case

b39e2eb

style(fmt): run go fmt

0781a6e

test(aws_cloudwatch_query_definition): add basic acceptance test

5528826

refactor(aws_cloudwatch_query_definition): implement import and fix r…

9c4b012

…ead with multiple results

refactor(aws_cloudwatch_query_definition): set query_definition_id in…

84e2097

… read

test(aws_cloudwatch_query_definition): include test for import

f21ad4d

refactor(aws_cloudwatch_query_defintion): make provider function matc…

f415d4b

…h convention

test(aws_cloudwatch_query_definition): test disappears case

2144f2a

test(aws_cloudwatch_query_definition): test update

96657fd

refactor(aws_cloudwatch_query_definition): handle underscores in quer…

d62d0f0

…y name

refactor(aws_cloudwatch_query_definition): use aws.String func

203abdb

refactor(cloudwatchlogs): generates page functions for DescribeQueryD…

b2fe015

…efinitions

refactor(aws_cloudwatch_query_definition): better error handling, var…

13bde98

…iable names, comments

refactor(aws_cloudwatch_query_definition): uses the generated paging …

277ec2e

…functions

style(finder): formats finder.go

d946107

test(aws_cloudwatch_query_definition): tests now use finder functions

53df017

docs(aws_cloudwatch_query_definition): adds website documentation for…

0373da6

… new resource

tlkamp requested a review from a team as a code owner March 3, 2021 15:39

github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Mar 3, 2021

github-actions bot reviewed Mar 3, 2021

View reviewed changes

Traci Kamp added 2 commits March 3, 2021 09:00

refactor(aws_cloudwatch_query_definition): uses aws.StringValue function

de3b9eb

chore(changelog): adds changelog entry for aws_cloudwatch_query_defin…

39b3d3e

…ition

refactor(aws_cloudwatch_query_definition): fixes importlint issues

b7939b8

gdavison self-assigned this Mar 29, 2021

gdavison approved these changes Mar 30, 2021

View reviewed changes

gdavison added a commit that referenced this pull request Mar 30, 2021

Merge pull request #17899 from StateFarmIns/resource/aws_cloudwatch_q…

1aedfc3

…uery_definition

gdavison merged commit d330df0 into hashicorp:main Mar 30, 2021

github-actions bot added this to the v3.35.0 milestone Mar 30, 2021

gdavison mentioned this pull request Mar 31, 2021

resource/aws_cloudwatch_query_definition: Add sweeper #18513

Closed

ghost locked as resolved and limited conversation to collaborators Apr 30, 2021

breathingdust removed the needs-triage Waiting for first response or review from a maintainer. label Sep 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New resource: aws cloudwatch query definition #17899

New resource: aws cloudwatch query definition #17899

tlkamp commented Mar 3, 2021

github-actions bot left a comment

gdavison left a comment

gdavison Mar 29, 2021

gdavison Mar 29, 2021

gdavison Mar 29, 2021

gdavison Mar 29, 2021

gdavison Mar 29, 2021

gdavison Mar 29, 2021

gdavison Mar 29, 2021

gdavison Mar 29, 2021

gdavison Mar 29, 2021

gdavison Mar 29, 2021

tlkamp commented Mar 31, 2021

ghost commented Apr 1, 2021

ghost commented Apr 30, 2021

	testAccCheckAWSCloudWatchQueryDefinitionDisappears(resourceName),
	testAccCheckResourceDisappears(testAccProvider, resourceAwsCloudWatchQueryDefinition(), resourceName),

	resource "aws_cloudwatch_query_definition" "%s" {
	resource "aws_cloudwatch_query_definition" "test" {

New resource: aws cloudwatch query definition #17899

New resource: aws cloudwatch query definition #17899

Conversation

tlkamp commented Mar 3, 2021

Community Note

github-actions bot left a comment

Choose a reason for hiding this comment

gdavison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlkamp commented Mar 31, 2021

ghost commented Apr 1, 2021

ghost commented Apr 30, 2021