Quote INFLUXDB_URLS in Telegraf config

[gedimin45]

Just installed Workflow 2.9 with an off-cluster InfluxDB and Telegraf won't start because of a parse error in config.toml on line 17. The contents are:

# Set Tag Configuration
[tags]
# Set Agent Configuration
[agent]
  interval = "10s"
  round_interval = true
  metric_buffer_limit = 100000
  collection_jitter = "1s"
  flush_interval = "1s"
  flush_jitter = "0s"
  debug = false
  quiet = true
  flush_buffer_when_full = true
  hostname = "..." 
# Set output configuration
[[outputs.influxdb]]
  urls = [10.15.0.1]
  database = "deis"
  precision = "ns"
  timeout = "5s"
   username = "deis" 
   password = "..." 
#...

When configured InfluxDB as off-cluster, its URL is mounted from a secret and so it does not have quotes around it. Thus, a the quote filter is needed. I am not sure, though, whether this breaks the case when InfluxDB is on-cluster as then the URL seems to be quoted.

[deis-admin]

Thanks for the contribution! Please ensure your commits follow our style guide. This code will be tested once a Deis maintainer reviews it.

[deis-bot]

@jchauncey and @sstarcher are potential reviewers of this pull request based on my analysis of git blame information. Thanks @ged15!

[mboersma]

Jenkins, add to whitelist.

[mboersma]

@jchauncey I recall some past changes around quoting things in this template file. This looks good to me, but could you review it when you have time?

[jchauncey]

urls = [{{ .INFLUXDB_URLS | quote }}]

The reason this is dangerous is because someone could specify multiple influx urls here and | quote would attempt to quote that entire string instead of the individual comma separate list.

This value expects urls = ["my.host.com", "my.host.bar"] but this would give you urls = ["my.host.com,my.host.bar"]

It's why I moved away from quoting the values directly within the template file and instead forced the user to do it in the chart.

This change would also mean that we are not consistent with how we handle urls within the monitor repo.

[mboersma]

@ged15, I poked at this and found an uglier templating change that seems to create valid TOML:

[[outputs.influxdb]]
  urls = [{{ $urls := .INFLUXDB_URLS | split "," }}{{ range $urls }}{{ . | quote }}, {{ end }}]

Tested this way:

$ INFLUXDB_URLS=http://www.test.com/ envtpl -in config.toml.tpl | grep urls
  urls = ["http://www.test.com/", ]
$ INFLUXDB_URLS=http://www.test.com/,http://www.test2.com/ envtpl -in config.toml.tpl | grep urls
  urls = ["http://www.test.com/", "http://www.test2.com/", ]

It's why I moved away from quoting the values directly within the template file and instead forced the user to do it in the chart.

Or perhaps that would work, to have the explicitly quoted, comma-separated string in your chart.

[gedimin45]

The approach in the PR is not good then probably. BTW, found another problem with quoting: https://github.com/deis/monitor/blob/master/grafana/rootfs/usr/share/grafana/start-grafana#L61 When the INFLUX_SERVICE_URL is defined and has quotes around it, the JSON produced becomes invalid:

{
  // ...
  "url": ""http://10.132.15.10:8086"",
  // ...
}

[bacongobbler]

gonna close this as "working as intended", since the expected value is a list as mentioned in https://github.com/deis/monitor/pull/164#issuecomment-266084903.