1
0
Fork 0
mirror of https://github.com/moby/moby.git synced 2022-11-09 12:21:53 -05:00

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>
This commit is contained in:
Yong Tang 2016-11-12 16:44:51 -08:00
parent 77fca662dd
commit 312cc7eebd
2 changed files with 66 additions and 17 deletions

View file

@ -59,20 +59,18 @@ func newListCommand(dockerCli *command.DockerCli) *cobra.Command {
return &cmd return &cmd
} }
type preProcessor struct { // listOptionsProcessor is used to set any container list options which may only
types.Container // be embedded in the format template.
opts *types.ContainerListOptions // 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 // Size sets the size of the map when called by a template execution.
// These are needed since they are available on the final object but are not func (o listOptionsProcessor) Size() bool {
// fields in types.Container o["size"] = true
// 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
return true return true
} }
@ -88,20 +86,20 @@ func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, er
options.Limit = 1 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) tmpl, err := templates.Parse(opts.format)
if err != nil { if err != nil {
return nil, err return nil, err
} }
optionsProcessor := listOptionsProcessor{}
// This shouldn't error out but swallowing the error makes it harder // This shouldn't error out but swallowing the error makes it harder
// to track down if preProcessor issues come up. Ref #24696 // 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 return nil, err
} }
// At the moment all we need is to capture .Size for preprocessor
options.Size = opts.size || optionsProcessor["size"]
return options, nil return options, nil
} }

View file

@ -46,6 +46,57 @@ func TestBuildContainerListOptions(t *testing.T) {
expectedLimit: 1, expectedLimit: 1,
expectedFilters: make(map[string]string), 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 { for _, c := range contexts {