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

Wrong PAM role added to pg_hba #407

Closed
jens-totemic opened this issue Nov 9, 2018 · 6 comments · Fixed by #409
Closed

Wrong PAM role added to pg_hba #407

jens-totemic opened this issue Nov 9, 2018 · 6 comments · Fixed by #409

Comments

@jens-totemic
Copy link
Contributor

jens-totemic commented Nov 9, 2018

I'd like to use the PAM support and noticed that the correct role name is not used in all places.

Steps to reproduce
I have set a PAM role in the operator config map:
pam_role_name: mypamusers. I replaced the entry in this file: https://github.com/zalando-incubator/postgres-operator/blob/master/manifests/configmap.yaml)

Then I commented out the lines for pg_hba: in the cluster definition in order to use the defaults that add the PAM support. I used this file as my starting point: https://github.com/zalando-incubator/postgres-operator/blob/master/manifests/complete-postgres-manifest.yaml

When I launch a new cluster using the two files above, I observe two things:

  • the pg_hba.conf file in the postgres pod looks like this:
# Do not edit this file manually!
# It will be overwritten by Patroni!
local   all             all                                   trust
hostssl all             +zalandos    127.0.0.1/32       pam
host    all             all                127.0.0.1/32       md5
hostssl all             +zalandos    ::1/128            pam
host    all             all                ::1/128            md5
hostssl replication     standby all                md5
hostnossl all           all                all                reject
hostssl all             +zalandos    all                pam
hostssl all             all                all                md5
  • a role zalandos is created in the postgres database

It appears that in both cases Spilo uses the ENV variable HUMAN_ROLE and it defaults to "zalandos" (see https://github.com/zalando/spilo/blob/master/postgres-appliance/scripts/configure_spilo.py#L349)

Expected behaviour

  • Spilo's HUMAN_ROLE variable should be set to value of pam_role_name
  • pg_hba.conf should use the value of pam_role_name
  • no role named zalandos should be added to the postgres database
@sdudoladov
Copy link
Member

sdudoladov commented Nov 9, 2018

The operator sets up the Spilo conf (env var in a container) correctly:

SPILO_CONFIGURATION={"postgresql":{"bin_dir":"/usr/lib/postgresql/10/bin","parameters":{"log_statement":"all","sha
red_buffers":"32MB"}},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encodin
g":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"mypamusers":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs"
:{"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"postgresql":{"parameters":{"max_c
onnections":"10"}}}}}

so looks like a bug in Spilo

@sdudoladov
Copy link
Member

@jens-totemic thanks for the detailed bug report :)

@Jan-M
Copy link
Member

Jan-M commented Nov 9, 2018

We just looked at this, seems the operator should add the HUMAN_ROLE also to the spilo config dict.

Sergey will test give this a try soonish to resolve this issue.

If in a rush just put a layer on top of our spilo with the human_role set.

@jens-totemic
Copy link
Contributor Author

jens-totemic commented Nov 9, 2018

Thanks for the quick response @zerg-junior and @Jan-M ! Do you know if the PR will also prevent creating the additional zalando role in the postgres db? Or is this something to file in the spilo repo?

@sdudoladov
Copy link
Member

sdudoladov commented Nov 12, 2018

Do you know if the PR will also prevent creating the additional zalando role in the postgres db

I run a quick test with the PR and default manifests, no zalandos role was created if you set pam_role_name: mypamusers in the operator configmap. There is robot_zmon infrastructure role coming from manifests/infrastructure-roles.yaml if you use this capability with the default infra role manifest

@jens-totemic
Copy link
Contributor Author

Thanks for confirming @zerg-junior !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants