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

luci-nginx: nginx is missing gzip support, although required by OpenWrt default config #4643

Closed
wirelessuer opened this issue Dec 6, 2020 · 10 comments

Comments

@wirelessuer
Copy link

I just compiled OpenWrt including luci from master branch (6.12.2020, luci version 555faf6)

Using the meta-package luci-nginx, the nginx server is unable to start up in OpenWrt.

Reason:
The default config for nginx requires gzip support, which isn't available (compiled in) in the packages selected from luci-nginx

Output of logread -f when starting nginx with /etc/init.d/nginx start:

Sun Dec  6 14:47:14 2020 daemon.err nginx_init: 2020/12/06 14:47:14 [emerg] 3028#0: unknown directive "gzip" in /etc/nginx/nginx.conf:23
Sun Dec  6 14:47:14 2020 daemon.err nginx_init: nginx: configuration file /etc/nginx/nginx.conf test failed
Sun Dec  6 14:47:14 2020 daemon.err nginx_init: NOT using conf file!

Content of the file /etc/nginx/nginx.conf on a fresh installed OpenWrt system:

# Please consider creating files in /etc/nginx/conf.d/ instead of editing this.
# For details see https://openwrt.org/docs/guide-user/services/webserver/nginx

worker_processes auto;

user root;

events {}

http {
        access_log off;
        log_format openwrt
                '$request_method $scheme://$host$request_uri => $status'
                ' (${body_bytes_sent}B in ${request_time}s) <- $http_referer';

        include mime.types;
        default_type application/octet-stream;
        sendfile on;

        client_max_body_size 128M;
        large_client_header_buffers 2 1k;

        gzip on;
        gzip_vary on;
        gzip_proxied any;

        root /www;

        include conf.d/*.conf;
}

I'm creating this bug as suggested by @Ansuel within the following thread at OpenWrt forum:
https://forum.openwrt.org/t/package-luci-nginx-nginx-server-not-starting-up-with-package-config/79019/3

@Ansuel
Copy link
Member

Ansuel commented Dec 6, 2020

What is strange is the fact that NGINX_HTTP_GZIP is enabled by default. Can you check your compile flag for the nginx-ssl package?

@wirelessuer
Copy link
Author

nginx-ssl is unset

Here the relevant section out of my build config (file .config)

#
# Web Servers/Proxies
#
# CONFIG_PACKAGE_apache is not set
CONFIG_PACKAGE_cgi-io=y
# CONFIG_PACKAGE_clamav is not set
# CONFIG_PACKAGE_etesync-server is not set
# CONFIG_PACKAGE_freshclam is not set
# CONFIG_PACKAGE_frpc is not set
# CONFIG_PACKAGE_frps is not set
# CONFIG_PACKAGE_gateway-go is not set
# CONFIG_PACKAGE_gunicorn3 is not set
# CONFIG_PACKAGE_haproxy is not set
# CONFIG_PACKAGE_haproxy-nossl is not set
# CONFIG_PACKAGE_kcptun-client is not set
# CONFIG_PACKAGE_kcptun-config is not set
# CONFIG_PACKAGE_kcptun-server is not set
# CONFIG_PACKAGE_lighttpd is not set
CONFIG_PACKAGE_nginx=y
# CONFIG_PACKAGE_nginx-all-module is not set
CONFIG_PACKAGE_nginx-mod-luci=y
# CONFIG_PACKAGE_nginx-ssl is not set
# CONFIG_PACKAGE_nginx-ssl-util is not set
CONFIG_PACKAGE_nginx-ssl-util-nopcre=y
# CONFIG_PACKAGE_polipo is not set
# CONFIG_PACKAGE_privoxy is not set
# CONFIG_PACKAGE_python3-gunicorn is not set
# CONFIG_PACKAGE_radicale is not set
# CONFIG_PACKAGE_radicale2 is not set
# CONFIG_PACKAGE_radicale2-examples is not set
# CONFIG_PACKAGE_shadowsocks-libev-config is not set
# CONFIG_PACKAGE_shadowsocks-libev-ss-local is not set
# CONFIG_PACKAGE_shadowsocks-libev-ss-redir is not set
# CONFIG_PACKAGE_shadowsocks-libev-ss-rules is not set
# CONFIG_PACKAGE_shadowsocks-libev-ss-server is not set
# CONFIG_PACKAGE_shadowsocks-libev-ss-tunnel is not set
# CONFIG_PACKAGE_sockd is not set
# CONFIG_PACKAGE_socksify is not set
# CONFIG_PACKAGE_spawn-fcgi is not set
# CONFIG_PACKAGE_squid is not set
# CONFIG_PACKAGE_tinyproxy is not set
# CONFIG_PACKAGE_uhttpd is not set
CONFIG_PACKAGE_uwsgi=y
CONFIG_PACKAGE_uwsgi-cgi-plugin=y
# CONFIG_PACKAGE_uwsgi-logfile-plugin is not set
CONFIG_PACKAGE_uwsgi-luci-support=y
# CONFIG_PACKAGE_uwsgi-python3-plugin is not set
CONFIG_PACKAGE_uwsgi-syslog-plugin=y
# end of Web Servers/Proxies

And for info the whole .config file as attachment:
config.zip

@Ansuel
Copy link
Member

Ansuel commented Dec 6, 2020

nginx is nginx-ssl that has the gzip config selected by default (in theory)...

@hnyman
Copy link
Contributor

hnyman commented Dec 6, 2020

nginx is nginx-ssl that has the gzip config selected by default (in theory)...

Only in theory as there is one erroneous line due to the old nginx/nginx-ssl separation.

I tested from menuconfig and with selecting :

  • nginx-ssl, the gzip=y gets selected ok.
  • nginx, it does not get selected

I think that is due to
https://github.com/openwrt/packages/blob/4c045a02601efda7b105e5678bbe5f609eee4026/net/nginx/Config_ssl.in#L9

menu "Configuration"
        depends on PACKAGE_nginx-ssl

That line causes the config menu to be dependent on "nginx-ssl" being explicitly selected. If nginx is selected via plain "nginx", then that does not hold true and the intended proper config defaults are not applied at all. I think that currently buildbot builds a faulty package...

Plain "nginx" is also still offered from menuconfig, and then the config gets screwed, as the proper config gets only evaluated is "nginx-ssl" is selected.

The dependency should be changed to be either ngnix or nging-ssl (and I think that it is only needed to keep menuconfig and .config more clean in case nginx are not selected at all)

Best solution might be to drop nginx-ssl in total, if there is now only one ssl-enabled nginx in any case.

@Ansuel
Copy link
Member

Ansuel commented Dec 6, 2020

I think the drop of nginx-ssl is a cleaner way to fix this. Will send some pm about this change.

@peter-stadler
Copy link

For solving the problem now, can we do step 4. from below (I did not test it yet without doing the not merged steps 2. and 3. before)? Else I would suggest to remove just the the line
https://github.com/openwrt/packages/blob/4c045a02601efda7b105e5678bbe5f609eee4026/net/nginx/Config_ssl.in#L9
(if possible) ...

I want to explain it a bit further TL;DR: The reason to keep nginx-ssl instead of nginx is the existence of nginx-all-module in connection with the behavior of opkg (its behavior is also the reason that I would suggest to keep nginx as a dummy package for now).

There are some aspects playing together:

  • We had three packages, where some could replace others by using PROVIDES in the Makefile:
    • nginx
    • nginx-ssl provided nginx
    • nginx-all-module provided nginx and nginx-ssl
  • Although luci-nginx depended upon nginx and luci-ssl-nginx depended upon nginx-ssl, both installed nginx-ssl as opkg selects the lexicographical last package providing the depended one.
  • A while ago we decided to always enable ssl for nginx; So, I tested a bit around and prepared an update path:
  1. nginx: enable ssl for all variants and remove nginx-util w/o ssl (merged PR)
  2. nginx-util: use UCI for server configuration (open PR)
  3. nginx: use UCI configuration provided by nginx-util (draft) needs an accepted patch for opkg not double deleting obsolete conf files else we get misleading error messages (but no other problem as far as I see).
  4. nginx: remove the nossl variant (draft)
  5. after a transition period nginx: remove dummies nginx, nginx-mod-luci-ssl and nginx-util (draft) needs the new patch for opkg making the selection of the provider consistent else opkg install luci-nginx would pull in nginx-ssl while opkg install nginx uses nginx-all-modules.
  • I think the best is to have nginx-ssl as the default package. As opkg selects the last provider, it must come lexicographical after nginx-all-module. This is the reason for getting rid of nginx instead of nginx-ssl.
  • For not changing the package and the luci tree at the same time, we want to provide both packages: nginx and nginx-ssl.
  • In the end nginx is just provided by nginx-ssl and by nginx-all-module, but there will be no own package anymore. In this case the opkg behaves differently, when installing a package that has DEPENDS:=+nginx and when calling opkg install nginx directly. I did made a patch (see step 5.) for this a while ago, but I am not sure @jow had time for it or who would look at it ... After making the behavior consistent in the two cases we can get rid of the dummy nginx package.

Just wanted to share my tests/thoughts for avoiding some pitfalls ...

@peter-stadler
Copy link

P.S.: As this is in the luci tree. We could change luci-nginx to depend on nginx-ssl for a quick fix, in the long run we want to have just one package here, too, don't we?

@hnyman
Copy link
Contributor

hnyman commented Dec 6, 2020

Sure, we should change and simplify both the main package side and the LuCI side at the same time.

@peter-stadler
Copy link

It would be safe to change the LuCI side right now (it would not even change the behavior from before, as it installed always nginx-ssl). I would opt for keeping luci-ssl-nginx ...

@jow-
Copy link
Contributor

jow- commented Mar 1, 2021

Closing this as it appears to be an nginx packaging error.

@jow- jow- closed this as completed Mar 1, 2021
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

No branches or pull requests

5 participants