Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set postgres db owner to confluence user. #54

Merged
merged 1 commit into from Nov 15, 2015

Conversation

patcon
Copy link
Contributor

@patcon patcon commented Sep 29, 2015

  • This allows a database to be imported using a more flexible backup,
    taken with the --no-owner flag. However, in order for the new
    database to work as expected, the database my be owned by the user
    expected to interact with it. This way, the table ownership is
    automatically set to this db user during import, and this db user can
    continue to access later.

@patcon
Copy link
Contributor Author

patcon commented Sep 29, 2015

Basically making the database be owned by the user accessing it, which gets around some edge-cases with importing/exporting where the confluence db user can't manage the database properly

@bflad
Copy link
Contributor

bflad commented Sep 29, 2015

Does the postgresql_database resource accept the owner attribute like the README documentation?
https://github.com/chef-cookbooks/database#examples

# create a postgresql database with additional parameters
postgresql_database 'mr_softie' do
  connection(
    :host     => '127.0.0.1',
    :port     => 5432,
    :username => 'postgres',
    :password => node['postgresql']['password']['postgres']
  )
  template 'DEFAULT'
  encoding 'DEFAULT'
  tablespace 'DEFAULT'
  connection_limit '-1'
  owner 'postgres'
  action :create
end

@patcon
Copy link
Contributor Author

patcon commented Sep 29, 2015

Ah, had never noticed that resource attr before :) thanks

@patcon
Copy link
Contributor Author

patcon commented Sep 29, 2015

Force pushed to take your suggestion into account @bflad

@patcon
Copy link
Contributor Author

patcon commented Sep 29, 2015

Hm. Except it might create a chicken and egg situation with the user, which we create second. We'll probably need to separate out the user :create from the user :grant if we want to get around that. Any objections?

@patcon
Copy link
Contributor Author

patcon commented Oct 1, 2015

(Not ready to be merged until the above comment is dealt with, to clarify)

@legal90
Copy link
Contributor

legal90 commented Nov 10, 2015

@patcon Could you please give an example - what problem does this pull request solve?

@patcon
Copy link
Contributor Author

patcon commented Nov 11, 2015

Sure!

Before this, the database was owned by postgres and confluence simply had grants on it. Any tables that were subsequently created via the confluence user DID have table-level ownership by confluence. This doesn't normally have obvious effects, but comes unto play when importing a backup in certain circumstances, when the behaviour of psql is to recreate the tables with ownership matching the database ownership (ie. postgres). In that case, all the table-level ownership is also postgres, and the confluence user can't do anything :)

Basically, it seems that it was an oversight that the database wasn't owned by the confluence database user.

@legal90 legal90 added this to the 2.0.0 milestone Nov 12, 2015
@legal90
Copy link
Contributor

legal90 commented Nov 12, 2015

@patcon Thank you! Now it's clear for me :)

But yes, this PR should be modified, because user should be already created when we try to create a DB. Otherwise, there will be an error like this:

       Recipe: confluence::database


           ================================================================================
           Error executing action `create` on resource 'postgresql_database[confluence]'
           ================================================================================

           PG::UndefinedObject
           -------------------
           ERROR:  role "confluence" does not exist

@patcon
Copy link
Contributor Author

patcon commented Nov 12, 2015

👍 :)

* PostgreSQL has database ownership that can be used instead of grants.
  Previously, the `confluence` user only had table-level ownership.
(This change allows a database to be imported using a more flexible
backup, taken with the `--no-owner` flag.)
@patcon
Copy link
Contributor Author

patcon commented Nov 15, 2015

ok, this should be good now, but I haven't tested it yet, so I'll have to do that later

Also, I realized by consulting the docs that postgres didn't need grants if the user owns the database, and mysql doesn't have table ownership -- only grants (it's a no-op resource attr in the mysql_database resource :)

@legal90
Copy link
Contributor

legal90 commented Nov 15, 2015

@patcon It works fine, thank you!

legal90 added a commit that referenced this pull request Nov 15, 2015
Set postgres db owner to confluence user.
@legal90 legal90 merged commit 45d5fc6 into sous-chefs:master Nov 15, 2015
legal90 added a commit that referenced this pull request Feb 19, 2016
* master: (27 commits)
  Run integration tests in Travis with Docker
  .kitchen.yml: Remove "ubuntu-15.04" platform support
  Add fixture cookbook
  Use ChefDK for running Rake tasks and Travis jobs
  Update contribution and testing guidelines according to Chef Software conventions
  test/integration: Use "infrataster" to check website availability
  Converted virtualhost derived attrs into helper functions. [#81]
  Add support of Confluence 5.8.16
  spec/database_spec: Fix data bag item
  Add spec for "database" recipe
  Add apt cookbook for test-kitchen success on DO.
  Allow helpers to be used within resource.
  Fixed rubocop errors. Removed redundant var.
  Added database connection helper.
  Set postgres db owner, and remove :grant action. [#54]
  Mark 'java' cookbook as a dependency
  Update README.md
  Allow to override SSO settings in the data bag
  Fix README.md
  Use common method "data_bag_item" for item loading
  ...
@lock
Copy link

lock bot commented May 10, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants