-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Update vpnsite_customer_gateway.go #26673
Conversation
Line 56 should be changed to optional as this is not a required field in the AWS SDK API. Having this as required breaks terraform applies for certificate based vpn connections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome @dionasaur 👋
It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.
Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.
Thanks again, and welcome to the community! 😃
Hey @dionasaur 👋 Thank you for taking the time to open this PR! In order for this to be reviewed, please take a look at the contribution guide section on making small changes to existing resources. |
Thanks Justin! sorry about that I am working on the acceptance test :) |
Hi Justin, I'm a little unfamiliar with the process but i performed some tests with make test and this is what it came back with: ~/d/h/terraform-provider-aws (main)> make test |
Hi @dionasaur, could you please reference the issue related to this ticket in the body of the PR description. Also please take a good look at our contributor guide. I'm happy to help you if the pull request looks as expected, if you have a tough time enhancing this pull request feel free to shout and I'll give a hand. |
Acceptance test output: % make testacc TESTARGS='-run=TestAccSiteVPNCustomerGateway_certificate' PKG=ec2 ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/ec2/... -v -count 1 -parallel 2 -run=TestAccSiteVPNCustomerGateway_certificate -timeout 180m === RUN TestAccSiteVPNCustomerGateway_certificate === PAUSE TestAccSiteVPNCustomerGateway_certificate === CONT TestAccSiteVPNCustomerGateway_certificate --- PASS: TestAccSiteVPNCustomerGateway_certificate (88.60s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/ec2 93.560s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀.
% make testacc TESTARGS='-run=TestAccSiteVPNCustomerGateway' PKG=ec2 ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/ec2/... -v -count 1 -parallel 2 -run=TestAccSiteVPNCustomerGateway -timeout 180m
=== RUN TestAccSiteVPNCustomerGatewayDataSource_filter
=== PAUSE TestAccSiteVPNCustomerGatewayDataSource_filter
=== RUN TestAccSiteVPNCustomerGatewayDataSource_id
=== PAUSE TestAccSiteVPNCustomerGatewayDataSource_id
=== RUN TestAccSiteVPNCustomerGateway_basic
=== PAUSE TestAccSiteVPNCustomerGateway_basic
=== RUN TestAccSiteVPNCustomerGateway_disappears
=== PAUSE TestAccSiteVPNCustomerGateway_disappears
=== RUN TestAccSiteVPNCustomerGateway_tags
=== PAUSE TestAccSiteVPNCustomerGateway_tags
=== RUN TestAccSiteVPNCustomerGateway_deviceName
=== PAUSE TestAccSiteVPNCustomerGateway_deviceName
=== RUN TestAccSiteVPNCustomerGateway_4ByteASN
=== PAUSE TestAccSiteVPNCustomerGateway_4ByteASN
=== RUN TestAccSiteVPNCustomerGateway_certificate
=== PAUSE TestAccSiteVPNCustomerGateway_certificate
=== CONT TestAccSiteVPNCustomerGatewayDataSource_filter
=== CONT TestAccSiteVPNCustomerGateway_tags
--- PASS: TestAccSiteVPNCustomerGatewayDataSource_filter (29.01s)
=== CONT TestAccSiteVPNCustomerGateway_basic
--- PASS: TestAccSiteVPNCustomerGateway_tags (57.53s)
=== CONT TestAccSiteVPNCustomerGateway_disappears
--- PASS: TestAccSiteVPNCustomerGateway_basic (29.37s)
=== CONT TestAccSiteVPNCustomerGatewayDataSource_id
--- PASS: TestAccSiteVPNCustomerGateway_disappears (24.12s)
=== CONT TestAccSiteVPNCustomerGateway_4ByteASN
--- PASS: TestAccSiteVPNCustomerGatewayDataSource_id (27.01s)
=== CONT TestAccSiteVPNCustomerGateway_certificate
--- PASS: TestAccSiteVPNCustomerGateway_4ByteASN (29.13s)
=== CONT TestAccSiteVPNCustomerGateway_deviceName
--- PASS: TestAccSiteVPNCustomerGateway_deviceName (28.59s)
--- PASS: TestAccSiteVPNCustomerGateway_certificate (88.01s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/ec2 178.692s
@dionasaur Thanks for the contribution 🎉 👏. |
@dionasaur, for future contributions, you can run
This runs every test in the ec2 package that matches |
This functionality has been released in v4.40.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. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Line 56 should be changed to optional as this is not a required field in the AWS SDK API. Having this as required breaks terraform applies for certificate based vpn connections.
Community Note
Relates #10548.
Output from acceptance testing: