diff --git a/render/detailed/connections.go b/render/detailed/connections.go index ce8deaa65..fd7528461 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -57,32 +57,9 @@ func (s connectionsByID) Less(i, j int) bool { return s[i].ID < s[j].ID } // Intermediate type used as a key to dedupe rows type connection struct { - remoteNodeID, localNodeID string - remoteAddr, localAddr string - port string // always the server-side port -} - -func newConnection(n report.Node, node report.Node, port string, endpointID string, localAddr string) connection { - c := connection{ - localNodeID: n.ID, - remoteNodeID: node.ID, - port: port, - } - // For internet nodes we break out individual addresses, both when - // the internet node is remote (an incoming connection from the - // internet) and 'local' (ie you are loading details on the - // internet node) - if isInternetNode(n) { - // We use the *endpoint* ID here since that has the reverse - // DNS information associated with it. - c.localNodeID = endpointID - c.localAddr = localAddr - } - return c -} - -func (row connection) ID() string { - return fmt.Sprintf("%s:%s-%s:%s-%s", row.remoteNodeID, row.remoteAddr, row.localNodeID, row.localAddr, row.port) + remoteNodeID string + addr string // for internet nodes only: the address + port string // destination port } type connectionCounters struct { @@ -94,19 +71,38 @@ func newConnectionCounters() *connectionCounters { return &connectionCounters{counted: map[string]struct{}{}, counts: map[connection]int{}} } -func (c *connectionCounters) add(sourceEndpoint report.Node, n report.Node, node report.Node, port string, endpointID string, localAddr string) { +func (c *connectionCounters) add(outgoing bool, localNode, remoteNode, localEndpoint, remoteEndpoint report.Node) { // We identify connections by their source endpoint, pre-NAT, to // ensure we only count them once. - connectionID := sourceEndpoint.ID - if copySourceEndpointID, _, ok := sourceEndpoint.Latest.LookupEntry("copy_of"); ok { - connectionID = copySourceEndpointID + srcEndpoint, dstEndpoint := remoteEndpoint, localEndpoint + if outgoing { + srcEndpoint, dstEndpoint = localEndpoint, remoteEndpoint + } + connectionID := srcEndpoint.ID + if copySrcEndpointID, _, ok := srcEndpoint.Latest.LookupEntry("copy_of"); ok { + connectionID = copySrcEndpointID } if _, ok := c.counted[connectionID]; ok { return } + + conn := connection{remoteNodeID: remoteNode.ID} + var ok bool + if _, _, conn.port, ok = report.ParseEndpointNodeID(dstEndpoint.ID); !ok { + return + } + // For internet nodes we break out individual addresses + if isInternetNode(localNode) { + if _, conn.addr, _, ok = report.ParseEndpointNodeID(localEndpoint.ID); !ok { + return + } + if set, ok := localEndpoint.Sets.Lookup(endpoint.ReverseDNSNames); ok && len(set) > 0 { + conn.addr = fmt.Sprintf("%s (%s)", set[0], conn.addr) + } + } + c.counted[connectionID] = struct{}{} - key := newConnection(n, node, port, endpointID, localAddr) - c.counts[key]++ + c.counts[conn]++ } func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool) []Connection { @@ -115,28 +111,18 @@ func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal // Use MakeNodeSummary to render the id and label of this node // TODO(paulbellamy): Would be cleaner if we hade just a // MakeNodeID(ns[row.remoteNodeID]). As we don't need the whole summary. - summary, ok := MakeNodeSummary(r, ns[row.remoteNodeID]) + summary, _ := MakeNodeSummary(r, ns[row.remoteNodeID]) connection := Connection{ - ID: row.ID(), + ID: fmt.Sprintf("%s-%s-%s", row.remoteNodeID, row.addr, row.port), NodeID: summary.ID, Label: summary.Label, Linkable: true, } - if !ok && row.remoteAddr != "" { - connection.Label = row.remoteAddr - connection.Linkable = false - } if includeLocal { - // Does localNode (which, in this case, is an endpoint) - // have a DNS record in it? - label := row.localAddr - if set, ok := r.Endpoint.Nodes[row.localNodeID].Sets.Lookup(endpoint.ReverseDNSNames); ok && len(set) > 0 { - label = fmt.Sprintf("%s (%s)", set[0], label) - } connection.Metadata = append(connection.Metadata, report.MetadataRow{ ID: "foo", - Value: label, + Value: row.addr, Datatype: number, }) } @@ -167,16 +153,10 @@ func incomingConnectionsSummary(topologyID string, r report.Report, n report.Nod if !node.Adjacency.Contains(n.ID) { continue } - // Work out what port they are talking to, and count the number of - // connections to that port. for _, remoteEndpoint := range endpointChildrenOf(node) { for _, localEndpointID := range remoteEndpoint.Adjacency.Intersection(localEndpointIDs) { localEndpointID = canonicalEndpointID(localEndpointIDCopies, localEndpointID) - _, localAddr, port, ok := report.ParseEndpointNodeID(localEndpointID) - if !ok { - continue - } - counts.add(remoteEndpoint, n, node, port, localEndpointID, localAddr) + counts.add(false, n, node, r.Endpoint.Nodes[localEndpointID], remoteEndpoint) } } } @@ -204,21 +184,11 @@ func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Nod if !ok { continue } - remoteEndpointIDs, remoteEndpointIDCopies := endpointChildIDsAndCopyMapOf(node) - for _, localEndpoint := range localEndpoints { - _, localAddr, _, ok := report.ParseEndpointNodeID(localEndpoint.ID) - if !ok { - continue - } for _, remoteEndpointID := range localEndpoint.Adjacency.Intersection(remoteEndpointIDs) { remoteEndpointID = canonicalEndpointID(remoteEndpointIDCopies, remoteEndpointID) - _, _, port, ok := report.ParseEndpointNodeID(remoteEndpointID) - if !ok { - continue - } - counts.add(localEndpoint, n, node, port, localEndpoint.ID, localAddr) + counts.add(true, n, node, localEndpoint, r.Endpoint.Nodes[remoteEndpointID]) } } } diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index 9c2d34b61..aff45bc47 100644 --- a/render/detailed/node_test.go +++ b/render/detailed/node_test.go @@ -25,6 +25,10 @@ func child(t *testing.T, r render.Renderer, id string) detailed.NodeSummary { return s.SummarizeMetrics() } +func connectionID(nodeID string) string { + return fmt.Sprintf("%s-%s-%d", nodeID, "", 80) +} + func TestMakeDetailedHostNode(t *testing.T) { renderableNodes := render.HostRenderer.Render(fixture.Report, nil) renderableNode := renderableNodes[fixture.ClientHostNodeID] @@ -149,7 +153,7 @@ func TestMakeDetailedHostNode(t *testing.T) { Columns: detailed.NormalColumns, Connections: []detailed.Connection{ { - ID: fmt.Sprintf("%s:%s-%s:%s-%d", fixture.ServerHostNodeID, "", fixture.ClientHostNodeID, "", 80), + ID: connectionID(fixture.ServerHostNodeID), NodeID: fixture.ServerHostNodeID, Label: "server", Linkable: true, @@ -257,7 +261,7 @@ func TestMakeDetailedContainerNode(t *testing.T) { Columns: detailed.NormalColumns, Connections: []detailed.Connection{ { - ID: fmt.Sprintf("%s:%s-%s:%s-%d", fixture.ClientContainerNodeID, "", fixture.ServerContainerNodeID, "", 80), + ID: connectionID(fixture.ClientContainerNodeID), NodeID: fixture.ClientContainerNodeID, Label: "client", Linkable: true, @@ -275,7 +279,7 @@ func TestMakeDetailedContainerNode(t *testing.T) { }, }, { - ID: fmt.Sprintf("%s:%s-%s:%s-%d", render.IncomingInternetID, "", fixture.ServerContainerNodeID, "", 80), + ID: connectionID(render.IncomingInternetID), NodeID: render.IncomingInternetID, Label: render.InboundMajor, Linkable: true, @@ -377,7 +381,7 @@ func TestMakeDetailedPodNode(t *testing.T) { Columns: detailed.NormalColumns, Connections: []detailed.Connection{ { - ID: fmt.Sprintf("%s:%s-%s:%s-%d", fixture.ClientPodNodeID, "", fixture.ServerPodNodeID, "", 80), + ID: connectionID(fixture.ClientPodNodeID), NodeID: fixture.ClientPodNodeID, Label: "pong-a", Linkable: true, @@ -395,7 +399,7 @@ func TestMakeDetailedPodNode(t *testing.T) { }, }, { - ID: fmt.Sprintf("%s:%s-%s:%s-%d", render.IncomingInternetID, "", fixture.ServerPodNodeID, "", 80), + ID: connectionID(render.IncomingInternetID), NodeID: render.IncomingInternetID, Label: render.InboundMajor, Linkable: true,