(CAT-1281) - Support to add cipher with respective ssl protocol #2440
Conversation
Ramesh7
force-pushed
the
CAT-1281-support-for-adding-cipher-for-protocols
branch
5 times, most recently
from
7d60a14 to
3d8e2e0
Compare
ekohl
left a comment
ekohl
left a comment
There was a problem hiding this comment.
I did not know this was even an Apache option.
Should it also modify the cipher parameter in apache::vhost?
manifests/mod/ssl.pp
Outdated
| Optional[Stdlib::Absolutepath] $ssl_key = undef, | ||
| Optional[Stdlib::Absolutepath] $ssl_ca = undef, | ||
| String $ssl_cipher = $apache::params::ssl_cipher, | ||
| Variant[String, Array, Hash] $ssl_cipher = $apache::params::ssl_cipher, |
There was a problem hiding this comment.
Why Array? I don't see code in the template to handle that. And can we further tighten Hash to Hash[String[1], String[1]]?
There was a problem hiding this comment.
Thanks for your comments, I wasn't aware that we can even define variable that level in puppet.
There was a problem hiding this comment.
Puppet data types are surprisingly powerful and I like them a lot.
Ramesh7
force-pushed
the
CAT-1281-support-for-adding-cipher-for-protocols
branch
2 times, most recently
from
1a84d7b to
5855384
Compare
I have already covered up for vhost here https://github.com/puppetlabs/puppetlabs-apache/pull/2440/files#diff-2842c1252a08a185c551b13fdab39118416a7d296217322d5ebb921c24899397R18-R21 |
Ramesh7
force-pushed
the
CAT-1281-support-for-adding-cipher-for-protocols
branch
from
5855384 to
9d00180
Compare
Ramesh7
force-pushed
the
CAT-1281-support-for-adding-cipher-for-protocols
branch
from
9d00180 to
8acbad4
Compare
smortex
left a comment
smortex
left a comment
There was a problem hiding this comment.
Nice! I was not aware of this. I added in-line comments for some improvements I see.
smortex
left a comment
smortex
left a comment
There was a problem hiding this comment.
As you said :-) With this fixed, LGTM!
Summary
Adding support for enable user to pass the different ciphers for different protocols.
Additional Context
Related Issues (if any)
Checklist
puppet apply)