Skip to content

Commit

Permalink
resource/aws_cloudwatch_log_group: Automatically trim :* suffix from …
Browse files Browse the repository at this point in the history
…ARN in API response (#14214)

* resource/aws_cloudwatch_log_group: Automatically trim :* suffix from ARN in API response

Reference: #13046
Reference: #13509

Previously:

```
    TestAccAWSDataSyncTask_CloudWatchLogGroupARN: testing.go:684: Step 0 error: errors during apply:

        Error: error creating DataSync Task: ValidationException: 1 validation error detected: Value 'arn:aws:logs:us-west-2:123456789012:log-group:tf-acc-test-4735468151095290255:*' at 'cloudWatchLogGroupArn' failed to satisfy constraint: Member must satisfy regular expression pattern: ^arn:(aws|aws-cn|aws-us-gov|aws-iso|aws-iso-b):logs:[a-z\-0-9]*:[0-9]{12}:log-group:([^:\*]*)$
```

Output from acceptance testing (`aws_route53_query_log` failure related to similar issue #13510):

```
--- PASS: TestAccAWSCloudWatchLogGroup_disappears (9.19s)
--- PASS: TestAccAWSCloudWatchLogGroup_namePrefix (13.55s)
--- PASS: TestAccAWSCloudWatchLogGroup_generatedName (13.99s)
--- PASS: TestAccAWSCloudWatchLogGroup_basic (15.24s)
--- PASS: TestAccAWSCloudWatchLogGroup_multiple (15.65s)
--- PASS: TestAccAWSCloudWatchLogGroup_namePrefix_retention (21.29s)
--- PASS: TestAccAWSCloudWatchLogGroup_retentionPolicy (24.99s)
--- PASS: TestAccAWSCloudWatchLogGroup_kmsKey (29.00s)
--- PASS: TestAccAWSCloudWatchLogGroup_tagging (35.60s)

--- PASS: TestAccAWSAPIGatewayStage_accessLogSettings (225.36s)
--- PASS: TestAccAWSAPIGatewayStage_accessLogSettings_kinesis (332.67s)

--- PASS: TestAccAWSAPIGatewayV2Stage_AccessLogSettings (56.73s)

--- PASS: TestAccAWSDataSyncTask_CloudWatchLogGroupARN (304.98s)

--- PASS: TestAccAWSDirectoryServiceLogSubscription_basic (1764.25s)

--- PASS: TestAccAWSElasticSearchDomain_LogPublishingOptions (688.17s)

--- PASS: TestAccAWSFlowLog_LogDestinationType_CloudWatchLogs (26.43s)

--- FAIL: TestAccAWSRoute53QueryLog_Basic (42.80s)
    TestAccAWSRoute53QueryLog_Basic: testing.go:684: Step 0 error: errors during apply:

        Error: Provider produced inconsistent final plan

        When expanding the plan for aws_cloudwatch_log_group.test to include new
        values learned so far during apply, provider "aws" produced an invalid new
        value for .name: was
        cty.StringVal("/aws/route53/testaccawsroute53querylog_basic-rsbvm.com"), but
        now cty.StringVal("/aws/route53/testaccawsroute53querylog_basic-rsbvm.com.").

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

--- PASS: TestAccAWSStorageGatewayGateway_CloudWatchLogs (220.06s)
```

* docs/resource/aws_cloudwatch_log_group: Fix typo
  • Loading branch information
bflad authored Jul 30, 2020
1 parent 7cb6299 commit 9fb4164
Show file tree
Hide file tree
Showing 12 changed files with 80 additions and 48 deletions.
11 changes: 3 additions & 8 deletions aws/resource_aws_api_gateway_stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ func resourceAwsApiGatewayStage() *schema.Resource {
"destination_arn": {
Type: schema.TypeString,
Required: true,
StateFunc: func(arn interface{}) string {
// arns coming from a TF reference to a log group contain a trailing `:*` which is not valid
return strings.TrimSuffix(arn.(string), ":*")
},
},
"format": {
Type: schema.TypeString,
Expand Down Expand Up @@ -356,10 +352,9 @@ func resourceAwsApiGatewayStageUpdate(d *schema.ResourceData, meta interface{})
if len(accessLogSettings) == 1 {
operations = append(operations,
&apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/accessLogSettings/destinationArn"),
// arns coming from a TF reference to a log group contain a trailing `:*` which is not valid
Value: aws.String(strings.TrimSuffix(d.Get("access_log_settings.0.destination_arn").(string), ":*")),
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/accessLogSettings/destinationArn"),
Value: aws.String(d.Get("access_log_settings.0.destination_arn").(string)),
}, &apigateway.PatchOperation{
Op: aws.String(apigateway.OpReplace),
Path: aws.String("/accessLogSettings/format"),
Expand Down
9 changes: 5 additions & 4 deletions aws/resource_aws_api_gateway_stage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ func TestAccAWSAPIGatewayStage_disappears(t *testing.T) {
func TestAccAWSAPIGatewayStage_accessLogSettings(t *testing.T) {
var conf apigateway.Stage
rName := acctest.RandString(5)
cloudwatchLogGroupResourceName := "aws_cloudwatch_log_group.test"
resourceName := "aws_api_gateway_stage.test"
clf := `$context.identity.sourceIp $context.identity.caller $context.identity.user [$context.requestTime] "$context.httpMethod $context.resourcePath $context.protocol" $context.status $context.responseLength $context.requestId`
json := `{ "requestId":"$context.requestId", "ip": "$context.identity.sourceIp", "caller":"$context.identity.caller", "user":"$context.identity.user", "requestTime":"$context.requestTime", "httpMethod":"$context.httpMethod", "resourcePath":"$context.resourcePath", "status":"$context.status", "protocol":"$context.protocol", "responseLength":"$context.responseLength" }`
Expand All @@ -146,7 +147,7 @@ func TestAccAWSAPIGatewayStage_accessLogSettings(t *testing.T) {
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
testAccMatchResourceAttrRegionalARN(resourceName, "access_log_settings.0.destination_arn", "logs", regexp.MustCompile(fmt.Sprintf("log-group:foo-bar-%s$", rName))),
resource.TestCheckResourceAttrPair(resourceName, "access_log_settings.0.destination_arn", cloudwatchLogGroupResourceName, "arn"),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", clf),
),
},
Expand All @@ -157,7 +158,7 @@ func TestAccAWSAPIGatewayStage_accessLogSettings(t *testing.T) {
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
testAccMatchResourceAttrRegionalARN(resourceName, "access_log_settings.0.destination_arn", "logs", regexp.MustCompile(fmt.Sprintf("log-group:foo-bar-%s$", rName))),
resource.TestCheckResourceAttrPair(resourceName, "access_log_settings.0.destination_arn", cloudwatchLogGroupResourceName, "arn"),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", json),
),
},
Expand All @@ -167,7 +168,7 @@ func TestAccAWSAPIGatewayStage_accessLogSettings(t *testing.T) {
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
testAccMatchResourceAttrRegionalARN(resourceName, "access_log_settings.0.destination_arn", "logs", regexp.MustCompile(fmt.Sprintf("log-group:foo-bar-%s$", rName))),
resource.TestCheckResourceAttrPair(resourceName, "access_log_settings.0.destination_arn", cloudwatchLogGroupResourceName, "arn"),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", xml),
),
},
Expand All @@ -177,7 +178,7 @@ func TestAccAWSAPIGatewayStage_accessLogSettings(t *testing.T) {
testAccCheckAWSAPIGatewayStageExists(resourceName, &conf),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(`/restapis/.+/stages/prod`)),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
testAccMatchResourceAttrRegionalARN(resourceName, "access_log_settings.0.destination_arn", "logs", regexp.MustCompile(fmt.Sprintf("log-group:foo-bar-%s$", rName))),
resource.TestCheckResourceAttrPair(resourceName, "access_log_settings.0.destination_arn", cloudwatchLogGroupResourceName, "arn"),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", csv),
),
},
Expand Down
3 changes: 0 additions & 3 deletions aws/resource_aws_apigatewayv2_stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ func resourceAwsApiGatewayV2Stage() *schema.Resource {
Type: schema.TypeString,
Required: true,
ValidateFunc: validateArn,
StateFunc: func(v interface{}) string {
return strings.TrimSuffix(v.(string), ":*")
},
},
"format": {
Type: schema.TypeString,
Expand Down
20 changes: 2 additions & 18 deletions aws/resource_aws_apigatewayv2_stage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package aws
import (
"fmt"
"regexp"
"strings"
"testing"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -260,7 +259,7 @@ func TestAccAWSAPIGatewayV2Stage_AccessLogSettings(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayV2StageExists(resourceName, &apiId, &v),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
testAccCheckAWSAPIGatewayV2StageAccessLogDestinationArn(resourceName, "access_log_settings.0.destination_arn", cloudWatchResourceName, "arn"),
resource.TestCheckResourceAttrPair(resourceName, "access_log_settings.0.destination_arn", cloudWatchResourceName, "arn"),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", "$context.identity.sourceIp $context.requestId"),
testAccMatchResourceAttrRegionalARNNoAccount(resourceName, "arn", "apigateway", regexp.MustCompile(fmt.Sprintf("/apis/.+/stages/%s", rName))),
resource.TestCheckResourceAttr(resourceName, "auto_deploy", "false"),
Expand Down Expand Up @@ -292,7 +291,7 @@ func TestAccAWSAPIGatewayV2Stage_AccessLogSettings(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAPIGatewayV2StageExists(resourceName, &apiId, &v),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.#", "1"),
testAccCheckAWSAPIGatewayV2StageAccessLogDestinationArn(resourceName, "access_log_settings.0.destination_arn", cloudWatchResourceName, "arn"),
resource.TestCheckResourceAttrPair(resourceName, "access_log_settings.0.destination_arn", cloudWatchResourceName, "arn"),
resource.TestCheckResourceAttr(resourceName, "access_log_settings.0.format", "$context.requestId"),
resource.TestCheckResourceAttr(resourceName, "auto_deploy", "false"),
resource.TestCheckResourceAttr(resourceName, "client_certificate_id", ""),
Expand Down Expand Up @@ -1042,21 +1041,6 @@ func testAccAWSAPIGatewayV2StageImportStateIdFunc(resourceName string) resource.
}
}

func testAccCheckAWSAPIGatewayV2StageAccessLogDestinationArn(resourceName, resourceKey, cloudWatchResourceName, cloudWatchResourceKey string) resource.TestCheckFunc {
return func(s *terraform.State) error {
cwRs, ok := s.RootModule().Resources[cloudWatchResourceName]
if !ok {
return fmt.Errorf("Resource not found: %s", cloudWatchResourceName)
}
cwArn, ok := cwRs.Primary.Attributes[cloudWatchResourceKey]
if !ok {
return fmt.Errorf("Attribute %q not found in resource %s", cloudWatchResourceKey, cloudWatchResourceName)
}

return resource.TestCheckResourceAttr(resourceName, resourceKey, strings.TrimSuffix(cwArn, ":*"))(s)
}
}

func testAccAWSAPIGatewayV2StageConfig_apiWebSocket(rName string) string {
return fmt.Sprintf(`
resource "aws_apigatewayv2_api" "test" {
Expand Down
5 changes: 2 additions & 3 deletions aws/resource_aws_cloudwatch_log_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package aws
import (
"fmt"
"log"
"strings"

"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
Expand Down Expand Up @@ -131,9 +132,7 @@ func resourceAwsCloudWatchLogGroupRead(d *schema.ResourceData, meta interface{})
return nil
}

log.Printf("[DEBUG] Found Log Group: %#v", *lg)

d.Set("arn", lg.Arn)
d.Set("arn", strings.TrimSuffix(aws.StringValue(lg.Arn), ":*"))
d.Set("name", lg.LogGroupName)
d.Set("kms_key_id", lg.KmsKeyId)
d.Set("retention_in_days", lg.RetentionInDays)
Expand Down
3 changes: 3 additions & 0 deletions aws/resource_aws_cloudwatch_log_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ func TestAccAWSCloudWatchLogGroup_basic(t *testing.T) {
Config: testAccAWSCloudWatchLogGroupConfig(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudWatchLogGroupExists(resourceName, &lg),
testAccCheckResourceAttrRegionalARN(resourceName, "arn", "logs", fmt.Sprintf("log-group:foo-bar-%d", rInt)),
resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("foo-bar-%d", rInt)),
resource.TestCheckResourceAttr(resourceName, "retention_in_days", "0"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
),
},
{
Expand Down
6 changes: 3 additions & 3 deletions aws/resource_aws_datasync_task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,10 +820,10 @@ resource "aws_cloudwatch_log_group" "test" {
}
resource "aws_datasync_task" "test" {
cloudwatch_log_group_arn = "${replace(aws_cloudwatch_log_group.test.arn, ":*", "")}"
destination_location_arn = "${aws_datasync_location_s3.destination.arn}"
cloudwatch_log_group_arn = aws_cloudwatch_log_group.test.arn
destination_location_arn = aws_datasync_location_s3.destination.arn
name = %q
source_location_arn = "${aws_datasync_location_nfs.source.arn}"
source_location_arn = aws_datasync_location_nfs.source.arn
}
`, rName, rName)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ data "aws_iam_policy_document" "ad-log-policy" {
type = "Service"
}
resources = ["${aws_cloudwatch_log_group.logs.arn}"]
resources = ["${aws_cloudwatch_log_group.logs.arn}:*"]
effect = "Allow"
}
Expand Down
4 changes: 0 additions & 4 deletions aws/resource_aws_flow_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ func resourceAwsFlowLog() *schema.Resource {
ForceNew: true,
ConflictsWith: []string{"log_group_name"},
ValidateFunc: validateArn,
StateFunc: func(arn interface{}) string {
// aws_cloudwatch_log_group arn attribute references contain a trailing `:*`, which breaks functionality
return strings.TrimSuffix(arn.(string), ":*")
},
},

"log_destination_type": {
Expand Down
61 changes: 59 additions & 2 deletions website/docs/guides/version-3-upgrade.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Upgrade topics:
- [Resource: aws_api_gateway_method_settings](#resource-aws_api_gateway_method_settings)
- [Resource: aws_autoscaling_group](#resource-aws_autoscaling_group)
- [Resource: aws_cloudfront_distribution](#resource-aws_cloudfront_distribution)
- [Resource: aws_cloudwatch_log_group](#resource-aws_cloudwatch_log_group)
- [Resource: aws_cognito_user_pool](#resource-aws_cognito_user_pool)
- [Resource: aws_dx_gateway](#resource-aws_dx_gateway)
- [Resource: aws_dx_gateway_association](#resource-aws_dx_gateway_association)
Expand Down Expand Up @@ -638,8 +639,64 @@ aws_cloudfront_distribution.example.active_trusted_signers.items
Updated references:

```
aws_cloudfront_distribution.example.trusted_signers.0.enabled
aws_cloudfront_distribution.example.trusted_signers.0.items
aws_cloudfront_distribution.example.trusted_signers[0].enabled
aws_cloudfront_distribution.example.trusted_signers[0].items
```

## Resource: aws_cloudwatch_log_group

### Removal of arn Wildcard Suffix

Previously, the resource returned the Amazon Resource Name (ARN) directly from the API, which included a `:*` suffix to denote all CloudWatch Log Streams under the CloudWatch Log Group. Most other AWS resources that return ARNs and many other AWS services do not use the `:*` suffix. The suffix is now automatically removed. For example, the resource previously returned an ARN such as `arn:aws:logs:us-east-1:123456789012:log-group:/example:*` but will now return `arn:aws:logs:us-east-1:123456789012:log-group:/example`.

Workarounds, such as using `replace()` as shown below, should be removed:

```hcl
resource "aws_cloudwatch_log_group" "example" {
name = "example"
}
resource "aws_datasync_task" "example" {
# ... other configuration ...
cloudwatch_log_group_arn = replace(aws_cloudwatch_log_group.example.arn, ":*", "")
}
```

Removing the `:*` suffix is a breaking change for some configurations. Fix these configurations using string interpolations as demonstrated below. For example, this configuration is now broken:

```hcl
data "aws_iam_policy_document" "ad-log-policy" {
statement {
actions = [
"logs:CreateLogStream",
"logs:PutLogEvents"
]
principals {
identifiers = ["ds.amazonaws.com"]
type = "Service"
}
resources = [aws_cloudwatch_log_group.example.arn]
effect = "Allow"
}
}
```

An updated configuration:

```hcl
data "aws_iam_policy_document" "ad-log-policy" {
statement {
actions = [
"logs:CreateLogStream",
"logs:PutLogEvents"
]
principals {
identifiers = ["ds.amazonaws.com"]
type = "Service"
}
resources = ["${aws_cloudwatch_log_group.example.arn}:*"]
effect = "Allow"
}
}
```

## Resource: aws_cognito_user_pool
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/cloudwatch_log_group.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ permissions for the CMK whenever the encrypted data is requested.

In addition to all arguments above, the following attributes are exported:

* `arn` - The Amazon Resource Name (ARN) specifying the log group.
* `arn` - The Amazon Resource Name (ARN) specifying the log group. Any `:*` suffix added by the API, denoting all CloudWatch Log Streams under the CloudWatch Log Group, is removed for greater compatibility with other AWS services that do not accept the suffix.


## Import
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ data "aws_iam_policy_document" "ad-log-policy" {
type = "Service"
}
resources = [aws_cloudwatch_log_group.example.arn]
resources = ["${aws_cloudwatch_log_group.example.arn}:*"]
effect = "Allow"
}
Expand Down

0 comments on commit 9fb4164

Please sign in to comment.