-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
r/api_gateway_domain_name: Add support for Regional API Endpoints #2290
r/api_gateway_domain_name: Add support for Regional API Endpoints #2290
Conversation
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.
Hi @xmackex
This looks good :), Thanks for the work! 👍
Just left a few comments, mostly nitpicks.
Can you add acceptance tests here so that we can ensure consistency & stability over time?
Also, can you add the related documentation?
Thanks!
"endpoint_configuration": { | ||
Type: schema.TypeSet, | ||
//Type: schema.TypeSet, | ||
//Optional: true, |
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.
These 2 lines should be removed
"regional_certificate_name": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
ConflictsWith: []string{"certificate_arn", "regional_certificate_name", "certificate_name"}, |
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.
Nitpick: don't you want to conflict with regional_certificate_arn
instead?
@@ -61,7 +75,12 @@ func resourceAwsApiGatewayDomainName() *schema.Resource { | |||
"certificate_arn": { | |||
Type: schema.TypeString, | |||
Optional: true, | |||
ConflictsWith: []string{"certificate_body", "certificate_chain", "certificate_name", "certificate_private_key"}, | |||
ConflictsWith: []string{"certificate_body", "certificate_chain", "certificate_name", "certificate_private_key", "regional_certificate_arn"}, |
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.
Don't we want to conflict with regional_certificate_name
also?
"regional_certificate_arn": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
ConflictsWith: []string{"certificate_body", "certificate_chain", "certificate_name", "certificate_private_key", "regional_certificate_name"}, |
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.
Don't we want to conflict with certificate_arn also?
This functionality was implemented in #2866 and will release in v1.20.0 of the AWS provider later this week. |
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! |
Added support for Regional Endpoints in API GW