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

Refactors (ready for review) #81

Closed
wants to merge 11 commits into from
Closed
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
2 changes: 2 additions & 0 deletions .sync.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
---
spec/spec_helper.rb:
hiera_config: spec/fixtures/hiera.yaml
coverage_report: true
minimum_code_coverage_percentage: 100
10 changes: 10 additions & 0 deletions data/os/AIX.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
ca_cert::supported: true
ca_cert::trusted_cert_dir: '/var/ssl/certs'
ca_cert::update_cmd: '/usr/bin/c_rehash'
ca_cert::cert_dir_group: 'system'
ca_cert::cert_dir_mode: '0755'
ca_cert::package_name: 'ca-certificates'
ca_cert::ca::ca_file_group: 'system'
ca_cert::ca::ca_file_mode: '0644'
ca_cert::ca::ca_file_extension: 'crt'
11 changes: 11 additions & 0 deletions data/os/Archlinux.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
ca_cert::supported: true
ca_cert::trusted_cert_dir: '/etc/ca-certificates/trust-source/anchors/'
ca_cert::distrusted_cert_dir: '/etc/ca-certificates/trust-source/blacklist'
ca_cert::update_cmd: 'trust extract-compat'
ca_cert::cert_dir_group: 'root'
ca_cert::cert_dir_mode: '0755'
ca_cert::package_name: 'ca-certificates'
ca_cert::ca::ca_file_group: 'root'
ca_cert::ca::ca_file_mode: '0644'
ca_cert::ca::ca_file_extension: 'crt'
10 changes: 10 additions & 0 deletions data/os/Debian.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
ca_cert::supported: true
ca_cert::trusted_cert_dir: '/usr/local/share/ca-certificates'
ca_cert::update_cmd: 'update-ca-certificates'
ca_cert::cert_dir_group: 'staff'
ca_cert::cert_dir_mode: '2665'
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does this default come from?
On a fresh install of ca-certificates in amd/debian:10,amd/debian:11 and amd/debian:12 containers the default is

root@60422e36a680:/# ls -l /usr/local/share/
total 0
drwxr-xr-x. 2 root root 6 May  9 14:06 ca-certificates

ca_cert::package_name: 'ca-certificates'
ca_cert::ca::ca_file_group: 'root'
ca_cert::ca::ca_file_mode: '0444'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not 0644 as default? Fresh debian container once more:
No files at all in /usr/local/share/ca-certificates and certificates in /usr/share/ca-certificates have 0644

root@b3059a7e2497:/# ls -l /usr/share/ca-certificates/mozilla/
total 560
-rw-r--r--. 1 root root 2772 Mar 11  2023  ACCVRAIZ1.crt
-rw-r--r--. 1 root root 1972 Mar 11  2023  AC_RAIZ_FNMT-RCM.crt
-rw-r--r--. 1 root root  904 Mar 11  2023  AC_RAIZ_FNMT-RCM_SERVIDORES_SEGUROS.crt

Copy link
Member

Choose a reason for hiding this comment

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

/usr/local/share/ca-certificates is the right place for certs installed by this module. See the FILES section of man update-ca-certificates.

I'd be fine with 0644 or 0444, but I think we often use 0444 as a way to show that the files are puppet-managed and shouldn't be manually edited.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was just referring to /usr/share/ca-certificates because that's provided by the ca-certificates package with mode 0644 as an argument for using the same when adding files to /usr/local/share/ca-certificates :)

ca_cert::ca::ca_file_extension: 'crt'
10 changes: 10 additions & 0 deletions data/os/RedHat.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
ca_cert::supported: true
ca_cert::trusted_cert_dir: '/etc/pki/ca-trust/source/anchors'
ca_cert::distrusted_cert_dir: '/etc/pki/ca-trust/source/blacklist'
ca_cert::update_cmd: 'update-ca-trust extract'
ca_cert::cert_dir_group: 'root'
ca_cert::cert_dir_mode: '0755'
ca_cert::ca::ca_file_group: 'root'
ca_cert::ca::ca_file_mode: '0644'
ca_cert::ca::ca_file_extension: 'crt'
9 changes: 9 additions & 0 deletions data/os/Solaris.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
ca_cert::trusted_cert_dir: '/etc/certs/CA/'
ca_cert::update_cmd: '/usr/sbin/svcadm restart /system/ca-certificates'
ca_cert::cert_dir_group: 'sys'
ca_cert::cert_dir_mode: '0755'
ca_cert::package_name: 'ca-certificates'
ca_cert::ca::ca_file_group: 'root'
ca_cert::ca::ca_file_mode: '0444'
ca_cert::ca::ca_file_extension: 'pem'
2 changes: 2 additions & 0 deletions data/os/Solaris/11.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
ca_cert::supported: true
11 changes: 11 additions & 0 deletions data/os/Suse.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
ca_cert::supported: true
ca_cert::trusted_cert_dir: '/etc/pki/trust/anchors'
ca_cert::distrusted_cert_dir: '/etc/pki/trust/blacklist'
ca_cert::update_cmd: 'update-ca-certificates'
ca_cert::cert_dir_group: 'root'
ca_cert::cert_dir_mode: '0755'
ca_cert::package_name: 'ca-certificates'
ca_cert::ca::ca_file_group: 'root'
ca_cert::ca::ca_file_mode: '0644'
ca_cert::ca::ca_file_extension: 'crt'
5 changes: 5 additions & 0 deletions data/os/Suse/10.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
ca_cert::trusted_cert_dir: '/etc/ssl/certs'
ca_cert::update_cmd: 'c_rehash'
ca_cert::package_name: 'openssl-certs'
ca_cert::ca::ca_file_extension: 'pem'
5 changes: 5 additions & 0 deletions data/os/Suse/11.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
ca_cert::trusted_cert_dir: '/etc/ssl/certs'
ca_cert::update_cmd: 'c_rehash'
ca_cert::package_name: 'openssl-certs'
ca_cert::ca::ca_file_extension: 'pem'
10 changes: 10 additions & 0 deletions data/os/Ubuntu.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
ca_cert::supported: true
ca_cert::trusted_cert_dir: '/usr/local/share/ca-certificates'
ca_cert::update_cmd: 'update-ca-certificates'
ca_cert::cert_dir_group: 'staff'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why staff as default? The ca-certificates package creates this directory with group root ...

root@cbe1f3cd0871:/# ls -l /usr/local/share/
total 0
drwxr-xr-x. 2 root root 6 May  9 17:31 ca-certificates

ca_cert::cert_dir_mode: '0755'
ca_cert::package_name: 'ca-certificates'
ca_cert::ca::ca_file_group: 'root'
ca_cert::ca::ca_file_mode: '0444'
Copy link
Contributor

Choose a reason for hiding this comment

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

Same reasoning as for Debian. See no reason why this should default to 0444 ..

ca_cert::ca::ca_file_extension: 'crt'
21 changes: 21 additions & 0 deletions hiera.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
version: 5

defaults: # Used for any hierarchy level that omits these keys.
datadir: data # This path is relative to hiera.yaml's directory.
data_hash: yaml_data # Use the built-in YAML backend.

hierarchy:
- name: "osfamily/major release"
paths:
# Used to distinguish between Debian and Ubuntu
- "os/%{facts.os.name}/%{facts.os.release.major}.yaml"
- "os/%{facts.os.family}/%{facts.os.release.major}.yaml"
# Used for Solaris
- "os/%{facts.os.family}/%{facts.kernelrelease}.yaml"
- name: "osfamily"
paths:
- "os/%{facts.os.name}.yaml"
- "os/%{facts.os.family}.yaml"
- name: 'common'
path: 'common.yaml'
103 changes: 49 additions & 54 deletions manifests/ca.pp
Original file line number Diff line number Diff line change
@@ -1,69 +1,64 @@
# ca.pp
# @summary
# Manage a CA Certificate on a system. This cannot manage pre-installed
# operating system CAs.
#
# Manage a CA Certificate on a system. This cannot manage pre-installed
# operating system CAs.
# @example
# ca_cert::ca { 'globalsign_org_intermediate':
# source => 'http://secure.globalsign.com/cacert/gsorganizationvalsha2g2r1.crt',
# }
#
# === Parameters
# @author
# Phil Fenstermacher <phillip.fenstermacher@gmail.com>
#
# [*ca_text*]
# @param ca_text
# The text of the CA certificate to install. Required if text is the source
# (default). If a different source is specified this parameter is ignored.
# [*source*]
#
# @param source
# Where the CA certificate should be retrieved from. text, http, https, ftp,
# file, and puppet protocols/sources are supported. If text, then the ca_text parameter
# is also required. Defaults to text.
# [*ensure*]
#
# @param ensure
# Whether or not the CA certificate should be on a system or not. Valid
# values are trusted, present, distrusted, and absent. Note: untrusted is
# not supported on Debian based systems - using it will log a warning
# and treat it the same as absent. (defaults to trusted)
# [*verify_https_cert*]
# and treat it the same as absent. (defaults to trusted).
#
# @param verify_https_cert
# When retrieving a certificate whether or not to validate the CA of the
# source. (defaults to true)
# [*checksum*]
#
# @param checksum
# The checksum of the file. (defaults to undef)
# [*checksum_type*]
#
# @param checksum_type
# The type of file checksum. (defauts to undef)
# [*ca_file_group*]
#
# @param ca_file_group
# The installed CA certificate's POSIX group permissions. This uses
# the same syntax as Puppet's native file resource's "group" parameter.
# (defaults to 'root' with the exeption of AIX which defaults to 'system')
# [*ca_file_mode*]
#
# @param ca_file_mode
# The installed CA certificate's POSIX filesystem permissions. This uses
# the same syntax as Puppet's native file resource's "mode" parameter.
# (defaults to '0444', i.e. world-readable)
#
# === Examples
# @param ca_file_extension
# File extenstion for the certificate.
#
# ca_cert::ca { 'globalsign_org_intermediate':
# source => 'http://secure.globalsign.com/cacert/gsorganizationvalsha2g2r1.crt',
# }
define ca_cert::ca (
Optional[String] $ca_text = undef,
String $source = 'text',
String $ensure = 'trusted',
Boolean $verify_https_cert = true,
Optional[String] $checksum = undef,
Optional[String] $ca_text = undef,
String $source = 'text',
String $ensure = 'trusted',
Boolean $verify_https_cert = true,
Optional[String] $checksum = undef,
Optional[String[1]] $checksum_type = undef,
Optional[String] $ca_file_group = undef,
Optional[String] $ca_file_mode = undef,
String[1] $ca_file_group = lookup('ca_cert::ca::ca_file_group'),
String[1] $ca_file_mode = lookup('ca_cert::ca::ca_file_mode'),
String[1] $ca_file_extension = lookup('ca_cert::ca::ca_file_extension'),
) {
include ca_cert::params
include ca_cert::update
require ca_cert::enable

if $ca_file_group == undef {
$file_group = $ca_cert::params::ca_file_group
} else {
$file_group = $ca_file_group
}

if $ca_file_mode == undef {
$file_mode = $ca_cert::params::ca_file_mode
} else {
$file_mode = $ca_file_mode
}

if ($ensure == 'trusted' or $ensure == 'distrusted') and $source == 'text' and !$ca_text {
fail('ca_text is required if source is set to text')
}
Expand Down Expand Up @@ -94,11 +89,11 @@
}

# Determine Full Resource Name
$resource_name = "${name}.${ca_cert::params::ca_file_extension}"
$resource_name = "${name}.${ca_file_extension}"

$ca_cert = $adjusted_ensure ? {
'distrusted' => "${ca_cert::params::distrusted_cert_dir}/${resource_name}",
default => "${ca_cert::params::trusted_cert_dir}/${resource_name}",
'distrusted' => "${ca_cert::distrusted_cert_dir}/${resource_name}",
default => "${ca_cert::trusted_cert_dir}/${resource_name}",
}

case $adjusted_ensure {
Expand All @@ -112,9 +107,9 @@
source => $source,
path => $ca_cert,
owner => 'root',
group => $file_group,
mode => $file_mode,
notify => Class['ca_cert::update'],
group => $ca_file_group,
mode => $ca_file_mode,
notify => Exec['ca_cert_update'],
}
}
'ftp', 'https', 'http': {
Expand All @@ -124,7 +119,7 @@
checksum => $checksum,
checksum_type => $checksum_type,
allow_insecure => !$verify_https_cert,
notify => Class['ca_cert::update'],
notify => Exec['ca_cert_update'],
}
}
'file': {
Expand All @@ -134,9 +129,9 @@
source => $source_path,
path => $ca_cert,
owner => 'root',
group => $file_group,
mode => $file_mode,
notify => Class['ca_cert::update'],
group => $ca_file_group,
mode => $ca_file_mode,
notify => Exec['ca_cert_update'],
}
}
'text': {
Expand All @@ -145,9 +140,9 @@
content => $ca_text,
path => $ca_cert,
owner => 'root',
group => $file_group,
mode => $file_mode,
notify => Class['ca_cert::update'],
group => $ca_file_group,
mode => $ca_file_mode,
notify => Exec['ca_cert_update'],
}
}
default: {
Expand All @@ -158,7 +153,7 @@
'absent': {
file { $ca_cert:
ensure => absent,
notify => Class['ca_cert::update'],
notify => Exec['ca_cert_update'],
}
}
default: {
Expand Down
23 changes: 0 additions & 23 deletions manifests/enable.pp

This file was deleted.

Loading