fix: don't return localhost as FQDN in macos (#693)

Author: rubenruizdegauna

pkg/sysinfo/hostname/hostname.go

@@ -5,9 +5,7 @@ package hostname
 import (
 	"errors"
 	"fmt"
-	"net"
 	"os"
-	"strings"
 	"sync"
 
 	"github.com/newrelic/infrastructure-agent/pkg/log"
@@ -107,24 +105,6 @@ func newInternalResolver(overrideFull string) *fallbackResolver {
 	}
 }
 
-// Looks up for the Fully Qualified Domain Name
-func getFqdnHostname(osHost string) (string, error) {
-	ips, err := net.LookupIP(osHost)
-	if err != nil {
-		return "", err
-	}
-
-	for _, ip := range ips {
-		hosts, err := net.LookupAddr(ip.String())
-		if err != nil || len(hosts) == 0 {
-			return "", err
-		}
-		trace.Hostname("found FQDN hosts: %s", strings.Join(hosts, ", "))
-		return strings.TrimSuffix(hosts[0], "."), nil
-	}
-	return "", errors.New("can't lookup FQDN")
-}
-
 // Implementation of the HostnameResolver interface that provides fallback capabilities
 // in case the full name resolving fails, returning the last successful value.
 // If the Full name resolution is "localhost" (known problem in some wrong FQDN configurations)

pkg/sysinfo/hostname/hostname_darwin.go

@@ -1,8 +1,16 @@
 // Copyright 2020 New Relic Corporation. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
+// +build darwin
+
 package hostname
 
-import "github.com/newrelic/infrastructure-agent/pkg/helpers"
+import (
+	"errors"
+	"github.com/newrelic/infrastructure-agent/pkg/helpers"
+	"github.com/newrelic/infrastructure-agent/pkg/trace"
+	"net"
+	"strings"
+)
 
 // attempts to determine the hostname, gracefully falling back until we
 // run out of options
@@ -22,3 +30,30 @@ func internalHostname() (hn string, err error) {
 	// return whatever we did get including the error
 	return
 }
+
+type ipLookupper func(host string) ([]net.IP, error)
+type addrLookupper func(addr string) (names []string, err error)
+
+var lookupIp ipLookupper = net.LookupIP
+var lookupAddr addrLookupper = net.LookupAddr
+
+// Looks up for the Fully Qualified Domain Name. Do not take into account localhost as FQDN
+func getFqdnHostname(osHost string) (string, error) {
+	ips, err := lookupIp(osHost)
+	if err != nil {
+		return "", err
+	}
+
+	for _, ip := range ips {
+		hosts, err := lookupAddr(ip.String())
+		if err != nil || len(hosts) == 0 {
+			return "", err
+		}
+		if hosts[0] == "localhost" {
+			continue
+		}
+		trace.Hostname("found FQDN hosts: %s", strings.Join(hosts, ", "))
+		return strings.TrimSuffix(hosts[0], "."), nil
+	}
+	return "", errors.New("can't lookup FQDN")
+}

pkg/sysinfo/hostname/hostname_darwin_test.go

@@ -0,0 +1,72 @@
+// Copyright 2020 New Relic Corporation. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+// +build darwin
+
+package hostname
+
+import (
+	"errors"
+	"github.com/stretchr/testify/assert"
+	"net"
+	"testing"
+)
+
+func ipLookUpperMock(ips []net.IP, err error) func(host string) ([]net.IP, error) {
+	return func(_ string) ([]net.IP, error) {
+		return ips, err
+	}
+}
+
+func addrLookUpperMock(names map[string][]string, err error) func(addr string) (names []string, err error) {
+	return func(lookUpIp string) ([]string, error) {
+		return names[lookUpIp], err
+	}
+}
+
+func TestGetFqdnHostname_DontReturnLocalhost(t *testing.T) {
+
+	tests := []struct {
+		name             string
+		ips              []net.IP
+		fqdns            map[string][]string
+		ipLookupError    error
+		addrLookupError  error
+		expectedHostname string
+		expectedErr      error
+	}{
+		{
+			name:             "no ips should return error",
+			expectedErr:      errors.New("can't lookup FQDN"),
+			expectedHostname: "",
+		},
+		{
+			name:             "only localhost should return error",
+			ips:              []net.IP{net.ParseIP("127.0.0.1")},
+			fqdns:            map[string][]string{"127.0.0.1": {"localhost"}},
+			expectedErr:      errors.New("can't lookup FQDN"),
+			expectedHostname: "",
+		},
+		{
+			name: "first valid fqdn should be returned",
+			ips:  []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("192.168.1.1"), net.ParseIP("10.0.0.1")},
+			fqdns: map[string][]string{
+				"127.0.0.1":   {"localhost"},
+				"192.168.1.1": {"test.local"},
+				"10.0.0.1":    {"another.test.local"},
+			},
+			expectedHostname: "test.local",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			lookupIp = ipLookUpperMock(tt.ips, tt.ipLookupError)
+			lookupAddr = addrLookUpperMock(tt.fqdns, tt.addrLookupError)
+
+			hostname, err := getFqdnHostname("some")
+
+			assert.Equal(t, tt.expectedErr, err)
+			assert.Equal(t, tt.expectedHostname, hostname)
+		})
+	}
+}

pkg/sysinfo/hostname/hostname_lin_win.go

@@ -0,0 +1,32 @@
+// Copyright 2020 New Relic Corporation. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+// +build linux windows
+
+package hostname
+
+import (
+	"errors"
+	"github.com/newrelic/infrastructure-agent/pkg/trace"
+	"net"
+	"strings"
+)
+
+// Looks up for the Fully Qualified Domain Name.
+// `localhost` should not be returned as FQDN, but until deciding how it affects we will maintain this version
+// for linux and windows for backwards compatibility
+func getFqdnHostname(osHost string) (string, error) {
+	ips, err := net.LookupIP(osHost)
+	if err != nil {
+		return "", err
+	}
+
+	for _, ip := range ips {
+		hosts, err := net.LookupAddr(ip.String())
+		if err != nil || len(hosts) == 0 {
+			return "", err
+		}
+		trace.Hostname("found FQDN hosts: %s", strings.Join(hosts, ", "))
+		return strings.TrimSuffix(hosts[0], "."), nil
+	}
+	return "", errors.New("can't lookup FQDN")
+}