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

Change SSLProtocol in apache::vhost to be space separated #1216

Merged
merged 3 commits into from
Oct 13, 2015

Conversation

bmfurtado
Copy link

@@ -26,7 +26,7 @@
SSLProxyEngine On
<%- end -%>
<%- if @ssl_protocol -%>
SSLProtocol <%= @ssl_protocol %>
SSLProtocol <%= @ssl_protocol.compact.join(' ') %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd go as far as to make it

[@ssl_protocol].flatten.compact.join(' ')

especially since the tests fail because of

         Detail: undefined method `compact' for "SSLv2":String

@HelenCampbell
Copy link
Contributor

Hello @bmfurtado ,
Well spotted on this! Thank you for your work and @igalic for your input to help get the tests passing :)
It would be awesome if you could put a test in place to cover the array input and the readme could also be updated to mention that a single space separated string also works. After that I'll be happy to merge this in.

@bmfurtado
Copy link
Author

@HelenCampbell I've added an acceptance test and updated the docs, is this ok?

@bmfurtado
Copy link
Author

Disclaimer: For reasons I haven't had time to investigate I can't run the tests and it doesn't look like Travis runs acceptance tests (for some reason?) so I'm not really sure this works :)

igalic added a commit that referenced this pull request Oct 13, 2015
Change SSLProtocol in apache::vhost to be space separated
@igalic igalic merged commit 3a1a4e4 into puppetlabs:master Oct 13, 2015
@igalic
Copy link
Contributor

igalic commented Oct 13, 2015

Let's see :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants