From e610728b750e7e5a19ba135dfe943b46b0abe8e8 Mon Sep 17 00:00:00 2001 From: Kenyon Ralph Date: Sun, 7 Mar 2021 16:27:34 -0800 Subject: [PATCH] nodejs::npm::global_config_entry: reimplement using ini_setting Manage npm global config entries using the ini_setting resource instead of exec resources using `npm config` commands. Closes #439 --- .fixtures.yml | 13 +-- lib/facter/npm_globalconfig_path.rb | 3 + manifests/npm/global_config_entry.pp | 79 ++------------ metadata.json | 4 + spec/acceptance/class_spec.rb | 49 +++------ spec/defines/global_config_entry_spec.rb | 125 +++++------------------ 6 files changed, 64 insertions(+), 209 deletions(-) create mode 100644 lib/facter/npm_globalconfig_path.rb diff --git a/.fixtures.yml b/.fixtures.yml index 1a297683..6bda1a82 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -1,9 +1,10 @@ fixtures: repositories: - "stdlib": "git://github.com/puppetlabs/puppetlabs-stdlib.git" - "apt": "git://github.com/puppetlabs/puppetlabs-apt.git" - "portage": "git://github.com/gentoo/puppet-portage.git" - "chocolatey": "git://github.com/puppetlabs/puppetlabs-chocolatey.git" + apt: git://github.com/puppetlabs/puppetlabs-apt.git + chocolatey: git://github.com/puppetlabs/puppetlabs-chocolatey.git + inifile: git://github.com/puppetlabs/puppetlabs-inifile.git + portage: git://github.com/gentoo/puppet-portage.git + stdlib: git://github.com/puppetlabs/puppetlabs-stdlib.git yumrepo_core: - repo: https://github.com/puppetlabs/puppetlabs-yumrepo_core.git - puppet_version: ">= 6.0.0" + repo: git://github.com/puppetlabs/puppetlabs-yumrepo_core.git + puppet_version: '>= 6.0.0' diff --git a/lib/facter/npm_globalconfig_path.rb b/lib/facter/npm_globalconfig_path.rb new file mode 100644 index 00000000..483a1fbd --- /dev/null +++ b/lib/facter/npm_globalconfig_path.rb @@ -0,0 +1,3 @@ +Facter.add('npm_globalconfig_path') do + setcode 'npm config get globalconfig' +end diff --git a/manifests/npm/global_config_entry.pp b/manifests/npm/global_config_entry.pp index 73d3101d..ca488b24 100644 --- a/manifests/npm/global_config_entry.pp +++ b/manifests/npm/global_config_entry.pp @@ -1,78 +1,13 @@ # See README.md for usage information. define nodejs::npm::global_config_entry ( Enum['present', 'absent'] $ensure = 'present', - $config_setting = $title, - $cmd_exe_path = $nodejs::cmd_exe_path, - $npm_path = $nodejs::npm_path, - $value = undef, + String[1] $config_setting = $title, + Optional[String[1]] $value = undef, ) { - include nodejs - - case $ensure { - 'absent': { - $command = "config delete ${config_setting} --global" - - # If this is a secret key, determine if it is set properly outside of NPM - # https://github.com/voxpupuli/puppet-nodejs/issues/326 - if $config_setting =~ /(_|:_)/ { - $onlyif_command = $facts['os']['family'] ? { - 'Windows' => "${cmd_exe_path} /C FOR /F %G IN ('${npm_path} config get globalconfig') DO IF EXIST %G (FINDSTR /B /C:\"${$config_setting}\" %G) ELSE (EXIT 1)", - default => "test -f $(${npm_path} config get globalconfig) && grep -qe \"^${$config_setting}\" $(${npm_path} config get globalconfig)", - } - } - else { - $onlyif_command = $facts['os']['family'] ? { - 'Windows' => "${cmd_exe_path} /C ${npm_path} get --global| FINDSTR /B \"${config_setting}\"", - default => "${npm_path} get --global | grep -e \"^${config_setting}\"", - } - } - } - default: { - $command = "config set ${config_setting} ${value} --global" - - # If this is a secret key, determine if it is set properly outside of NPM - # https://github.com/voxpupuli/puppet-nodejs/issues/326 - if $config_setting =~ /(_|:_)/ { - $onlyif_command = $facts['os']['family'] ? { - 'Windows' => "${cmd_exe_path} /V /C FOR /F %G IN ('${npm_path} config get globalconfig') DO IF EXIST %G (FINDSTR /B /C:\"${$config_setting}=\\\"${$value}\\\"\" %G & IF !ERRORLEVEL! EQU 0 ( EXIT 1 ) ELSE ( EXIT 0 )) ELSE ( EXIT 0 )", - default => "! test -f $(${npm_path} config get globalconfig) || ! grep -qe '^${$config_setting}=\"\\?${$value}\"\\?$' $(${npm_path} config get globalconfig)", - } - } - else { - $onlyif_command = $facts['os']['family'] ? { - 'Windows' => "${cmd_exe_path} /C FOR /F %i IN ('${npm_path} get ${config_setting} --global') DO IF \"%i\" NEQ \"${value}\" ( EXIT 0 ) ELSE ( EXIT 1 )", - default => "test \"$(${npm_path} get ${config_setting} --global | tr -d '\n')\" != \"${value}\"", - } - } - } - } - - if $nodejs::npm_package_ensure != 'absent' { - $exec_require = "Package[${nodejs::npm_package_name}]" - } elsif $nodejs::repo_class == '::nodejs::repo::nodesource' { - $exec_require = "Package[${nodejs::nodejs_package_name}]" - } else { - $exec_require = undef - } - - #Determine exec provider - $provider = $facts['os']['family'] ? { - 'Windows' => 'windows', - default => 'shell', - } - - # set a sensible path on Unix - $exec_path = $facts['os']['family'] ? { - 'Windows' => undef, - 'Darwin' => ['/bin', '/usr/bin', '/opt/local/bin', '/usr/local/bin'], - default => ['/bin', '/usr/bin', '/usr/local/bin'], - } - - exec { "npm_config ${ensure} ${title}": - command => "${npm_path} ${command}", - path => $exec_path, - provider => $provider, - onlyif => $onlyif_command, - require => $exec_require, + ini_setting { $title: + ensure => $ensure, + path => $facts['npm_globalconfig_path'], + setting => $config_setting, + value => $value, } } diff --git a/metadata.json b/metadata.json index 3e765651..c12e1890 100644 --- a/metadata.json +++ b/metadata.json @@ -69,6 +69,10 @@ { "name": "puppetlabs/stdlib", "version_requirement": ">= 4.25.0 < 7.0.0" + }, + { + "name": "puppetlabs/inifile", + "version_requirement": ">= 4.0.0 < 6.0.0" } ] } diff --git a/spec/acceptance/class_spec.rb b/spec/acceptance/class_spec.rb index e16a52c0..bb7a340f 100644 --- a/spec/acceptance/class_spec.rb +++ b/spec/acceptance/class_spec.rb @@ -39,46 +39,29 @@ expect(pkg_output.stdout).to match 'epel' end end + end +end - context 'set global_config_entry secret', if: fact('os.family') == 'RedHat' do - let :pp do - "class { 'nodejs': }; nodejs::npm::global_config_entry { '//path.to.registry/:_secret': ensure => present, value => 'cGFzc3dvcmQ=', require => Package[nodejs],}" - end - - it_behaves_like 'an idempotent resource' - - describe package('nodejs') do - it { is_expected.to be_installed } - end - - describe 'npm config' do - it 'contains the global_config_entry secret' do - npm_output = shell('cat $(/usr/bin/npm config get globalconfig)') - expect(npm_output.stdout).to match '//path.to.registry/:_secret="cGFzc3dvcmQ="' - end - end + context 'set a global_config_entry' do + let :pp do + <<~PUPPET + class { 'nodejs': } + nodejs::npm::global_config_entry { '//path.to.registry/:_secret': + ensure => present, + value => 'cGFzc3dvcmQ=', + } + PUPPET end - context 'set global_config_entry secret unquoted', if: fact('os.family') == 'RedHat' do - let :pp do - "class { 'nodejs': }; nodejs::npm::global_config_entry { '//path.to.registry/:_secret': ensure => present, value => 'cGFzc3dvcmQ', require => Package[nodejs],}" - end - - it_behaves_like 'an idempotent resource' - - describe package('nodejs') do - it { is_expected.to be_installed } - end + it_behaves_like 'an idempotent resource' - describe 'npm config' do - it 'contains the global_config_entry secret' do - npm_output = shell('cat $(/usr/bin/npm config get globalconfig)') - expect(npm_output.stdout).to match '//path.to.registry/:_secret=cGFzc3dvcmQ' - end + describe 'npm config' do + it 'contains the global_config_entry secret' do + npm_output = shell('cat $(npm config get globalconfig)') + expect(npm_output.stdout).to match '//path.to.registry/:_secret = cGFzc3dvcmQ=' end end end -end # Must uninstall the default nodesource repo and packages which come from there before attempting # to install native packages. diff --git a/spec/defines/global_config_entry_spec.rb b/spec/defines/global_config_entry_spec.rb index 15221b57..8f64ff0a 100644 --- a/spec/defines/global_config_entry_spec.rb +++ b/spec/defines/global_config_entry_spec.rb @@ -11,24 +11,21 @@ facts end - let(:npm_path) do - if facts[:os]['family'] == 'FreeBSD' - '/usr/local/bin/npm' - else - '/usr/bin/npm' - end - end - context 'with name set to proxy and value set to proxy.domain' do let(:title) { 'proxy' } let :params do { - value: 'proxy.domain' + ensure: 'present', + value: 'proxy.domain', } end - it 'npm config set proxy proxy.domain should be executed' do - is_expected.to contain_exec('npm_config present proxy').with('command' => "#{npm_path} config set proxy proxy.domain --global") + it do + is_expected.to contain_ini_setting(title).with( + ensure: params[:ensure], + setting: title, + value: params[:value], + ) end end @@ -36,12 +33,17 @@ let(:title) { 'https-proxy' } let :params do { + ensure: 'present', value: 'proxy.domain' } end - it 'npm config set https-proxy proxy.domain should be executed' do - is_expected.to contain_exec('npm_config present https-proxy').with('command' => "#{npm_path} config set https-proxy proxy.domain --global") + it do + is_expected.to contain_ini_setting(title).with( + ensure: params[:ensure], + setting: title, + value: params[:value], + ) end end @@ -53,8 +55,11 @@ } end - it 'npm config delete color should be executed' do - is_expected.to contain_exec('npm_config absent color').with('command' => "#{npm_path} config delete color --global") + it do + is_expected.to contain_ini_setting(title).with( + ensure: params[:ensure], + setting: title, + ) end end @@ -62,93 +67,17 @@ let(:title) { '//path.to.registry/:_secret' } let :params do { + ensure: 'present', value: 'cGFzc3dvcmQ=', - ensure: 'present' } end - it 'npm config set :_secret should be executed' do - is_expected.to contain_exec('npm_config present //path.to.registry/:_secret').with('command' => "#{npm_path} config set //path.to.registry/:_secret cGFzc3dvcmQ= --global") - end - end - - context 'with ensure npm package set to present' do - let(:pre_condition) do - <<-PUPPET - class { 'nodejs': - nodejs_package_name => 'node-package-name', - npm_package_name => 'npm-package-name', - npm_package_ensure => present, - } - PUPPET - end - let(:title) { 'prefer-online' } - let :params do - { - value: 'true' - } - end - - it 'npm config set prefer-online should be executed and require npm package' do - is_expected.to contain_exec('npm_config present prefer-online').with('command' => "#{npm_path} config set prefer-online true --global").that_requires('Package[npm-package-name]') - end - - it 'npm config set prefer-online should not require node package' do - is_expected.not_to contain_exec('npm_config present prefer-online').with('command' => "#{npm_path} config set prefer-online true --global").that_requires('Package[node-package-name]') - end - end - - context 'with ensure npm package set to absent and repo class set to nodesource' do - let(:pre_condition) do - <<-PUPPET - class { 'nodejs': - nodejs_package_name => 'node-package-name', - npm_package_ensure => absent, - repo_class => '::nodejs::repo::nodesource', - } - PUPPET - end - let(:title) { 'loglevel' } - let :params do - { - value: 'debug' - } - end - - it 'npm config set loglevel should be executed and require nodejs package' do - is_expected.to contain_exec('npm_config present loglevel').with('command' => "#{npm_path} config set loglevel debug --global").that_requires('Package[node-package-name]') - end - end - - context 'with ensure npm package set to absent and repo class set to something else' do - let(:pre_condition) do - <<-PUPPET - class something_else { } - class { 'nodejs': - nodejs_package_name => 'node-package-name', - npm_package_name => 'npm-package-name', - npm_package_ensure => absent, - repo_class => '::something_else', - } - PUPPET - end - let(:title) { 'init-version' } - let :params do - { - value: '0.0.1' - } - end - - it 'npm config set init-version should be executed' do - is_expected.to contain_exec('npm_config present init-version').with('command' => "#{npm_path} config set init-version 0.0.1 --global") - end - - it 'npm config set init-version should not require npm package' do - is_expected.not_to contain_exec('npm_config present init-version').with('command' => "#{npm_path} config set init-version 0.0.1 --global").that_requires('Package[npm-package-name]') - end - - it 'npm config set init-version should not require node package' do - is_expected.not_to contain_exec('npm_config present init-version').with('command' => "#{npm_path} config set init-version 0.0.1 --global").that_requires('Package[node-package-name]') + it do + is_expected.to contain_ini_setting(title).with( + ensure: params[:ensure], + setting: title, + value: params[:value], + ) end end end