Fix networkRange
race condition and global state corruption
#945
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.
While the
networkRange
function calculateslastIP
, it usesnet.IPv4zero
ornet.IPv6zero
as a starting value from which itbuilds the final last IP. The issue is that these are global
variables defined in the standard library, and modifying them affects
all future users of those variables across the program.
This leads to a not-so-rare race condition when creating multiple
libvirt networks. These networks are being created concurrently and they
both race modifying that global variable. Their
lastIP
DHCPconfiguration gets mixed up as result. The manner in which the
configuration gets mixed up is unpredictable, but it could lead to a
corrupt configuration that cannot be applied.
The solution is to copy the zero IP addresses to a new variable rather
than use them directly. Instead for simplicity I just instantiate an
empty slice with the same length as the net IP, which would be filled
with zeroes anyway.
This commit also solves another unrelated bug in
getNetworkIPConfig
where the
^
operator was used as if it's a power operator, while inreality it's a bitwise-xor that leads to slightly incorrect results.
This makes the validation only slightly wrong and is overly strict (e.g.
it will not allow you to create
/28
IPv4 network even though suchnetwork has far more available addresses than the condition blocks)
A similar race condition can be simply reproduced and visualized with
this short go code:
Then run:
To see it happen for short moments.
Alternatively, create this
main.tf
file:And run:
Until the bug reproduces with the following error:
Please make sure you read the contributor documentation before opening a Pull Request.