puppetlabs / facter

Collect and display system facts
https://puppet.com/open-source/#osp
Apache License 2.0
616 stars 494 forks source link

networking: Workaround 2693 by ignoring resolv.conf entries starting with dot #2694

Open Geod24 opened 3 months ago

Geod24 commented 3 months ago

Currently, systems without a FQDN can be mishandled by facter for certain /etc/resolv.conf content. This was initially noticed when systemd-resolved was installed on a host without domain.

systemd-resolved stub resolver sets 'search .' as a search domain, which results in the following hostname/domain/fqdn triplet: foo, ., foo... See: https://github.com/systemd/systemd/blob/v255/src/resolve/resolv.conf#L19

This is wrong on multiple levels: first, facter does not seem to handle '.' (the root level) well, and there have been PRs to remove the trailing dot in FQDN as far back as 2012 (PR 200), leaving user with a convenient, but sometimes ambiguous string that is not fully qualified.

More fundamentally, facter sould not be using 'search' or 'domain' in resolv.conf to begin with, as those are client resolution options, and usually set for convenience, but are not part of the hostname. However, removing this lookup is much more involved, as facter has been doing this for years.

Hence, this commit implements a a middle ground solution to support top-level/domainless servers without making a breaking change.

puppetlabs-jenkins commented 3 months ago

Can one of the admins verify this patch?

Geod24 commented 2 months ago

@cthorn42 any chance you could take a look at this ?

ekohl commented 2 months ago

More fundamentally, facter sould not be using 'search' or 'domain' in resolv.conf to begin with, as those are client resolution options, and usually set for convenience, but are not part of the hostname. However, removing this lookup is much more involved, as facter has been doing this for years.

I don't think this is true.

Quoting man 1 hostname:

The FQDN (Fully Qualified Domain Name) of the system is the name that the resolver(3) returns for the host name, such as, ursula.example.com. It is usually the hostname followed by the DNS domain name (the part after the first dot). You can check the FQDN using hostname --fqdn or the domain name using dnsdomainname.

Quoting man 3 resolver:

The res_ninit() and res_init() functions read the configuration files (see resolv.conf(5)) to get the default domain name and name server address(es). If no server is given, the local host is tried. If no domain is given, that associated with the local host is used. It can be overridden with the environment variable LOCALDOMAIN. res_ninit() or res_init() is normally executed by the first call to one of the other functions. Every call to res_ninit() requires a corresponding call to res_nclose() to free memory allocated by res_ninit() and subsequent calls to res_nquery().

So on Linux libc can certainly use it.

However, removing this lookup is much more involved, as facter has been doing this for years.

You could replace it with a native implementation. Quoting man 1 hostname again:

Technically: The FQDN is the name getaddrinfo(3) returns for the host name returned by gethostname(2). The DNS domain name is the part after the first dot.

I think that means you can use:

Socket.getaddrinfo(Socket.gethostname, nil, nil, nil, nil, Socket::AI_CANONNAME)
h0tw1r3 commented 2 months ago

I think most sys admins would expect puppet's fqdn / hostname to return exactly what the hostname command does.

This is a quick port of the standard linux hostname C code with the addition of falling back to parsing /etc/resolv.conf.

#!/opt/puppetlabs/puppet/bin/ruby

require 'socket'

def fqdn
    # Addrinfo.getaddrinfo
    result = Addrinfo.getaddrinfo(Socket.gethostname, nil, nil, Socket::SOCK_DGRAM, nil, Socket::AI_CANONNAME)
        .map { |a| a.canonname }
        .find { |n| !n.nil? and n.include? '.' }

    return result if !result.nil?

    # Socket.getnameinfo
    result = Socket.getifaddrs().filter_map do |ifap|
        next unless ifap.addr
        next unless (ifap.flags & Socket::IFF_LOOPBACK) == 0
        next if (ifap.flags & Socket::IFF_UP) == 0

        family = ifap.addr.afamily
        next unless (family == Socket::AF_INET || family == Socket::AF_INET6)

        next if family == Socket::AF_INET6 && ifap.addr.ipv6_linklocal?

        n = Socket.getnameinfo(ifap.addr, Socket::NI_NAMEREQD)
        n[0] if n and !n[0].nil? and n[0].include? '.'
    end.first

    return result if !result.nil?

    # Fallback to parsing /etc/resolv.conf
    content = File.read('/etc/resolv.conf', chomp: true)
    domain = if content =~ /^domain\s+(\S+)/
             Regexp.last_match[1]
         elsif content =~ /^search\s+(\S+)/
             Regexp.last_match[1]
         end
    # strip leading and trailing .
    domain.sub!(/^\.?(.*?)\.?$/, '\1') if domain

    result = [Socket.gethostname]
    result.append(domain) if domain and domain.length > 1

    result.join('.')
end
b4ldr commented 1 month ago

More fundamentally, facter sould not be using 'search' or 'domain' in resolv.conf to begin with, as those are client resolution options, and usually set for convenience, but are not part of the hostname. However, removing this lookup is much more involved, as facter has been doing this for years.

I don't think this is true.

I think this is definitely true for the search path but i agree using the domain part seems reasonable if there is no better info