r/cloudtrail: Add support CloudTrail EventSelector #2258

kjmkznr · 2017-11-12T13:03:04Z

Closes #887

Add support event_selector parameter into aws_cloudtrail resource.
This feature supports settings to Amazon S3 object-level API operations logging.

AWS Documentations

Acceptance Test

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCloudTrail'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCloudTrail -timeout 120m
=== RUN   TestAccAWSCloudTrailServiceAccount_basic
--- PASS: TestAccAWSCloudTrailServiceAccount_basic (48.50s)
=== RUN   TestAccAWSCloudTrail_importBasic
--- PASS: TestAccAWSCloudTrail_importBasic (103.48s)
=== RUN   TestAccAWSCloudTrail
=== RUN   TestAccAWSCloudTrail/Trail
=== RUN   TestAccAWSCloudTrail/Trail/isMultiRegion
=== RUN   TestAccAWSCloudTrail/Trail/logValidation
=== RUN   TestAccAWSCloudTrail/Trail/kmsKey
=== RUN   TestAccAWSCloudTrail/Trail/tags
=== RUN   TestAccAWSCloudTrail/Trail/basic
=== RUN   TestAccAWSCloudTrail/Trail/enableLogging
=== RUN   TestAccAWSCloudTrail/Trail/eventSelector
=== RUN   TestAccAWSCloudTrail/Trail/cloudwatch
--- PASS: TestAccAWSCloudTrail (1317.96s)
    --- PASS: TestAccAWSCloudTrail/Trail (1317.96s)
        --- PASS: TestAccAWSCloudTrail/Trail/isMultiRegion (212.59s)
        --- PASS: TestAccAWSCloudTrail/Trail/logValidation (142.34s)
        --- PASS: TestAccAWSCloudTrail/Trail/kmsKey (111.42s)
        --- PASS: TestAccAWSCloudTrail/Trail/tags (204.39s)
        --- PASS: TestAccAWSCloudTrail/Trail/basic (169.49s)
        --- PASS: TestAccAWSCloudTrail/Trail/enableLogging (197.31s)
        --- PASS: TestAccAWSCloudTrail/Trail/eventSelector (129.95s)
        --- PASS: TestAccAWSCloudTrail/Trail/cloudwatch (150.49s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       1469.950s

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCloudTrail_importBasic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCloudTrail_importBasic -timeout 120m
=== RUN   TestAccAWSCloudTrail_importBasic
--- PASS: TestAccAWSCloudTrail_importBasic (94.65s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       94.665s

kjmkznr · 2017-12-10T04:23:13Z

Link #887

bsx · 2017-12-10T10:20:48Z

aws/resource_aws_cloudtrail.go

+									"type": &schema.Schema{
+										Type:         schema.TypeString,
+										Required:     true,
+										ValidateFunc: validation.StringInSlice([]string{"AWS::S3::Object"}, false),


CloudTrail as of recently also supports Lambda, so you might want to add "AWS::Lambda::Function" to the list of allowed types.

Thanks comment!
I added Lambda.

antonbabenko · 2018-01-08T12:17:42Z

What else should be done on this to get it into?

bflad

@kjmkznr thanks so much for contributing this and sorry its taken so long to get a maintainer review. Please take a look at the feedback and let me know if you have any questions or when this can be reviewed again. Overall great job and we'll get this shipped soon! 😄

bflad · 2018-02-06T01:53:10Z

aws/resource_aws_cloudtrail.go

@@ -72,6 +73,46 @@ func resourceAwsCloudTrail() *schema.Resource {
 				Optional:     true,
 				ValidateFunc: validateArn,
 			},
+			"event_selector": &schema.Schema{


Nitpick: the &schema.Schema for the attributes are unnecessary since Go 1.7 (except for the one with Elem: &schema.Schema below)

bflad · 2018-02-06T01:55:42Z

aws/resource_aws_cloudtrail.go

+						"read_write_type": &schema.Schema{
+							Type:         schema.TypeString,
+							Required:     true,
+							ValidateFunc: validation.StringInSlice([]string{"All", "ReadOnly", "WriteOnly"}, false),


Nitpick: The AWS SDK provides constants for all of these:

"All": cloudtrail.ReadWriteTypeAll

"ReadOnly": cloudtrail.ReadWriteTypeReadOnly

"WriteOnly": cloudtrail.ReadWriteTypeWriteOnly

bflad · 2018-02-06T02:01:36Z

aws/resource_aws_cloudtrail.go

+					Schema: map[string]*schema.Schema{
+						"read_write_type": &schema.Schema{
+							Type:         schema.TypeString,
+							Required:     true,


This attribute should be:

Optional: true, Default: cloudtrail.ReadWriteTypeAll,

To match the CloudTrail API documentation

bflad · 2018-02-06T02:06:13Z

aws/resource_aws_cloudtrail.go

+		return err
+	}
+
+	d.Set("event_selector", flattenAwsCloudTrailEventSelector(eventSelectorsOut.EventSelectors))


We should check and return for errors here since it is not a simple string attribute:

if err := d.Set("event_selector", flattenAwsCloudTrailEventSelector(eventSelectorsOut.EventSelectors)); err != nil { return err }

bflad · 2018-02-06T02:09:56Z

aws/resource_aws_cloudtrail.go

@@ -300,6 +358,13 @@ func resourceAwsCloudTrailUpdate(d *schema.ResourceData, meta interface{}) error
 		}
 	}

+	if d.HasChange("event_selector") {


d.HasChange here will return true on resource creation (as resourceAwsCloudTrailCreate calls resourceAwsCloudTrailUpdate), so that means PutEventSelectors is called twice. You can use !d.IsNewResource() && d.HasChange("event_selector") to prevent that 👍

bflad · 2018-02-06T02:20:59Z

website/docs/r/cloudtrail.html.markdown

+#### Data Resource Arguments
+For **data_resource** the following attributes are supported.
+
+* `type` (Required) - The resource type in witch you want to log data events. You can specify only the follwing value: "AWS::S3::Object"


We support "AWS::Lambda::Function" in the validation function as well

bflad · 2018-02-06T02:22:38Z

website/docs/r/cloudtrail.html.markdown

+For **data_resource** the following attributes are supported.
+
+* `type` (Required) - The resource type in witch you want to log data events. You can specify only the follwing value: "AWS::S3::Object"
+* `values` (Required) - A list of ARN for the specified S3 buckets and object prefies..


Typo: prefixes

bflad · 2018-02-06T02:29:03Z

aws/resource_aws_cloudtrail_test.go

+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.#", "1"),
+					resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.#", "1"),
+				),


This should check the other event_selector attributes as well:

resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.0.type", "AWS::S3::Object") resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.0.values.#", "2") resource.TestMatchResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.0.values.0", regexp.MustCompile(`^arn:[^:]+:s3:::.+/foobar$`)) resource.TestMatchResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.0.values.1", regexp.MustCompile(`^arn:[^:]+:s3:::.+/baz$`)) resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.include_management_events", "false"), resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.read_write_type", "ReadOnly"),

bflad · 2018-02-06T02:34:06Z

aws/resource_aws_cloudtrail.go

+						},
+
+						"data_resource": &schema.Schema{
+							Type:     schema.TypeSet,


This might be easier to work with as a schema.TypeList (especially for acceptance testing) unless there's a good reason to keep it a set.

bflad · 2018-02-06T02:39:12Z

aws/resource_aws_cloudtrail_test.go

+					resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.#", "2"),
+					resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.#", "1"),
+					resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.1.data_resource.#", "2"),
+				),


This should check the other event_selector attributes as well:

resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.0.type", "AWS::S3::Object"), resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.0.values.#", "2"), resource.TestMatchResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.0.values.0", regexp.MustCompile(`^arn:[^:]+:s3:::.+/foobar$`)), resource.TestMatchResourceAttr("aws_cloudtrail.foobar", "event_selector.0.data_resource.0.values.1", regexp.MustCompile(`^arn:[^:]+:s3:::.+/baz$`)), resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.include_management_events", "true"), resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.0.read_write_type", "ReadOnly"), resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.1.data_resource.0.type", "AWS::S3::Object") resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.1.data_resource.0.values.#", "1"), resource.TestMatchResourceAttr("aws_cloudtrail.foobar", "event_selector.1.data_resource.0.values.0", regexp.MustCompile(`^arn:[^:]+:s3:::.+/tf1$`)), resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.1.data_resource.1.type", "AWS::S3::Object"), resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.1.data_resource.1.values.#", "1"), resource.TestMatchResourceAttr("aws_cloudtrail.foobar", "event_selector.1.data_resource.1.values.0", regexp.MustCompile(`^arn:[^:]+:s3:::.+/tf2$`)), resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.1.include_management_events", "false"), resource.TestCheckResourceAttr("aws_cloudtrail.foobar", "event_selector.1.read_write_type", "All"),

kjmkznr · 2018-02-13T12:51:52Z

Thanks for reviews!
I will fixes in this week.

kjmkznr · 2018-02-17T10:58:48Z

@bflad
Thank you for the reviews.
I fixed all review points.

bflad

Thanks so much @kjmkznr for your work here! LGTM! 🚀

=== RUN   TestAccAWSCloudTrailServiceAccount_basic
--- PASS: TestAccAWSCloudTrailServiceAccount_basic (10.50s)
=== RUN   TestAccAWSCloudTrail_include_global_service_events
--- PASS: TestAccAWSCloudTrail_include_global_service_events (18.55s)
=== RUN   TestAccAWSCloudTrail_importBasic
--- PASS: TestAccAWSCloudTrail_importBasic (19.74s)
=== RUN   TestAccAWSCloudTrail
=== RUN   TestAccAWSCloudTrail/Trail
=== RUN   TestAccAWSCloudTrail/Trail/basic
=== RUN   TestAccAWSCloudTrail/Trail/cloudwatch
=== RUN   TestAccAWSCloudTrail/Trail/enableLogging
=== RUN   TestAccAWSCloudTrail/Trail/tags
=== RUN   TestAccAWSCloudTrail/Trail/eventSelector
=== RUN   TestAccAWSCloudTrail/Trail/isMultiRegion
=== RUN   TestAccAWSCloudTrail/Trail/logValidation
=== RUN   TestAccAWSCloudTrail/Trail/kmsKey
--- PASS: TestAccAWSCloudTrail (257.44s)

kjmkznr · 2018-02-20T02:02:42Z

Thanks!

bflad · 2018-02-27T12:47:52Z

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

ghost · 2020-04-07T17:53:56Z

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!

radeksimko added enhancement Requests to existing resources that expand the functionality or scope. size/L Managed by automation to categorize the size of a PR. labels Nov 13, 2017

bsx reviewed Dec 10, 2017

View reviewed changes

radeksimko added the service/cloudtrail Issues and PRs that pertain to the cloudtrail service. label Jan 12, 2018

bflad mentioned this pull request Jan 16, 2018

Feature Request: Support CloudTrail EventSelectors #887

Closed

bflad suggested changes Feb 6, 2018

View reviewed changes

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Feb 6, 2018

kjmkznr added 5 commits February 13, 2018 12:08

r/cloudtrail: Add support CloudTrail EventSelector

0a5a58c

r/cloudtrail: Add supports lambda to event selector

96c0fad

r/cloudtrail: Remove &schema.Schema declarations

1b46292

r/cloudtrail: Changed to constant values

d225552

r/cloudtrail: Changed to optional attribute read_write_type

ca2f8ee

kjmkznr added 9 commits February 17, 2018 06:48

r/cloudtrail: Check and return error

7848490

r/cloudtrail: Prevent twice call PutEventSelectors when create resource

75b38e5

r/cloudtrail: Do not use setter

cb1f87f

website/cloudtrail: Fix missing value

02f7276

website/cloudtrail: Fix typo

9e468b1

r/cloudtrail: Add more check attributes

544fb81

r/cloudtrail: Change data_resource type to TypeList

3df08f0

r/cloudtrail: Add test for AWS::Lambda::Function data_resource

28d11a9

r/cloudtrail: Prevent default configurations show difference

1ac5f16

kjmkznr force-pushed the f-aws-cloudtrail-event-selector-param branch from e101c27 to 1ac5f16 Compare February 17, 2018 10:01

ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 17, 2018

bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label Feb 19, 2018

bflad approved these changes Feb 19, 2018

View reviewed changes

bflad added this to the v1.10.0 milestone Feb 19, 2018

bflad merged commit 9b6dff6 into hashicorp:master Feb 19, 2018

bflad mentioned this pull request Feb 19, 2018

Support object-level events on s3 buckets #3425

Closed

bflad added a commit that referenced this pull request Feb 19, 2018

Update CHANGELOG for #2258

23f0892

kjmkznr deleted the f-aws-cloudtrail-event-selector-param branch February 20, 2018 02:03

ryandeivert mentioned this pull request Mar 21, 2018

[tf] fix to address terraform support for cloudtrail event_selector airbnb/streamalert#644

Merged

ghost locked and limited conversation to collaborators Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

r/cloudtrail: Add support CloudTrail EventSelector #2258

r/cloudtrail: Add support CloudTrail EventSelector #2258

kjmkznr commented Nov 12, 2017 •

edited by bflad

Loading

kjmkznr commented Dec 10, 2017

bsx Dec 10, 2017

kjmkznr Dec 13, 2017

antonbabenko commented Jan 8, 2018

bflad left a comment

bflad Feb 6, 2018

bflad Feb 6, 2018

bflad Feb 6, 2018

bflad Feb 6, 2018

bflad Feb 6, 2018

bflad Feb 6, 2018

bflad Feb 6, 2018

bflad Feb 6, 2018

bflad Feb 6, 2018

bflad Feb 6, 2018

kjmkznr commented Feb 13, 2018

kjmkznr commented Feb 17, 2018

bflad left a comment

kjmkznr commented Feb 20, 2018

bflad commented Feb 27, 2018

ghost commented Apr 7, 2020

r/cloudtrail: Add support CloudTrail EventSelector #2258

r/cloudtrail: Add support CloudTrail EventSelector #2258

Conversation

kjmkznr commented Nov 12, 2017 • edited by bflad Loading

AWS Documentations

Acceptance Test

kjmkznr commented Dec 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonbabenko commented Jan 8, 2018

bflad 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

kjmkznr commented Feb 13, 2018

kjmkznr commented Feb 17, 2018

bflad left a comment

Choose a reason for hiding this comment

kjmkznr commented Feb 20, 2018

bflad commented Feb 27, 2018

ghost commented Apr 7, 2020

kjmkznr commented Nov 12, 2017 •

edited by bflad

Loading