Skip to content
Snippets Groups Projects
Commit c4754762 authored by Richard Clamp's avatar Richard Clamp
Browse files

Only set the password when it is supplied

Here we make use of the `property_is_set?` function to gate the
management of the password when the property has been supplied, and then
treat `nil` to mean explicitly reset the password.

In order to add confidence that it is working we add a new test
cookbook, and add a spec/chef/resources/postgres_user_spec.rb to make
use of it.

We also add an explcit execute resource, to fixup user options if needed

Sadly these tests reveal that the handling of nil values for properties
fails when using chef 12.  When extracted[1] we can see this works
with chef 13, but as an upgrade is out of scope for now, we mark the
example as pending.

[1] https://gitlab.com/richardc/lwrp-nil-headache/
parent fed48ae5
No related branches found
No related tags found
No related merge requests found
Showing
with 189 additions and 4 deletions
......@@ -35,6 +35,28 @@
"|grep -x #{db_user}"])
end
def user_options(db_user)
db_user_safe = db_user.scan(/[a-z_][a-z0-9_-]*[$]?/).first
query = "SELECT usecreatedb, usesuper, userepl, usebypassrls FROM pg_shadow WHERE usename='#{db_user_safe}'"
values = do_shell_out(
%(/opt/gitlab/bin/#{service_cmd} -d template1 -c "#{query}" -tA)
).stdout.chomp.split('|').map { |v| v == 't' }
options = %w(CREATEDB SUPERUSER REPLICATION BYPASSRLS)
Hash[options.zip(values)]
end
def user_options_set?(db_user, options)
active_options = user_options(db_user)
options.map(&:upcase).each do |option|
if option =~ /^NO(.*)/
return false if active_options[$1]
else
return false if !active_options[option]
end
end
true
end
def user_hashed_password(db_user)
db_user_safe = db_user.scan(/[a-z_][a-z0-9_-]*[$]?/).first
do_shell_out(
......
......@@ -18,4 +18,21 @@
not_if { !helper.is_running? || helper.user_exists?(username) }
end
if property_is_set?(:password)
query = %(ALTER USER \\"#{username}\\" #{options.join(' ')})
query << if password.nil?
%( WITH PASSWORD NULL )
else
%( WITH PASSWORD '#{password}')
end
execute "set password for #{username} postgresql user" do
command %(/opt/gitlab/bin/#{helper.service_cmd} -d template1 -c "#{query}")
user account_helper.postgresql_user
# Added retries to give the service time to start on slower systems
retries 20
not_if { !helper.is_running? || !helper.user_exists?(username) || helper.user_password_match?(username, password) }
end
end
query = %(ALTER USER \\"#{username}\\" #{options.join(' ')})
......@@ -21,3 +38,2 @@
query = %(ALTER USER \\"#{username}\\" #{options.join(' ')})
query << %( WITH PASSWORD '#{password}') unless password.nil?
......@@ -23,6 +39,6 @@
execute "set password for #{username} postgresql user" do
execute "set options for #{username} postgresql user" do
command %(/opt/gitlab/bin/#{helper.service_cmd} -d template1 -c "#{query}")
user account_helper.postgresql_user
# Added retries to give the service time to start on slower systems
retries 20
......@@ -25,7 +41,7 @@
command %(/opt/gitlab/bin/#{helper.service_cmd} -d template1 -c "#{query}")
user account_helper.postgresql_user
# Added retries to give the service time to start on slower systems
retries 20
not_if { !helper.is_running? || !helper.user_exists?(username) || helper.user_password_match?(username, password) }
not_if { !helper.is_running? || !helper.user_exists?(username) || helper.user_options_set?(username, options) }
end
end
require 'chef_helper'
describe 'postgresql_user' do
before do
allow_any_instance_of(PgHelper).to receive(:is_running?).and_return(true)
allow_any_instance_of(PgHelper).to receive(:user_exists?).and_return(false, true)
allow_any_instance_of(PgHelper).to receive(:user_password_match?).and_return(false)
allow_any_instance_of(PgHelper).to receive(:user_options_set?).and_return(false)
end
let(:runner) { ChefSpec::SoloRunner.new(step_into: ['postgresql_user']) }
context 'create' do
let(:chef_run) { runner.converge('test_gitlab_postgresql_user::create') }
it 'creates a user' do
expect(chef_run).to run_execute('create example postgresql user')
end
end
context 'password' do
context 'not specified' do
let(:chef_run) { runner.converge('test_gitlab_postgresql_user::password_unspecified') }
it 'does not set the password of the no_password user' do
expect(chef_run).not_to run_execute('set password for no_password postgresql user')
end
end
context 'nil' do
let(:chef_run) { runner.converge('test_gitlab_postgresql_user::password_nil') }
it 'does set the password of the nil_password user' do
pending 'upgrade to chef 13'
expect(chef_run).to run_execute('set password for nil_password postgresql user')
.with(command: /PASSWORD NULL/)
end
end
context 'md5' do
let(:chef_run) { runner.converge('test_gitlab_postgresql_user::password_md5') }
it 'does set the password of the md5_password user' do
expect(chef_run).to run_execute('set password for md5_password postgresql user')
.with(command: /PASSWORD 'e99b79fbdf9b997e6918df2385e60f5c'/)
end
end
context 'empty' do
let(:chef_run) { runner.converge('test_gitlab_postgresql_user::password_empty') }
it 'does set the password of the empty_password user' do
expect(chef_run).to run_execute('set password for empty_password postgresql user')
.with(command: /PASSWORD ''/)
end
end
end
context 'options' do
context 'SUPERUSER' do
let(:chef_run) { runner.converge('test_gitlab_postgresql_user::options_superuser') }
it 'does set SUPERUSER' do
expect(chef_run).to run_execute('set options for example postgresql user')
.with(command: /\bSUPERUSER\b/)
end
end
end
end
......@@ -35,7 +35,7 @@
config.platform = platform
config.version = version
config.cookbook_path = ['files/gitlab-cookbooks/']
config.cookbook_path = ['files/gitlab-cookbooks/', 'spec/fixtures/cookbooks']
config.log_level = :error
config.filter_run focus: true
......
name 'test_gitlab_postgresql_user'
depends 'gitlab'
postgresql_user 'example' do
end
postgresql_user 'example' do
options ['SUPERUSER']
end
postgresql_user 'empty_password' do
password ''
end
postgresql_user 'md5_password' do
# This is not a secret password:
# ruby -rdigest -e 'puts Digest::MD5.hexdigest("toomanysecrets" + "md5_password")'
password 'e99b79fbdf9b997e6918df2385e60f5c'
end
postgresql_user 'nil_password' do
password nil
end
postgresql_user 'no_password' do
end
......@@ -10,6 +10,64 @@
allow(subject).to receive(:service_cmd) { 'gitlab-psql' }
end
describe '#user_options' do
before do
result = spy('shellout')
allow(result).to receive(:stdout).and_return("f|f|t|f\n")
allow(subject).to receive(:do_shell_out).and_return(result)
end
it 'returns hash from query' do
expect(subject.user_options('')).to eq(
{
'SUPERUSER' => false,
'CREATEDB' => false,
'REPLICATION' => true,
'BYPASSRLS' => false
}
)
end
end
describe '#user_options_set?' do
let(:default_options) do
{
'SUPERUSER' => false,
'CREATEDB' => false,
'REPLICATION' => true,
'BYPASSRLS' => false
}
end
context 'default user options' do
before do
allow(subject).to receive(:user_options).and_return(default_options)
end
it 'returns true when no options are asked about' do
expect(subject.user_options_set?('', [])).to be_truthy
end
it 'returns true when options are set to their defaults' do
expect(subject.user_options_set?('', ['NOSUPERUSER'])).to be_truthy
end
it 'returns false when options are set away from their defaults' do
expect(subject.user_options_set?('', ['SUPERUSER'])).to be_falsey
end
end
context 'modified user' do
before do
allow(subject).to receive(:user_options).and_return(default_options.merge({ 'SUPERUSER' => true }))
end
it 'returns false when options is not what we expect' do
expect(subject.user_options_set?('', ['NOSUPERUSER'])).to be_falsey
end
end
end
describe '#user_password_match?' do
before do
# user: gitlab pass: test123
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment