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

provider/aws: Allow ARN identifier to be set #11359

Merged
merged 3 commits into from
Jan 23, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package aws

import (
"fmt"

"github.com/hashicorp/terraform/helper/schema"
)

Expand All @@ -12,7 +14,7 @@ func dataSourceAwsBillingServiceAccount() *schema.Resource {
Read: dataSourceAwsBillingServiceAccountRead,

Schema: map[string]*schema.Schema{
"arn": &schema.Schema{
"arn": {
Type: schema.TypeString,
Computed: true,
},
Expand All @@ -23,7 +25,7 @@ func dataSourceAwsBillingServiceAccount() *schema.Resource {
func dataSourceAwsBillingServiceAccountRead(d *schema.ResourceData, meta interface{}) error {
d.SetId(billingAccountId)

d.Set("arn", "arn:aws:iam::"+billingAccountId+":root")
d.Set("arn", fmt.Sprintf("arn:%s:iam::%s:root", meta.(*AWSClient).partition, billingAccountId))

return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ func TestAccAWSBillingServiceAccount_basic(t *testing.T) {
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccCheckAwsBillingServiceAccountConfig,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_billing_service_account.main", "id", "386209384616"),
Expand Down
6 changes: 3 additions & 3 deletions builtin/providers/aws/data_source_aws_elb_service_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ func dataSourceAwsElbServiceAccount() *schema.Resource {
Read: dataSourceAwsElbServiceAccountRead,

Schema: map[string]*schema.Schema{
"region": &schema.Schema{
"region": {
Type: schema.TypeString,
Optional: true,
},
"arn": &schema.Schema{
"arn": {
Type: schema.TypeString,
Computed: true,
},
Expand All @@ -52,7 +52,7 @@ func dataSourceAwsElbServiceAccountRead(d *schema.ResourceData, meta interface{}
if accid, ok := elbAccountIdPerRegionMap[region]; ok {
d.SetId(accid)

d.Set("arn", "arn:aws:iam::"+accid+":root")
d.Set("arn", fmt.Sprintf("arn:%s:iam::%s:root", meta.(*AWSClient).partition, accid))

return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ func TestAccAWSElbServiceAccount_basic(t *testing.T) {
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccCheckAwsElbServiceAccountConfig,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_elb_service_account.main", "id", "797873946194"),
resource.TestCheckResourceAttr("data.aws_elb_service_account.main", "arn", "arn:aws:iam::797873946194:root"),
),
},
resource.TestStep{
{
Config: testAccCheckAwsElbServiceAccountExplicitRegionConfig,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_elb_service_account.regional", "id", "156460612806"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,28 @@ func resourceAwsCloudFrontOriginAccessIdentity() *schema.Resource {
},

Schema: map[string]*schema.Schema{
"comment": &schema.Schema{
"comment": {
Type: schema.TypeString,
Optional: true,
Default: "",
},
"caller_reference": &schema.Schema{
"caller_reference": {
Type: schema.TypeString,
Computed: true,
},
"cloudfront_access_identity_path": &schema.Schema{
"cloudfront_access_identity_path": {
Type: schema.TypeString,
Computed: true,
},
"etag": &schema.Schema{
"etag": {
Type: schema.TypeString,
Computed: true,
},
"iam_arn": &schema.Schema{
"iam_arn": {
Type: schema.TypeString,
Computed: true,
},
"s3_canonical_user_id": &schema.Schema{
"s3_canonical_user_id": {
Type: schema.TypeString,
Computed: true,
},
Expand Down Expand Up @@ -81,7 +81,8 @@ func resourceAwsCloudFrontOriginAccessIdentityRead(d *schema.ResourceData, meta
d.Set("etag", resp.ETag)
d.Set("s3_canonical_user_id", resp.CloudFrontOriginAccessIdentity.S3CanonicalUserId)
d.Set("cloudfront_access_identity_path", fmt.Sprintf("origin-access-identity/cloudfront/%s", *resp.CloudFrontOriginAccessIdentity.Id))
d.Set("iam_arn", fmt.Sprintf("arn:aws:iam::cloudfront:user/CloudFront Origin Access Identity %s", *resp.CloudFrontOriginAccessIdentity.Id))
d.Set("iam_arn", fmt.Sprintf("arn:%s:iam::cloudfront:user/CloudFront Origin Access Identity %s",
meta.(*AWSClient).partition, *resp.CloudFrontOriginAccessIdentity.Id))
return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestAccAWSCloudFrontOriginAccessIdentity_basic(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckCloudFrontOriginAccessIdentityDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccAWSCloudFrontOriginAccessIdentityConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudFrontOriginAccessIdentityExistence("aws_cloudfront_origin_access_identity.origin_access_identity"),
Expand Down Expand Up @@ -46,7 +46,7 @@ func TestAccAWSCloudFrontOriginAccessIdentity_noComment(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckCloudFrontOriginAccessIdentityDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccAWSCloudFrontOriginAccessIdentityNoCommentConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudFrontOriginAccessIdentityExistence("aws_cloudfront_origin_access_identity.origin_access_identity"),
Expand Down
6 changes: 3 additions & 3 deletions builtin/providers/aws/resource_aws_ecs_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error {
d.Set("name", service.ServiceName)

// Save task definition in the same format
if strings.HasPrefix(d.Get("task_definition").(string), "arn:aws:ecs:") {
if regexp.MustCompile(`^arn:[\w-]+:ecs:`).MatchString(d.Get("task_definition").(string)) {
d.Set("task_definition", service.TaskDefinition)
} else {
taskDefinition := buildFamilyAndRevisionFromARN(*service.TaskDefinition)
Expand All @@ -292,7 +292,7 @@ func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error {
d.Set("desired_count", service.DesiredCount)

// Save cluster in the same format
if strings.HasPrefix(d.Get("cluster").(string), "arn:aws:ecs:") {
if regexp.MustCompile(`^arn:[\w-]+:ecs:`).MatchString(d.Get("cluster").(string)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I originally suggested using regexp matching here, but now that I'm looking at the whole diff I'm thinking we could just use the same pattern as elsewhere and avoid the loose matching - i.e.

if strings.HasPrefix(d.Get("cluster").(string), "arn:"+meta.(*AWSClient).partition+":ecs:") {

or something similar w/ fmt.Sprintf - depending on what looks better.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks/sounds good to me, I'll make that change and push

d.Set("cluster", service.ClusterArn)
} else {
clusterARN := getNameFromARN(*service.ClusterArn)
Expand All @@ -301,7 +301,7 @@ func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error {

// Save IAM role in the same format
if service.RoleArn != nil {
if strings.HasPrefix(d.Get("iam_role").(string), "arn:aws:iam:") {
if regexp.MustCompile(`^arn:[\w-]+:iam:`).MatchString(d.Get("iam_role").(string)) {
d.Set("iam_role", service.RoleArn)
} else {
roleARN := getNameFromARN(*service.RoleArn)
Expand Down
14 changes: 7 additions & 7 deletions builtin/providers/aws/resource_aws_iam_saml_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,20 @@ func resourceAwsIamSamlProvider() *schema.Resource {
},

Schema: map[string]*schema.Schema{
"arn": &schema.Schema{
"arn": {
Type: schema.TypeString,
Computed: true,
},
"valid_until": &schema.Schema{
"valid_until": {
Type: schema.TypeString,
Computed: true,
},
"name": &schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"saml_metadata_document": &schema.Schema{
"saml_metadata_document": {
Type: schema.TypeString,
Required: true,
},
Expand Down Expand Up @@ -75,7 +75,7 @@ func resourceAwsIamSamlProviderRead(d *schema.ResourceData, meta interface{}) er

validUntil := out.ValidUntil.Format(time.RFC1123)
d.Set("arn", d.Id())
name, err := extractNameFromIAMSamlProviderArn(d.Id())
name, err := extractNameFromIAMSamlProviderArn(d.Id(), meta.(*AWSClient).partition)
if err != nil {
return err
}
Expand Down Expand Up @@ -112,9 +112,9 @@ func resourceAwsIamSamlProviderDelete(d *schema.ResourceData, meta interface{})
return err
}

func extractNameFromIAMSamlProviderArn(arn string) (string, error) {
func extractNameFromIAMSamlProviderArn(arn, partition string) (string, error) {
// arn:aws:iam::123456789012:saml-provider/tf-salesforce-test
r := regexp.MustCompile("^arn:aws:iam::[0-9]{12}:saml-provider/(.+)$")
r := regexp.MustCompile(fmt.Sprintf("^arn:%s:iam::[0-9]{12}:saml-provider/(.+)$", partition))
submatches := r.FindStringSubmatch(arn)
if len(submatches) != 2 {
return "", fmt.Errorf("Unable to extract name from a given ARN: %q", arn)
Expand Down
4 changes: 2 additions & 2 deletions builtin/providers/aws/resource_aws_iam_saml_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ func TestAccAWSIAMSamlProvider_basic(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckIAMSamlProviderDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccIAMSamlProviderConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckIAMSamlProvider("aws_iam_saml_provider.salesforce"),
),
},
resource.TestStep{
{
Config: testAccIAMSamlProviderConfigUpdate,
Check: resource.ComposeTestCheckFunc(
testAccCheckIAMSamlProvider("aws_iam_saml_provider.salesforce"),
Expand Down
18 changes: 9 additions & 9 deletions builtin/providers/aws/resource_aws_lambda_permission.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/hashicorp/terraform/helper/schema"
)

var LambdaFunctionRegexp = `^(arn:aws:lambda:)?([a-z]{2}-[a-z]+-\d{1}:)?(\d{12}:)?(function:)?([a-zA-Z0-9-_]+)(:(\$LATEST|[a-zA-Z0-9-_]+))?$`
var LambdaFunctionRegexp = `^(arn:[\w-]+:lambda:)?([a-z]{2}-[a-z]+-\d{1}:)?(\d{12}:)?(function:)?([a-zA-Z0-9-_]+)(:(\$LATEST|[a-zA-Z0-9-_]+))?$`

func resourceAwsLambdaPermission() *schema.Resource {
return &schema.Resource{
Expand All @@ -24,42 +24,42 @@ func resourceAwsLambdaPermission() *schema.Resource {
Delete: resourceAwsLambdaPermissionDelete,

Schema: map[string]*schema.Schema{
"action": &schema.Schema{
"action": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateLambdaPermissionAction,
},
"function_name": &schema.Schema{
"function_name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateLambdaFunctionName,
},
"principal": &schema.Schema{
"principal": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"qualifier": &schema.Schema{
"qualifier": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: validateLambdaQualifier,
},
"source_account": &schema.Schema{
"source_account": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: validateAwsAccountId,
},
"source_arn": &schema.Schema{
"source_arn": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: validateArn,
},
"statement_id": &schema.Schema{
"statement_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Expand Down Expand Up @@ -216,7 +216,7 @@ func resourceAwsLambdaPermissionRead(d *schema.ResourceData, meta interface{}) e
}

// Save Lambda function name in the same format
if strings.HasPrefix(d.Get("function_name").(string), "arn:aws:lambda:") {
if regexp.MustCompile(`^arn:[\w-]+:lambda:`).MatchString(d.Get("function_name").(string)) {
// Strip qualifier off
trimmedArn := strings.TrimSuffix(statement.Resource, ":"+qualifier)
d.Set("function_name", trimmedArn)
Expand Down
26 changes: 19 additions & 7 deletions builtin/providers/aws/resource_aws_lambda_permission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,18 @@ func TestLambdaPermissionGetQualifierFromLambdaAliasOrVersionArn_alias(t *testin
}
}

func TestLambdaPermissionGetQualifierFromLambdaAliasOrVersionArn_govcloud(t *testing.T) {
arnWithAlias := "arn:aws-us-gov:lambda:us-west-2:187636751137:function:lambda_function_name:testalias"
expectedQualifier := "testalias"
qualifier, err := getQualifierFromLambdaAliasOrVersionArn(arnWithAlias)
if err != nil {
t.Fatalf("Expected no error when getting qualifier: %s", err)
}
if qualifier != expectedQualifier {
t.Fatalf("Expected qualifier to match (%q != %q)", qualifier, expectedQualifier)
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for the test


func TestLambdaPermissionGetQualifierFromLambdaAliasOrVersionArn_version(t *testing.T) {
arnWithVersion := "arn:aws:lambda:us-west-2:187636751137:function:lambda_function_name:223"
expectedQualifier := "223"
Expand Down Expand Up @@ -141,7 +153,7 @@ func TestAccAWSLambdaPermission_basic(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSLambdaPermissionDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccAWSLambdaPermissionConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckLambdaPermissionExists("aws_lambda_permission.allow_cloudwatch", &statement),
Expand All @@ -165,7 +177,7 @@ func TestAccAWSLambdaPermission_withRawFunctionName(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSLambdaPermissionDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccAWSLambdaPermissionConfig_withRawFunctionName,
Check: resource.ComposeTestCheckFunc(
testAccCheckLambdaPermissionExists("aws_lambda_permission.with_raw_func_name", &statement),
Expand All @@ -188,7 +200,7 @@ func TestAccAWSLambdaPermission_withQualifier(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSLambdaPermissionDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccAWSLambdaPermissionConfig_withQualifier,
Check: resource.ComposeTestCheckFunc(
testAccCheckLambdaPermissionExists("aws_lambda_permission.with_qualifier", &statement),
Expand Down Expand Up @@ -217,7 +229,7 @@ func TestAccAWSLambdaPermission_multiplePerms(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSLambdaPermissionDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccAWSLambdaPermissionConfig_multiplePerms,
Check: resource.ComposeTestCheckFunc(
// 1st
Expand All @@ -236,7 +248,7 @@ func TestAccAWSLambdaPermission_multiplePerms(t *testing.T) {
regexp.MustCompile(":function:lambda_function_name_perm_multiperms$")),
),
},
resource.TestStep{
{
Config: testAccAWSLambdaPermissionConfig_multiplePermsModified,
Check: resource.ComposeTestCheckFunc(
// 1st
Expand Down Expand Up @@ -277,7 +289,7 @@ func TestAccAWSLambdaPermission_withS3(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSLambdaPermissionDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: fmt.Sprintf(testAccAWSLambdaPermissionConfig_withS3_tpl, rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckLambdaPermissionExists("aws_lambda_permission.with_s3", &statement),
Expand All @@ -303,7 +315,7 @@ func TestAccAWSLambdaPermission_withSNS(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSLambdaPermissionDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccAWSLambdaPermissionConfig_withSNS,
Check: resource.ComposeTestCheckFunc(
testAccCheckLambdaPermissionExists("aws_lambda_permission.with_sns", &statement),
Expand Down
7 changes: 4 additions & 3 deletions builtin/providers/aws/resource_aws_sns_topic_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func resourceAwsSnsTopicPolicyRead(d *schema.ResourceData, meta interface{}) err
}

func resourceAwsSnsTopicPolicyDelete(d *schema.ResourceData, meta interface{}) error {
accountId, err := getAccountIdFromSnsTopicArn(d.Id())
accountId, err := getAccountIdFromSnsTopicArn(d.Id(), meta.(*AWSClient).partition)
if err != nil {
return err
}
Expand Down Expand Up @@ -134,9 +134,10 @@ func resourceAwsSnsTopicPolicyDelete(d *schema.ResourceData, meta interface{}) e
return nil
}

func getAccountIdFromSnsTopicArn(arn string) (string, error) {
func getAccountIdFromSnsTopicArn(arn, partition string) (string, error) {
// arn:aws:sns:us-west-2:123456789012:test-new
re := regexp.MustCompile("^arn:aws:sns:[^:]+:([0-9]{12}):.+")
// arn:aws-us-gov:sns:us-west-2:123456789012:test-new
re := regexp.MustCompile(fmt.Sprintf("^arn:%s:sns:[^:]+:([0-9]{12}):.+", partition))
matches := re.FindStringSubmatch(arn)
if len(matches) != 2 {
return "", fmt.Errorf("Unable to get account ID from ARN (%q)", arn)
Expand Down
Loading