Fix file permissions of /opt/stackstorm/packs and /opt/stackstorm/virtualenvs

.fixtures.yml

@@ -21,3 +21,4 @@ fixtures:
     selinux: "puppet/selinux"
     nginx: "puppet/nginx"
     nodejs: "puppet/nodejs"
+    recursive_file_permissions: "npwalker/recursive_file_permissions"

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)
 

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'

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'

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'

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'

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'

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'

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'

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'

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'

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'

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,

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<||>
 }

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

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": [

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'

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
       {

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