fix(nginx_k8s): close file handles returned by container.pull

nginx_k8s/src/charmlibs/nginx_k8s/_nginx.py

@@ -93,21 +93,9 @@ def _configure_tls(self, private_key: str, server_cert: str, ca_cert: str) -> No
 
         if self._container.can_connect():
             # Read the current content of the files (if they exist)
-            current_server_cert = (
-                self._container.pull(self.CERT_PATH).read()
-                if self._container.exists(self.CERT_PATH)
-                else ''
-            )
-            current_private_key = (
-                self._container.pull(self.KEY_PATH).read()
-                if self._container.exists(self.KEY_PATH)
-                else ''
-            )
-            current_ca_cert = (
-                self._container.pull(self.CA_CERT_PATH).read()
-                if self._container.exists(self.CA_CERT_PATH)
-                else ''
-            )
+            current_server_cert = self._read_if_exists(self.CERT_PATH)
+            current_private_key = self._read_if_exists(self.KEY_PATH)
+            current_ca_cert = self._read_if_exists(self.CA_CERT_PATH)
 
             if (
                 current_server_cert == server_cert
@@ -122,6 +110,13 @@ def _configure_tls(self, private_key: str, server_cert: str, ca_cert: str) -> No
             logger.debug('running update-ca-certificates')
             self._container.exec(['update-ca-certificates', '--fresh']).wait()
 
+    def _read_if_exists(self, path: str) -> str:
+        """Return the text contents of ``path`` in the workload, or '' if missing."""
+        if not self._container.exists(path):
+            return ''
+        with self._container.pull(path) as f:
+            return f.read()
+
     def _delete_certificates(self) -> None:
         """Delete the certificate files from disk and run update-ca-certificates."""
         with _tracer.start_as_current_span('delete ca cert'):
@@ -166,7 +161,8 @@ def _has_config_changed(self, new_config: str) -> bool:
 
         try:
             with _tracer.start_as_current_span('read config'):
-                current_config = self._container.pull(self.NGINX_CONFIG).read()
+                with self._container.pull(self.NGINX_CONFIG) as f:
+                    current_config = f.read()
         except pebble.PathError:
             logger.debug('nginx configuration file not found at %s', str(self.NGINX_CONFIG))
             # file does not exist! it's probably because it's the first time we're generating it.

nginx_k8s/src/charmlibs/nginx_k8s/_tls_config.py

@@ -64,21 +64,9 @@ def _sync_certificates(self, tls_config: TLSConfig) -> None:
         if self._container.can_connect():
             # Read the current content of the files (if they exist)
             with _tracer.start_as_current_span('read tls config files'):
-                current_server_cert = (
-                    self._container.pull(self.CERT_PATH).read()
-                    if self._container.exists(self.CERT_PATH)
-                    else ''
-                )
-                current_private_key = (
-                    self._container.pull(self.KEY_PATH).read()
-                    if self._container.exists(self.KEY_PATH)
-                    else ''
-                )
-                current_ca_cert = (
-                    self._container.pull(self.CA_CERT_PATH).read()
-                    if self._container.exists(self.CA_CERT_PATH)
-                    else ''
-                )
+                current_server_cert = self._read_if_exists(self.CERT_PATH)
+                current_private_key = self._read_if_exists(self.KEY_PATH)
+                current_ca_cert = self._read_if_exists(self.CA_CERT_PATH)
 
             if (
                 current_server_cert == tls_config.server_cert
@@ -96,6 +84,13 @@ def _sync_certificates(self, tls_config: TLSConfig) -> None:
             if self._update_ca_certificates_on_restart:
                 self._container.exec(['update-ca-certificates', '--fresh']).wait()
 
+    def _read_if_exists(self, path: str) -> str:
+        """Return the text contents of ``path`` in the workload, or '' if missing."""
+        if not self._container.exists(path):
+            return ''
+        with self._container.pull(path) as f:
+            return f.read()
+
     def _delete_certificates(self) -> None:
         """Delete the certificate files from disk and run update-ca-certificates."""
         with _tracer.start_as_current_span('delete tls config files'):

nginx_k8s/tests/unit/test_no_resource_leaks.py

@@ -0,0 +1,84 @@
+# Copyright 2026 Canonical
+# See LICENSE file for licensing details.
+
+"""Regression tests for ResourceWarning leaks from container.pull().
+
+``ops.Container.pull()`` returns a real Python file object (backed by a
+``tempfile.NamedTemporaryFile`` under real pebble, or by ``open()`` in
+``ops.testing``). Failing to close that handle emits a ``ResourceWarning``
+when the object is garbage-collected, which fails any test run under
+``-W error`` and indicates a real file-descriptor leak in production.
+
+CPython releases the unreferenced file object as soon as ``pull(...).read()``
+returns (refcount drops to zero), so the destructor fires synchronously and
+the warning lands inside the ``catch_warnings`` window without an explicit
+``gc.collect()``. Avoiding that call keeps these tests insensitive to
+unrelated cycle-collected leaks elsewhere in the stack (e.g. in
+``scenario.Context``).
+"""
+
+from __future__ import annotations
+
+import warnings
+from dataclasses import replace
+
+from ops import testing
+
+from charmlibs.nginx_k8s import Nginx, TLSConfig, TLSConfigManager
+
+
+def _assert_no_unclosed_file_warnings(caught: list[warnings.WarningMessage]) -> None:
+    leaks = [
+        w
+        for w in caught
+        if issubclass(w.category, ResourceWarning) and 'unclosed file' in str(w.message)
+    ]
+    assert not leaks, '\n'.join(str(w.message) for w in leaks)
+
+
+def test_has_config_changed_closes_pull_handle(
+    ctx: testing.Context, nginx_container: testing.Container, tmp_path
+):
+    """``Nginx._has_config_changed`` must close the file returned by ``pull()``."""
+    config_file = tmp_path / 'nginx.conf'
+    config_file.write_text('foo')
+    container = replace(
+        nginx_container,
+        mounts={
+            'config': testing.Mount(location=Nginx.NGINX_CONFIG, source=str(config_file)),
+        },
+    )
+    state = testing.State(containers={container})
+
+    with ctx(ctx.on.update_status(), state=state) as mgr:
+        nginx = Nginx(mgr.charm.unit.get_container(nginx_container.name))
+        with warnings.catch_warnings(record=True) as caught:
+            warnings.simplefilter('always')
+            assert nginx._has_config_changed('foo') is False
+        _assert_no_unclosed_file_warnings(caught)
+
+
+def test_sync_certificates_closes_pull_handles(
+    ctx: testing.Context, nginx_container: testing.Container, tmp_path
+):
+    """``TLSConfigManager._sync_certificates`` must close every ``pull()`` handle."""
+    mounts = {}
+    for path, name in (
+        (TLSConfigManager.KEY_PATH, 'key'),
+        (TLSConfigManager.CERT_PATH, 'cert'),
+        (TLSConfigManager.CA_CERT_PATH, 'cacert'),
+    ):
+        f = tmp_path / name
+        f.write_text('foo')
+        mounts[name] = testing.Mount(location=path, source=str(f))
+    container = replace(nginx_container, mounts=mounts)
+    state = testing.State(containers={container})
+
+    with ctx(ctx.on.update_status(), state=state) as mgr:
+        tls_mgr = TLSConfigManager(mgr.charm.unit.get_container(nginx_container.name))
+        with warnings.catch_warnings(record=True) as caught:
+            warnings.simplefilter('always')
+            tls_mgr._sync_certificates(
+                TLSConfig(server_cert='foo', ca_cert='foo', private_key='foo'),
+            )
+        _assert_no_unclosed_file_warnings(caught)