From cbc70e72148dbe4db412d7a6d6a56c0112418ecf Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 3 Nov 2018 17:05:33 +0100 Subject: [PATCH 01/10] Use Puppet's new type system for several APIs (including internals) In a0d1d799 Puppet 3.x has been dropped in favor of Puppet 4 which introduced a new type system which made the validate_* function sof `puppetlabs-stdlib` obsolete. The most relevant components now use a type system as well, the rest will follow or declared as deprecated. Fixes #186 --- manifests/init.pp | 28 +++++++++------------------- manifests/instance.pp | 18 +++++++++--------- manifests/instance/download.pp | 13 ++++--------- manifests/instances.pp | 11 ++++++++++- 4 files changed, 32 insertions(+), 38 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 30255c2..c0063aa 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -38,26 +38,16 @@ # } # class nodejs( - $version = $::nodejs::params::version, - $target_dir = $::nodejs::params::target_dir, - $make_install = $::nodejs::params::make_install, - $node_path = $::nodejs::params::node_path, - $cpu_cores = $::nodejs::params::cpu_cores, - $instances = $::nodejs::params::instances, - $instances_to_remove = $::nodejs::params::instances_to_remove, - $download_timeout = $::nodejs::params::download_timeout, - $build_deps = $::nodejs::params::build_deps, + String $version = $::nodejs::params::version, + String $target_dir = $::nodejs::params::target_dir, + Boolean $make_install = $::nodejs::params::make_install, + String $node_path = $::nodejs::params::node_path, + Integer $cpu_cores = $::nodejs::params::cpu_cores, + Hash[String, Hash] $instances = $::nodejs::params::instances, + Array[String] $instances_to_remove = $::nodejs::params::instances_to_remove, + Integer $download_timeout = $::nodejs::params::download_timeout, + Boolean $build_deps = $::nodejs::params::build_deps, ) inherits ::nodejs::params { - validate_string($node_path) - validate_integer($cpu_cores) - validate_string($version) - validate_string($target_dir) - validate_bool($make_install) - validate_hash($instances) - validate_array($instances_to_remove) - validate_integer($download_timeout) - validate_bool($build_deps) - $node_version = evaluate_version($version) $nodejs_default_path = '/usr/local/node/node-default' diff --git a/manifests/instance.pp b/manifests/instance.pp index f6188f2..82647fd 100644 --- a/manifests/instance.pp +++ b/manifests/instance.pp @@ -23,19 +23,19 @@ # [*timeout*] # Maximum download timeout. # -define nodejs::instance($ensure, $version, $target_dir, $make_install, $cpu_cores, $default_node_version, $timeout) { +define nodejs::instance( + Pattern[/^present|absent$/] $ensure, + String $version, + String $target_dir, + Boolean $make_install, + Integer $cpu_cores, + Optional[String] $default_node_version, + Integer $timeout +) { if $caller_module_name != $module_name { warning('nodejs::instance is private!') } - validate_string($ensure) - validate_re($ensure, '^(present|absent)$') - validate_integer($cpu_cores) - validate_string($version) - validate_string($target_dir) - validate_bool($make_install) - validate_integer($timeout) - include ::nodejs::params $node_unpack_folder = "${::nodejs::params::install_dir}/node-${version}" diff --git a/manifests/instance/download.pp b/manifests/instance/download.pp index 8edcca1..9054ed2 100644 --- a/manifests/instance/download.pp +++ b/manifests/instance/download.pp @@ -15,16 +15,11 @@ # Timeout for the download command. # define nodejs::instance::download( - $source, - $destination, - $unless_test = true, - $timeout = 0 + String $source, + String $destination, + Boolean $unless_test = true, + Integer $timeout = 0 ) { - validate_bool($unless_test) - validate_string($destination) - validate_string($source) - validate_integer($timeout) - if $caller_module_name != $module_name { warning('::nodejs::install::download is not meant for public use!') } diff --git a/manifests/instances.pp b/manifests/instances.pp index 36147d3..17ff7c1 100644 --- a/manifests/instances.pp +++ b/manifests/instances.pp @@ -26,7 +26,16 @@ # [*download_timeout*] # Maximum time for the download of the nodejs sources. # -class nodejs::instances($instances, $node_version, $target_dir, $make_install, $cpu_cores, $instances_to_remove, $nodejs_default_path, $download_timeout) { +class nodejs::instances( + Hash[String, Hash] $instances, + String $node_version, + String $target_dir, + Boolean $make_install, + Integer $cpu_cores, + Array[String] $instances_to_remove, + String $nodejs_default_path, + Integer $download_timeout +) { if $caller_module_name != $module_name { warning('nodejs::instances is private!') } From 9bf43eef2e4acfb1d610e424149a5bd02a194f88 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 3 Nov 2018 17:27:17 +0100 Subject: [PATCH 02/10] Drop hacky `ruby` setup To evaluate the Puppet catalogue on the Puppet Master, the client doesn't need `rubygems` as well. This was an ancient hack (maintained over years) to avoid such a setup in test environments for Puppet such as Vagrant. Instead something like this might be implemented: ``` ruby Vagrant.configure(2) do |config| # ... config.vm.provision :puppet do |puppet| puppet.module_path = ['puppet', ...] end end ``` With the `manifests/default.pp`: ``` puppet node default { contain ::nodejs package { ['ruby', 'rubygems']: ensure => present, } -> Class['::nodejs'] } ``` --- CHANGELOG.md | 8 ++++++++ README.md | 7 +++++-- manifests/instance/pkgs.pp | 13 ++----------- spec/classes/nodejs_instance_pkgs_spec.rb | 8 -------- 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb1173..00e33d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ This document lists the changes of all recent versions since `2.0.0`. +## 2.1.0 + +### Minor breaking changes + +* Dropped EOLed Puppet 3.x +* Don't install dev dependencies (`ruby`) anymore with `build_deps => true`. It's only needed on + the Puppet Master and shouldn't be deployed onto each node. + ## 2.0.3 * ([#184](https://github.com/willdurand/puppet-nodejs/issues/184)) Added support diff --git a/README.md b/README.md index 76e1864..5855a04 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,11 @@ mod 'puppetlabs/stdlib', '5.1.0' mod 'puppetlabs/gcc', '0.3.0' ``` +The Puppet Master which evaluates the catalogue before deploying each node requires +the following modules to properly evaluate the functions bundled with this module: + +* [semver](https://rubygems.org/gems/semver/versions/1.0.1) + ## Usage ### Deploying a precompiled package @@ -258,9 +263,7 @@ class { '::nodejs': In this case you'll need to take care of the following packages: - `tar` -- `ruby` - `wget` -- `semver` (GEM used by ruby) - `make` (if `make_install` = `true`) - `gcc` compiler (if `make_install` = `true`) diff --git a/manifests/instance/pkgs.pp b/manifests/instance/pkgs.pp index 726a1c6..4127fb8 100644 --- a/manifests/instance/pkgs.pp +++ b/manifests/instance/pkgs.pp @@ -11,21 +11,12 @@ # # class { '::nodejs::instance::pkgs': } # -class nodejs::instance::pkgs($make_install = false) { +class nodejs::instance::pkgs(Boolean $make_install = false) { if $caller_module_name != $module_name { warning('nodejs::instance::pkgs is private!') } - $rubygems = $::osfamily ? { - 'Debian' => 'rubygems-integration', - default => 'rubygems' - } - - ensure_packages(['tar', 'wget', 'ruby', $rubygems]) - ensure_packages(['semver'], { - provider => gem, - require => Package['ruby'], - }) + ensure_packages(['tar', 'wget']) if $make_install { include ::gcc diff --git a/spec/classes/nodejs_instance_pkgs_spec.rb b/spec/classes/nodejs_instance_pkgs_spec.rb index eac5270..878b5c5 100644 --- a/spec/classes/nodejs_instance_pkgs_spec.rb +++ b/spec/classes/nodejs_instance_pkgs_spec.rb @@ -9,14 +9,6 @@ describe 'module dependency management' do it { should contain_package('wget') } it { should contain_package('tar') } - it { should contain_package('rubygems-integration') } - end - - describe 'redhat OS' do - let(:facts) {{ - :osfamily => 'RedHat', - }} - it { should contain_package('rubygems') } end describe 'includes compiler-related dependencies' do From 6d3aade6bfa5f60f47358aa50154fb61fea97962 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 3 Nov 2018 17:38:37 +0100 Subject: [PATCH 03/10] Make `$nodejs_default_path` configurable It's now listed in `::nodejs::params` as sensitive default, but in edge-cases it might be helpful to specify a different directory than `/usr/local/node/node-default` as directory for the default nodejs instance. --- manifests/init.pp | 2 +- manifests/params.pp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/manifests/init.pp b/manifests/init.pp index c0063aa..fee1c01 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -47,9 +47,9 @@ Array[String] $instances_to_remove = $::nodejs::params::instances_to_remove, Integer $download_timeout = $::nodejs::params::download_timeout, Boolean $build_deps = $::nodejs::params::build_deps, + String $nodejs_default_path = $::nodejs::params::nodejs_default_path, ) inherits ::nodejs::params { $node_version = evaluate_version($version) - $nodejs_default_path = '/usr/local/node/node-default' if $build_deps { Anchor['nodejs::start'] -> diff --git a/manifests/params.pp b/manifests/params.pp index 6b13559..657ccd2 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -24,4 +24,5 @@ $instances_to_remove = [] $download_timeout = 0 $build_deps = true + $nodejs_default_path = '/usr/local/node/node-default' } From e2a2cde2de04e2eef38a10de4340acf7589145b1 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 3 Nov 2018 17:59:07 +0100 Subject: [PATCH 04/10] Refactor `install_dir` usage to avoid `::nodejs::params` references inside the module The `::nodejs::params` class shall only be used as container with sensitive defaults to provide a working setup on an average OS. It should never be referenced directly inside the module's code, instead `::nodejs` should pass it down to all internal APIs. This is not a breaking change and much rather a cosmetic change as it only affects private parts of the module and `::nodejs` has a parameter with a default value. --- manifests/init.pp | 5 +++++ manifests/instance.pp | 15 +++++++-------- manifests/instances.pp | 6 +++++- spec/classes/nodejs_instances_spec.rb | 5 +++++ spec/defines/nodejs_instance_spec.rb | 9 +++++++++ 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index fee1c01..095025a 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -29,6 +29,9 @@ # [*build_deps*] # Optional parameter whether or not to allow the module to installs its dependant packages. # +# [*install_dir*] +# Installation directory for all node instances. By default `/usr/local/node`. +# # == Example: # # include nodejs @@ -48,6 +51,7 @@ Integer $download_timeout = $::nodejs::params::download_timeout, Boolean $build_deps = $::nodejs::params::build_deps, String $nodejs_default_path = $::nodejs::params::nodejs_default_path, + String $install_dir = $::nodejs::params::install_dir, ) inherits ::nodejs::params { $node_version = evaluate_version($version) @@ -68,6 +72,7 @@ instances_to_remove => $instances_to_remove, nodejs_default_path => $nodejs_default_path, download_timeout => $download_timeout, + install_dir => $install_dir, } -> # TODO remove! file { '/etc/profile.d/nodejs.sh': diff --git a/manifests/instance.pp b/manifests/instance.pp index 82647fd..3ca48b9 100644 --- a/manifests/instance.pp +++ b/manifests/instance.pp @@ -30,15 +30,14 @@ Boolean $make_install, Integer $cpu_cores, Optional[String] $default_node_version, - Integer $timeout + Integer $timeout, + String $install_dir ) { if $caller_module_name != $module_name { warning('nodejs::instance is private!') } - include ::nodejs::params - - $node_unpack_folder = "${::nodejs::params::install_dir}/node-${version}" + $node_unpack_folder = "${install_dir}/node-${version}" if $ensure == present { $node_os = $::kernel ? { @@ -65,7 +64,7 @@ ensure_resource('file', 'nodejs-install-dir', { ensure => 'directory', - path => $::nodejs::params::install_dir, + path => $install_dir, owner => 'root', group => 'root', mode => '0644', @@ -73,14 +72,14 @@ ::nodejs::instance::download { "nodejs-download-${version}": source => "https://nodejs.org/dist/${version}/${node_filename}", - destination => "${::nodejs::params::install_dir}/${node_filename}", + destination => "${install_dir}/${node_filename}", require => File['nodejs-install-dir'], timeout => $timeout, } file { "nodejs-check-tar-${version}": ensure => 'file', - path => "${::nodejs::params::install_dir}/${node_filename}", + path => "${install_dir}/${node_filename}", owner => 'root', group => 'root', mode => '0644', @@ -98,7 +97,7 @@ exec { "nodejs-unpack-${version}": command => "tar -xzvf ${node_filename} -C ${node_unpack_folder} --strip-components=1", path => $::path, - cwd => $::nodejs::params::install_dir, + cwd => $install_dir, user => 'root', unless => "test -f ${node_symlink_target}", require => [ diff --git a/manifests/instances.pp b/manifests/instances.pp index 17ff7c1..481669f 100644 --- a/manifests/instances.pp +++ b/manifests/instances.pp @@ -34,7 +34,8 @@ Integer $cpu_cores, Array[String] $instances_to_remove, String $nodejs_default_path, - Integer $download_timeout + Integer $download_timeout, + String $install_dir, ) { if $caller_module_name != $module_name { warning('nodejs::instances is private!') @@ -49,6 +50,7 @@ cpu_cores => $cpu_cores, default_node_version => undef, timeout => $download_timeout, + install_dir => $install_dir, } } else { create_resources('::nodejs::instance', node_instances($instances, true), { @@ -58,6 +60,7 @@ cpu_cores => $cpu_cores, default_node_version => undef, timeout => $download_timeout, + install_dir => $install_dir, }) if !defined(Nodejs::Instance["nodejs-custom-instance-${$node_version}"]) { @@ -73,6 +76,7 @@ target_dir => $target_dir, default_node_version => $node_version, timeout => $download_timeout, + install_dir => $install_dir, }) } diff --git a/spec/classes/nodejs_instances_spec.rb b/spec/classes/nodejs_instances_spec.rb index 48b63f9..994c8ea 100644 --- a/spec/classes/nodejs_instances_spec.rb +++ b/spec/classes/nodejs_instances_spec.rb @@ -29,6 +29,7 @@ :cpu_cores => 2, :nodejs_default_path => '/usr/local/node/node-default', :download_timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_nodejs__instance('nodejs-custom-instance-v5.0.0') \ @@ -55,6 +56,7 @@ :cpu_cores => 2, :nodejs_default_path => '/usr/local/node/node-default', :download_timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_nodejs__instance("nodejs-custom-instance-v4.4.7") \ @@ -99,6 +101,7 @@ :instances_to_remove => [], :nodejs_default_path => '/usr/local/node/node-default', :download_timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_nodejs__instance("nodejs-custom-instance-v6.7.0") \ @@ -128,6 +131,7 @@ :instances_to_remove => ['v6.4.0'], :nodejs_default_path => '/usr/local/node/node-default', :download_timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_nodejs__instance("nodejs-uninstall-custom-v6.4.0") \ @@ -154,6 +158,7 @@ :instances_to_remove => [], :nodejs_default_path => '/usr/local/node/node-default', :download_timeout => 0, + :install_dir => '/usr/local/node', }} it { should compile.and_raise_error(/Cannot create a default instance with version/) } diff --git a/spec/defines/nodejs_instance_spec.rb b/spec/defines/nodejs_instance_spec.rb index d1fef8c..f24ec14 100644 --- a/spec/defines/nodejs_instance_spec.rb +++ b/spec/defines/nodejs_instance_spec.rb @@ -21,6 +21,7 @@ :cpu_cores => 2, :default_node_version => nil, :timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_nodejs__instance__download('nodejs-download-v4.4.7') \ @@ -50,6 +51,7 @@ :cpu_cores => 1, :default_node_version => nil, :timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_exec('nodejs-make-install-v6.2.0') \ @@ -69,6 +71,7 @@ :cpu_cores => 2, :default_node_version => nil, :timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_file('nodejs-install-dir') \ @@ -141,6 +144,7 @@ :cpu_cores => 1, :default_node_version => nil, :timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_exec('nodejs-make-install-v6.0.0') \ @@ -159,6 +163,7 @@ :cpu_cores => 1, :default_node_version => nil, :timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_file('nodejs-symlink-bin-with-version-v6.2.0') \ @@ -177,6 +182,7 @@ :make_install => false, :default_node_version => nil, :timeout => 0, + :install_dir => '/usr/local/node', }} it { should_not contain_exec('nodejs-make-install-v6.2.0') } @@ -192,6 +198,7 @@ :cpu_cores => 1, :default_node_version => 'v4.6.0', :timeout => 0, + :install_dir => '/usr/local/node', }} it { should contain_file('/usr/local/node/node-v0.12.0') \ @@ -212,6 +219,7 @@ :cpu_cores => 1, :default_node_version => 'v5.6.0', :timeout => 0, + :install_dir => '/usr/local/node', }} it { should compile.and_raise_error(/Can't remove the instance/) } @@ -227,6 +235,7 @@ :cpu_cores => 1, :default_node_version => 'v5.6.0', :timeout => 0, + :install_dir => '/usr/local/node', }} describe 'os' do From c090fdfdfab387c5884fce03ad10ad1a849bc3b6 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 3 Nov 2018 18:15:59 +0100 Subject: [PATCH 05/10] Deprecate `::nodejs::npm` Please refer to the changelog and the readme for further information. Fixes #187 --- CHANGELOG.md | 6 ++++++ README.md | 10 +++++++++- manifests/npm.pp | 2 ++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00e33d6..30d06ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ This document lists the changes of all recent versions since `2.0.0`. * Don't install dev dependencies (`ruby`) anymore with `build_deps => true`. It's only needed on the Puppet Master and shouldn't be deployed onto each node. +### Further changes + +* Deprecated `::nodejs::npm`. The feature was always out of scope and introduced several hacks + to support several edge-cases. Instead it's recommended to write a custom module suited + for your application when deploying dependencies into a given directory. + ## 2.0.3 * ([#184](https://github.com/willdurand/puppet-nodejs/issues/184)) Added support diff --git a/README.md b/README.md index 5855a04..6792266 100644 --- a/README.md +++ b/README.md @@ -206,7 +206,15 @@ package { 'express': } ``` -### NPM installer +### NPM installer (deprecated) + +Note: this API is deprecated and will be removed in `3.0`. It's recommended to either package your +applications properly using `npm` and install them as package using the `npm` provider or to directly +run `npm install` when deploying your application (e.g. with a custom Puppet module). + +This module is focused on setting up an environment with `nodejs`, application deployment should be handled +in its own module. In the end this was just a wrapper on top of `npm` which runs an `exec` with +`npm install` and a configurable user and lacks proper `ensure => absent` support. The `nodejs` installer can be used if a npm package should not be installed globally, but in a certain directory. diff --git a/manifests/npm.pp b/manifests/npm.pp index 5430b25..c8da767 100644 --- a/manifests/npm.pp +++ b/manifests/npm.pp @@ -56,6 +56,8 @@ $options = undef, $home_dir = '/root', ) { + warning("::nodejs::npm is deprecated and will thus be removed in 3.0! Instead, package your applications directly with npm and use the npm provider or directly implement this directly in your application's puppet module.") + validate_string($version) validate_string($exec_user) validate_bool($list) From b604443720a9575d344e5c2842f708948c46678b Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 4 Nov 2018 13:14:49 +0100 Subject: [PATCH 06/10] Make the node.js source (by default nodejs.org/dist) configurable As requested it might be helpful to specify your own location for either source or binary distribution. Possible cases where this might be needed are e.g. corporate environments without internet access or to deploy a patched/forked version of NodeJS to Puppet nodes. As the NodeJS distribution provides several ways to index the available versions, the source_server behaves different when used with a custom server. ``` class { '::nodejs': source => 'https://your-server.com/nodejs.tar.gz', } ``` You can't specify a version, but you need to directly specify the source ot be installed. `make_install => true` can be used for source builds with Puppet as well. Fixes #185 --- CHANGELOG.md | 5 +++++ README.md | 23 ++++++++++++++++++++++- manifests/init.pp | 5 +++++ manifests/instance.pp | 13 +++++++++++-- manifests/instances.pp | 10 ++++++++++ manifests/params.pp | 1 + spec/defines/nodejs_instance_spec.rb | 18 ++++++++++++++++++ 7 files changed, 72 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30d06ac..0840318 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ This document lists the changes of all recent versions since `2.0.0`. * Don't install dev dependencies (`ruby`) anymore with `build_deps => true`. It's only needed on the Puppet Master and shouldn't be deployed onto each node. +### Other notable changes + +* ([#185](https://github.com/willdurand/puppet-nodejs/issues/185)) Allow to specify a custom source + for NodeJS to override `nodejs.org/dist`. + ### Further changes * Deprecated `::nodejs::npm`. The feature was always out of scope and introduced several hacks diff --git a/README.md b/README.md index 6792266..2e916bd 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,6 @@ This module allows you to install [Node.js](https://nodejs.org/) and [willdurand/nodejs](https://forge.puppetlabs.com/willdurand/nodejs). ### Announcements - * From now on `2.0` is maintenance-only and accepts bugfixes until `2.2` is released. On `master` the active development on `2.1` is currently happening. The docs of `2.0` can be found [here](https://github.com/willdurand/puppet-nodejs/tree/2.0#puppet-nodejs) @@ -78,6 +77,28 @@ class { 'nodejs': } ``` +### Using a custom source + +It's also possible to deploy NodeJS instances to Puppet nodes from your own server. +This can be helpful when e.g. distributing your own, patched version of NodeJS. + +The source can be specified like this: + +``` puppet +class { '::nodejs': + source => 'https://example.org/dist-nodejs', +} +``` + +It's also possible to compile the custom instance from source: + +``` puppet +class { '::nodejs': + source => 'https://example.org/src-nodejs', + make_install => true, +} +``` + ### Setup with a given download timeout Due to infrastructures with slower connections the download timeout of the nodejs binaries can be increased diff --git a/manifests/init.pp b/manifests/init.pp index 095025a..c7e116b 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -32,6 +32,9 @@ # [*install_dir*] # Installation directory for all node instances. By default `/usr/local/node`. # +# [*source*] +# Which source to use instead of `nodejs.org/dist`. Optional parameter, `undef` by default. +# # == Example: # # include nodejs @@ -52,6 +55,7 @@ Boolean $build_deps = $::nodejs::params::build_deps, String $nodejs_default_path = $::nodejs::params::nodejs_default_path, String $install_dir = $::nodejs::params::install_dir, + Optional[String] $source = $::nodejs::params::source, ) inherits ::nodejs::params { $node_version = evaluate_version($version) @@ -73,6 +77,7 @@ nodejs_default_path => $nodejs_default_path, download_timeout => $download_timeout, install_dir => $install_dir, + source => $source, } -> # TODO remove! file { '/etc/profile.d/nodejs.sh': diff --git a/manifests/instance.pp b/manifests/instance.pp index 3ca48b9..231f9df 100644 --- a/manifests/instance.pp +++ b/manifests/instance.pp @@ -23,6 +23,9 @@ # [*timeout*] # Maximum download timeout. # +# [*source*] +# Which source to use instead of `nodejs.org/dist`. Optional parameter, `undef` by default. +# define nodejs::instance( Pattern[/^present|absent$/] $ensure, String $version, @@ -31,7 +34,8 @@ Integer $cpu_cores, Optional[String] $default_node_version, Integer $timeout, - String $install_dir + String $install_dir, + Optional[String] $source = undef, ) { if $caller_module_name != $module_name { warning('nodejs::instance is private!') @@ -70,8 +74,13 @@ mode => '0644', }) + $download_source = $source ? { + undef => "https://nodejs.org/dist/${version}/${node_filename}", + default => $source, + } + ::nodejs::instance::download { "nodejs-download-${version}": - source => "https://nodejs.org/dist/${version}/${node_filename}", + source => $download_source, destination => "${install_dir}/${node_filename}", require => File['nodejs-install-dir'], timeout => $timeout, diff --git a/manifests/instances.pp b/manifests/instances.pp index 481669f..78b71b2 100644 --- a/manifests/instances.pp +++ b/manifests/instances.pp @@ -26,6 +26,12 @@ # [*download_timeout*] # Maximum time for the download of the nodejs sources. # +# [*install_dir*] +# Where to deploy the NodeJS instances into. +# +# [*source*] +# Where to fetch the NodeJS instances (either sources or binary distributions). +# class nodejs::instances( Hash[String, Hash] $instances, String $node_version, @@ -36,6 +42,7 @@ String $nodejs_default_path, Integer $download_timeout, String $install_dir, + Optional[String] $source = undef, ) { if $caller_module_name != $module_name { warning('nodejs::instances is private!') @@ -51,6 +58,7 @@ default_node_version => undef, timeout => $download_timeout, install_dir => $install_dir, + source => $source, } } else { create_resources('::nodejs::instance', node_instances($instances, true), { @@ -61,6 +69,7 @@ default_node_version => undef, timeout => $download_timeout, install_dir => $install_dir, + source => $source, }) if !defined(Nodejs::Instance["nodejs-custom-instance-${$node_version}"]) { @@ -77,6 +86,7 @@ default_node_version => $node_version, timeout => $download_timeout, install_dir => $install_dir, + source => $source, }) } diff --git a/manifests/params.pp b/manifests/params.pp index 657ccd2..dc9fe61 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -25,4 +25,5 @@ $download_timeout = 0 $build_deps = true $nodejs_default_path = '/usr/local/node/node-default' + $source = undef } diff --git a/spec/defines/nodejs_instance_spec.rb b/spec/defines/nodejs_instance_spec.rb index f24ec14..14adcf0 100644 --- a/spec/defines/nodejs_instance_spec.rb +++ b/spec/defines/nodejs_instance_spec.rb @@ -238,6 +238,24 @@ :install_dir => '/usr/local/node', }} + describe 'custom source' do + let(:params) {{ + :version => 'v6.0.0', + :ensure => 'present', + :make_install => false, + :target_dir => '/usr/local/bin', + :cpu_cores => 1, + :default_node_version => 'v5.6.0', + :timeout => 0, + :install_dir => '/usr/local/node', + :source => 'https://example.org/dist-nodejs' + }} + + it { should contain_nodejs__instance__download('nodejs-download-v6.0.0') \ + .with_source('https://example.org/dist-nodejs') \ + } + end + describe 'os' do describe 'darwin' do let(:facts) {{ From 8e5d7021317e31feee0be30a3a4f12a436f67f81 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 4 Nov 2018 15:14:09 +0100 Subject: [PATCH 07/10] Lint metadata.json during `rake test` --- Gemfile | 2 ++ Rakefile | 1 + 2 files changed, 3 insertions(+) diff --git a/Gemfile b/Gemfile index 5765953..5743d2b 100644 --- a/Gemfile +++ b/Gemfile @@ -18,4 +18,6 @@ group :devel do gem 'coveralls', require: false gem 'rubocop' + + gem 'metadata-json-lint' end diff --git a/Rakefile b/Rakefile index a83a35d..d687a61 100644 --- a/Rakefile +++ b/Rakefile @@ -22,6 +22,7 @@ PuppetSyntax.exclude_paths = exclude_paths desc "Run syntax, lint, and spec tests." task :test => [ + :metadata_lint, :rubocop, :syntax, :lint, From 0118c8aa493972c7599042841dd2cbb46870f4e3 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 4 Nov 2018 15:15:34 +0100 Subject: [PATCH 08/10] Minor dependency bumps Don't support EOLed stdlib versions anymore, introduce upper-bound version constraints in `metadata.json` to ensure no accidental breakage with new major releases. --- .fixtures.yml | 2 +- metadata.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.fixtures.yml b/.fixtures.yml index 992d32d..13ac131 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -4,7 +4,7 @@ fixtures: repositories: stdlib: repo: "git://github.com/puppetlabs/puppetlabs-stdlib.git" - ref: "4.12.0" + ref: "5.1.0" gcc: repo: "git://github.com/puppetlabs/puppetlabs-gcc.git" ref: "0.3.0" diff --git a/metadata.json b/metadata.json index 49c757c..0cf501e 100644 --- a/metadata.json +++ b/metadata.json @@ -47,11 +47,11 @@ "dependencies": [ { "name": "puppetlabs/stdlib", - "version_requirement": ">=3.2.1" + "version_requirement": ">=4.12.0 <6.0" }, { "name": "puppetlabs/gcc", - "version_requirement": ">=0.3.0" + "version_requirement": ">=0.3.0 <1.0" } ], "requirements": [ From 8b70b55f1f778e5df5cfb78578b57a236c466565 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 6 Nov 2018 14:21:46 +0100 Subject: [PATCH 09/10] Fix RuboCop configuration due to fallout from `stdlib` bump RuboCop searches for all `.rubocop.yml` inside the cwd, also the (broken) one from `puppetlabs-stdlib` which has deprecated rules and requires extra gems that aren't used by this module. --- Rakefile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Rakefile b/Rakefile index d687a61..bfb5fd9 100644 --- a/Rakefile +++ b/Rakefile @@ -4,7 +4,9 @@ require 'puppet-syntax/tasks/puppet-syntax' require 'puppet_blacksmith/rake_tasks' require 'rubocop/rake_task' -RuboCop::RakeTask.new +RuboCop::RakeTask.new(:rubocop_local) do |t| + t.options = ['-c', '.rubocop.yml'] +end PuppetLint.configuration.log_format = "%{path}:%{line}:%{check}:%{KIND}:%{message}" PuppetLint.configuration.fail_on_warnings = false @@ -23,7 +25,7 @@ PuppetSyntax.exclude_paths = exclude_paths desc "Run syntax, lint, and spec tests." task :test => [ :metadata_lint, - :rubocop, + :rubocop_local, :syntax, :lint, :spec, From 98b26e05437bc476e5ab1024dac601f353e85401 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 6 Nov 2018 14:58:37 +0100 Subject: [PATCH 10/10] Introduce acceptance testing of Puppet manifests with beaker Due to several changes inside to the dependencies in the module this is a rather drastic change. The module `puppetlabs-gcc` seems to be abandoned (Puppet 3.x only support, last commit in 2015) which broke an automated install with enabled dependency resolution (as done in beaker). The logic of the module is now directly in `::nodejs::instance::pkgs`. To automate such testing processesin the future, `beaker` has been introduced with `docker` as hypervisor (it's far more lightweight than a full-blown `vagrant` setup). Currently we only support Ubuntu 16.04 LTS in the testing matrix, but whenever distros start to conflict with this module, the test suite can be easily extended. As `PUPPET_VERSION` is interpreted by beaker as well the test suite has been split into two tasks (:test and :acceptance) as gem's version constraint will break APT while provisioning docker. Fixes #188 --- .fixtures.yml | 3 --- .gitignore | 11 ++++++++++- .travis.yml | 3 +++ CHANGELOG.md | 3 +++ Gemfile | 9 +++++++++ README.md | 6 ++++-- Rakefile | 7 ++++++- manifests/init.pp | 2 +- manifests/instance.pp | 3 +-- manifests/instance/pkgs.pp | 10 +++++++++- metadata.json | 8 ++------ shell.nix | 5 +++++ spec/acceptance/nodejs_spec.rb | 14 ++++++++++++++ spec/acceptance/nodesets/ubuntu-1604-x64.yml | 8 ++++++++ spec/classes/nodejs_instance_pkgs_spec.rb | 2 +- spec/spec_base.rb | 12 ++++++++++++ spec/spec_helper.rb | 19 ++++++------------- spec/spec_helper_acceptance.rb | 10 ++++++++++ 18 files changed, 104 insertions(+), 31 deletions(-) create mode 100644 spec/acceptance/nodejs_spec.rb create mode 100644 spec/acceptance/nodesets/ubuntu-1604-x64.yml create mode 100644 spec/spec_base.rb create mode 100644 spec/spec_helper_acceptance.rb diff --git a/.fixtures.yml b/.fixtures.yml index 13ac131..70def94 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -5,6 +5,3 @@ fixtures: stdlib: repo: "git://github.com/puppetlabs/puppetlabs-stdlib.git" ref: "5.1.0" - gcc: - repo: "git://github.com/puppetlabs/puppetlabs-gcc.git" - ref: "0.3.0" diff --git a/.gitignore b/.gitignore index b50b7a3..f06f93e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,15 @@ +# RSpec spec/fixtures/ +/coverage + +# Puppet pkg/ + +# Bundler Gemfile.lock /.bundle/ /vendor/ -/coverage + +# Beaker +log/ +junit/ diff --git a/.travis.yml b/.travis.yml index db96e6e..d1a5920 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,7 @@ env: - PUPPET_VERSION="~> 4.10.12" STRICT_VARIABLES=yes RUBY_VERSION="2.4.0" TARGET=test - PUPPET_VERSION="~> 5.5.1" RUBY_VERSION="2.4.0" TARGET=test - PUPPET_VERSION="~> 6.0.3" RUBY_VERSION="2.5.1" TARGET=test + - TARGET=acceptance RUBY_VERSION="2.5.1" BEAKER_setfile=spec/acceptance/nodesets/ubuntu-1604-x64.yml before_install: - gem update bundler @@ -17,3 +18,5 @@ before_install: script: "rake $TARGET" sudo: false + +services: docker diff --git a/CHANGELOG.md b/CHANGELOG.md index 0840318..3966fcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ This document lists the changes of all recent versions since `2.0.0`. * ([#185](https://github.com/willdurand/puppet-nodejs/issues/185)) Allow to specify a custom source for NodeJS to override `nodejs.org/dist`. +* Don't depend on `puppetlabs-gcc` anymore. It wasn't updated by PuppetLabs since 2015 + and according to its metadata it doesn't support recent Puppet version which breaks the + `beaker` build. ### Further changes diff --git a/Gemfile b/Gemfile index 5743d2b..859e2f5 100644 --- a/Gemfile +++ b/Gemfile @@ -21,3 +21,12 @@ group :devel do gem 'metadata-json-lint' end + +group :beaker do + gem 'beaker' + gem 'beaker-rspec' + gem 'beaker-puppet' + gem 'beaker-docker' + gem 'beaker-puppet_install_helper' + gem 'beaker-module_install_helper' +end diff --git a/README.md b/README.md index 2e916bd..0fec104 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,6 @@ This module allows you to install [Node.js](https://nodejs.org/) and The module depends on the following well-adopted and commonly used modules: * [puppetlabs/stdlib](https://github.com/puppetlabs/puppetlabs-stdlib) -* [puppetlabs/gcc](https://github.com/puppetlabs/puppetlabs-gcc) The easiest approach to install this module is by using [r10k](https://github.com/puppetlabs/r10k): @@ -33,7 +32,6 @@ forge 'http://forge.puppetlabs.com' mod 'willdurand/nodejs', '2.0.3' mod 'puppetlabs/stdlib', '5.1.0' -mod 'puppetlabs/gcc', '0.3.0' ``` The Puppet Master which evaluates the catalogue before deploying each node requires @@ -303,8 +301,12 @@ The easiest way to get started is using [`bundler`](https://bundler.io): ``` bundle install bundle exec rake test +PUPPET_INSTALL_TYPE=agent BEAKER_setfile=spec/acceptance/nodesets/ubuntu-1604-x64.yml bundle exec rake acceptance ``` +**Note:** to run the acceptance tests that are part of rake's `test` target, +[Docker](https://www.docker.com/) is required. + ## Authors * William Durand () diff --git a/Rakefile b/Rakefile index bfb5fd9..0c95f4e 100644 --- a/Rakefile +++ b/Rakefile @@ -22,7 +22,12 @@ exclude_paths = [ PuppetLint.configuration.ignore_paths = exclude_paths PuppetSyntax.exclude_paths = exclude_paths -desc "Run syntax, lint, and spec tests." +desc "Run acceptance with beaker" +RSpec::Core::RakeTask.new(:acceptance) do |t| + t.pattern = 'spec/acceptance' +end + +desc "Run syntax, lint, spec and acceptance tests." task :test => [ :metadata_lint, :rubocop_local, diff --git a/manifests/init.pp b/manifests/init.pp index c7e116b..c446c7a 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -57,7 +57,7 @@ String $install_dir = $::nodejs::params::install_dir, Optional[String] $source = $::nodejs::params::source, ) inherits ::nodejs::params { - $node_version = evaluate_version($version) + $node_version = evaluate_version($version) if $build_deps { Anchor['nodejs::start'] -> diff --git a/manifests/instance.pp b/manifests/instance.pp index 231f9df..76b0dca 100644 --- a/manifests/instance.pp +++ b/manifests/instance.pp @@ -131,8 +131,7 @@ timeout => 0, require => [ Exec["nodejs-unpack-${version}"], - Class['::gcc'], - Package['make'], + Class['::nodejs::instance::pkgs'], ], before => File["nodejs-symlink-bin-with-version-${version}"], } diff --git a/manifests/instance/pkgs.pp b/manifests/instance/pkgs.pp index 4127fb8..78dde71 100644 --- a/manifests/instance/pkgs.pp +++ b/manifests/instance/pkgs.pp @@ -19,7 +19,15 @@ ensure_packages(['tar', 'wget']) if $make_install { - include ::gcc + # inherited from https://github.com/puppetlabs/puppetlabs-gcc/blob/master/manifests/params.pp, + # but the module is abandoned and only supports Puppet3. + $gcc_packages = $::osfamily ? { + 'RedHat' => ['gcc', 'gcc-g++'], + 'Debian' => ['gcc', 'build-essential'], + default => fail("Class['::nodejs::instances::pkgs']: unsupported osfamily: ${::osfamily}") + } + + ensure_packages($gcc_packages) ensure_packages(['make']) } } diff --git a/metadata.json b/metadata.json index 0cf501e..752e58a 100644 --- a/metadata.json +++ b/metadata.json @@ -47,17 +47,13 @@ "dependencies": [ { "name": "puppetlabs/stdlib", - "version_requirement": ">=4.12.0 <6.0" - }, - { - "name": "puppetlabs/gcc", - "version_requirement": ">=0.3.0 <1.0" + "version_requirement": ">= 4.12.0 < 6.0.0" } ], "requirements": [ { "name": "puppet", - "version_requirement": ">=4.3.0 <7.0.0" + "version_requirement": ">= 4.3.0 < 7.0.0" } ] } diff --git a/shell.nix b/shell.nix index 25ba74c..591353b 100644 --- a/shell.nix +++ b/shell.nix @@ -5,4 +5,9 @@ mkShell { buildInputs = [ ruby bundler rubocop ]; + + shellHook = '' + export PUPPET_INSTALL_TYPE=agent + export BEAKER_setfile=spec/acceptance/nodesets/ubuntu-1604-x64.yml + ''; } diff --git a/spec/acceptance/nodejs_spec.rb b/spec/acceptance/nodejs_spec.rb new file mode 100644 index 0000000..7129c1e --- /dev/null +++ b/spec/acceptance/nodejs_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper_acceptance' + +describe 'nodejs acceptance test' do + context 'with default arguments' do + it 'compiles and applies the catalogue' do + pp = <<-EOS + class { '::nodejs': } + EOS + + apply_manifest(pp, catch_failures: true) + apply_manifest(pp, catch_changes: true) + end + end +end diff --git a/spec/acceptance/nodesets/ubuntu-1604-x64.yml b/spec/acceptance/nodesets/ubuntu-1604-x64.yml new file mode 100644 index 0000000..5e5e7f3 --- /dev/null +++ b/spec/acceptance/nodesets/ubuntu-1604-x64.yml @@ -0,0 +1,8 @@ +HOSTS: + ubuntu-16.04: + roles: [master] + hypervisor: docker + platform: ubuntu-16.04-amd64 + image: ubuntu:16.04 +CONFIG: + type: foss diff --git a/spec/classes/nodejs_instance_pkgs_spec.rb b/spec/classes/nodejs_instance_pkgs_spec.rb index 878b5c5..9402740 100644 --- a/spec/classes/nodejs_instance_pkgs_spec.rb +++ b/spec/classes/nodejs_instance_pkgs_spec.rb @@ -23,6 +23,6 @@ it { should contain_package('tar') } it { should contain_package('make') } - it { should contain_class('gcc') } + it { should contain_package('gcc') } end end diff --git a/spec/spec_base.rb b/spec/spec_base.rb new file mode 100644 index 0000000..836f47f --- /dev/null +++ b/spec/spec_base.rb @@ -0,0 +1,12 @@ +require 'rubygems' +require 'puppetlabs_spec_helper/module_spec_helper' +require 'rspec' + + +RSpec.configure do |config| + config.after(:suite) do + RSpec::Puppet::Coverage.report! + end +end + +$:.unshift File.join(File.dirname(__FILE__), 'fixtures', 'modules', 'stdlib', 'lib') diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dbf445f..8c45ffe 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,9 +1,10 @@ -require 'rubygems' -require 'puppetlabs_spec_helper/module_spec_helper' +require 'spec_base' require 'webmock/rspec' -require 'rspec' +require 'coveralls' + +Coveralls.wear! -WebMock.disable_net_connect!() +WebMock.disable_net_connect! nodejs_response = < 200, :body => nodejs_response, :headers => {}) end - - config.after(:suite) do - RSpec::Puppet::Coverage.report! - end end - -$:.unshift File.join(File.dirname(__FILE__), 'fixtures', 'modules', 'stdlib', 'lib') - -require 'coveralls' -Coveralls.wear! diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb new file mode 100644 index 0000000..76fabc2 --- /dev/null +++ b/spec/spec_helper_acceptance.rb @@ -0,0 +1,10 @@ +require 'spec_base' +require 'beaker-rspec' +require 'beaker-puppet' +require 'beaker/puppet_install_helper' +require 'beaker/module_install_helper' + +run_puppet_install_helper +install_ca_certs +install_module_on(hosts) +install_module_dependencies_on(hosts)