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

Parameters sorted incorrectly inside nginx.conf #40

Open
TaiSHiNet opened this issue Jul 9, 2014 · 24 comments
Open

Parameters sorted incorrectly inside nginx.conf #40

TaiSHiNet opened this issue Jul 9, 2014 · 24 comments

Comments

@TaiSHiNet
Copy link
Contributor

Currently I'm suffering an issue due to contents being sorted with includes first:

http {
    access_log /var/log/nginx/access.log;
    client_max_body_size 10m;
    default_type application/octet-stream;
    error_log /var/log/nginx/error.log;
    gzip off;
    gzip_disable "msie6";

    include /etc/nginx/mime.types;
    include /etc/nginx/conf.d/*.conf;
    include /etc/nginx/sites-enabled/*;
    keepalive_timeout 65;
    proxy_cache_path /var/run/nginx-cache keys_zone=temp:100m;
    proxy_temp_path /var/run/nginx-tmp;
    sendfile on;
    server_names_hash_bucket_size 128;
    tcp_nodelay on;
    tcp_nopush on;
    types_hash_max_size 2048;
    underscores_in_headers on;
}

Given that I'm trying to use cache, this makes my configuration fail (vhost is loaded before cache is defined)

@TaiSHiNet
Copy link
Contributor Author

cc: @cheuschober
Yeah... me again :P

@cheuschober
Copy link
Contributor

@TaiSHiNet, Thanks for the report.

@cheuschober
Copy link
Contributor

@TaiSHiNet, Could you share your pillar? I thought the nginx pieces accept lists of dicts; is this your main nginx.conf? We might have to change the data structure at

https://github.com/saltstack-formulas/nginx-formula/blob/master/nginx/ng/map.jinja#L44

to an odict([]). I had hoped to avoid that since it's a lot easier to use an updated pillar module than it is to bring in a new utils (breaking this formula for a greater number of users till Helium releases).

@TaiSHiNet
Copy link
Contributor Author

init.sls

nginx:
  ng:
    # These are usually set by grains in map.jinja
    lookup:
      package: nginx
      service: nginx
      webuser: www-data
      conf_file: /etc/nginx/nginx.conf
      vhost_available: /etc/nginx/sites-available
      vhost_enabled: /etc/nginx/sites-enabled
      vhost_use_symlink: True

    # Source compilation is not currently a part of nginx.ng
    from_source: False

    package:
      opts: {} # this partially exposes parameters of pkg.installed

    service:
      enable: True # Whether or not the service will be enabled/running or dead
      opts: {} # this partially exposes parameters of service.running / service.dead

    server:
      opts: {} # this partially exposes file.managed parameters as they relate to the main nginx.conf file

      config: 
        worker_processes: 4
        pid: /run/nginx.pid
        events:
          worker_connections: 768
        http:
          sendfile: 'on'
          include:
            - /etc/nginx/mime.types
            - /etc/nginx/conf.d/*.conf
            - /etc/nginx/sites-enabled/*
          server_names_hash_bucket_size: 128
          underscores_in_headers: 'on'
          client_max_body_size: 10m

    vhosts:
      disabled_postfix: .disabled # a postfix appended to files when doing non-symlink disabling
      symlink_opts: {} # partially exposes file.symlink params when symlinking enabled sites
      rename_opts: {} # partially exposes file.rename params when not symlinking disabled/enabled sites
      managed_opts: {} # partially exposes file.managed params for managed vhost files
      dir_opts: {} # partially exposes file.directory params for site available/enabled dirs

      managed:
        default:
          enabled: False
          config:
            - server:
              - listen: 
                - 80
                - default_server
              - return: 444

cache.sls

nginx:
  ng:
    server:
      config:
        http:
          proxy_cache_path: /var/run/nginx-cache keys_zone=temp:100m
          proxy_temp_path: /var/run/nginx-tmp

Right now they're separated but I played with them on the same config and changing line position, tried naming one before the other on the pillar top

@cheuschober
Copy link
Contributor

Hrm. I know I solved this for my team but I'm away from my work computer. Will have to leave it for the weekend then I can see what's up. In either case I'm thinking I might re-do the templating there anyway -- the two template files are so similar I might as well combine them and ensure the same data construction/serialization that works well for vhost config also works for the main nginx.conf.

@TaiSHiNet
Copy link
Contributor Author

Hit me up whenever you want on weekdays, I'll gladly drop tests for you on my env

@TaiSHiNet
Copy link
Contributor Author

Any news on this?
/hug

@cheuschober
Copy link
Contributor

Haven't forgotten about it and am absolutely committed to fixing but just have had a couple weeks worth of other work drop in my lap.

@ghost
Copy link

ghost commented Sep 19, 2014

Offering a double /hug for a fix :)

@Seldaek
Copy link

Seldaek commented Apr 15, 2015

I just hit the same issue with fastcgi_cache_path and fastcgi_cache both defined in http{} but when sorted the cache comes first and creates a conflict when cache_path tries to define the cache name.

I could solve it by moving fastcgi_cache into the server{} block in my vhosts, but that's just lucky because fastcgi sorts before include so definitely would be happy to see this fixed as well.

@Kurt108
Copy link

Kurt108 commented Jun 3, 2015

+1 for this issue. From my perspective it is impossible to configure an upstream proxy because the vhost is always included before the cache_path or the cache directive is printed before the cache_path.

@TaiSHiNet
Copy link
Contributor Author

My current workaround is creating a vhost called 000-whatever, but I'd love to have this fixed

@Kurt108
Copy link

Kurt108 commented Jun 13, 2015

I also made a workaround which works for me:

https://github.com/kurt---/nginx-formula/tree/enable-proxy

it works by adding a proxy.conf-configuration-file which is inserted before the proxy_cache-directive. The config-file enables the cache (and logging-option) and one can enable the cache if desired. to enable the additional access-log one need to put these into the vhost-config else we ran into the same problem with options in the wrong order.

So this is not a fix but it works for me to enable a proxy in nginx.

EvaSDK added a commit to EvaSDK/nginx-formula that referenced this issue Aug 15, 2015
This is crap but configuration for http scope cannot be both ordered and
merged from map.jinja it seems, see issue saltstack-formulas#40.
@outime
Copy link
Contributor

outime commented Oct 5, 2015

Is this still broken?

@gravyboat
Copy link
Contributor

@outime This is specifically for the ng formula as far as I'm aware, are you using that portion?

@aboe76
Copy link
Member

aboe76 commented Nov 9, 2015

@gravyboat it still broken only @EvaSDK workaround works.

@puneetk
Copy link
Contributor

puneetk commented Nov 9, 2015

@EvaSDK or @aboe76 can you send a pull request with the work around please, we ll get it into mainline for the users of the formula.

EvaSDK added a commit to EvaSDK/nginx-formula that referenced this issue Nov 12, 2015
This is crap but configuration for http scope cannot be both ordered and
merged from map.jinja it seems, see issue saltstack-formulas#40.
@aboe76
Copy link
Member

aboe76 commented Nov 13, 2015

@EvaSDK thanks, for this, I now it doesn't look nice but it works.

@EvaSDK
Copy link
Contributor

EvaSDK commented Nov 14, 2015

yeah, I hope someone finds a better workaround but at least it gets it working for some use cases.

@bentwire
Copy link

This also affects setting the logging format as well. Due to log_format being defined in the config after access_log.

lingonl added a commit to lingonl/nginx-formula that referenced this issue May 26, 2016
Adding sort_keys=False to json() allows nginx.server.config to be sorted as needed for the resulting config file to work with access_log+log_format (issue saltstack-formulas#102) and other parameters (issue saltstack-formulas#40)
@EvaSDK
Copy link
Contributor

EvaSDK commented Mar 6, 2017

And this is back again with nginx loadable module support landing in Debian.
Now, one needs to include some files prior to http context in nginx.conf configuration file otherwise the directives are loaded after parsing the enabled sites which does not work as include is sorted after http.

EvaSDK added a commit to EvaSDK/nginx-formula that referenced this issue Mar 6, 2017
Another occurence of the declarations sorting problem from issue saltstack-formulas#40.

It will contain instructions that must be executed first on Debian.
EvaSDK added a commit to EvaSDK/nginx-formula that referenced this issue Mar 6, 2017
Another occurence of the declarations sorting problem from issue saltstack-formulas#40.

It will contain instructions that must be executed first on Debian.
@ernstae
Copy link

ernstae commented May 23, 2017

This is definitely causing a problem in Ubuntu for me. We are using the streaming module for TCP load balancing in a few of our environments.
Currently, the workaround is to hackily do this (added it to the top of nginx/ng/files/nginx.conf) in our environment

+# this is a hack because it won't add correctly in the pillar (gets re-ordereed)
+include /etc/nginx/modules-enabled/*.conf;

making sure this is on the first (or nearly first) line of the nginx configuration, because without it the stream{} directive is unable to load.

This reared its ugly head tonight, because we rebased with your repo, and the hacky customization got missed in the internal pull request on our saltstack repo.

Any chance this use case could be handled?

@ernstae
Copy link

ernstae commented May 23, 2017

I have self-resolved on my end, by pushing the needed configuration into the pillar data.
Prior to the pull request #120 -- this wasn't working in our environment.

It works now by adding another include to the pillar:

    ng:
      server:
        config:
          include: '/etc/nginx/modules-enabled/*.conf'
          stream:
            server:
              listen: 7999

@noelmcloughlin
Copy link
Member

See: saltstack/salt#12161

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