saltstack-formulas / nginx-formula

Nginx Salt Formula
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
163 stars 419 forks source link

Parameters sorted incorrectly inside nginx.conf #40

Open TaiSHiNet opened 10 years ago

TaiSHiNet commented 10 years ago

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 commented 10 years ago

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

cheuschober commented 10 years ago

@TaiSHiNet, Thanks for the report.

cheuschober commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

TaiSHiNet commented 10 years ago

Any news on this? /hug

cheuschober commented 10 years ago

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 commented 10 years ago

Offering a double /hug for a fix :)

Seldaek commented 9 years ago

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 commented 9 years ago

+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 commented 9 years ago

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

Kurt108 commented 9 years ago

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.

outime commented 9 years ago

Is this still broken?

gravyboat commented 9 years ago

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

aboe76 commented 9 years ago

@gravyboat it still broken only @EvaSDK workaround works.

puneetk commented 9 years ago

@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.

aboe76 commented 8 years ago

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

EvaSDK commented 8 years ago

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

bentwire commented 8 years ago

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

EvaSDK commented 7 years ago

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.

ernstae commented 7 years ago

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 commented 7 years ago

I have self-resolved on my end, by pushing the needed configuration into the pillar data. Prior to the pull request https://github.com/saltstack-formulas/nginx-formula/pull/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 commented 5 years ago

See: https://github.com/saltstack/salt/issues/12161