diff --git a/probe/endpoint/nat.go b/probe/endpoint/nat.go index 18ae534d1..ab1b8f5f1 100644 --- 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) { diff --git a/probe/endpoint/nat_test.go b/probe/endpoint/nat_internal_test.go similarity index 54% rename from probe/endpoint/nat_test.go rename to probe/endpoint/nat_internal_test.go index 49580d512..81ebb051a 100644 --- a/probe/endpoint/nat_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)) } diff --git a/probe/endpoint/reporter.go b/probe/endpoint/reporter.go index 98c086155..0311ad3a0 100644 --- 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