[FACT-380] Private ipaddress is used as ipaddress fact Created: 2014/03/13  Updated: 2016/06/09  Resolved: 2016/01/27

Status: Closed
Project: Facter
Component/s: None
Affects Version/s: FACT 1.7.4, FACT 2.4.0, FACT 2.4.6
Fix Version/s: FACT 2.4.5

Type: Bug Priority: Major
Reporter: Tomas Barton Assignee: Unassigned
Resolution: Won't Fix Votes: 13
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Blocks
Relates
relates to FACT-1318 update to ipaddress fact in 2.4.5 bre... Closed
relates to FACT-1316 [Facter 2.4.5] On Virtuozzo container... Closed
relates to FACT-1319 ipaddress retruns now 127.0.0.2 since... Closed
Template:
Story Points: 2
Sprint: Client 2016-01-13, Client 2016-01-27
CS Priority: Reviewed
Release Notes: Bug Fix
Release Notes Summary: Previously, Facter would use the first interface returned by 'ifconfig' as the primary interface for the ipaddress/macaddress/netmask facts.

On Linux. It now determines the primary interface by looking for the default route and using the interface specified there.

 Description   

After adding docker interface all servers have the same :ipaddress fact. It seems that Facter is using the very first alphabetically sorted interface as the primary one.

docker0 Link encap:Ethernet HWaddr 12:de:2d:b1:9d:14
inet addr:172.17.42.1 Bcast:0.0.0.0 Mask:255.255.0.0

eth0 Link encap:Ethernet HWaddr 00:25:90:d1:d1:7a
inet addr: ...

I think facter should ignore private addresses if there is an interface with a public ipaddress.

https://github.com/deric/puppet-mesos/pull/1



 Comments   
Comment by Adam T Backer [ 2014/11/19 ]

I dont think factor should ignore private ip addresses. This would eliminate a lot of use cases.

I think the proper logic for "$ipaddress" should be to assign the ip address of the interface that provides access to the default route.

so, in this case:

root@dns101:~$ ip route show|grep default
default via 10.7.119.254 dev eth0

ipaddress == ipaddress_eth0

Certainly picking the highest alphabetical is very unlikely to produce a value for $ipaddress that would be desired. Given that we have $ipaddress_${interface} for all interfaces, the only thing potentially missing would be a variable that defines the ip address on an interface for each configured route.

i.e. given:

xoom@dns101:~$ ip route show
default via 10.7.119.254 dev eth0
10.7.112.0/21 dev eth0  proto kernel  scope link  src 10.7.113.1
172.17.0.0/16 dev docker0  proto kernel  scope link  src 172.17.42.1

we would set
$ipaddress = $ipaddress_eth0
$ipaddress_172_17_0_0_16 = $ipaddress_docker0
$ipaddress_10_7_112_0_21 = $ipaddress_eth0

This way you can enumerate the external representation of your host (discounting NAT, obviously) to a particular network while being agnostic of the interface on which it is applied.

If this proves to be desired logic, I would happily provide a patch.

Comment by Tomas Barton [ 2014/11/20 ]

I think getting the main $ipaddress fact from default route is a good idea.

Comment by Adam T Backer [ 2014/11/20 ]

This bug represents a big issue when using Foreman, which relies on Puppet to do sane things with $ipaddress. In this case, all hosts running docker will update themselves with the same IP address (172.17.42.1).

If you then attempt to edit any settings underneath the host via the Foreman UI, it will complain that it is unable to save your changes because

  1. The configured IP Address does not match the Subnet the host is configured for (foreman-smart-proxy for DHCP), if you are using foreman to manage IP association.
  2. The IP address is already in use by every other node with Docker setup.

In our case, we use Foreman as a source of truth and an external fact provider for Foreman parameters. This prevents us from editing these values using the UI. I have not yet tested if parameters can be manipulated via the API as an alternative but I believe they may.

Comment by Neil Katin [ 2014/11/20 ]

Speaking as a foreman/puppet/docker user, Adam's proposed "default route selects ipaddress" change would solve our use cases.

Comment by Adam T Backer [ 2014/11/25 ]

Quick hacky little workaround, tested on Ubuntu 14.04 and CentOS 6.3/4/5:

in facter/ipaddress.rb:

 31     default_route_interface = Facter::Util::Resolution.exec('ip route show |grep default|grep -Po "(?<=dev ).*$"')
 32     output = Facter::Util::IP.exec_ifconfig(["#{default_route_interface} 2>/dev/null"])

Comment by Sam Weston [ 2015/01/28 ]

Your code didn't work properly for me. Instead I used:

default_route_interface = Facter::Util::Resolution.exec('ip route show |grep default|cut -d " " -f 5')
output = Facter::Util::IP.exec_ifconfig(["#{default_route_interface} 2>/dev/null"])

It's horribly hacky but it works.

This is really causing us problems since introducing docker a few days ago. Please fix it!

Thanks

Comment by Sam Weston [ 2015/01/28 ]

Same problem with macaddress.rb

Another hacky fix I'm using for the time being:

18    default_route_interface = Facter::Util::Resolution.exec('ip route show |grep default|cut -d " " -f 5')
19   output = Facter::Util::IP.exec_ifconfig(["-a | grep #{default_route_interface} -A 7","2>/dev/null"])

Comment by Neil Katin [ 2015/01/28 ]

I agree this is really annoying; unfortuately it issue falls in the cracks between docker and puppet, and only effects those who use both.

This is how we worked around the issue, without needing to change docker, puppet, or our puppet configurations.

We changed the docker startup scripts to use a bridge named 'zzdocker0' instead of 'docker0'. This is clearly a hack, but it was effective enough to prevent problems.

Here's the systemd file we used to automate that change:

docker.service

[Unit]
Description=Docker Application Container Engine
Documentation=http://docs.docker.com
After=network.target docker.socket
Requires=docker.socket
 
 
[Service]
Type=notify
Environment="BRIDGE=zzdocker0"
EnvironmentFile=-/etc/sysconfig/docker
#   
# set up the bridge
ExecStartPre=/usr/sbin/brctl addbr ${BRIDGE}
ExecStartPre=/usr/sbin/ip addr add 172.16.41.1/16 dev ${BRIDGE}
ExecStartPre=/usr/sbin/ip link set dev $BRIDGE up
#   
# actual start of docker
ExecStart=/usr/bin/docker -d -H fd:// --bridge=${BRIDGE} $OPTIONS
#   
# clean up bridge afterwards
ExecStopPost=/usr/sbin/ip link set dev $BRIDGE down
ExecStopPost=/usr/sbin/brctl delbr $BRIDGE
#   
LimitNOFILE=1048576
LimitNPROC=1048576
 
[Install]
Also=docker.socket

Comment by Sam Weston [ 2015/01/30 ]

This fix is much less hacky than mine and won't break when facter updates, so thanks. However I think this is definitely a problem with puppet behaviour and could be triggered in any situation where the primary network interface isn't at the top of the ifconfig list. If I weren't so hopeless at writing code I'd rewrite the ruby method to use default route and use the 'ip' command instead of 'ifconfig' for newer distributions that support it.

Comment by Jason Hane [ 2015/02/27 ]

This is a huge showstopper but an easy fix. I agree with using the default route's IP address. I think that is the most logical solution. Can we get a commitment for resolution?

Comment by Jason Hane [ 2015/03/01 ]

I worked around this by creating a custom fact to override the ipaddress fact.

require 'facter'
require 'facter/core/execution'
 
if File.exist? '/sbin/ip'
  interfaces = Facter::Core::Execution.exec('/sbin/ip route show')
  default_interface = nil
  ipaddress = nil
  if interfaces
    interfaces.each do |inf|
      if inf.match(/^default/)
        default_interface = inf.split()[-1]
      end
    end
  end
 
  if default_interface
    lines = Facter::Core::Execution.exec("/sbin/ip addr show dev #{default_interface}")
    lines.each do |line|
      line.strip!
      if line.match(/^inet .*#{default_interface}/)
        ipaddress = line.split()[1].split('/')[0]
      end
    end
  end
 
  Facter.add("default_interface") do
    setcode do
      default_interface
    end
  end
  Facter.add("default_ipaddress") do
    setcode do
      ipaddress
    end
  end
end

Comment by Roger Ignazio [ 2015/04/08 ]

I was talking to Branan Riley about this today and we might want to consider improving this logic for Facter 3. As several people have stated above, it seems sane to use the default route to determine the interface used to populate the ipaddress fact.

Comment by Branan Riley [ 2015/04/08 ]

This may actually already be the case in facter 3. I'm poking the networking code now.

Comment by Branan Riley [ 2015/04/08 ]

Yup, this is the case in cfacter at present. So this will be resolved when we ship Facter 3.

Comment by Sam Weston [ 2015/04/09 ]

Thanks for the updates!

Comment by BJ Taylor [ 2015/06/19 ]

I liked the script submitted by Jason Hane, but it didn't work for me. I'm running ubuntu 14.04, facter 2.4.4, and python 2.7.6. So I fixed it so that it would work on my system. Submitting it here in case it is helpful to anyone else.

require 'facter'
require 'facter/core/execution'
 
default_interface = nil
ipaddress = nil
if File.exist? '/sbin/route'
  interfaces = Facter::Core::Execution.exec('/sbin/route')
  if interfaces
    interfaces.split("\n").each do |inf|
      if /^default/.match(inf)
        default_interface = inf.split()[-1]
      end
    end
  end
end
 
if File.exist? '/sbin/ip'
  if default_interface
    lines = Facter::Core::Execution.exec("/sbin/ip addr show dev #{default_interface}")
    lines.split("\n").each do |line|
      line.strip!
      if /^inet .*#{default_interface}/.match(line)
        ipaddress = /(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})\/\d{1,2}/.match(line)[1]
      end
    end
  end
 
  Facter.add("default_interface") do
    setcode do
      default_interface
    end
  end
  Facter.add("default_ipaddress") do
    setcode do
      ipaddress
    end
  end
end

Comment by Byron Miller [ 2015/11/02 ]

Just wanted to throw in my 2 cents, important to get resolution on this - something like what has been recommended to use default route. Doing lots of docker and I too have the problem of foreman incorporating these interfaces and blowing up.

Comment by Eric Sorenson [ 2016/01/06 ]

PE 3.8 will likely be supported through the end of 2016 and there's a very long tail of Ruby facter users, so I don't think that deferring a fix is the right way to go. I agree with Adam T Backer's proposed code implementation (address of the interface through which the default route goes) and would be OK with this going out in a x.Y release of facter that could be bundled into a PE3.8.x maint release. Though semver lawyers may argue that this is a backwards-incompatible change, we have established precedent for correcting erroneous resolution of facts, and I would suggest that the current code is brain-dead enough to be erroneous.

Comment by Jason Ashby [ 2016/01/12 ]

Eric Sorenson is this issue resolved in the Puppet 4 "bundle"?

Another workaround for those interested... Instead of using a custom fact, you can override facter variables with an environment variable, e.g.

# /etc/environment
FACTER_ipaddress=172.20.2.2

just prefix any fact name with FACTER_ and add to your global environment file (/etc/environment for me on CentOS 6). (Side note - if you're testing via puppet agent -t, be sure to source that environment file (or log out and back in) in order to realize the change.)

Comment by Eric Sorenson [ 2016/01/12 ]

Jason Ashby - the puppet-agent all-in-one bundle uses Facter 3 which has a reimplementation of this logic in c++ which may be subject to similar guesswork: https://github.com/puppetlabs/facter/blob/master/lib/src/facts/resolvers/networking_resolver.cc#L60-L67

But it's got built-in support for a top-level networking hash which contains all the info for the probed interfaces on the host, allowing you to enumerate them in a much more reasonable way:

{
  dhcp => "192.168.82.254",
  domain => "vmlocal",
  fqdn => "deglitch.vmlocal",
  hostname => "deglitch",
  interfaces => {
    eth0 => {
      bindings => [
        {
          address => "192.168.82.241",
          netmask => "255.255.255.0",
          network => "192.168.82.0"
        }
      ],
      bindings6 => [
        {
          address => "fe80::20c:29ff:fe3f:eb40",
          netmask => "ffff:ffff:ffff:ffff::",
          network => "fe80::"
        }
      ],
      dhcp => "192.168.82.254",
...

Comment by Branan Riley [ 2016/01/12 ]

Jason AshbyEric Sorenson We have implementations to find the primary interface on Linux, Solaris, Windows, and AIX. Barring a situation where a machine somehow doesn't have a default route (in which case "pick an interface at random" is sort of the best we've got) this should behave correctly in the puppet-agent bundles.

This ticket is at this point just tracking backporting this behavior to Facter 2 for those that can't yet upgrade

Comment by Sean Griffin [ 2016/01/20 ]

Verified on centos-7, with docker installed and docker service running, in isolation of puppet.

I got two vms from the pooler and ran the following commands:
yum update
yum install -y docker docker.io
service docker start
ifconfig (shows docker interface sorting before ens160)
yum install -y net-tools pciutils (required by facter)

On one system, I installed facter 2.4.4 from builds.puppetlabs.net (5/19/2015)
and tested.

[root@y3j0un3nnh7ugh1 ~]# rpm -i http://builds.puppetlabs.lan/facter/2.4.4/artifacts/el/7/products/x86_64/facter-2.4.4-1.el7.x86_64.rpm
[root@y3j0un3nnh7ugh1 ~]# facter --version
2.4.4
[root@y3j0un3nnh7ugh1 ~]# ifconfig
docker0: flags=4099<UP,BROADCAST,MULTICAST>  mtu 1500
        inet 172.17.42.1  netmask 255.255.0.0  broadcast 0.0.0.0
        ether 02:42:4e:98:4b:fe  txqueuelen 0  (Ethernet)
        RX packets 0  bytes 0 (0.0 B)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 0  bytes 0 (0.0 B)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
 
ens160: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet 10.32.118.212  netmask 255.255.240.0  broadcast 10.32.127.255
        inet6 fe80::250:56ff:fe8f:4a62  prefixlen 64  scopeid 0x20<link>
        ether 00:50:56:8f:4a:62  txqueuelen 1000  (Ethernet)
        RX packets 125983  bytes 240629266 (229.4 MiB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 23896  bytes 2180655 (2.0 MiB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
 
lo: flags=73<UP,LOOPBACK,RUNNING>  mtu 65536
        inet 127.0.0.1  netmask 255.0.0.0
        inet6 ::1  prefixlen 128  scopeid 0x10<host>
        loop  txqueuelen 0  (Local Loopback)
        RX packets 0  bytes 0 (0.0 B)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 0  bytes 0 (0.0 B)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
 
[root@y3j0un3nnh7ugh1 ~]# ip route show
default via 10.32.112.1 dev ens160
10.32.112.0/20 dev ens160  proto kernel  scope link  src 10.32.118.212
169.254.0.0/16 dev ens160  scope link  metric 1002
172.17.0.0/16 dev docker0  proto kernel  scope link  src 172.17.42.1
[root@y3j0un3nnh7ugh1 ~]# facter ipaddress
172.17.42.1
[root@y3j0un3nnh7ugh1 ~]# facter macaddress
02:42:4e:98:4b:fe
[root@y3j0un3nnh7ugh1 ~]# #gets the (wrong) addresses from the docker0 interface.
[root@y3j0un3nnh7ugh1 ~]#

On the other system, I installed facter 2.4.5 from builds.puppetlabs.net (1/14/2016) and tested, verifying correct behavior.

[root@whis9djdmcri856 ~]# rpm -i http://builds.puppetlabs.lan/facter/ee7669b5b07c15f996ca7738501873b1057f84dc/artifacts/el/7/products/x86_64/facter-2.4.4.13-1.el7.x86_64.rpm
[root@whis9djdmcri856 ~]# facter --version
2.4.5
[root@whis9djdmcri856 ~]# ifconfig
docker0: flags=4099<UP,BROADCAST,MULTICAST>  mtu 1500
        inet 172.17.42.1  netmask 255.255.0.0  broadcast 0.0.0.0
        ether 02:42:97:e1:94:66  txqueuelen 0  (Ethernet)
        RX packets 0  bytes 0 (0.0 B)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 0  bytes 0 (0.0 B)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
 
ens160: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet 10.32.123.245  netmask 255.255.240.0  broadcast 10.32.127.255
        inet6 fe80::250:56ff:fe8f:a5b8  prefixlen 64  scopeid 0x20<link>
        ether 00:50:56:8f:a5:b8  txqueuelen 1000  (Ethernet)
        RX packets 127273  bytes 240849164 (229.6 MiB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 25432  bytes 2307059 (2.2 MiB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
 
lo: flags=73<UP,LOOPBACK,RUNNING>  mtu 65536
        inet 127.0.0.1  netmask 255.0.0.0
        inet6 ::1  prefixlen 128  scopeid 0x10<host>
        loop  txqueuelen 0  (Local Loopback)
        RX packets 0  bytes 0 (0.0 B)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 0  bytes 0 (0.0 B)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
 
[root@whis9djdmcri856 ~]# ip route show
default via 10.32.112.1 dev ens160
10.32.112.0/20 dev ens160  proto kernel  scope link  src 10.32.123.245
169.254.0.0/16 dev ens160  scope link  metric 1002
172.17.0.0/16 dev docker0  proto kernel  scope link  src 172.17.42.1
[root@whis9djdmcri856 ~]# facter ipaddress
10.32.123.245
[root@whis9djdmcri856 ~]# facter macaddress
00:50:56:8f:a5:b8
[root@y3j0un3nnh7ugh1 ~]# #gets the (correct) addresses from the ens160 interface.
[root@whis9djdmcri856 ~]#

Comment by Poil [ 2016/01/21 ]

This major change is a fucking joke, you've just broken all our production !

Comment by Kylo Ginsberg [ 2016/01/22 ]

Poil can you provide any details on what is broken? FACT-1316 was reported against 2.4.5, so we'll certainly be looking at that, but if there's some other problem, we'd love to fix it stat. Thanks!

Comment by Poil [ 2016/01/22 ]

Hi Kylo Ginsberg,

On our production we have 3 network interfaces.

  • Admin (first iface),
  • Backup (2nd iface)
  • Production (3rd iface) - private IP to the loadbalancer or public IP if no LB (and default gw)

Before the admin interface address was equal to $::ipaddress, except in some very rare case (docker is a case and in this case we were configuring service manually), the listen address for some administration service was determined via $:ipaddress.
Now when we don't have a loadbalancer, the fact $::ipaddress is the value of the production interface.

I really don't understand why you change this fact, it's a fact, it's was the IP address of the first interface, you can't change a fact in each version or it's not more a fact.

You had to create a new fact, ipaddress_public, ipaddress_of_default_route or what you want but changing a fact is totally crazy.

Best regards,

Comment by Roman Mueller [ 2016/01/23 ]

I generally don't disagree with the change of $::ipaddress, I think it makes sense.

But this was pushed out in a minor backwards-compatible bug-fix release.
How was this change classified as a bug-fix?!

Until this change $::ipaddress was documented as:
"On the Unixes, does an ifconfig and returns the first non 127.0.0.0/8 subnetted IP it finds."

This clearly changes the behaviour some have relied on.
For some this could be a breaking change - and it wasn't even mentioned in the announcement e-mail!
I don't think it should have been released in a minor version release.

Comment by Kylo Ginsberg [ 2016/01/25 ]

Roman Mueller, Poil (and watchers): we've decided to revert this fix and release facter 2.4.6 with that change. Thank you for your comments.

Users who want a fix for this issue can move up to facter 3.x. It's TBD whether we'll return and attempt to fix this in facter 2.x. I'll move this ticket to Needs Information to reflect that.

Comment by Branan Riley [ 2016/01/27 ]

Given 1) how terribly trying to do this in 2.x broke people and 2) This is the actual behavior of Facter 3 right now, we're not going to do this in Facter 2.

I'll get some information on some workarounds up here soon so folks who are stuck on Facter 2 can figure out their ipaddress when they have weird interface names

Comment by Stefan Lasiewski [ 2016/06/09 ]

Branan Riley : Are you able to provide information on some workarounds, as you said in January?

We just got bit by this, and it's causing some bad behavior on some of our boxes.

Generated at Sat Dec 14 19:23:18 PST 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.