Skip to content

Properly check for ipset ranges, fixes #26453#26535

Merged
jfindlay merged 1 commit intosaltstack:developfrom
bobrik:ipset-ranges
Aug 21, 2015
Merged

Properly check for ipset ranges, fixes #26453#26535
jfindlay merged 1 commit intosaltstack:developfrom
bobrik:ipset-ranges

Conversation

@bobrik
Copy link
Copy Markdown
Contributor

@bobrik bobrik commented Aug 20, 2015

This add support for idempotent runs of ipset.present state if ranges are used as entries. IP1-IP2 and IP/MASK variants are supported.

Not sure if the branch is correct, though. I'd like to see this in the next release (2015.8.0).

@jfindlay jfindlay added Minor Change Platform Relates to OS, containers, platform-based utilities like FS, system based apps Execution-Module labels Aug 20, 2015
@jfindlay
Copy link
Copy Markdown
Contributor

Nice, thanks, @bobrik.

@jfindlay
Copy link
Copy Markdown
Contributor

@bobrik, there are some lint errors (look for proposed fix:). I think we should also add some documentation and examples to the check function that illustrates the new range and subnet checking support. Otherwise looks great.

This add support for idempotent runs of `ipset.present` state
if ranges are used as entries. IP1-IP2 and IP/MASK variants
are supported.
@bobrik
Copy link
Copy Markdown
Contributor Author

bobrik commented Aug 21, 2015

PR is updated and linter is happy now.

@jfindlay
Copy link
Copy Markdown
Contributor

Thanks @bobrik.

jfindlay added a commit that referenced this pull request Aug 21, 2015
Properly check for ipset ranges, fixes #26453
@jfindlay jfindlay merged commit 4703ac3 into saltstack:develop Aug 21, 2015
@bobrik
Copy link
Copy Markdown
Contributor Author

bobrik commented Aug 21, 2015

@jfindlay any chance to see this in 2015.8?

@jfindlay jfindlay added the bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch label Aug 21, 2015
@jfindlay
Copy link
Copy Markdown
Contributor

@bobrik, I think we can backport it, but we need to add documentation about the added range checking feature.

@rallytime rallytime added Bugfix - [Done] back-ported and removed bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch labels Aug 21, 2015
@garethgreenaway
Copy link
Copy Markdown
Contributor

@bobrik I'm curious what version of ipset you tested this with as I'm seeing some different results with your change.

@bobrik
Copy link
Copy Markdown
Contributor Author

bobrik commented Aug 26, 2015

@garethgreenaway my change doesn't touch anything that calls ipset. ipset.check happens entirely in python. I'm using ipset v6.23 on jessie.

How different are your results?

@garethgreenaway
Copy link
Copy Markdown
Contributor

ipset.check doesn't call the ipset binary but the call to _find_set_members within ipset.check does. The major different I'm seeing is related to the IP range. With ipset v6.25.1 when an ip range is specified the result is a network (or series of networks) with prefix lengths, eg. 192.168.1.0/24, not a list of IPs. So ipset.check never matches following the initial insertion of the ipset rules. I'll spin up a jessie VM and see if I can see the difference. Thanks!

@bobrik
Copy link
Copy Markdown
Contributor Author

bobrik commented Aug 26, 2015

@garethgreenaway it can depend on ipset type, I use hash:ip and when you insert range you always get list of IPs, not networks, back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Execution-Module Platform Relates to OS, containers, platform-based utilities like FS, system based apps

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants