TLS option changes#901
Conversation
Replace `ssl_disabled_ciphers` and `tls_disabled_versions` with `tls_ciphers` and `tls_min_version`. A companion to theforeman/smart-proxy@67f34e2
|
Looks like GHAs are having a moment. |
| Array[String] $trusted_hosts = $foreman_proxy::params::trusted_hosts, | ||
| Array[String] $ssl_disabled_ciphers = [], | ||
| Array[String] $tls_disabled_versions = [], | ||
| Optional[String] $tls_ciphers = undef, |
There was a problem hiding this comment.
Just out of curiosity: how do we deal with this during upgrades?..
There was a problem hiding this comment.
I added a changelog entry marking this as a breaking change. A bit heavy handed, but better than nothing.
I could also open a PR against the installer to add a migration to remove the old value from the stored answers, but I'm not sure if that's necessary.
There was a problem hiding this comment.
I added a changelog entry marking this as a breaking change. A bit heavy handed, but better than nothing.
I'd also suggest a changelog entry in https://github.com/theforeman/foreman-documentation/blob/master/guides/doc-Release_Notes/topics/foreman.adoc. Those are a bit more visible to users.
I could also open a PR against the installer to add a migration to remove the old value from the stored answers, but I'm not sure if that's necessary.
IIRC the installer (or really, Kafo) automatically removes keys that no longer exist. The only reason you'd need a migration is to convert the old value to the new. I don't think it's really feasible to do so and an explicit release note is IMHO good enough.
There was a problem hiding this comment.
The PR to foreman-documentation is up: theforeman/foreman-documentation#5067
| ## [31.0.0](https://github.com/theforeman/puppet-foreman_proxy/tree/31.0.0) (unreleased) | ||
|
|
||
| **Breaking changes:** | ||
|
|
||
| - Replace `ssl_disabled_ciphers` and `tls_disabled_versions` with `tls_ciphers` and `tls_min_version`. [\#901](https://github.com/theforeman/puppet-foreman_proxy/pull/901) ([adamruzicka](https://github.com/adamruzicka)) |
There was a problem hiding this comment.
No need to do this. It's automatically generated on release based on the merged PRs. I already added the backwards-incompatible label which puts it in the right category.
There was a problem hiding this comment.
I've added a revert commit for this change
This reverts commit 030a00b.
pr-processor
Bot
added
Needs re-review
and removed
Waiting on contributor
Needs re-review
labels
4 participants
Replace
ssl_disabled_ciphersandtls_disabled_versionswithtls_ciphersandtls_min_version.A companion to theforeman/smart-proxy#947