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.

Sign up for GitHub

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

Jump to bottom

support nginx_* env variables on nginx container and vhosts #1326

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

support nginx_* env variables on nginx container and vhosts #1326

wants to merge 1 commit into from

Conversation

poma
Copy link

@poma poma commented Sep 5, 2019 •
edited
Loading

Implementation of #1324 feature request. Environment variables starting with nginx_* are added as nginx config statements, for example: nginx_client_max_body_size=30M. Variables of jwilder/nginx-proxy container are added to the global config, and variables on others containers are added to the respective vhosts.

Example docker-compose.yml:

version: '2.1'

services:
  nginx:
    image: jwilder/nginx-proxy
    ports:
      - 80:80
      - 443:443
    volumes:
      - /var/run/docker.sock:/var/run/docker.sock:ro
    environment:
      nginx_client_max_body_size: 42M

  app:
    image: app
    environment:
      VIRTUAL_HOST: example.com
      nginx_client_max_body_size: 1337M

Currently only lower case variables are supported (until nginx-proxy/docker-gen#306 is merged). Also if there is an easy way to merge Env dicts from all containers with the same vhost, we should do that, currently Env map is taken from the first container for each vhost.

@pnbecker
Copy link

I like this PR and think it would be a helpful addition.

@poma
Copy link
Author

poma commented Dec 13, 2019

While we are waiting for this to be merged, you can use updated docker-gen from the repo poma/docker-gen that already contains this PR

bugficks pushed a commit to bugficks/nginx-proxy that referenced this pull request Jul 16, 2020
support nginx_* env variables on nginx container and vhosts nginx-pro…
388ed38 
…xy#1326 nginx-proxy#1326
@buchdag buchdag added kind/feature-request Issue requesting a new feature status/pr-needs-tests This PR needs new or additional test(s) status/pr-needs-docs This PR needs new or additional documentation labels Apr 5, 2021
@buchdag buchdag added type/feat PR for a new feature and removed kind/feature-request Issue requesting a new feature labels Apr 29, 2021
@buchdag buchdag self-assigned this Aug 8, 2021
@poma
Copy link
Author

poma commented Jun 14, 2022

Rebased PR onto the current master, now also supports upper case "NGINX_*" env vars.

One problem I've encountered is that some containers export export "NGINX_*" env vars that are invalid in the context of nginx conf and this might lead to broken nginx config generation. Notably nginx:alpine exports NGINX_VERSION which is an invalid in nginx config, this might happen if you want to put another nginx instance behind the main nginx proxy. There is a workaround of overriding exported value with an empty string. But it's still a breaking change that can break a working server when people update to it, so be careful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@buchdag buchdag

Labels
status/pr-needs-docs This PR needs new or additional documentation status/pr-needs-tests This PR needs new or additional test(s) type/feat PR for a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@poma @pnbecker @buchdag