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

Add valid netmask random generator #54

Merged
merged 1 commit into from Oct 6, 2014
Merged

Conversation

elyezer
Copy link
Contributor

@elyezer elyezer commented Oct 6, 2014

I was motivated to create this because this regex used to validate netmasks https://github.com/theforeman/foreman/blob/develop/lib/net/validations.rb#L8.

Also got some info at http://www.iplocation.net/tools/netmask.php.

@Ichimonji10
Copy link
Contributor

The set of network masks generated by this function is incomplete. There are valid uses cases for a /31, /32, /0, /1, etc CIDR. The web page linked to is correct when it states that the IP addresses in that table are "examples of commonly used netmasks." That table is not exhaustive.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.31%) when pulling 6c915ab on elyezer:gen-netmask into 75bef96 on omaciel:master.

@Ichimonji10
Copy link
Contributor

Tools from the standard library can be used to generate subnet masks. For example:

>>> import ipaddress
>>> str(ipaddress.ip_network('0.0.0.0/20').netmask)
'255.255.240.0'

@Ichimonji10
Copy link
Contributor

The above only works on Python 3. I'm not sure if there's a similarly convenient tool in Python 2.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.31%) when pulling 3573791 on elyezer:gen-netmask into 75bef96 on omaciel:master.


For more info: http://www.iplocation.net/tools/netmask.php

:returns: A random valid netmask with the format NNN.NNN.NNN.NNN
Copy link
Contributor

Choose a reason for hiding this comment

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

"The netmask is chosen from :data:fauxfactory.constants.VALID_NETMASKS."

Copy link
Contributor

Choose a reason for hiding this comment

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

I just want to make it very clear to clients that they are not really getting a random netmask. They are getting a random netmask from a subset of all available IPv4 netmasks.

@elyezer elyezer force-pushed the gen-netmask branch 2 times, most recently from 186807a to e0ba773 Compare October 6, 2014 17:40
For more info: http://www.iplocation.net/tools/netmask.php

:returns: The netmask is chosen from
:data:fauxfactory.constants.VALID_NETMASKS.
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll need to surround "fauxfactory.constants.VALID_NETMASKS" with backticks. That didn't show up in my earlier comment. (whoops)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was in doubt about that, thanks for the update.

@Ichimonji10
Copy link
Contributor

ACK pending comment

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.44%) when pulling 186807a on elyezer:gen-netmask into 75bef96 on omaciel:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.31%) when pulling d9fc73c on elyezer:gen-netmask into 75bef96 on omaciel:master.

@@ -511,6 +512,19 @@ def gen_mac(delimiter=":"):
return _make_unicode(mac)


def gen_netmask():
Copy link
Contributor

Choose a reason for hiding this comment

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

Can an issue be created requesting configurable minimum and maximum values for the netmask? A /32 or /0 netmask is valid, and we should be able to generate them, even if those values seem nonsensical.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can imagine something like this:

gen_netmask(min_cidr=0, max_cidr=20)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue #55 created

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 4be42e0 on elyezer:gen-netmask into 75bef96 on omaciel:master.

@Ichimonji10
Copy link
Contributor

FYI, ipaddress is available in Python 2 via a library. See here.

@omaciel
Copy link
Owner

omaciel commented Oct 6, 2014

ACK

omaciel added a commit that referenced this pull request Oct 6, 2014
Add valid netmask random generator
@omaciel omaciel merged commit 713949c into omaciel:master Oct 6, 2014
@elyezer elyezer deleted the gen-netmask branch October 6, 2014 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants