diff --git a/.fixtures.yml b/.fixtures.yml index 576564a3..a36b92b3 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -21,3 +21,4 @@ fixtures: selinux: "puppet/selinux" nginx: "puppet/nginx" nodejs: "puppet/nodejs" + recursive_file_permissions: "npwalker/recursive_file_permissions" diff --git a/CHANGELOG.md b/CHANGELOG.md index 136a1c20..58f7be44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Development +- Fixed `/opt/stackstorm/packs` and `/opt/stackstorm/virtualenv` resources to be idempotent + and manage the ownership of these directories recursively in a much more efficient manner. + Instead of using the `file` resource with `recurse => true` we now utilize the module + `npwalker/recursive_file_permissions`. #278 (Bugfix) (Enhancement) + Contributed by @nmaludy ## 1.6.0 (Feb 17, 2020) diff --git a/build/centos6-puppet5/Puppetfile b/build/centos6-puppet5/Puppetfile index 25115941..01c7bb79 100644 --- a/build/centos6-puppet5/Puppetfile +++ b/build/centos6-puppet5/Puppetfile @@ -62,3 +62,4 @@ mod 'computology-packagecloud' mod 'puppet-selinux' mod 'puppet-nginx' mod 'puppet-nodejs' +mod 'npwalker-recursive_file_permissions' diff --git a/build/centos6-puppet6/Puppetfile b/build/centos6-puppet6/Puppetfile index 25115941..01c7bb79 100644 --- a/build/centos6-puppet6/Puppetfile +++ b/build/centos6-puppet6/Puppetfile @@ -62,3 +62,4 @@ mod 'computology-packagecloud' mod 'puppet-selinux' mod 'puppet-nginx' mod 'puppet-nodejs' +mod 'npwalker-recursive_file_permissions' diff --git a/build/centos7-puppet5/Puppetfile b/build/centos7-puppet5/Puppetfile index 25115941..01c7bb79 100644 --- a/build/centos7-puppet5/Puppetfile +++ b/build/centos7-puppet5/Puppetfile @@ -62,3 +62,4 @@ mod 'computology-packagecloud' mod 'puppet-selinux' mod 'puppet-nginx' mod 'puppet-nodejs' +mod 'npwalker-recursive_file_permissions' diff --git a/build/centos7-puppet6/Puppetfile b/build/centos7-puppet6/Puppetfile index 25115941..01c7bb79 100644 --- a/build/centos7-puppet6/Puppetfile +++ b/build/centos7-puppet6/Puppetfile @@ -62,3 +62,4 @@ mod 'computology-packagecloud' mod 'puppet-selinux' mod 'puppet-nginx' mod 'puppet-nodejs' +mod 'npwalker-recursive_file_permissions' diff --git a/build/ubuntu14-puppet5/Puppetfile b/build/ubuntu14-puppet5/Puppetfile index 25115941..01c7bb79 100644 --- a/build/ubuntu14-puppet5/Puppetfile +++ b/build/ubuntu14-puppet5/Puppetfile @@ -62,3 +62,4 @@ mod 'computology-packagecloud' mod 'puppet-selinux' mod 'puppet-nginx' mod 'puppet-nodejs' +mod 'npwalker-recursive_file_permissions' diff --git a/build/ubuntu14-puppet6/Puppetfile b/build/ubuntu14-puppet6/Puppetfile index 25115941..01c7bb79 100644 --- a/build/ubuntu14-puppet6/Puppetfile +++ b/build/ubuntu14-puppet6/Puppetfile @@ -62,3 +62,4 @@ mod 'computology-packagecloud' mod 'puppet-selinux' mod 'puppet-nginx' mod 'puppet-nodejs' +mod 'npwalker-recursive_file_permissions' diff --git a/build/ubuntu16-puppet5/Puppetfile b/build/ubuntu16-puppet5/Puppetfile index 25115941..01c7bb79 100644 --- a/build/ubuntu16-puppet5/Puppetfile +++ b/build/ubuntu16-puppet5/Puppetfile @@ -62,3 +62,4 @@ mod 'computology-packagecloud' mod 'puppet-selinux' mod 'puppet-nginx' mod 'puppet-nodejs' +mod 'npwalker-recursive_file_permissions' diff --git a/build/ubuntu16-puppet6/Puppetfile b/build/ubuntu16-puppet6/Puppetfile index 25115941..01c7bb79 100644 --- a/build/ubuntu16-puppet6/Puppetfile +++ b/build/ubuntu16-puppet6/Puppetfile @@ -62,3 +62,4 @@ mod 'computology-packagecloud' mod 'puppet-selinux' mod 'puppet-nginx' mod 'puppet-nodejs' +mod 'npwalker-recursive_file_permissions' diff --git a/build/ubuntu18-puppet5/Puppetfile b/build/ubuntu18-puppet5/Puppetfile index 72f2b1e8..621c9f49 100644 --- a/build/ubuntu18-puppet5/Puppetfile +++ b/build/ubuntu18-puppet5/Puppetfile @@ -64,3 +64,4 @@ mod 'puppet-nginx' mod 'puppet-nodejs' mod 'puppetlabs-inifile' mod 'puppetlabs-stdlib' +mod 'npwalker-recursive_file_permissions' diff --git a/build/ubuntu18-puppet6/Puppetfile b/build/ubuntu18-puppet6/Puppetfile index 72f2b1e8..621c9f49 100644 --- a/build/ubuntu18-puppet6/Puppetfile +++ b/build/ubuntu18-puppet6/Puppetfile @@ -64,3 +64,4 @@ mod 'puppet-nginx' mod 'puppet-nodejs' mod 'puppetlabs-inifile' mod 'puppetlabs-stdlib' +mod 'npwalker-recursive_file_permissions' diff --git a/manifests/init.pp b/manifests/init.pp index 1475c537..055005f3 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -65,6 +65,8 @@ # Set the number of actionrunner processes to start # @param packs # Hash of st2 packages to be installed +# @param packs_group +# Name of the group that will own the /opt/stackstorm/packs directory (default: st2packs) # @param index_url # Url to the StackStorm Exchange index file. (default undef) # @param mistral_db_host @@ -245,6 +247,7 @@ $cli_auth_url = "http://${::st2::params::hostname}:${::st2::params::auth_port}", $actionrunner_workers = $::st2::params::actionrunner_workers, $packs = {}, + $packs_group = $::st2::params::packs_group_name, $index_url = undef, $mistral_db_host = $::st2::params::hostname, $mistral_db_name = $::st2::params::mistral_db_name, diff --git a/manifests/pack.pp b/manifests/pack.pp index dbbb9985..199474c8 100644 --- a/manifests/pack.pp +++ b/manifests/pack.pp @@ -24,43 +24,6 @@ include st2 $_cli_username = $::st2::cli_username $_cli_password = $::st2::cli_password - $_st2_packs_group = $::st2::params::packs_group_name - - ensure_resource('group', $_st2_packs_group, { - 'ensure' => present, - }) - - ensure_resource('file', '/opt/stackstorm', { - 'ensure' => 'directory', - 'owner' => 'root', - 'group' => 'root', - 'mode' => '0755', - }) - - ensure_resource('file', '/opt/stackstorm/packs', { - 'ensure' => 'directory', - 'owner' => 'root', - 'group' => $_st2_packs_group, - 'recurse' => true, - 'tag' => 'st2::subdirs', - }) - - ensure_resource('file', '/opt/stackstorm/configs', { - 'ensure' => 'directory', - 'owner' => 'st2', - 'group' => 'root', - 'mode' => '0755', - 'tag' => 'st2::subdirs', - }) - - ensure_resource('file', '/opt/stackstorm/virtualenvs', { - 'ensure' => 'directory', - 'owner' => 'root', - 'group' => $_st2_packs_group, - 'mode' => '0755', - 'tag' => 'st2::subdirs', - }) - st2_pack { $pack: ensure => $ensure, @@ -86,10 +49,6 @@ ~> Exec<| tag == 'st2::register-configs' |> } - Group[$_st2_packs_group] - -> File['/opt/stackstorm'] - -> File<| tag == 'st2::subdirs' |> - Service<| tag == 'st2::service' |> -> St2_pack<||> Exec<| tag == 'st2::reload' |> -> St2_pack<||> } diff --git a/manifests/profile/server.pp b/manifests/profile/server.pp index 37bf95e3..c1643a04 100644 --- a/manifests/profile/server.pp +++ b/manifests/profile/server.pp @@ -65,6 +65,7 @@ $rabbitmq_port = $::st2::rabbitmq_port, $rabbitmq_vhost = $::st2::rabbitmq_vhost, $index_url = $::st2::index_url, + $packs_group = $::st2::packs_group_name, ) inherits st2 { include st2::notices include st2::params @@ -100,6 +101,46 @@ 'tag' => 'st2::server', }) + ensure_resource('group', $packs_group, { + 'ensure' => present, + }) + + ensure_resource('file', '/opt/stackstorm/configs', { + 'ensure' => 'directory', + 'owner' => 'st2', + 'group' => 'root', + 'mode' => '0755', + 'tag' => 'st2::server', + }) + + ensure_resource('file', '/opt/stackstorm/packs', { + 'ensure' => 'directory', + 'owner' => 'root', + 'group' => $packs_group, + 'mode' => '0775', + 'tag' => 'st2::server', + }) + + ensure_resource('file', '/opt/stackstorm/virtualenvs', { + 'ensure' => 'directory', + 'owner' => 'root', + 'group' => $packs_group, + 'mode' => '0755', + 'tag' => 'st2::server', + }) + + recursive_file_permissions { '/opt/stackstorm/packs': + owner => 'root', + group => $packs_group, + tag => 'st2::server', + } + + recursive_file_permissions { '/opt/stackstorm/virtualenvs': + owner => 'root', + group => $packs_group, + tag => 'st2::server', + } + ######################################## ## Config file { $conf_dir: @@ -371,4 +412,7 @@ Service<| tag == 'st2::service' |> ~> Exec<| tag == 'st2::reload' |> + + St2_pack<||> + ~> Recursive_file_permissions<| tag == 'st2::server' |> } diff --git a/metadata.json b/metadata.json index acd037bb..5cd42592 100644 --- a/metadata.json +++ b/metadata.json @@ -67,6 +67,10 @@ { "name": "puppet/nodejs", "version_requirement": ">= 1.3.0 < 8.0.0" + }, + { + "name": "npwalker/recursive_file_permissions", + "version_requirement": ">= 0.6.0 < 1.0.0" } ], "operatingsystem_support": [ diff --git a/spec/classes/profile/server_spec.rb b/spec/classes/profile/server_spec.rb new file mode 100644 index 00000000..9fac4d3f --- /dev/null +++ b/spec/classes/profile/server_spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' + +describe 'st2::profile::server' do + all_os = { + supported_os: [ + { + 'operatingsystem' => 'RedHat', + 'operatingsystemrelease' => ['6', '7'], + }, + { + 'operatingsystem' => 'Ubuntu', + 'operatingsystemrelease' => ['16.04', '18.04'], + }, + ], + } + + on_supported_os(all_os).each do |os, os_facts| + let(:facts) do + os_facts.merge( + sudoversion: '1.8.23', + ) + end + + context "on #{os} with default options" do + it { is_expected.to compile.with_all_deps } + it { is_expected.to contain_class('st2::profile::server') } + + it do + is_expected.to contain_file('/opt/stackstorm').with( + ensure: 'directory', + owner: 'root', + group: 'root', + mode: '0755', + tag: 'st2::server', + ) + end + + it { is_expected.to contain_group('st2packs') } + + it do + is_expected.to contain_file('/opt/stackstorm/configs').with( + ensure: 'directory', + owner: 'st2', + group: 'root', + mode: '0755', + tag: 'st2::server', + ) + end + + it do + is_expected.to contain_file('/opt/stackstorm/packs').with( + ensure: 'directory', + owner: 'root', + group: 'st2packs', + tag: 'st2::server', + ) + end + + it do + is_expected.to contain_file('/opt/stackstorm/virtualenvs').with( + ensure: 'directory', + owner: 'root', + group: 'st2packs', + tag: 'st2::server', + ) + end + + it do + is_expected.to contain_recursive_file_permissions('/opt/stackstorm/packs').with( + owner: 'root', + group: 'st2packs', + tag: 'st2::server', + ) + end + + it do + is_expected.to contain_recursive_file_permissions('/opt/stackstorm/virtualenvs').with( + owner: 'root', + group: 'st2packs', + tag: 'st2::server', + ) + end + end # context 'on #{os} with default options' + end # on_supported_os(all_os) +end # describe 'st2::profile::server' diff --git a/spec/defines/pack_spec.rb b/spec/defines/pack_spec.rb index 201d7327..9e2fb300 100644 --- a/spec/defines/pack_spec.rb +++ b/spec/defines/pack_spec.rb @@ -17,33 +17,6 @@ } end - it do - is_expected.to contain_file('/opt/stackstorm').with( - ensure: 'directory', - owner: 'root', - group: 'root', - mode: '0755', - ) - end - it do - is_expected.to contain_file('/opt/stackstorm/configs').with( - ensure: 'directory', - owner: 'st2', - group: 'root', - mode: '0755', - ) - end - it do - is_expected.to contain_file('/opt/stackstorm/packs').with( - ensure: 'directory', - owner: 'root', - group: 'st2packs', - ) - end - it do - is_expected.to contain_group('st2packs') - end - context 'when declared with config' do let(:params) do { diff --git a/test/integration/stackstorm/controls/st2_test.rb b/test/integration/stackstorm/controls/st2_test.rb index eedcf447..dbef22dd 100644 --- a/test/integration/stackstorm/controls/st2_test.rb +++ b/test/integration/stackstorm/controls/st2_test.rb @@ -48,4 +48,25 @@ it { should_not be_executable.by('owner') } it { should_not be_executable.by('group') } end + + describe file('/opt/stackstorm/configs') do + it { should exist } + its('owner') { should eq 'st2' } + its('group') { should eq 'root' } + its('mode') { should cmp '0755' } + end + + describe file('/opt/stackstorm/packs') do + it { should exist } + its('owner') { should eq 'root' } + its('group') { should eq 'st2packs' } + its('mode') { should cmp '0775' } + end + + describe file('/opt/stackstorm/virtualenvs') do + it { should exist } + its('owner') { should eq 'root' } + its('group') { should eq 'st2packs' } + its('mode') { should cmp '0755' } + end end