search

NeoAssist / docker-keepalived

Dockerized keepalived to ease HA in deployments with multiple hosts. Provides failover for Virtual IPs (VIP) to be always online even if a host fails. Initially aimed to help Rancher HA deployments
MIT License
65 stars 37 forks source link

Fix VIRTUAL_IP regex #14

Closed cavemandaveman closed 7 years ago

cavemandaveman commented 7 years ago

The regex wouldn't allow for 0s in any field. For example, if I need to use 192.168.0.11 (a valid IP) it would throw an error. It's a lot uglier, but it works!

cavemandaveman commented 7 years ago

Actually, hold on this PR. I'm running into build issues when it's trying to install keepalived:

ERROR: unsatisfiable constraints:
  add-0:
    masked in: cache
    satisfies: world[add]
  keepalived (missing):
    required by:
cavemandaveman commented 7 years ago

Okay the above issue is fixed if you merge in #15. Once that happens, I can rebase this.

sjiveson commented 7 years ago

Thanks. Merged that one. Just checking out this one.

sjiveson commented 7 years ago

Looks like your regex allows a first octet of 224 and upwards, just checkout out the rest

sjiveson commented 7 years ago

I think this works, what do you think?

^(([1-9]|[1-9][0-9]|1[0-9]{2}|2[0-2][0-3])\.)(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-5][0-5])\.){2}([1-9]|[1-9][0-9]|1[0-9]{2}|2[0-5][0-5])$

cavemandaveman commented 7 years ago

Any octet can be 0-255, why are you limiting the first at 223?

One other option I came across is ipcalc which is preinstalled in Alpine. You can pass the IP and mask and it will tell you if it's valid or not. A valid IP will exit 0:

/ # ipcalc -hp 192.168.0.11/23
PREFIX=23
HOSTNAME=192.168.0.11

and an invlaid IP will exit 1:

/ # ipcalc -hp 255.255.255.256/32
ipcalc: bad IP address: 255.255.255.256

This could replace the two regex conditionals for VIRTUAL_IP and VIRTUAL_MASK with a single command.

sjiveson commented 7 years ago

Anything above 223 on the first octet is a multicast address or reserved and shouldn't be used.

I'd rather stick with a regex to keep things as light as possible as others may not use alpine base. However, I'm happy to be swayed if others agree with @cavemandaveman - anyone?

cavemandaveman commented 7 years ago

I'm okay with regex, just threw out ipcalc as a suggestion. I updated this branch to include your revised expression.

sjiveson commented 7 years ago

Thanks, I thought I'd do some further testing, it looks like 10.11.022.3 is valid which isn't good, fixing...

sjiveson commented 7 years ago

Scratch that, it's good. Let's see when it next crops up! :-)