Merge pull request #3270 from candlepin/mhorky/ent-5580-container-det…

…ection

ENT-5580: Disable the proper container detection

.github/workflows/pytest.yml

@@ -42,7 +42,6 @@ jobs:
 
       - name: "Run pytest"
         env:
-          SUBMAN_TEST_IN_CONTAINER: "1"
           PYTEST_ADDOPTS:
             "--color=yes --code-highlight=yes --showlocals
             --cov 'src/' --cov-report 'term:skip-covered'

TESTING.md

@@ -87,16 +87,15 @@ Enter the container (assuming you are in the project root) and run a pre-test sc
 NAME="subscription-manager"  # Or something more descriptive, like "rhsm-cs9"
 podman run -it --rm \
   --name $NAME \
-  -v .:/subscription-manager --workdir /subscription-manager \
-  --env 'SUBMAN_TEST_IN_CONTAINER=1' --privileged \
+  -v .:/subscription-manager --workdir /subscription-manager --privileged \
   $IMAGE bash
 bash scripts/container-pre-test.sh
 ```
 
 Then you can run the test suite. You have to use `dbus-run-session` wrapper, because D-Bus is not running in containers:
 
 ```bash
-SUBMAN_TEST_IN_CONTAINER=1 dbus-run-session python3 -m pytest
+dbus-run-session python3 -m pytest
 ```
 
 ### Local subscription-manager images

src/rhsm/config.py

@@ -107,54 +107,26 @@
 
 def in_container() -> bool:
     """
-    Are we running in a docker container or not?
+    Are we running in a container or not?
     """
-    # For development in containers we must be able to turn container detection
-    # off
+    # For development in containers we must be able to turn container detection off
     if os.environ.get("SMDEV_CONTAINER_OFF", ""):
         return False
 
-    def in_ocp() -> bool:
-        """
-        Is the system running as pod in OCP (OpenShift Container Platform)?
-
-        Check some of the canonical environment variables set by Kubernets
-        (on which OCP is based):
-        https://kubernetes.io/docs/concepts/containers/container-environment/
-        in particular, look for the "kubernetes" default service
+    # If the path exists, we are in a container.
+    # In UBI containers (RHEL, CentOS), path HOST_CONFIG_DIR='/etc/rhsm-host/'
+    # is a symlink to /run/secrets/rhsm. That path is a symlink/Podman secret
+    # specified in /usr/share/containers/mounts.conf, pointing to host's directory
+    # /usr/share/rhel/secrets. The directories inside are themselves symlinks
+    # to other host directories populated by subscription-manager.
+    # If this secret (= the container directory /etc/rhsm-host/) exists,
+    #   the system is considered to be a container.
+    # If this secret does not exist,
+    #   the system is considered to be a non-container.
+    if os.path.isdir(HOST_CONFIG_DIR):
+        log.debug(f"Container detected: found certificate directory {HOST_CONFIG_DIR}.")
+        return True
 
-        "container=oci" is generally set by Red Hat-based containers.
-        """
-        return (
-            os.environ.get("KUBERNETES_PORT", "") != ""
-            and os.environ.get("KUBERNETES_SERVICE_HOST", "") != ""
-            and os.environ.get("KUBERNETES_SERVICE_PORT", "") != ""
-            and os.environ.get("container", "") == "oci"
-        )
-
-    # Known locations to check for as an easy way to detect whether
-    # we are running in a container; note that pods in OCP are not
-    # considered containers but standalone systems
-    locations: List[str] = [
-        # podman:
-        # https://github.com/containers/podman/issues/6192
-        # https://github.com/containers/podman/issues/3586#issuecomment-661918679
-        "/run/.containerenv",
-        # docker:
-        # https://github.com/moby/moby/issues/18355
-        "/.dockerenv",
-        # The host rhsm configuration was shared with us, so assume
-        # we must be running in a container
-        HOST_CONFIG_DIR,
-    ]
-    for fn in locations:
-        if os.path.exists(fn):
-            log.debug(f"in_container(): found '{fn}', may be a container")
-            is_ocp = in_ocp()
-            if is_ocp:
-                log.debug("in_container(): found kubernetes/OCP environment, not considering container")
-                return False
-            return True
     return False
 
 

test/rhsm/unit/test_config.py

@@ -17,7 +17,6 @@
     InterpolationDepthError,
     NoSectionError,
 )
-import os
 from tempfile import NamedTemporaryFile
 import unittest
 
@@ -403,57 +402,31 @@ def test_get_repo_ca_cert(self):
 
 
 class InContainerTests(unittest.TestCase):
-    def test_in_container_off(self):
-        # This must be set in the test environment by CI in order to know for
-        # sure one way or the other whether we really are in a container.
-        # It must NOT be set to something other than the truth of whether we are
-        # in a container otherwise this test will no longer be meaningful
-        really_in_container = True if os.environ.get("SUBMAN_TEST_IN_CONTAINER", False) else False
-        with patch.dict(os.environ, {"SMDEV_CONTAINER_OFF": "True"}):
-            self.assertFalse(in_container())
-        self.assertEqual(in_container(), really_in_container)
-
-    @patch.dict("os.environ", {}, clear=True)
-    @patch("os.path.exists")
-    def test_existing_dotcontainer(self, exists_mock):
-        def exists_file(path):
-            return path == "/run/.containerenv"
-
-        exists_mock.side_effect = exists_file
-        self.assertTrue(in_container())
-
-    @patch.dict("os.environ", {}, clear=True)
-    @patch("os.path.exists")
-    def test_existing_dotdockerenv(self, exists_mock):
-        def exists_file(path):
-            return path == "/.dockerenv"
-
-        exists_mock.side_effect = exists_file
-        self.assertTrue(in_container())
-
-    @patch.dict("os.environ", {}, clear=True)
-    @patch("os.path.exists")
-    def test_existing_rhsm_host(self, exists_mock):
-        def exists_file(path):
-            return path == "/etc/rhsm-host/"
-
-        exists_mock.side_effect = exists_file
-        self.assertTrue(in_container())
-
-    @patch.dict(
-        "os.environ",
-        {
-            "KUBERNETES_PORT": "tcp://10.0.0.1:443",
-            "KUBERNETES_SERVICE_HOST": "10.0.0.1",
-            "KUBERNETES_SERVICE_PORT": "443",
-            "container": "oci",
-        },
-        clear=True,
-    )
-    @patch("os.path.exists")
-    def test_ocp(self, exists_mock):
-        def exists_file(path):
-            return path == "/run/.containerenv"
-
-        exists_mock.side_effect = exists_file
-        self.assertFalse(in_container())
+    """Test that config.is_container() detects container system via /etc/rhsm-host/.
+
+    In previous versions of subscription-manager (starting with 1.29), the
+    container detection was extended to also cover files like
+    - /.dockerenv
+    - /run/.containerenv
+    and environment variables like
+    - KUBERNETES_*
+    - container == 'oci'
+
+    This, however, broke many workflows that relied on registering containers
+    during build or runtime, and required poorly supportable workarounds, so it
+    was reverted back to 'simple' detection.
+    """
+
+    @patch("os.path.isdir")
+    def test_etc_rhsm_host_exists(self, os_path_isdir_mock):
+        os_path_isdir_mock.side_effect = lambda path: path == "/etc/rhsm-host/"
+
+        exists: bool = in_container()
+        self.assertTrue(exists)
+
+    @patch("os.path.isdir")
+    def test_etc_rhsm_host_does_not_exist(self, os_path_isdir_mock):
+        os_path_isdir_mock.side_effect = lambda path: path != "/etc/rhsm-host/"
+
+        exists: bool = in_container()
+        self.assertFalse(exists)