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

add foreman_proxy::plugin::remote_execution::ssh::ssh_log_level param #739

Merged
merged 1 commit into from
Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions examples/remote_execution_ssh-ssh_log_level.pp
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
include foreman_proxy
class { 'foreman_proxy::plugin::remote_execution::ssh':
ssh_log_level => 'debug',
}
3 changes: 3 additions & 0 deletions manifests/plugin/remote_execution/ssh.pp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
#
# $remote_working_dir:: Remote working directory on clients
#
# $ssh_log_level:: Configure ssh client LogLevel
#
# === Advanced parameters:
#
# $enabled:: Enables/disables the plugin
Expand All @@ -41,6 +43,7 @@
Stdlib::Absolutepath $remote_working_dir = '/var/tmp',
Boolean $ssh_kerberos_auth = false,
Enum['ssh', 'ssh-async'] $mode = 'ssh',
Optional[Foreman_proxy::Sshloglevel] $ssh_log_level = undef,
) {
$ssh_identity_path = "${ssh_identity_dir}/${ssh_identity_file}"

Expand Down
16 changes: 16 additions & 0 deletions spec/acceptance/remote_execution_ssh_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,20 @@
include_examples 'the example', 'remote_execution_ssh.pp'

it_behaves_like 'the default foreman proxy application'

describe file('/etc/foreman-proxy/settings.d/remote_execution_ssh.yml') do
its(:content) { is_expected.to_not match %r{:ssh_log_level:} }
end
end

describe 'Scenario: install foreman-proxy with remote_execution ssh plugin and ssh_log_level param' do
before(:context) { purge_installed_packages }

include_examples 'the example', 'remote_execution_ssh-ssh_log_level.pp'

it_behaves_like 'the default foreman proxy application'

describe file('/etc/foreman-proxy/settings.d/remote_execution_ssh.yml') do
its(:content) { is_expected.to match %r{:ssh_log_level: debug} }
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
with_content(/^:enabled: https/).
with_content(%r{:ssh_identity_key_file: /var/lib/foreman-proxy/ssh/id_rsa_foreman_proxy}).
with_content(%r{:kerberos_auth: false}).
without_content(%r{:ssh_log_level:}).
with({
:ensure => 'file',
:owner => 'root',
Expand Down Expand Up @@ -42,6 +43,7 @@
:install_key => true,
:ssh_kerberos_auth => true,
:mode => 'ssh-async',
:ssh_log_level => 'debug',
} end

it { should contain_class('foreman_proxy::plugin::dynflow') }
Expand All @@ -55,6 +57,7 @@
with_content(%r{:remote_working_dir: /tmp}).
with_content(%r{:kerberos_auth: true}).
with_content(%r{:mode: ssh-async}).
with_content(%r{:ssh_log_level: debug}).
with({
:ensure => 'file',
:owner => 'root',
Expand Down
16 changes: 16 additions & 0 deletions spec/type_aliases/sshloglevel_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
require 'spec_helper'

describe 'Foreman_proxy::Sshloglevel' do
# inconsistent with ssh_config(5)
# see https://github.com/theforeman/smart_proxy_remote_execution_ssh/blob/master/lib/smart_proxy_remote_execution_ssh/plugin.rb#L3
known_log_levels = %w[
debug
info
error
fatal
]
it { is_expected.to allow_values(*known_log_levels) }
it { is_expected.not_to allow_value(nil) }
it { is_expected.not_to allow_value('all') }
it { is_expected.not_to allow_value('loud') }
end
3 changes: 3 additions & 0 deletions templates/plugin/remote_execution_ssh.yml.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
:local_working_dir: <%= scope.lookupvar('::foreman_proxy::plugin::remote_execution::ssh::local_working_dir') %>
:remote_working_dir: <%= scope.lookupvar('::foreman_proxy::plugin::remote_execution::ssh::remote_working_dir') %>
:kerberos_auth: <%= scope.lookupvar('::foreman_proxy::plugin::remote_execution::ssh::ssh_kerberos_auth') %>
<% ssh_log_level = scope.lookupvar('::foreman_proxy::plugin::remote_execution::ssh::ssh_log_level'); if ssh_log_level -%>
:ssh_log_level: <%= ssh_log_level %>
<% end -%>
Copy link
Member

Choose a reason for hiding this comment

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

Curious your thoughts on the if inclusion here versus setting a default value to the default from the plugin (:error) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a mild preference for unset parameters to not appear in the config file. At the time, I was planning to rework the gem to allow the ssh_config(5) values to be passed directly through. The gem doesn't allow DEBUG3 to be set and there is a check that prevents :ssh_log_level: debug from being used unless the entire foreman-proxy is also set to DEBUG. Now that it sounds like serious work in that gem is imminent I'm not sure it is worth the effort.

Copy link
Member

Choose a reason for hiding this comment

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

I have a mild preference for unset parameters to not appear in the config file

Sorry, what I was getting at is why not set it to :error rather than undef.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is the question I answered. The gem already has a hard coded default value for when the configuration isn't present in the foreman-proxy plugin config file.


# Whether to run remote execution jobs asynchronously
:mode: <%= scope.lookupvar("::foreman_proxy::plugin::remote_execution::ssh::mode") %>
3 changes: 3 additions & 0 deletions types/sshloglevel.pp
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# The possible openssh LogLevel values. Note that these are values allowed by the
# smart_proxy_remote_execution_ssh gem and are not consistent with the ssh_config(5) man page.
type Foreman_proxy::Sshloglevel = Enum['debug', 'info', 'error', 'fatal']