DNS resolution for inter-region vpc peering #7627
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
size/XL
ewbankkit
commented
Feb 20, 2019
| @@ -22,6 +22,7 @@ var testAccProvidersWithTLS map[string]terraform.ResourceProvider | |||
| var testAccProviderFactories func(providers *[]*schema.Provider) map[string]terraform.ResourceProviderFactory | |||
| var testAccProvider *schema.Provider | |||
| var testAccTemplateProvider *schema.Provider | |||
| var testAccProviderFunc func() *schema.Provider | |||
There was a problem hiding this comment.
Can also be used in other cross-region acceptance tests:
$ grep 'return testAccProvider' *.go
provider_test.go: testAccProviderFunc = func() *schema.Provider { return testAccProvider }
resource_aws_docdb_cluster_test.go: return testAccCheckDocDBClusterExistsWithProvider(n, v, func() *schema.Provider { return testAccProvider })
resource_aws_instance_test.go: return testAccCheckInstanceExistsWithProvider(n, i, func() *schema.Provider { return testAccProvider })
resource_aws_neptune_cluster_test.go: return testAccCheckAWSNeptuneClusterExistsWithProvider(n, v, func() *schema.Provider { return testAccProvider })
resource_aws_rds_cluster_test.go: return testAccCheckAWSClusterExistsWithProvider(n, v, func() *schema.Provider { return testAccProvider })
resource_aws_route53_zone_association_test.go: return testAccCheckRoute53ZoneAssociationExistsWithProvider(n, zone, func() *schema.Provider { return testAccProvider })
resource_aws_route53_zone_test.go: return testAccCreateRandomRoute53RecordsInZoneIdWithProvider(func() *schema.Provider { return testAccProvider }, zone, recordsCount)
resource_aws_route53_zone_test.go: return testAccCheckRoute53ZoneExistsWithProvider(n, zone, func() *schema.Provider { return testAccProvider })
resource_aws_s3_bucket_test.go: return testAccCheckAWSS3BucketExistsWithProvider(n, func() *schema.Provider { return testAccProvider })There was a problem hiding this comment.
I'll add a Tech Debt issue to tidy these up.
There was a problem hiding this comment.
I believe many of the testAccCheck...ExistsWithProvider() functions are extraneous or adding unnecessary complexity. Its easier to setup the acceptance testing configurations to only need the "normal" testAccCheck...Exists() function with its implicit testAccProvider usage.
There was a problem hiding this comment.
Fixed the merge conflict with the removal of the template provider on merge.
|
Acceptance tests: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVPCPeeringConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 3 -run=TestAccAWSVPCPeeringConnection_ -timeout 120m
=== RUN TestAccAWSVPCPeeringConnection_importBasic
=== PAUSE TestAccAWSVPCPeeringConnection_importBasic
=== RUN TestAccAWSVPCPeeringConnection_basic
=== PAUSE TestAccAWSVPCPeeringConnection_basic
=== RUN TestAccAWSVPCPeeringConnection_accept
=== PAUSE TestAccAWSVPCPeeringConnection_accept
=== RUN TestAccAWSVPCPeeringConnection_plan
=== PAUSE TestAccAWSVPCPeeringConnection_plan
=== RUN TestAccAWSVPCPeeringConnection_tags
=== PAUSE TestAccAWSVPCPeeringConnection_tags
=== RUN TestAccAWSVPCPeeringConnection_options
=== PAUSE TestAccAWSVPCPeeringConnection_options
=== RUN TestAccAWSVPCPeeringConnection_optionsNoAccept
=== PAUSE TestAccAWSVPCPeeringConnection_optionsNoAccept
=== RUN TestAccAWSVPCPeeringConnection_failedState
=== PAUSE TestAccAWSVPCPeeringConnection_failedState
=== RUN TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
=== PAUSE TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
=== RUN TestAccAWSVPCPeeringConnection_region
=== PAUSE TestAccAWSVPCPeeringConnection_region
=== CONT TestAccAWSVPCPeeringConnection_importBasic
=== CONT TestAccAWSVPCPeeringConnection_region
=== CONT TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
--- PASS: TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept (16.95s)
=== CONT TestAccAWSVPCPeeringConnection_failedState
--- PASS: TestAccAWSVPCPeeringConnection_importBasic (31.08s)
=== CONT TestAccAWSVPCPeeringConnection_optionsNoAccept
--- PASS: TestAccAWSVPCPeeringConnection_failedState (17.98s)
=== CONT TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_region (35.24s)
=== CONT TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_optionsNoAccept (20.35s)
=== CONT TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_plan (28.07s)
=== CONT TestAccAWSVPCPeeringConnection_accept
--- PASS: TestAccAWSVPCPeeringConnection_tags (49.04s)
=== CONT TestAccAWSVPCPeeringConnection_basic
--- PASS: TestAccAWSVPCPeeringConnection_options (50.04s)
--- PASS: TestAccAWSVPCPeeringConnection_basic (28.26s)
--- PASS: TestAccAWSVPCPeeringConnection_accept (62.39s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 141.928s
$ VPC_PEER_AWS_ACCESS_KEY_ID=xxxxxxxxxxxxxxxxxxxx\
VPC_PEER_AWS_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\
make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVPCPeeringConnectionAccepter_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 3 -run=TestAccAWSVPCPeeringConnectionAccepter_ -timeout 120m
=== RUN TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount
=== RUN TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount
=== RUN TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount
=== RUN TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount (31.63s)
=== CONT TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount (35.22s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount (35.79s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount (38.98s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 70.631s
$ VPC_PEER_AWS_ACCESS_KEY_ID=xxxxxxxxxxxxxxxxxxxx\
VPC_PEER_AWS_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\
make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVpcPeeringConnectionOptions_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 3 -run=TestAccAWSVpcPeeringConnectionOptions_ -timeout 120m
=== RUN TestAccAWSVpcPeeringConnectionOptions_importBasic
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_importBasic
=== RUN TestAccAWSVpcPeeringConnectionOptions_sameRegionSameAccount
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_sameRegionSameAccount
=== RUN TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount
=== RUN TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount
=== CONT TestAccAWSVpcPeeringConnectionOptions_importBasic
=== CONT TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount
=== CONT TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount
--- PASS: TestAccAWSVpcPeeringConnectionOptions_importBasic (34.14s)
=== CONT TestAccAWSVpcPeeringConnectionOptions_sameRegionSameAccount
--- PASS: TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount (36.73s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount (38.04s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_sameRegionSameAccount (32.49s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 66.643s |
ewbankkit
force-pushed
the
issue-6730
branch
from
0d359fe
to
17d8934
Compare
ghost
added
size/XXL
ewbankkit
commented
Feb 24, 2019
| if err != nil { | ||
| return fmt.Errorf("Error setting VPC Peering Connection accepter information: %s", err) | ||
| } | ||
| if err := d.Set("accepter", flattenVpcPeeringConnectionOptions(pc.AccepterVpcInfo.PeeringOptions)); err != nil { |
There was a problem hiding this comment.
flattenVpcPeeringConnectionOptions handles pc.AccepterVpcInfo.PeeringOptions == nil
ewbankkit
commented
Feb 24, 2019
| if err != nil { | ||
| return fmt.Errorf("Error setting VPC Peering Connection requester information: %s", err) | ||
| } | ||
| if err := d.Set("requester", flattenVpcPeeringConnectionOptions(pc.RequesterVpcInfo.PeeringOptions)); err != nil { |
There was a problem hiding this comment.
flattenVpcPeeringConnectionOptions handles pc.RequesterVpcInfo.PeeringOptions == nil
ewbankkit
commented
Feb 24, 2019
| if len(v) > 0 { | ||
| req.RequesterPeeringConnectionOptions = expandVpcPeeringConnectionOptions(v[0].(map[string]interface{})) | ||
| VpcPeeringConnectionId: aws.String(d.Id()), | ||
| AccepterPeeringConnectionOptions: expandVpcPeeringConnectionOptions(d.Get("accepter").(*schema.Set).List(), crossRegionPeering), |
There was a problem hiding this comment.
expandVpcPeeringConnectionOptions handles empty set
ewbankkit
commented
Feb 24, 2019
| } | ||
|
|
||
| if d.HasChange("accepter") || d.HasChange("requester") { | ||
| _, ok := d.GetOk("auto_accept") | ||
| if !ok && pc.Status != nil && *pc.Status.Code != "active" { | ||
| if statusCode == ec2.VpcPeeringConnectionStateReasonCodeActive || statusCode == ec2.VpcPeeringConnectionStateReasonCodeProvisioning { |
There was a problem hiding this comment.
VPC peering connection is in provisioning state if it has just been created.
DNS options can be set if the connection is in this state.
ewbankkit
commented
Feb 24, 2019
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func resourceAwsVpcPeeringConnectionOptionsUpdate(d *schema.ResourceData, meta interface{}) error { | ||
| if err := resourceAwsVpcPeeringConnectionModifyOptions(d, meta); err != nil { | ||
| conn := meta.(*AWSClient).ec2conn |
There was a problem hiding this comment.
Need to get the peering connection info here to determine whether or not it's a cross-region peering.
There was a problem hiding this comment.
I think in the future we can avoid logic like this by saving this information into a computed attribute.
ewbankkit
changed the title
|
Removing WIP. |
ewbankkit
commented
Feb 24, 2019
| // peer_vpc_id - The ID of the requester VPC | ||
| // peer_owner_id - The AWS account ID of the owner of the requester VPC | ||
| // peer_region - The region of the accepter VPC | ||
| // ** TODO |
There was a problem hiding this comment.
I'll add an issue to investigate this.
|
do you have any estimates when it can be merged and accessable in new terraform version? |
bflad
reviewed
Apr 10, 2019
|
|
||
| func TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount(t *testing.T) { | ||
| key := "VPC_PEER_AWS_ACCESS_KEY_ID" |
There was a problem hiding this comment.
You're definitely on the right track here, but I would like to call out that for cross-account testing we are trying to standardize on a setup like the one that can be found in https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_guardduty_invite_accepter_test.go -- the gist being:
- Test
PreCheckincludestestAccAlternateAccountPreCheck(t)to ensure a standardizedAWS_ALTERNATE_ACCESS_KEY_ID+AWS_ALTERNATE_SECRET_ACCESS_KEYorAWS_ALTERNATE_PROFILEset of information is provided for cross-account testing credentials ProviderFactoriesis used instead ofProviderslike it is here 👍- Test configurations pull in a standardized
testAccAlternateAccountProviderConfig()and use theaws.alternateprovider alias for the necessary cross-account resources. The resource that is being tested should not use the provider alias to simplify the testing setup. - Any import testing steps include the
Configto match the previous step
That being said, these particular acceptance tests are also dealing with same account different region and different account different region testing. The framework mentioned above needs some extensions to support standardized region selection as well, the preference being an optional AWS_ALTERNATE_REGION environment variable in conjunction with two new provider configurations similar to testAccAlternateAccountProviderConfig(), e.g.
func testAccGetAlternateRegion() string {
v := os.Getenv("AWS_ALTERNATE_REGION")
if v == "" {
return "us-east-1"
}
return v
}
func testAccAlternateAccountAlternateRegionProviderConfig() string {
return fmt.Sprintf(`
provider "aws" {
access_key = %[1]q
alias = "alternate"
profile = %[2]q
region = %[3]q
secret_key = %[4]q
}
`, os.Getenv("AWS_ALTERNATE_ACCESS_KEY_ID"), os.Getenv("AWS_ALTERNATE_PROFILE"), testAccGetAlternateRegion(), os.Getenv("AWS_ALTERNATE_SECRET_ACCESS_KEY"))
}
func testAccAlternateRegionProviderConfig() string {
return fmt.Sprintf(`
provider "aws" {
alias = "alternate"
region = %[1]q
}
`, testAccGetAlternateRegion())
}The above should cover most of our acceptance testing use cases while allowing us to use a standard framework for implementations across the provider. 👍
There was a problem hiding this comment.
@bflad Yes, I only saw those recommendations after making these changes - I'll incorporate the recommendations and enhance where appropriate. Thanks.
ewbankkit
force-pushed
the
issue-6730
branch
2 times, most recently
from
a7b9f74
to
5a3eb57
Compare
|
Changes made to the acceptance testing framework. |
|
OK, acceptance tests re-run: aws_vpc_peering_connection$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVPCPeeringConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 3 -run=TestAccAWSVPCPeeringConnection_ -timeout 120m
=== RUN TestAccAWSVPCPeeringConnection_basic
=== PAUSE TestAccAWSVPCPeeringConnection_basic
=== RUN TestAccAWSVPCPeeringConnection_accept
=== PAUSE TestAccAWSVPCPeeringConnection_accept
=== RUN TestAccAWSVPCPeeringConnection_plan
=== PAUSE TestAccAWSVPCPeeringConnection_plan
=== RUN TestAccAWSVPCPeeringConnection_tags
=== PAUSE TestAccAWSVPCPeeringConnection_tags
=== RUN TestAccAWSVPCPeeringConnection_options
=== PAUSE TestAccAWSVPCPeeringConnection_options
=== RUN TestAccAWSVPCPeeringConnection_optionsNoAccept
=== PAUSE TestAccAWSVPCPeeringConnection_optionsNoAccept
=== RUN TestAccAWSVPCPeeringConnection_failedState
=== PAUSE TestAccAWSVPCPeeringConnection_failedState
=== RUN TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
=== PAUSE TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
=== RUN TestAccAWSVPCPeeringConnection_region
=== PAUSE TestAccAWSVPCPeeringConnection_region
=== CONT TestAccAWSVPCPeeringConnection_basic
=== CONT TestAccAWSVPCPeeringConnection_region
=== CONT TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
--- PASS: TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept (18.36s)
=== CONT TestAccAWSVPCPeeringConnection_failedState
--- PASS: TestAccAWSVPCPeeringConnection_basic (34.00s)
=== CONT TestAccAWSVPCPeeringConnection_optionsNoAccept
--- PASS: TestAccAWSVPCPeeringConnection_region (34.86s)
=== CONT TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_failedState (18.50s)
=== CONT TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_optionsNoAccept (21.80s)
=== CONT TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_plan (29.65s)
=== CONT TestAccAWSVPCPeeringConnection_accept
--- PASS: TestAccAWSVPCPeeringConnection_tags (50.03s)
--- PASS: TestAccAWSVPCPeeringConnection_options (52.53s)
--- PASS: TestAccAWSVPCPeeringConnection_accept (65.37s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 150.876saws_vpc_peering_connection_options$ AWS_ALTERNATE_ACCESS_KEY_ID=AAAAAAAAAAAAAAAAAAAA AWS_ALTERNATE_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVpcPeeringConnectionOptions_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 3 -run=TestAccAWSVpcPeeringConnectionOptions_ -timeout 120m
=== RUN TestAccAWSVpcPeeringConnectionOptions_sameRegionSameAccount
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_sameRegionSameAccount
=== RUN TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount
=== RUN TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount
=== CONT TestAccAWSVpcPeeringConnectionOptions_sameRegionSameAccount
=== CONT TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount
=== CONT TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount
--- PASS: TestAccAWSVpcPeeringConnectionOptions_sameRegionSameAccount (36.27s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount (40.59s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount (44.88s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 44.946saws_vpc_peering_connection_accepter$ AWS_ALTERNATE_ACCESS_KEY_ID=AAAAAAAAAAAAAAAAAAAA AWS_ALTERNATE_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVPCPeeringConnectionAccepter_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 3 -run=TestAccAWSVPCPeeringConnectionAccepter_ -timeout 120m
=== RUN TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount
=== RUN TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount
=== RUN TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount
=== RUN TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount (34.58s)
=== CONT TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount (37.71s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount (45.33s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount (35.74s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 70.362s |
|
Ready for re-review. |
ewbankkit
commented
May 9, 2019
.github/CONTRIBUTING.md
Outdated
| @@ -313,7 +314,7 @@ manually sourced values from documentation. | |||
| - [ ] Check [Regions and Endpoints S3 website endpoints](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_website_region_endpoints) and add Route53 Hosted Zone ID if available to `aws/hosted_zones.go` | |||
| - [ ] Check [CloudTrail Supported Regions docs](https://docs.aws.amazon.com/awscloudtrail/latest/userguide/cloudtrail-supported-regions.html) and add AWS Account ID if available to `aws/data_source_aws_cloudtrail_service_account.go` | |||
| - [ ] Check [Elastic Load Balancing Access Logs docs](https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/enable-access-logs.html#attach-bucket-policy) and add Elastic Load Balancing Account ID if available to `aws/data_source_aws_elb_service_account.go` | |||
| - [ ] Check [Redshift Database Audit Logging docs](https://docs.aws.amazon.com/redshift/latest/mgmt/db-auditing.html) and add AWS Account ID if available to `aws/data_source_aws_redshift_service_account.go` | |||
| - [ ] Check [Redshift Database Audit Logging docs](https://docs.aws.amazon.com/redshift/latest/mgmt/db-auditing.html#rs-db-auditing-cloud-trail-rs-acct-ids) and add AWS Account ID if available to `aws/data_source_aws_redshift_service_account.go` | |||
|
Rebased to resolve merge conflict. |
|
Rebased again to resolve merge conflict. |
|
Rebased again to resolve merge conflict. |
|
Rebased again to resolve merge conflicts. $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVPCPeeringConnection_'==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 3 -run=TestAccAWSVPCPeeringConnection_ -timeout 120m
=== RUN TestAccAWSVPCPeeringConnection_basic
=== PAUSE TestAccAWSVPCPeeringConnection_basic
=== RUN TestAccAWSVPCPeeringConnection_accept
=== PAUSE TestAccAWSVPCPeeringConnection_accept
=== RUN TestAccAWSVPCPeeringConnection_plan
=== PAUSE TestAccAWSVPCPeeringConnection_plan
=== RUN TestAccAWSVPCPeeringConnection_tags
=== PAUSE TestAccAWSVPCPeeringConnection_tags
=== RUN TestAccAWSVPCPeeringConnection_options
=== PAUSE TestAccAWSVPCPeeringConnection_options
=== RUN TestAccAWSVPCPeeringConnection_optionsNoAccept
=== PAUSE TestAccAWSVPCPeeringConnection_optionsNoAccept
=== RUN TestAccAWSVPCPeeringConnection_failedState
=== PAUSE TestAccAWSVPCPeeringConnection_failedState
=== RUN TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
=== PAUSE TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
=== RUN TestAccAWSVPCPeeringConnection_region
=== PAUSE TestAccAWSVPCPeeringConnection_region
=== CONT TestAccAWSVPCPeeringConnection_basic
=== CONT TestAccAWSVPCPeeringConnection_region
=== CONT TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept
--- PASS: TestAccAWSVPCPeeringConnection_peerRegionAndAutoAccept (18.72s)
=== CONT TestAccAWSVPCPeeringConnection_failedState
--- PASS: TestAccAWSVPCPeeringConnection_basic (34.23s)
=== CONT TestAccAWSVPCPeeringConnection_optionsNoAccept
--- PASS: TestAccAWSVPCPeeringConnection_region (35.04s)
=== CONT TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_failedState (19.00s)
=== CONT TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_optionsNoAccept (21.50s)
=== CONT TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_plan (29.48s)
=== CONT TestAccAWSVPCPeeringConnection_accept
--- PASS: TestAccAWSVPCPeeringConnection_options (52.59s)
--- PASS: TestAccAWSVPCPeeringConnection_tags (51.05s)
--- PASS: TestAccAWSVPCPeeringConnection_accept (66.18s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 151.439s
$ go test ./aws -v -sweep=us-east-1,us-west-2 -sweep-run=aws_vpc_peering_connection -timeout 10h
2019/08/02 16:17:41 [DEBUG] Running Sweepers for region (us-east-1):
2019/08/02 16:17:41 [INFO] Building AWS auth structure
2019/08/02 16:17:41 [INFO] Setting AWS metadata API timeout to 100ms
2019/08/02 16:17:42 [INFO] Ignoring AWS metadata API endpoint at default location as it doesn't return any instance-id
2019/08/02 16:17:42 [INFO] AWS Auth provider used: "EnvProvider"
2019/08/02 16:17:42 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/08/02 16:17:43 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/08/02 16:17:43 Sweeper Tests ran:
- aws_vpc_peering_connection
2019/08/02 16:17:43 [DEBUG] Running Sweepers for region (us-west-2):
2019/08/02 16:17:43 [INFO] Building AWS auth structure
2019/08/02 16:17:43 [INFO] Setting AWS metadata API timeout to 100ms
2019/08/02 16:17:44 [INFO] Ignoring AWS metadata API endpoint at default location as it doesn't return any instance-id
2019/08/02 16:17:44 [INFO] AWS Auth provider used: "EnvProvider"
2019/08/02 16:17:44 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/08/02 16:17:44 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/08/02 16:17:45 Sweeper Tests ran:
- aws_vpc_peering_connection
ok github.com/terraform-providers/terraform-provider-aws/aws 3.191s |
|
@bflad I think all the review issues you raised have been addressed. |
|
Hi @ewbankkit I was hoping to help push forward this PR. Is this ready to be reviewed again? |
|
@nywilken Yes, ready for review. Thanks. |
ewbankkit
force-pushed
the
issue-6730
branch
from
9a204e0
to
95696b2
Compare
|
Rebased to resolve merge commits. $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVPCPeeringConnection_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 2 -run=TestAccAWSVPCPeeringConnection_ -timeout 120m
=== RUN TestAccAWSVPCPeeringConnection_basic
=== PAUSE TestAccAWSVPCPeeringConnection_basic
=== RUN TestAccAWSVPCPeeringConnection_plan
=== PAUSE TestAccAWSVPCPeeringConnection_plan
=== RUN TestAccAWSVPCPeeringConnection_tags
=== PAUSE TestAccAWSVPCPeeringConnection_tags
=== RUN TestAccAWSVPCPeeringConnection_options
=== PAUSE TestAccAWSVPCPeeringConnection_options
=== RUN TestAccAWSVPCPeeringConnection_failedState
=== PAUSE TestAccAWSVPCPeeringConnection_failedState
=== RUN TestAccAWSVPCPeeringConnection_peerRegionAutoAccept
=== PAUSE TestAccAWSVPCPeeringConnection_peerRegionAutoAccept
=== RUN TestAccAWSVPCPeeringConnection_region
=== PAUSE TestAccAWSVPCPeeringConnection_region
=== RUN TestAccAWSVPCPeeringConnection_accept
=== PAUSE TestAccAWSVPCPeeringConnection_accept
=== RUN TestAccAWSVPCPeeringConnection_optionsNoAutoAccept
=== PAUSE TestAccAWSVPCPeeringConnection_optionsNoAutoAccept
=== CONT TestAccAWSVPCPeeringConnection_basic
=== CONT TestAccAWSVPCPeeringConnection_optionsNoAutoAccept
--- PASS: TestAccAWSVPCPeeringConnection_optionsNoAutoAccept (21.30s)
=== CONT TestAccAWSVPCPeeringConnection_accept
--- PASS: TestAccAWSVPCPeeringConnection_basic (32.76s)
=== CONT TestAccAWSVPCPeeringConnection_region
--- PASS: TestAccAWSVPCPeeringConnection_region (35.24s)
=== CONT TestAccAWSVPCPeeringConnection_peerRegionAutoAccept
--- PASS: TestAccAWSVPCPeeringConnection_peerRegionAutoAccept (17.82s)
=== CONT TestAccAWSVPCPeeringConnection_failedState
--- PASS: TestAccAWSVPCPeeringConnection_accept (71.87s)
=== CONT TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_failedState (18.57s)
=== CONT TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_tags (32.87s)
=== CONT TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_options (54.19s)
--- PASS: TestAccAWSVPCPeeringConnection_plan (29.31s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 166.597s
$ AWS_ALTERNATE_ACCESS_KEY_ID=AAAAAAAAAAAAAAAAAAAA AWS_ALTERNATE_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVpcPeeringConnectionOptions_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAWSVpcPeeringConnectionOptions_ -timeout 120m
=== RUN TestAccAWSVpcPeeringConnectionOptions_basic
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_basic
=== RUN TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount
=== RUN TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount
=== PAUSE TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount
=== CONT TestAccAWSVpcPeeringConnectionOptions_basic
=== CONT TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount
=== CONT TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount
--- PASS: TestAccAWSVpcPeeringConnectionOptions_basic (37.22s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount (43.46s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount (44.19s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 44.221s
$ AWS_ALTERNATE_ACCESS_KEY_ID=AAAAAAAAAAAAAAAAAAAA AWS_ALTERNATE_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSVPCPeeringConnectionAccepter_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAWSVPCPeeringConnectionAccepter_ -timeout 120m
=== RUN TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount
=== RUN TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount
=== RUN TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount
=== RUN TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount
=== PAUSE TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount
=== CONT TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount (35.74s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount (39.81s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount (39.92s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount (42.08s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 42.123s
$ go test ./aws -v -sweep=us-east-1,us-west-2 -sweep-run=aws_vpc_peering_connection -timeout 10h
2019/09/08 17:52:17 [DEBUG] Running Sweepers for region (us-east-1):
2019/09/08 17:52:17 [INFO] Building AWS auth structure
2019/09/08 17:52:17 [INFO] Setting AWS metadata API timeout to 100ms
2019/09/08 17:52:17 [INFO] Ignoring AWS metadata API endpoint at default location as it doesn't return any instance-id
2019/09/08 17:52:17 [INFO] AWS Auth provider used: "EnvProvider"
2019/09/08 17:52:17 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/09/08 17:52:17 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/09/08 17:52:18 Sweeper Tests ran:
- aws_vpc_peering_connection
2019/09/08 17:52:18 [DEBUG] Running Sweepers for region (us-west-2):
2019/09/08 17:52:18 [INFO] Building AWS auth structure
2019/09/08 17:52:18 [INFO] Setting AWS metadata API timeout to 100ms
2019/09/08 17:52:18 [INFO] Ignoring AWS metadata API endpoint at default location as it doesn't return any instance-id
2019/09/08 17:52:18 [INFO] AWS Auth provider used: "EnvProvider"
2019/09/08 17:52:18 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/09/08 17:52:19 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/09/08 17:52:20 Sweeper Tests ran:
- aws_vpc_peering_connection
ok github.com/terraform-providers/terraform-provider-aws/aws 3.083s
|
|
Is there anything stopping this merge? |
ewbankkit
force-pushed
the
issue-6730
branch
from
95696b2
to
2f41175
Compare
bflad
approved these changes
Sep 27, 2019
There was a problem hiding this comment.
After rebasing and minor adjustments noted below, pulling this in. Thanks for the hard work here @ewbankkit and sorry for the slow review process.
--- PASS: TestAccAWSVPCPeeringConnection_accept (72.88s)
--- PASS: TestAccAWSVPCPeeringConnection_basic (43.19s)
--- PASS: TestAccAWSVPCPeeringConnection_failedState (30.05s)
--- PASS: TestAccAWSVPCPeeringConnection_options (61.69s)
--- PASS: TestAccAWSVPCPeeringConnection_optionsNoAutoAccept (41.69s)
--- PASS: TestAccAWSVPCPeeringConnection_peerRegionAutoAccept (20.44s)
--- PASS: TestAccAWSVPCPeeringConnection_plan (38.92s)
--- PASS: TestAccAWSVPCPeeringConnection_region (46.76s)
--- PASS: TestAccAWSVPCPeeringConnection_tags (46.62s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_differentRegionDifferentAccount (51.57s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_differentRegionSameAccount (46.22s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegionDifferentAccount (44.01s)
--- PASS: TestAccAWSVPCPeeringConnectionAccepter_sameRegionSameAccount (28.72s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_basic (47.93s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_differentRegionSameAccount (46.49s)
--- PASS: TestAccAWSVpcPeeringConnectionOptions_sameRegionDifferentAccount (36.81s)
| export AWS_ALTERNATE_REGION=... | ||
| ``` | ||
|
|
||
| #### Acceptance Testing Guidelines |
There was a problem hiding this comment.
This was moved up in the file earlier this year, so fixing on merge.
| @@ -22,6 +22,7 @@ var testAccProvidersWithTLS map[string]terraform.ResourceProvider | |||
| var testAccProviderFactories func(providers *[]*schema.Provider) map[string]terraform.ResourceProviderFactory | |||
| var testAccProvider *schema.Provider | |||
| var testAccTemplateProvider *schema.Provider | |||
| var testAccProviderFunc func() *schema.Provider | |||
There was a problem hiding this comment.
Fixed the merge conflict with the removal of the template provider on merge.
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func resourceAwsVpcPeeringConnectionOptionsUpdate(d *schema.ResourceData, meta interface{}) error { | ||
| if err := resourceAwsVpcPeeringConnectionModifyOptions(d, meta); err != nil { | ||
| conn := meta.(*AWSClient).ec2conn |
There was a problem hiding this comment.
I think in the future we can avoid logic like this by saving this information into a computed attribute.
|
This has been released in version 2.31.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
|
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! |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6730.
The heart of this PR is the addition of a cross-region-peering flag to
expandVpcPeeringConnectionOptionsand some changes in callers to be able to set this.I took the opportunity to add a whole bunch more acceptance tests for the
aws_vpc_peering_connection,aws_vpc_peering_connection_accepterandaws_vpc_peering_connection_optionsresources including some cross-account tests that will only run iif theVPC_PEER_AWS_ACCESS_KEY_IDandVPC_PEER_AWS_SECRET_ACCESS_KEYenvironment variables are set to values for the AWS account that accepts the VPC peering.