Commit eb1f67fc authored by Nathan Ward's avatar Nathan Ward
Browse files

Make sql pool_connect_timeout conditional around FreeRADIUS v3.1.x 

Only set the sql connect_timout in the config if:
- it (via `pool_connect_timeout`) is intentionally set in the params, or
- we are on FR3.1.x, in which case set the default of 3.0
parent 4d560eec

README.md

+1 −0
Original line number Diff line number Diff line
@@ -1536,6 +1536,7 @@ be closed. Default: 60. 


Connection timeout (in seconds). The maximum amount of time to wait for a new

connection to be established. Default: '3.0'.

This parameter should only be set when using FreeRADIUS 3.1.x.



#### `freeradius::statusclient`

manifests/sql.pp

+35 −1
Original line number Diff line number Diff line
@@ -31,7 +31,7 @@ define freeradius::sql ( 
  Optional[Integer] $pool_min                    = 1,

  Optional[Integer] $pool_spare                  = 1,

  Optional[Integer] $pool_idle_timeout           = 60,

  Optional[Float] $pool_connect_timeout          = 3.0,

  Optional[Float] $pool_connect_timeout          = undef,

) {

  $fr_package          = $::freeradius::params::fr_package

  $fr_service          = $::freeradius::params::fr_service
@@ -52,6 +52,40 @@ define freeradius::sql ( 
    fail('$num_sql_socks must be an integer')

  }



  # Warn if the user tries to set a FreeRADIUS 3.1.x specific parameter, and

  # we detect that they are not on (or not installing) a FreeRADIUS 3.1.x

  # then show them some errors

  # Additionally, if we are on FreeRADIUS 3.1.x then allow defaults for some

  # parameters, otherwise leave them set as specified when this define

  # is called.

  if $::freeradius::fr_3_1 {

    if $pool_connect_timeout != undef {

      warning(@("WARN"/L)

        The `pool_connect_timeout` parameter requires FreeRADIUS 3.1.x, \

        i.e. the experimental branch. You are running \

        `${facts['freeradius_version']}`. In the future, attempting to set \

        it on this version may fail.

        |-WARN

      )

    }



    $resolved_pool_connect_timeout = $pool_connect_timeout ? {

      undef   => 3.0,

      default => $pool_connect_timeout,

    }



  } else {

    if $pool_connect_timeout != undef {

      fail(@("FAIL"/L)

        The `pool_connect_timeout` parameter requires FreeRADIUS 3.1.x, \

        i.e. the experimental branch. You are running \

        `${facts['freeradius_version']}`.

        |-FAIL

      )

    }

  }





  # Determine default location of query file

  $queryfile = "${fr_basepath}/sql/queries.conf"

spec/defines/sql_spec.rb

+54 −1
Original line number Diff line number Diff line
@@ -5,7 +5,11 @@ describe 'freeradius::sql' do 
    context "on #{os}" do

      include_context 'freeradius_default'



      let(:facts) { os_facts }

      let(:facts) do

        os_facts.merge(

          freeradius_version: '3.0.21',

        )

      end



      let(:title) { 'test' }
@@ -26,6 +30,7 @@ describe 'freeradius::sql' do 
          .with_content(%r{^\s+login = "radius"$})

          .with_content(%r{^\s+password = "test_password"$})

          .with_content(%r{^\s+postauth_table = "radpostauth"$})

          .without_content(%r{^\s+connect_timeout = .*})

          .with_ensure('present')

          .with_group('radiusd')

          .with_mode('0640')
@@ -79,6 +84,54 @@ describe 'freeradius::sql' do 
            .with_source('puppet:///modules/path/to/custom/query/file')

        end

      end



      context 'when freeradius::fr_3_1 is true' do

        let(:facts) do

          super().merge(

            'freeradius_version' => '3.1.1',

          )

        end



        let(:node_params) do

          {

            'freeradius::fr_3_1' => true,

          }

        end



        it do

          is_expected.to contain_file('/etc/raddb/mods-available/test')

          .with_content(%r{^\s+connect_timeout = 3.0})

        end



        context 'with pool_connect_timeout specified' do

          let(:params) do

            super().merge(

              pool_connect_timeout: 5.0,

            )

          end



          it do

            is_expected.to contain_file('/etc/raddb/mods-available/test')

            .with_content(%r{^\s+connect_timeout = 5.0})

          end



          # it do

          #   expect(catalogue).to satisfy('contain connect_timeout warning') { |c| c.resource_refs.any? { |r| r =~ %r{^warning_test: The `pool_connect_timeout` parameter requires FreeRADIUS 3.1.x.*In the future/} } }

          # end

        end

      end



      # context 'with pool_connect_timeout specified' do

      #   let(:params) do

      #     super().merge(

      #       pool_connect_timeout: 5.0,

      #     )

      #   end



      #   it do

      #     is_expected.to compile.and_raise_error(%r{^The `pool_connect_timeout` parameter requires FreeRADIUS 3.1.x})

      #   end

      # end

    end

  end

end

templates/sql.conf.erb

+3 −2
Original line number Diff line number Diff line
@@ -333,7 +333,7 @@ sql <%= @name %> { 
		#  unused for this length of time will be closed.

#		idle_timeout = 60

		idle_timeout = <%= @pool_idle_timeout %>



<%- unless @resolved_pool_connect_timeout.nil? -%>

		#  Connection timeout (in seconds).  The maximum amount of

		#  time to wait for a new connection to be established.

		#  Not supported by:
@@ -341,8 +341,9 @@ sql <%= @name %> { 
		#    rlm_sql_oracle - Not possible.

		#    rlm_sql_postgresql - Should be set via the radius_db string instead.

		#

		connect_timeout = <%= @pool_connect_timeout %>

		connect_timeout = <%= @resolved_pool_connect_timeout %>



<%- end -%>

		#  NOTE: All configuration settings are enforced.  If a

		#  connection is closed because of "idle_timeout",

		#  "uses", or "lifetime", then the total number of