Accept pre-existing rule with invalid name #192
Conversation
This patch fixes up a pre-existing rule whose name does not type-validate with a valid name (typically one without a numeric prefix in the comment). Fixes puppetlabs#116 Signed-off-by: Joe Julian <me@joejulian.name>
|
Can one of the admins verify this patch? |
|
I still haven't built a vm for rspec testing so I don't know if the tests I wrote are any good yet. |
|
@joejulian you don't need to build a VM, just install Vagrant + Virtualbox from dmg, deb or rpm, and follow the instructions: https://github.com/puppetlabs/puppetlabs-firewall#testing ... the necessary VM's get downloaded for you depending on what node set you choose. |
| # Puppet-firewall requires that all rules have comments (resource names) and match this | ||
| # regex and will fail if a rule in iptables does not have a comment. We get around this | ||
| # by appending a high level | ||
| if not /^\d+[[:alpha:][:digit:][:punct:][:space:]]+$/ =~ hash[:name] |
There was a problem hiding this comment.
This looks almost correct, but if am I mistaken, won't the logic of this 'if' always apply even for the empty case which the next is meant to catch? Since hash[:name] == nil or '' will match this expression first ...
You almost want to do:
if ! hash[:name]
...
elsif not /^ ... / =~ hash[:name]
..
end
Don't you? Otherwise cases without a comment will just get a number, without the hexdigest to fill out the text.
|
ok to test |
|
Merged build triggered. (Status: PENDING, Details: null) |
|
Merged build started. (Status: PENDING, Details: http://box.bob.sh:8080/job/puppetlabs-firewall-system/229/) |
|
Merged build finished. (Status: SUCCESS, Details: http://box.bob.sh:8080/job/puppetlabs-firewall-system/229/) |
Accept pre-existing rule with invalid name
This patch fixes up a pre-existing rule whose name does not
type-validate with a valid name (typically one without a numeric
prefix in the comment).
Fixes #116
Signed-off-by: Joe Julian me@joejulian.name