Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add settable nginx daemon group #1126

Merged
merged 2 commits into from
Oct 24, 2017
Merged

add settable nginx daemon group #1126

merged 2 commits into from
Oct 24, 2017

Conversation

miksercz
Copy link
Contributor

Hello,
in a problem I was trying to solve with your module today, I needed to specify both user and group name for the nginx.conf name descriptor, as these were not the same. Simply setting the daemon_user param to $user $group doesn't work, because it is used below in config.pp as an owner for multiple directories:

file { $log_dir:
  ensure => directory,
  mode   => $log_mode,
  owner  => $daemon_user,
  group  => $log_group,
}

Therefore I believe I had to add this parameter, which fixes this issue nicely.

@wyardley
Copy link
Collaborator

Thanks, we had #647 which was going to implement this, however it never ended up getting rebased or updated, and we closed it.

This looks like the right fix to me, but it needs spec tests, please let us know if you need help figuring out how to add tests for this.

https://github.com/voxpupuli/puppet-nginx/blob/master/spec/classes/nginx_spec.rb#L341-L350
https://github.com/voxpupuli/puppet-nginx/blob/master/spec/classes/nginx_spec.rb#L1069-L1074
etc.... probably you'd want to actually test the behavior in that template, e.g., that the group is present in the generated config when specified, and suppressed when not.

@miksercz
Copy link
Contributor Author

Sorry for the long wait - I added two tests with help from people on slack, let me know if you want to test any additional cases.

@wyardley wyardley merged commit 9aeba87 into voxpupuli:master Oct 24, 2017
@wyardley
Copy link
Collaborator

Thanks for the follow-through @miksercz

cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Sep 13, 2019
add settable nginx daemon group
Rubueno pushed a commit to Rubueno/puppet-nginx that referenced this pull request Oct 19, 2020
add settable nginx daemon group
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants