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

invalid config generated when ssl is false and listen_port == ssl_port #648

Closed
PierreGambarotto opened this issue Jun 18, 2015 · 11 comments
Closed
Labels
bug Something isn't working

Comments

@PierreGambarotto
Copy link

in vhost.pp, when $ssl is false and $listen_port == $ssl_port (which is a not so logical config, albeit a not illegal one), no header and footer fragment are defined for the vhost file, thus generating an invalid config file.

I think a fix would be to add and not $ssl as a guard in the if condition, i.e. change

if ($listen_port != $ssl_port) { 

to

if ($listen_port != $ssl_port and not $ssl) { 
@3flex 3flex added the bug Something isn't working label Jun 18, 2015
@mvintila
Copy link
Contributor

I have this in my config:

nginx::resource::vhost {'vpp':
    server_name => $servernames,
    listen_port     => 443,
    ssl_port         => 443,
    ssl                 => true,
    ssl_cert         => '/etc/ssl/private/lb/cert.cer',
    ssl_key         => '/etc/ssl/private/lb/cert.key',
    proxy            => 'https://railsweb',
    location_cfg_append => { 
      'rewrite' => '^ https://$server_name$request_uri? permanent',
    },
    require => File ['/etc/ssl/private/lb']
  }

And i am still getting the bug, so it's not related to ssl => true parameter.

@mvintila
Copy link
Contributor

Bug reproduced using version 0.2.6 of the module and the above configuration.

@mvintila
Copy link
Contributor

Even downgrading to 0.0.10 produces the same config file with the "location" directives above the "server" block.

I think the problem is related to defining locations that point to ssl-enabled vhosts. When the configuration is defined so that there are both ssl and plain servers($ssl_port != $listen_port), the locations will be generated in the plain server block, above the ssl-enabled server block.

When $ssl_port == $listen_port, the plain part is simply removed, but the "location" directives remain in the same spot as before, above the ssl-enabled server block, outside of any server block.

@tarcinil
Copy link

I am running into this as well.

@tarcinil
Copy link

I was able to resolve this as you need to have

    ssl => true,
    ssl_only => true,

on the location as well as the vhost. I noticed that this is also in the documentation but I would recommend that the documentation express it a little clearer.

@jeremycrussell
Copy link

Upvoting @tarcinil , this isn't clear in the docs...

@klingenm
Copy link

I have run into this as well.

@wyardley
Copy link
Collaborator

Is anyone willing to contribute a PR for this, and / or do some functional testing to a proposed fix?

@wyardley
Copy link
Collaborator

Also, can someone test whether this still happens with most recent release (0.5.0)?

@wyardley
Copy link
Collaborator

ps - As of now, I think lines 122-132 in the README do sort of explain this.

@wyardley
Copy link
Collaborator

wyardley commented Nov 3, 2017

I think a warning should be given now:
https://github.com/voxpupuli/puppet-nginx/blob/master/manifests/resource/server.pp#L302-L306
Closing via #1015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants