search

matomo-org / matomo-nginx

Nginx configuration for running Matomo
408 stars 121 forks source link

Don't have configuration errors by default #46

Closed flip111 closed 6 years ago

flip111 commented 6 years ago

Some lines gives configuration errors, such as:

https://github.com/matomo-org/matomo-nginx/blob/14415ffd127722eb9b339e97bd1c621931ef6aa1/sites-available/matomo.conf#L22-L23

The reason is obvious .. the lines point to a non-existing file. The purpose of these lines is that they should be edited manually. I would like to ask that all lines that yield invalid configuration that need to be edited manually to be commented out or remove and place a description instead.

Right now i can not install the matomo nginx script and then run let's encrypt certbot because it will choke on these configuration errors. The two lines in the link in particularly don't need to be there because certbot will offer to add them (with nginx flag) if the conf file can be found.

I'm making an unattended install script and like as little manual intervention as possible.

Findus23 commented 6 years ago

Hi,

I'm not sure, but wouldn't nginx not also throw an error if the ssl_certificate is missing?

flip111 commented 6 years ago

I'm not sure about that either. I think the configuration would not work, but it won't be a critical error that prevents nginx from starting or checking for further problems. We need to test this.

Findus23 commented 6 years ago

Just tested it and when the path is invalid nginx throws an error, but when the lines are commented out, nginx behaves strange. No error on nginx -t and reloading just makes nginx continue with the old ssl cert. Only when restarting it ignores the vhost.

Honestly this is more confusing than the current way, so I'd recommend you (while you are already automating it) to just comment out the two lines with sed:

sed -i '/ ssl_certificate /s/^/#/' sites-available/matomo.conf  
sed -i '/ ssl_certificate_key /s/^/#/' sites-available/matomo.conf
flip111 commented 6 years ago

What you say makes sense. Just that ... who would change config and NOT restart/reload nginx? Is this a thing to change config while being online? o_O

I can use sed for my purposes. But still I would recommend to minimize the lines that give config errors.

Please feel free to close if my opinion is not shared.

On Fri, Oct 12, 2018, 20:26 Lukas Winkler notifications@github.com wrote:

Just tested it and when the path is invalid nginx throws an error, but when the lines are commented out, nginx behaves strange. No error on nginx -t and reloading just makes nginx continue with the old ssl cert. Only when restarting it ignores the vhost.

Honestly this is more confusing than the current way, so I'd recommend you (while you are already automating it) to just comment out the two lines with sed:

sed -i '/ ssl_certificate /s/^/#/' sites-available/matomo.conf sed -i '/ ssl_certificate_key /s/^/#/' sites-available/matomo.conf

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/matomo-org/matomo-nginx/issues/46#issuecomment-429400098, or mute the thread https://github.com/notifications/unsubscribe-auth/ACI_gL0vo4fyMKmpwbOQH-QqMxNrcAiFks5ukNCxgaJpZM4XZ27X .

Findus23 commented 6 years ago

Hi, Most of the time I only reload and not restart nginx, but regarding ssl certs it sometimes behaves a bit odd.

I think I'll close this unless someone also thinks the same.