chr4-cookbooks / iptables-ng

Cookbook to maintain iptables rules and policies on different platforms, respecting the way the os handles these settings.
GNU General Public License v3.0
38 stars 33 forks source link

rule resources is flagged as updated every converge #38

Closed dpetzel closed 9 years ago

dpetzel commented 9 years ago

We run the updated-resources handler on our nodes and we recently started using this cookbook (nice work btw!). One thing we've noticed is that the all iptables-ng_rule resources show up in the report (which means they were flagged as updated) each and every converge.

I dug into this a little bit and figured out why, but I'm not sure how you'd prefer to fix it. The following line results in the resource ALWAYS getting marked as updated:

new_resource.updated_by_last_action(true) if edit_rule(:create)

At first glance this all looks well and good, but what is happening is that the edit_rule method is not returning the r.updated_by_last_action? value as the code seems to assume, but instead is returning an Array of IP versions. Since the method is returning the Array, rather than a boolean, this is causing an evaluation of true

I think the easiest fix would be to move the evaluation inside the edit_rule method, so things would look more like this:

action :create do
  edit_rule(:create)
end

def edit_rule(exec_action)
  # all the same stuff it already does
    r = file rule_path do
      owner    'root'
      group    node['root_group']
      mode     00600
      content  rule_file
      notifies :create, 'ruby_block[create_rules]', :delayed
      notifies :create, 'ruby_block[restart_iptables]', :delayed
      action   exec_action
    end

    new_resource.updated_by_last_action(true) if r.updated_by_last_action?
end
chr4 commented 9 years ago

You are right, this is clearly a bug. Thanks for spotting and debugging! I agree to your solution, this looks pretty clean and is nicer than adding an extra variable storing the status. I'll test this and then come back to you!

chr4 commented 9 years ago

Can you check whether this fixes it? https://github.com/chr4-cookbooks/iptables-ng/pull/39

dpetzel commented 9 years ago

Looks good on my end, just converged a couple nodes with this change and no longer seeing the resources marked as updated each run.

chr4 commented 9 years ago

Thanks. I'll close this in the favor of the pull request and run some more tests on it, resp. make foodcritic shut up.