From 6f1e52cd0dca6762a542dbe0080149f79b77b3a0 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Fri, 1 Jul 2016 14:40:20 +0000 Subject: [PATCH 1/6] Allow disabling controls in probes --- app/controls_test.go | 2 +- app/pipes_internal_test.go | 2 +- probe/appclient/app_client.go | 15 ++++++++----- probe/appclient/app_client_internal_test.go | 10 ++++----- probe/appclient/report_publisher.go | 25 ++++++++++++++++++--- probe/probe.go | 3 ++- probe/probe_internal_test.go | 4 ++-- prog/main.go | 2 ++ prog/probe.go | 3 ++- 9 files changed, 47 insertions(+), 19 deletions(-) diff --git a/app/controls_test.go b/app/controls_test.go index 2e1a7c520..34ee06fbf 100644 --- a/app/controls_test.go +++ b/app/controls_test.go @@ -43,7 +43,7 @@ func TestControl(t *testing.T) { Value: "foo", } }) - client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, controlHandler) + client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, controlHandler, false) if err != nil { t.Fatal(err) } diff --git a/app/pipes_internal_test.go b/app/pipes_internal_test.go index 308919755..d01e3960e 100644 --- a/app/pipes_internal_test.go +++ b/app/pipes_internal_test.go @@ -83,7 +83,7 @@ func TestPipeClose(t *testing.T) { probeConfig := appclient.ProbeConfig{ ProbeID: "foo", } - client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, nil) + client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, nil, false) if err != nil { t.Fatal(err) } diff --git a/probe/appclient/app_client.go b/probe/appclient/app_client.go index bffaf0d5c..a66a95dcd 100644 --- a/probe/appclient/app_client.go +++ b/probe/appclient/app_client.go @@ -57,11 +57,12 @@ type appClient struct { readers chan io.Reader // For controls - control xfer.ControlHandler + noControls bool + control xfer.ControlHandler } // NewAppClient makes a new appClient. -func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlHandler) (AppClient, error) { +func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlHandler, noControls bool) (AppClient, error) { httpTransport, err := pc.getHTTPTransport(hostname) if err != nil { return nil, err @@ -82,9 +83,10 @@ func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlH wsDialer: websocket.Dialer{ TLSClientConfig: httpTransport.TLSClientConfig, }, - conns: map[string]xfer.Websocket{}, - readers: make(chan io.Reader, 2), - control: control, + conns: map[string]xfer.Websocket{}, + readers: make(chan io.Reader, 2), + noControls: noControls, + control: control, }, nil } @@ -231,6 +233,9 @@ func (c *appClient) controlConnection() (bool, error) { } func (c *appClient) ControlConnection() { + if c.noControls { + return + } go func() { log.Infof("Control connection to %s starting", c.target) defer log.Infof("Control connection to %s exiting", c.target) diff --git a/probe/appclient/app_client_internal_test.go b/probe/appclient/app_client_internal_test.go index 2a990e3a1..5d8cd149a 100644 --- a/probe/appclient/app_client_internal_test.go +++ b/probe/appclient/app_client_internal_test.go @@ -109,14 +109,14 @@ func TestAppClientPublish(t *testing.T) { Insecure: false, } - p, err := NewAppClient(pc, u.Host, s.URL, nil) + p, err := NewAppClient(pc, u.Host, s.URL, nil, false) if err != nil { t.Fatal(err) } defer p.Stop() // First few reports might be dropped as the client is spinning up. - rp := NewReportPublisher(p) + rp := NewReportPublisher(p, false) for i := 0; i < 10; i++ { if err := rp.Publish(rpt); err != nil { t.Error(err) @@ -158,7 +158,7 @@ func TestAppClientDetails(t *testing.T) { ProbeID: "", Insecure: false, } - p, err := NewAppClient(pc, u.Host, s.URL, nil) + p, err := NewAppClient(pc, u.Host, s.URL, nil, false) if err != nil { t.Fatal(err) } @@ -203,12 +203,12 @@ func TestStop(t *testing.T) { Insecure: false, } - p, err := NewAppClient(pc, u.Host, s.URL, nil) + p, err := NewAppClient(pc, u.Host, s.URL, nil, false) if err != nil { t.Fatal(err) } - rp := NewReportPublisher(p) + rp := NewReportPublisher(p, false) // Make sure the app received our report and is stuck for done := false; !done; { diff --git a/probe/appclient/report_publisher.go b/probe/appclient/report_publisher.go index 2e2134147..f1a51c958 100644 --- a/probe/appclient/report_publisher.go +++ b/probe/appclient/report_publisher.go @@ -8,18 +8,37 @@ import ( // A ReportPublisher uses a buffer pool to serialise reports, which it // then passes to a publisher type ReportPublisher struct { - publisher Publisher + publisher Publisher + noControls bool } // NewReportPublisher creates a new report publisher -func NewReportPublisher(publisher Publisher) *ReportPublisher { +func NewReportPublisher(publisher Publisher, noControls bool) *ReportPublisher { return &ReportPublisher{ - publisher: publisher, + publisher: publisher, + noControls: noControls, } } +func removeControls(r report.Report) report.Report { + r.Endpoint.Controls = report.Controls{} + r.Process.Controls = report.Controls{} + r.Container.Controls = report.Controls{} + r.ContainerImage.Controls = report.Controls{} + r.Pod.Controls = report.Controls{} + r.Service.Controls = report.Controls{} + r.Deployment.Controls = report.Controls{} + r.ReplicaSet.Controls = report.Controls{} + r.Host.Controls = report.Controls{} + r.Overlay.Controls = report.Controls{} + return r +} + // Publish serialises and compresses a report, then passes it to a publisher func (p *ReportPublisher) Publish(r report.Report) error { + if p.noControls { + r = removeControls(r) + } buf := &bytes.Buffer{} r.WriteBinary(buf) return p.publisher.Publish(buf) diff --git a/probe/probe.go b/probe/probe.go index 61124a933..a9139fc4d 100644 --- a/probe/probe.go +++ b/probe/probe.go @@ -68,11 +68,12 @@ type Ticker interface { func New( spyInterval, publishInterval time.Duration, publisher appclient.Publisher, + noControls bool, ) *Probe { result := &Probe{ spyInterval: spyInterval, publishInterval: publishInterval, - publisher: appclient.NewReportPublisher(publisher), + publisher: appclient.NewReportPublisher(publisher, noControls), quit: make(chan struct{}), spiedReports: make(chan report.Report, reportBufferSize), shortcutReports: make(chan report.Report, reportBufferSize), diff --git a/probe/probe_internal_test.go b/probe/probe_internal_test.go index 5b4aac0f4..7c92451c2 100644 --- a/probe/probe_internal_test.go +++ b/probe/probe_internal_test.go @@ -19,7 +19,7 @@ func TestApply(t *testing.T) { endpointNode = report.MakeNodeWith(endpointNodeID, map[string]string{"5": "6"}) ) - p := New(0, 0, nil) + p := New(0, 0, nil, false) p.AddTagger(NewTopologyTagger()) r := report.MakeReport() @@ -95,7 +95,7 @@ func TestProbe(t *testing.T) { pub := mockPublisher{make(chan report.Report, 10)} - p := New(10*time.Millisecond, 100*time.Millisecond, pub) + p := New(10*time.Millisecond, 100*time.Millisecond, pub, false) p.AddReporter(mockReporter{want}) p.Start() defer p.Stop() diff --git a/prog/main.go b/prog/main.go index f1c9da818..505c0bc66 100644 --- a/prog/main.go +++ b/prog/main.go @@ -67,6 +67,7 @@ type probeFlags struct { logLevel string resolver string noApp bool + noControls bool useConntrack bool // Use conntrack for endpoint topo spyProcs bool // Associate endpoints with processes (must be root) @@ -145,6 +146,7 @@ func main() { flag.DurationVar(&flags.probe.publishInterval, "probe.publish.interval", 3*time.Second, "publish (output) interval") flag.DurationVar(&flags.probe.spyInterval, "probe.spy.interval", time.Second, "spy (scan) interval") flag.StringVar(&flags.probe.pluginsRoot, "probe.plugins.root", "/var/run/scope/plugins", "Root directory to search for plugins") + flag.BoolVar(&flags.probe.noControls, "probe.no-controls", false, "Disable controls (read-only mode)") flag.BoolVar(&flags.probe.insecure, "probe.insecure", false, "(SSL) explicitly allow \"insecure\" SSL connections and transfers") flag.StringVar(&flags.probe.resolver, "probe.resolver", "", "IP address & port of resolver to use. Default is to use system resolver.") diff --git a/prog/probe.go b/prog/probe.go index 43b58a1b9..657b73a35 100644 --- a/prog/probe.go +++ b/prog/probe.go @@ -117,6 +117,7 @@ func probeMain(flags probeFlags) { return appclient.NewAppClient( probeConfig, hostname, endpoint, xfer.ControlHandlerFunc(controls.HandleControlRequest), + flags.noControls, ) }) defer clients.Stop() @@ -128,7 +129,7 @@ func probeMain(flags probeFlags) { resolver := appclient.NewResolver(targets, dnsLookupFn, clients.Set) defer resolver.Stop() - p := probe.New(flags.spyInterval, flags.publishInterval, clients) + p := probe.New(flags.spyInterval, flags.publishInterval, clients, flags.noControls) hostReporter := host.NewReporter(hostID, hostName, probeID, version, clients) defer hostReporter.Stop() From c0a672c02a0485fab17f3ceaaf6c5c9bb7365eaa Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Fri, 1 Jul 2016 16:51:57 +0000 Subject: [PATCH 2/6] Review feedback --- app/controls_test.go | 2 +- app/pipes_internal_test.go | 2 +- probe/appclient/app_client.go | 15 ++++-------- probe/appclient/app_client_internal_test.go | 6 ++--- probe/appclient/multi_client.go | 26 ++++++++++----------- probe/appclient/multi_client_test.go | 4 ++-- prog/probe.go | 6 ++--- 7 files changed, 28 insertions(+), 33 deletions(-) diff --git a/app/controls_test.go b/app/controls_test.go index 34ee06fbf..2e1a7c520 100644 --- a/app/controls_test.go +++ b/app/controls_test.go @@ -43,7 +43,7 @@ func TestControl(t *testing.T) { Value: "foo", } }) - client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, controlHandler, false) + client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, controlHandler) if err != nil { t.Fatal(err) } diff --git a/app/pipes_internal_test.go b/app/pipes_internal_test.go index d01e3960e..308919755 100644 --- a/app/pipes_internal_test.go +++ b/app/pipes_internal_test.go @@ -83,7 +83,7 @@ func TestPipeClose(t *testing.T) { probeConfig := appclient.ProbeConfig{ ProbeID: "foo", } - client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, nil, false) + client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, nil) if err != nil { t.Fatal(err) } diff --git a/probe/appclient/app_client.go b/probe/appclient/app_client.go index a66a95dcd..bffaf0d5c 100644 --- a/probe/appclient/app_client.go +++ b/probe/appclient/app_client.go @@ -57,12 +57,11 @@ type appClient struct { readers chan io.Reader // For controls - noControls bool - control xfer.ControlHandler + control xfer.ControlHandler } // NewAppClient makes a new appClient. -func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlHandler, noControls bool) (AppClient, error) { +func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlHandler) (AppClient, error) { httpTransport, err := pc.getHTTPTransport(hostname) if err != nil { return nil, err @@ -83,10 +82,9 @@ func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlH wsDialer: websocket.Dialer{ TLSClientConfig: httpTransport.TLSClientConfig, }, - conns: map[string]xfer.Websocket{}, - readers: make(chan io.Reader, 2), - noControls: noControls, - control: control, + conns: map[string]xfer.Websocket{}, + readers: make(chan io.Reader, 2), + control: control, }, nil } @@ -233,9 +231,6 @@ func (c *appClient) controlConnection() (bool, error) { } func (c *appClient) ControlConnection() { - if c.noControls { - return - } go func() { log.Infof("Control connection to %s starting", c.target) defer log.Infof("Control connection to %s exiting", c.target) diff --git a/probe/appclient/app_client_internal_test.go b/probe/appclient/app_client_internal_test.go index 5d8cd149a..d8acc0ed7 100644 --- a/probe/appclient/app_client_internal_test.go +++ b/probe/appclient/app_client_internal_test.go @@ -109,7 +109,7 @@ func TestAppClientPublish(t *testing.T) { Insecure: false, } - p, err := NewAppClient(pc, u.Host, s.URL, nil, false) + p, err := NewAppClient(pc, u.Host, s.URL, nil) if err != nil { t.Fatal(err) } @@ -158,7 +158,7 @@ func TestAppClientDetails(t *testing.T) { ProbeID: "", Insecure: false, } - p, err := NewAppClient(pc, u.Host, s.URL, nil, false) + p, err := NewAppClient(pc, u.Host, s.URL, nil) if err != nil { t.Fatal(err) } @@ -203,7 +203,7 @@ func TestStop(t *testing.T) { Insecure: false, } - p, err := NewAppClient(pc, u.Host, s.URL, nil, false) + p, err := NewAppClient(pc, u.Host, s.URL, nil) if err != nil { t.Fatal(err) } diff --git a/probe/appclient/multi_client.go b/probe/appclient/multi_client.go index 061373059..91428507b 100644 --- a/probe/appclient/multi_client.go +++ b/probe/appclient/multi_client.go @@ -23,11 +23,12 @@ type ClientFactory func(string, string) (AppClient, error) type multiClient struct { clientFactory ClientFactory - mtx sync.Mutex - sema semaphore - clients map[string]AppClient // holds map from app id -> client - ids map[string]report.IDList // holds map from hostname -> app ids - quit chan struct{} + mtx sync.Mutex + sema semaphore + clients map[string]AppClient // holds map from app id -> client + ids map[string]report.IDList // holds map from hostname -> app ids + quit chan struct{} + noControls bool } type clientTuple struct { @@ -53,14 +54,15 @@ type MultiAppClient interface { } // NewMultiAppClient creates a new MultiAppClient. -func NewMultiAppClient(clientFactory ClientFactory) MultiAppClient { +func NewMultiAppClient(clientFactory ClientFactory, noControls bool) MultiAppClient { return &multiClient{ clientFactory: clientFactory, - sema: newSemaphore(maxConcurrentGET), - clients: map[string]AppClient{}, - ids: map[string]report.IDList{}, - quit: make(chan struct{}), + sema: newSemaphore(maxConcurrentGET), + clients: map[string]AppClient{}, + ids: map[string]report.IDList{}, + quit: make(chan struct{}), + noControls: noControls, } } @@ -100,9 +102,7 @@ func (c *multiClient) Set(hostname string, endpoints []string) { hostIDs := report.MakeIDList() for tuple := range clients { hostIDs = hostIDs.Add(tuple.ID) - - _, ok := c.clients[tuple.ID] - if !ok { + if _, ok := c.clients[tuple.ID]; !ok && !c.noControls { c.clients[tuple.ID] = tuple.AppClient tuple.AppClient.ControlConnection() } diff --git a/probe/appclient/multi_client_test.go b/probe/appclient/multi_client_test.go index 98126c1cd..5a24519b5 100644 --- a/probe/appclient/multi_client_test.go +++ b/probe/appclient/multi_client_test.go @@ -67,7 +67,7 @@ func TestMultiClient(t *testing.T) { } ) - mp := appclient.NewMultiAppClient(factory) + mp := appclient.NewMultiAppClient(factory, false) defer mp.Stop() // Add two hostnames with overlapping apps, check we don't add the same app twice @@ -89,7 +89,7 @@ func TestMultiClient(t *testing.T) { } func TestMultiClientPublish(t *testing.T) { - mp := appclient.NewMultiAppClient(factory) + mp := appclient.NewMultiAppClient(factory, false) defer mp.Stop() sum := func() int { return a1.publish + a2.publish + b2.publish + b3.publish } diff --git a/prog/probe.go b/prog/probe.go index 657b73a35..f461664cb 100644 --- a/prog/probe.go +++ b/prog/probe.go @@ -113,13 +113,13 @@ func probeMain(flags probeFlags) { ProbeID: probeID, Insecure: flags.insecure, } - clients := appclient.NewMultiAppClient(func(hostname, endpoint string) (appclient.AppClient, error) { + clientFactory := func(hostname, endpoint string) (appclient.AppClient, error) { return appclient.NewAppClient( probeConfig, hostname, endpoint, xfer.ControlHandlerFunc(controls.HandleControlRequest), - flags.noControls, ) - }) + } + clients := appclient.NewMultiAppClient(clientFactory, flags.noControls) defer clients.Stop() dnsLookupFn := net.LookupIP From 205ee5b4e6bacdaf107a184327cac5ad5708e58a Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Fri, 1 Jul 2016 17:30:10 +0000 Subject: [PATCH 3/6] Fix fixprobe --- experimental/fixprobe/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/fixprobe/main.go b/experimental/fixprobe/main.go index e5bc06bee..f490a089f 100644 --- a/experimental/fixprobe/main.go +++ b/experimental/fixprobe/main.go @@ -54,7 +54,7 @@ func main() { log.Fatal(err) } - rp := appclient.NewReportPublisher(client) + rp := appclient.NewReportPublisher(client, false) for range time.Tick(*publishInterval) { rp.Publish(fixedReport) } From c0d5c394ad4049c435054258d2ac619fffb98230 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Mon, 4 Jul 2016 14:32:07 +0000 Subject: [PATCH 4/6] Fix demoprobe --- experimental/demoprobe/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/demoprobe/main.go b/experimental/demoprobe/main.go index 264c36435..e896f6446 100644 --- a/experimental/demoprobe/main.go +++ b/experimental/demoprobe/main.go @@ -31,7 +31,7 @@ func main() { if err != nil { log.Fatal(err) } - rp := appclient.NewReportPublisher(client) + rp := appclient.NewReportPublisher(client, false) rand.Seed(time.Now().UnixNano()) for range time.Tick(*publishInterval) { From 6175880725c96e96416d33289039a9e000799bb0 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Mon, 4 Jul 2016 14:44:17 +0000 Subject: [PATCH 5/6] Review feedback --- probe/appclient/multi_client.go | 6 ++++-- probe/appclient/report_publisher.go | 18 +++------------- report/report.go | 32 ++++++++++++++++++----------- 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/probe/appclient/multi_client.go b/probe/appclient/multi_client.go index 91428507b..012556b8d 100644 --- a/probe/appclient/multi_client.go +++ b/probe/appclient/multi_client.go @@ -102,9 +102,11 @@ func (c *multiClient) Set(hostname string, endpoints []string) { hostIDs := report.MakeIDList() for tuple := range clients { hostIDs = hostIDs.Add(tuple.ID) - if _, ok := c.clients[tuple.ID]; !ok && !c.noControls { + if _, ok := c.clients[tuple.ID]; !ok { c.clients[tuple.ID] = tuple.AppClient - tuple.AppClient.ControlConnection() + if !c.noControls { + tuple.AppClient.ControlConnection() + } } } c.ids[hostname] = hostIDs diff --git a/probe/appclient/report_publisher.go b/probe/appclient/report_publisher.go index f1a51c958..890d22c95 100644 --- a/probe/appclient/report_publisher.go +++ b/probe/appclient/report_publisher.go @@ -20,24 +20,12 @@ func NewReportPublisher(publisher Publisher, noControls bool) *ReportPublisher { } } -func removeControls(r report.Report) report.Report { - r.Endpoint.Controls = report.Controls{} - r.Process.Controls = report.Controls{} - r.Container.Controls = report.Controls{} - r.ContainerImage.Controls = report.Controls{} - r.Pod.Controls = report.Controls{} - r.Service.Controls = report.Controls{} - r.Deployment.Controls = report.Controls{} - r.ReplicaSet.Controls = report.Controls{} - r.Host.Controls = report.Controls{} - r.Overlay.Controls = report.Controls{} - return r -} - // Publish serialises and compresses a report, then passes it to a publisher func (p *ReportPublisher) Publish(r report.Report) error { if p.noControls { - r = removeControls(r) + r.WalkTopologies(func(t *report.Topology) { + t.Controls = report.Controls{} + }) } buf := &bytes.Buffer{} r.WriteBinary(buf) diff --git a/report/report.go b/report/report.go index 610d9d13b..406471e3b 100644 --- a/report/report.go +++ b/report/report.go @@ -197,18 +197,26 @@ func (r Report) Merge(other Report) Report { // Topologies returns a slice of Topologies in this report func (r Report) Topologies() []Topology { - return []Topology{ - r.Endpoint, - r.Process, - r.Container, - r.ContainerImage, - r.Pod, - r.Service, - r.Deployment, - r.ReplicaSet, - r.Host, - r.Overlay, - } + result := []Topology{} + r.WalkTopologies(func(t *Topology) { + result = append(result, *t) + }) + return result +} + +// WalkTopologies iterates through the Topologies of the report, +// potentially modifying them +func (r *Report) WalkTopologies(f func(*Topology)) { + f(&r.Endpoint) + f(&r.Process) + f(&r.Container) + f(&r.ContainerImage) + f(&r.Pod) + f(&r.Service) + f(&r.Deployment) + f(&r.ReplicaSet) + f(&r.Host) + f(&r.Overlay) } // Topology gets a topology by name From 5bb6185515d2b08d9b6cbdc644102b72fa4a91ef Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Thu, 7 Jul 2016 12:36:33 +0000 Subject: [PATCH 6/6] Clarify no-controls flag --- prog/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prog/main.go b/prog/main.go index 505c0bc66..58072bd05 100644 --- a/prog/main.go +++ b/prog/main.go @@ -146,7 +146,7 @@ func main() { flag.DurationVar(&flags.probe.publishInterval, "probe.publish.interval", 3*time.Second, "publish (output) interval") flag.DurationVar(&flags.probe.spyInterval, "probe.spy.interval", time.Second, "spy (scan) interval") flag.StringVar(&flags.probe.pluginsRoot, "probe.plugins.root", "/var/run/scope/plugins", "Root directory to search for plugins") - flag.BoolVar(&flags.probe.noControls, "probe.no-controls", false, "Disable controls (read-only mode)") + flag.BoolVar(&flags.probe.noControls, "probe.no-controls", false, "Disable controls (e.g. start/stop containers, terminals, logs ...)") flag.BoolVar(&flags.probe.insecure, "probe.insecure", false, "(SSL) explicitly allow \"insecure\" SSL connections and transfers") flag.StringVar(&flags.probe.resolver, "probe.resolver", "", "IP address & port of resolver to use. Default is to use system resolver.")