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
Remove short-names for gateways with all-numeric Top Level Domains #164
Conversation
The following is the coverage report on the affected files.
|
|
||
// Validate that the Top Level Domain of a given hostname is not fully numeric. | ||
// Example: '1234' is an invalid TLD | ||
func isValidTopLevelDomain(domain string) bool { |
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.
We may want to check if there is any other invalid format. But it does not need to be included in this PR.
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.
The error being thrown was from this code I think - https://github.com/istio/istio/blob/eb63aae13235c98f99cea6591d4401c569576949/pkg/config/validation/validation.go#L170
There are a few more validation checks for the entire domain being done here, like the length, being compared to a wildcard domain etc. But if the base domain here is valid, for the trimmed domains to be invalid, only the TLD seemed relevant. But the assumption here is that we know that the base domain is valid, which I'm not sure of
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shreejad, ZhiminXiang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Since I didn't have time to comment :/
Fixes #7742 in Knative Serving
While exposing short-names for cluster local gateways, validate that the trimmed domain name does not have an all-numeric Top Level Domain.