-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
providers/aws: Add hosted_zone_id and region to attributes #1865
providers/aws: Add hosted_zone_id and region to attributes #1865
Conversation
"ap-northeast-1": "Z2M4EHUR26P7ZW", | ||
"sa-east-1": "Z7KQH4QJS55SO", | ||
"us-gov-west-1": "Z31GFT0UA1I2HV", | ||
} |
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.
This is unable to be generated from the API json. Attempted to add to aws-sdk in aws/aws-sdk-go#222, but that was rejected
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.
Can you document here the why (they're looking for a generated means, etc), and maybe a TODO
so we know that this should ideally go away in the future when aws-sdk-go finally picks it up?
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.
Added the docs. Will they pick it up? Doesn't seem likely, unless the API schema somehow exposes it.
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.
@justincampbell I was asking our contact at AWS about this, linking him directly to the related PR at aws-sdk-go.
He was assuring me that if these zone IDs were to change, it would most likely mean changing the domain for S3 websites and/or CloudFront global domain (unlikely to happen in one day, if ever) and more importantly if that ever happens, they would give more than enough time to users to adopt this change.
He also said they may expose it via API at some point, but from my experience, it could mean 6 months, but also 6 years, or even never. There isn't any specific ETA nor guarantee.
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.
I think your comment here is well written, I can't think of a better one.
/cc @radeksimko |
0787c50
to
35692c6
Compare
35692c6
to
73651e2
Compare
) | ||
|
||
func TestHostedZoneIDForRegion(t *testing.T) { | ||
if r := HostedZoneIDForRegion("us-east-1"); r != "Z3AQBSTGFYJSTF" { |
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.
Testing a lookup function feels a bit unnecessary, but it's no harm :)
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.
It was slightly more complex previously because it normalized ""
to "us-east-1"
, but yes a little overkill now.
providers/aws: Add hosted_zone_id and region to attributes
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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
As requested in #1769