From 312cc7eebd326cc500e5742115d1e92e94ff60ee Mon Sep 17 00:00:00 2001
From: Yong Tang <yong.tang.github@outlook.com>
Date: Sat, 12 Nov 2016 16:44:51 -0800
Subject: [PATCH] Use `map[string]bool` for `preProcessor` to ignore unknwon
 field

This fix is an attempt to address the issue raised in 28339. In
`docker ps`, the formatter needs to expose all fields of `types.Container`
to `preProcessor` so that template could be executed.

This direct exposing is unreliable and could cause issues as user may incorrectly
assume all fields in `types.Container` will be available for templating.

However, the purpose of `preProcessor` is to only find out if `.Size`
is defined (so that opts.size could be set accordingly).

This fix defines `preProcessor` as `map[string]bool` with a func `Size()`.
In this way, any unknown fields will be ignored.

This fix adds several test cases to the existing `TestBuildContainerListOptions`.

This fix fixes 28339.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
---
 cli/command/container/list.go    | 32 ++++++++++----------
 cli/command/container/ps_test.go | 51 ++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/cli/command/container/list.go b/cli/command/container/list.go
index b4cdfa2eb3..60c2462986 100644
--- a/cli/command/container/list.go
+++ b/cli/command/container/list.go
@@ -59,20 +59,18 @@ func newListCommand(dockerCli *command.DockerCli) *cobra.Command {
 	return &cmd
 }
 
-type preProcessor struct {
-	types.Container
-	opts *types.ContainerListOptions
+// listOptionsProcessor is used to set any container list options which may only
+// be embedded in the format template.
+// This is passed directly into tmpl.Execute in order to allow the preprocessor
+// to set any list options that were not provided by flags (e.g. `.Size`).
+// It is using a `map[string]bool` so that unknown fields passed into the
+// template format do not cause errors. These errors will get picked up when
+// running through the actual template processor.
+type listOptionsProcessor map[string]bool
 
-	// Fields that need to exist so the template doesn't error out
-	// These are needed since they are available on the final object but are not
-	// fields in types.Container
-	// TODO(cpuguy83): this seems rather broken
-	Networks, CreatedAt, RunningFor bool
-}
-
-// Size sets the size option when called by a template execution.
-func (p *preProcessor) Size() bool {
-	p.opts.Size = true
+// Size sets the size of the map when called by a template execution.
+func (o listOptionsProcessor) Size() bool {
+	o["size"] = true
 	return true
 }
 
@@ -88,20 +86,20 @@ func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, er
 		options.Limit = 1
 	}
 
-	// Currently only used with Size, so we can determine if the user
-	// put {{.Size}} in their format.
-	pre := &preProcessor{opts: options}
 	tmpl, err := templates.Parse(opts.format)
 
 	if err != nil {
 		return nil, err
 	}
 
+	optionsProcessor := listOptionsProcessor{}
 	// This shouldn't error out but swallowing the error makes it harder
 	// to track down if preProcessor issues come up. Ref #24696
-	if err := tmpl.Execute(ioutil.Discard, pre); err != nil {
+	if err := tmpl.Execute(ioutil.Discard, optionsProcessor); err != nil {
 		return nil, err
 	}
+	// At the moment all we need is to capture .Size for preprocessor
+	options.Size = opts.size || optionsProcessor["size"]
 
 	return options, nil
 }
diff --git a/cli/command/container/ps_test.go b/cli/command/container/ps_test.go
index 9df4dfd5fa..62b0545274 100644
--- a/cli/command/container/ps_test.go
+++ b/cli/command/container/ps_test.go
@@ -46,6 +46,57 @@ func TestBuildContainerListOptions(t *testing.T) {
 			expectedLimit:   1,
 			expectedFilters: make(map[string]string),
 		},
+		{
+			psOpts: &psOptions{
+				all:    true,
+				size:   false,
+				last:   5,
+				filter: filters,
+				// With .Size, size should be true
+				format: "{{.Size}}",
+			},
+			expectedAll:   true,
+			expectedSize:  true,
+			expectedLimit: 5,
+			expectedFilters: map[string]string{
+				"foo": "bar",
+				"baz": "foo",
+			},
+		},
+		{
+			psOpts: &psOptions{
+				all:    true,
+				size:   false,
+				last:   5,
+				filter: filters,
+				// With .Size, size should be true
+				format: "{{.Size}} {{.CreatedAt}} {{.Networks}}",
+			},
+			expectedAll:   true,
+			expectedSize:  true,
+			expectedLimit: 5,
+			expectedFilters: map[string]string{
+				"foo": "bar",
+				"baz": "foo",
+			},
+		},
+		{
+			psOpts: &psOptions{
+				all:    true,
+				size:   false,
+				last:   5,
+				filter: filters,
+				// Without .Size, size should be false
+				format: "{{.CreatedAt}} {{.Networks}}",
+			},
+			expectedAll:   true,
+			expectedSize:  false,
+			expectedLimit: 5,
+			expectedFilters: map[string]string{
+				"foo": "bar",
+				"baz": "foo",
+			},
+		},
 	}
 
 	for _, c := range contexts {