1. docker
  2. Commit 3e9dd556954a23d299aa64bcbb789e3782fab186
Browse code

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>
(cherry picked from commit 312cc7eebd326cc500e5742115d1e92e94ff60ee)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>

Yong Tang authored on 2016/11/13 09:44:51
Showing 2 changed files
cli/command/container/list.go
History View file @ 3e9dd55
... ... 
@@ -59,20 +59,18 @@ func newListCommand(dockerCli *command.DockerCli) *cobra.Command {
59 59 
 	return &cmd
60 60 
 }
61 61
62 
-type preProcessor struct {
63 
-	types.Container
64 
-	opts *types.ContainerListOptions
65 
-
66 
-	// Fields that need to exist so the template doesn't error out
67 
-	// These are needed since they are available on the final object but are not
68 
-	// fields in types.Container
69 
-	// TODO(cpuguy83): this seems rather broken
70 
-	Networks, CreatedAt, RunningFor bool
71 
-}
72 
-
73 
-// Size sets the size option when called by a template execution.
74 
-func (p *preProcessor) Size() bool {
75 
-	p.opts.Size = true
62 
+// listOptionsProcessor is used to set any container list options which may only
63 
+// be embedded in the format template.
64 
+// This is passed directly into tmpl.Execute in order to allow the preprocessor
65 
+// to set any list options that were not provided by flags (e.g. `.Size`).
66 
+// It is using a `map[string]bool` so that unknown fields passed into the
67 
+// template format do not cause errors. These errors will get picked up when
68 
+// running through the actual template processor.
69 
+type listOptionsProcessor map[string]bool
70 
+
71 
+// Size sets the size of the map when called by a template execution.
72 
+func (o listOptionsProcessor) Size() bool {
73 
+	o["size"] = true
76 74 
 	return true
77 75 
 }
78 76
... ... 
@@ -88,20 +86,20 @@ func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, er
88 88 
 		options.Limit = 1
89 89 
 	}
90 90
91 
-	// Currently only used with Size, so we can determine if the user
92 
-	// put {{.Size}} in their format.
93 
-	pre := &preProcessor{opts: options}
94 91 
 	tmpl, err := templates.Parse(opts.format)
95 92
96 93 
 	if err != nil {
97 94 
 		return nil, err
98 95 
 	}
99 96
97 
+	optionsProcessor := listOptionsProcessor{}
100 98 
 	// This shouldn't error out but swallowing the error makes it harder
101 99 
 	// to track down if preProcessor issues come up. Ref #24696
102 
-	if err := tmpl.Execute(ioutil.Discard, pre); err != nil {
100 
+	if err := tmpl.Execute(ioutil.Discard, optionsProcessor); err != nil {
103 101 
 		return nil, err
104 102 
 	}
103 
+	// At the moment all we need is to capture .Size for preprocessor
104 
+	options.Size = opts.size || optionsProcessor["size"]
105 105
106 106 
 	return options, nil
107 107 
 }
cli/command/container/ps_test.go
History View file @ 3e9dd55
... ... 
@@ -46,6 +46,57 @@ func TestBuildContainerListOptions(t *testing.T) {
46 46 
 			expectedLimit:   1,
47 47 
 			expectedFilters: make(map[string]string),
48 48 
 		},
49 
+		{
50 
+			psOpts: &psOptions{
51 
+				all:    true,
52 
+				size:   false,
53 
+				last:   5,
54 
+				filter: filters,
55 
+				// With .Size, size should be true
56 
+				format: "{{.Size}}",
57 
+			},
58 
+			expectedAll:   true,
59 
+			expectedSize:  true,
60 
+			expectedLimit: 5,
61 
+			expectedFilters: map[string]string{
62 
+				"foo": "bar",
63 
+				"baz": "foo",
64 
+			},
65 
+		},
66 
+		{
67 
+			psOpts: &psOptions{
68 
+				all:    true,
69 
+				size:   false,
70 
+				last:   5,
71 
+				filter: filters,
72 
+				// With .Size, size should be true
73 
+				format: "{{.Size}} {{.CreatedAt}} {{.Networks}}",
74 
+			},
75 
+			expectedAll:   true,
76 
+			expectedSize:  true,
77 
+			expectedLimit: 5,
78 
+			expectedFilters: map[string]string{
79 
+				"foo": "bar",
80 
+				"baz": "foo",
81 
+			},
82 
+		},
83 
+		{
84 
+			psOpts: &psOptions{
85 
+				all:    true,
86 
+				size:   false,
87 
+				last:   5,
88 
+				filter: filters,
89 
+				// Without .Size, size should be false
90 
+				format: "{{.CreatedAt}} {{.Networks}}",
91 
+			},
92 
+			expectedAll:   true,
93 
+			expectedSize:  false,
94 
+			expectedLimit: 5,
95 
+			expectedFilters: map[string]string{
96 
+				"foo": "bar",
97 
+				"baz": "foo",
98 
+			},
99 
+		},
49 100 
 	}
50 101
51 102 
 	for _, c := range contexts {