Enable passing a prefix for IP addresses

[mfalesni]

I hope tests say all, intended to replace https://github.com/RedHatQE/cfme_tests/blob/master/utils/randomness.py#L5

btw. what's wrong on map(str, prefix_ipv4 or []) ? Shorter than the list comprehension 😄

[elyezer]

We may need a prefix for ipv6 too. With that said I'm in favor of changing the argument name to prefix and use that for both, ipv4 and ipv6 generation.

A ValueError could be raised if the prefix type is not a list. I will be one person that will try to pass a string instead.

[mfalesni]

@elyezer I can rename it, but I am not entirely familiar with ipv6, so someone else will probably have to do it 😃

[elyezer]

@mfalesni the ipv6 generation is pretty straight forward, and expecting a list you can decrease the number of random generation parts then join all together, the same approach for ipv4 you've done.

[omaciel]

@elyezer @mfalesni does it make sense for both of you to work on this new feature? :)

[mfalesni]

Ok, will take a look on ipv6 too.

[elyezer]

What I thought would be something like:

max_prefix_len = 7 if not ipv6 else 3 - int(ip3)
prefix = [str(field) for field in prefix_ipv4 or []]
prefix_len = len(prefix)
if prefix_len > max_prefix_len:
    raise ValueError('Prefix {} would lead to no randomness at all'.format(repr(prefix)))

if ipv6:
    ipaddr = u':'.join('{0:x}'.format(
        random.randint(0, 2**16 - 1)
    ) for i in range(8 - prefix_len))
    if prefix is not None:
        ipaddr = '{}:{}'.format(':'.join(prefix), ipaddr)
else:
    rng = 3 if ip3 else 4
    ipaddr = u".".join([str(random.randrange(0, 255, 1)) for _ in range(rng - prefix_len)])
    if prefix is not None:
        ipaddr = '{}.{}'.format('.'.join(prefix), ipaddr)
    if ip3:
        ipaddr = ipaddr + u".0"

We can improve the prefix logic in order to check after generating the random part.

[mfalesni]

@elyezer Just pushed something.

[omaciel]

I'd remove the eg. here and add a , right before the etc as such: generation, etc..

[elyezer]

APT

[omaciel]

My previous comment may have gotten squashed, but: what does disabling randomness at all means?

[Ichimonji10]

How about:

:raises: ``ValueError`` if ``prefix`` defines a full IP addess or has no elements.

[mfalesni]

@omaciel If you specify a prefix with length of 4, then there is no randomness involved. Maybe the phrase should be written differently, I'll gladly accept an advice.

[mfalesni]

@Ichimonji10 the :raises: cannot be so simply written because these limits also apply for ipv6 which has 8 fields therefore max. 7 fields of prefix.

[Ichimonji10]

@mfalesni I've updated the suggested docstring. Have a second look.

[mfalesni]

@Ichimonji10 Why no elements? If the prefix is empty, then nothing special happens ...

[Ichimonji10]

@mfalesni The code will not be orthogonal if you allow an empty prefix to be specified. A user can specify an emtpy prefix either by passing in None or by passing in an empty iterable such as [] or (). Non-orthogonal code is harder to understand and more complex.

Now, if you do want to allow empty prefixes, that's fine. It makes a lot of sense, actually. But in that case, disallow None and instead write this:

def gen_ipaddr(ip3=False, ipv6=False, prefix=()):

If you do that, then there is only one way to specify an empty prefix: by passing in an empty iterable.

[mfalesni]

@Ichimonji10 @elyezer @omaciel Updated according to the comments.

[omaciel]

ACK

[elyezer]

ACK

[omaciel]

Thank you @mfalesni

10:00:18 omaciel: mfalesni++
10:00:19 robottelo: mfalesni has 1 point
10:00:56 elyezer: mfalesni++
10:00:57 robottelo: mfalesni has 2 points
10:00:59 omaciel: elyezer: ++