Bug 887815
Summary: | a comprehensive keystone.conf file should be included in the RPMS(s) | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | Red Hat OpenStack | Reporter: | Nikola Dipanov <ndipanov> | ||||||||
Component: | openstack-keystone | Assignee: | Alan Pevec <apevec> | ||||||||
Status: | CLOSED ERRATA | QA Contact: | Pavel Sedlák <psedlak> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | high | ||||||||||
Version: | 1.0 (Essex) | CC: | apevec, ayoung, dyocum, eglynn, markmc, pbrady | ||||||||
Target Milestone: | snapshot5 | Keywords: | Reopened, Triaged | ||||||||
Target Release: | 2.1 | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | openstack-keystone-2012.2.3-6.el6ost | Doc Type: | Bug Fix | ||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | 887334 | Environment: | |||||||||
Last Closed: | 2013-04-04 20:22:41 UTC | Type: | Bug | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Bug Depends On: | |||||||||||
Bug Blocks: | 892420, 909013 | ||||||||||
Attachments: |
|
Description
Nikola Dipanov
2012-12-17 12:01:48 UTC
Unlike Nova, Keystone RPM is not shipping own copy of keystone.conf, it edits https://github.com/openstack/keystone/blob/master/etc/keystone.conf.sample so doc comments are preserved. See bug #887804 for the conclusions we came to about Cinder For keystone, things are a bit different 1) The defaults which we change using openstack-config e.g.: openstack-config --set etc/keystone.conf DEFAULT log_file %{_localstatedir}/log/keystone/keystone.log and the PasteDeploy config (starting with [filter:debug]) should go into a /usr/share/keystone/keystone-dist.conf file and the systemd units and init scripts will need to be updated to do: --config-file=/usr/share/keystone/keystone-dist.conf --config-file=/etc/keystone/keystone.conf This means that users can start with an empty /etc/keystone/keystone.conf file and still be using the distribution defaults This file should not be marked as %config(noreplace) since it is not intended to be user-editable One thing to watch out for here - keystone-all uses pastedeploy to load the wsgi apps from config_file[0] so a user won't be able to change the wsgi config by editing /etc/keystone/keystone.conf I see keystone uses /usr/share/openstack-keystone rather than /usr/share/keystone. I guess I'd prefer the latter for consistency with e.g. /etc/keystone but it's a question of whether it's safe to change now. Not a big deal, either is fine) 2-4) Modulo the PasteDeploy stuff, upstream's keystone.conf.sample contains nothing but comments explaining what options are available. That's exactly what we want for /etc/keystone/keystone.conf 5) In the productized packages, we should strip the sample keystone.conf to only include options we recommend using and perhaps improve the descriptions. 6) N/A From apevec - "One thing to verify is Nova selinux policy: /etc/nova/nova.conf is etc_t while files under /usr/share/nova/ get usr_t" While at it, paste-deploy config should be moved to a separate ini file like in other OpenStack services. Backport of this recent upstream change https://review.openstack.org/#/c/20505 should help, along with introducing paste_deploy.config_file parameter like in Glance (instead of assuming first .conf or hardcoded 'keystone.conf' is a paste-deploy ini file) (In reply to comment #3) > and the PasteDeploy config (starting with [filter:debug]) should go > into a /usr/share/keystone/keystone-dist.conf file and the With the backport of https://review.openstack.org/24126 we can move PasteDeploy config part to a separate /usr/share/keystone/keystone-paste.ini > One thing to watch out for here - keystone-all uses pastedeploy to load > the wsgi apps from config_file[0] so a user won't be able to change the > wsgi config by editing /etc/keystone/keystone.conf Above patch introduces paste_deploy.config_file parameter, so user would be able to change wsgi config but then they're on their own. Created attachment 709299 [details]
dist-git patch
Created attachment 709310 [details]
dist-git patch
I had a quick look at the corresponding -patches change for this: https://code.engineering.redhat.com/gerrit/gitweb?p=keystone.git;a=commit;h=069a57e I notice that the [sql].connection var is commentd but still set to the upstream default. Ideally that value would be left commented and updated automatically at rpm build time from the corresponding -dist.conf On a related note, cli tools may not be aware of the -dist.conf and so maybe fail to sync the DB etc? Is there a requirement to auto add -dist.conf to the config set? as was done in this nova patch for example: https://code.engineering.redhat.com/gerrit/gitweb?p=nova.git;a=commit;h=634259d (In reply to comment #9) > I notice that the [sql].connection var is commentd but still set to the > upstream default. Ideally that value would be left commented and > updated automatically at rpm build time from the corresponding -dist.conf Indeed, I missed point 5) in comment 3. I'll look how is that handled in nova/cinder. > On a related note, cli tools may not be aware of the -dist.conf > and so maybe fail to sync the DB etc? Is there a requirement to > auto add -dist.conf to the config set? yes, I need to fix keystone-manage Follow up spec patch: -%{_datadir}/keystone +%dir %{_datadir}/keystone +%attr(0640, root, keystone) %{_datadir}/keystone/keystone-dist.conf +%attr(0640, root, keystone) %{_datadir}/keystone/keystone-dist-paste.ini +%attr(0755, root, root) %{_datadir}/keystone/sample_data.sh +%attr(0644, root, root) %{_datadir}/keystone/%{name}.upstart Maybe I do not understand it right, so please correct me if I'm mistaken, but following parts looks wrong. Snippets from /etc/keystone/keystone.conf. ------ # Name of log file to output to. If not set, logging will go to stdout. # log_file = /var/log/keystone/keystone.log ------ But default (at least for me) in /usr/.../keystone-dist.conf is set to /var/log/keystone/keystone.log. So for user "If not set" actually means that log file, not stdout. I think better could be "if set to empty" etc. ------ [sql] # The SQLAlchemy connection string used to connect to the database # connection = mysql://keystone:keystone@localhost/keystone connection = mysql://keystone_admin:ks123456.33.11/keystone ------ Connection is set here, and also in -dist.conf: connection = mysql://keystone:keystone@localhost/keystone Why is the commented-out line left in place and why the correct value (the one in etc) is not placed in -dist.conf? Same goes for [catalog]-driver: - set in both places to same value - not commented-out in /etc - original original comment/example left in /etc (so there are two driver= lines) ------ [paste_deploy] # Name of the paste configuration file that defines the available pipelines ------ (EOF) Shouldn't be there an example like "# config_file = ..."? Valid point about "If not set, logging will go to stdout." These comments are generated upstream, automatically from source, so the best fix would be to adjust comments so they don't assume a single point of config. There are various issues with those logfile comments actually, which I've addressed upstream with: https://review.openstack.org/24889 So the fix will bubble up at some stage, and if we think important enough to expediate we can propose that to rh-folsom-rhel-6-patches. *-dist.conf are the distribution defaults. I.E. not editable by users. These defaults are reflected by the commented values in the user editable files. config options are cumulative, with /etc overriding /usr. Installation specific config changes should only be done in /etc. Whether the commented line is left in place, is debatable. I think it's OK though as it does document the dist default in a single place. I agree that the # config_file should have a comment in place, Maybe that's also needed in the corresponding upstream change: https://review.openstack.org/#/c/24126 Created attachment 713375 [details]
checks if keystone.conf has precedence against keystone-dist.conf
Verified that /etc/keystone/keystone.conf contains options with descriptions as keystone.conf.example provided by upstream also default distribution and paste/pipeline configs are in separate files. Old version used: openstack-keystone-2012.2.1-3.el6ost Verified in version: openstack-keystone-2012.2.3-6.el6ost In old version there was /usr/share/openstack-keystone now it is /usr/share/keystone as mentioned. In new version: - /usr/share/keystone/openstack-keystone.upstart was changed and it contains two --config paths (/etc and -dist ones) - /etc/init.d/openstack-keystone also specifies two --config paths - /usr/share/keystone/ contains new keystone-dist.conf and keystone-dist-paste.ini - - in keystone-dist.conf there is [paste-deploy]/config_file path pointing to keystone-dist-paste.ini - - keystone-dist-paste.ini contains pipeline settings (unlike keystone.conf or keystone-dist.conf) Tested precedence of keystone.conf and keystone-dist.conf with attached bash script: - adds admin_token to dist.conf and uses original one to authenticate - disables original token and uses the newly added (from dist.conf) to authenticate - script also verifies that dist.conf is replaced (new token cleaned) when package is reinstalled Also when only working SQL connection is specified only in one of keystone.conf or keystone.dist.conf keystone-manage db_sync works - so is able to use both of them (compared to when there no valid connection specified). Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. http://rhn.redhat.com/errata/RHSA-2013-0708.html |