Change the default deployment layout

This changes the default deployment layout. The main change is that
MEDIA_ROOT gets its own directory. This allows limiting the file
permissions in a shared Pulp 2 + Pulp 3 deployment and the SELinux file
contexts. Another benefit is compatibility with django_extensions'
unreferenced_files command which lists all files in MEDIA_ROOT that are
not in the database.

Other paths are kept on the same absolute paths, but the diff looks
bigger because they used derive from MEDIA_ROOT.

The documentation is updated to show the latest best practices.

fixes #7178

CHANGES/7178.feature

@@ -0,0 +1,9 @@
+Change the default deployment layout
+
+This changes the default deployment layout. The main change is that MEDIA_ROOT gets its own
+directory. This allows limiting the file permissions in a shared Pulp 2 + Pulp 3 deployment and the
+SELinux file contexts. Another benefit is compatibility with django_extensions' unreferenced_files
+command which lists all files in MEDIA_ROOT that are not in the database.
+
+Other paths are kept on the same absolute paths. The documentation is updated to show the latest
+best practices.

docs/settings.rst

@@ -55,7 +55,7 @@ DEFAULT_FILE_STORAGE
 MEDIA_ROOT
 ^^^^^^^^^^
 
-   The location where Pulp will store files. By default this is `/var/lib/pulp/`.
+   The location where Pulp will store files. By default this is `/var/lib/pulp/media`.
 
    If you're using S3, point this to the path in your bucket you want to save files. See the
    :ref:`storage documentation <storage>` for more info.
@@ -65,7 +65,7 @@ MEDIA_ROOT
    * mode: 750
    * owner: pulp (the account that pulp runs under)
    * group: pulp (the group of the account that pulp runs under)
-   * SELinux context: system_u:object_r:var_lib_t:s0
+   * SELinux context: system_u:object_r:pulpcore_var_lib_t:s0
 
 LOGGING
 ^^^^^^^
@@ -138,10 +138,10 @@ WORKING_DIRECTORY
 
    It should have permissions of:
 
-   * mode: 755
+   * mode: 750
    * owner: pulp (the account that pulp runs under)
    * group: pulp (the group of the account that pulp runs under)
-   * SELinux context: unconfined_u:object_r:var_lib_t:s0
+   * SELinux context: system_u:object_r:pulpcore_var_lib_t:s0
 
 .. note::
 

pulpcore/app/apps.py

@@ -185,6 +185,10 @@ class PulpAppConfig(PulpPluginAppConfig):
     # The version of this app
     version = "3.10.0.dev"
 
+    def ready(self):
+        super().ready()
+        from . import checks  # noqa
+
 
 def _populate_access_policies(sender, **kwargs):
     from pulpcore.app.util import get_view_urlpattern

pulpcore/app/checks.py

@@ -0,0 +1,43 @@
+from pathlib import Path
+
+from django.conf import settings
+from django.core.checks import Warning as CheckWarning, register
+
+
+@register(deploy=True)
+def storage_paths(app_configs, **kwargs):
+    warnings = []
+
+    if settings.DEFAULT_FILE_STORAGE == "pulpcore.app.models.storage.FileSystem":
+        try:
+            media_root_dev = Path(settings.MEDIA_ROOT).stat().st_dev
+        except OSError:
+            media_root_dev = None
+            warnings.append(
+                CheckWarning(
+                    "Your MEDIA_ROOT setting points to a path that does not exist.",
+                    id="pulpcore.W001",
+                )
+            )
+
+        try:
+            upload_temp_dir_dev = Path(settings.FILE_UPLOAD_TEMP_DIR).stat().st_dev
+        except OSError:
+            upload_temp_dir_dev = None
+            warnings.append(
+                CheckWarning(
+                    "Your FILE_UPLOAD_TEMP_DIR setting points to a path that does not exist.",
+                    id="pulpcore.W002",
+                )
+            )
+
+        if media_root_dev and media_root_dev != upload_temp_dir_dev:
+            warnings.append(
+                CheckWarning(
+                    "MEDIA_ROOT and FILE_UPLOAD_TEMP_DIR are on different filesystems. "
+                    "It is highly recommended that these live on the same filesystem",
+                    id="pulpcore.W003",
+                )
+            )
+
+    return warnings

pulpcore/app/models/storage.py

@@ -98,7 +98,7 @@ def _save(self, name, content, max_length=None):
 
 def get_artifact_path(sha256digest):
     """
-    Determine the absolute path where a file backing the Artifact should be stored.
+    Determine the relative path where a file backing the Artifact should be stored.
 
     Args:
         sha256digest (str): sha256 digest of the file for the Artifact
@@ -112,14 +112,14 @@ def get_artifact_path(sha256digest):
 
 def get_temp_file_path(pulp_id):
     """
-    Determine the absolute path where a file backing the PulpTemporaryFile should be stored.
+    Determine the relative path where a file backing the PulpTemporaryFile should be stored.
 
     Args:
         pulp_id (uuid): An identifier identifying the file for the PulpTemporaryFile
 
     Returns:
-        A string representing the absolute path where a file backing the PulpTemporaryFile should be
-        stored
+        A string representing the path where a file backing the PulpTemporaryFile should be
+        stored. This path is relative to the storage.
     """
     pulp_id_str = str(pulp_id)
     return os.path.join("tmp/files", pulp_id_str[:2], pulp_id_str[2:])

pulpcore/app/settings.py

@@ -8,21 +8,21 @@
 https://docs.djangoproject.com/en/1.11/ref/settings/
 """
 
-import os
 import sys
 
 from contextlib import suppress
 from gettext import gettext as _
 from importlib import import_module
+from pathlib import Path
 from pkg_resources import iter_entry_points
 
 from django.core.exceptions import ImproperlyConfigured
 from django.db import connection
 
 from pulpcore import constants
 
-# Build paths inside the project like this: os.path.join(BASE_DIR, ...)
-BASE_DIR = os.path.dirname(os.path.abspath(__file__))
+# Build paths inside the project like this: BASE_DIR / ...
+BASE_DIR = Path(__file__).absolute().parent
 
 # Quick-start development settings - unsuitable for production
 # See https://docs.djangoproject.com/en/1.11/howto/deployment/checklist/
@@ -32,20 +32,21 @@
 
 ALLOWED_HOSTS = ["*"]
 
-MEDIA_ROOT = "/var/lib/pulp/"
+DEPLOY_ROOT = Path("/var/lib/pulp")
+MEDIA_ROOT = str(DEPLOY_ROOT / "media")  # Django 3.1 adds support for pathlib.Path
 
 ADMIN_SITE_URL = "admin/"
 
 # Static files (CSS, JavaScript, Images)
 # https://docs.djangoproject.com/en/1.11/howto/static-files/
 
 STATIC_URL = "/assets/"
-STATIC_ROOT = os.path.join(MEDIA_ROOT, STATIC_URL.lstrip("/"))
+STATIC_ROOT = DEPLOY_ROOT / STATIC_URL.strip("/")
 
 DEFAULT_FILE_STORAGE = "pulpcore.app.models.storage.FileSystem"
 
-FILE_UPLOAD_TEMP_DIR = os.path.join(MEDIA_ROOT, "tmp/")
-WORKING_DIRECTORY = os.path.join(MEDIA_ROOT, "tmp/")
+FILE_UPLOAD_TEMP_DIR = DEPLOY_ROOT / "tmp"
+WORKING_DIRECTORY = FILE_UPLOAD_TEMP_DIR
 CHUNKED_UPLOAD_DIR = "upload"
 
 # List of upload handler classes to be applied in order.
@@ -118,7 +119,7 @@
 TEMPLATES = [
     {
         "BACKEND": "django.template.backends.django.DjangoTemplates",
-        "DIRS": [os.path.join(BASE_DIR, "templates")],
+        "DIRS": [BASE_DIR / "templates"],
         "APP_DIRS": True,
         "OPTIONS": {
             "context_processors": [

pulpcore/tests/functional/api/test_crd_artifacts.py

@@ -6,7 +6,7 @@
 
 from pulp_smash import api, cli, config, utils
 from pulp_smash.exceptions import CalledProcessError
-from pulp_smash.pulp3.constants import ARTIFACTS_PATH, MEDIA_PATH
+from pulp_smash.pulp3.constants import ARTIFACTS_PATH
 from pulp_smash.pulp3.utils import delete_orphans
 from requests.exceptions import HTTPError
 
@@ -165,14 +165,15 @@ def test_all(self):
         storage = utils.get_pulp_setting(cli_client, "DEFAULT_FILE_STORAGE")
         if storage != "pulpcore.app.models.storage.FileSystem":
             self.skipTest("this test only works for filesystem storage")
+        media_root = utils.get_pulp_setting(cli_client, "MEDIA_ROOT")
 
         api_client = api.Client(cfg, api.json_handler)
 
         # create
         files = {"file": utils.http_get(FILE_URL)}
         artifact = api_client.post(ARTIFACTS_PATH, files=files)
         self.addCleanup(api_client.delete, artifact["pulp_href"])
-        cmd = ("ls", os.path.join(MEDIA_PATH, artifact["file"]))
+        cmd = ("ls", os.path.join(media_root, artifact["file"]))
         cli_client.run(cmd, sudo=True)
 
         # delete

pulpcore/tests/functional/api/using_plugin/test_orphans.py

@@ -6,7 +6,6 @@
 from pulp_smash import cli, config, utils
 from pulp_smash.exceptions import CalledProcessError
 from pulp_smash.pulp3.bindings import monitor_task
-from pulp_smash.pulp3.constants import MEDIA_PATH
 from pulp_smash.pulp3.utils import (
     delete_orphans,
     delete_version,
@@ -52,6 +51,7 @@ def setUpClass(cls):
         cls.api_client = ApiClient(configuration)
         cls.cli_client = cli.Client(cls.cfg)
         cls.storage = utils.get_pulp_setting(cls.cli_client, "DEFAULT_FILE_STORAGE")
+        cls.media_root = utils.get_pulp_setting(cls.cli_client, "MEDIA_ROOT")
 
     def test_clean_orphan_content_unit(self):
         """Test whether orphan content units can be clean up.
@@ -92,7 +92,8 @@ def test_clean_orphan_content_unit(self):
 
         if self.storage == "pulpcore.app.models.storage.FileSystem":
             # Verify that the artifact is present on disk.
-            artifact_path = os.path.join(MEDIA_PATH, artifacts_api.read(content["artifact"]).file)
+            relative_path = artifacts_api.read(content["artifact"]).file
+            artifact_path = os.path.join(self.media_root, relative_path)
             cmd = ("ls", artifact_path)
             self.cli_client.run(cmd, sudo=True)
 
@@ -124,7 +125,7 @@ def test_clean_orphan_artifact(self):
         artifact = artifacts_api.create(file=__file__)
 
         if self.storage == "pulpcore.app.models.storage.FileSystem":
-            cmd = ("ls", os.path.join(MEDIA_PATH, artifact.file))
+            cmd = ("ls", os.path.join(self.media_root, artifact.file))
             self.cli_client.run(cmd, sudo=True)
 
         delete_orphans()

pulpcore/tests/functional/api/using_plugin/test_repair.py

@@ -4,7 +4,7 @@
 from urllib.parse import urljoin
 
 from pulp_smash import api, cli, config, utils
-from pulp_smash.pulp3.constants import MEDIA_PATH, BASE_PATH
+from pulp_smash.pulp3.constants import BASE_PATH
 from pulp_smash.pulp3.utils import (
     delete_orphans,
     gen_repo,
@@ -67,13 +67,14 @@ def setUp(self):
         repo = self.api_client.get(repo["pulp_href"])
 
         # STEP 2
+        media_root = utils.get_pulp_setting(self.cli_client, "MEDIA_ROOT")
         content1, content2 = sample(get_content(repo)[FILE_CONTENT_NAME], 2)
         # Muddify one artifact on disk.
-        artifact1_path = os.path.join(MEDIA_PATH, self.api_client.get(content1["artifact"])["file"])
+        artifact1_path = os.path.join(media_root, self.api_client.get(content1["artifact"])["file"])
         cmd1 = ("sed", "-i", "-e", r"$a bit rot", artifact1_path)
         self.cli_client.run(cmd1, sudo=True)
         # Delete another one from disk.
-        artifact2_path = os.path.join(MEDIA_PATH, self.api_client.get(content2["artifact"])["file"])
+        artifact2_path = os.path.join(media_root, self.api_client.get(content2["artifact"])["file"])
         cmd2 = ("rm", artifact2_path)
         self.cli_client.run(cmd2, sudo=True)
 

pulpcore/tests/unit/models/test_content.py

@@ -67,7 +67,8 @@ def test_storage_location(self):
             temp_file.save()
 
         assert temp_file.file.name.startswith("tmp/files/")
-        assert temp_file.file.file.name.startswith("/var/lib/pulp/tmp/files")
+        name = temp_file.file.file.name
+        assert name.startswith("/var/lib/pulp/media/tmp/files"), name
 
     def test_read_temp_file(self):
         with tempfile.NamedTemporaryFile("ab") as tf: