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

Cannot add TCP Proxy configuration via "drop-in" config files #257

Closed
dylannorthrup opened this issue Aug 17, 2024 · 3 comments
Closed

Cannot add TCP Proxy configuration via "drop-in" config files #257

dylannorthrup opened this issue Aug 17, 2024 · 3 comments

Comments

@dylannorthrup
Copy link

Issue

On current Linux-based Docker images, it's straightforward to add a file to /etc/nginx/conf.d and have it processed by nginx because of the include /etc/nginx/conf.d/*.conf; entry in the http { ... } block of the nginx.conf file. But this can't be done with TCP Proxy configuration. Doing so generates the following error:

nginx: [emerg] "stream" directive is not allowed here in /etc/nginx/conf.d/proxy.conf:1

I can work around the issue by making a copy of nginx.conf and adding an include /path/to/alternate/conf.d/*.conf; directive outside the http { ... } block and doing a bind mount at runtime for the containers, but that leaves open the possibility of upstream config changes in the container will be missed because of my local changes.

Configurations tested

I tested with the 1.2.6 containers for jammy and rocky and both generated an error when the following contents were in /etc/nginx/conf.d/rabbitmq.conf

stream {
  [upstream](https://nginx.org/r/upstream) rabbitmq_backend {
    [server](https://nginx.org/r/server) rabbitmq-test:5672
  }

  log_format proxy '$remote_addr [$time_local] '
                   '$protocol $status $bytes_sent $bytes_received '
                   '$session_time "$upstream_addr" '
                   '"$upstream_bytes_sent" "$upstream_bytes_received" "$upstream_connect_time"';
  [server](https://nginx.org/r/server) {
    [listen](https://nginx.org/r/listen)      5671 ssl;

    [ssl_protocols](https://nginx.org/r/ssl_protocols)           TLSv1.3 TLSv1.2 TLSv1.1 TLSv1;
    [ssl_ciphers](https://nginx.org/r/ssl_ciphers)             RC4:HIGH:!aNULL:!MD5;
    [ssl_handshake_timeout](https://nginx.org/r/ssl_handshake_timeout)   30s;

    [ssl_certificate](https://nginx.org/r/ssl_certificate)       /etc/rabbitmq/ssl/rabbitmq-test.fullchain.pem;
    [ssl_certificate_key](https://nginx.org/r/ssl_certificate_key)   /etc/rabbitmq/ssl/rabbitmq-test.key;

    [proxy_connect_timeout](https://nginx.org/r/proxy_connect_timeout) 5s;
    [proxy_pass](https://nginx.org/r/proxy_pass) rabbitmq_backend;
  }
}

[server](https://nginx.org/r/server) {
  [listen](https://nginx.org/r/listen) 15671 ssl;
  [server_name](https://nginx.org/r/server_name) rabbitmq-test rabbitmq-test-01;

  [location](https://nginx.org/r/location) / {
    [proxy_pass](https://nginx.org/r/proxy_pass) localhost:15672;
    [proxy_set_header](https://nginx.org/r/proxy_set_header) Host $http_host;
    [proxy_set_header](https://nginx.org/r/proxy_set_header) X-Real-IP $remote_addr;
    [proxy_set_header](https://nginx.org/r/proxy_set_header) X-Forwarded-For $proxy_add_x_forwarded_for;
    [proxy_set_header](https://nginx.org/r/proxy_set_header) X-Forwarded-Proto $scheme;;
  }
}
@neomantra
Copy link
Member

I'm really sorry I didn't pick this up months ago. Thank you for preparing the report.

I just looked into the stream directive and see it is meant to be in the main section. We are including that /etc/nginx/conf.d/*.conf right here.

include can be include in any, per this doc. Perhaps we add something like this right at the end of the nginx.conf?

/etc/nginx/conf.d/*.main and one puts stream stanzas and more in there? For example, your configuration above would be in /etc/nginx/conf.d/rabbitmq.main and included at the end.

neomantra added a commit that referenced this issue Oct 22, 2024
Adds `include /etc/nginx/conf.d/*.conf;` to `nginx.conf` for injecting config into main stanza.

**NOTE:** This may be a **breaking change** for those bind-mounting existing files named `*.main`.
@dylannorthrup
Copy link
Author

I'm really sorry I didn't pick this up months ago. Thank you for preparing the report.

Life happens. It's understandable.

I just looked into the stream directive and see it is meant to be in the main section. We are including that /etc/nginx/conf.d/*.conf right here.

Yeah, and that include is inside the http {} section.

include can be include in any, per this doc. Perhaps we add something like this right at the end of the nginx.conf?

/etc/nginx/conf.d/*.main and one puts stream stanzas and more in there? For example, your configuration above would be in /etc/nginx/conf.d/rabbitmq.main and included at the end.

My workaround so far has been to add include /etc/nginx/tcp-conf.d/*.conf; after the http {} block. The naming is likely overly specific given configuration other than stream proxy configs could be put there. I'm happy to use whatever convention you and the team decide on and will adapt my configuration to that.

Thanks for taking a look at the issue!

@neomantra
Copy link
Member

Great, this was added in 1.25.3.2-2 (and also 1.27 series), using include /etc/nginx/conf.d/*.main. I opted to keep it in the same folder to reduce the need for multiple bind-points to nginx configuration.

I just re-discovered that this was reported long ago #171 and I didn't address it then -- sorry to all who needed this and glad I got it done now.

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

2 participants