Merge branch '230717-add-mail-smtp-pool-gem' into 'master'

Support SMTP connection pooling when sending emails

See merge request gitlab-org/gitlab!57805

.gitlab-ci.yml

@@ -102,6 +102,7 @@ include:
   - local: .gitlab/ci/qa.gitlab-ci.yml
   - local: .gitlab/ci/reports.gitlab-ci.yml
   - local: .gitlab/ci/rails.gitlab-ci.yml
+  - local: .gitlab/ci/vendored-gems.gitlab-ci.yml
   - local: .gitlab/ci/review.gitlab-ci.yml
   - local: .gitlab/ci/rules.gitlab-ci.yml
   - local: .gitlab/ci/setup.gitlab-ci.yml

.gitlab/ci/rules.gitlab-ci.yml

@@ -920,6 +920,16 @@
     - <<: *if-merge-request
       changes: [".gitlab/ci/rails.gitlab-ci.yml"]
 
+#######################
+# Vendored gems rules #
+#######################
+
+.vendor:rules:mail-smtp_pool:
+  rules:
+    - <<: *if-merge-request
+      changes: ["vendor/gems/mail-smtp_pool/**/*"]
+    - <<: *if-merge-request-title-run-all-rspec
+
 ##################
 # Releases rules #
 ##################

.gitlab/ci/vendored-gems.gitlab-ci.yml

+vendor mail-smtp_pool:
+  extends:
+    - .vendor:rules:mail-smtp_pool
+  needs: []
+  trigger:
+    include: vendor/gems/mail-smtp_pool/.gitlab-ci.yml
+    strategy: depend

Gemfile

@@ -511,6 +511,7 @@ gem 'erubi', '~> 1.9.0'
 # Monkey-patched in `config/initializers/mail_encoding_patch.rb`
 # See https://gitlab.com/gitlab-org/gitlab/issues/197386
 gem 'mail', '= 2.7.1'
+gem 'mail-smtp_pool', '~> 0.1.0', path: 'vendor/gems/mail-smtp_pool', require: false
 
 # File encryption
 gem 'lockbox', '~> 0.6.2'

Gemfile.lock

+PATH
+  remote: vendor/gems/mail-smtp_pool
+  specs:
+    mail-smtp_pool (0.1.0)
+      connection_pool (~> 2.0)
+      mail (~> 2.7)
+
 PATH
   remote: vendor/shims/mimemagic
   specs:
@@ -1483,6 +1490,7 @@ DEPENDENCIES
   loofah (~> 2.2)
   lru_redux
   mail (= 2.7.1)
+  mail-smtp_pool (~> 0.1.0)!
   marginalia (~> 1.10.0)
   memory_profiler (~> 0.9)
   method_source (~> 1.0)

changelogs/unreleased/230717-add-mail-smtp-pool-gem.yml

+---
+title: Add support for SMTP connection pooling when sending emails
+merge_request: 57805
+author:
+type: added

config/initializers/smtp_settings.rb.sample

@@ -22,3 +22,28 @@ if Rails.env.production?
     openssl_verify_mode: 'peer' # See ActionMailer documentation for other possible options
   }
 end
+
+# To use an SMTP connection pool, uncomment the following section:
+#
+# require 'mail/smtp_pool'
+#
+# ActionMailer::Base.add_delivery_method :smtp_pool, Mail::SMTPPool
+#
+# if Rails.env.production?
+#   Rails.application.config.action_mailer.delivery_method = :smtp_pool
+#
+#   ActionMailer::Base.delivery_method = :smtp_pool
+#   ActionMailer::Base.smtp_pool_settings = {
+#     pool: Mail::SMTPPool.create_pool(
+#       pool_size: Gitlab::Runtime.max_threads,
+#       address: "email.server.com",
+#       port: 465,
+#       user_name: "smtp",
+#       password: "123456",
+#       domain: "gitlab.company.com",
+#       authentication: :login,
+#       enable_starttls_auto: true,
+#       openssl_verify_mode: 'peer' # See ActionMailer documentation for other possible options
+#     )
+#   }
+# end

vendor/gems/mail-smtp_pool/.gitignore

+Gemfile.lock
+*.gem
+.bundle

vendor/gems/mail-smtp_pool/.gitlab-ci.yml

+workflow:
+  rules:
+    - if: $CI_MERGE_REQUEST_ID
+
+.rspec:
+  cache:
+    key: mail-smtp_pool-ruby
+    paths:
+      - vendor/gems/mail-smtp_pool/vendor/ruby
+  before_script:
+    - cd vendor/gems/mail-smtp_pool
+    - ruby -v                                   # Print out ruby version for debugging
+    - gem install bundler --no-document         # Bundler is not installed with the image
+    - bundle config set --local path 'vendor'   # Install dependencies into ./vendor/ruby
+    - bundle install -j $(nproc)
+  script:
+    - bundle exec rspec
+
+rspec-2.6:
+  image: "ruby:2.6"
+  extends: .rspec
+
+rspec-2.7:
+  image: "ruby:2.7"
+  extends: .rspec
+
+rspec-3.0:
+  image: "ruby:3.0"
+  extends: .rspec

vendor/gems/mail-smtp_pool/Gemfile

+# frozen_string_literal: true
+
+source 'https://rubygems.org'
+
+gemspec

vendor/gems/mail-smtp_pool/Gemfile.lock

+PATH
+  remote: .
+  specs:
+    mail-smtp_pool (0.1.0)
+      connection_pool (~> 2.0)
+      mail (~> 2.7)
+
+GEM
+  remote: https://rubygems.org/
+  specs:
+    connection_pool (2.2.3)
+    diff-lcs (1.4.4)
+    mail (2.7.1)
+      mini_mime (>= 0.1.1)
+    mini_mime (1.0.2)
+    rspec (3.10.0)
+      rspec-core (~> 3.10.0)
+      rspec-expectations (~> 3.10.0)
+      rspec-mocks (~> 3.10.0)
+    rspec-core (3.10.1)
+      rspec-support (~> 3.10.0)
+    rspec-expectations (3.10.1)
+      diff-lcs (>= 1.2.0, < 2.0)
+      rspec-support (~> 3.10.0)
+    rspec-mocks (3.10.2)
+      diff-lcs (>= 1.2.0, < 2.0)
+      rspec-support (~> 3.10.0)
+    rspec-support (3.10.2)
+
+PLATFORMS
+  ruby
+
+DEPENDENCIES
+  mail-smtp_pool!
+  rspec (~> 3.10.0)
+
+BUNDLED WITH
+   2.1.4

vendor/gems/mail-smtp_pool/LICENSE

+The MIT License (MIT)
+
+Copyright (c) 2016-2021 GitLab B.V.
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+THE SOFTWARE.

vendor/gems/mail-smtp_pool/README.md

+# Mail::SMTPPool
+
+This gem is an extension to `Mail` that allows delivery of emails using an SMTP connection pool
+
+## Installation
+
+Add this line to your application's Gemfile:
+
+```ruby
+gem 'mail-smtp_pool'
+```
+
+And then execute:
+
+```shell
+bundle
+```
+
+Or install it yourself as:
+
+```shell
+gem install mail-smtp_pool
+```
+
+## Usage with ActionMailer
+
+```ruby
+# config/environments/development.rb
+
+Rails.application.configure do
+  ...
+
+  ActionMailer::Base.add_delivery_method :smtp_pool, Mail::SMTPPool
+
+  config.action_mailer.perform_deliveries = true
+  config.action_mailer.smtp_pool_settings = {
+    pool: Mail::SMTPPool.create_pool(
+      pool_size:            5,
+      pool_timeout:         5,
+      address:              'smtp.gmail.com',
+      port:                 587,
+      domain:               'example.com',
+      user_name:            '<username>',
+      password:             '<password>',
+      authentication:       'plain',
+      enable_starttls_auto: true
+    )
+  }
+end
+```
+
+Configuration options:
+
+* `pool_size` - The maximum number of SMTP connections in the pool. Connections are created lazily as needed.
+* `pool_timeout` - The number of seconds to wait for a connection in the pool to be available. A `Timeout::Error` exception is raised when this is exceeded.
+
+This also accepts all options supported by `Mail::SMTP`. See https://www.rubydoc.info/gems/mail/2.6.1/Mail/SMTP for more information.

vendor/gems/mail-smtp_pool/lib/mail/smtp_pool.rb

+# frozen_string_literal: true
+
+require 'connection_pool'
+require 'mail/smtp_pool/connection'
+
+module Mail
+  class SMTPPool
+    POOL_DEFAULTS = {
+      pool_size: 5,
+      pool_timeout: 5
+    }.freeze
+
+    class << self
+      def create_pool(settings = {})
+        pool_settings = POOL_DEFAULTS.merge(settings)
+        smtp_settings = settings.reject { |k, v| POOL_DEFAULTS.keys.include?(k) }
+
+        ConnectionPool.new(size: pool_settings[:pool_size], timeout: pool_settings[:pool_timeout]) do
+          Mail::SMTPPool::Connection.new(smtp_settings)
+        end
+      end
+    end
+
+    def initialize(settings)
+      raise ArgumentError, 'pool is required. You can create one using Mail::SMTPPool.create_pool.' if settings[:pool].nil?
+
+      @pool = settings[:pool]
+    end
+
+    def deliver!(mail)
+      @pool.with { |conn| conn.deliver!(mail) }
+    end
+  end
+end

vendor/gems/mail-smtp_pool/lib/mail/smtp_pool/connection.rb

+# frozen_string_literal: true
+
+# A connection object that can be used to deliver mail.
+#
+# This is meant to be used in a pool so the main difference between this
+# and Mail::SMTP is that this expects deliver! to be called multiple times.
+#
+# SMTP connection reset and error handling is handled by this class and
+# the SMTP connection is not closed after a delivery.
+
+require 'mail'
+
+module Mail
+  class SMTPPool
+    class Connection < Mail::SMTP
+      def initialize(values)
+        super
+
+        @smtp_session = nil
+      end
+
+      def deliver!(mail)
+        response = Mail::SMTPConnection.new(connection: smtp_session, return_response: true).deliver!(mail)
+
+        settings[:return_response] ? response : self
+      end
+
+      def finish
+        finish_smtp_session if @smtp_session && @smtp_session.started?
+      end
+
+      private
+
+      def smtp_session
+        return start_smtp_session if @smtp_session.nil? || !@smtp_session.started?
+        return @smtp_session if reset_smtp_session
+
+        finish_smtp_session
+        start_smtp_session
+      end
+
+      def start_smtp_session
+        @smtp_session = build_smtp_session.start(settings[:domain], settings[:user_name], settings[:password], settings[:authentication])
+      end
+
+      def reset_smtp_session
+        !@smtp_session.instance_variable_get(:@error_occurred) && @smtp_session.rset.success?
+      rescue Net::SMTPError, IOError
+        false
+      end
+
+      def finish_smtp_session
+        @smtp_session.finish
+      rescue Net::SMTPError, IOError
+      ensure
+        @smtp_session = nil
+      end
+    end
+  end
+end

vendor/gems/mail-smtp_pool/mail-smtp_pool.gemspec

+# frozen_string_literal: true
+
+lib = File.expand_path('lib', __dir__)
+$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
+
+Gem::Specification.new do |spec|
+  spec.name = 'mail-smtp_pool'
+  spec.version = '0.1.0'
+  spec.authors = ['Heinrich Lee Yu']
+  spec.email = ['heinrich@gitlab.com']
+
+  spec.summary = 'Mail extension for sending using an SMTP connection pool'
+  spec.homepage = 'https://gitlab.com/gitlab-org/gitlab/-/tree/master/vendor/gems/mail-smtp_pool'
+  spec.metadata = { 'source_code_uri' => 'https://gitlab.com/gitlab-org/gitlab/-/tree/master/vendor/gems/mail-smtp_pool' }
+  spec.license = 'MIT'
+
+  spec.files = Dir['lib/**/*.rb']
+  spec.require_paths = ['lib']
+
+  # Please maintain alphabetical order for dependencies
+  spec.add_runtime_dependency 'connection_pool', '~> 2.0'
+  spec.add_runtime_dependency 'mail', '~> 2.7'
+
+  # Please maintain alphabetical order for dev dependencies
+  spec.add_development_dependency 'rspec', '~> 3.10.0'
+end

vendor/gems/mail-smtp_pool/spec/lib/mail/smtp_pool/connection_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Mail::SMTPPool::Connection do
+  let(:connection) { described_class.new({}) }
+  let(:mail) do
+    Mail.new do
+      from    'mikel@test.lindsaar.net'
+      to      'you@test.lindsaar.net'
+      subject 'This is a test email'
+      body    'Test body'
+    end
+  end
+
+  after do
+    MockSMTP.clear_deliveries
+  end
+
+  describe '#deliver!' do
+    it 'delivers mail using the same SMTP connection' do
+      mock_smtp = MockSMTP.new
+
+      expect(Net::SMTP).to receive(:new).once.and_return(mock_smtp)
+      expect(mock_smtp).to receive(:sendmail).twice.and_call_original
+      expect(mock_smtp).to receive(:rset).once.and_call_original
+
+      connection.deliver!(mail)
+      connection.deliver!(mail)
+
+      expect(MockSMTP.deliveries.size).to eq(2)
+    end
+
+    context 'when RSET fails' do
+      let(:mock_smtp) { MockSMTP.new }
+      let(:mock_smtp_2) { MockSMTP.new }
+
+      before do
+        expect(Net::SMTP).to receive(:new).twice.and_return(mock_smtp, mock_smtp_2)
+      end
+
+      context 'with an IOError' do
+        before do
+          expect(mock_smtp).to receive(:rset).once.and_raise(IOError)
+        end
+
+        it 'creates a new SMTP connection' do
+          expect(mock_smtp).to receive(:sendmail).once.and_call_original
+          expect(mock_smtp).to receive(:finish).once.and_call_original
+          expect(mock_smtp_2).to receive(:sendmail).once.and_call_original
+
+          connection.deliver!(mail)
+          connection.deliver!(mail)
+
+          expect(MockSMTP.deliveries.size).to eq(2)
+        end
+      end
+
+      context 'with an SMTP error' do
+        before do
+          expect(mock_smtp).to receive(:rset).once.and_raise(Net::SMTPServerBusy)
+        end
+
+        it 'creates a new SMTP connection' do
+          expect(mock_smtp).to receive(:sendmail).once.and_call_original
+          expect(mock_smtp).to receive(:finish).once.and_call_original
+          expect(mock_smtp_2).to receive(:sendmail).once.and_call_original
+
+          connection.deliver!(mail)
+          connection.deliver!(mail)
+
+          expect(MockSMTP.deliveries.size).to eq(2)
+        end
+
+        context 'and closing the old connection fails' do
+          before do
+            expect(mock_smtp).to receive(:finish).once.and_raise(IOError)
+          end
+
+          it 'creates a new SMTP connection' do
+            expect(mock_smtp).to receive(:sendmail).once.and_call_original
+            expect(mock_smtp_2).to receive(:sendmail).once.and_call_original
+
+            connection.deliver!(mail)
+            connection.deliver!(mail)
+
+            expect(MockSMTP.deliveries.size).to eq(2)
+          end
+        end
+      end
+    end
+  end
+end

vendor/gems/mail-smtp_pool/spec/lib/mail/smtp_pool_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Mail::SMTPPool do
+  describe '.create_pool' do
+    it 'sets the default pool settings' do
+      expect(ConnectionPool).to receive(:new).with(size: 5, timeout: 5).once
+
+      described_class.create_pool
+    end
+
+    it 'allows overriding pool size and timeout' do
+      expect(ConnectionPool).to receive(:new).with(size: 3, timeout: 2).once
+
+      described_class.create_pool(pool_size: 3, pool_timeout: 2)
+    end
+
+    it 'creates an SMTP connection with the correct settings' do
+      settings = { address: 'smtp.example.com', port: '465' }
+
+      smtp_pool = described_class.create_pool(settings)
+
+      expect(Mail::SMTPPool::Connection).to receive(:new).with(settings).once.and_call_original
+
+      smtp_pool.checkout
+    end
+  end
+
+  describe '#initialize' do
+    it 'raises an error if a pool is not specified' do
+      expect { described_class.new({}) }.to raise_error(
+        ArgumentError, 'pool is required. You can create one using Mail::SMTPPool.create_pool.'
+      )
+    end
+  end
+
+  describe '#deliver!' do
+    let(:mail) do
+      Mail.new do
+        from    'mikel@test.lindsaar.net'
+        to      'you@test.lindsaar.net'
+        subject 'This is a test email'
+        body    'Test body'
+      end
+    end
+
+    after do
+      MockSMTP.clear_deliveries
+    end
+
+    it 'delivers mail using a connection from the pool' do
+      connection_pool = double(ConnectionPool)
+      connection = double(Mail::SMTPPool::Connection)
+
+      expect(connection_pool).to receive(:with).and_yield(connection)
+      expect(connection).to receive(:deliver!).with(mail)
+
+      described_class.new(pool: connection_pool).deliver!(mail)
+    end
+
+    it 'delivers mail' do
+      described_class.new(pool: described_class.create_pool).deliver!(mail)
+
+      expect(MockSMTP.deliveries.size).to eq(1)
+    end
+  end
+end

vendor/gems/mail-smtp_pool/spec/spec_helper.rb

+# frozen_string_literal: true
+
+require 'mail/smtp_pool'
+
+# Original mockup from ActionMailer
+# Based on https://github.com/mikel/mail/blob/22a7afc23f253319965bf9228a0a430eec94e06d/spec/spec_helper.rb#L74-L138
+class MockSMTP
+  def self.deliveries
+    @@deliveries
+  end
+
+  def self.security
+    @@security
+  end
+
+  def initialize
+    @@deliveries = []
+    @@security = nil
+    @started = false
+  end
+
+  def sendmail(mail, from, to)
+    @@deliveries << [mail, from, to]
+    'OK'
+  end
+
+  def rset
+    Net::SMTP::Response.parse('250 OK')
+  end
+
+  def start(*args)
+    @started = true
+
+    if block_given?
+      result = yield(self)
+      @started = false
+
+      return result
+    else
+      return self
+    end
+  end
+
+  def started?
+    @started
+  end
+
+  def finish
+    @started = false
+    return true
+  end
+
+  def self.clear_deliveries
+    @@deliveries = []
+  end
+
+  def self.clear_security
+    @@security = nil
+  end
+
+  def enable_tls(context)
+    raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @@security && @@security != :enable_tls
+    @@security = :enable_tls
+    context
+  end
+
+  def enable_starttls(context = nil)
+    raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @@security == :enable_tls
+    @@security = :enable_starttls
+    context
+  end
+
+  def enable_starttls_auto(context)
+    raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @@security == :enable_tls
+    @@security = :enable_starttls_auto
+    context
+  end
+end
+
+class Net::SMTP
+  def self.new(*args)
+    MockSMTP.new
+  end
+end