Merge pull request #269 from netmanagers/master

javierbertoli · web-flow · commit a30001da351d · 2021-03-11T14:09:15.000-03:00
Check nginx config before deploying &amp; various passenger fixes
diff --git a/.yamllint b/.yamllint
@@ -14,7 +14,7 @@ ignore: |
   .cache/
   .git/
   node_modules/
-  test/**/states/**/*.sls
+  test/salt/**/*.sls
   .kitchen/
 
 yaml-files:
diff --git a/nginx/config.sls b/nginx/config.sls
@@ -31,3 +31,6 @@ nginx_config:
     - context:
         config: {{ nginx.server.config|json(sort_keys=False) }}
 {% endif %}
+{% if nginx.check_config_before_apply %}
+    - check_cmd: /usr/sbin/nginx -t -c
+{% endif %}
diff --git a/nginx/map.jinja b/nginx/map.jinja
@@ -9,7 +9,7 @@
         'Debian': {
             'package': 'nginx',
             'passenger_package': 'passenger',
-            'passenger_config_file': '/etc/nginx/conf.d/passenger.conf',
+            'passenger_config_file': '/etc/nginx/conf.d/mod-http-passenger.conf',
             'service': 'nginx',
             'webuser': 'www-data',
             'conf_file': '/etc/nginx/nginx.conf',
@@ -112,6 +112,7 @@
     'install_from_ppa': False,
     'install_from_repo': False,
     'install_from_phusionpassenger': False,
+    'check_config_before_apply': False,
     'ppa_version': 'stable',
     'source_version': '1.10.0',
     'source_hash': '8ed647c3dd65bc4ced03b0e0f6bf9e633eff6b01bac772bcf97077d58bc2be4d',
diff --git a/nginx/passenger.sls b/nginx/passenger.sls
@@ -25,6 +25,7 @@ passenger_install:
       - pkg: nginx_install
     - require_in:
       - service: nginx_service
+      - file: nginx_config
 
 /etc/nginx/passenger.conf:
   file.absent:
@@ -46,6 +47,7 @@ passenger_config:
       - service: nginx_service
     - require_in:
       - service: nginx_service
+      - file: nginx_config
     - require:
       - file: /etc/nginx/passenger.conf
       - pkg: passenger_install
diff --git a/nginx/pkg.sls b/nginx/pkg.sls
@@ -163,7 +163,7 @@ nginx_phusionpassenger_yum_repo:
     - baseurl: 'https://oss-binaries.phusionpassenger.com/yum/passenger/el/$releasever/$basearch'
     - repo_gpgcheck: 1
     - gpgcheck: 0
-    - gpgkey: 'https://packagecloud.io/gpg.key'
+    - gpgkey: 'https://oss-binaries.phusionpassenger.com/yum/definitions/RPM-GPG-KEY.asc'
     - enabled: True
     - sslverify: 1
     - sslcacert: /etc/pki/tls/certs/ca-bundle.crt
diff --git a/nginx/snippets.sls b/nginx/snippets.sls
@@ -28,4 +28,9 @@ nginx_snippet_{{ snippet }}:
     - context:
         config: {{ config|json() }}
         nginx: {{ _nginx|json() }}
+    - require:
+      - file: nginx_snippets_dir
+    - require_in:
+      - file: nginx_config
+      - service: nginx_service
 {% endfor %}
diff --git a/pillar.example b/pillar.example
@@ -28,6 +28,17 @@ nginx:
   source_version: '1.10.0'
   source_hash: ''
 
+  # Check the configuration before applying:
+  # To prevent applying a configuration that might break nginx, set this
+  # parameter to true so the configuration is checked BEFORE applying. If
+  # the check fails, the state will fail and it won't be deployed.
+  # CAVEAT: As the configuration file is created in a temp dir, it can't
+  # have relative references or it will fail to check. You'll need to
+  # specify full paths where required (ie, `include`, `load_module`,
+  # `snippets`, etc.0
+  # Defaults to false
+  check_config_before_apply: false
+
   # These are usually set by grains in map.jinja
   # Typically you can comment these out.
   lookup:
diff --git a/test/integration/default/controls/config.rb b/test/integration/default/controls/config.rb
@@ -1,11 +1,13 @@
+# frozen_string_literal: true
+
 # Set defaults, use debian as base
 
 server_available = '/etc/nginx/sites-available'
-server_enabled	 = '/etc/nginx/sites-enabled'
+server_enabled = '/etc/nginx/sites-enabled'
 
 # Override by platform family
 case platform[:family]
-when 'redhat','fedora'
+when 'redhat', 'fedora'
   server_available = '/etc/nginx/conf.d'
   server_enabled   = '/etc/nginx/conf.d'
 when 'suse'
@@ -22,9 +24,13 @@
     it { should be_owned_by 'root' }
     it { should be_grouped_into 'root' }
     its('mode') { should cmp '0644' }
-    its('content') { should include %Q[    log_format main '$remote_addr - $remote_user [$time_local] $status '
+    its('content') do
+      # rubocop:disable Metrics/LineLength
+      should include %(    log_format main '$remote_addr - $remote_user [$time_local] $status '
                     '"$request" $body_bytes_sent "$http_referer" '
-                    '"$http_user_agent" "$http_x_forwarded_for"';] }
+                    '"$http_user_agent" "$http_x_forwarded_for"';)
+      # rubocop:enable Metrics/LineLength
+    end
   end
 
   # snippets configuration
@@ -40,12 +46,11 @@
 
   # sites configuration
   [server_available, server_enabled].each do |dir|
-
-    describe file ("#{dir}/default") do
-     it { should_not exist }
+    describe file "#{dir}/default" do
+      it { should_not exist }
     end
 
-    describe file ("#{dir}/mysite") do
+    describe file "#{dir}/mysite" do
       it { should be_file }
       it { should be_owned_by 'root' }
       it { should be_grouped_into 'root' }
@@ -57,6 +62,5 @@
       its('content') { should include 'try_files $uri $uri/ =404;' }
       its('content') { should include 'include snippets/letsencrypt.conf;' }
     end
-
   end
 end
diff --git a/test/integration/default/controls/install.rb b/test/integration/default/controls/install.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 control 'Nginx package' do
   title 'should be installed'
 
diff --git a/test/integration/default/controls/service.rb b/test/integration/default/controls/service.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 control 'Nginx service' do
   title 'should be running and enabled'
 
diff --git a/test/integration/passenger/README.md b/test/integration/passenger/README.md
@@ -0,0 +1,50 @@
+# InSpec Profile: `passenger`
+
+This shows the implementation of the `passenger` InSpec [profile](https://github.com/inspec/inspec/blob/master/docs/profiles.md).
+
+## Verify a profile
+
+InSpec ships with built-in features to verify a profile structure.
+
+```bash
+$ inspec check passenger
+Summary
+-------
+Location: passenger
+Profile: profile
+Controls: 4
+Timestamp: 2019-06-24T23:09:01+00:00
+Valid: true
+
+Errors
+------
+
+Warnings
+--------
+```
+
+## Execute a profile
+
+To run all **supported** controls on a local machine use `inspec exec /path/to/profile`.
+
+```bash
+$ inspec exec passenger
+..
+
+Finished in 0.0025 seconds (files took 0.12449 seconds to load)
+8 examples, 0 failures
+```
+
+## Execute a specific control from a profile
+
+To run one control from the profile use `inspec exec /path/to/profile --controls name`.
+
+```bash
+$ inspec exec passenger --controls package
+.
+
+Finished in 0.0025 seconds (files took 0.12449 seconds to load)
+1 examples, 0 failures
+```
+
+See an [example control here](https://github.com/inspec/inspec/blob/master/examples/profile/controls/example.rb).
diff --git a/test/integration/passenger/controls/config.rb b/test/integration/passenger/controls/config.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+# Set defaults, use debian as base
+
+# Override by OS Family
+case platform[:family]
+when 'redhat', 'centos', 'fedora'
+  server_available = '/etc/nginx/conf.d'
+  server_enabled   = '/etc/nginx/conf.d'
+  passenger_mod = '/usr/lib64/nginx/modules/ngx_http_passenger_module.so'
+  passenger_root = '/usr/share/ruby/vendor_ruby/phusion_passenger/locations.ini'
+  passenger_config_file = '/etc/nginx/conf.d/passenger.conf'
+  should_not_exist_file = '/etc/nginx/conf.d/mod-http-passenger.conf'
+when 'debian', 'ubuntu'
+  server_available = '/etc/nginx/sites-available'
+  server_enabled   = '/etc/nginx/sites-enabled'
+  passenger_mod = '/usr/lib/nginx/modules/ngx_http_passenger_module.so'
+  passenger_root = '/usr/lib/ruby/vendor_ruby/phusion_passenger/locations.ini'
+  passenger_config_file = '/etc/nginx/conf.d/mod-http-passenger.conf'
+  should_not_exist_file = '/etc/nginx/conf.d/passenger.conf'
+end
+
+control 'Passenger configuration' do
+  title 'should match desired lines'
+
+  # main configuration
+  describe file('/etc/nginx/nginx.conf') do
+    its('content') { should include "load_module #{passenger_mod}" }
+  end
+
+  describe file(passenger_config_file) do
+    it { should be_file }
+    it { should be_owned_by 'root' }
+    it { should be_grouped_into 'root' }
+    its('mode') { should cmp '0644' }
+    its('content') { should include "passenger_root #{passenger_root};" }
+    its('content') { should include 'passenger_ruby /usr/bin/ruby;' }
+  end
+
+  describe file(should_not_exist_file) do
+    it { should_not exist }
+  end
+
+  # sites configuration
+  [server_available, server_enabled].each do |dir|
+    describe file "#{dir}/default" do
+      it { should_not exist }
+    end
+
+    describe file "#{dir}/mysite" do
+      it { should be_file }
+      it { should be_owned_by 'root' }
+      it { should be_grouped_into 'root' }
+      its('mode') { should cmp '0644' }
+      its('content') { should include 'passenger_enabled on;' }
+    end
+  end
+end
diff --git a/test/integration/passenger/controls/install.rb b/test/integration/passenger/controls/install.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+control 'Nginx package' do
+  title 'should be installed'
+
+  describe package('nginx') do
+    it { should be_installed }
+  end
+end
+
+control 'Passenger packages' do
+  title 'should be installed'
+
+  # Override by OS Family
+  passenger_mod_pkg = case platform[:family]
+                      when 'redhat', 'centos', 'fedora'
+                        'nginx-mod-http-passenger'
+                      when 'debian', 'ubuntu'
+                        'libnginx-mod-http-passenger'
+                      end
+
+  describe package('passenger') do
+    it { should be_installed }
+  end
+  describe package(passenger_mod_pkg) do
+    it { should be_installed }
+  end
+end
diff --git a/test/integration/passenger/controls/service.rb b/test/integration/passenger/controls/service.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+control 'Nginx service' do
+  title 'should be running and enabled'
+
+  describe service('nginx') do
+    it { should be_enabled }
+    it { should be_running }
+  end
+end
+
+control 'Passenger module' do
+  title 'should be running and enabled'
+
+  describe 'Passenger engine' do
+    it 'passenger-config should say configuration "looks good"' do
+      expect(command(
+        '/usr/bin/passenger-config validate-install --auto'
+      ).stdout).to match(/looks good/)
+    end
+
+    it 'passenger-memory-stats should return Passenger stats' do
+      expect(command('/usr/sbin/passenger-memory-stats').stdout).to match(
+        %r{nginx: master process /usr/sbin/nginx.*Passenger watchdog.*Passenger core.*}m
+      )
+    end
+  end
+end
diff --git a/test/integration/passenger/inspec.yml b/test/integration/passenger/inspec.yml
@@ -0,0 +1,12 @@
+# -*- coding: utf-8 -*-
+# vim: ft=yaml
+---
+name: default
+title: nginx formula
+maintainer: SaltStack Formulas
+license: Apache-2.0
+summary: Verify that the nginx formula is setup and configured correctly
+supports:
+  - platform-name: debian
+  - platform-name: ubuntu
+  - platform-name: centos
diff --git a/test/salt/passenger/pillar/nginx.sls b/test/salt/passenger/pillar/nginx.sls

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+# frozen_string_literal: true`
	`2`	`+`
`1`	`3`	`control 'Nginx package' do`
`2`	`4`	`title 'should be installed'`
`3`	`5`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+# frozen_string_literal: true`
	`2`	`+`
`1`	`3`	`control 'Nginx service' do`
`2`	`4`	`title 'should be running and enabled'`
`3`	`5`