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

/etc/nginx/conf.d/default.conf should be named 50-default.conf to help config file ordering #2593

Open
p12tic opened this issue Mar 1, 2025 · 3 comments · May be fixed by #2601
Open

/etc/nginx/conf.d/default.conf should be named 50-default.conf to help config file ordering #2593

p12tic opened this issue Mar 1, 2025 · 3 comments · May be fixed by #2601

Comments

@p12tic
Copy link
Contributor

p12tic commented Mar 1, 2025

Currently nginx-proxy puts its configuration into a file named default.conf which is included via include /etc/nginx/conf.d/*.conf; directive. The nginx configuration is order dependent, thus if users rely on anything in default.conf being ordered before or after their config file, they must choose the name for the config file accordingly. Currently the name must be something that comes either before or after default.conf in alphabetical order. This is confusing.

A simple fix for this would be to rename the generated config file from default.conf to have some number prepended, such as 50-default.conf.

Please let me know if this issue is something for which a PR would be accepted. I would then go prepare one.

@p12tic
Copy link
Contributor Author

p12tic commented Mar 13, 2025

I assumed that adding feature tag means acknowledgement and created a PR.

@buchdag
Copy link
Member

buchdag commented Mar 23, 2025

Hi @p12tic

Sorry if adding the tag was misleading, that's just to help me categorize issues.

I agree the generated config file being named default.conf does not help with config ordering, but that was not a random choice : that's the name of the default config that's already inside the base nginx images.

➜  ~ docker run --rm nginx:1.27 cat /etc/nginx/conf.d/default.conf 
server {
    listen       80;
    server_name  localhost;

    #access_log  /var/log/nginx/host.access.log  main;

    location / {
        root   /usr/share/nginx/html;
        index  index.html index.htm;
    }

    #error_page  404              /404.html;

    # redirect server error pages to the static page /50x.html
    #
    error_page   500 502 503 504  /50x.html;
    location = /50x.html {
        root   /usr/share/nginx/html;
    }

    # proxy the PHP scripts to Apache listening on 127.0.0.1:80
    #
    #location ~ \.php$ {
    #    proxy_pass   http://127.0.0.1;
    #}

    # pass the PHP scripts to FastCGI server listening on 127.0.0.1:9000
    #
    #location ~ \.php$ {
    #    root           html;
    #    fastcgi_pass   127.0.0.1:9000;
    #    fastcgi_index  index.php;
    #    fastcgi_param  SCRIPT_FILENAME  /scripts$fastcgi_script_name;
    #    include        fastcgi_params;
    #}

    # deny access to .htaccess files, if Apache's document root
    # concurs with nginx's one
    #
    #location ~ /\.ht {
    #    deny  all;
    #}
}

So if we generate the template to another file (say /etc/nginx/conf.d/50-default.conf), we'll always have this default.conf file present and included by nginx, causing it to listen to 80 and serving the content of /usr/share/nginx/html when receiving a request for localhost.

That's not something we want, and this could be fixed by removing this file from the image, but that would cause another issue: people expect the config to be generated at /etc/nginx/conf.d/default.conf and some have automation based on that assumption. It's been like for 10 years + and unfortunately we can't decide to unilaterally and irreversibly change this now.

If we're going to change that, even if the reasons are sound, the change should be optional at first. Then if we choose to make it default, it should still be possible to fallback to the previous behaviour.

@p12tic
Copy link
Contributor Author

p12tic commented Mar 24, 2025

Thanks a lot for an explanation. Things are always more complicated than they seem from the first sight. Renaming is clearly not an option given the additional complexity. I guess a note about config file ordering in the documentation would be enough.

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

Successfully merging a pull request may close this issue.

2 participants