The service_user and group option were being locally overriden in the service

[ericfode]

recipe and causing problems when being used with watch resources on centos.
the workaround was to set the user and group explicitly to root.

To fix it i moved the logic for determining the user to use out to the
attributes file.

[coveralls]

Coverage remained the same when pulling b310593 on ericfode:attribute-fix into 5a4e1a4 on johnbellone:master.

[ewr]

I ran into this yesterday on Ubuntu with a service definition and ended up manually just setting the attributes until I had more time to take a look at it.

Is there a reason the service doesn't run as a consul user on non-runit inits? If it runs as a non-root user there, it wouldn't seem like there's a consul-specific reason to do so. If not, perhaps a better fix would be to always run as the non-root user.

[johnbellone]

@ewr I'll have to take a look at it. The ideal would be that the agent runs as the consul role account irregardless of what flavor of Linux we're configuring.

[johnbellone]

A quick glance I don't see a reason why we can't simply default to running as the consul user and replace everything with node['consul']['user'] and nix the idea of a service_user.

/cc @reset

[johnbellone]

This is a breaking change.

[ericfode]

It may be worth keeping in mind the consul client does not need to be as privileged as the service user... Also it's a breaking change to do that... (Or do you mean that the proposed change is a breaking change?)

[reset]

@johnbellone @ericfode hmm that's weird, I actually don't know why we weren't just running as the Consul user the entire time.

[johnbellone]

It's a breaking change if we change it to always use the consul user. I don't think it'll affect anything, but we need to be careful.

[reset]

@johnbellone yeah, my preference is to cut a minor or patch release including a few resources to perform a migration for whatever it is that we're doing and then immediately afterwards bumping major or minor and remove that migration. If it's not possible we can always just bump the major or minor ver.

[johnbellone]

Err on the side of caution. The service will write files out as root. If the service is running we check owner and chown?

[johnbellone]

This is same as #96.

[donaldguy]

@johnbellone This does not work, for wrapper cookbooks at least. init_style isn't set to runit until after compile of the cookbook, by which the user and group are already hardcoded as root on both node and in recipe variables

I am (working on) working around by using a chef-sugar compile_time block, but the better option might be to do the conditional evaluation in a ruby_block resource at runtime and use lazy attributes, OR just go all the way to LWRP territory

[donaldguy]

yeah, no luck doing

compile_time do
  node.default['consul']['init_style'] = 'runit'
end

I think I'm just gonna have to manually (re)set service_user and service_group myself, but even that probably has to be done at compile time to propagate from a wrapper cookbook and I'm not 100% that will work either (will report back after testing)

We largely follow the patterns layed out by @reset in his chefconf talk last year (https://www.chef.io/blog/chefconf-talks/the-berkshelf-way-jamie-winsor/ ) ... so I guess I might just have to deviate from that slightly and stick it as an environment override... ? (neither Berkshelf nor Policyfiles seems to provide a place to persist that at the right place in the load order... we can start passing dna.json's around but I'd rather not)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.