From 532047919a65a12999cf33b5360532044e6de0cb Mon Sep 17 00:00:00 2001
From: Hunter Haugen <hunter@puppetlabs.com>
Date: Thu, 21 Aug 2014 11:12:23 -0700
Subject: [PATCH] Remove firewall management

The firewall management really belongs outside of the module that
manages postgresql, like in a profile.
---
 README.md                                     | 11 +------
 manifests/globals.pp                          |  2 --
 manifests/params.pp                           | 14 --------
 manifests/server.pp                           |  3 --
 manifests/server/firewall.pp                  | 14 --------
 metadata.json                                 |  4 ---
 .../firewall_default_parameters_spec.rb       | 32 -------------------
 spec/spec_helper_acceptance.rb                |  1 -
 spec/unit/classes/server_spec.rb              | 12 -------
 9 files changed, 1 insertion(+), 92 deletions(-)
 delete mode 100644 manifests/server/firewall.pp
 delete mode 100644 spec/acceptance/firewall_default_parameters_spec.rb

diff --git a/README.md b/README.md
index f3d5875..d3f4bac 100644
--- a/README.md
+++ b/README.md
@@ -26,7 +26,7 @@ The PostgreSQL module allows you to easily manage postgres databases with Puppet
 Module Description
 -------------------
 
-PostgreSQL is a high-performance, free, open-source relational database server. The postgresql module allows you to manage PostgreSQL packages and services on several operating systems, while also supporting basic management of PostgreSQL databases and users. The module offers support for managing firewall for postgres ports on RedHat-based distros, as well as support for basic management of common security settings.
+PostgreSQL is a high-performance, free, open-source relational database server. The postgresql module allows you to manage PostgreSQL packages and services on several operating systems, while also supporting basic management of PostgreSQL databases and users. The module offers support for basic management of common security settings.
 
 Setup
 -----
@@ -35,7 +35,6 @@ Setup
 
 * package/service/configuration files for PostgreSQL
 * listened-to ports
-* system firewall (optional)
 * IP and mask (optional)
 
 **Introductory Questions**
@@ -44,7 +43,6 @@ The postgresql module offers many security configuration settings. Before gettin
 
 * Do you want/need to allow remote connections?
     * If yes, what about TCP connections?
-* Would you prefer to work around your current firewall settings or overwrite some of them?
 * How restrictive do you want the database superuser's permissions to be?
 
 Your answers to these questions will determine which of the module's parameters you'll want to specify values for.
@@ -71,7 +69,6 @@ For a more customized configuration:
       ip_mask_allow_all_users    => '0.0.0.0/0',
       listen_addresses           => '*',
       ipv4acls                   => ['hostssl all johndoe 192.168.0.0/24 cert'],
-      manage_firewall            => true,
       postgres_password          => 'TPSrep0rt!',
     }
 
@@ -376,9 +373,6 @@ This will set the default database locale for all databases created with this mo
 
 On Debian you'll need to ensure that the 'locales-all' package is installed for full functionality of Postgres.
 
-####`firewall_supported`
-This allows you to override the automated detection to see if your OS supports the `firewall` module.
-
 ####`manage_package_repo`
 If `true` this will setup the official PostgreSQL repositories on your host. Defaults to `false`.
 
@@ -470,9 +464,6 @@ This will set the default database locale for all databases created with this mo
 
 On Debian you'll need to ensure that the 'locales-all' package is installed for full functionality of Postgres.
 
-####`manage_firewall`
-This value defaults to `false`. Many distros ship with a fairly restrictive firewall configuration which will block the port that postgres tries to listen on. If you'd like for the puppet module to open this port for you (using the [puppetlabs-firewall](http://forge.puppetlabs.com/puppetlabs/firewall) module), change this value to true. Check the documentation for `puppetlabs/firewall` to ensure the rest of the global setup is applied, to ensure things like persistence and global rules are set correctly.
-
 ####`manage_pg_hba_conf`
 This value defaults to `true`. Whether or not manage the pg_hba.conf. If set to `true`, puppet will overwrite this file. If set to `false`, puppet will not modify the file.
 
diff --git a/manifests/globals.pp b/manifests/globals.pp
index 1998364..32aa9c0 100644
--- a/manifests/globals.pp
+++ b/manifests/globals.pp
@@ -41,10 +41,8 @@ class postgresql::globals (
   $encoding             = undef,
   $locale               = undef,
 
-  $manage_firewall      = undef,
   $manage_pg_hba_conf   = undef,
   $manage_pg_ident_conf = undef,
-  $firewall_supported   = undef,
 
   $manage_package_repo  = undef
 ) {
diff --git a/manifests/params.pp b/manifests/params.pp
index a581b9c..add8ceb 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -13,7 +13,6 @@ class postgresql::params inherits postgresql::globals {
   $service_ensure             = 'running'
   $service_enable             = true
   $service_provider           = $service_provider
-  $manage_firewall            = $manage_firewall
   $manage_pg_hba_conf         = pick($manage_pg_hba_conf, true)
   $manage_pg_ident_conf       = pick($manage_pg_ident_conf, true)
   $package_ensure             = 'present'
@@ -24,7 +23,6 @@ class postgresql::params inherits postgresql::globals {
       $user               = pick($user, 'postgres')
       $group              = pick($group, 'postgres')
       $needs_initdb       = pick($needs_initdb, true)
-      $firewall_supported = pick($firewall_supported, true)
       $version_parts      = split($version, '[.]')
       $package_version    = "${version_parts[0]}${version_parts[1]}"
 
@@ -75,10 +73,6 @@ class postgresql::params inherits postgresql::globals {
     }
 
     'Archlinux': {
-      # Based on the existing version of the firewall module, this is normally
-      # true for Archlinux, but archlinux users want more control.
-      # so they can set it themself
-      $firewall_supported = pick($firewall_supported, true)
       $needs_initdb       = pick($needs_initdb, true)
       $user               = pick($user, 'postgres')
       $group              = pick($group, 'postgres')
@@ -146,8 +140,6 @@ class postgresql::params inherits postgresql::globals {
       $confdir              = pick($confdir, "/etc/postgresql/${version}/main")
       $service_status       = pick($service_status, "/etc/init.d/${service_name} status | /bin/egrep -q 'Running clusters: .+|online'")
       $psql_path            = pick($psql_path, "/usr/bin/psql")
-
-      $firewall_supported   = pick($firewall_supported, true)
     }
 
     'FreeBSD': {
@@ -170,7 +162,6 @@ class postgresql::params inherits postgresql::globals {
       $service_status       = pick($service_status, "/usr/local/etc/rc.d/${service_name} status")
       $psql_path            = pick($psql_path, "${bindir}/psql")
 
-      $firewall_supported   = pick($firewall_supported, false)
       $needs_initdb         = pick($needs_initdb, true)
     }
 
@@ -194,15 +185,10 @@ class postgresql::params inherits postgresql::globals {
       $service_status       = pick($service_status, "/etc/init.d/${service_name} status")
       $psql_path            = pick($psql_path, "${bindir}/psql")
 
-      $firewall_supported   = pick($firewall_supported, false)
       $needs_initdb         = pick($needs_initdb, true)
     }
 
     default: {
-      # Based on the existing version of the firewall module, this is normally
-      # false for other OS, but this allows an escape hatch to override it.
-      $firewall_supported = pick($firewall_supported, false)
-
       $psql_path            = pick($psql_path, "${bindir}/psql")
 
       # Since we can't determine defaults on our own, we rely on users setting
diff --git a/manifests/server.pp b/manifests/server.pp
index fdd71e8..960815b 100644
--- a/manifests/server.pp
+++ b/manifests/server.pp
@@ -42,10 +42,8 @@ class postgresql::server (
   $encoding                   = $postgresql::params::encoding,
   $locale                     = $postgresql::params::locale,
 
-  $manage_firewall            = $postgresql::params::manage_firewall,
   $manage_pg_hba_conf         = $postgresql::params::manage_pg_hba_conf,
   $manage_pg_ident_conf       = $postgresql::params::manage_pg_ident_conf,
-  $firewall_supported         = $postgresql::params::firewall_supported,
 
   #Deprecated
   $version                    = undef,
@@ -68,6 +66,5 @@ class postgresql::server (
   class { "${pg}::config": }->
   class { "${pg}::service": }->
   class { "${pg}::passwd": }->
-  class { "${pg}::firewall": }->
   anchor { "${pg}::end": }
 }
diff --git a/manifests/server/firewall.pp b/manifests/server/firewall.pp
deleted file mode 100644
index e8bb4b7..0000000
--- a/manifests/server/firewall.pp
+++ /dev/null
@@ -1,14 +0,0 @@
-# PRIVATE CLASS: do not use directly
-class postgresql::server::firewall {
-  $manage_firewall    = $postgresql::server::manage_firewall
-  $firewall_supported = $postgresql::server::firewall_supported
-  $port               = $postgresql::server::port
-
-  if ($manage_firewall and $firewall_supported) {
-    firewall { "${port} accept - postgres":
-      port   => $port,
-      proto  => 'tcp',
-      action => 'accept',
-    }
-  }
-}
diff --git a/metadata.json b/metadata.json
index 7ab77aa..4c7b0d6 100644
--- a/metadata.json
+++ b/metadata.json
@@ -71,10 +71,6 @@
       "name": "puppetlabs/stdlib",
       "version_requirement": "4.x"
     },
-    {
-      "name": "puppetlabs/firewall",
-      "version_requirement": ">= 0.0.4"
-    },
     {
       "name": "puppetlabs/apt",
       "version_requirement": ">=1.1.0 <2.0.0"
diff --git a/spec/acceptance/firewall_default_parameters_spec.rb b/spec/acceptance/firewall_default_parameters_spec.rb
deleted file mode 100644
index aea3c6b..0000000
--- a/spec/acceptance/firewall_default_parameters_spec.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-require 'spec_helper_acceptance'
-
-# These tests are designed to ensure that the module with firewall enabled,
-# when ran with defaults, sets up everything correctly and allows us to connect
-# to Postgres.
-describe 'postgres::server with firewall', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfamily')) do
-  it 'with defaults' do
-    pp = <<-EOS
-      class { 'firewall': } ->
-      class { 'postgresql::server':
-        manage_firewall => true,
-      }
-    EOS
-
-    apply_manifest(pp, :catch_failures => true)
-    apply_manifest(pp, :catch_changes => true)
-  end
-
-  describe port(5432) do
-    it { is_expected.to be_listening }
-  end
-
-  it 'can connect with psql' do
-    psql('--command="\l" postgres', 'postgres') do |r|
-      expect(r.stdout).to match(/List of databases/)
-    end
-  end
-
-end
-
-
-
diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb
index 8618c1f..97685b1 100644
--- a/spec/spec_helper_acceptance.rb
+++ b/spec/spec_helper_acceptance.rb
@@ -97,7 +97,6 @@ RSpec.configure do |c|
       end
 
       on host, puppet('module','install','puppetlabs-stdlib'), { :acceptable_exit_codes => [0,1] }
-      on host, puppet('module','install','puppetlabs-firewall'), { :acceptable_exit_codes => [0,1] }
       on host, puppet('module','install','puppetlabs-apt'), { :acceptable_exit_codes => [0,1] }
       on host, puppet('module','install','puppetlabs-concat'), { :acceptable_exit_codes => [0,1] }
     end
diff --git a/spec/unit/classes/server_spec.rb b/spec/unit/classes/server_spec.rb
index d7ae4d0..e6d0852 100644
--- a/spec/unit/classes/server_spec.rb
+++ b/spec/unit/classes/server_spec.rb
@@ -39,18 +39,6 @@ describe 'postgresql::server', :type => :class do
     end
   end
 
-  describe 'manage_firewall => true' do
-    let(:params) do
-      {
-        :manage_firewall => true,
-      }
-    end
-
-    it 'should create firewall rule' do
-      is_expected.to contain_firewall("5432 accept - postgres")
-    end
-  end
-
   describe 'package_ensure => absent' do
     let(:params) do
       {