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

nginx 0.6.0: bad location block causes nginx restart to fail #1029

Closed
ardrigh opened this issue Feb 28, 2017 · 6 comments
Closed

nginx 0.6.0: bad location block causes nginx restart to fail #1029

ardrigh opened this issue Feb 28, 2017 · 6 comments

Comments

@ardrigh
Copy link
Contributor

ardrigh commented Feb 28, 2017

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 4.5.3 (Puppet Enterprise 2016.2)
  • Ruby: ruby 2.1.9p490
  • Distribution: CentOS 7
  • Module version: puppet-nginx 0.6.0

How to reproduce (e.g Puppet code you use)

Trying to create a default entry to redirect HTTP to HTTPS

nginx::nginx_servers:
  '_':
    ensure: 'present'
    listen_options: 'default_server'
    listen_port: 80
    ssl: false
    ssl_redirect: true

What are you seeing

Nginx service fails to restart due to location block outside the server block in the generated file:

# MANAGED BY PUPPET
server {
  listen *:80 default_server;
  server_name           _;

  return 301 https://$host$request_uri;
  access_log            /var/log/nginx/_.access.log combined;
  error_log             /var/log/nginx/_.error.log;
}

  location / {
    index     index.html index.htm index.php;
  }

What behaviour did you expect instead

A location block inside the server block, or a better solution would be no location block for this redirect.

# MANAGED BY PUPPET
server {
  listen *:80 default_server;
  server_name           _;

  return 301 https://$host$request_uri;
  access_log            /var/log/nginx/_.access.log combined;
  error_log             /var/log/nginx/_.error.log;
}

Output log

Info: /Stage[main]/Nginx/Nginx::Resource::Server[_]/Concat[/etc/nginx/conf.d/_.conf]/File[/etc/nginx/conf.d/_.conf]: Scheduling refresh of Class[Nginx::Service]
Info: Concat[/etc/nginx/conf.d/_.conf]: Scheduling refresh of Class[Nginx::Service]
Info: Nginx::Resource::Location[_-default]: Scheduling refresh of Class[Nginx::Service]
Info: Class[Nginx::Service]: Scheduling refresh of Service[nginx]
Error: /Stage[main]/Nginx::Service/Service[nginx]: Failed to call refresh: Could not restart Service[nginx]: Execution of '/bin/systemctl restart nginx' returned 1: Job for nginx.service failed because the control process exited with error code. See "systemctl status nginx.service" and "journalctl -xe" for details.
Error: /Stage[main]/Nginx::Service/Service[nginx]: Could not restart Service[nginx]: Execution of '/bin/systemctl restart nginx' returned 1: Job for nginx.service failed because the control process exited with error code. See "systemctl status nginx.service" and "journalctl -xe" for details.

Any additional information you'd like to impart

Adding use_default_location: false with ssl_redirect: true will remove the location block.
This should not be required with ssl_redirect: true

@ardrigh
Copy link
Contributor Author

ardrigh commented Mar 1, 2017

After some more testing, it looks like defining the location blocks using an example similar to the README causes problems.

From the README:

nginx::nginx_servers:
  'www.puppetlabs.com':
    www_root: '/var/www/www.puppetlabs.com'
  'rack.puppetlabs.com':
    proxy: 'http://puppet_rack_app'
nginx::nginx_locations:
  'static':
    location: '~ "^/static/[0-9a-fA-F]{8}\/(.*)$"'
    server: www.puppetlabs.com
    www_root: /var/www/html
  'userContent':
    location: /userContent
    server: www.puppetlabs.com
    www_root: /var/www/html

If I setup this type of block in Puppet, it is always creating the location blocks outside of the server block and nginx cannot restart.

If I move the location settings under the nginx::nginx_servers block using the locations hash, it now places the location blocks correctly. And drop using nginx::nginx_locations entirely.

Is using locations hash with nginx::nginx_servers the best practice for using this module?

Is this due to changes that haven't been updated in the documentation?

@sanigo
Copy link

sanigo commented Mar 14, 2017

Yes, i have exactly the same problem.

@ardrigh
Copy link
Contributor Author

ardrigh commented Mar 28, 2017

As an update to this ticket for anyone reading, here is an example of defining a Server and Location using Hiera data. This is used for a nginx proxy for a local Conflunce server 6 install.

The first entry "_" is the default server on port 80 with a straight redirect to SSL port 443

The important part is specifying locations inside the server definition:

nginx::nginx_servers:
  '_':
    ensure: 'present'
    listen_options: 'default_server'
    listen_port: 80
    ssl: false
    ssl_redirect: true
    use_default_location: false
  'confluence':
    ensure: 'present'
    index_files: ~
    listen_options: 'default_server'
    listen_port: 443
    locations:
      'confluence':
        ensure: 'present'
        location: '/'
        proxy_set_header:
          - 'X-Forwarded-Host $host'
          - 'X-Forwarded-Server $host'
          - 'X-Forwarded-For $proxy_add_x_forwarded_for'
        proxy: 'http://localhost:8090/'
      'synchrony-proxy':
        ensure: 'present'
        location: '/synchrony-proxy'
        proxy: 'http://localhost:8090/synchrony-proxy'
        proxy_http_version: '1.1'
        proxy_set_header:
          - 'X-Forwarded-Host $host'
          - 'X-Forwarded-Server $host'
          - 'X-Forwarded-For $proxy_add_x_forwarded_for'
          - 'Upgrade $http_upgrade'
          - 'Connection "Upgrade"'
    server_name:
      - 'wiki.example.com'
      - 'wiki'
    ssl: true
    ssl_cert: '/etc/pki/tls/certs/cert_file'
    ssl_key: '/etc/pki/tls/private/cert_key'
    ssl_port: 443
    use_default_location: false

@oranenj
Copy link
Contributor

oranenj commented Mar 31, 2017

I think this happens because in nginx::resource::server, setting ssl_redirect to true causes ssl_only to become true, which creates "ssl locations" with the wrong order, but the ssl header fragments don't actually get created.

@dol
Copy link
Contributor

dol commented Apr 11, 2017

Related to this issue: Their is no test coverage of nginx::nginx_servers::locations

cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this issue Sep 13, 2019
@nafetsreuab
Copy link

@ardrigh
you made my day with your hiera example. Man this was a tough one. thank you.

Rubueno pushed a commit to Rubueno/puppet-nginx that referenced this issue Oct 19, 2020
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

5 participants