Fix handling of forwarders addresses with custom port. #6269
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rcritten
reviewed
May 19, 2022
rjeffman
force-pushed
the
dns-fix-f36-dnspython-2-2
branch
6 times, most recently
from
13d536b
to
fc9031d
Compare
Related to: https://pagure.io/freeipa/issue/9158 Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
rjeffman
force-pushed
the
dns-fix-f36-dnspython-2-2
branch
3 times, most recently
from
a09b9f5
to
9756730
Compare
Related to: https://pagure.io/freeipa/issue/9158 Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
Related to: https://pagure.io/freeipa/issue/9158 Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
When setting a DNS forwarder, IPA allows the use of a custom port using the format '<ip> port <port>', and this configuration is validated with dnspython to ensure the forwarder is resolvable. Starting with dnspython 2.2.0 the Resolver.nameservers property, used to resolve the forwarders IP address, validates the IP address when the value is assigned to property, and as the forwarder format is not an IP address, it fails and a ValueError exception is raised. Modifying the way forwarders are handled when validating them prevents the exception to be raised, and test for the correct port. Fixes: https://pagure.io/freeipa/issue/9158 Signed-off-by: Rafael Guterres Jeffman <rjeffman@redhat.com>
rjeffman
force-pushed
the
dns-fix-f36-dnspython-2-2
branch
from
df5327d
to
af65a3e
Compare
rjeffman
force-pushed
the
dns-fix-f36-dnspython-2-2
branch
from
f27510e
to
2ecce7c
Compare
rjeffman
force-pushed
the
dns-fix-f36-dnspython-2-2
branch
2 times, most recently
from
ee14537
to
29119ba
Compare
rjeffman
force-pushed
the
dns-fix-f36-dnspython-2-2
branch
from
29119ba
to
2a42f87
Compare
rcritten
reviewed
Jun 22, 2022
| # case, split the string and add the IP part to res.nameservers, | ||
| # and the ip:port pair to res.nameserver_ports dict. | ||
| nameserver_ip = re.sub(r'\s+', ' ', nameserver_ip.strip()) | ||
| nameserver_ip, *port = nameserver_ip.split(" port ") |
There was a problem hiding this comment.
For ease of testing I wonder if this should be put into a separate function. Then you could use straight pytest to pass in a variety of values with ease.
There was a problem hiding this comment.
Wouldn't it make more sense to fix this in DNSResolver?
Here is an example: master...t-woerner:freeipa:fix_dns_resolver_for_nameservers_with_ports
|
bind is allowing to specify the port for a forwarders with "port " additionally to the IP addresses. But dnspython that is used to verify the list of forwarders (nameservers) is only allowing to have IP addresses in the list. With dnspython version 2.20 there is a new validator in dns.resolver.BaseResolver that ensures this. |
|
Dropping this PR in favor of #6408 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When setting a DNS forwarder, IPA allows the use of a custom port using
the format ' port ', and this configuration is validated with
dnspython to ensure the forwarder is resolvable.
Starting with dnspython 2.2.0 the Resolver.nameservers property, used
to resolve the forwarders IP address, validates the IP address when
the value is assigned to property, and as the forwarder format is not
an IP address, it fails and a ValueError exception is raised.
Modifying the way forwarders are handled when validating them prevents
the exception to be raised, and test for the correct port.
Fixes: https://pagure.io/freeipa/issue/9158