Merge branch '1590-add-webhook-delivery-method-option-to-config-mail_room-yml' into 'master'

Add webhook delivery method options to config/mail_room.yml

See merge request gitlab-org/gitlab!80832

config/gitlab.yml.example

@@ -230,7 +230,23 @@ production: &base
     #   client_id: "YOUR-CLIENT-ID"
     #   client_secret: "YOUR-CLIENT-SECRET"
 
-    # File that contains the shared secret key for verifying access for mailroom's incoming_email.
+    # How mailroom delivers email content to Rails. There are two methods at the moment:
+    # - sidekiq: mailroom pushes the email content to Sidekiq directly. This job
+    # is then picked up by Sidekiq.
+    # - webhook: mailroom triggers a HTTP POST request to Rails web server. The
+    # content is embedded into the request body.
+    # Default is sidekiq.
+    # delivery_method: sidekiq
+
+    # When the delivery method is webhook, those configs tell the url that
+    # mailroom can contact to. Note that the combined url must not end with "/".
+    # At the moment, the webhook delivery method doesn't support HTTP/HTTPs via
+    # UNIX socket.
+    # gitlab_url: "http://gitlab.example"
+
+    # When the delivery method is webhook, this config is the file that
+    # contains the shared secret key for verifying access for mailroom's
+    # incoming_email.
     # Default is '.gitlab_mailroom_secret' relative to Rails.root (i.e. root of the GitLab app).
     # secret_file: /home/git/gitlab/.gitlab_mailroom_secret
 

config/mail_room.yml

 :mailboxes:
   <%
     require_relative "../lib/gitlab/mail_room" unless defined?(Gitlab::MailRoom)
-    Gitlab::MailRoom.enabled_configs.each do |_key, config|
+    Gitlab::MailRoom.enabled_configs.each do |key, config|
   %>
     -
       :host: <%= config[:host].to_json %>
@@ -26,6 +26,7 @@
       <%= config.slice(:inbox_options).to_yaml(indentation: 8).gsub(/^---\n/, '') %>
       <% end %>
 
+      <% if config[:delivery_method] == Gitlab::MailRoom::DELIVERY_METHOD_SIDEKIQ %>
       :delivery_method: sidekiq
       :delivery_options:
         :redis_url: <%= config[:redis_url].to_json %>
@@ -41,6 +42,15 @@
             :port: <%= sentinel[:port] %>
           <% end %>
         <% end %>
+      <% elsif config[:delivery_method] == Gitlab::MailRoom::DELIVERY_METHOD_WEBHOOK %>
+      :delivery_method: postback
+      :delivery_options:
+        :delivery_url: <%= config[:gitlab_url] %>/api/v4/internal/mail_room/<%= key %>
+        :jwt_auth_header: <%= Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER %>
+        :jwt_issuer: <%= Gitlab::MailRoom::INTERNAL_API_REQUEST_JWT_ISSUER %>
+        :jwt_algorithm: "HS256"
+        :jwt_secret_path: <%= config[:secret_file] %>
+      <% end %>
 
       :arbitration_method: redis
       :arbitration_options:

lib/gitlab/mail_room.rb

@@ -12,6 +12,11 @@ module Gitlab
   module MailRoom
     RAILS_ROOT_DIR = Pathname.new('../..').expand_path(__dir__).freeze
 
+    DELIVERY_METHOD_SIDEKIQ = 'sidekiq'
+    DELIVERY_METHOD_WEBHOOK = 'webhook'
+    INTERNAL_API_REQUEST_HEADER = 'Gitlab-Mailroom-Api-Request'
+    INTERNAL_API_REQUEST_JWT_ISSUER = 'gitlab-mailroom'
+
     DEFAULT_CONFIG = {
       enabled: false,
       port: 143,
@@ -20,7 +25,8 @@ module Gitlab
       mailbox: 'inbox',
       idle_timeout: 60,
       log_path: RAILS_ROOT_DIR.join('log', 'mail_room_json.log'),
-      expunge_deleted: false
+      expunge_deleted: false,
+      delivery_method: DELIVERY_METHOD_SIDEKIQ
     }.freeze
 
     # Email specific configuration which is merged with configuration
@@ -63,7 +69,9 @@ module Gitlab
         return {} unless File.exist?(config_file)
 
         config = merged_configs(config_key)
+
         config.merge!(redis_config) if enabled?(config)
+
         config[:log_path] = File.expand_path(config[:log_path], RAILS_ROOT_DIR)
 
         config

lib/gitlab/mail_room/authenticator.rb

@@ -6,8 +6,6 @@ module Gitlab
       include JwtAuthenticatable
 
       SecretConfigurationError = Class.new(StandardError)
-      INTERNAL_API_REQUEST_HEADER = 'Gitlab-Mailroom-Api-Request'
-      INTERNAL_API_REQUEST_JWT_ISSUER = 'gitlab-mailroom'
 
       # Only allow token generated within the last 5 minutes
       EXPIRATION = 5.minutes
@@ -18,9 +16,10 @@ module Gitlab
           return false if enabled_configs[mailbox_type].blank?
 
           decode_jwt(
-            request_headers[INTERNAL_API_REQUEST_HEADER],
+            request_headers[Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER],
             secret(mailbox_type),
-            issuer: INTERNAL_API_REQUEST_JWT_ISSUER, iat_after: Time.current - EXPIRATION
+            issuer: Gitlab::MailRoom::INTERNAL_API_REQUEST_JWT_ISSUER,
+            iat_after: Time.current - EXPIRATION
           )
         rescue JWT::DecodeError => e
           ::Gitlab::AppLogger.warn("Fail to decode MailRoom JWT token: #{e.message}") if Rails.env.development?

spec/lib/gitlab/mail_room/authenticator_spec.rb

@@ -44,7 +44,7 @@ RSpec.describe Gitlab::MailRoom::Authenticator do
   describe '#verify_api_request' do
     let(:incoming_email_secret) { SecureRandom.hex(16) }
     let(:service_desk_email_secret) { SecureRandom.hex(16) }
-    let(:payload) { { iss: described_class::INTERNAL_API_REQUEST_JWT_ISSUER, iat: (Time.current - 5.minutes + 1.second).to_i } }
+    let(:payload) { { iss: Gitlab::MailRoom::INTERNAL_API_REQUEST_JWT_ISSUER, iat: (Time.current - 5.minutes + 1.second).to_i } }
 
     before do
       allow(described_class).to receive(:secret).with(:incoming_email).and_return(incoming_email_secret)
@@ -54,7 +54,7 @@ RSpec.describe Gitlab::MailRoom::Authenticator do
     context 'verify a valid token' do
       it 'returns the decoded payload' do
         encoded_token = JWT.encode(payload, incoming_email_secret, 'HS256')
-        headers = { described_class::INTERNAL_API_REQUEST_HEADER => encoded_token }
+        headers = { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => encoded_token }
 
         expect(described_class.verify_api_request(headers, 'incoming_email')[0]).to match a_hash_including(
           "iss" => "gitlab-mailroom",
@@ -62,7 +62,7 @@ RSpec.describe Gitlab::MailRoom::Authenticator do
         )
 
         encoded_token = JWT.encode(payload, service_desk_email_secret, 'HS256')
-        headers = { described_class::INTERNAL_API_REQUEST_HEADER => encoded_token }
+        headers = { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => encoded_token }
 
         expect(described_class.verify_api_request(headers, 'service_desk_email')[0]).to match a_hash_including(
           "iss" => "gitlab-mailroom",
@@ -74,7 +74,7 @@ RSpec.describe Gitlab::MailRoom::Authenticator do
     context 'verify an invalid token' do
       it 'returns false' do
         encoded_token = JWT.encode(payload, 'wrong secret', 'HS256')
-        headers = { described_class::INTERNAL_API_REQUEST_HEADER => encoded_token }
+        headers = { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => encoded_token }
 
         expect(described_class.verify_api_request(headers, 'incoming_email')).to eq(false)
       end
@@ -83,7 +83,7 @@ RSpec.describe Gitlab::MailRoom::Authenticator do
     context 'verify a valid token but wrong mailbox type' do
       it 'returns false' do
         encoded_token = JWT.encode(payload, incoming_email_secret, 'HS256')
-        headers = { described_class::INTERNAL_API_REQUEST_HEADER => encoded_token }
+        headers = { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => encoded_token }
 
         expect(described_class.verify_api_request(headers, 'service_desk_email')).to eq(false)
       end
@@ -94,18 +94,18 @@ RSpec.describe Gitlab::MailRoom::Authenticator do
 
       it 'returns false' do
         encoded_token = JWT.encode(payload, incoming_email_secret, 'HS256')
-        headers = { described_class::INTERNAL_API_REQUEST_HEADER => encoded_token }
+        headers = { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => encoded_token }
 
         expect(described_class.verify_api_request(headers, 'incoming_email')).to eq(false)
       end
     end
 
     context 'verify a valid token but expired' do
-      let(:payload) { { iss: described_class::INTERNAL_API_REQUEST_JWT_ISSUER, iat: (Time.current - 5.minutes - 1.second).to_i } }
+      let(:payload) { { iss: Gitlab::MailRoom::INTERNAL_API_REQUEST_JWT_ISSUER, iat: (Time.current - 5.minutes - 1.second).to_i } }
 
       it 'returns false' do
         encoded_token = JWT.encode(payload, incoming_email_secret, 'HS256')
-        headers = { described_class::INTERNAL_API_REQUEST_HEADER => encoded_token }
+        headers = { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => encoded_token }
 
         expect(described_class.verify_api_request(headers, 'incoming_email')).to eq(false)
       end
@@ -125,7 +125,7 @@ RSpec.describe Gitlab::MailRoom::Authenticator do
 
       it 'returns false' do
         encoded_token = JWT.encode(payload, incoming_email_secret, 'HS256')
-        headers = { described_class::INTERNAL_API_REQUEST_HEADER => encoded_token }
+        headers = { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => encoded_token }
 
         expect(described_class.verify_api_request(headers, 'incoming_email')).to eq(false)
       end
@@ -133,7 +133,7 @@ RSpec.describe Gitlab::MailRoom::Authenticator do
 
     context 'verify headers for a non-existing mailbox type' do
       it 'returns false' do
-        headers = { described_class::INTERNAL_API_REQUEST_HEADER => 'something' }
+        headers = { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => 'something' }
 
         expect(described_class.verify_api_request(headers, 'invalid_mailbox_type')).to eq(false)
       end

spec/lib/gitlab/mail_room/mail_room_spec.rb

@@ -4,16 +4,30 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::MailRoom do
   let(:default_port) { 143 }
+  let(:log_path) { Rails.root.join('log', 'mail_room_json.log').to_s }
+
+  let(:fake_redis_queues) do
+    double(
+      url: "localhost",
+      db: 99,
+      sentinels: [{ host: 'localhost', port: 1234 }],
+      sentinels?: true
+    )
+  end
+
   let(:yml_config) do
     {
       enabled: true,
+      host: 'mail.example.com',
       address: 'address@example.com',
+      user: 'user@example.com',
+      password: 'password',
       port: default_port,
       ssl: false,
       start_tls: false,
       mailbox: 'inbox',
       idle_timeout: 60,
-      log_path: Rails.root.join('log', 'mail_room_json.log').to_s,
+      log_path: log_path,
       expunge_deleted: false
     }
   end
@@ -30,6 +44,7 @@ RSpec.describe Gitlab::MailRoom do
   end
 
   before do
+    allow(Gitlab::Redis::Queues).to receive(:new).and_return(fake_redis_queues)
     allow(described_class).to receive(:load_yaml).and_return(configs)
     described_class.instance_variable_set(:@enabled_configs, nil)
   end
@@ -39,6 +54,8 @@ RSpec.describe Gitlab::MailRoom do
   end
 
   describe '#enabled_configs' do
+    let(:first_value) { described_class.enabled_configs.each_value.first }
+
     context 'when both email and address is set' do
       it 'returns email configs' do
         expect(described_class.enabled_configs.size).to eq(2)
@@ -76,7 +93,7 @@ RSpec.describe Gitlab::MailRoom do
       let(:custom_config) { { enabled: true, address: 'address@example.com' } }
 
       it 'overwrites missing values with the default' do
-        expect(described_class.enabled_configs.each_value.first[:port]).to eq(Gitlab::MailRoom::DEFAULT_CONFIG[:port])
+        expect(first_value[:port]).to eq(Gitlab::MailRoom::DEFAULT_CONFIG[:port])
       end
     end
 
@@ -85,23 +102,24 @@ RSpec.describe Gitlab::MailRoom do
 
       it 'returns only encoming_email' do
         expect(described_class.enabled_configs.size).to eq(1)
-        expect(described_class.enabled_configs.each_value.first[:worker]).to eq('EmailReceiverWorker')
+        expect(first_value[:worker]).to eq('EmailReceiverWorker')
       end
     end
 
     describe 'setting up redis settings' do
-      let(:fake_redis_queues) { double(url: "localhost", db: 99, sentinels: "yes, them", sentinels?: true) }
-
-      before do
-        allow(Gitlab::Redis::Queues).to receive(:new).and_return(fake_redis_queues)
+      it 'sets delivery method to Sidekiq by default' do
+        config = first_value
+        expect(config).to include(
+          delivery_method: 'sidekiq'
+        )
       end
 
       it 'sets redis config' do
-        config = described_class.enabled_configs.each_value.first
+        config = first_value
         expect(config).to include(
           redis_url: 'localhost',
           redis_db: 99,
-          sentinels: 'yes, them'
+          sentinels: [{ host: 'localhost', port: 1234 }]
         )
       end
     end
@@ -111,7 +129,7 @@ RSpec.describe Gitlab::MailRoom do
         let(:custom_config) { { log_path: 'tiny_log.log' } }
 
         it 'expands the log path to an absolute value' do
-          new_path = Pathname.new(described_class.enabled_configs.each_value.first[:log_path])
+          new_path = Pathname.new(first_value[:log_path])
           expect(new_path.absolute?).to be_truthy
         end
       end
@@ -120,7 +138,7 @@ RSpec.describe Gitlab::MailRoom do
         let(:custom_config) { { log_path: '/dev/null' } }
 
         it 'leaves the path as-is' do
-          expect(described_class.enabled_configs.each_value.first[:log_path]).to eq '/dev/null'
+          expect(first_value[:log_path]).to eq '/dev/null'
         end
       end
     end
@@ -164,4 +182,148 @@ RSpec.describe Gitlab::MailRoom do
       end
     end
   end
+
+  describe 'config/mail_room.yml' do
+    let(:mail_room_template) { ERB.new(File.read(Rails.root.join("./config/mail_room.yml"))).result }
+    let(:mail_room_yml) { YAML.safe_load(mail_room_template, permitted_classes: [Symbol]) }
+
+    shared_examples 'renders mail-specific config file correctly' do
+      it 'renders mail room config file correctly' do
+        expect(mail_room_yml[:mailboxes]).to be_an(Array)
+        expect(mail_room_yml[:mailboxes].length).to eq(2)
+
+        expect(mail_room_yml[:mailboxes]).to all(
+          match(
+            a_hash_including(
+              host: 'mail.example.com',
+              port: default_port,
+              ssl: false,
+              start_tls: false,
+              email: 'user@example.com',
+              password: 'password',
+              idle_timeout: 60,
+              logger: {
+                log_path: log_path
+              },
+              name: 'inbox',
+
+              delete_after_delivery: true,
+              expunge_deleted: false
+            )
+          )
+        )
+      end
+    end
+
+    shared_examples 'renders arbitration options correctly' do
+      it 'renders arbitration options correctly' do
+        expect(mail_room_yml[:mailboxes]).to be_an(Array)
+        expect(mail_room_yml[:mailboxes].length).to eq(2)
+        expect(mail_room_yml[:mailboxes]).to all(
+          match(
+            a_hash_including(
+              arbitration_method: "redis",
+              arbitration_options: {
+                redis_url: "localhost",
+                namespace: "mail_room:gitlab",
+                sentinels: [{ host: "localhost", port: 1234 }]
+              }
+            )
+          )
+        )
+      end
+    end
+
+    shared_examples 'renders the sidekiq delivery method and options correctly' do
+      it 'renders the sidekiq delivery method and options correctly' do
+        expect(mail_room_yml[:mailboxes]).to be_an(Array)
+        expect(mail_room_yml[:mailboxes].length).to eq(2)
+
+        expect(mail_room_yml[:mailboxes][0]).to match(
+          a_hash_including(
+            delivery_method: 'sidekiq',
+            delivery_options: {
+              redis_url: "localhost",
+              redis_db: 99,
+              namespace: "resque:gitlab",
+              queue: "email_receiver",
+              worker: "EmailReceiverWorker",
+              sentinels: [{ host: "localhost", port: 1234 }]
+            }
+          )
+        )
+        expect(mail_room_yml[:mailboxes][1]).to match(
+          a_hash_including(
+            delivery_method: 'sidekiq',
+            delivery_options: {
+              redis_url: "localhost",
+              redis_db: 99,
+              namespace: "resque:gitlab",
+              queue: "service_desk_email_receiver",
+              worker: "ServiceDeskEmailReceiverWorker",
+              sentinels: [{ host: "localhost", port: 1234 }]
+            }
+          )
+        )
+      end
+    end
+
+    context 'when delivery_method is implicit' do
+      it_behaves_like 'renders mail-specific config file correctly'
+      it_behaves_like 'renders arbitration options correctly'
+      it_behaves_like 'renders the sidekiq delivery method and options correctly'
+    end
+
+    context 'when delivery_method is explicitly sidekiq' do
+      let(:custom_config) { { delivery_method: 'sidekiq' } }
+
+      it_behaves_like 'renders mail-specific config file correctly'
+      it_behaves_like 'renders arbitration options correctly'
+      it_behaves_like 'renders the sidekiq delivery method and options correctly'
+    end
+
+    context 'when delivery_method is webhook (internally postback in mail_room)' do
+      let(:custom_config) do
+        {
+          delivery_method: 'webhook',
+          gitlab_url: 'http://gitlab.example',
+          secret_file: '/path/to/secret/file'
+        }
+      end
+
+      it_behaves_like 'renders mail-specific config file correctly'
+      it_behaves_like 'renders arbitration options correctly'
+
+      it 'renders the webhook (postback) delivery method and options correctly' do
+        expect(mail_room_yml[:mailboxes]).to be_an(Array)
+        expect(mail_room_yml[:mailboxes].length).to eq(2)
+
+        expect(mail_room_yml[:mailboxes][0]).to match(
+          a_hash_including(
+            delivery_method: 'postback',
+            delivery_options: {
+              delivery_url: "http://gitlab.example/api/v4/internal/mail_room/incoming_email",
+              jwt_auth_header: Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER,
+              jwt_issuer: Gitlab::MailRoom::INTERNAL_API_REQUEST_JWT_ISSUER,
+              jwt_algorithm: 'HS256',
+              jwt_secret_path: '/path/to/secret/file'
+            }
+          )
+        )
+
+        expect(mail_room_yml[:mailboxes][1]).to match(
+          a_hash_including(
+            delivery_method: 'postback',
+            delivery_options: {
+              delivery_url: "http://gitlab.example/api/v4/internal/mail_room/service_desk_email",
+              jwt_auth_header: Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER,
+              jwt_issuer: Gitlab::MailRoom::INTERNAL_API_REQUEST_JWT_ISSUER,
+              jwt_algorithm: 'HS256',
+              jwt_secret_path: '/path/to/secret/file'
+            }
+          )
+        )
+      end
+    end
+  end
 end

spec/requests/api/internal/mail_room_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe API::Internal::MailRoom do
     }
   end
 
-  let(:auth_payload) { { 'iss' => Gitlab::MailRoom::Authenticator::INTERNAL_API_REQUEST_JWT_ISSUER, 'iat' => (Time.now - 10.seconds).to_i } }
+  let(:auth_payload) { { 'iss' => Gitlab::MailRoom::INTERNAL_API_REQUEST_JWT_ISSUER, 'iat' => (Time.now - 10.seconds).to_i } }
 
   let(:incoming_email_secret) { 'incoming_email_secret' }
   let(:service_desk_email_secret) { 'service_desk_email_secret' }
@@ -51,7 +51,7 @@ RSpec.describe API::Internal::MailRoom do
     context 'handle incoming_email successfully' do
       let(:auth_headers) do
         jwt_token = JWT.encode(auth_payload, incoming_email_secret, 'HS256')
-        { Gitlab::MailRoom::Authenticator::INTERNAL_API_REQUEST_HEADER => jwt_token }
+        { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => jwt_token }
       end
 
       it 'schedules a EmailReceiverWorker job with raw email content' do
@@ -71,7 +71,7 @@ RSpec.describe API::Internal::MailRoom do
     context 'handle service_desk_email successfully' do
       let(:auth_headers) do
         jwt_token = JWT.encode(auth_payload, service_desk_email_secret, 'HS256')
-        { Gitlab::MailRoom::Authenticator::INTERNAL_API_REQUEST_HEADER => jwt_token }
+        { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => jwt_token }
       end
 
       it 'schedules a ServiceDeskEmailReceiverWorker job with raw email content' do
@@ -91,7 +91,7 @@ RSpec.describe API::Internal::MailRoom do
     context 'email content exceeds limit' do
       let(:auth_headers) do
         jwt_token = JWT.encode(auth_payload, incoming_email_secret, 'HS256')
-        { Gitlab::MailRoom::Authenticator::INTERNAL_API_REQUEST_HEADER => jwt_token }
+        { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => jwt_token }
       end
 
       before do
@@ -134,7 +134,7 @@ RSpec.describe API::Internal::MailRoom do
     context 'wrong token authentication' do
       let(:auth_headers) do
         jwt_token = JWT.encode(auth_payload, 'wrongsecret', 'HS256')
-        { Gitlab::MailRoom::Authenticator::INTERNAL_API_REQUEST_HEADER => jwt_token }
+        { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => jwt_token }
       end
 
       it 'responds with 401 Unauthorized' do
@@ -147,7 +147,7 @@ RSpec.describe API::Internal::MailRoom do
     context 'wrong mailbox type authentication' do
       let(:auth_headers) do
         jwt_token = JWT.encode(auth_payload, service_desk_email_secret, 'HS256')
-        { Gitlab::MailRoom::Authenticator::INTERNAL_API_REQUEST_HEADER => jwt_token }
+        { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => jwt_token }
       end
 
       it 'responds with 401 Unauthorized' do
@@ -160,7 +160,7 @@ RSpec.describe API::Internal::MailRoom do
     context 'not supported mailbox type' do
       let(:auth_headers) do
         jwt_token = JWT.encode(auth_payload, incoming_email_secret, 'HS256')
-        { Gitlab::MailRoom::Authenticator::INTERNAL_API_REQUEST_HEADER => jwt_token }
+        { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => jwt_token }
       end
 
       it 'responds with 401 Unauthorized' do
@@ -181,7 +181,7 @@ RSpec.describe API::Internal::MailRoom do
 
       let(:auth_headers) do
         jwt_token = JWT.encode(auth_payload, service_desk_email_secret, 'HS256')
-        { Gitlab::MailRoom::Authenticator::INTERNAL_API_REQUEST_HEADER => jwt_token }
+        { Gitlab::MailRoom::INTERNAL_API_REQUEST_HEADER => jwt_token }
       end
 
       it 'responds with 401 Unauthorized' do