Merge branch '2721-don-t-add-duplicate-entries-to-pg_auth' into 'master'

Resolve "Don't add duplicate entries to pg_auth"

Closes #2721

See merge request !1945

files/gitlab-cookbooks/gitlab-ee/libraries/pgbouncer_helper.rb

@@ -14,4 +14,12 @@ class PgbouncerHelper
       "#{setting}=#{value}"
     end.join(' ').chomp
   end
+
+  def pg_auth_users
+    results = node['gitlab']['pgbouncer']['users'].to_hash
+    node['gitlab']['pgbouncer']['databases'].each do |_db, settings|
+      results[settings['user']] = { 'password' => settings['password'] }
+    end
+    results
+  end
 end

files/gitlab-cookbooks/gitlab-ee/recipes/pgbouncer.rb

@@ -15,6 +15,7 @@
 #
 account_helper = AccountHelper.new(node)
+pgb_helper = PgbouncerHelper.new(node)
 include_recipe 'gitlab::postgresql_user'
@@ -31,7 +32,7 @@ end
 template "#{node['gitlab']['pgbouncer']['data_directory']}/pg_auth" do
   source "pg_auth.erb"
-  variables node['gitlab']['pgbouncer'].to_hash
+  helper(:pgb_helper) { pgb_helper }
 end
 runit_service 'pgbouncer' do

files/gitlab-cookbooks/gitlab-ee/templates/default/pg_auth.erb

-<% @databases.each do |_db, settings| %>
-"<%= settings['user'] %>" "md5<%= settings['password'] %>"
-<% end %>
-<% @users.each do |user, info| %>
-"<%= user %>" "md5<%= info['password'] %>"
+<% pgb_helper.pg_auth_users.each do |user, settings| %>
+"<%= user %>" "md5<%= settings['password'] %>"
 <% end %>

spec/chef/gitlab-ee/libraries/pgbouncer_helper_spec.rb

+require 'chef_helper'
+
+describe PgbouncerHelper do
+  let(:chef_run) { converge_config(ee: true) }
+  subject { described_class.new(chef_run.node) }
+
+  before do
+    allow(Gitlab).to receive(:[]).and_call_original
+  end
+
+  describe '#pg_auth_users' do
+    context 'by default' do
+      it 'by default' do
+        expect(subject.pg_auth_users).to be_empty
+      end
+    end
+
+    context 'with users' do
+      before do
+        stub_gitlab_rb(
+          pgbouncer: {
+            users: {
+              fakeuser: {
+                password: 'fakepassword'
+              }
+            }
+          }
+        )
+      end
+
+      it 'returns a hash' do
+        expect(subject.pg_auth_users).to eq 'fakeuser' => { 'password' => 'fakepassword' }
+      end
+    end
+
+    context 'with databases' do
+      before do
+        stub_gitlab_rb(
+          pgbouncer: {
+            databases: {
+              fakedb: {
+                user: 'fakeuser',
+                password: 'fakepassword'
+              }
+            }
+          }
+        )
+      end
+
+      it 'returns a hash' do
+        expect(subject.pg_auth_users).to eq 'fakeuser' => { 'password' => 'fakepassword' }
+      end
+    end
+
+    context 'with both' do
+      before do
+        stub_gitlab_rb(
+          pgbouncer: {
+            databases: {
+              fakedb: {
+                user: 'fakedbuser',
+                password: 'fakepassword'
+              }
+            },
+            users: {
+              fakeusersuser: {
+                password: 'fakepassword'
+              }
+            }
+          }
+        )
+      end
+
+      it 'should merge both' do
+        expected = {
+          'fakeusersuser' => { 'password' => 'fakepassword' }, 'fakedbuser' => { 'password' => 'fakepassword' }
+        }
+        expect(subject.pg_auth_users).to eq expected
+      end
+    end
+
+    context 'with multiple databases' do
+      before do
+        stub_gitlab_rb(
+          pgbouncer: {
+            databases: {
+              fakedb_one: {
+                user: 'fakeuser',
+                password: 'fakepassword'
+              },
+              fakedb_two: {
+                user: 'fakeuser',
+                password: 'fakepassword'
+              }
+            }
+          }
+        )
+      end
+
+      it 'should only have one entry' do
+        expect(subject.pg_auth_users).to eq 'fakeuser' => { 'password' => 'fakepassword' }
+      end
+    end
+  end
+end