Skip to content
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

Ip addr validation #58

Closed
wants to merge 4 commits into from
Closed

Ip addr validation #58

wants to merge 4 commits into from

Conversation

MartinBasti
Copy link
Contributor

@MartinBasti MartinBasti commented Sep 5, 2016

No description provided.

@ghost ghost self-assigned this Sep 7, 2016
@ghost
Copy link

ghost commented Sep 7, 2016

@mbasti-rh In last patch, please copy-paste the warnings also into replicainstall.py

@@ -55,8 +55,10 @@ def test_ip_address():
('241.1.2.3',),
('169.254.1.2',),
('10.11.12.0/24', (10, 11, 12, 0), 24),
('10.0.0.255', (10, 0, 0, 255), 24),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test fails with:
def check_ipaddress():
try:
ip = ipautil.CheckedIPAddress(addr, match_local=False)
assert ip.words == words and ip.prefixlen == prefixlen
except Exception:

      assert words is None and prefixlen is None

E assert ((10, 0, 0, 255) is None)

CheckedIPAddress(10.0.0.255).prefixlen is 8 not 24

Currently cloud environments uses heavily prefix /32 (/128) what makes
IPA validators to fail. IPA should not care if IP address is network or not.
This commit allows usage of network addresses in:
* host plugin
* dns plugin
* server-installer
* client-installer

https://fedorahosted.org/freeipa/ticket/5814
Currently environments may use prefix /31 on point-to-point connections what
makes IPA validators to fail. IPA should not care if IP address is broadcast
or not. In some cases (when prefix is not specified) IPA cannot decide
properly if broadcast address is really broadcast.

This commit allows usage of broadcast addresses in:
* host plugin
* dns plugin
* server-installer
* client-installer

https://fedorahosted.org/freeipa/ticket/5814
There is no reason (RFC) why we should prevent users to add multicast
addresses to A/AAAA records

https://fedorahosted.org/freeipa/ticket/5814
@martbab
Copy link
Contributor

martbab commented Sep 7, 2016

@mbasti-rh you forgot to copy-paste the code to promote_check function.

@pvoborni ^^ and that's why we need to refactor installer code

@ghost ghost added the ack Pull Request approved, can be merged label Sep 7, 2016
@ghost ghost added the pushed Pull Request has already been pushed label Sep 7, 2016
@ghost ghost closed this Sep 7, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
2 participants