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

BREAKING: Drop puppet 3 support. Replace validate_* functions with Puppet 4 data type validations #264

Merged
merged 2 commits into from
Feb 21, 2017
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
34 changes: 18 additions & 16 deletions manifests/artifactory.pp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# * url: artifactory download url filepath. NOTE: replaces server, port, url_path parameters.
# * server: artifactory server name (deprecated).
# * port: artifactory server port (deprecated).
# * url_path: artifactory file path http:://{server}:{port}/artifactory/{url_path} (deprecated).
# * url_path: artifactory file path http://{server}:{port}/artifactory/{url_path} (deprecated).
# * owner: file owner (see archive params for defaults).
# * group: file group (see archive params for defaults).
# * mode: file mode (see archive params for defaults).
Expand Down Expand Up @@ -43,20 +43,20 @@
# }
#
define archive::artifactory (
$path = $name,
$ensure = present,
$url = undef,
$server = undef,
$port = undef,
$url_path = undef,
$owner = undef,
$group = undef,
$mode = undef,
$archive_path = undef,
$extract = undef,
$extract_path = undef,
$creates = undef,
$cleanup = undef,
String $path = $name,
Enum['present', 'absent'] $ensure = present,
Optional[Pattern[/^https?:\/\//]] $url = undef,
Optional[String] $server = undef,
Optional[Integer] $port = undef,
Optional[String] $url_path = undef,
Optional[String] $owner = undef,
Optional[String] $group = undef,
Optional[String] $mode = undef,
Optional[Boolean] $extract = undef,
Optional[String] $extract_path = undef,
Optional[String] $creates = undef,
Optional[Boolean] $cleanup = undef,
Optional[Stdlib::Compat::Absolute_path] $archive_path = undef,
Copy link
Contributor

Choose a reason for hiding this comment

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

how about we use Tea::Absolutepath here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hum, why you want to add additional dependencies? The stdlib data types based on the tea types. Not?

Copy link

Choose a reason for hiding this comment

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

The ::compat:: types are specifically designed to match the horror that was the validate_ function that went before. There is Stdlib::Absolutepath to do better path checking.

Copy link
Contributor

Choose a reason for hiding this comment

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

so we should then, officially, deprecate Tea in favour for Stdlib… not that it was ever in widespread use to begin with…

i hope

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Improve standards instead build a own world. That why i'm doing this PR. :)

) {

include ::archive::params
Expand All @@ -67,7 +67,9 @@
$file_path = $path
}

validate_absolute_path($file_path)
if $file_path !~ Stdlib::Compat::Absolute_path {
fail("archive::artifactory[${name}]: \$name or \$archive_path must be an absolute path!") # lint:ignore:trailing_comma
}

if $url {
$file_url = $url
Expand Down
31 changes: 15 additions & 16 deletions manifests/download.pp
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,22 @@
# }
#
define archive::download (
$url,
$ensure = present,
$checksum = true,
$digest_url = undef,
$digest_string = undef,
$digest_type = 'md5', # bad default!
$timeout = 120, # ignored
$src_target = '/usr/src',
$allow_insecure = false, # ignored
$follow_redirects = false, # ignored (default)
$verbose = true, # ignored
$path = $::path, # ignored
$proxy_server = undef,
$user = undef,
String $url,
Enum['present', 'absent'] $ensure = present,
Boolean $checksum = true,
Optional[String] $digest_url = undef,
Optional[String] $digest_string = undef,
Optional[Enum['none', 'md5', 'sha1', 'sha2','sh256', 'sha384', 'sha512']] $digest_type = 'md5', # bad default!
Integer $timeout = 120, # ignored
Stdlib::Compat::Absolute_path $src_target = '/usr/src',
Boolean $allow_insecure = false, # ignored
Boolean $follow_redirects = false, # ignored (default)
Boolean $verbose = true, # ignored
String $path = $::path, # ignored
Optional[String] $proxy_server = undef,
Optional[String] $user = undef,
) {

$target = is_absolute_path($title) ? {
$target = ($title =~ Stdlib::Compat::Absolute_path) ? {
false => "${src_target}/${title}",
default => $title,
}
Expand Down
36 changes: 19 additions & 17 deletions manifests/go.pp
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
# download from go
define archive::go (
$server,
$port,
$url_path,
$md5_url_path,
$username,
$password,
$path = $name,
$owner = undef,
$group = undef,
$mode = undef,
$archive_path = undef,
$ensure = present,
$extract = undef,
$extract_path = undef,
$creates = undef,
$cleanup = undef,
String $server,
Integer $port,
String $url_path,
String $md5_url_path,
String $username,
String $password,
Enum['present', 'absent'] $ensure = present,
String $path = $name,
Optional[String] $owner = undef,
Optional[String] $group = undef,
Optional[String] $mode = undef,
Optional[Boolean] $extract = undef,
Optional[String] $extract_path = undef,
Optional[String] $creates = undef,
Optional[Boolean] $cleanup = undef,
Optional[Stdlib::Compat::Absolute_path] $archive_path = undef,
) {

include ::archive::params
Expand All @@ -26,7 +26,9 @@
$file_path = $path
}

validate_absolute_path($file_path)
if $file_path !~ Stdlib::Compat::Absolute_path {
fail("archive::go[${name}]: \$name or \$archive_path must be an absolute path!") # lint:ignore:trailing_comma
}

$go_url = "http://${server}:${port}"
$file_url = "${go_url}/${url_path}"
Expand Down
12 changes: 6 additions & 6 deletions manifests/init.pp
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
# }
#
class archive (
$seven_zip_name = $archive::params::seven_zip_name,
$seven_zip_provider = $archive::params::seven_zip_provider,
$seven_zip_source = undef,
$aws_cli_install = false,
Optional[String] $seven_zip_name = $archive::params::seven_zip_name,
Optional[String] $seven_zip_provider = $archive::params::seven_zip_provider,
Optional[String] $seven_zip_source = undef,
Boolean $aws_cli_install = false,
) inherits archive::params {

if $::osfamily == 'Windows' and !($seven_zip_provider in ['', undef]) {
if $facts['os']['family'] == 'Windows' and !($seven_zip_provider in ['', undef]) {
package { '7zip':
ensure => present,
name => $seven_zip_name,
Expand All @@ -38,7 +38,7 @@

if $aws_cli_install {
# TODO: Windows support.
if $::osfamily != 'Windows' {
if $facts['os']['family'] != 'Windows' {
# Using bundled install option:
# http://docs.aws.amazon.com/cli/latest/userguide/installing.html#install-bundle-other-os

Expand Down
57 changes: 26 additions & 31 deletions manifests/nexus.pp
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,30 @@
# }
#
define archive::nexus (
$url,
$gav,
$repository,
$ensure = present,
$checksum_type = 'md5',
$checksum_verify = true,
$packaging = 'jar',
$classifier = undef,
$extension = undef,
$username = undef,
$password = undef,
$user = undef,
$owner = undef,
$group = undef,
$mode = undef,
$extract = undef,
$extract_path = undef,
$extract_flags = undef,
$extract_command = undef,
$creates = undef,
$cleanup = undef,
$proxy_server = undef,
$proxy_type = undef,
$allow_insecure = undef,
String $url,
String $gav,
String $repository,
Enum['present', 'absent'] $ensure = present,
Enum['none', 'md5', 'sha1', 'sha2','sh256', 'sha384', 'sha512'] $checksum_type = 'md5',
Boolean $checksum_verify = true,
String $packaging = 'jar',
Optional[String] $classifier = undef,
Optional[String] $extension = undef,
Optional[String] $username = undef,
Optional[String] $password = undef,
Optional[String] $user = undef,
Optional[String] $owner = undef,
Optional[String] $group = undef,
Optional[String] $mode = undef,
Optional[Boolean] $extract = undef,
Optional[String] $extract_path = undef,
Optional[String] $extract_flags = undef,
Optional[String] $extract_command = undef,
Optional[String] $creates = undef,
Optional[Boolean] $cleanup = undef,
Optional[String] $proxy_server = undef,
Optional[String] $proxy_type = undef,
Optional[Boolean] $allow_insecure = undef,
) {

include ::archive::params
Expand All @@ -63,15 +63,11 @@
'c' => $classifier,
'e' => $extension,

}
}.filter |$keys, $values| { $values != undef }

$artifact_url = assemble_nexus_url($url, delete_undef_values($query_params))
$artifact_url = assemble_nexus_url($url, $query_params)
$checksum_url = regsubst($artifact_url, "p=${packaging}", "p=${packaging}.${checksum_type}")

if $allow_insecure != undef {
validate_bool($allow_insecure)
}

archive { $name:
ensure => $ensure,
source => $artifact_url,
Expand Down Expand Up @@ -103,5 +99,4 @@
mode => $file_mode,
require => Archive[$name],
}

}
2 changes: 1 addition & 1 deletion manifests/params.pp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# archive settings such as default user and file mode.
#
class archive::params {
case $::osfamily {
case $facts['os']['family'] {
'Windows': {
$path = $::archive_windir
$owner = 'S-1-5-32-544' # Adminstrators
Expand Down
8 changes: 4 additions & 4 deletions manifests/staging.pp
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
# backwards compatibility class for staging module.
#
class archive::staging (
$path = $archive::params::path,
$owner = $archive::params::owner,
$group = $archive::params::group,
$mode = $archive::params::mode,
String $path = $archive::params::path,
String $owner = $archive::params::owner,
String $group = $archive::params::group,
String $mode = $archive::params::mode,
) inherits archive::params {
include '::archive'

Expand Down
4 changes: 2 additions & 2 deletions metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
"requirements": [
{
"name": "puppet",
"version_requirement": ">= 3.8.7 < 5.0.0"
"version_requirement": ">=4.6.1 <5.0.0"
}
],
"name": "puppet-archive",
Expand All @@ -105,7 +105,7 @@
"dependencies": [
{
"name": "puppetlabs/stdlib",
"version_requirement": ">= 4.6.0 < 5.0.0"
"version_requirement": ">= 4.13.0 < 5.0.0"
}
]
}
8 changes: 4 additions & 4 deletions spec/classes/archive_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
context 'RHEL' do
let(:facts) do
{
osfamily: 'RedHat',
os: { family: 'RedHat' },
operatingsystem: 'RedHat',
puppetversion: '3.7.3'
puppetversion: '4.4.0'
}
end

Expand Down Expand Up @@ -35,7 +35,7 @@
describe 'Windows' do
let(:default_facts) do
{
osfamily: 'Windows',
os: { family: 'Windows' },
operatingsystem: 'Windows',
archive_windir: 'C:/staging'
}
Expand All @@ -44,7 +44,7 @@
context 'default 7zip chcolatey package' do
let(:facts) do
{
puppetversion: '3.7.3'
puppetversion: '4.4.0'
}.merge(default_facts)
end

Expand Down
6 changes: 3 additions & 3 deletions spec/classes/staging_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

describe 'archive::staging' do
context 'RHEL Puppet opensource' do
let(:facts) { { osfamily: 'RedHat', puppetversion: '3.7.3' } }
let(:facts) { { os: { family: 'RedHat' }, puppetversion: '4.4.0' } }

it { is_expected.to contain_class 'archive' }
it do
Expand All @@ -15,7 +15,7 @@
end

context 'RHEL Puppet opensource with params' do
let(:facts) { { osfamily: 'RedHat', puppetversion: '3.7.3' } }
let(:facts) { { os: { family: 'RedHat' }, puppetversion: '4.4.0' } }

let(:params) do
{
Expand All @@ -39,7 +39,7 @@
context 'Windows Puppet Enterprise' do
let(:facts) do
{
osfamily: 'Windows',
os: { family: 'Windows' },
puppetversion: '3.4.3 (Puppet Enterprise 3.2.3)',
archive_windir: 'C:/Windows/Temp/staging'
}
Expand Down
2 changes: 1 addition & 1 deletion spec/defines/artifactory_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'

describe 'archive::artifactory' do
let(:facts) { { osfamily: 'RedHat', puppetversion: '3.7.3' } }
let(:facts) { { os: { family: 'RedHat' }, puppetversion: '4.4.0' } }

before do
MockFunction.new('artifactory_sha1') do |f|
Expand Down
6 changes: 3 additions & 3 deletions spec/defines/go_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'

describe 'archive::go' do
let(:facts) { { osfamily: 'RedHat', puppetversion: '3.7.3' } }
let(:facts) { { os: { family: 'RedHat' }, puppetversion: '4.4.0' } }

before do
MockFunction.new('go_md5') do |f|
Expand All @@ -14,7 +14,7 @@
let(:params) do
{
server: 'home.lan',
port: '8081',
port: 8081,
url_path: 'go/example.zip',
md5_url_path: 'go/example.zip/checksum',
username: 'username',
Expand Down Expand Up @@ -47,7 +47,7 @@
{
archive_path: '/opt/app',
server: 'home.lan',
port: '8081',
port: 8081,
url_path: 'go/example.zip',
md5_url_path: 'go/example.zip/checksum',
username: 'username',
Expand Down
Loading