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.

Sign up for GitHub

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

Add an option to install npm deps from package.json #300

Merged
merged 1 commit into from
Oct 5, 2017
Merged

Add an option to install npm deps from package.json #300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 24 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ except version ranges. The title simply must be a unique, arbitrary value.
specified as an array.
* The user parameter is provided should you wish to run npm install or npm rm
as a specific user.
* If you want to use a package.json supplied by a module to install dependencies
(e.g. if you have a NodeJS server app), set the parameter use_package_json to true.
The package name is then only used for the resource name. source parameter is ignored.

nodejs::npm parameters:

Expand All @@ -159,6 +162,7 @@ nodejs::npm parameters:
* uninstall_options: option flags invoked during removal (optional).
* npm_path: defaults to the value listed in `nodejs::params`
* user: defaults to undef
* use_package_json: read and install modules listed in package.json in target dir and install those in subdirectory node_modules (defaults to false)

Examples:

Expand Down Expand Up @@ -286,6 +290,16 @@ nodejs::npm { 'express with options':
}
```

Install dependencies from package.json:

```puppet
nodejs::npm { 'serverapp':
ensure => 'present',
target => '/opt/serverapp',
use_package_json => true,
}
```

Uninstall any versions of express in /opt/packages regardless of source:

```puppet
Expand All @@ -296,6 +310,16 @@ nodejs::npm { 'remove all express packages':
}
```

Uninstall dependencies from package.json:

```puppet
nodejs::npm { 'serverapp':
Copy link
Member

Choose a reason for hiding this comment

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

docs \o/

ensure => 'absent',
target => '/opt/serverapp',
use_package_json => true,
}
```

### nodejs::npm::global_config_entry

nodejs::npm::global_config_entry can be used to set/remove global npm
Expand Down
57 changes: 41 additions & 16 deletions manifests/npm.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
Array $uninstall_options = [],
$home_dir = '/root',
$user = undef,
Boolean $use_package_json = false,
Copy link
Member

Choose a reason for hiding this comment

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

datatypes \o/

) {

$install_options_string = join($install_options, ' ')
Expand Down Expand Up @@ -37,21 +38,34 @@
default => 'grep',
}

$install_check = $::osfamily ? {
'Windows' => "${npm_path} ls --long --parseable | ${grep_command} \"${target}\\node_modules\\${install_check_package_string}\"",
default => "${npm_path} ls --long --parseable | ${grep_command} \"${target}/node_modules/${install_check_package_string}\"",
$dirsep = $::osfamily ? {
'Windows' => "\\",
default => '/'
}

$list_command = "${npm_path} ls --long --parseable"
$install_check = "${list_command} | ${grep_command} \"${target}${dirsep}node_modules${dirsep}${install_check_package_string}\""
Copy link
Member

Choose a reason for hiding this comment

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

I have mixed feelings about a using a variable for a directory separator in a string - I personally prefer explicit versus implicit values for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMHO this makes the code more readable - the string just differ by the dir seps...

Maybe let's replace it with a call to regsubst on Windows to replace / with \\?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe let's replace it with a call to regsubst on Windows to replace / with \?

That's an idea. Not sure if there will be unintended side-effects to doing this though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this is not even necessary...

According to https://docs.puppet.com/puppet/5.2/lang_windows_file_paths.html#when-to-use-each-kind-of-slash, we can just use slashes.

I don't have a Windows Puppet at hands to prove this true or false - anything available on your side?

Copy link
Member

Choose a reason for hiding this comment

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

@poikilotherm Er I'm not sure if I'm much help - I haven't tested Puppet on Windows with anything for...quite some some time. Looking at the docs in more detail though, it seems as though we do care about what type of slashes, because we're using cmd.exe to run the grep-alternative and cmd.exe requires backslashes.

Copy link
Contributor

Choose a reason for hiding this comment

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

/ should just work, both in puppet and in node.


if $ensure == 'absent' {
$npm_command = 'rm'
$options = $uninstall_options_string

exec { "npm_${npm_command}_${name}":
command => "${npm_path} ${npm_command} ${package_string} ${options}",
onlyif => $install_check,
user => $user,
cwd => $target,
require => Class['nodejs'],
if $use_package_json {
exec { "npm_${npm_command}_${name}":
command => "${npm_path} ${npm_command} * ${options}",
onlyif => $list_command,
user => $user,
cwd => "${target}${dirsep}node_modules",
require => Class['nodejs'],
}
} else {
exec { "npm_${npm_command}_${name}":
command => "${npm_path} ${npm_command} ${package_string} ${options}",
onlyif => $install_check,
user => $user,
cwd => $target,
require => Class['nodejs'],
}
}
} else {
$npm_command = 'install'
Expand All @@ -60,13 +74,24 @@
Nodejs::Npm::Global_config_entry<| title == 'https-proxy' |> -> Exec["npm_install_${name}"]
Nodejs::Npm::Global_config_entry<| title == 'proxy' |> -> Exec["npm_install_${name}"]

exec { "npm_${npm_command}_${name}":
command => "${npm_path} ${npm_command} ${package_string} ${options}",
unless => $install_check,
user => $user,
cwd => $target,
environment => "HOME=${home_dir}",
require => Class['nodejs'],
if $use_package_json {
exec { "npm_${npm_command}_${name}":
command => "${npm_path} ${npm_command} ${options}",
unless => $list_command,
user => $user,
cwd => $target,
environment => "HOME=${home_dir}",
require => Class['nodejs'],
}
} else {
exec { "npm_${npm_command}_${name}":
command => "${npm_path} ${npm_command} ${package_string} ${options}",
unless => $install_check,
user => $user,
cwd => $target,
environment => "HOME=${home_dir}",
require => Class['nodejs'],
}
}
}
}
84 changes: 84 additions & 0 deletions spec/defines/nodejs_npm_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,26 @@
end
end

# npm install with package.json
context 'with package set to a valid value, ensure set to present and use_package_json set to true' do
let :params do
{
ensure: 'present',
package: 'express',
target: '/home/user/project',
use_package_json: true
}
end

it 'the command should be npm install' do
is_expected.to contain_exec('npm_install_express').with('command' => '/usr/bin/npm install ')
end

it 'the list_command should check if all deps are installed in /home/user/project/node_modules' do
is_expected.to contain_exec('npm_install_express').with('unless' => '/usr/bin/npm ls --long --parseable')
end
end

# npm rm <package>
context 'with package set to express and ensure set to absent' do
let :params do
Expand Down Expand Up @@ -356,6 +376,28 @@
)
end
end

# npm uninstall with package.json
context 'with package set to express, ensure set to absent and use_package_json set to true' do
let :params do
{
ensure: 'absent',
package: 'express',
target: '/home/user/project',
use_package_json: true
}
end

it 'the command should be npm rm * in subdirectory node_modules' do
is_expected.to contain_exec('npm_rm_express').with('command' => '/usr/bin/npm rm * ')
is_expected.to contain_exec('npm_rm_express').with('cwd' => '/home/user/project/node_modules')
end

it 'the list_command should check if modules are installed in /home/user/project/node_modules' do
is_expected.to contain_exec('npm_rm_express').with('onlyif' => '/usr/bin/npm ls --long --parseable')
is_expected.to contain_exec('npm_rm_express').with('cwd' => '/home/user/project/node_modules')
end
end
end

context 'when run on Darwin' do
Expand Down Expand Up @@ -682,6 +724,26 @@
end
end

# npm install with package.json
context 'with package set to a valid value, ensure set to present and use_package_json set to true' do
let :params do
{
ensure: 'present',
package: 'express',
target: 'C:\Users\test\project',
use_package_json: true
}
end

it 'the command should be npm install' do
is_expected.to contain_exec('npm_install_express').with('command' => '"C:\Program Files\nodejs\npm.cmd" install ')
end

it 'the list_command should check if all deps are installed in /home/user/project/node_modules' do
is_expected.to contain_exec('npm_install_express').with('unless' => '"C:\Program Files\nodejs\npm.cmd" ls --long --parseable')
end
end

# npm rm <package>
context 'with package set to express and ensure set to absent' do
let :params do
Expand Down Expand Up @@ -716,6 +778,28 @@
is_expected.to contain_exec('npm_rm_express').with('command' => '"C:\Program Files\nodejs\npm.cmd" rm express --save')
end
end

# npm uninstall with package.json
context 'with package set to express, ensure set to absent and use_package_json set to true' do
let :params do
{
ensure: 'absent',
package: 'express',
target: 'C:\Users\test\project',
use_package_json: true
}
end

it 'the command should be npm rm * in subdirectory node_modules' do
is_expected.to contain_exec('npm_rm_express').with('command' => '"C:\Program Files\nodejs\npm.cmd" rm * ')
is_expected.to contain_exec('npm_rm_express').with('cwd' => 'C:\Users\test\project\node_modules')
end

it 'the list_command should check if modules are installed in C:\Users\test\project\node_modules' do
is_expected.to contain_exec('npm_rm_express').with('onlyif' => '"C:\Program Files\nodejs\npm.cmd" ls --long --parseable')
is_expected.to contain_exec('npm_rm_express').with('cwd' => 'C:\Users\test\project\node_modules')
end
end
end
end
end