From 9cec40ea3cdac35f4dde5453cfa7dc674f437c8b Mon Sep 17 00:00:00 2001 From: Paul Forman Date: Sun, 22 Nov 2015 12:54:11 -0700 Subject: [PATCH] Add missing error-checks from code review Some error-checking was omitted. Specifically, the cloudTrailSetLogging call in the Create function was ignoring the return and cloudTrailGetLoggingStatus could crash on a nil-dereference during the return. Fixed both. Fixed some needless casting in cloudTrailGetLoggingStatus. Clarified error message in acceptance tests. Removed needless option from example in docs. --- builtin/providers/aws/resource_aws_cloudtrail.go | 14 ++++++++++---- .../providers/aws/resource_aws_cloudtrail_test.go | 2 +- .../docs/providers/aws/r/cloudtrail.html.markdown | 1 - 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/builtin/providers/aws/resource_aws_cloudtrail.go b/builtin/providers/aws/resource_aws_cloudtrail.go index 303073422..53def2fb1 100644 --- a/builtin/providers/aws/resource_aws_cloudtrail.go +++ b/builtin/providers/aws/resource_aws_cloudtrail.go @@ -91,7 +91,10 @@ func resourceAwsCloudTrailCreate(d *schema.ResourceData, meta interface{}) error // AWS CloudTrail sets newly-created trails to false. if v, ok := d.GetOk("enable_logging"); ok && v.(bool) { - cloudTrailSetLogging(conn, v.(bool), d.Id()) + err := cloudTrailSetLogging(conn, v.(bool), d.Id()) + if err != nil { + return err + } } return resourceAwsCloudTrailRead(d, meta) @@ -125,7 +128,7 @@ func resourceAwsCloudTrailRead(d *schema.ResourceData, meta interface{}) error { d.Set("include_global_service_events", trail.IncludeGlobalServiceEvents) d.Set("sns_topic_name", trail.SnsTopicName) - logstatus, err := cloudTrailGetLoggingStatus(conn, *trail.Name) + logstatus, err := cloudTrailGetLoggingStatus(conn, trail.Name) if err != nil { return err } @@ -191,11 +194,14 @@ func resourceAwsCloudTrailDelete(d *schema.ResourceData, meta interface{}) error return err } -func cloudTrailGetLoggingStatus(conn *cloudtrail.CloudTrail, id string) (bool, error) { +func cloudTrailGetLoggingStatus(conn *cloudtrail.CloudTrail, id *string) (bool, error) { GetTrailStatusOpts := &cloudtrail.GetTrailStatusInput{ - Name: aws.String(id), + Name: id, } resp, err := conn.GetTrailStatus(GetTrailStatusOpts) + if err != nil { + return false, fmt.Errorf("Error retrieving logging status of CloudTrail (%s): %s", id, err) + } return *resp.IsLogging, err } diff --git a/builtin/providers/aws/resource_aws_cloudtrail_test.go b/builtin/providers/aws/resource_aws_cloudtrail_test.go index 2d3e807c6..c276135ce 100644 --- a/builtin/providers/aws/resource_aws_cloudtrail_test.go +++ b/builtin/providers/aws/resource_aws_cloudtrail_test.go @@ -115,7 +115,7 @@ func testAccCheckCloudTrailLoggingEnabled(n string, desired bool, trail *cloudtr return err } if *resp.IsLogging != desired { - return fmt.Errorf("Logging status is incorrect") + return fmt.Errorf("Expected logging status %t, given %t", desired, *resp.IsLogging) } return nil diff --git a/website/source/docs/providers/aws/r/cloudtrail.html.markdown b/website/source/docs/providers/aws/r/cloudtrail.html.markdown index e63a22dd2..6bffee09e 100644 --- a/website/source/docs/providers/aws/r/cloudtrail.html.markdown +++ b/website/source/docs/providers/aws/r/cloudtrail.html.markdown @@ -16,7 +16,6 @@ resource "aws_cloudtrail" "foobar" { name = "tf-trail-foobar" s3_bucket_name = "${aws_s3_bucket.foo.id}" s3_key_prefix = "/prefix" - enable_logging = true include_global_service_events = false }