Bug 1753930: Adds support for trailing dot in noProxy domain names #2405
Conversation
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@danehans: This pull request references Bugzilla bug 1753930, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
size/S
|
I need to get further verification on the bz. It does not appear that a trailing dot is required to no_proxy google metadata. /hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
/approve What was the original motivation to not accept a trailing dot? |
openshift-ci-robot
added
the
approved
| {"1.2.3.4", true}, | ||
| {"1.2.3.4.", false}, | ||
| {"abc.", false}, | ||
| {"1.2.3.4.", true}, |
There was a problem hiding this comment.
Is everything a string or do IPs have special handling?
There was a problem hiding this comment.
1.2.3.4. is a valid hostname if we allow a trailing dot. rfc 1123 is being used as the basis for domain name validation:
One aspect of host name syntax is hereby changed: the restriction on the first character is relaxed to allow either a letter or a digit. Host software MUST support this more liberal syntax.
Previously the proceeding dot was stripped prior to rfc 1123 validation, so .1.2.3.4 was and is still considered a valid name. Now the trailing dot will be stripped prior to rfc 1123 validation, so .1.2.3.4, 1.2.3.4. and .1.2.3.4. are also considered valid.
There was a problem hiding this comment.
The values are also run against net.ParseCIDR, but since .1.2.3.4, 1.2.3.4. and .1.2.3.4. are considered valid by rfc 1123 after stripping proceeding/trailing dots, they are considered valid.
|
is this code reused by the proxy controller? if not, does that validation logic also have to change? |
|
/retest |
|
@bparees openshift/cluster-network-operator#325 adds GCP metadata hostnames to default noProxy. |
|
@sdodson ptal at https://bugzilla.redhat.com/show_bug.cgi?id=1753930#c10 for responses to your PR comments. |
what does that have to do w/ supporting/tolerating trailing dots during validation? |
@bparees ptal at https://bugzilla.redhat.com/show_bug.cgi?id=1753930#c10 regarding trailing dot support. |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/test e2e-aws-upgrade |
|
/test e2e-aws-upgrade |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danehans, sdodson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/cherry-pick release-4.2 |
|
@danehans: new pull request created: #2464 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
7 participants
Previously a domain name with a trailing dot would be considered invalid. This PR adds support for noProxy domain names that end with a trailing dot.
/cc @sdodson @bparees