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

Separating the options with a space to avoid invalid one like "ssldefault" #218

Merged
merged 1 commit into from
Jan 2, 2014
Merged

Conversation

andreyev
Copy link

@andreyev andreyev commented Jan 2, 2014

No description provided.

jfryman pushed a commit that referenced this pull request Jan 2, 2014
Separating the options with a space to avoid invalid one like "ssldefault"
@jfryman jfryman merged commit b921b80 into voxpupuli:master Jan 2, 2014
@jfryman
Copy link
Contributor

jfryman commented Jan 2, 2014

Super! Thanks!

@3flex
Copy link
Contributor

3flex commented Jan 2, 2014

This should be reverted. The tests (now broken) already test for this case, see https://github.com/jfryman/puppet-nginx/blob/master/spec/defines/resource_vhost_spec.rb#L300 for the test which will check for a correctly separated ssl and default.

This passed just fine before the merge of this PR because listen_options is already prepended by a space in the template.

@jfryman
Copy link
Contributor

jfryman commented Jan 2, 2014

Ah, yes it does, and now it's double-whitespaced. Hmmm...

@andreyev is there a scenario where you're running into this behavior?

@3flex 3flex mentioned this pull request Jan 2, 2014
jfryman pushed a commit that referenced this pull request Jan 2, 2014
@andreyev
Copy link
Author

andreyev commented Jan 2, 2014

Well, which information I should provide to help you to help me? ;-)

I think that any value I pass to 'listen_options' is 'joined' to ssl if it's true. I'm using this manifest:

  nginx::resource::vhost { '_':
    ensure          => present,
    listen_options  => 'default',
    proxy           => "http://localhost:8080",
    ssl             => true,
    ssl_cert        => "somefile.pem",
    ssl_key         => "somefile.key",
  }

Which generate this config file (on /tmp/nginx.d/_-700-ssl):

server {
  listen       *:443 ssldefault;

  server_name  _;

  ssl on;

  ssl_certificate           /etc/nginx/_.crt;
  ssl_certificate_key       /etc/nginx/_.key;
  ssl_session_cache         shared:SSL:10m;
  ssl_session_timeout       5m;
  ssl_protocols             SSLv3 TLSv1 TLSv1.1 TLSv1.2;
  ssl_ciphers               HIGH:!aNULL:!MD5;
  ssl_prefer_server_ciphers on;

  access_log            /var/log/nginx/ssl-_.access.log;
  error_log             /var/log/nginx/ssl-_.error.log;

I've installed jfryman/nginx module's which librarian-puppet:

# librarian-puppet show --verbose
[Librarian] Ruby Version: 1.8.7
[Librarian] Ruby Platform: x86_64-linux
[Librarian] Rubygems Version: 1.3.7
[Librarian] Librarian Version: 0.0.24
[Librarian] Librarian Adapter: puppet
[Librarian] Project: /mnt/cds/puppet
[Librarian] Specfile: Puppetfile
[Librarian] Lockfile: Puppetfile.lock
[Librarian] Git: /usr/bin/git
[Librarian] Git Version: git version 1.7.1
[Librarian] Git Environment Variables:
[Librarian]   (empty)
jfryman/nginx (0.0.6)
maestrodev/maven (1.1.9)
maestrodev/wget (1.3.1)
puppetlabs/apt (1.4.0)
puppetlabs/mysql (2.1.0)
puppetlabs/stdlib (4.1.0)

And running this on a CentOS 6.4.

Anything else?

TIA!!!

@3flex
Copy link
Contributor

3flex commented Jan 2, 2014

You have version 0.0.6 installed which indeed does have that problem, since fixed in master.

If you want to use the Forge releases and avoid this bug @jfryman will need to push a new version.

@jfryman
Copy link
Contributor

jfryman commented Jan 2, 2014

That's an easy fix. I think we're due for a release anyway. Lots of stuff went in over the holiday.

@jfryman
Copy link
Contributor

jfryman commented Jan 2, 2014

@andreyev
Copy link
Author

andreyev commented Jan 3, 2014

Great!! Thanks @jfryman and @3flex !!

grooverdan pushed a commit to openquery/puppet-nginx that referenced this pull request Mar 4, 2014
cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Oct 23, 2017
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

Successfully merging this pull request may close these issues.

3 participants