Allow braces to appear in dhclient output

[eb3095]

Proposed Commit Message

Allow braces to appear in dhclient output

dhclient output that contains brackets for pxe variables will break the dhclient parsing regex line. This fix retains the current
functionality while fixing this particular issue.

Not sure if there are any cases I may be missing that the regex there was intended to resolve, but this line passes the tests and
fixes my particular issue with it. It seems to function exactly as it did before from what I can see.

Test Steps

This was the following output that broke this line,

lease {
  interface "enp1s0f0";
  fixed-address 45.63.17.162;
  filename "http://100.100.100.1/getbootconfig.php?mac=${netX/mac}";
  server-name "100.100.100.1";
  option subnet-mask 255.255.254.0;
  option routers 45.63.16.1;
  option dhcp-lease-time 86400;
  option dhcp-message-type 5;
  option domain-name-servers 108.61.10.10;
  option dhcp-server-identifier 169.254.1.1;
  option dhcp-renewal-time 43200;
  option rfc3442-classless-static-routes 0,45,63,16,1,32,169,254,169,254,45,63,16,1;
  option broadcast-address 45.63.17.255;
  option dhcp-rebinding-time 75600;
  option host-name "blankedout";
  renew 4 2021/05/27 03:29:58;
  rebind 4 2021/05/27 14:17:16;
  expire 4 2021/05/27 17:17:16;
}

Checklist:

• [ X ] My code follows the process laid out in the documentation
• [ X ] I have updated or added any unit tests accordingly
• [ X ] I have updated or added any documentation accordingly

[TheRealFalcon]

I suspect the reason for the regex disallowing the } character is to ensure we don't capture the closing } of the lease, which you've successfully modified. However, I'll wait for @blackboxsw to chime in here, since he is the lucky recipient of the git blame.

@eb3095 , in the mean time, would you be able to add/modify a unit test to include the case that generated this issue? It would go in test_dhcp.py

[eb3095]

I suspect the reason for the regex disallowing the } character is to ensure we don't capture the closing } of the lease, which you've successfully modified. However, I'll wait for @blackboxsw to chime in here, since he is the lucky recipient of the git blame.

@eb3095 , in the mean time, would you be able to add/modify a unit test to include the case that generated this issue? It would go in test_dhcp.py

Sure, will do!

[blackboxsw]

@eb3095 thanks for this PR. Changeset looks good and makes sense per ISC dhclient docs. While I think we could still run aground if DHCP output on some environments emit multiple leases on a single line (seems silly) we won't be able to parse that lease format. But, since we don't have any bugs in the last 4 years related to the original changeset, let's not bother unless another issue comes up.

Validated on EC2 that we continue to parse leases appropriately with this changeset.

[blackboxsw]

From man dchp.client I see now that lease declarations such as filename, server-name, script-name can all contain arbitrary "string" values, so trying to limit our regex and stop matching at the first the "}" it encounters will cause some significant issue, as you mentioned.

The description of a lease declaration from man dhclient.conf is

       A  lease  statement  consists  of the lease keyword, followed by a left
       curly brace, followed by one or more lease declaration statements, fol‐
       lowed  by  a  right  curly brace.  The following lease declarations are
       possible:

I can see by the vague text it could be possible that a multiple DHCP lease declarations could be emitted all on one line as in lease { ... } lease { ... }, in that case our regex ending with a curly brace newline "}\n" would continue to break there,... But, I'd say since we have gotten this far on multiple distributions we probably don't need to solve that issue in this PR. We can deal with that if it comes up.

[eb3095]

Just adding to this for future reference when/if it comes up. I saw that also but I have never personally seen it resolve like that, might need a special set of very intentional circumstances. However if it ever does come up I do not think regex is the correct answer. It took me quite some time and fiddling to get this to work, and that would be excessively difficult. I would sooner take a customized algorithmic approach to handling this if it comes up by counting the brackets. Every { adds 1 to a var, every } reduces by one, concluding the parsing only when that var == 0. Again just adding this for the next unfortunate sap left to stare this issue down! If anyone wants me to implement this now however I wouldnt mind doing so and adding the test case for it.