When address munging fails, there is no guidance as to where the failure actually occured about puppetlabs-firewall HOT 21 CLOSED

Can you be more specific, perhaps provide an example?

from puppetlabs-firewall.

Sorry, should have done that first time around. I am starting to think this stems from a fundamental misunderstanding of how puppet runs though!

In my set up I am collecting hostnames and populating the hosts file with them. I am using hostnames in the firewall module to set rules e.g.

firewall { "115 allow SSH from backup":
    source  => "backup",
    port    => "22",
    action  => "accept",
}

I've put the hostname collection in an initial stage, and the firewall stuff in a later stage, so when I add a new machine and puppet runs for the first time I'd expect it to collect the IP address for the backup machine first then the firewall stuff should be able to resolve the source name ok. What I actually get is an error before any of the run stages get to start:

Error: Failed to apply catalog: Parameter source failed on Firewall[115 allow SSH from backup]: Munging failed for value "backup" in class source: no address for backup

This then kills the whole puppet run on the client. It's like the firewall module is resolving names before the module is applied.

from puppetlabs-firewall.

@OllyButters I think its because we do the resolution in the type before applying the rule (ie. its done on the puppet master), which kind of sucks in your case, because I'm guessing you want the rule to be laid out directly to the iptables command, and let iptables do the resolution. We don't have a switch or option to do that today.

from puppetlabs-firewall.

ok, fair enough. I can work around it now that I know what's happening. Thanks for looking at it.

from puppetlabs-firewall.

@OllyButters raise a feature request if you like.

from puppetlabs-firewall.

Probably not worth it - it's a five minute fix for me (put a default entry for each of my machines in the hosts file of my vanilla OS image, after a couple of runs a newly deployed one will sort itself out). What might be useful is to make the client give a warning if it can't do the name resolving instead of stopping the whole puppet run.

from puppetlabs-firewall.

@OllyButters Hmm. I think because its done in the type, I'm not entirely sure we can do that. Its a compilation error, and those fail hard. If it was done in the provider we wouldn't have such an issue, as we can just fail the resource. Hmm.

from puppetlabs-firewall.

I just wrote a dnslookup() parser function to handle it for me during
compilation... i made it spit out nice error if it fails.

Curtis Ruck
Anytime: 210-857-1126

On Fri, Apr 12, 2013 at 9:21 AM, Ken Barber [email protected]:

@OllyButters https://github.com/OllyButters Hmm. I think because its
done in the type, I'm not entirely sure we can do that. Its a compilation
error, and those fail hard. If it was done in the provider we wouldn't have
such an issue, as we can just fail the resource. Hmm.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/157#issuecomment-16292341
.

from puppetlabs-firewall.

@ruckc but it still fails hard right?

from puppetlabs-firewall.

@ruckc maybe we just shouldn't do the lookup ourselves and pass on what users give us. Telling them to 'use your own resolver' like your dnslookup() suggestion for server side resolution ...

from puppetlabs-firewall.

I would agree, when you first added the address munging last summer/fall it
broke quite a bit which is why I went with my dnslookup() to handle my own
munging. Would this be something that should be incorporated into the
module to allow different types of user controlled munging?

Curtis Ruck
Anytime: 210-857-1126

On Fri, Apr 12, 2013 at 9:37 AM, Ken Barber [email protected]:

@ruckc https://github.com/ruckc maybe we just shouldn't do the lookup
ourselves and pass on what users give us. Telling them to 'use your own
resolver' like your dnslookup() suggestion for server side resolution ...

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/157#issuecomment-16293162
.

from puppetlabs-firewall.

Turns out I can't fix this by putting some default values in hosts files.

In the end I edited the host_to_ip function so it does an initial check of the value passed into it, if it can't resolve it it then it adds a dummy value to it. This then gets passed to the rest of the function. This way it won't fall over on getaddress if it can't resolve it. This should work fine for me for now, but I agree that allowing the use of our own resolving mechanism would be better.

from puppetlabs-firewall.

@kbarber I'm not certain I agree about the name resolution being done on the puppet master. Take an example set up with a puppet master and two clients - hostA and hostB. If I am just using /etc/hosts files for the name resolution and all three have a fully populated list of each other then on hostA I can say something like

firewall { "115 allow SSH from hostB":
source => "hostB",
port => "22",
action => "accept",
}

and all is fine. If I delete the entry for hostB from the /etc/hosts file on hostA then it falls over at the name resolution stage. Surly this means the name resolution is happening on hostA?

from puppetlabs-firewall.

I disagree with name munging done on the puppet master at all. Split horizon dns can often mean that the hostname may not resolve to the same address on a client as it does on the master. I use that myself frequently for specifying a router, nas drive, even printer.

from puppetlabs-firewall.

I think this is an issue for me as well. I need the agent to pull down its /etc/hosts file first before the puppet firewall module is called. Setting this up in run stages does not help, as @OllyButters mentioned.

Instead of failing hard when host_to_ip cannot resolve a hostname, can it just spit out a warning? @OllyButters, can you share your fix?

from puppetlabs-firewall.

@snobear - This is the hack I put in. It's in lib/puppet/util/firewall.rb and version 0.2.1 of the firewall module. If you ask for a hostname that isn't in the hosts file it just uses 127.0.0.1. After a cycle or two the real IP addresses get collected by puppetDB and the hosts file gets them. The firewall then sorts itself out on the next run. It's an ugly hack, and I'm not proud of it!

def host_to_ip(value)
    begin
      ##################################
      #ollys hack
      #Definitely needs more work and will probably break under certain situations. 
      if value =~ /\//
        #cidr notation input, well a slash at least.
        #Pass through and do nothing
      else
    #get the hostname from the hosts file
        hosts=`getent hosts #{value}`
        #Set a default value if empty return 127.0.0.1
        value="127.0.0.1"
        #Check there is something returned      
        if hosts.to_s != ''
          #split on spaces - take the first
          host_array = hosts.split(' ')
          value=host_array.first
        end
      end
      #End ollys hack
      #################################
      value = Puppet::Util::IPCidr.new(value)
    rescue
      value = Puppet::Util::IPCidr.new(Resolv.getaddress(value))
    end

    return nil if value.prefixlen == 0
    value.cidr
  end

from puppetlabs-firewall.

Oh OK, I didn't realized the fix was in the recent version (maybe close this issue)? Thanks for the update.

from puppetlabs-firewall.

@snobear - sorry, I don't think I was terribly clear above. I am using version 0.2.1 and I have edited it on my local install to have the above code. This is not in the main version, and given how ugly a hack this is I'd be surprised if it ever did end up there.

from puppetlabs-firewall.

@OllyButters - Gotcha. I ended up using a similar hack. in lib/puppet/type/firewall.rb:

munge do |value|
  begin
    @resource.host_to_ip(value)
  rescue Exception => e
    #self.fail("host_to_ip failed for #{value}, exception #{e}")
    "172.0.0.0"
  end
end

and did the same for the other munge do block a little further down. I think a better solution would be to not fail here and just print a warning. Instead of self.fail, whats the way to just spit out a warning or info in puppet or ruby? self.warn?

from puppetlabs-firewall.

So it look's like the fixes are still trying to resolve the host to ip? What if you're using a host because the ip can change? I have dynamic dns setup on my laptop so I can access stuff where ever I am (with a iptables reload). Is this really bad? Should I be doing something else or should the source not resolve the hostname so it's left to the firewall to do it?

from puppetlabs-firewall.

Hello! We are doing some house keeping and noticed that this issue has been open for a long time.

We're going to close it but please do raise another issue if the issue still persists. 😄

from puppetlabs-firewall.