Refs #22165 - Add installer support for disabling hsts #614
Conversation
templates/settings.yaml.erb
Outdated
| @@ -30,6 +30,9 @@ | |||
| :ssl_ca_file: <%= scope.lookupvar("foreman::client_ssl_ca") %> | |||
| :ssl_priv_key: <%= scope.lookupvar("foreman::client_ssl_key") %> | |||
|
|
|||
| # HSTS setting | |||
| :disable_hsts: <%= scope.lookupvar("foreman::disable_hsts") %> | |||
There was a problem hiding this comment.
IMHO this is a badly named variable: it should always be positive (so enable_hsts`` defaulting to true```)
There was a problem hiding this comment.
Still hasn't been merged in core - see related PR (and request change there so noone merges it)
There was a problem hiding this comment.
I thought about this and I don't think we should change it. If we default to true here and make the name e.g. enable_hsts that will cause any users who haven't updated their settings.yaml with the new option to default to false, thus disabling HSTS for them.
There was a problem hiding this comment.
I disagree. If there's a hardcoded default to true then any user who hasn't updated their settings.yaml should get the hardcoded default (since it's not present) which means an enabled HSTS setup.
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
@ekohl updated to match the change in core |
|
@ekohl core has been merged, rebased this on master. |
manifests/init.pp
Outdated
| @@ -206,6 +206,9 @@ | |||
| # $jobs_service:: Name of the service for running the background Dynflow executor | |||
| # | |||
| # $dynflow_in_core:: Whether the Dynflow executor service is provided by Foreman or tasks | |||
| # | |||
| # $hsts_enabled:: Disable HSTS enforcement in https requests | |||
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
merged, thanks @tbrisker! |
No description provided.