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

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

Merged
merged 2 commits into from
Jul 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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