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

SSLProxyVerify #1311

Merged
merged 1 commit into from Dec 26, 2015
Merged

SSLProxyVerify #1311

merged 1 commit into from Dec 26, 2015

Conversation

occelebi
Copy link

I've added SSLProxyVerify directive in mod ssl.

@bmjen
Copy link
Contributor

bmjen commented Dec 21, 2015

@occelebi can you please make the updates as suggested by @igalic in the applied labels? Feel free to ask if any assistance or explanation is needed.

@occelebi
Copy link
Author

@bmjen I've added validation test and docs.

@@ -154,6 +154,7 @@
'ssl_verify_depth' => '3',
'ssl_options' => '+ExportCertData',
'ssl_openssl_conf_cmd' => 'DHParameters "foo.pem"',
'ssl_proxy_verify' => 'require',
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the template checks for existence of @ssl_proxy_verify before adding the line to the config, should this default be to undef? By setting a default, this means that the SSLProxyVerify line will always be present in the default config. Is that what we want?

cc/ @igalic

Copy link
Author

Choose a reason for hiding this comment

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

@bmjen What would you suggest ? I might remove it. It doesn't bother me at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops sorry @occelebi . I misread the diff and didn't see this was in the test and not manifest. This is fine the way it is! Thanks!

bmjen added a commit that referenced this pull request Dec 26, 2015
@bmjen bmjen merged commit ca5e7e5 into puppetlabs:master Dec 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants