Merge ~akaris/cloud-init:bug1684349 into cloud-init:master

[Andreas Karis]

Fix mask2cidr error with integer value

mask2cidr error with integer value - argument of type 'int' is not iterable. Method mask2cidr is not type safe. It tries to take into account that this can be a prefix (so it does not contain ':' nor '.') and then simly return mask without action. The problem is that if mask is an integer, then this throws an error, so this patch casts mask to type string in the if/elif.

LP: #1684349

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:4f75b2d0962a48abbaa6af78d663ac4a30af356c
https://jenkins.ubuntu.com/server/job/cloud-init-ci/264/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/264
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/264
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/264
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/264
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/264

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/264/rebuild

review: Approve (continuous-integration)

[Lars Kellogg-Stedman (larsks)]

If code is actually calling this with an integer argument, this looks like a fine temporary fix.

I say "temporary" because I'm a little confused by the return value of this function, too, which looks like it may be an int (when it calls ipv4mask2cidr) or a string (when it calls ipv6mask2cidr) or whatever "mask" happens to me if either of the previous conditionals fail. I will try to ping smoser about it tomorrow.

[Dimitri John Ledkov (xnox)]

It is called with whatever the config source provided.

It may be called with cidr, it could be an ipv6 netmask, or an ipv4 netmask.

Always return cidr.

I guess it would be better named any2cidr()

[Chad Smith (chad.smith)]

Generally, if we are changing functionality, I'd love to see a unit test exercising this functionality so we don't have to test it with integration tests.

It appears that this changeset will only pass this type problem down into ipv4mask2cidr and ipv6mask2cidr which will fall over with the same traceback as they attempt the same string search in mask(which is still an int).

Scott mentioned in IRC today that he's working a branch cleaning up network_state so I'm wondering if we just wait on the cleanup there to see if it addresses this issue.

review: Needs Fixing

[Scott Moser (smoser)]

I've marked this 'merged'
the change was present at
 https://code.launchpad.net/~xnox/cloud-init/+git/cloud-init/+merge/324010

i have a lot of further improvements to that general area, fixing Andreas's issues with the type safety of that function. Theres more to do, but you can see at https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324274

diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
index db3c357..9e9c05a 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -734,9 +734,9 @@ def ipv6mask2cidr(mask):
 
 
 def mask2cidr(mask):
-    if ':' in mask:
+    if ':' in str(mask):
         return ipv6mask2cidr(mask)
-    elif '.' in mask:
+    elif '.' in str(mask):
         return ipv4mask2cidr(mask)
     else:
         return mask