Modules 1192 - rename facts param on actionpolicy::rule to remove conflict with $facts hash

[jantman]

The gist of MODULES-1192 is that trusted_node_data=true is now (3.6) a "recommended and safe" puppet config setting; this enables the $facts hash, which makes $facts a reserved variable and therefore mcollective::actionpolicy::rule generates a compile-time failure.

This branch will probably need some fixup by me, but I've included my intermediate work in order to fully test and convey what I've done:

• pull in puppetlabs_spec_helper from master, to include not-yet-released MODULES-1202 - add module_spec_helper support for 3.6 config items puppetlabs/puppetlabs_spec_helper#70 which lets us configure trusted_node_data
• add missing test environments for 3.5.0 and 3.6.0 with strict_variables set to true; also add 3.6.0 test environments with future parser, and with future parser and trusted_node_data (the latter being the one of relevance to this ticket). IMO, we should keep the first and last of these - 3.5.0 with strict variables set, and 3.6.0 with both future parser and trusted_node_data, the latter being the most future-proof at the moment (as far as I know)
• add spec tests for the mcollective::actionpolicy::rule define, which was previously untested. Add one that uses the facts parameter.
• rename the param on mcollective::actionpolicy::rule from "facts" to "fact_filter" (this is the actual MODULES-1192 work). I've updated the README appropriately for the new param, but I'm not sure what the correct place is to document essentially a breaking change (side note on this - as I mentioned in the Jira ticket, I was unable to find a single piece of public code that uses this param).

[jantman]

Ok, I'll need to revisit this and figure out why travis is so horribly angry, and so vastly different from my local spec runs... probably won't be able to get to it until this evening or tomorrow.

[jantman]

Ok, I've gotten this to the point that the only specs failing are the two for the 3.0.0 Puppet gem, because of "Invalid resource type anchor" - it looks to me that this has been happening for a while?

[hunner]

Oops, I was going to release this...

[hunner]

Released as 0.7.0

[jantman]

fixed the gemfile puppetlabs_spec_helper thing

[jantman]

Ok, the rebase wasn't actually that bad at all, it looks like the only conflicts were in .travis.yml and Gemfile. It was a bit of a pain, but not too time consuming.

[jantman]

@hunner do you remember why this is still just sitting here? I honestly don't remember... was I supposed to do something?

[jantman]

@hunner ping again - is this waiting on me?

[jantman]

Rebase on master and solve merge conflicts in .travis.yml; squash down to one commit

[mojomax]

@jantman just a friendly FYI in case you missed my comment on the MODULES-1192 ticket

you need to update the actionpolicy.erb as well:

diff --git a/modules/mcollective/templates/actionpolicy.erb b/modules/mcollective/templates/actionpolicy.erb
index 893859d..0950e75 100644
--- a/modules/mcollective/templates/actionpolicy.erb
+++ b/modules/mcollective/templates/actionpolicy.erb
@@ -1,7 +1,7 @@
 policy default <%= @data['default'] %>
 <%
     lines = @data['lines'].collect do |line|
-      line.values_at(*%w{ action callerid actions facts classes }).join("\t")
+      line.values_at(*%w{ action callerid actions fact_filter classes }).join("\t")
     end
 -%>
 <%= lines.sort.join("\n") %>

[jantman]

Thanks, @mojomax I'd missed that comment. Fix made, pushed up, testing now.

[duritong]

Any news on getting this merged?

[igalic]

@duritong thanks for the bump.
this will need a rebase. if someone here wants to take it up, i'll be happy to review it again & merge it.

[duritong]

This is not necessary, because the hash still has the key facts:
https://github.com/puppet-community/puppet-mcollective/pull/161/files#diff-26b416a07573e48cd8576ce727abff8aL18

You break the policy file with that change

[jantman]

I can look at this again if nobody else wants to pick it up, but to be honest, it's been so long since I've touched this that I barely remember these portions of code...

[duritong]

I added #209, which is a rebased & fixed PR

[ffrank]

Closing this in favor of #209. Hashtag: Housekeeping.