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

do not start lighttpd when nginx is installed #2480

Closed
wants to merge 2 commits into from

Conversation

fabianfrz
Copy link
Member

the nginx plugin takes over and handles those requests so the port can be reused for other things like a reverse proxy or serving local files.

@AdSchellevis
Copy link
Member

@fabianfrz I think we should make this more generic, someone else might prefer apache which in this case requires our core package to be changed again for just disabling it. Cluttering dependencies is usually not a good idea.

@AdSchellevis
Copy link
Member

If I'm not mistaken, you should be able to add a similar construction like this https://github.com/opnsense/core/blob/18.1.9/src/etc/inc/plugins.inc.d/webgui.inc#L31-L38 for Nginx, which only leaves the minor problem of disabling the core webserver.

@fabianfrz
Copy link
Member Author

@AdSchellevis the problem is that nginx is started via RC and because lighttpd is started early, I would have to kill the process already running which means I would have to manipulate the RC file. I also do use the files generated here (certificate, dh parameters etc. so the user does not see any difference).

@AdSchellevis
Copy link
Member

I understand, but still don't think we should clutter these things together, so maybe we need to split some functionality first to offer a swap-in replacement, adding all kinds of hooks specific for software not installed is something we should try to avoid.

At the moment I don't have a lot of time available, so I can't help out with a better solution now, maybe after 18.7 comes out.

Copy link
Member

@fichtner fichtner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat discussed already, we need to do this another way so I'm closing this until 18.7 is out and we can all play with Nginx based on the prebuilt plugin.

@fichtner fichtner closed this Jul 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants