From 8bf753a51b8456ead140aeef7ce4e097b067e573 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Wed, 8 Mar 2017 02:16:42 -0800 Subject: [PATCH] Revert "Revert "Add options to hide args and env vars (#2306)"" (#2311) * Revert "Revert "Add options to hide args and env vars (#2306)"" * Make linter happy --- probe/docker/container.go | 38 +++++--- probe/docker/container_test.go | 42 +++++++- probe/docker/controls_test.go | 10 +- probe/docker/registry.go | 46 ++++++--- probe/docker/registry_test.go | 15 ++- probe/process/reporter.go | 26 +++-- probe/process/reporter_test.go | 170 ++++++++++++++++++++------------- prog/main.go | 26 ++--- prog/probe.go | 14 ++- 9 files changed, 266 insertions(+), 121 deletions(-) diff --git a/probe/docker/container.go b/probe/docker/container.go index 419fb8605..19973bcf3 100644 --- a/probe/docker/container.go +++ b/probe/docker/container.go @@ -98,20 +98,24 @@ type Container interface { type container struct { sync.RWMutex - container *docker.Container - stopStats chan<- bool - latestStats docker.Stats - pendingStats [60]docker.Stats - numPending int - hostID string - baseNode report.Node + container *docker.Container + stopStats chan<- bool + latestStats docker.Stats + pendingStats [60]docker.Stats + numPending int + hostID string + baseNode report.Node + noCommandLineArguments bool + noEnvironmentVariables bool } // NewContainer creates a new Container -func NewContainer(c *docker.Container, hostID string) Container { +func NewContainer(c *docker.Container, hostID string, noCommandLineArguments bool, noEnvironmentVariables bool) Container { result := &container{ - container: c, - hostID: hostID, + container: c, + hostID: hostID, + noCommandLineArguments: noCommandLineArguments, + noEnvironmentVariables: noEnvironmentVariables, } result.baseNode = result.getBaseNode() return result @@ -369,18 +373,28 @@ func (c *container) env() map[string]string { return result } +func (c *container) getSanitizedCommand() string { + result := c.container.Path + if !c.noCommandLineArguments { + result = result + " " + strings.Join(c.container.Args, " ") + } + return result +} + func (c *container) getBaseNode() report.Node { result := report.MakeNodeWith(report.MakeContainerNodeID(c.ID()), map[string]string{ ContainerID: c.ID(), ContainerCreated: c.container.Created.Format(time.RFC3339Nano), - ContainerCommand: c.container.Path + " " + strings.Join(c.container.Args, " "), + ContainerCommand: c.getSanitizedCommand(), ImageID: c.Image(), ContainerHostname: c.Hostname(), }).WithParents(report.EmptySets. Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(c.Image()))), ) result = result.AddPrefixPropertyList(LabelPrefix, c.container.Config.Labels) - result = result.AddPrefixPropertyList(EnvPrefix, c.env()) + if !c.noEnvironmentVariables { + result = result.AddPrefixPropertyList(EnvPrefix, c.env()) + } return result } diff --git a/probe/docker/container_test.go b/probe/docker/container_test.go index 1098e13c7..b042c70c9 100644 --- a/probe/docker/container_test.go +++ b/probe/docker/container_test.go @@ -2,6 +2,7 @@ package docker_test import ( "net" + "strings" "testing" "time" @@ -40,7 +41,7 @@ func TestContainer(t *testing.T) { defer mtime.NowReset() const hostID = "scope" - c := docker.NewContainer(container1, hostID) + c := docker.NewContainer(container1, hostID, false, false) s := newMockStatsGatherer() err := c.StartGatheringStats(s) if err != nil { @@ -69,7 +70,7 @@ func TestContainer(t *testing.T) { docker.RemoveContainer: {Dead: true}, } want := report.MakeNodeWith("ping;", map[string]string{ - "docker_container_command": " ", + "docker_container_command": "ping foo.bar.local", "docker_container_created": "0001-01-01T00:00:00Z", "docker_container_id": "ping", "docker_container_name": "pong", @@ -79,6 +80,7 @@ func TestContainer(t *testing.T) { "docker_container_state": "running", "docker_container_state_human": c.Container().State.String(), "docker_container_uptime": uptime.String(), + "docker_env_FOO": "secret-bar", }).WithLatestControls( controls, ).WithMetrics(report.Metrics{ @@ -125,3 +127,39 @@ func TestContainer(t *testing.T) { t.Errorf("%v != %v", have, []string{"1.2.3.4", "5.6.7.8"}) } } + +func TestContainerHidingArgs(t *testing.T) { + const hostID = "scope" + c := docker.NewContainer(container1, hostID, true, false) + node := c.GetNode() + node.Latest.ForEach(func(k string, _ time.Time, v string) { + if strings.Contains(v, "foo.bar.local") { + t.Errorf("Found command line argument in node") + } + }) +} + +func TestContainerHidingEnv(t *testing.T) { + const hostID = "scope" + c := docker.NewContainer(container1, hostID, false, true) + node := c.GetNode() + node.Latest.ForEach(func(k string, _ time.Time, v string) { + if strings.Contains(v, "secret-bar") { + t.Errorf("Found environment variable in node") + } + }) +} + +func TestContainerHidingBoth(t *testing.T) { + const hostID = "scope" + c := docker.NewContainer(container1, hostID, true, true) + node := c.GetNode() + node.Latest.ForEach(func(k string, _ time.Time, v string) { + if strings.Contains(v, "foo.bar.local") { + t.Errorf("Found command line argument in node") + } + if strings.Contains(v, "secret-bar") { + t.Errorf("Found environment variable in node") + } + }) +} diff --git a/probe/docker/controls_test.go b/probe/docker/controls_test.go index 855ea326d..2a3aafe1b 100644 --- a/probe/docker/controls_test.go +++ b/probe/docker/controls_test.go @@ -18,7 +18,10 @@ func TestControls(t *testing.T) { mdc := newMockClient() setupStubs(mdc, func() { hr := controls.NewDefaultHandlerRegistry() - registry, _ := docker.NewRegistry(10*time.Second, nil, false, "", hr, "") + registry, _ := docker.NewRegistry(docker.RegistryOptions{ + Interval: 10 * time.Second, + HandlerRegistry: hr, + }) defer registry.Stop() for _, tc := range []struct{ command, result string }{ @@ -59,7 +62,10 @@ func TestPipes(t *testing.T) { mdc := newMockClient() setupStubs(mdc, func() { hr := controls.NewDefaultHandlerRegistry() - registry, _ := docker.NewRegistry(10*time.Second, nil, false, "", hr, "") + registry, _ := docker.NewRegistry(docker.RegistryOptions{ + Interval: 10 * time.Second, + HandlerRegistry: hr, + }) defer registry.Stop() test.Poll(t, 100*time.Millisecond, true, func() interface{} { diff --git a/probe/docker/registry.go b/probe/docker/registry.go index 2fd3bcef8..1082def1c 100644 --- a/probe/docker/registry.go +++ b/probe/docker/registry.go @@ -51,13 +51,15 @@ type ContainerUpdateWatcher func(report.Node) type registry struct { sync.RWMutex - quit chan chan struct{} - interval time.Duration - collectStats bool - client Client - pipes controls.PipeClient - hostID string - handlerRegistry *controls.HandlerRegistry + quit chan chan struct{} + interval time.Duration + collectStats bool + client Client + pipes controls.PipeClient + hostID string + handlerRegistry *controls.HandlerRegistry + noCommandLineArguments bool + noEnvironmentVariables bool watchers []ContainerUpdateWatcher containers *radix.Tree @@ -93,9 +95,21 @@ func newDockerClient(endpoint string) (Client, error) { return docker_client.NewClient(endpoint) } +// RegistryOptions are used to initialize the Registry +type RegistryOptions struct { + Interval time.Duration + Pipes controls.PipeClient + CollectStats bool + HostID string + HandlerRegistry *controls.HandlerRegistry + DockerEndpoint string + NoCommandLineArguments bool + NoEnvironmentVariables bool +} + // NewRegistry returns a usable Registry. Don't forget to Stop it. -func NewRegistry(interval time.Duration, pipes controls.PipeClient, collectStats bool, hostID string, handlerRegistry *controls.HandlerRegistry, dockerEndpoint string) (Registry, error) { - client, err := NewDockerClientStub(dockerEndpoint) +func NewRegistry(options RegistryOptions) (Registry, error) { + client, err := NewDockerClientStub(options.DockerEndpoint) if err != nil { return nil, err } @@ -107,12 +121,14 @@ func NewRegistry(interval time.Duration, pipes controls.PipeClient, collectStats pipeIDToexecID: map[string]string{}, client: client, - pipes: pipes, - interval: interval, - collectStats: collectStats, - hostID: hostID, - handlerRegistry: handlerRegistry, + pipes: options.Pipes, + interval: options.Interval, + collectStats: options.CollectStats, + hostID: options.HostID, + handlerRegistry: options.HandlerRegistry, quit: make(chan chan struct{}), + noCommandLineArguments: options.NoCommandLineArguments, + noEnvironmentVariables: options.NoEnvironmentVariables, } r.registerControls() @@ -339,7 +355,7 @@ func (r *registry) updateContainerState(containerID string, intendedState *strin o, ok := r.containers.Get(containerID) var c Container if !ok { - c = NewContainerStub(dockerContainer, r.hostID) + c = NewContainerStub(dockerContainer, r.hostID, r.noCommandLineArguments, r.noEnvironmentVariables) r.containers.Insert(containerID, c) } else { c = o.(Container) diff --git a/probe/docker/registry_test.go b/probe/docker/registry_test.go index 2f110b5e7..0fd6bcaa2 100644 --- a/probe/docker/registry_test.go +++ b/probe/docker/registry_test.go @@ -22,7 +22,11 @@ import ( func testRegistry() docker.Registry { hr := controls.NewDefaultHandlerRegistry() - registry, _ := docker.NewRegistry(10*time.Second, nil, true, "", hr, "") + registry, _ := docker.NewRegistry(docker.RegistryOptions{ + Interval: 10 * time.Second, + CollectStats: true, + HandlerRegistry: hr, + }) return registry } @@ -203,6 +207,10 @@ var ( ID: "ping", Name: "pong", Image: "baz", + Path: "ping", + Args: []string{ + "foo.bar.local", + }, State: client.State{ Pid: 2, Running: true, @@ -226,6 +234,9 @@ var ( }, }, Config: &client.Config{ + Env: []string{ + "FOO=secret-bar", + }, Labels: map[string]string{ "foo1": "bar1", "foo2": "bar2", @@ -294,7 +305,7 @@ func setupStubs(mdc *mockDockerClient, f func()) { return mdc, nil } - docker.NewContainerStub = func(c *client.Container, _ string) docker.Container { + docker.NewContainerStub = func(c *client.Container, _ string, _ bool, _ bool) docker.Container { return &mockContainer{c} } diff --git a/probe/process/reporter.go b/probe/process/reporter.go index 5dcbfcfd9..a84267a86 100644 --- a/probe/process/reporter.go +++ b/probe/process/reporter.go @@ -2,6 +2,7 @@ package process import ( "strconv" + "strings" "github.com/weaveworks/common/mtime" "github.com/weaveworks/scope/report" @@ -37,20 +38,22 @@ var ( // Reporter generates Reports containing the Process topology. type Reporter struct { - scope string - walker Walker - jiffies Jiffies + scope string + walker Walker + jiffies Jiffies + noCommandLineArguments bool } // Jiffies is the type for the function used to fetch the elapsed jiffies. type Jiffies func() (uint64, float64, error) // NewReporter makes a new Reporter. -func NewReporter(walker Walker, scope string, jiffies Jiffies) *Reporter { +func NewReporter(walker Walker, scope string, jiffies Jiffies, noCommandLineArguments bool) *Reporter { return &Reporter{ - scope: scope, - walker: walker, - jiffies: jiffies, + scope: scope, + walker: walker, + jiffies: jiffies, + noCommandLineArguments: noCommandLineArguments, } } @@ -85,7 +88,6 @@ func (r *Reporter) processTopology() (report.Topology, error) { for _, tuple := range []struct{ key, value string }{ {PID, pidstr}, {Name, p.Name}, - {Cmdline, p.Cmdline}, {Threads, strconv.Itoa(p.Threads)}, } { if tuple.value != "" { @@ -93,6 +95,14 @@ func (r *Reporter) processTopology() (report.Topology, error) { } } + if p.Cmdline != "" { + if r.noCommandLineArguments { + node = node.WithLatests(map[string]string{Cmdline: strings.Split(p.Cmdline, " ")[0]}) + } else { + node = node.WithLatests(map[string]string{Cmdline: p.Cmdline}) + } + } + if p.PPID > 0 { node = node.WithLatests(map[string]string{PPID: strconv.Itoa(p.PPID)}) } diff --git a/probe/process/reporter_test.go b/probe/process/reporter_test.go index d4bfe2a31..c741874e7 100644 --- a/probe/process/reporter_test.go +++ b/probe/process/reporter_test.go @@ -21,80 +21,116 @@ func (m *mockWalker) Walk(f func(process.Process, process.Process)) error { return nil } -func TestReporter(t *testing.T) { - processes := []process.Process{ - {PID: 1, PPID: 0, Name: "init"}, - {PID: 2, PPID: 1, Name: "bash"}, - {PID: 3, PPID: 1, Name: "apache", Threads: 2}, - {PID: 4, PPID: 2, Name: "ping", Cmdline: "ping foo.bar.local"}, - {PID: 5, PPID: 1, Cmdline: "tail -f /var/log/syslog"}, - } +var processes = []process.Process{ + {PID: 1, PPID: 0, Name: "init"}, + {PID: 2, PPID: 1, Name: "bash"}, + {PID: 3, PPID: 1, Name: "apache", Threads: 2}, + {PID: 4, PPID: 2, Name: "ping", Cmdline: "ping foo.bar.local"}, + {PID: 5, PPID: 1, Cmdline: "tail -f /var/log/syslog"}, +} + +func testReporter(t *testing.T, noCommandLineArguments bool, test func(report.Report)) { walker := &mockWalker{processes: processes} getDeltaTotalJiffies := func() (uint64, float64, error) { return 0, 0., nil } now := time.Now() mtime.NowForce(now) defer mtime.NowReset() - rpt, err := process.NewReporter(walker, "", getDeltaTotalJiffies).Report() + rpt, err := process.NewReporter(walker, "", getDeltaTotalJiffies, noCommandLineArguments).Report() if err != nil { t.Error(err) } - - // It reports the init process - node, ok := rpt.Process.Nodes[report.MakeProcessNodeID("", "1")] - if !ok { - t.Errorf("Expected report to include the pid 1 init") - } - if name, ok := node.Latest.Lookup(process.Name); !ok || name != processes[0].Name { - t.Errorf("Expected %q got %q", processes[0].Name, name) - } - - // It reports plain processes (with parent pid, and metrics) - node, ok = rpt.Process.Nodes[report.MakeProcessNodeID("", "2")] - if !ok { - t.Errorf("Expected report to include the pid 2 bash") - } - if name, ok := node.Latest.Lookup(process.Name); !ok || name != processes[1].Name { - t.Errorf("Expected %q got %q", processes[1].Name, name) - } - if ppid, ok := node.Latest.Lookup(process.PPID); !ok || ppid != fmt.Sprint(processes[1].PPID) { - t.Errorf("Expected %d got %q", processes[1].PPID, ppid) - } - if memoryUsage, ok := node.Metrics[process.MemoryUsage]; !ok { - t.Errorf("Expected memory usage metric, but not found") - } else if sample, ok := memoryUsage.LastSample(); !ok { - t.Errorf("Expected memory usage metric to have a sample, but there were none") - } else if sample.Value != 0. { - t.Errorf("Expected memory usage metric sample %f, got %f", 0., sample.Value) - } - - // It reports thread-counts - node, ok = rpt.Process.Nodes[report.MakeProcessNodeID("", "3")] - if !ok { - t.Errorf("Expected report to include the pid 3 apache") - } - if threads, ok := node.Latest.Lookup(process.Threads); !ok || threads != fmt.Sprint(processes[2].Threads) { - t.Errorf("Expected %d got %q", processes[2].Threads, threads) - } - - // It reports the Cmdline - node, ok = rpt.Process.Nodes[report.MakeProcessNodeID("", "4")] - if !ok { - t.Errorf("Expected report to include the pid 4 ping") - } - if cmdline, ok := node.Latest.Lookup(process.Cmdline); !ok || cmdline != fmt.Sprint(processes[3].Cmdline) { - t.Errorf("Expected %q got %q", processes[3].Cmdline, cmdline) - } - - // It reports processes without a Name - node, ok = rpt.Process.Nodes[report.MakeProcessNodeID("", "5")] - if !ok { - t.Errorf("Expected report to include the pid 5 tail") - } - if name, ok := node.Latest.Lookup(process.Name); ok { - t.Errorf("Expected no name, but got %q", name) - } - if cmdline, ok := node.Latest.Lookup(process.Cmdline); !ok || cmdline != fmt.Sprint(processes[4].Cmdline) { - t.Errorf("Expected %q got %q", processes[4].Cmdline, cmdline) - } + test(rpt) +} + +func TestInit(t *testing.T) { + test := func(rpt report.Report) { + node, ok := rpt.Process.Nodes[report.MakeProcessNodeID("", "1")] + if !ok { + t.Errorf("Expected report to include the pid 1 init") + } + if name, ok := node.Latest.Lookup(process.Name); !ok || name != processes[0].Name { + t.Errorf("Expected %q got %q", processes[0].Name, name) + } + } + testReporter(t, false, test) +} + +func TestProcesses(t *testing.T) { + test := func(rpt report.Report) { + node, ok := rpt.Process.Nodes[report.MakeProcessNodeID("", "2")] + if !ok { + t.Errorf("Expected report to include the pid 2 bash") + } + if name, ok := node.Latest.Lookup(process.Name); !ok || name != processes[1].Name { + t.Errorf("Expected %q got %q", processes[1].Name, name) + } + if ppid, ok := node.Latest.Lookup(process.PPID); !ok || ppid != fmt.Sprint(processes[1].PPID) { + t.Errorf("Expected %d got %q", processes[1].PPID, ppid) + } + if memoryUsage, ok := node.Metrics[process.MemoryUsage]; !ok { + t.Errorf("Expected memory usage metric, but not found") + } else if sample, ok := memoryUsage.LastSample(); !ok { + t.Errorf("Expected memory usage metric to have a sample, but there were none") + } else if sample.Value != 0. { + t.Errorf("Expected memory usage metric sample %f, got %f", 0., sample.Value) + } + } + + testReporter(t, false, test) +} + +func TestThreadCounts(t *testing.T) { + test := func(rpt report.Report) { + node, ok := rpt.Process.Nodes[report.MakeProcessNodeID("", "3")] + if !ok { + t.Errorf("Expected report to include the pid 3 apache") + } + if threads, ok := node.Latest.Lookup(process.Threads); !ok || threads != fmt.Sprint(processes[2].Threads) { + t.Errorf("Expected %d got %q", processes[2].Threads, threads) + } + } + testReporter(t, false, test) +} + +func TestCmdline(t *testing.T) { + test := func(rpt report.Report) { + node, ok := rpt.Process.Nodes[report.MakeProcessNodeID("", "4")] + if !ok { + t.Errorf("Expected report to include the pid 4 ping") + } + if cmdline, ok := node.Latest.Lookup(process.Cmdline); !ok || cmdline != fmt.Sprint(processes[3].Cmdline) { + t.Errorf("Expected %q got %q", processes[3].Cmdline, cmdline) + } + } + testReporter(t, false, test) +} + +func TestAnonymous(t *testing.T) { + test := func(rpt report.Report) { + node, ok := rpt.Process.Nodes[report.MakeProcessNodeID("", "5")] + if !ok { + t.Errorf("Expected report to include the pid 5 tail") + } + if name, ok := node.Latest.Lookup(process.Name); ok { + t.Errorf("Expected no name, but got %q", name) + } + if cmdline, ok := node.Latest.Lookup(process.Cmdline); !ok || cmdline != fmt.Sprint(processes[4].Cmdline) { + t.Errorf("Expected %q got %q", processes[4].Cmdline, cmdline) + } + } + testReporter(t, false, test) +} + +func TestCmdlineRemoval(t *testing.T) { + test := func(rpt report.Report) { + node, ok := rpt.Process.Nodes[report.MakeProcessNodeID("", "4")] + if !ok { + t.Errorf("Expected report to include the pid 4 ping") + } + if cmdline, ok := node.Latest.Lookup(process.Cmdline); !ok || cmdline != fmt.Sprint("ping") { + t.Errorf("Expected %q got %q", "ping", cmdline) + } + } + testReporter(t, true, test) } diff --git a/prog/main.go b/prog/main.go index 1bbafee25..dfc012a71 100644 --- a/prog/main.go +++ b/prog/main.go @@ -80,17 +80,19 @@ type flags struct { } type probeFlags struct { - token string - httpListen string - publishInterval time.Duration - spyInterval time.Duration - pluginsRoot string - insecure bool - logPrefix string - logLevel string - resolver string - noApp bool - noControls bool + token string + httpListen string + publishInterval time.Duration + spyInterval time.Duration + pluginsRoot string + insecure bool + logPrefix string + logLevel string + resolver string + noApp bool + noControls bool + noCommandLineArguments bool + noEnvironmentVariables bool useConntrack bool // Use conntrack for endpoint topo conntrackBufferSize int // Sie of kernel buffer for conntrack @@ -266,6 +268,8 @@ func main() { 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 (e.g. start/stop containers, terminals, logs ...)") + flag.BoolVar(&flags.probe.noCommandLineArguments, "probe.omit.cmd-args", false, "Disable collection of command-line arguments") + flag.BoolVar(&flags.probe.noEnvironmentVariables, "probe.omit.env-vars", false, "Disable collection of environment variables") 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 50fee324d..8ff1ee6d4 100644 --- a/prog/probe.go +++ b/prog/probe.go @@ -163,7 +163,7 @@ func probeMain(flags probeFlags, targets []appclient.Target) { processCache = process.NewCachingWalker(process.NewWalker(flags.procRoot)) scanner = procspy.NewConnectionScanner(processCache) p.AddTicker(processCache) - p.AddReporter(process.NewReporter(processCache, hostID, process.GetDeltaTotalJiffies)) + p.AddReporter(process.NewReporter(processCache, hostID, process.GetDeltaTotalJiffies, flags.noCommandLineArguments)) } dnsSnooper, err := endpoint.NewDNSSnooper() @@ -195,7 +195,17 @@ func probeMain(flags probeFlags, targets []appclient.Target) { log.Errorf("Docker: problem with bridge %s: %v", flags.dockerBridge, err) } } - if registry, err := docker.NewRegistry(flags.dockerInterval, clients, true, hostID, handlerRegistry, dockerEndpoint); err == nil { + options := docker.RegistryOptions{ + Interval: flags.dockerInterval, + Pipes: clients, + CollectStats: true, + HostID: hostID, + HandlerRegistry: handlerRegistry, + DockerEndpoint: dockerEndpoint, + NoCommandLineArguments: flags.noCommandLineArguments, + NoEnvironmentVariables: flags.noEnvironmentVariables, + } + if registry, err := docker.NewRegistry(options); err == nil { defer registry.Stop() if flags.procEnabled { p.AddTagger(docker.NewTagger(registry, processCache))