Merge pull request #2310 from weaveworks/revert-2306-wip-2222

Revert "Add options to hide args and env vars (#2306)"

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
 }
 

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;<container>", 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")
-		}
-	})
-}

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{} {

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)

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}
 	}
 

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,21 +85,14 @@ 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 != "" {
 				node = node.WithLatests(map[string]string{tuple.key: tuple.value})
 			}
 		}
 
-		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)})
 		}

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)
-	}
 }