Empty condition in configuration crashes beats

[adriansr]

When a condition from the configuration has no sub-fields, NewCondition will return a nil condition. However, this nil condition will result in a panic when any of its methods is invoked.

• Version: 6.2.4
• Operating System: N/A
• Discuss Forum URL: https://discuss.elastic.co/t/filebeats-autodiscover-segmentation-violation/133342
• Steps to Reproduce:

Use this broken config from the discuss thread above (note that the equals clause is wrongly indented at the same level as condition):

filebeat.autodiscover:
  providers:
    - type: docker
      templates:
        - condition:
          equals:
            docker.container.image: nginx
          config:
            - module: nginx
              access:
                prospector:
                  type: docker
                  container.ids:
                    - "${data.docker.container.id}"
              error:
                prospector:
                  type: docker
                  container.ids:
                    - "${data.docker.container.id}"

will result in a panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0xebfea6]
goroutine 58 [running]:
github.com/elastic/beats/libbeat/processors.(*Condition).Check(0x0, 0x1f36d60, 0xc42007d5c0, 0x0)
	/go/src/github.com/elastic/beats/libbeat/processors/condition.go:205 +0x26
github.com/elastic/beats/libbeat/autodiscover/template.(*Mapper).GetConfig(0xc42049c380, 0xc42007d5c0, 0xc4204bc528, 0xac369f, 0x10)
	/go/src/github.com/elastic/beats/libbeat/autodiscover/template/config.go:62 +0x63e
github.com/elastic/beats/libbeat/autodiscover/providers/docker.(*Provider).publish(0xc4205340c0, 0xc42007d5c0)
	/go/src/github.com/elastic/beats/libbeat/autodiscover/providers/docker/docker.go:137 +0x3d
github.com/elastic/beats/libbeat/autodiscover/providers/docker.(*Provider).emitContainer(0xc4205340c0, 0xc42015a240, 0x16c6c00, 0x5)
	/go/src/github.com/elastic/beats/libbeat/autodiscover/providers/docker/docker.go:110 +0x78a
github.com/elastic/beats/libbeat/autodiscover/providers/docker.(*Provider).Start.func1(0xc4205340c0)
	/go/src/github.com/elastic/beats/libbeat/autodiscover/providers/docker/docker.go:74 +0x1e9
created by github.com/elastic/beats/libbeat/autodiscover/providers/docker.(*Provider).Start
	/go/src/github.com/elastic/beats/libbeat/autodiscover/providers/docker/docker.go:65 +0x3f

Apart from NewCondition always returning an usable condition, we should try to warn the user or err on this configuration mistake

[ph]

@adriansr oops, I will create a PR for that.

[ph]

Just a note here, using an empty condition in the processorpart of the config will not cause any errors and the messages returned to the users are meaningful, I don't know too much about the autodiscover part of the code but I will take a look.

processors:
- condition:

2018-05-28T15:45:11.352-0400    ERROR   instance/beat.go:667    Exiting: error initializing publisher: error initializing processors: the processor condition doesn't exist
Exiting: error initializing publisher: error initializing processors: the processor condition doesn't exist

[exekias]

I can have a look if you want @ph

[adriansr]

Thanks for investigating @ph @exekias. Feel free to update the description to make it more meaningful

[ph]

@exekias I think it might be better you take a look, the thing that worries me as @adriansr stated is the NewCondition constructor should never return an invalid condition, by looking a the code, the following should not be there:

beats/libbeat/processors/condition.go

Lines 69 to 72 in 7abc684

	if config == nil {
	// empty condition
	return nil, nil
	}

By looking at the code path this seems to be the only possible way to return a nil condition, but by changing to return an error instead it breaks some unit test in the autodiscover module.