Adds public IP of LB to localIPs slice (#55)

* Adds public IP of LB to localIPs slice

Virtual machines will be part of managed instance groups, which sit
behind a pass-through TCP load balancer. In such cases, packets arriving
to the machine from the load balancer will have a destination address of
the load balancer's public IP, which is not a local address on the
machine. Without these changes uuid-annotator will refuse to annoate a
connection, since it doesn't recognize either the Src of Dest IP address
as being its own (annotate.FindDirection() fails). These changes cause
siteannotator.New() to append the public IP address from siteinfo to
localIPs if it is a virtual machine.

* Fixes a typo in a comment

Author: nkinkade

main.go

@@ -102,6 +102,17 @@ func main() {
 	localAddrs, err := net.InterfaceAddrs()
 	rtx.Must(err, "Could not read local addresses")
 	localIPs := findLocalIPs(localAddrs)
+
+	// Load the siteinfo annotations for "site" specific metadata. Additionally,
+	// if this is a virtual site, New() will append the public IP of the
+	// managed instance group's load balancer to localIPs. If uuid-annotator
+	// does not know about the public IP of the load balancer, then it will fail
+	// to annotate anything because it doesn't recognize its own public address
+	// in either the Src or Dest of incoming tcp-info events.
+	js, err := content.FromURL(mainCtx, siteinfo.URL)
+	rtx.Must(err, "Could not load siteinfo URL")
+	site, localIPs := siteannotator.New(mainCtx, mlabHostname, js, localIPs)
+
 	p, err := content.FromURL(mainCtx, maxmindurl.URL)
 	rtx.Must(err, "Could not get maxmind data from url")
 	geo := geoannotator.New(mainCtx, p, localIPs)
@@ -132,10 +143,6 @@ func main() {
 	}()
 
 	if *eventsocket.Filename != "" {
-		// Load the siteinfo annotations for "site" specific metadata.
-		js, err := content.FromURL(mainCtx, siteinfo.URL)
-		rtx.Must(err, "Could not load siteinfo URL")
-		site := siteannotator.New(mainCtx, mlabHostname, js, localIPs)
 
 		// Generate .json files for every UUID discovered.
 		h := handler.New(*datadir, *eventbuffersize, []annotator.Annotator{geo, asn, site})

siteannotator/server.go

@@ -30,16 +30,16 @@ type siteAnnotator struct {
 var ErrHostnameNotFound = errors.New("hostname not found")
 
 // New makes a new server Annotator using metadata from siteinfo JSON.
-func New(ctx context.Context, hostname string, js content.Provider, localIPs []net.IP) annotator.Annotator {
+func New(ctx context.Context, hostname string, js content.Provider, localIPs []net.IP) (annotator.Annotator, []net.IP) {
 	g := &siteAnnotator{
 		siteinfoSource: js,
 		hostname:       hostname,
-		localIPs:       localIPs,
 	}
 	var err error
-	g.server, err = g.load(ctx)
+	g.server, localIPs, err = g.load(ctx, localIPs)
+	g.localIPs = localIPs
 	rtx.Must(err, "Could not load annotation db")
-	return g
+	return g, localIPs
 }
 
 // Annotate assigns the server geolocation and ASN metadata.
@@ -109,22 +109,34 @@ func parseCIDR(v4, v6 string) (net.IPNet, net.IPNet, error) {
 }
 
 // load unconditionally loads siteinfo dataset and returns them.
-func (g *siteAnnotator) load(ctx context.Context) (*annotator.ServerAnnotations, error) {
+func (g *siteAnnotator) load(ctx context.Context, localIPs []net.IP) (*annotator.ServerAnnotations, []net.IP, error) {
 	js, err := g.siteinfoSource.Get(ctx)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	var s map[string]siteinfoAnnotation
 	err = json.Unmarshal(js, &s)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	if v, ok := s[g.hostname]; ok {
 		g.v4, g.v6, err = parseCIDR(v.Network.IPv4, v.Network.IPv6)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
-		return &v.Annotation, nil
+		// If this is a virtual site, append the site's public IP address to
+		// localIPs. The public address of the load balancer is not known on any
+		// interface on the machine. Without adding it to localIPs,
+		// uuid-annotator will fail to recognize its own public address in
+		// either the Src or Dest fields of incoming tcp-info events, and will
+		// fail to annotate anything.
+		if v.Type == "virtual" {
+			// Ignore IPNet and error, since parseCIDR() above has already
+			// validated the IP addresses.
+			ip, _, _ := net.ParseCIDR(v.Network.IPv4)
+			localIPs = append(localIPs, ip)
+		}
+		return &v.Annotation, localIPs, nil
 	}
-	return nil, ErrHostnameNotFound
+	return nil, nil, ErrHostnameNotFound
 }

siteannotator/server_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net"
 	"net/url"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -188,7 +189,7 @@ func TestNew(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			setUp()
 			ctx := context.Background()
-			g := New(ctx, tt.hostname, *tt.provider, tt.localIPs)
+			g, _ := New(ctx, tt.hostname, *tt.provider, tt.localIPs)
 			ann := annotator.Annotations{}
 			if err := g.Annotate(tt.ID, &ann); (err != nil) != tt.wantErr {
 				t.Errorf("srvannotator.Annotate() error = %v, wantErr %v", err, tt.wantErr)
@@ -201,13 +202,14 @@ func TestNew(t *testing.T) {
 }
 func Test_srvannotator_load(t *testing.T) {
 	var bad content.Provider
+	var testLocalIPs []net.IP = []net.IP{net.ParseIP("10.0.0.1")}
 	tests := []struct {
-		name     string
-		provider *content.Provider
-		hostname string
-		ID       *inetdiag.SockID
-		want     *annotator.ServerAnnotations
-		wantErr  bool
+		name         string
+		provider     *content.Provider
+		hostname     string
+		want         *annotator.ServerAnnotations
+		wantLocalIPs []net.IP
+		wantErr      bool
 	}{
 		{
 			name:     "success",
@@ -231,6 +233,7 @@ func Test_srvannotator_load(t *testing.T) {
 					},
 				},
 			},
+			wantLocalIPs: testLocalIPs,
 		},
 		{
 			name:     "success-project-flat-name",
@@ -254,6 +257,7 @@ func Test_srvannotator_load(t *testing.T) {
 					},
 				},
 			},
+			wantLocalIPs: testLocalIPs,
 		},
 		{
 			name:     "success-no-six",
@@ -269,6 +273,23 @@ func Test_srvannotator_load(t *testing.T) {
 					ASName: "TATA COMMUNICATIONS (AMERICA) INC",
 				},
 			},
+			wantLocalIPs: testLocalIPs,
+		},
+		{
+			name:     "success-append-localips",
+			provider: &localRawfile,
+			hostname: "mlab1-six06.mlab-sandbox.measurement-lab.org",
+			want: &annotator.ServerAnnotations{
+				Site:    "six06",
+				Machine: "mlab1",
+				Geo: &annotator.Geolocation{
+					City: "New York",
+				},
+				Network: &annotator.Network{
+					ASName: "TATA COMMUNICATIONS (AMERICA) INC",
+				},
+			},
+			wantLocalIPs: append(testLocalIPs, net.ParseIP("64.86.148.129")),
 		},
 		{
 			name:     "error-bad-ipv4",
@@ -322,7 +343,10 @@ func Test_srvannotator_load(t *testing.T) {
 				hostname:       tt.hostname,
 			}
 			ctx := context.Background()
-			an, err := g.load(ctx)
+			an, localIPs, err := g.load(ctx, testLocalIPs)
+			if !reflect.DeepEqual(localIPs, tt.wantLocalIPs) {
+				t.Errorf("srvannotator.load() want localIPs %v, got %v", tt.wantLocalIPs, localIPs)
+			}
 			if (err != nil) != tt.wantErr {
 				t.Errorf("srvannotator.Annotate() error = %v, wantErr %v", err, tt.wantErr)
 			}

testdata/annotations.json

@@ -46,6 +46,23 @@
       },
       "Type": "virtual"
    },
+   "mlab1-six06.mlab-sandbox.measurement-lab.org": {
+      "Annotation": {
+         "Geo": {
+            "City": "New York"
+         },
+         "Machine": "mlab1",
+         "Network": {
+            "ASName": "TATA COMMUNICATIONS (AMERICA) INC"
+         },
+         "Site": "six06"
+      },
+      "Network": {
+         "IPv4": "64.86.148.129/32",
+         "IPv6": "2001:5a0:4300::/64"
+      },
+      "Type": "virtual"
+   },
    "mlab1-six01.mlab-sandbox.measurement-lab.org": {
       "Annotation": {
          "Geo": {