Merge pull request #189 from willdurand/cleanup-and-refactor-internals

Preparation for 2.1

.fixtures.yml

@@ -4,7 +4,4 @@ fixtures:
   repositories:
     stdlib:
       repo: "git://github.com/puppetlabs/puppetlabs-stdlib.git"
-      ref: "4.12.0"
-    gcc:
-      repo: "git://github.com/puppetlabs/puppetlabs-gcc.git"
-      ref: "0.3.0"
+      ref: "5.1.0"

.gitignore

@@ -1,6 +1,15 @@
+# RSpec
 spec/fixtures/
+/coverage
+
+# Puppet
 pkg/
+
+# Bundler
 Gemfile.lock
 /.bundle/
 /vendor/
-/coverage
+
+# Beaker
+log/
+junit/

.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

CHANGELOG.md

@@ -2,6 +2,28 @@
 
 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.
+
+### 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`.
+* 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
+
+* 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

Gemfile

@@ -18,4 +18,15 @@ group :devel do
   gem 'coveralls', require: false
 
   gem 'rubocop'
+
+  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

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)
@@ -25,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):
 
@@ -34,9 +32,13 @@ 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
+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
@@ -73,6 +75,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
@@ -201,7 +225,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.
@@ -258,9 +290,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`)
 
@@ -271,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 (<[email protected]>)

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
@@ -20,9 +22,15 @@ 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 => [
-  :rubocop,
+  :metadata_lint,
+  :rubocop_local,
   :syntax,
   :lint,
   :spec,

manifests/init.pp

@@ -29,6 +29,12 @@
 # [*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`.
+#
+# [*source*]
+#   Which source to use instead of `nodejs.org/dist`. Optional parameter, `undef` by default.
+#
 # == Example:
 #
 #  include nodejs
@@ -38,28 +44,20 @@
 #  }
 #
 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,
+  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  {
-  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'
+  $node_version = evaluate_version($version)
 
   if $build_deps {
     Anchor['nodejs::start'] ->
@@ -78,6 +76,8 @@
       instances_to_remove => $instances_to_remove,
       nodejs_default_path => $nodejs_default_path,
       download_timeout    => $download_timeout,
+      install_dir         => $install_dir,
+      source              => $source,
     } ->
     # TODO remove!
     file { '/etc/profile.d/nodejs.sh':

manifests/instance.pp

@@ -23,22 +23,25 @@
 # [*timeout*]
 #   Maximum download timeout.
 #
-define nodejs::instance($ensure, $version, $target_dir, $make_install, $cpu_cores, $default_node_version, $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,
+  String $target_dir,
+  Boolean $make_install,
+  Integer $cpu_cores,
+  Optional[String] $default_node_version,
+  Integer $timeout,
+  String $install_dir,
+  Optional[String] $source = undef,
+) {
   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}"
+  $node_unpack_folder = "${install_dir}/node-${version}"
 
   if $ensure == present {
     $node_os = $::kernel ? {
@@ -65,22 +68,27 @@
 
     ensure_resource('file', 'nodejs-install-dir', {
       ensure => 'directory',
-      path   => $::nodejs::params::install_dir,
+      path   => $install_dir,
       owner  => 'root',
       group  => 'root',
       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}",
-      destination => "${::nodejs::params::install_dir}/${node_filename}",
+      source      => $download_source,
+      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 +106,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 => [
@@ -123,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}"],
       }

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!')
   }