Adding Varnish sudo rules for unprivileged users #3097

bstromski · 2017-08-07T18:40:05Z

Required for all PRs:

Signed CLA.
Associated README.md updated.

Varnishstat requires read access to the underlying varnish libs at /var/lib/varnish. Since some users don't run telegraf as root we either need to setfacls on this directory or enable sudo to execute this command (default is false).

Looking back in history a bit, it seems sudo is the preferred method:
https://www.varnish-cache.org/lists/pipermail/varnish-misc/2011-November/021426.html

sudo = false

$ telegraf --config /etc/telegraf/telegraf.conf --input-filter varnish --test
* Plugin: inputs.varnish, Collection 1
2017-08-07T18:33:17Z E! error gathering metrics: error running varnishstat: Command timed out.

sudo = true

$ telegraf --config /etc/telegraf/telegraf.conf --input-filter varnish --test
* Plugin: inputs.varnish, Collection 1
> varnish,section=MAIN backend_req=61925i,cache_hit=521i,cache_miss=1120i 1502130756000000000

danielnelson · 2017-08-07T18:57:06Z

What about adding the telegraf user to the varnish group? This would be preferable to using sudo, so if this works then we should just add something to the documentation suggesting it.

bstromski · 2017-08-07T19:17:59Z

Since some of these files are owned by root, changing the group wouldn't help much (unless a chown was done). facl's do work, but managing extended acl's recursively on any of our varnish boxes seems like a bit of a hack compared to granting sudo rights to telegraf. I'm actually a bit surprised other users haven't run into this issue before.

# ll /var/lib/varnish
total 4
drwxr-xr-x 5 root root 4096 Aug  3 13:03 hostname

# ll /var/lib/varnish/*/
total 82956
drwxr-xr-x 2 varnish varnish     4096 Jun 16 17:58 vcl_boot.1497653924.215858936
drwxr-xr-x 2 varnish varnish     4096 Mar 14 10:49 vcl_reload_2017-03-14T10:49:05
drwxr-xr-x 2 varnish varnish     4096 Apr  5 10:49 vcl_reload_2017-04-05T10:49:49
-rw-r----- 1 root    root    84934656 Aug  7 14:12 _.vsm

I noticed that one of the varnish unit tests failed. I'm looking into whats going on there now and will update once that's sorted.

danielnelson · 2017-08-07T20:41:06Z

Here is my varnish directory on debian stretch. This a clean install, I haven't configured anything, but all the files you have that are owned by root are using the varnish group on my system:

$ ll -R /var/lib/varnish/
/var/lib/varnish/:
total 4
drwxr-xr-x 3 varnish varnish 4096 Aug  7 11:53 debian-stretch

/var/lib/varnish/debian-stretch:
total 82948
drwxr-xr-x 2 varnish varnish     4096 Aug  7 11:53 vcl_boot.1502132009.746212482
-rw-r----- 1 root    varnish 84934656 Aug  7 13:40 _.vsm

/var/lib/varnish/debian-stretch/vcl_boot.1502132009.746212482:
total 40
-rwxr-x--- 1 varnish varnish 40752 Aug  7 11:53 vgc.so

bstromski · 2017-08-07T21:11:00Z

Ok, this could be a RHEL or package version thing. Just installed the base 4.1.3-1.el7 rpm on my RHEL7 desktop and I'm still seeing group ownership as root.

Is there any harm in making this sudo option available in the telegraf config? The code sets the flag as off by default, so it will only be executed if a user passes the flag. Personally, I would rather have the telegraf user execute this via sudo then altering groups or facl's.

Thanks,
Ben

danielnelson · 2017-08-07T21:42:31Z

plugins/inputs/varnish/varnish.go

@@ -32,6 +33,9 @@ var defaultStats = []string{"MAIN.cache_hit", "MAIN.cache_miss", "MAIN.uptime"}
 var defaultBinary = "/usr/bin/varnishstat"

 var sampleConfig = `
+  ## If running as a restricted user you can prepend sudo for additional access:
+	sudo = true


Lets call it use_sudo to match the fail2ban plugin. Indent the line only 2 spaces, set it to false (since that's default), and comment it out with a single hash (because it is not required).

## If running as a restricted user you can prepend sudo for additional access: # sudo = false

In the plugin README we should add a paragraph that discusses permissions and recommends adding the telegraf user to the varnish group over using sudo. It should show how to give the telegraf user password-less sudo access to the varnishstat binary.

danielnelson · 2017-08-07T21:43:41Z

plugins/inputs/varnish/varnish.go

+
+	if useSudo {
+		cmdArgs = append([]string{cmdName}, cmdArgs...)
+		cmd = exec.Command("/bin/sudo", cmdArgs...)


Use the sudo found in path, on my system sudo is /usr/bin/sudo, also I think we ought to run sudo with the -n flag.

bstromski · 2017-08-08T18:51:27Z

Thanks @danielnelson, updated per your request! Let me know if there is anything else you want updated.

danielnelson · 2017-08-08T20:07:19Z

plugins/inputs/varnish/README.md

+[[inputs.varnish]]
+  use_sudo = true
+
+$ echo "telegraf ALL = NOPASSWD: /usr/bin/varnishstat" >>/etc/sudoers


We should recommend using visudo to edit the sudoers file, also minor thing but I'd prefer the runas spec:

telegraf ALL=(ALL) NOPASSWD: /usr/bin/varnishstat

danielnelson · 2017-08-08T20:08:35Z

plugins/inputs/varnish/README.md

+It's important to note that this plugin references varnishstat, which may require additional permissions to execute successfully.
+Depending on the user/group permissions of the telegraf user executing this plugin, you may need to alter the group membership, set facls, or use sudo.
+
+**Group membership**:


I think we should recommend this method.

danielnelson · 2017-08-08T20:09:00Z

plugins/inputs/varnish/README.md

@@ -7,6 +7,9 @@ This plugin gathers stats from [Varnish HTTP Cache](https://varnish-cache.org/)
 ```toml
 # A plugin to collect stats from Varnish HTTP Cache
 [[inputs.varnish]]
+   ## If running as a restricted user you can prepend sudo for additional access:
+	 #use_sudo = false


Indention still needs worked out here, 2 spaces

danielnelson · 2017-08-08T20:10:09Z

plugins/inputs/varnish/varnish.go

@@ -32,6 +33,9 @@ var defaultStats = []string{"MAIN.cache_hit", "MAIN.cache_miss", "MAIN.uptime"}
 var defaultBinary = "/usr/bin/varnishstat"

 var sampleConfig = `
+  ## If running as a restricted user you can prepend sudo for additional access:
+	#use_sudo = false


Also need indention here, 2 spaces. Test with telegraf --usage varnish

bstromski · 2017-08-08T21:15:10Z

@danielnelson Updated with your feedback and fixed the spacing issue

$ telegraf --usage=varnish

# A plugin to collect stats from Varnish HTTP Cache
[[inputs.varnish]]
  ## If running as a restricted user you can prepend sudo for additional access:
  #use_sudo = false

  ## The default location of the varnishstat binary can be overridden with:
  binary = "/usr/bin/varnishstat"

  ## By default, telegraf gather stats for 3 metric points.
  ## Setting stats will override the defaults shown below.
  ## Glob matching can be used, ie, stats = ["MAIN.*"]
  ## stats may also be set to ["*"], which will collect all stats
  stats = ["MAIN.cache_hit", "MAIN.cache_miss", "MAIN.uptime"]

bstromski · 2017-08-09T15:21:46Z

It looks like ci/circleci failed, but not on anything related to this PR:

ok  	github.com/influxdata/telegraf/plugins/inputs/varnish	1.100s

--- FAIL: TestMemcachedGeneratesMetrics (0.00s)
	Error Trace:	memcached_test.go:25
	Error:      	Received unexpected error:
	            	dial tcp 127.0.0.1:11211: getsockopt: connection refused
FAIL
FAIL	github.com/influxdata/telegraf/plugins/inputs/memcached	0.020s

danielnelson reviewed Aug 7, 2017

View reviewed changes

bstromski force-pushed the varnish_sudo branch from 0d22302 to 3c3c91c Compare August 8, 2017 18:47

danielnelson reviewed Aug 8, 2017

View reviewed changes

Adding Varnish sudo rules for unprivileged users

d96e99d

bstromski force-pushed the varnish_sudo branch from 3c3c91c to d96e99d Compare August 8, 2017 21:12

danielnelson added this to the 1.4.0 milestone Aug 9, 2017

danielnelson merged commit 8a2373e into influxdata:master Aug 9, 2017

maxunt pushed a commit that referenced this pull request Jun 26, 2018

Add option to run varnish under sudo (#3097)

83c9b8a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Varnish sudo rules for unprivileged users #3097

Adding Varnish sudo rules for unprivileged users #3097

bstromski commented Aug 7, 2017

danielnelson commented Aug 7, 2017

bstromski commented Aug 7, 2017

danielnelson commented Aug 7, 2017

bstromski commented Aug 7, 2017

danielnelson Aug 7, 2017

danielnelson Aug 7, 2017

bstromski commented Aug 8, 2017

danielnelson Aug 8, 2017

danielnelson Aug 8, 2017

danielnelson Aug 8, 2017

danielnelson Aug 8, 2017

bstromski commented Aug 8, 2017

bstromski commented Aug 9, 2017

Adding Varnish sudo rules for unprivileged users #3097

Adding Varnish sudo rules for unprivileged users #3097

Conversation

bstromski commented Aug 7, 2017

Required for all PRs:

danielnelson commented Aug 7, 2017

bstromski commented Aug 7, 2017

danielnelson commented Aug 7, 2017

bstromski commented Aug 7, 2017

danielnelson Aug 7, 2017

Choose a reason for hiding this comment

danielnelson Aug 7, 2017

Choose a reason for hiding this comment

bstromski commented Aug 8, 2017

danielnelson Aug 8, 2017

Choose a reason for hiding this comment

danielnelson Aug 8, 2017

Choose a reason for hiding this comment

danielnelson Aug 8, 2017

Choose a reason for hiding this comment

danielnelson Aug 8, 2017

Choose a reason for hiding this comment

bstromski commented Aug 8, 2017

bstromski commented Aug 9, 2017