Un-export NATMapper

Lots of TODOs to clean up...
2026-07-28 01:31:17 +00:00 · 2015-10-02 17:53:44 +02:00
parent 36ce1089f4
commit 6ae5077515
3 changed files with 102 additions and 44 deletions
--- a/probe/endpoint/nat.go
+++ b/probe/endpoint/nat.go
@@ -16,14 +16,13 @@ type endpointMapping struct {
 	rewrittenPort int
 }

-// NATMapper rewrites a report to deal with NAT's connections
-type NATMapper struct {
+// natMapper rewrites a report to deal with NAT'd connections.
+type natMapper struct {
 	Conntracker
 }

-// MakeNATMapper is exposed for testing
-func MakeNATMapper(ct Conntracker) NATMapper {
-	return NATMapper{ct}
+func makeNATMapper(ct Conntracker) natMapper {
+	return natMapper{ct}
 }

 func toMapping(f Flow) *endpointMapping {
@@ -47,10 +46,10 @@ func toMapping(f Flow) *endpointMapping {
 	return &mapping
 }

-// ApplyNAT duplicates Nodes in the endpoint topology of a
-// report, based on the NAT table as returns by natTable.
-func (n NATMapper) ApplyNAT(rpt report.Report, scope string) {
-	if n.Conntracker == nil {
+// applyNAT duplicates Nodes in the endpoint topology of a report, based on
+// the NAT table.
+func (n natMapper) applyNAT(rpt report.Report, scope string) {
+	if n.Conntracker == nil { // TODO(pb)
 		return
 	}
 	n.Conntracker.WalkFlows(func(f Flow) {
--- a/probe/endpoint/nat_internal_test.go
+++ b/probe/endpoint/nat_internal_test.go
@@ -1,19 +1,19 @@
-package endpoint_test
+package endpoint

 import (
+	"encoding/xml"
 	"reflect"
 	"testing"

-	"github.com/weaveworks/scope/probe/endpoint"
 	"github.com/weaveworks/scope/report"
 	"github.com/weaveworks/scope/test"
 )

 type mockConntracker struct {
-	flows []endpoint.Flow
+	flows []Flow
 }

-func (m *mockConntracker) WalkFlows(f func(endpoint.Flow)) {
+func (m *mockConntracker) WalkFlows(f func(Flow)) {
 	for _, flow := range m.flows {
 		f(flow)
 	}
@@ -21,6 +21,67 @@ func (m *mockConntracker) WalkFlows(f func(endpoint.Flow)) {

 func (m *mockConntracker) Stop() {}

+// TODO(pb): dedupe later
+func makeFlow(ty string) Flow {
+	return Flow{
+		XMLName: xml.Name{
+			Local: "flow",
+		},
+		Type: ty,
+	}
+}
+
+// TODO(pb): dedupe later
+func addMeta(f *Flow, dir, srcIP, dstIP string, srcPort, dstPort int) *Meta {
+	meta := Meta{
+		XMLName: xml.Name{
+			Local: "meta",
+		},
+		Direction: dir,
+		Layer3: Layer3{
+			XMLName: xml.Name{
+				Local: "layer3",
+			},
+			SrcIP: srcIP,
+			DstIP: dstIP,
+		},
+		Layer4: Layer4{
+			XMLName: xml.Name{
+				Local: "layer4",
+			},
+			SrcPort: srcPort,
+			DstPort: dstPort,
+			Proto:   TCP,
+		},
+	}
+	f.Metas = append(f.Metas, meta)
+	return &meta
+}
+
+// TODO(pb): dedupe later
+func addIndependant(f *Flow, id int64, state string) *Meta {
+	meta := Meta{
+		XMLName: xml.Name{
+			Local: "meta",
+		},
+		Direction: "independent",
+		ID:        id,
+		State:     state,
+		Layer3: Layer3{
+			XMLName: xml.Name{
+				Local: "layer3",
+			},
+		},
+		Layer4: Layer4{
+			XMLName: xml.Name{
+				Local: "layer4",
+			},
+		},
+	}
+	f.Metas = append(f.Metas, meta)
+	return &meta
+}
+
 func TestNat(t *testing.T) {
 	// test that two containers, on the docker network, get their connections mapped
 	// correctly.
@@ -36,27 +97,26 @@ func TestNat(t *testing.T) {
 		flow.Original = addMeta(&flow, "original", "2.3.4.5", "1.2.3.4", 222222, 80)
 		flow.Reply = addMeta(&flow, "reply", "10.0.47.1", "2.3.4.5", 80, 222222)
 		ct := &mockConntracker{
-			flows: []endpoint.Flow{flow},
+			flows: []Flow{flow},
 		}

 		have := report.MakeReport()
 		originalID := report.MakeEndpointNodeID("host1", "10.0.47.1", "80")
 		have.Endpoint.AddNode(originalID, report.MakeNodeWith(report.Metadata{
-			endpoint.Addr: "10.0.47.1",
-			endpoint.Port: "80",
-			"foo":         "bar",
+			Addr:  "10.0.47.1",
+			Port:  "80",
+			"foo": "bar",
 		}))

 		want := have.Copy()
 		want.Endpoint.AddNode(report.MakeEndpointNodeID("host1", "1.2.3.4", "80"), report.MakeNodeWith(report.Metadata{
-			endpoint.Addr: "1.2.3.4",
-			endpoint.Port: "80",
-			"copy_of":     originalID,
-			"foo":         "bar",
+			Addr:      "1.2.3.4",
+			Port:      "80",
+			"copy_of": originalID,
+			"foo":     "bar",
 		}))

-		natmapper := endpoint.MakeNATMapper(ct)
-		natmapper.ApplyNAT(have, "host1")
+		makeNATMapper(ct).applyNAT(have, "host1")
 		if !reflect.DeepEqual(want, have) {
 			t.Fatal(test.Diff(want, have))
 		}
@@ -69,27 +129,26 @@ func TestNat(t *testing.T) {
 		flow.Original = addMeta(&flow, "original", "10.0.47.2", "1.2.3.4", 22222, 80)
 		flow.Reply = addMeta(&flow, "reply", "1.2.3.4", "2.3.4.5", 80, 22223)
 		ct := &mockConntracker{
-			flows: []endpoint.Flow{flow},
+			flows: []Flow{flow},
 		}

 		have := report.MakeReport()
 		originalID := report.MakeEndpointNodeID("host2", "10.0.47.2", "22222")
 		have.Endpoint.AddNode(originalID, report.MakeNodeWith(report.Metadata{
-			endpoint.Addr: "10.0.47.2",
-			endpoint.Port: "22222",
-			"foo":         "baz",
+			Addr:  "10.0.47.2",
+			Port:  "22222",
+			"foo": "baz",
 		}))

 		want := have.Copy()
 		want.Endpoint.AddNode(report.MakeEndpointNodeID("host2", "2.3.4.5", "22223"), report.MakeNodeWith(report.Metadata{
-			endpoint.Addr: "2.3.4.5",
-			endpoint.Port: "22223",
-			"copy_of":     originalID,
-			"foo":         "baz",
+			Addr:      "2.3.4.5",
+			Port:      "22223",
+			"copy_of": originalID,
+			"foo":     "baz",
 		}))

-		natmapper := endpoint.MakeNATMapper(ct)
-		natmapper.ApplyNAT(have, "host1")
+		makeNATMapper(ct).applyNAT(have, "host1")
 		if !reflect.DeepEqual(want, have) {
 			t.Fatal(test.Diff(want, have))
 		}
--- a/probe/endpoint/reporter.go
+++ b/probe/endpoint/reporter.go
@@ -27,7 +27,7 @@ type Reporter struct {
 	includeProcesses bool
 	includeNAT       bool
 	conntracker      Conntracker
-	natmapper        *NATMapper
+	natMapper        *natMapper
 	reverseResolver  *reverseResolver
 }

@@ -51,9 +51,9 @@ var SpyDuration = prometheus.NewSummaryVec(
 func NewReporter(hostID, hostName string, includeProcesses bool, useConntrack bool) *Reporter {
 	var (
 		conntracker Conntracker
-		natmapper   *NATMapper
+		natMapper   *natMapper
 	)
-	if ConntrackModulePresent() {
+	if ConntrackModulePresent() { // TODO(pb)
 		if useConntrack {
 			var err error
 			if conntracker, err = NewConntracker(true); err != nil {
@@ -61,8 +61,8 @@ func NewReporter(hostID, hostName string, includeProcesses bool, useConntrack bo
 			}
 		}
 		if natmapperConntracker, err := NewConntracker(true, "--any-nat"); err == nil {
-			m := MakeNATMapper(natmapperConntracker)
-			natmapper = &m
+			m := makeNATMapper(natmapperConntracker)
+			natMapper = &m // TODO(pb): if we only ever use this as a pointer, newNATMapper
 		} else {
 			log.Printf("Failed to start conntracker for NAT mapper: %v", err)
 		}
@@ -72,18 +72,18 @@ func NewReporter(hostID, hostName string, includeProcesses bool, useConntrack bo
 		hostName:         hostName,
 		includeProcesses: includeProcesses,
 		conntracker:      conntracker,
-		natmapper:        natmapper,
+		natMapper:        natMapper,
 		reverseResolver:  newReverseResolver(),
 	}
 }

 // Stop stop stop
 func (r *Reporter) Stop() {
-	if r.conntracker != nil {
+	if r.conntracker != nil { // TODO(pb): this should never be nil (implies interface)
 		r.conntracker.Stop()
 	}
-	if r.natmapper != nil {
-		r.natmapper.Stop()
+	if r.natMapper != nil { // TODO(pb): this should never be nil (implies interface)
+		r.natMapper.Stop()
 	}
 	r.reverseResolver.stop()
 }
@@ -138,8 +138,8 @@ func (r *Reporter) Report() (report.Report, error) {
 		})
 	}

-	if r.natmapper != nil {
-		r.natmapper.ApplyNAT(rpt, r.hostID)
+	if r.natMapper != nil { // TODO(pb): should never be nil
+		r.natMapper.applyNAT(rpt, r.hostID)
 	}

 	return rpt, nil