Skip to content
This repository has been archived by the owner on Aug 13, 2024. It is now read-only.

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

Merged
merged 1 commit into from
Oct 17, 2018

Conversation

q2digger
Copy link
Contributor

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

@jdauphant
Copy link
Owner

Thanks @q2digger

@jdauphant jdauphant merged commit 6863343 into jdauphant:master Oct 17, 2018
@perryk
Copy link
Contributor

perryk commented Jan 1, 2019

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"

src: "/urs/share/nginx/modules/{{ item }}.conf"

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
Copy link
Owner

@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
Copy link
Contributor

perryk commented Jan 5, 2019

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
Copy link
Owner

@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
Copy link
Owner

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

perryk added a commit to perryk/ansible-role-nginx that referenced this pull request Jan 6, 2019
jdauphant pushed a commit that referenced this pull request Jan 8, 2019
* Fix typo in modules config and restrict to EPEL (#232)

* Fixes warning from duplicate when's in modules configuration (#233)
mrwacky42 added a commit to HeadspaceMeditation/ansible-role-nginx that referenced this pull request Jan 18, 2020
* Explicitly setting the nginx configuration file in (jdauphant#223)

the "check nginx configuration" handler.

* Fixing Ansible 2.7.0 deprication warnings (jdauphant#225)

* * Fixing Ansible 2.7.0 deprication warnings
  For further details take a look at: https://docs.ansible.com/ansible/devel/porting_guides/porting_guide_2.7.html#using-a-loop-on-a-package-module-via-squash-actions

* * Remving travis deprecation warning - Moving from "--sudo" to "--become"

* * Ignoring symlinks errors during ansible_check_mode

* Small spelling correction (jdauphant#228)

* Add support to declare nginx modules in config file (jdauphant#227)

* We can declare nginx modules now

* We can declare nginx modules now

* Correct load_module definition in template

* Add task to remove `default.conf` from sites-enabled/ (jdauphant#231)

* Add task to remove `default.conf` from sites-enabled/

* Check if `default` site is not inside user config

* fix modules definition and add README section about this feature (jdauphant#232)

* Fix typo in modules config and restrict to EPEL (jdauphant#232) (jdauphant#235)

* Fix typo in modules config and restrict to EPEL (jdauphant#232)

* Fixes warning from duplicate when's in modules configuration (jdauphant#233)

* Extends support for configuring modules (jdauphant#236) (jdauphant#237)

Module configuration should now work for the following:

 Centos/RHEL with either EPEL or Official Nginx repo
 Debian/Ubuntu with either standard APT repo or Official Nginx repo

Please see issue jdauphant#236 for further details.

* Update README.md

* download mime.types file if it's missing (jdauphant#241)

* configuration: allow templates for conf.d independent files (jdauphant#238)

* Fix for jdauphant#242 Stick to ansible-lint rules. (jdauphant#243)

* trailing whitespace

* [701] Role info should contain description

* [601] Don't compare to literal True/False

* [502] All tasks should be named

* [206] Variables should have spaces before and after: {{ var_name }}

* skip_ansible_lint rule [403] Package installs should not use latest

* [204] Lines should be no longer than 160 chars

Co-authored-by: Timo Runge <timorunge@users.noreply.github.com>
Co-authored-by: TheSycamore <TheSycamore@users.noreply.github.com>
Co-authored-by: Dmitry Ge <22640222+q2digger@users.noreply.github.com>
Co-authored-by: Tommaso <p.tommy93@gmail.com>
Co-authored-by: Perry Kollmorgen <pcjkollmorgen@hotmail.com>
Co-authored-by: Julien DAUPHANT <jdauphant@users.noreply.github.com>
Co-authored-by: Tony Crowe <hellotonycrowe@protonmail.com>
Co-authored-by: paulrbr-fl <43074087+paulrbr-fl@users.noreply.github.com>
Co-authored-by: Bas <bas.meijer@me.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants