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

Fixes #34684 - install pulp-cli #252

Merged
merged 5 commits into from
Apr 4, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
92 changes: 92 additions & 0 deletions manifests/cli.pp
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# = Pulpcore command line interface
#
# This class installs the Pulpcore command line interface (CLI).
#
# === Parameters:
#
# $pulpcore_url:: URL on which Pulpcore runs
Copy link
Member Author

Choose a reason for hiding this comment

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

in other places (foreman_proxy, foreman_proxy_content) we call this pulpcore_api_url.

pulp-cli itself calls it base_url.

🚲 shed?

Copy link
Member

Choose a reason for hiding this comment

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

Given it's not exposed to users, I don't care that much. Let's start with this.

#
# $username:: Username for authentication
#
# $password:: Password for authentication
#
# $cert:: Client certificate for authentication
#
# $key:: Client key for authentication
#
# === Advanced parameters:
#
# $manage_root_config:: Whether to manage /root/.config/pulp configuration.
#
# $api_root:: Absolute API base path on server (not including 'api/v3/')
#
# $verify_ssl:: Verify SSL connection
#
# $dry_run:: Trace commands without performing any unsafe HTTP calls
#
# $version:: pulp-cli package version, it's passed to ensure parameter of package resource
# can be set to specific version number, 'latest', 'present' etc.
#
class pulpcore::cli (
Optional[Stdlib::HTTPUrl] $pulpcore_url = undef,
Copy link
Member Author

Choose a reason for hiding this comment

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

should this default to https://${fqdn}?

Copy link
Member

Choose a reason for hiding this comment

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

It's not exposed to users so I think this is fine.

String $version = 'installed',
Optional[String] $username = undef,
Optional[String] $password = undef,
Optional[String] $cert = undef,
Optional[String] $key = undef,
Optional[String] $api_root = undef,
Boolean $verify_ssl = true,
Boolean $dry_run = true,
Boolean $manage_root_config = true,
) {
package { 'pulp-cli':
ensure => $version,
}
-> file { '/etc/pulp/cli.toml':
evgeni marked this conversation as resolved.
Show resolved Hide resolved
ensure => file,
owner => 'root',
group => 'root',
mode => '0644',
evgeni marked this conversation as resolved.
Show resolved Hide resolved
content => epp(
'pulpcore/cli-config.toml.epp',
{
base_url => $pulpcore_url,
api_root => $api_root,
verify_ssl => $verify_ssl,
dry_run => $dry_run,
}
),
}

if $manage_root_config and (($username and $password) or ($cert and $key)) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we have some validates above that throw a clean error if for either combo one is supplied and not the other?

Copy link
Member Author

Choose a reason for hiding this comment

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

So in pulp-cli the validation is a bit more involved: https://github.com/pulp/pulp-cli/blob/aa50bc63e949e7f413bce6bc10d323d119e61b09/pulpcore/cli/common/openapi.py#L49-L62

I think for this module, it would be sufficient to check:

  • (username OR password) AND (cert OR key) → error
  • password AND !username → error
  • key AND !cert → error

Everything else is imho valid (like setting username via config, but using CLI --password for the password, or providing a cert/key combo in the cert parameter).

file { '/root/.config':
ensure => directory,
owner => 'root',
group => 'root',
mode => '0600',
}
file { '/root/.config/pulp':
ensure => directory,
owner => 'root',
group => 'root',
mode => '0600',
}
file { '/root/.config/pulp/cli.toml':
ensure => file,
owner => 'root',
group => 'root',
mode => '0600',
content => epp(
'pulpcore/cli-config.toml.epp',
{
username => $username,
password => $password,
cert => $cert,
key => $key,
}
),
}
}

Anchor <| title == 'pulpcore::repo' |> ~> Package <| tag == 'pulpcore::cli' |>
}
88 changes: 88 additions & 0 deletions spec/acceptance/cli_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
require 'spec_helper_acceptance'

describe 'installation of server with cli' do
it_behaves_like 'an idempotent resource' do
let(:manifest) do
<<-PUPPET
include pulpcore
ekohl marked this conversation as resolved.
Show resolved Hide resolved
class { 'pulpcore::cli':
pulpcore_url => "https://${facts['fqdn']}/",
cert => "/etc/pulpcore-certs/client-cert.pem",
key => "/etc/pulpcore-certs/client-key.pem",
}
PUPPET
end
end

include_examples 'the default pulpcore application'

describe command("pulp config validate --location /etc/pulp/cli.toml") do
its(:exit_status) { is_expected.to eq 0 }
its(:stdout) { is_expected.to match(/valid pulp-cli config/) }
its(:stderr) { is_expected.to eq '' }
end

describe command("REQUESTS_CA_BUNDLE=/etc/pulpcore-certs/ca-cert.pem pulp status") do
its(:exit_status) { is_expected.to eq 0 }
its(:stdout) { is_expected.to match(/versions/) }
its(:stderr) { is_expected.not_to match(/Error/) }
# currently this contains a warning:
# SubjectAltNameWarning: Certificate for centos7-64.example.com has no `subjectAltName`, falling back to check for a `commonName` for now. This feature is being removed by major browsers and deprecated by RFC 2818.
unless %w[centos redhat].include?(os[:family]) && os[:release].to_i == 7
its(:stderr) { is_expected.to eq '' }
end
end

describe command("REQUESTS_CA_BUNDLE=/etc/pulpcore-certs/ca-cert.pem pulp user list") do
its(:exit_status) { is_expected.to eq 0 }
its(:stdout) { is_expected.to match(/admin/) }
its(:stderr) { is_expected.not_to match(/Error/) }
# currently this contains a warning:
# SubjectAltNameWarning: Certificate for centos7-64.example.com has no `subjectAltName`, falling back to check for a `commonName` for now. This feature is being removed by major browsers and deprecated by RFC 2818.
unless %w[centos redhat].include?(os[:family]) && os[:release].to_i == 7
its(:stderr) { is_expected.to eq '' }
end
end
end

describe 'installation of cli only' do
it_behaves_like 'an idempotent resource' do
let(:manifest) do
<<-PUPPET
include pulpcore::cli
PUPPET
end
end

describe command("pulp config validate --location /etc/pulp/cli.toml") do
its(:exit_status) { is_expected.to eq 0 }
its(:stdout) { is_expected.to match(/valid pulp-cli config/) }
its(:stderr) { is_expected.to eq '' }
end
end

describe 'installation of cli only with password' do
it_behaves_like 'an idempotent resource' do
let(:manifest) do
<<-PUPPET
class { 'pulpcore::cli':
pulpcore_url => "https://${facts['fqdn']}/",
username => "admin",
password => "changeme",
}
PUPPET
end
end

describe command("pulp config validate --location /etc/pulp/cli.toml") do
its(:exit_status) { is_expected.to eq 0 }
its(:stdout) { is_expected.to match(/valid pulp-cli config/) }
its(:stderr) { is_expected.to eq '' }
end

describe command("pulp config validate --location /root/.config/pulp/cli.toml") do
its(:exit_status) { is_expected.to eq 0 }
its(:stdout) { is_expected.to match(/valid pulp-cli config/) }
its(:stderr) { is_expected.to eq '' }
end
end
13 changes: 13 additions & 0 deletions spec/classes/cli_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
require 'spec_helper'

describe 'pulpcore::cli' do
on_supported_os.each do |os, os_facts|
context "on #{os}" do
let(:facts) { os_facts }
let(:pre_condition) { 'include pulpcore::cli' }

it { is_expected.to compile.with_all_deps }
it { is_expected.to contain_package('pulp-cli') }
Comment on lines +9 to +10
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I should add some asserts on the config content too?

end
end
end
9 changes: 8 additions & 1 deletion spec/setup_acceptance_node.pp
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,20 @@
$client_cert = "${directory}/client-cert.pem"
$client_key = "${directory}/client-key.pem"

# EL7 lacks openssl -addext
if $facts['os']['release']['major'] == '7' {
$ca_cmd = "openssl req -nodes -x509 -newkey rsa:2048 -subj '/CN=${facts['networking']['fqdn']}' -keyout '${ca_key}' -out '${ca_cert}' -days 365"
} else {
$ca_cmd = "openssl req -nodes -x509 -newkey rsa:2048 -subj '/CN=${facts['networking']['fqdn']}' -addext 'subjectAltName = DNS:${facts['networking']['fqdn']}' -keyout '${ca_key}' -out '${ca_cert}' -days 365"
}

exec { 'Create certificate directory':
command => "mkdir -p ${directory}",
path => ['/bin', '/usr/bin'],
creates => $directory,
}
-> exec { 'Generate certificate':
command => "openssl req -nodes -x509 -newkey rsa:2048 -subj '/CN=${facts['networking']['fqdn']}' -keyout '${ca_key}' -out '${ca_cert}' -days 365",
command => $ca_cmd,
path => ['/bin', '/usr/bin'],
creates => $ca_cert,
logoutput => 'on_failure',
Expand Down
35 changes: 35 additions & 0 deletions templates/cli-config.toml.epp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<%- |
Optional[Stdlib::HTTPUrl] $base_url = undef,
Optional[String] $username = undef,
Optional[String] $password = undef,
Optional[String] $cert = undef,
Optional[String] $key = undef,
Optional[String] $api_root = undef,
Optional[Boolean] $verify_ssl = undef,
Optional[Boolean] $dry_run = undef,
| -%>
[cli]
<% if $base_url { -%>
base_url = "<%= $base_url %>"
<% } -%>
<% unless $verify_ssl =~ Undef { -%>
verify_ssl = <%= $verify_ssl %>
<% } -%>
<% unless $dry_run =~ Undef { -%>
dry_run = <%= $dry_run %>
<% } -%>
<% if $username { -%>
username = "<%= $username %>"
<% } -%>
<% if $password { -%>
password = "<%= $password %>"
<% } -%>
<% if $cert { -%>
cert = "<%= $cert %>"
<% } -%>
<% if $key { -%>
key = "<%= $key %>"
<% } -%>
<% if $api_root { -%>
api_root = "<%= $api_root %>"
<% } -%>