remove network_public_ip fact

[igalic]

this fact seems to be going against the general philosophy and
methodology of the module: it's accessing an external unknown and
potentially untrusted third-party services.

as there is no way to partially disable facts in puppet, it is probably
better, albeit more conservative, to remove this fact.

[igalic]

@wolfspyre ping, i'm putting this PR out there for discussion and i'd like to have some feedback from you, as you are the original author.

[igalic]

@rdark ping, you're contributing #65, which seems a rather big bandaid for something that in my very conservative world-view is a bad idea in general.

[rdark]

@igalic I can see where you're coming from, and I agree in principal; although I would argue that this fact gives a useful deployment-agnostic way to retrieve the current public IP for a node (at least one that has a simple network topology).

Perhaps it should be moved out to a standalone module? It could also do with moving to a different provider and a rewrite for facter 2 layout (see the ec2 fact in facter 2 core for an example).

Re: #65 - Cache failback for facts, as well as more general fact caching is something that has been discussed by puppet-dev for the past 4+ years, but it's never really successfully implemented. I was hoping to see it make an appearance in facter 2, but it's still not there. I would really like to see it implemented in facter core, but in leui of this we need band-aid solutions like this.

E.G: https://tickets.puppetlabs.com/browse/FACT-348

[igalic]

+1 on moving to a standalone (different?) module.

[wolfspyre]

The code for that particular app (on google appengine) is here: https://github.com/jcsalterego/ip-echo

It was chosen because latency was low across most of the internet (tested from NJ, SJC, AUS, LON, SHA, and a few other places) unlike many other IP discovery tools which had higher latency in some regions of the world.

I'm uncertain what you mean about going against the general philosophy of the module. The module attempts to manage the network stack of nodes, and provides information about the configured interface(s). I consider it relevant, and useful to know what address The Internet sees a node as.

I'm open to being convinced that the code should be removed, but I'm not there yet. What benefit do you see in removing this functionality?

[Xylakant]

I'd like to add that this fact fails horribly [1] for machines that are not allowed to connect to random services on the internet. It also potentially leaks information about machines existing at a certain location. I'm in favor of removing it.

[1] it times out and throws errors.

[igalic]

so it's been a while, that i talked to @adrienthebo at puppetconf, and some of the things he said kind of made sense. he also told me he had talked to @wolfspyre about this.

i'll try to restate my reasoning behind this pull request:

first of all: i fully agree that knowing the external ip address is extremely useful in many, if not all cases.

however, it is possible, and in my opinion should remain possible, to setup machines without direct access to the internet.

this especially "useful" in restricted environments (anything government contract, or, say, PCI environments)

• since it's not possible to (partially) disable facts, and since
• this module is enormously useful to people even without the fact »bundled«,

as such i propose,

• rather than not using the module at all,
• or maintaining an own fork, or similar

to remove this fact from this module, and move it into a separate module

[adrienthebo]

Merged per the reasons given above.