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.
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
Add support for route53 in the us-isob-east-1 region #703
Add support for route53 in the us-isob-east-1 region #703
Changes from 1 commit
91a8318
b468cd5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Does the AWS behavior in C2S and SC2S follow other partitions where there is one region used for global resources? In particular, if the region used for the cluster is us-iso-west-1, will the us-iso-east-1 region need to be used when querying the resource tagging API for route53 resources?
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 have to ask AWS about this. I will submit a support case to AWS when I go on site today but might not hear back from them until next week.
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 submitted a case to AWS on this topic and heard back today. The "correct" method is to point all route53 requests to route53.c2s.ic.gov but AWS is leaving the route53.us-iso-east-1.c2s.ic.gov alias in place because it has been around for so long that they don't want to break anything by removing it. This means that I need to change the PR because the us-iso-west-1 region will result in route53.us-iso-west-1.cs2.ic.gov which will not exist.
I could change the code to say, if isoB, use route53.sc2s.sgov.gov, if iso use route53.c2s.ic.gov
Thoughts?
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 don't think that we need any of the special logic in this switch any more, regardless of the partition. The AWS SDK selects the correct endpoints to use, and we should be making use of the selections made by the SDK.
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.
cluster-ingress-operator/vendor/github.com/aws/aws-sdk-go/aws/endpoints/defaults.go
Lines 10893 to 10905 in fe11656
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 suppose we can save the removal of redundant logic for future work.