search

jdauphant / ansible-role-nginx

Ansible role to install and manage nginx configuration
655 stars 302 forks source link

fix modules definition and add README section about this feature #232

Closed q2digger closed 6 years ago

q2digger commented 6 years ago

I have fixed modules definition (correct path in RHEL & CentOS) and add few words into README.md about the feature.

jdauphant commented 6 years ago

Thanks @q2digger

perryk commented 5 years ago

Hi, I've just been trying out this role and found at least a small typo, although possibly some further issues relating to this specific commit.

Firstly this line has "urs/share" which is likely meant to be "/usr/share"

https://github.com/jdauphant/ansible-role-nginx/blob/68633437e49f0e740f4dcae7754e025755ab89c2/tasks/configuration.yml#L65

Next, I'm unsure if this feature for loading modules is work in progress or how it worked previously, I personally however don't have a /usr/share/nginx path on Centos as I've needed to install Nginx using the nginx_official_repo option rather than from Epel. This installed a newer version plus allowed me to compile modsecurity v3 successfully.

This role is otherwise working well for me so far, thanks for putting it together ! :)

jdauphant commented 5 years ago

@perryk Well spot, it should not work to load module. We should have differents paths depends on the OS and options. Can you improve that ? ( the previous solution was maybe more universal )

perryk commented 5 years ago

Sure, I have taken a look and I have something mostly working. It is a little complicated by the fact that the epel supplied modules have config files already and the official Nginx repo supplied modules do not. I might just do a PR initially to simply fix the typo and restrict the code to only when using epel in addition to the current restriction for Centos/RHEL.

I can do another PR to change things back to more how they worked previously and account for both repos. The only thing might be if anyone is already using the current setup of having links from /usr/share to modules-enabled, they might need to remove those links so this can go back to having links in modules-available to modules-enabled.

Perhaps @q2digger will have some feedback on this also ?

jdauphant commented 5 years ago

@perryk If you fix the typo and restrict the code to only when we use epel, it will be perfect as a first step. Someone will improve to support more cases or if you get an idea to manage your case later you just have to do a new PR.

jdauphant commented 5 years ago

And thanks again for you help, you are helping the communauty on the way