Project

Profile

Help

Task #413

Our unit tests require read permissions on server.conf

Added by rbarlow almost 6 years ago. Updated over 1 year ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Platform Release:
2.8.0
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
Sprint:
Quarter:

Description

I attempted to fix our unit tests to not require reading server.conf, but it turns out that it is more effort than I want to do today to refactor how our configuration loading happens during unit tests.

Refactoring how we do configuration loading will be a very good thing to do in general, especially if we want to support virtualenv development.

For now, pulp-dev.py installs server.conf world-readable. When we find time to fix this bug, we should install server.conf as root:apache 640, just like the RPM. This way the development environment will more closely resemble the production environment, which will be a good thing.

+ This bug was cloned from Bugzilla Bug #1084708 +


Related issues

Has duplicate Pulp - Issue #1230: Ability to run unit tests on a system without pulp-server installedCLOSED - DUPLICATE<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Blocked by Pulp - Issue #607: server/config.py reads server.conf when the module is loaded.CLOSED - CURRENTRELEASE<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

Associated revisions

Revision d2286409 View on GitHub
Added by semyers almost 5 years ago

Block attempts to load server.conf when running unittests

Revision 8214187e View on GitHub
Added by semyers almost 5 years ago

Block attempts to load server.conf when running tests

https://pulp.plan.io/issues/413 re #413

Revision 8214187e View on GitHub
Added by semyers almost 5 years ago

Block attempts to load server.conf when running tests

https://pulp.plan.io/issues/413 re #413

Revision 8214187e View on GitHub
Added by semyers almost 5 years ago

Block attempts to load server.conf when running tests

https://pulp.plan.io/issues/413 re #413

Revision 8214187e View on GitHub
Added by semyers almost 5 years ago

Block attempts to load server.conf when running tests

https://pulp.plan.io/issues/413 re #413

Revision 625bc071 View on GitHub
Added by semyers almost 5 years ago

Block attempts to load server.conf when running tests

https://pulp.plan.io/issues/413 re #413

Revision c3118537 View on GitHub
Added by semyers almost 5 years ago

Block attempts to load server.conf when running tests

https://pulp.plan.io/issues/413 re #413

Revision c33bdeb3 View on GitHub
Added by semyers almost 5 years ago

Block attempts to load server.conf when running tests

https://pulp.plan.io/issues/413 re #413

Revision b3c960a3 View on GitHub
Added by semyers almost 5 years ago

Block attempts to load server.conf when running tests

https://pulp.plan.io/issues/413 re #413

History

#1 Updated by rbarlow almost 6 years ago

Due to https://bugzilla.redhat.com/show_bug.cgi?id=1176259 deleting my development database, I am raising the severity on this issue to high. If we make it so that /etc/pulp/server.conf can be installed at 640 (as it is in production), our dev environments will be more similar to production and we can also guarantee that the unit tests won't have access to our DB settings in order to do things like drop the database.

+ This comment was cloned from Bugzilla #1084708 comment 1 +

#2 Updated by rbarlow almost 6 years ago

We likely have to fix #1160369 in order to fix this issue.

https://bugzilla.redhat.com/show_bug.cgi?id=1160369

+ This comment was cloned from Bugzilla #1084708 comment 2 +

#3 Updated by bmbouter over 5 years ago

  • Blocked by Issue #607: server/config.py reads server.conf when the module is loaded. added

#4 Updated by rbarlow about 5 years ago

  • Has duplicate Issue #1230: Ability to run unit tests on a system without pulp-server installed added

#5 Updated by semyers about 5 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to semyers

#6 Updated by semyers almost 5 years ago

server.conf permissions/existence is no longer required in core. For this issue to be complete, I'll be checking all the plugins' tests as well and making sure they're acting properly.

The change #607 is backward-compatible, so if any plugins are broken by an unreadable server.conf, it probably means that they're doing shady things to read the server conf.

Regarding this: "Refactoring how we do configuration loading will be a very good thing to do in general, especially if we want to support virtualenv development," #607 lays the groundwork for that. You can now modify the list of default config files before the config files are loaded, so that should be much easier to manage now.

#7 Updated by semyers almost 5 years ago

it probably means that they're doing shady things to read the server conf

...or it just means they're using celery, because we try to read the server config before instantiating the global celery object. It's an easy fix, but one that need to be applied per-plugin test suite.

#9 Updated by semyers almost 5 years ago

I opened several PRs, most are merged with a couple more to go. I neglected to ref this issue in the pulp_rpm PR; here's that commit:
https://pulp.plan.io/projects/pulp_rpm/repository/revisions/d2286409bf89053dd12a4e7c465476414dc6ba92

Edit: bmbouter reminded me that you can retroactively link commits to issues; this issue has been properly associated to the commit.

#10 Updated by semyers almost 5 years ago

  • Status changed from ASSIGNED to MODIFIED
  • Platform Release set to master

All related PRs have been merged, moving to MODIFIED (sorry for skipping you, POST...).

#11 Updated by dkliban@redhat.com over 4 years ago

  • Platform Release changed from master to 2.8.0

#12 Updated by dkliban@redhat.com over 4 years ago

  • Status changed from MODIFIED to 5

#13 Updated by dkliban@redhat.com over 4 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE

#14 Updated by bmbouter over 1 year ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF