Update preferred endpoint preference order (#1583)

Rather than pick addresses totally at random, this changes the
preference to be private IP4 > public IP4 > DNS. (Ordering is not
guaranteed, but in many cases there will only be one of each type of IP
anyways.) All three of these are preferred over IP6, which also goes
public > private.

This seems likely to match the common case where you're getting into a
machine on the private side from the public side. It's harder to know if
DNS is private or not, so IPs are preferred in this case.

internal/libs/endpoint/preferencer.go

@@ -2,8 +2,7 @@ package endpoint
 
 import (
 	"context"
-	"math/rand"
-	"time"
+	"net"
 
 	"github.com/hashicorp/boundary/internal/errors"
 )
@@ -47,10 +46,52 @@ func (p *preferencer) Choose(ctx context.Context, opt ...Option) (string, error)
 
 	switch len(p.matchers) {
 	case 0:
-		// We have no matchers, so pick one at random
-		allAddrs := append(opts.withIpAddrs, opts.withDnsNames...)
-		rng := rand.New(rand.NewSource(time.Now().UnixNano()))
-		return allAddrs[rng.Intn(len(allAddrs))], nil
+		// We have no matchers, so pick a private address if we have one (since
+		// those are most likely to be what the user is trying to gain access to
+		// via a worker). If we don't have one, pick any IP address, as that's
+		// also more likely desired than a DNS name. Otherwise, pick a DNS name.
+		// IPv6 follows same rules as 4, but least preferenced.
+		nonPrivateIp4s := make([]string, 0, len(opts.withIpAddrs))
+		nonPrivateIp6s := make([]string, 0, len(opts.withIpAddrs))
+		privateIp6s := make([]string, 0, len(opts.withIpAddrs))
+
+		for _, ipStr := range opts.withIpAddrs {
+			ipVal := net.ParseIP(ipStr)
+			if ipVal != nil {
+				switch ipVal.To4() {
+				case nil: // it's v6
+					if ipVal.IsPrivate() {
+						privateIp6s = append(privateIp6s, ipStr)
+					} else {
+						nonPrivateIp6s = append(nonPrivateIp6s, ipStr)
+					}
+				default:
+					if ipVal.IsPrivate() {
+						// private IPv4 is most highly preferenced, so return it directly
+						return ipStr, nil
+					}
+					nonPrivateIp4s = append(nonPrivateIp4s, ipStr)
+				}
+			}
+		}
+
+		switch {
+		case len(nonPrivateIp4s) > 0:
+			return nonPrivateIp4s[0], nil
+
+		case len(opts.withDnsNames) > 0:
+			return opts.withDnsNames[0], nil
+
+		case len(privateIp6s) > 0:
+			return privateIp6s[0], nil
+
+		case len(nonPrivateIp6s) > 0:
+			return nonPrivateIp6s[0], nil
+
+		default:
+			// We literally have nothing if we get here, so return nothing
+			return "", nil
+		}
 
 	default:
 		for _, m := range p.matchers {

internal/libs/endpoint/preferencer_test.go

@@ -91,20 +91,62 @@ func TestPreferencer(t *testing.T) {
 			})
 		}
 	})
-	t.Run("noPrefRandom", func(t *testing.T) {
+	t.Run("noPrefReturnsPrivate", func(t *testing.T) {
+		const privAddr = "192.168.4.3"
 		p, err := NewPreferencer(ctx)
 		require.NoError(t, err)
-		checkMap := map[string]int{}
-		for i := 0; i < 200; i++ {
-			out, err := p.Choose(
-				ctx,
-				WithIpAddrs([]string{"48.134.5.1", "1.2.7.56"}),
-				WithDnsNames([]string{"foo.bar.com", "bar.baz.com"}),
-			)
-			require.NoError(t, err)
-			checkMap[out] = checkMap[out] + 1
-		}
-		// Ensure that we've inserted all four keys
-		assert.Len(t, checkMap, 4)
+		out, err := p.Choose(
+			ctx,
+			WithIpAddrs([]string{"48.134.5.1", "2001::1", privAddr}),
+			WithDnsNames([]string{"foo.bar.com", "bar.baz.com"}),
+		)
+		require.NoError(t, err)
+		assert.Equal(t, privAddr, out)
+	})
+	t.Run("noPrefNoPrivateReturnsIp4", func(t *testing.T) {
+		const exp = "48.134.5.1"
+		p, err := NewPreferencer(ctx)
+		require.NoError(t, err)
+		out, err := p.Choose(
+			ctx,
+			WithIpAddrs([]string{"2001::1", exp}),
+			WithDnsNames([]string{"foo.bar.com", "bar.baz.com"}),
+		)
+		require.NoError(t, err)
+		assert.Equal(t, exp, out)
+	})
+	t.Run("noPrefNoIp4ReturnsDns", func(t *testing.T) {
+		const exp = "foo.bar.com"
+		p, err := NewPreferencer(ctx)
+		require.NoError(t, err)
+		out, err := p.Choose(
+			ctx,
+			WithIpAddrs([]string{"2001::1"}),
+			WithDnsNames([]string{exp}),
+		)
+		require.NoError(t, err)
+		assert.Equal(t, exp, out)
+	})
+	t.Run("ip6OnlyReturnsPrivate", func(t *testing.T) {
+		const exp = "fc00::1"
+		p, err := NewPreferencer(ctx)
+		require.NoError(t, err)
+		out, err := p.Choose(
+			ctx,
+			WithIpAddrs([]string{"2001::1", exp}),
+		)
+		require.NoError(t, err)
+		assert.Equal(t, exp, out)
+	})
+	t.Run("ip6OnlyPublic", func(t *testing.T) {
+		const exp = "2001::1"
+		p, err := NewPreferencer(ctx)
+		require.NoError(t, err)
+		out, err := p.Choose(
+			ctx,
+			WithIpAddrs([]string{exp}),
+		)
+		require.NoError(t, err)
+		assert.Equal(t, exp, out)
 	})
 }