From dcc7389127d92717ea93d7f5e05e208a2354ef13 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Tue, 7 Mar 2017 17:51:27 +0100 Subject: [PATCH] Revert "Add options to hide args and env vars (#2306)" This reverts commit 764afb6301f2715c5d1814d7bc773d40b42610f9. --- probe/docker/container.go | 38 +++++++++------------------- probe/docker/container_test.go | 42 ++----------------------------- probe/docker/controls_test.go | 10 ++------ probe/docker/registry.go | 45 ++++++++++++---------------------- probe/docker/registry_test.go | 15 ++---------- probe/process/reporter.go | 26 ++++++-------------- probe/process/reporter_test.go | 15 +----------- prog/main.go | 26 +++++++++----------- prog/probe.go | 14 ++--------- 9 files changed, 55 insertions(+), 176 deletions(-) diff --git a/probe/docker/container.go b/probe/docker/container.go index 19973bcf3..419fb8605 100644 --- a/probe/docker/container.go +++ b/probe/docker/container.go @@ -98,24 +98,20 @@ 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 - noCommandLineArguments bool - noEnvironmentVariables bool + container *docker.Container + stopStats chan<- bool + latestStats docker.Stats + pendingStats [60]docker.Stats + numPending int + hostID string + baseNode report.Node } // NewContainer creates a new Container -func NewContainer(c *docker.Container, hostID string, noCommandLineArguments bool, noEnvironmentVariables bool) Container { +func NewContainer(c *docker.Container, hostID string) Container { result := &container{ - container: c, - hostID: hostID, - noCommandLineArguments: noCommandLineArguments, - noEnvironmentVariables: noEnvironmentVariables, + container: c, + hostID: hostID, } result.baseNode = result.getBaseNode() return result @@ -373,28 +369,18 @@ 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.getSanitizedCommand(), + ContainerCommand: c.container.Path + " " + strings.Join(c.container.Args, " "), 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) - if !c.noEnvironmentVariables { - result = result.AddPrefixPropertyList(EnvPrefix, c.env()) - } + result = result.AddPrefixPropertyList(EnvPrefix, c.env()) return result } diff --git a/probe/docker/container_test.go b/probe/docker/container_test.go index b042c70c9..1098e13c7 100644 --- a/probe/docker/container_test.go +++ b/probe/docker/container_test.go @@ -2,7 +2,6 @@ package docker_test import ( "net" - "strings" "testing" "time" @@ -41,7 +40,7 @@ func TestContainer(t *testing.T) { defer mtime.NowReset() const hostID = "scope" - c := docker.NewContainer(container1, hostID, false, false) + c := docker.NewContainer(container1, hostID) s := newMockStatsGatherer() err := c.StartGatheringStats(s) if err != nil { @@ -70,7 +69,7 @@ func TestContainer(t *testing.T) { docker.RemoveContainer: {Dead: true}, } want := report.MakeNodeWith("ping;", map[string]string{ - "docker_container_command": "ping foo.bar.local", + "docker_container_command": " ", "docker_container_created": "0001-01-01T00:00:00Z", "docker_container_id": "ping", "docker_container_name": "pong", @@ -80,7 +79,6 @@ 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{ @@ -127,39 +125,3 @@ 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 2a3aafe1b..855ea326d 100644 --- a/probe/docker/controls_test.go +++ b/probe/docker/controls_test.go @@ -18,10 +18,7 @@ func TestControls(t *testing.T) { mdc := newMockClient() setupStubs(mdc, func() { hr := controls.NewDefaultHandlerRegistry() - registry, _ := docker.NewRegistry(docker.RegistryOptions{ - Interval: 10 * time.Second, - HandlerRegistry: hr, - }) + registry, _ := docker.NewRegistry(10*time.Second, nil, false, "", hr, "") defer registry.Stop() for _, tc := range []struct{ command, result string }{ @@ -62,10 +59,7 @@ func TestPipes(t *testing.T) { mdc := newMockClient() setupStubs(mdc, func() { hr := controls.NewDefaultHandlerRegistry() - registry, _ := docker.NewRegistry(docker.RegistryOptions{ - Interval: 10 * time.Second, - HandlerRegistry: hr, - }) + registry, _ := docker.NewRegistry(10*time.Second, nil, false, "", 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 1292d8c2e..2fd3bcef8 100644 --- a/probe/docker/registry.go +++ b/probe/docker/registry.go @@ -51,15 +51,13 @@ 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 - noCommandLineArguments bool - noEnvironmentVariables bool + quit chan chan struct{} + interval time.Duration + collectStats bool + client Client + pipes controls.PipeClient + hostID string + handlerRegistry *controls.HandlerRegistry watchers []ContainerUpdateWatcher containers *radix.Tree @@ -95,20 +93,9 @@ func newDockerClient(endpoint string) (Client, error) { return docker_client.NewClient(endpoint) } -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(options RegistryOptions) (Registry, error) { - client, err := NewDockerClientStub(options.DockerEndpoint) +func NewRegistry(interval time.Duration, pipes controls.PipeClient, collectStats bool, hostID string, handlerRegistry *controls.HandlerRegistry, dockerEndpoint string) (Registry, error) { + client, err := NewDockerClientStub(dockerEndpoint) if err != nil { return nil, err } @@ -120,14 +107,12 @@ func NewRegistry(options RegistryOptions) (Registry, error) { pipeIDToexecID: map[string]string{}, client: client, - pipes: options.Pipes, - interval: options.Interval, - collectStats: options.CollectStats, - hostID: options.HostID, - handlerRegistry: options.HandlerRegistry, + pipes: pipes, + interval: interval, + collectStats: collectStats, + hostID: hostID, + handlerRegistry: handlerRegistry, quit: make(chan chan struct{}), - noCommandLineArguments: options.NoCommandLineArguments, - noEnvironmentVariables: options.NoEnvironmentVariables, } r.registerControls() @@ -354,7 +339,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, r.noCommandLineArguments, r.noEnvironmentVariables) + c = NewContainerStub(dockerContainer, r.hostID) r.containers.Insert(containerID, c) } else { c = o.(Container) diff --git a/probe/docker/registry_test.go b/probe/docker/registry_test.go index 0fd6bcaa2..2f110b5e7 100644 --- a/probe/docker/registry_test.go +++ b/probe/docker/registry_test.go @@ -22,11 +22,7 @@ import ( func testRegistry() docker.Registry { hr := controls.NewDefaultHandlerRegistry() - registry, _ := docker.NewRegistry(docker.RegistryOptions{ - Interval: 10 * time.Second, - CollectStats: true, - HandlerRegistry: hr, - }) + registry, _ := docker.NewRegistry(10*time.Second, nil, true, "", hr, "") return registry } @@ -207,10 +203,6 @@ var ( ID: "ping", Name: "pong", Image: "baz", - Path: "ping", - Args: []string{ - "foo.bar.local", - }, State: client.State{ Pid: 2, Running: true, @@ -234,9 +226,6 @@ var ( }, }, Config: &client.Config{ - Env: []string{ - "FOO=secret-bar", - }, Labels: map[string]string{ "foo1": "bar1", "foo2": "bar2", @@ -305,7 +294,7 @@ func setupStubs(mdc *mockDockerClient, f func()) { return mdc, nil } - docker.NewContainerStub = func(c *client.Container, _ string, _ bool, _ bool) docker.Container { + docker.NewContainerStub = func(c *client.Container, _ string) docker.Container { return &mockContainer{c} } diff --git a/probe/process/reporter.go b/probe/process/reporter.go index a84267a86..5dcbfcfd9 100644 --- a/probe/process/reporter.go +++ b/probe/process/reporter.go @@ -2,7 +2,6 @@ package process import ( "strconv" - "strings" "github.com/weaveworks/common/mtime" "github.com/weaveworks/scope/report" @@ -38,22 +37,20 @@ var ( // Reporter generates Reports containing the Process topology. type Reporter struct { - scope string - walker Walker - jiffies Jiffies - noCommandLineArguments bool + scope string + walker Walker + jiffies Jiffies } // 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, noCommandLineArguments bool) *Reporter { +func NewReporter(walker Walker, scope string, jiffies Jiffies) *Reporter { return &Reporter{ - scope: scope, - walker: walker, - jiffies: jiffies, - noCommandLineArguments: noCommandLineArguments, + scope: scope, + walker: walker, + jiffies: jiffies, } } @@ -88,6 +85,7 @@ 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 != "" { @@ -95,14 +93,6 @@ 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 dd583fc48..d4bfe2a31 100644 --- a/probe/process/reporter_test.go +++ b/probe/process/reporter_test.go @@ -35,7 +35,7 @@ func TestReporter(t *testing.T) { mtime.NowForce(now) defer mtime.NowReset() - rpt, err := process.NewReporter(walker, "", getDeltaTotalJiffies, false).Report() + rpt, err := process.NewReporter(walker, "", getDeltaTotalJiffies).Report() if err != nil { t.Error(err) } @@ -97,17 +97,4 @@ func TestReporter(t *testing.T) { 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) } - - // It doesn't report Cmdline args when asked not to - rpt, err = process.NewReporter(walker, "", getDeltaTotalJiffies, true).Report() - if err != nil { - t.Error(err) - } - 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) - } } diff --git a/prog/main.go b/prog/main.go index dfc012a71..1bbafee25 100644 --- a/prog/main.go +++ b/prog/main.go @@ -80,19 +80,17 @@ 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 - noCommandLineArguments bool - noEnvironmentVariables 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 useConntrack bool // Use conntrack for endpoint topo conntrackBufferSize int // Sie of kernel buffer for conntrack @@ -268,8 +266,6 @@ 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 8ff1ee6d4..50fee324d 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, flags.noCommandLineArguments)) + p.AddReporter(process.NewReporter(processCache, hostID, process.GetDeltaTotalJiffies)) } dnsSnooper, err := endpoint.NewDNSSnooper() @@ -195,17 +195,7 @@ func probeMain(flags probeFlags, targets []appclient.Target) { log.Errorf("Docker: problem with bridge %s: %v", flags.dockerBridge, err) } } - 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 { + if registry, err := docker.NewRegistry(flags.dockerInterval, clients, true, hostID, handlerRegistry, dockerEndpoint); err == nil { defer registry.Stop() if flags.procEnabled { p.AddTagger(docker.NewTagger(registry, processCache))