From 2983b716317dc22f6f631d9783361e2758478990 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 11:58:51 +0530 Subject: [PATCH 01/27] idle timeout implementation --- MIGRATION.md | 109 ++++++++++++++++ docs/CONNECTION_MODES.md | 39 ++++++ .../plugin_utils/manager/manager_process.py | 62 ++++++++- .../plugin_utils/manager/platform_manager.py | 29 ++++- .../plugin_utils/manager/process_manager.py | 1 + plugins/plugin_utils/platform/config.py | 21 +++- .../manager/test_platform_service_idle.py | 118 ++++++++++++++++++ 7 files changed, 374 insertions(+), 5 deletions(-) create mode 100644 MIGRATION.md create mode 100644 docs/CONNECTION_MODES.md create mode 100644 tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py diff --git a/MIGRATION.md b/MIGRATION.md new file mode 100644 index 00000000..d624a827 --- /dev/null +++ b/MIGRATION.md @@ -0,0 +1,109 @@ +# Migration guide: persistent Gateway connections (AAP 2.7 / `ansible.platform`) + +This document describes backward compatibility, the recommended migration path, and an optional phased deprecation plan for the **persistent manager / connection-plugin** work in the `ansible.platform` collection. It applies to automation targeting **Ansible Automation Platform 2.7** and the `ansible.platform` collection version bundled or pinned with that release. + +Confirm the exact collection version and any Red Hat release notes for your environment; behavior described here follows the current collection implementation. + +--- + +## What changed (summary) + +- **Connection plugin `ansible.platform.http`** is the supported way to route Gateway traffic. It implements `get_client()`, which chooses **direct** (ephemeral manager per task) or **persistent** (reuse one manager process and HTTP session across tasks in a play) mode. +- **Action plugins** for platform modules call `_get_or_spawn_manager()`, which **prefers** the connection plugin's `get_client()` when `ansible_connection` is `ansible.platform.http`. They still work with **`ansible_connection: local`** by spawning an **ephemeral** manager (`spawn_ephemeral_client()`), but that path does **not** integrate with the connection plugin's persistent lifecycle or facts in the same way. +- **Direct mode** (default) still uses the same manager-based stack as persistent mode; the difference is **lifecycle** (new ephemeral manager per task vs reuse). Performance tuning is primarily about **persistent** mode and fewer TLS/auth round-trips. +- **Stale socket recovery**: when reusing a persistent manager, if the socket file exists but the process is gone, the connection plugin detects a **stale socket**, removes it, and spawns a new manager. + +--- + +## Backward compatibility (today) + +| Configuration | Behavior | Persistent reuse across tasks? | +|---------------|----------|--------------------------------| +| `ansible_connection: ansible.platform.http` and `persistent: false` (default) | Ephemeral manager per task via connection plugin | No | +| `ansible_connection: ansible.platform.http` and `persistent: true` (or equivalent vars, see below) | One manager per play/host/credential set; facts cache socket + authkey | Yes | + +Existing playbooks that use **`connection: local`** and pass Gateway options continue to run **without** switching the connection plugin, as long as they use modules that have a matching **action plugin** (the normal case for resource modules in this collection). + +--- + +## Migration path + +### 1. Use the platform HTTP connection plugin (recommended) + +Set the inventory host (or group vars) that represents the Gateway to use the collection connection plugin: + +```yaml +# inventory.yml (example) +all: + children: + gateway_hosts: + hosts: + aap_gateway: + ansible_host: gateway.example.com # informational; API target is still gateway_url + ansible_connection: ansible.platform.http + # Optional: enable persistent mode for this host + ansible_platform_use_persistent_connection: true +``` + +FQCN for the plugin transport is **`ansible.platform.http`** (see `transport` in `plugins/connection/http.py`). + +### 2. Gateway URL and credentials + +The action layer builds a `GatewayConfig` via `extract_gateway_config()` from **task arguments** and **host/task variables**. At minimum you must supply a Gateway base URL: + +| Purpose | Task / host variables (priority order in code) | +|---------|------------------------------------------------| +| Gateway URL | `gateway_url` or `gateway_hostname` | +| Username / password | `gateway_username` / `gateway_password`, or aliases `aap_username` / `aap_password` | +| OAuth token | `gateway_token` or `aap_token` (with special handling so a module-created `aap_token` dict does not override user/password auth) | +| TLS / timeout | `gateway_validate_certs`, `gateway_request_timeout` (and `aap_*` aliases where documented in fragments) | + +**Automation Controller (AAP) job templates:** map your **credential** or **extra variables** so the above keys are present for the Gateway host (or for `localhost` if you use a single inventory host for API tasks). The exact credential type and injectors depend on your Controller version; align injectors with the variable names this collection reads (`gateway_*` / `aap_*`). + +### 3. Enabling persistent vs direct mode + +Resolution order for **persistent** behavior (connection plugin `get_client()`): + +1. Connection option **`persistent`** if Ansible supplies it for `ansible.platform.http` (for example via plugin configuration that maps to `get_option('persistent')`). +2. If that option is unset, **`ansible_platform_use_persistent_connection`** from host vars or task vars (and a host-var form under `hostvars[inventory_hostname]`). +3. Else **`ansible_platform_persistent`** (same scoping as above). +4. Else environment **`ANSIBLE_PLATFORM_PERSISTENT`**. +5. Else INI **`[platform_connection] persistent=`** (see plugin `DOCUMENTATION`). +6. Default: **false** (direct / ephemeral per task). + +In practice, most playbooks use **`ansible_platform_use_persistent_connection`** or **`ansible_platform_persistent`** (as in integration and Molecule scenarios). + +Truthy values are boolean `true` or strings `true`, `yes`, `1` (see `_truthy()` in the connection plugin). + +When persistent mode spawns a manager, the action plugin result may include **cacheable facts**: + +- `platform_manager_socket` +- `platform_manager_authkey` +- `gateway_url` (when returned by the connection plugin) + +These allow the next task to reuse the same manager. Changing **URL or credentials** changes the derived socket identity; do not expect reuse across different Gateway identities. + +### 4. Operational notes + +- **Socket locations**: persistent managers use `$(TMPDIR or system temp)/ansible_platform/`; ephemeral paths used by direct mode may use short paths under `/tmp/ap/` (see connection plugin and `spawn_ephemeral_client()`). +- **AF_UNIX**: if Unix domain sockets are unavailable, the local fallback can use `DirectHTTPClient` without a manager process (see `spawn_ephemeral_client()`). +- **`platform_connection_mode`**: still parsed into `GatewayConfig` for compatibility; routing between persistent and direct is controlled by the **connection plugin** options/vars above, not by switching this field alone. + +--- + +## Parallel support: modules and action plugins + +- Platform **resource modules** in this collection are intended to run with their **action plugins**, which perform validation, manager acquisition, and API execution. +- **Parallel support** means you may keep **`connection: local`** during a transition while you test **`ansible.platform.http`** on staging inventories. Both paths use the manager architecture (except AF_UNIX fallback), but only the HTTP connection plugin provides **centralized** persistent vs direct policy and stale-socket handling aligned with connection-level configuration. + +--- + +## Quick checklist + +- [ ] Set `ansible_connection: ansible.platform.http` on the Gateway inventory host (or group). +- [ ] Supply `gateway_url` / `gateway_hostname` and auth (`gateway_username`/`gateway_password` or token vars). +- [ ] Decide on **persistent** (`ansible_platform_use_persistent_connection: true` or connection option `persistent: true`) vs **direct** (default). +- [ ] Validate job templates and credentials inject the same variable names your playbooks expect. +- [ ] After upgrade, run a multi-task playbook once with persistent mode and confirm fact-driven reuse (or benchmark latency improvement). + +For architecture background, see `docs/03-sdk-architecture.md` and `docs/06-foundation-components.md` in this repository. diff --git a/docs/CONNECTION_MODES.md b/docs/CONNECTION_MODES.md new file mode 100644 index 00000000..75bff6b8 --- /dev/null +++ b/docs/CONNECTION_MODES.md @@ -0,0 +1,39 @@ +# Connection modes (standard vs persistent manager) + +The `ansible.platform.http` connection plugin can run in **direct (standard)** mode or **persistent (experimental)** mode. Persistent mode spawns a separate **manager process** that keeps an HTTP session and shared caches across tasks. + +## Idle timeout (`idle_timeout`) + +Manager processes would otherwise stay alive until the Ansible owner process exits or the socket is cleaned up. To reduce orphaned managers and memory use, the manager tracks **last activity** (RPC calls such as `execute` and `lookup_resource_id`, and HTTP requests made through the service) and shuts down automatically when nothing has run for longer than the configured **idle timeout**. + +| Behavior | Detail | +|----------|--------| +| Default | **3600** seconds (1 hour) | +| Disable | Set to **0** (manager only exits via owner PID watchdog, signals, or explicit shutdown — not recommended for production) | +| Poll interval | A background thread checks idle state every **60** seconds | +| On timeout | The manager calls `PlatformService.shutdown()`, stops the RPC server, removes the Unix socket (and `.meta` if present), and exits | + +### Configuration + +Set the timeout in seconds using either: + +- **`gateway_idle_timeout`** in task arguments or inventory/host variables, or +- **`ansible_platform_manager_idle_timeout`** in inventory/host variables (alias). + +Example (inventory): + +```yaml +gateway_idle_timeout: 1800 +``` + +Values are passed through `GatewayConfig` into the manager subprocess when the connection plugin spawns the process. + +### Test-only: poll interval + +For automated tests, the check interval can be overridden with the environment variable **`ANSIBLE_PLATFORM_IDLE_POLL_SECONDS`** (default `60`). Production deployments should rely on the default 60-second polling. + +## Related components + +- **`GatewayConfig`** (`plugins/plugin_utils/platform/config.py`) — holds `idle_timeout` and other gateway settings. +- **`PlatformService`** (`plugins/plugin_utils/manager/platform_manager.py`) — records activity and implements `should_exit_for_idle()`. +- **`manager_process.py`** — standalone entry point: starts the idle monitor thread and performs socket cleanup on exit. diff --git a/plugins/plugin_utils/manager/manager_process.py b/plugins/plugin_utils/manager/manager_process.py index 09a115da..ca0801da 100644 --- a/plugins/plugin_utils/manager/manager_process.py +++ b/plugins/plugin_utils/manager/manager_process.py @@ -12,6 +12,9 @@ import traceback from pathlib import Path +# Interval between idle checks (seconds). Production default: 60. +MANAGER_IDLE_POLL_INTERVAL = int(os.environ.get("ANSIBLE_PLATFORM_IDLE_POLL_SECONDS", "60")) + def main(): """Main entry point for the manager process.""" @@ -34,8 +37,8 @@ def _safe_argv(): pass if len(sys.argv) < 10: - print(f"ERROR: Expected 9 args, got {len(sys.argv) - 1}", file=sys.stderr) - print(f"Args received: {_safe_argv()}", file=sys.stderr) + print(f"ERROR: Expected at least 9 args (optional 10th: idle_timeout), got {len(sys.argv) - 1}", file=sys.stderr) + print(f"Args received: {sys.argv}", file=sys.stderr) sys.exit(1) marker = Path("/tmp/ansible_platform_manager_started.txt") @@ -57,6 +60,7 @@ def log_marker(msg): gateway_token = sys.argv[7] or None gateway_validate_certs = sys.argv[8].lower() == "true" gateway_request_timeout = float(sys.argv[9]) + gateway_idle_timeout = float(sys.argv[10]) if len(sys.argv) > 10 else 3600.0 log_marker("Arguments parsed successfully") log_marker("Reading environment variables...") @@ -146,6 +150,7 @@ def log_marker(msg): verify_ssl=gateway_validate_certs, request_timeout=gateway_request_timeout, connection_mode="experimental", # Persistent manager is always experimental mode + idle_timeout=gateway_idle_timeout, ) with open(error_log, "a") as f: f.write("GatewayConfig created successfully\n") @@ -335,6 +340,59 @@ def _owner_watchdog(): _init_thread = threading.Thread(target=_init_service, daemon=True) _init_thread.start() + # Idle shutdown: after idle_timeout seconds with no RPC/API activity, exit and remove socket + if float(config.idle_timeout) > 0: + + def _idle_monitor(): + import time as _time + + from ansible_collections.ansible.platform.plugins.plugin_utils.manager.process_manager import ProcessManager + + while True: + _time.sleep(MANAGER_IDLE_POLL_INTERVAL) + if not _service_ready.is_set(): + continue + svc = _service_container.get("service") + if svc is None: + continue + if not svc.should_exit_for_idle(): + continue + try: + with open(error_log, "a") as _f: + _f.write("Idle timeout exceeded, shutting down manager\n") + _f.flush() + except Exception: + pass + try: + _shutdown_service() + except Exception as _e: + try: + with open(error_log, "a") as _f: + _f.write(f"Idle shutdown (service): {_e}\n") + _f.flush() + except Exception: + pass + try: + server.shutdown() + except Exception as _e: + try: + with open(error_log, "a") as _f: + _f.write(f"Idle shutdown (server): {_e}\n") + _f.flush() + except Exception: + pass + try: + ProcessManager.cleanup_old_socket(socket_path) + except Exception: + pass + os._exit(0) + + _idle_thread = threading.Thread(target=_idle_monitor, daemon=True, name="idle-timeout") + _idle_thread.start() + with open(error_log, "a") as f: + f.write(f"Idle timeout monitor started (interval={MANAGER_IDLE_POLL_INTERVAL}s, idle_timeout={config.idle_timeout}s)\n") + f.flush() + try: server.serve_forever() except KeyboardInterrupt: diff --git a/plugins/plugin_utils/manager/platform_manager.py b/plugins/plugin_utils/manager/platform_manager.py index 5b7d1809..ffeb3cb9 100644 --- a/plugins/plugin_utils/manager/platform_manager.py +++ b/plugins/plugin_utils/manager/platform_manager.py @@ -10,6 +10,7 @@ import logging import threading import time +import time from dataclasses import asdict from multiprocessing.managers import BaseManager from socketserver import ThreadingMixIn @@ -112,12 +113,35 @@ def __init__(self, config: GatewayConfig): self._shutdown_requested = False self._shutdown_lock = threading.Lock() - # Idle-tracking state (monotonic clock — not affected by NTP slew or wall-clock changes) + # Idle timeout: last time the service handled user-facing work (RPC / HTTP) + self._activity_lock = threading.Lock() + self._last_activity_monotonic = time.monotonic() + + # Idle timeout: last time the service handled user-facing work (RPC / HTTP) self._activity_lock = threading.Lock() self._last_activity_monotonic = time.monotonic() self.retry_config = RetryConfig(max_attempts=3, initial_delay=1.0, max_delay=60.0, exponential_base=2.0, jitter=True) + def record_activity(self) -> None: + """Mark the service as recently active (RPC or HTTP traffic).""" + with self._activity_lock: + self._last_activity_monotonic = time.monotonic() + + def seconds_since_last_activity(self) -> float: + """Wall-clock elapsed seconds since the last record_activity() call.""" + with self._activity_lock: + return time.monotonic() - self._last_activity_monotonic + + def should_exit_for_idle(self) -> bool: + """True if idle_timeout is enabled and exceeded (and not already shutting down).""" + if self.config.idle_timeout <= 0: + return False + with self._shutdown_lock: + if self._shutdown_requested: + return False + return self.seconds_since_last_activity() >= self.config.idle_timeout + def _make_request(self, method: str, url: str, operation: str = "http_request", resource: str = "unknown", **kwargs) -> "requests.Response": """ Make HTTP request with retry logic (using decorator pattern). @@ -137,6 +161,7 @@ def _make_request(self, method: str, url: str, operation: str = "http_request", Raises: PlatformError: Classified platform error """ + self.record_activity() @retry_http_request(config=self.retry_config) def _execute_with_retry(): @@ -492,6 +517,7 @@ def execute(self, operation: str, module_name: str, ansible_data_dict: dict) -> Raises: ValueError: If operation is unknown or execution fails """ + self.record_activity() logger.info("Executing %s on %s", operation, module_name) # Pop action-only flags before building dataclass (action sets _platform_enforced for enforced state) @@ -1102,6 +1128,7 @@ def lookup_resource_id(self, endpoint: str, lookup_field: str, lookup_value: str Resolve a resource name to ID by GET list with filter. Used by mixins to resolve FKs (e.g. service_cluster name -> id). """ + self.record_activity() if not lookup_value: return None if str(lookup_value).isdigit(): diff --git a/plugins/plugin_utils/manager/process_manager.py b/plugins/plugin_utils/manager/process_manager.py index d27998be..938a1dba 100644 --- a/plugins/plugin_utils/manager/process_manager.py +++ b/plugins/plugin_utils/manager/process_manager.py @@ -243,6 +243,7 @@ def spawn_manager_process( gateway_config.oauth_token or "", str(gateway_config.verify_ssl), str(gateway_config.request_timeout), + str(gateway_config.idle_timeout), ] logger.debug("Command: %s %s [args: socket_path, socket_dir, identifier, gateway_url, ...]", sys.executable, script_path) diff --git a/plugins/plugin_utils/platform/config.py b/plugins/plugin_utils/platform/config.py index 2b3a319b..edae4f32 100644 --- a/plugins/plugin_utils/platform/config.py +++ b/plugins/plugin_utils/platform/config.py @@ -26,6 +26,9 @@ class GatewayConfig: verify_ssl: bool = True request_timeout: float = 10.0 connection_mode: str = "standard" # "standard" or "experimental" + #: Seconds with no API activity before the persistent manager process exits. + #: Set to 0 to disable idle shutdown (not recommended for production). + idle_timeout: float = 3600.0 def __post_init__(self): """Normalize URL after initialization.""" @@ -33,7 +36,13 @@ def __post_init__(self): self.base_url = self._normalize_url(self.base_url) if original_url != self.base_url: logger.debug("Normalized gateway URL: %s -> %s", original_url, self.base_url) - logger.info("GatewayConfig initialized: base_url=%s, verify_ssl=%s, timeout=%s", self.base_url, self.verify_ssl, self.request_timeout) + logger.info( + "GatewayConfig initialized: base_url=%s, verify_ssl=%s, timeout=%s, idle_timeout=%s", + self.base_url, + self.verify_ssl, + self.request_timeout, + self.idle_timeout, + ) @staticmethod def _normalize_url(url: str) -> str: @@ -104,6 +113,10 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars gateway_token = gateway_token_raw gateway_validate_certs = task_args.get("gateway_validate_certs") if "gateway_validate_certs" in task_args else host_vars.get("gateway_validate_certs", True) gateway_request_timeout = task_args.get("gateway_request_timeout") or host_vars.get("gateway_request_timeout") or 10.0 + # Persistent manager idle shutdown (seconds); 0 disables idle-based exit + gateway_idle_timeout = task_args.get("gateway_idle_timeout") + if gateway_idle_timeout is None: + gateway_idle_timeout = host_vars.get("gateway_idle_timeout") or host_vars.get("ansible_platform_manager_idle_timeout") # Connection mode: "standard" (default) or "experimental" (persistent manager) connection_mode = task_args.get("platform_connection_mode") or host_vars.get("platform_connection_mode") or "standard" @@ -117,7 +130,7 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars "Gateway config extracted: url=%s, auth_method=%s, verify_ssl=%s, timeout=%s", gateway_url, auth_method, gateway_validate_certs, gateway_request_timeout ) - config = GatewayConfig( + config_kwargs = dict( base_url=gateway_url or "", username=gateway_username, password=gateway_password, @@ -126,6 +139,10 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars request_timeout=gateway_request_timeout, connection_mode=connection_mode, ) + if gateway_idle_timeout is not None: + config_kwargs["idle_timeout"] = float(gateway_idle_timeout) + + config = GatewayConfig(**config_kwargs) logger.debug("GatewayConfig created successfully") return config diff --git a/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py b/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py new file mode 100644 index 00000000..255de97c --- /dev/null +++ b/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py @@ -0,0 +1,118 @@ +# (c) 2026 Red Hat Inc. +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +"""Unit tests for PlatformService idle timeout activity tracking.""" + +from __future__ import absolute_import, division, print_function + +import unittest +from pathlib import Path +from unittest.mock import MagicMock, patch + +from ansible_collections.ansible.platform.plugins.plugin_utils.manager.platform_manager import PlatformService +from ansible_collections.ansible.platform.plugins.plugin_utils.manager.process_manager import ProcessManager +from ansible_collections.ansible.platform.plugins.plugin_utils.platform.config import GatewayConfig, extract_gateway_config + + +def _make_platform_service(): + """PlatformService with network and credentials mocked.""" + mock_response = MagicMock() + mock_response.headers = {"Content-Type": "application/json"} + mock_response.json.return_value = {} + mock_session = MagicMock() + mock_session.get.return_value = mock_response + mock_requests = MagicMock() + mock_requests.Session.return_value = mock_session + mock_store = MagicMock() + mock_store.get_auth_credentials.return_value = ("admin", "admin", None) + with patch("ansible_collections.ansible.platform.plugins.plugin_utils.manager.platform_manager.get_credential_manager") as mock_cred: + mock_cred.return_value.get_or_create_store.return_value = mock_store + with patch("ansible_collections.ansible.platform.plugins.plugin_utils.manager.platform_manager._get_requests") as mock_get_requests: + mock_get_requests.return_value = mock_requests + config = GatewayConfig(base_url="https://127.0.0.1", username="admin", password="admin", idle_timeout=30.0) + return PlatformService(config) + + +class TestGatewayConfigIdle(unittest.TestCase): + def test_gateway_config_default_idle_timeout(self): + c = GatewayConfig(base_url="https://example.com/") + self.assertEqual(c.idle_timeout, 3600.0) + + def test_extract_gateway_config_idle_timeout_from_task_args(self): + c = extract_gateway_config( + task_args={"gateway_url": "https://gw.example", "gateway_username": "a", "gateway_password": "b", "gateway_idle_timeout": 7200}, + host_vars={}, + required=True, + ) + self.assertEqual(c.idle_timeout, 7200.0) + + def test_extract_gateway_config_idle_timeout_from_host_vars(self): + c = extract_gateway_config( + task_args={"gateway_url": "https://gw.example", "gateway_username": "a", "gateway_password": "b"}, + host_vars={"ansible_platform_manager_idle_timeout": 1800}, + required=True, + ) + self.assertEqual(c.idle_timeout, 1800.0) + + +class TestPlatformServiceIdle(unittest.TestCase): + def setUp(self): + self.platform_service = _make_platform_service() + + def test_should_exit_for_idle_disabled_when_zero(self): + self.platform_service.config.idle_timeout = 0 + self.assertFalse(self.platform_service.should_exit_for_idle()) + + def test_should_exit_for_idle_false_before_threshold(self): + self.platform_service.config.idle_timeout = 1000.0 + with patch("time.monotonic", return_value=100.0): + self.platform_service.record_activity() + with patch("time.monotonic", return_value=200.0): + self.assertEqual(self.platform_service.seconds_since_last_activity(), 100.0) + self.assertFalse(self.platform_service.should_exit_for_idle()) + + def test_should_exit_for_idle_true_after_threshold(self): + self.platform_service.config.idle_timeout = 10.0 + with patch("time.monotonic", return_value=1000.0): + self.platform_service.record_activity() + with patch("time.monotonic", return_value=1020.0): + self.assertTrue(self.platform_service.should_exit_for_idle()) + + def test_should_exit_for_idle_false_after_shutdown_requested(self): + self.platform_service.config.idle_timeout = 1.0 + with patch("time.monotonic", return_value=0.0): + self.platform_service.record_activity() + self.platform_service.shutdown() + with patch("time.monotonic", return_value=99999.0): + self.assertFalse(self.platform_service.should_exit_for_idle()) + + def test_record_activity_updates_timestamp(self): + with patch("time.monotonic", side_effect=[10.0, 20.0, 25.0]): + self.platform_service.record_activity() + self.platform_service.record_activity() + self.assertEqual(self.platform_service.seconds_since_last_activity(), 5.0) + + +class TestProcessManagerIdleArgv(unittest.TestCase): + def test_spawn_manager_includes_idle_timeout_in_command(self): + cfg = GatewayConfig(base_url="https://example.com/", username="u", password="p", idle_timeout=123.0) + # tests/unit/plugins/plugin_utils/manager/ -> five parents up to platform/ + script = Path(__file__).resolve().parent.parent.parent.parent.parent / "plugins" / "plugin_utils" / "manager" / "manager_process.py" + with patch("ansible_collections.ansible.platform.plugins.plugin_utils.manager.process_manager.subprocess.Popen") as mock_popen: + mock_popen.return_value.pid = 99999 + ProcessManager.spawn_manager_process( + script_path=script, + socket_path="/tmp/x.sock", + socket_dir="/tmp", + identifier="h1", + gateway_config=cfg, + authkey_b64="YQ==", + sys_path=["/x"], + owner_pid=None, + ) + cmd = mock_popen.call_args[0][0] + self.assertEqual(cmd[-1], "123.0") + + +if __name__ == "__main__": + unittest.main() From d9fb53d98da0df08f89de6bac64daa26fe693aff Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 12:09:54 +0530 Subject: [PATCH 02/27] exception block --- plugins/plugin_utils/manager/manager_process.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/plugin_utils/manager/manager_process.py b/plugins/plugin_utils/manager/manager_process.py index ca0801da..af5fd021 100644 --- a/plugins/plugin_utils/manager/manager_process.py +++ b/plugins/plugin_utils/manager/manager_process.py @@ -383,9 +383,10 @@ def _idle_monitor(): pass try: ProcessManager.cleanup_old_socket(socket_path) - except Exception: - pass - os._exit(0) + except Exception as _e: + print(f"Idle shutdown (socket cleanup failed): {_e}", file=sys.stderr) + finally: + os._exit(0) _idle_thread = threading.Thread(target=_idle_monitor, daemon=True, name="idle-timeout") _idle_thread.start() From e8b3ba4a722106f2d4175fed307d2cd9fa8a1098 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 12:26:05 +0530 Subject: [PATCH 03/27] remove connection mode --- docs/CONNECTION_MODES.md | 39 ------------------- .../plugin_utils/manager/manager_process.py | 2 - .../plugin_utils/manager/platform_manager.py | 1 - plugins/plugin_utils/platform/config.py | 3 -- 4 files changed, 45 deletions(-) delete mode 100644 docs/CONNECTION_MODES.md diff --git a/docs/CONNECTION_MODES.md b/docs/CONNECTION_MODES.md deleted file mode 100644 index 75bff6b8..00000000 --- a/docs/CONNECTION_MODES.md +++ /dev/null @@ -1,39 +0,0 @@ -# Connection modes (standard vs persistent manager) - -The `ansible.platform.http` connection plugin can run in **direct (standard)** mode or **persistent (experimental)** mode. Persistent mode spawns a separate **manager process** that keeps an HTTP session and shared caches across tasks. - -## Idle timeout (`idle_timeout`) - -Manager processes would otherwise stay alive until the Ansible owner process exits or the socket is cleaned up. To reduce orphaned managers and memory use, the manager tracks **last activity** (RPC calls such as `execute` and `lookup_resource_id`, and HTTP requests made through the service) and shuts down automatically when nothing has run for longer than the configured **idle timeout**. - -| Behavior | Detail | -|----------|--------| -| Default | **3600** seconds (1 hour) | -| Disable | Set to **0** (manager only exits via owner PID watchdog, signals, or explicit shutdown — not recommended for production) | -| Poll interval | A background thread checks idle state every **60** seconds | -| On timeout | The manager calls `PlatformService.shutdown()`, stops the RPC server, removes the Unix socket (and `.meta` if present), and exits | - -### Configuration - -Set the timeout in seconds using either: - -- **`gateway_idle_timeout`** in task arguments or inventory/host variables, or -- **`ansible_platform_manager_idle_timeout`** in inventory/host variables (alias). - -Example (inventory): - -```yaml -gateway_idle_timeout: 1800 -``` - -Values are passed through `GatewayConfig` into the manager subprocess when the connection plugin spawns the process. - -### Test-only: poll interval - -For automated tests, the check interval can be overridden with the environment variable **`ANSIBLE_PLATFORM_IDLE_POLL_SECONDS`** (default `60`). Production deployments should rely on the default 60-second polling. - -## Related components - -- **`GatewayConfig`** (`plugins/plugin_utils/platform/config.py`) — holds `idle_timeout` and other gateway settings. -- **`PlatformService`** (`plugins/plugin_utils/manager/platform_manager.py`) — records activity and implements `should_exit_for_idle()`. -- **`manager_process.py`** — standalone entry point: starts the idle monitor thread and performs socket cleanup on exit. diff --git a/plugins/plugin_utils/manager/manager_process.py b/plugins/plugin_utils/manager/manager_process.py index af5fd021..e3a38fdb 100644 --- a/plugins/plugin_utils/manager/manager_process.py +++ b/plugins/plugin_utils/manager/manager_process.py @@ -12,7 +12,6 @@ import traceback from pathlib import Path -# Interval between idle checks (seconds). Production default: 60. MANAGER_IDLE_POLL_INTERVAL = int(os.environ.get("ANSIBLE_PLATFORM_IDLE_POLL_SECONDS", "60")) @@ -340,7 +339,6 @@ def _owner_watchdog(): _init_thread = threading.Thread(target=_init_service, daemon=True) _init_thread.start() - # Idle shutdown: after idle_timeout seconds with no RPC/API activity, exit and remove socket if float(config.idle_timeout) > 0: def _idle_monitor(): diff --git a/plugins/plugin_utils/manager/platform_manager.py b/plugins/plugin_utils/manager/platform_manager.py index ffeb3cb9..7385dc45 100644 --- a/plugins/plugin_utils/manager/platform_manager.py +++ b/plugins/plugin_utils/manager/platform_manager.py @@ -113,7 +113,6 @@ def __init__(self, config: GatewayConfig): self._shutdown_requested = False self._shutdown_lock = threading.Lock() - # Idle timeout: last time the service handled user-facing work (RPC / HTTP) self._activity_lock = threading.Lock() self._last_activity_monotonic = time.monotonic() diff --git a/plugins/plugin_utils/platform/config.py b/plugins/plugin_utils/platform/config.py index edae4f32..290bbf62 100644 --- a/plugins/plugin_utils/platform/config.py +++ b/plugins/plugin_utils/platform/config.py @@ -26,8 +26,6 @@ class GatewayConfig: verify_ssl: bool = True request_timeout: float = 10.0 connection_mode: str = "standard" # "standard" or "experimental" - #: Seconds with no API activity before the persistent manager process exits. - #: Set to 0 to disable idle shutdown (not recommended for production). idle_timeout: float = 3600.0 def __post_init__(self): @@ -113,7 +111,6 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars gateway_token = gateway_token_raw gateway_validate_certs = task_args.get("gateway_validate_certs") if "gateway_validate_certs" in task_args else host_vars.get("gateway_validate_certs", True) gateway_request_timeout = task_args.get("gateway_request_timeout") or host_vars.get("gateway_request_timeout") or 10.0 - # Persistent manager idle shutdown (seconds); 0 disables idle-based exit gateway_idle_timeout = task_args.get("gateway_idle_timeout") if gateway_idle_timeout is None: gateway_idle_timeout = host_vars.get("gateway_idle_timeout") or host_vars.get("ansible_platform_manager_idle_timeout") From 533b111e0a89b5e25608bd7a2f0f4bfcf4eb93c0 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 13:14:05 +0530 Subject: [PATCH 04/27] add molecule test --- .../molecule/idle_timeout_mock/cleanup.yml | 14 +++ .../molecule/idle_timeout_mock/converge.yml | 88 +++++++++++++++++++ .../molecule/idle_timeout_mock/molecule.yml | 34 +++++++ .../molecule/idle_timeout_mock/verify.yml | 10 +++ 4 files changed, 146 insertions(+) create mode 100644 extensions/molecule/idle_timeout_mock/cleanup.yml create mode 100644 extensions/molecule/idle_timeout_mock/converge.yml create mode 100644 extensions/molecule/idle_timeout_mock/molecule.yml create mode 100644 extensions/molecule/idle_timeout_mock/verify.yml diff --git a/extensions/molecule/idle_timeout_mock/cleanup.yml b/extensions/molecule/idle_timeout_mock/cleanup.yml new file mode 100644 index 00000000..4ba8723a --- /dev/null +++ b/extensions/molecule/idle_timeout_mock/cleanup.yml @@ -0,0 +1,14 @@ +--- +- name: Cleanup — idle timeout scenario + hosts: localhost + connection: local + gather_facts: false + tasks: + - name: Best-effort cleanup any leftover manager sockets + ansible.builtin.shell: >- + rm -f /tmp/ansible_platform/manager_*_localhost_*.sock + /tmp/ansible_platform/manager_*_localhost_*.sock.meta + || true + changed_when: false + failed_when: false +... diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml new file mode 100644 index 00000000..544098d1 --- /dev/null +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -0,0 +1,88 @@ +--- +# Converge: spawn persistent manager, then assert it exits after idle_timeout and removes its socket. + +- name: Ensure mock Gateway is reachable + hosts: localhost + connection: local + gather_facts: false + vars: + gateway_hostname: "http://127.0.0.1:8000" + tasks: + - name: Wait for mock Gateway health endpoint + ansible.builtin.uri: + url: "{{ gateway_hostname }}/health" + method: GET + status_code: 200 + register: health + retries: 12 + delay: 5 + until: health.status == 200 + vars: + ansible_connection: local + +- name: Converge — idle timeout (mock, http persistent) + hosts: localhost + connection: ansible.platform.http + gather_facts: false + vars: + gateway_hostname: "http://127.0.0.1:8000" + gateway_username: "mock" + gateway_password: "testpass" + gateway_validate_certs: false + ansible_platform_use_persistent_connection: true + gateway_idle_timeout: 3 + manager_socket_dir: "/tmp/ansible_platform" + + tasks: + - name: Trigger manager activity (state exists is non-fatal) + ansible.platform.user: + username: "idle-timeout-molecule-user" + state: exists + register: exists_result + + - name: Assert module call succeeded + ansible.builtin.assert: + that: + - exists_result is not failed + fail_msg: "Expected state:exists to succeed. result={{ exists_result }}" + + - name: Get uid + ansible.builtin.command: id -u + register: uid + changed_when: false + + - name: Find manager socket for localhost + current uid + ansible.builtin.find: + paths: "{{ manager_socket_dir }}" + patterns: + - "manager_{{ uid.stdout }}_localhost_*.sock" + file_type: file + register: sock_find + + - name: Assert a manager socket was created + ansible.builtin.assert: + that: + - sock_find.files | length > 0 + fail_msg: "Expected a manager socket in {{ manager_socket_dir }}. found={{ sock_find }}" + + - name: Record socket path + ansible.builtin.set_fact: + manager_socket_path: "{{ (sock_find.files | sort(attribute='mtime'))[-1].path }}" + manager_meta_path: "{{ (sock_find.files | sort(attribute='mtime'))[-1].path }}.meta" + + - name: Wait past idle_timeout (poll=2s in scenario molecule.yml) + ansible.builtin.pause: + seconds: 8 + + - name: Assert socket removed after idle shutdown + ansible.builtin.wait_for: + path: "{{ manager_socket_path }}" + state: absent + timeout: 30 + + - name: Assert meta removed after idle shutdown + ansible.builtin.wait_for: + path: "{{ manager_meta_path }}" + state: absent + timeout: 30 +... diff --git a/extensions/molecule/idle_timeout_mock/molecule.yml b/extensions/molecule/idle_timeout_mock/molecule.yml new file mode 100644 index 00000000..4cee0e1a --- /dev/null +++ b/extensions/molecule/idle_timeout_mock/molecule.yml @@ -0,0 +1,34 @@ +--- +# Scenario: verify persistent manager exits after idle_timeout (mock Gateway). +# Requires the mock to be running (e.g. "molecule test --all" or default scenario create). + +driver: + name: default + +platforms: + - name: localhost +ansible: + executor: + args: + ansible_playbook: + - --inventory=${MOLECULE_SCENARIO_DIRECTORY}/../inventory.yml + env: + ANSIBLE_PLATFORM_IDLE_POLL_SECONDS: "2" + +provisioner: + name: ansible + playbooks: + converge: converge.yml + verify: verify.yml + cleanup: cleanup.yml + config_options: + defaults: + collections_path: "${MOLECULE_SCENARIO_DIRECTORY}/../../../../../../" + log_verbosity: 4 + +scenario: + test_sequence: + - converge + - verify + - cleanup +... diff --git a/extensions/molecule/idle_timeout_mock/verify.yml b/extensions/molecule/idle_timeout_mock/verify.yml new file mode 100644 index 00000000..522231fd --- /dev/null +++ b/extensions/molecule/idle_timeout_mock/verify.yml @@ -0,0 +1,10 @@ +--- +- name: Verify — idle timeout scenario + hosts: localhost + connection: local + gather_facts: false + tasks: + - name: No-op verify (assert true) + ansible.builtin.assert: + that: true +... From 513f7690bb807944d6fbacd006cfac7338d3586c Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 13:17:31 +0530 Subject: [PATCH 05/27] fix molecule tests --- extensions/molecule/idle_timeout_mock/cleanup.yml | 2 ++ extensions/molecule/idle_timeout_mock/verify.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/extensions/molecule/idle_timeout_mock/cleanup.yml b/extensions/molecule/idle_timeout_mock/cleanup.yml index 4ba8723a..820c1b4a 100644 --- a/extensions/molecule/idle_timeout_mock/cleanup.yml +++ b/extensions/molecule/idle_timeout_mock/cleanup.yml @@ -3,6 +3,8 @@ hosts: localhost connection: local gather_facts: false + vars: + ansible_connection: local tasks: - name: Best-effort cleanup any leftover manager sockets ansible.builtin.shell: >- diff --git a/extensions/molecule/idle_timeout_mock/verify.yml b/extensions/molecule/idle_timeout_mock/verify.yml index 522231fd..7c81e650 100644 --- a/extensions/molecule/idle_timeout_mock/verify.yml +++ b/extensions/molecule/idle_timeout_mock/verify.yml @@ -3,6 +3,8 @@ hosts: localhost connection: local gather_facts: false + vars: + ansible_connection: local tasks: - name: No-op verify (assert true) ansible.builtin.assert: From f259e80b6746313cc9a4941688a99e1f675628f9 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 13:22:12 +0530 Subject: [PATCH 06/27] fix molecule --- extensions/molecule/idle_timeout_mock/molecule.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/extensions/molecule/idle_timeout_mock/molecule.yml b/extensions/molecule/idle_timeout_mock/molecule.yml index 4cee0e1a..89717f85 100644 --- a/extensions/molecule/idle_timeout_mock/molecule.yml +++ b/extensions/molecule/idle_timeout_mock/molecule.yml @@ -8,10 +8,6 @@ driver: platforms: - name: localhost ansible: - executor: - args: - ansible_playbook: - - --inventory=${MOLECULE_SCENARIO_DIRECTORY}/../inventory.yml env: ANSIBLE_PLATFORM_IDLE_POLL_SECONDS: "2" From 791b6aa79b5d9691385e49d8e589cbe15f9712c3 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 13:26:32 +0530 Subject: [PATCH 07/27] fix molecule test --- extensions/molecule/idle_timeout_mock/molecule.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/extensions/molecule/idle_timeout_mock/molecule.yml b/extensions/molecule/idle_timeout_mock/molecule.yml index 89717f85..dc35ba5d 100644 --- a/extensions/molecule/idle_timeout_mock/molecule.yml +++ b/extensions/molecule/idle_timeout_mock/molecule.yml @@ -8,6 +8,9 @@ driver: platforms: - name: localhost ansible: + executor: + args: + ansible_playbook: [] env: ANSIBLE_PLATFORM_IDLE_POLL_SECONDS: "2" From 18b5ebba0cec0540fc4b327c7281c597a22f5e33 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 13:38:47 +0530 Subject: [PATCH 08/27] fix molecule test --- .github/workflows/molecule-mock.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/molecule-mock.yml b/.github/workflows/molecule-mock.yml index 40840078..be320654 100644 --- a/.github/workflows/molecule-mock.yml +++ b/.github/workflows/molecule-mock.yml @@ -76,7 +76,7 @@ jobs: curl -sf http://127.0.0.1:8000/health - name: Run ${{ matrix.scenario }} - run: molecule test -s ${{ matrix.scenario }} --all + run: molecule test -s ${{ matrix.scenario }} - name: Stop mock Gateway if: always() From 008ff69d35f8ce1cfdb2f0c588572dbea5c1b942 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 14:01:40 +0530 Subject: [PATCH 09/27] fix molecule --- extensions/molecule/idle_timeout_mock/converge.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml index 544098d1..b1ec33dd 100644 --- a/extensions/molecule/idle_timeout_mock/converge.yml +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -34,17 +34,18 @@ manager_socket_dir: "/tmp/ansible_platform" tasks: - - name: Trigger manager activity (state exists is non-fatal) + - name: Trigger manager activity (create a user) ansible.platform.user: username: "idle-timeout-molecule-user" - state: exists - register: exists_result + password: "TestPassword123!" + state: present + register: user_result - name: Assert module call succeeded ansible.builtin.assert: that: - - exists_result is not failed - fail_msg: "Expected state:exists to succeed. result={{ exists_result }}" + - user_result is not failed + fail_msg: "Manager failed to process the request against the mock server. result={{ user_result }}" - name: Get uid ansible.builtin.command: id -u From e54d66bed8f1593d8b1897646c25b38e6e4ac1eb Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 14:06:03 +0530 Subject: [PATCH 10/27] fix molecule --- .../molecule/idle_timeout_mock/converge.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml index b1ec33dd..5b9f977e 100644 --- a/extensions/molecule/idle_timeout_mock/converge.yml +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -37,7 +37,7 @@ - name: Trigger manager activity (create a user) ansible.platform.user: username: "idle-timeout-molecule-user" - password: "TestPassword123!" + password: "TestPassword123!" state: present register: user_result @@ -51,6 +51,8 @@ ansible.builtin.command: id -u register: uid changed_when: false + vars: + ansible_connection: local - name: Find manager socket for localhost + current uid ansible.builtin.find: @@ -59,31 +61,43 @@ - "manager_{{ uid.stdout }}_localhost_*.sock" file_type: file register: sock_find + vars: + ansible_connection: local - name: Assert a manager socket was created ansible.builtin.assert: that: - sock_find.files | length > 0 fail_msg: "Expected a manager socket in {{ manager_socket_dir }}. found={{ sock_find }}" + vars: + ansible_connection: local - name: Record socket path ansible.builtin.set_fact: manager_socket_path: "{{ (sock_find.files | sort(attribute='mtime'))[-1].path }}" manager_meta_path: "{{ (sock_find.files | sort(attribute='mtime'))[-1].path }}.meta" + vars: + ansible_connection: local - name: Wait past idle_timeout (poll=2s in scenario molecule.yml) ansible.builtin.pause: seconds: 8 + vars: + ansible_connection: local - name: Assert socket removed after idle shutdown ansible.builtin.wait_for: path: "{{ manager_socket_path }}" state: absent timeout: 30 + vars: + ansible_connection: local - name: Assert meta removed after idle shutdown ansible.builtin.wait_for: path: "{{ manager_meta_path }}" state: absent timeout: 30 + vars: + ansible_connection: local ... From 57026390d5cc96a3c226e8a34882ddf36c1be050 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 14:08:38 +0530 Subject: [PATCH 11/27] fix molecule --- extensions/molecule/idle_timeout_mock/cleanup.yml | 6 +++++- extensions/molecule/idle_timeout_mock/converge.yml | 14 +++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/extensions/molecule/idle_timeout_mock/cleanup.yml b/extensions/molecule/idle_timeout_mock/cleanup.yml index 820c1b4a..8ef27c2d 100644 --- a/extensions/molecule/idle_timeout_mock/cleanup.yml +++ b/extensions/molecule/idle_timeout_mock/cleanup.yml @@ -8,8 +8,12 @@ tasks: - name: Best-effort cleanup any leftover manager sockets ansible.builtin.shell: >- - rm -f /tmp/ansible_platform/manager_*_localhost_*.sock + TMPDIR="$(python3 -c 'import tempfile; print(tempfile.gettempdir())')" + rm -f + /tmp/ansible_platform/manager_*_localhost_*.sock /tmp/ansible_platform/manager_*_localhost_*.sock.meta + "${TMPDIR}/ansible_platform/manager_*_localhost_*.sock" + "${TMPDIR}/ansible_platform/manager_*_localhost_*.sock.meta" || true changed_when: false failed_when: false diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml index 5b9f977e..5acf11ca 100644 --- a/extensions/molecule/idle_timeout_mock/converge.yml +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -31,7 +31,6 @@ gateway_validate_certs: false ansible_platform_use_persistent_connection: true gateway_idle_timeout: 3 - manager_socket_dir: "/tmp/ansible_platform" tasks: - name: Trigger manager activity (create a user) @@ -47,6 +46,19 @@ - user_result is not failed fail_msg: "Manager failed to process the request against the mock server. result={{ user_result }}" + - name: Determine controller tempdir (for manager socket dir) + ansible.builtin.command: python3 -c "import tempfile; print(tempfile.gettempdir())" + register: tmpdir + changed_when: false + vars: + ansible_connection: local + + - name: Set manager socket directory (ProcessManager default) + ansible.builtin.set_fact: + manager_socket_dir: "{{ tmpdir.stdout | trim }}/ansible_platform" + vars: + ansible_connection: local + - name: Get uid ansible.builtin.command: id -u register: uid From c3a3abe633896c0d0f086d8920cc9af06481b39c Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 14:11:37 +0530 Subject: [PATCH 12/27] fix tc --- .../molecule/idle_timeout_mock/converge.yml | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml index 5acf11ca..098b1fe0 100644 --- a/extensions/molecule/idle_timeout_mock/converge.yml +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -59,6 +59,12 @@ vars: ansible_connection: local + - name: Set ephemeral socket directory (direct/ephemeral managers) + ansible.builtin.set_fact: + ephemeral_socket_dir: "{{ tmpdir.stdout | trim }}/ap" + vars: + ansible_connection: local + - name: Get uid ansible.builtin.command: id -u register: uid @@ -66,28 +72,43 @@ vars: ansible_connection: local - - name: Find manager socket for localhost + current uid + - name: Find persistent manager socket for localhost + current uid ansible.builtin.find: paths: "{{ manager_socket_dir }}" patterns: - "manager_{{ uid.stdout }}_localhost_*.sock" file_type: file - register: sock_find + register: sock_find_persistent + vars: + ansible_connection: local + + - name: Find ephemeral manager socket for localhost + current uid + ansible.builtin.find: + paths: "{{ ephemeral_socket_dir }}" + patterns: + - "manager_{{ uid.stdout }}_e*_*.sock" + - "manager_{{ uid.stdout }}_e*.sock" + file_type: file + register: sock_find_ephemeral vars: ansible_connection: local - - name: Assert a manager socket was created + - name: Assert a persistent manager socket was created ansible.builtin.assert: that: - - sock_find.files | length > 0 - fail_msg: "Expected a manager socket in {{ manager_socket_dir }}. found={{ sock_find }}" + - sock_find_persistent.files | length > 0 + fail_msg: >- + Expected a *persistent* manager socket in {{ manager_socket_dir }} but found none. + If sockets exist under {{ ephemeral_socket_dir }}, the connection likely ran in direct mode + instead of persistent. persistent_found={{ sock_find_persistent }} + ephemeral_found={{ sock_find_ephemeral }} vars: ansible_connection: local - name: Record socket path ansible.builtin.set_fact: - manager_socket_path: "{{ (sock_find.files | sort(attribute='mtime'))[-1].path }}" - manager_meta_path: "{{ (sock_find.files | sort(attribute='mtime'))[-1].path }}.meta" + manager_socket_path: "{{ (sock_find_persistent.files | sort(attribute='mtime'))[-1].path }}" + manager_meta_path: "{{ (sock_find_persistent.files | sort(attribute='mtime'))[-1].path }}.meta" vars: ansible_connection: local From f9888190bf7ce5a7c1eb42d7fde85ee83b572209 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 14:14:00 +0530 Subject: [PATCH 13/27] increase timeout --- extensions/molecule/idle_timeout_mock/converge.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml index 098b1fe0..4e277d98 100644 --- a/extensions/molecule/idle_timeout_mock/converge.yml +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -30,7 +30,9 @@ gateway_password: "testpass" gateway_validate_certs: false ansible_platform_use_persistent_connection: true - gateway_idle_timeout: 3 + # Keep this long enough that local verification tasks don't race the idle monitor + # (poll interval is forced to 2s in molecule.yml via ANSIBLE_PLATFORM_IDLE_POLL_SECONDS). + gateway_idle_timeout: 15 tasks: - name: Trigger manager activity (create a user) @@ -114,7 +116,7 @@ - name: Wait past idle_timeout (poll=2s in scenario molecule.yml) ansible.builtin.pause: - seconds: 8 + seconds: 20 vars: ansible_connection: local From da4955924fe6807e1b55eea7c87686b99893a7de Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 14:16:36 +0530 Subject: [PATCH 14/27] fix tc --- .../molecule/idle_timeout_mock/cleanup.yml | 3 ++ .../molecule/idle_timeout_mock/converge.yml | 50 ++++++------------- 2 files changed, 18 insertions(+), 35 deletions(-) diff --git a/extensions/molecule/idle_timeout_mock/cleanup.yml b/extensions/molecule/idle_timeout_mock/cleanup.yml index 8ef27c2d..c85353f5 100644 --- a/extensions/molecule/idle_timeout_mock/cleanup.yml +++ b/extensions/molecule/idle_timeout_mock/cleanup.yml @@ -9,11 +9,14 @@ - name: Best-effort cleanup any leftover manager sockets ansible.builtin.shell: >- TMPDIR="$(python3 -c 'import tempfile; print(tempfile.gettempdir())')" + HOME_DIR="${HOME:-$(getent passwd "$(id -u)" | cut -d: -f6)}" rm -f /tmp/ansible_platform/manager_*_localhost_*.sock /tmp/ansible_platform/manager_*_localhost_*.sock.meta "${TMPDIR}/ansible_platform/manager_*_localhost_*.sock" "${TMPDIR}/ansible_platform/manager_*_localhost_*.sock.meta" + "${HOME_DIR}/.ansible/tmp/"*/ansible_platform/manager_*_localhost_*.sock + "${HOME_DIR}/.ansible/tmp/"*/ansible_platform/manager_*_localhost_*.sock.meta || true changed_when: false failed_when: false diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml index 4e277d98..cb338cb4 100644 --- a/extensions/molecule/idle_timeout_mock/converge.yml +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -48,25 +48,6 @@ - user_result is not failed fail_msg: "Manager failed to process the request against the mock server. result={{ user_result }}" - - name: Determine controller tempdir (for manager socket dir) - ansible.builtin.command: python3 -c "import tempfile; print(tempfile.gettempdir())" - register: tmpdir - changed_when: false - vars: - ansible_connection: local - - - name: Set manager socket directory (ProcessManager default) - ansible.builtin.set_fact: - manager_socket_dir: "{{ tmpdir.stdout | trim }}/ansible_platform" - vars: - ansible_connection: local - - - name: Set ephemeral socket directory (direct/ephemeral managers) - ansible.builtin.set_fact: - ephemeral_socket_dir: "{{ tmpdir.stdout | trim }}/ap" - vars: - ansible_connection: local - - name: Get uid ansible.builtin.command: id -u register: uid @@ -74,24 +55,23 @@ vars: ansible_connection: local - - name: Find persistent manager socket for localhost + current uid - ansible.builtin.find: - paths: "{{ manager_socket_dir }}" - patterns: - - "manager_{{ uid.stdout }}_localhost_*.sock" - file_type: file - register: sock_find_persistent + - name: Determine HOME (for Molecule tmp paths) + ansible.builtin.command: sh -lc 'printf "%s" "$HOME"' + register: home_dir + changed_when: false vars: ansible_connection: local - - name: Find ephemeral manager socket for localhost + current uid + - name: Find persistent manager socket for localhost + current uid (common locations) ansible.builtin.find: - paths: "{{ ephemeral_socket_dir }}" + paths: + - "/tmp/ansible_platform" + - "{{ home_dir.stdout }}/.ansible/tmp" patterns: - - "manager_{{ uid.stdout }}_e*_*.sock" - - "manager_{{ uid.stdout }}_e*.sock" + - "manager_{{ uid.stdout }}_localhost_*.sock" file_type: file - register: sock_find_ephemeral + recurse: true + register: sock_find_persistent vars: ansible_connection: local @@ -100,10 +80,9 @@ that: - sock_find_persistent.files | length > 0 fail_msg: >- - Expected a *persistent* manager socket in {{ manager_socket_dir }} but found none. - If sockets exist under {{ ephemeral_socket_dir }}, the connection likely ran in direct mode - instead of persistent. persistent_found={{ sock_find_persistent }} - ephemeral_found={{ sock_find_ephemeral }} + Expected a *persistent* manager socket but found none. + searched_paths=['/tmp/ansible_platform','{{ home_dir.stdout }}/.ansible/tmp'] + persistent_found={{ sock_find_persistent }} vars: ansible_connection: local @@ -111,6 +90,7 @@ ansible.builtin.set_fact: manager_socket_path: "{{ (sock_find_persistent.files | sort(attribute='mtime'))[-1].path }}" manager_meta_path: "{{ (sock_find_persistent.files | sort(attribute='mtime'))[-1].path }}.meta" + manager_socket_dir: "{{ (sock_find_persistent.files | sort(attribute='mtime'))[-1].path | dirname }}" vars: ansible_connection: local From 47a57c3b4204cac4c35a246769a4d900d4d29e22 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 14:20:04 +0530 Subject: [PATCH 15/27] fix tc --- extensions/molecule/idle_timeout_mock/converge.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml index cb338cb4..acf49de0 100644 --- a/extensions/molecule/idle_timeout_mock/converge.yml +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -69,7 +69,7 @@ - "{{ home_dir.stdout }}/.ansible/tmp" patterns: - "manager_{{ uid.stdout }}_localhost_*.sock" - file_type: file + file_type: any recurse: true register: sock_find_persistent vars: From a10e94d3513e9e40f03d8a9b5826689281df7358 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 14:23:27 +0530 Subject: [PATCH 16/27] fix lint --- extensions/molecule/idle_timeout_mock/cleanup.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/molecule/idle_timeout_mock/cleanup.yml b/extensions/molecule/idle_timeout_mock/cleanup.yml index c85353f5..3d5cc005 100644 --- a/extensions/molecule/idle_timeout_mock/cleanup.yml +++ b/extensions/molecule/idle_timeout_mock/cleanup.yml @@ -6,10 +6,10 @@ vars: ansible_connection: local tasks: - - name: Best-effort cleanup any leftover manager sockets + - name: Cleanup any leftover manager sockets ansible.builtin.shell: >- TMPDIR="$(python3 -c 'import tempfile; print(tempfile.gettempdir())')" - HOME_DIR="${HOME:-$(getent passwd "$(id -u)" | cut -d: -f6)}" + HOME_DIR="$(python3 -c 'import os,pwd; print(os.environ.get(\"HOME\") or pwd.getpwuid(os.getuid()).pw_dir)')" rm -f /tmp/ansible_platform/manager_*_localhost_*.sock /tmp/ansible_platform/manager_*_localhost_*.sock.meta From 1c3d87ac9f4d174298cdebc4758e6339a33b17b2 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Tue, 31 Mar 2026 17:29:51 +0530 Subject: [PATCH 17/27] fix lint --- extensions/molecule/idle_timeout_mock/converge.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml index acf49de0..1759df37 100644 --- a/extensions/molecule/idle_timeout_mock/converge.yml +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -30,8 +30,6 @@ gateway_password: "testpass" gateway_validate_certs: false ansible_platform_use_persistent_connection: true - # Keep this long enough that local verification tasks don't race the idle monitor - # (poll interval is forced to 2s in molecule.yml via ANSIBLE_PLATFORM_IDLE_POLL_SECONDS). gateway_idle_timeout: 15 tasks: From 829cd01f9c123550a326bed235d2de3a5520ee9f Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Thu, 2 Apr 2026 14:38:23 +0530 Subject: [PATCH 18/27] address review comments --- .../plugin_utils/manager/manager_process.py | 50 +++- plugins/plugin_utils/platform/config.py | 21 +- .../manager/test_manager_process_redaction.py | 240 ++++++++++++++++++ .../manager/test_platform_service_idle.py | 147 +++++++++++ 4 files changed, 445 insertions(+), 13 deletions(-) create mode 100644 tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py diff --git a/plugins/plugin_utils/manager/manager_process.py b/plugins/plugin_utils/manager/manager_process.py index e3a38fdb..85ed2106 100644 --- a/plugins/plugin_utils/manager/manager_process.py +++ b/plugins/plugin_utils/manager/manager_process.py @@ -12,7 +12,45 @@ import traceback from pathlib import Path -MANAGER_IDLE_POLL_INTERVAL = int(os.environ.get("ANSIBLE_PLATFORM_IDLE_POLL_SECONDS", "60")) +def _compute_poll_interval(idle_timeout: float) -> int: + """Return the idle-monitor sleep interval derived from the configured timeout. + + The interval is set to 10 % of ``idle_timeout`` so the manager checks + roughly 10 times per timeout window, giving a worst-case overshoot of + one poll interval (10 % of the timeout) instead of a fixed 60 s. + + Bounds: + - Floor: 5 s — avoids busy-looping for very short timeouts (e.g. tests). + - Cap: 60 s — avoids infrequent checks for very long timeouts. + - ``idle_timeout <= 0`` (disabled): returns 60 s (interval is irrelevant). + + The ``ANSIBLE_PLATFORM_IDLE_POLL_SECONDS`` environment variable overrides + this calculation entirely and is intended only for test environments where + a sub-second or very short poll period is needed. + """ + env_override = os.environ.get("ANSIBLE_PLATFORM_IDLE_POLL_SECONDS") + if env_override is not None: + return int(env_override) + if idle_timeout <= 0: + return 60 + return max(5, min(60, int(idle_timeout / 10))) + +_SENSITIVE_ARGV_POSITIONS = {5, 6, 7} + + +def _redact_argv(argv=None): + """Return a copy of argv with credential positions replaced by ''. + + Always safe to log — sensitive positions (username, password, token) are + replaced even when the value is an empty string, so length cannot be inferred. + """ + if argv is None: + argv = sys.argv + redacted = list(argv) + for i in _SENSITIVE_ARGV_POSITIONS: + if i < len(redacted) and redacted[i]: + redacted[i] = "" + return redacted def main(): @@ -31,13 +69,13 @@ def _safe_argv(): marker = Path("/tmp/ansible_platform_manager_started.txt") with open(marker, "a") as f: f.write(f"Script started with {len(sys.argv)} args\n") - f.write(f"Args: {_safe_argv()}\n") + f.write(f"Args: {_redact_argv()}\n") except Exception: pass if len(sys.argv) < 10: print(f"ERROR: Expected at least 9 args (optional 10th: idle_timeout), got {len(sys.argv) - 1}", file=sys.stderr) - print(f"Args received: {sys.argv}", file=sys.stderr) + print(f"Args received: {_redact_argv()}", file=sys.stderr) sys.exit(1) marker = Path("/tmp/ansible_platform_manager_started.txt") @@ -339,6 +377,8 @@ def _owner_watchdog(): _init_thread = threading.Thread(target=_init_service, daemon=True) _init_thread.start() + idle_poll_interval = _compute_poll_interval(config.idle_timeout) + if float(config.idle_timeout) > 0: def _idle_monitor(): @@ -347,7 +387,7 @@ def _idle_monitor(): from ansible_collections.ansible.platform.plugins.plugin_utils.manager.process_manager import ProcessManager while True: - _time.sleep(MANAGER_IDLE_POLL_INTERVAL) + _time.sleep(idle_poll_interval) if not _service_ready.is_set(): continue svc = _service_container.get("service") @@ -389,7 +429,7 @@ def _idle_monitor(): _idle_thread = threading.Thread(target=_idle_monitor, daemon=True, name="idle-timeout") _idle_thread.start() with open(error_log, "a") as f: - f.write(f"Idle timeout monitor started (interval={MANAGER_IDLE_POLL_INTERVAL}s, idle_timeout={config.idle_timeout}s)\n") + f.write(f"Idle timeout monitor started (interval={idle_poll_interval}s, idle_timeout={config.idle_timeout}s)\n") f.flush() try: diff --git a/plugins/plugin_utils/platform/config.py b/plugins/plugin_utils/platform/config.py index 290bbf62..0487073e 100644 --- a/plugins/plugin_utils/platform/config.py +++ b/plugins/plugin_utils/platform/config.py @@ -111,9 +111,17 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars gateway_token = gateway_token_raw gateway_validate_certs = task_args.get("gateway_validate_certs") if "gateway_validate_certs" in task_args else host_vars.get("gateway_validate_certs", True) gateway_request_timeout = task_args.get("gateway_request_timeout") or host_vars.get("gateway_request_timeout") or 10.0 - gateway_idle_timeout = task_args.get("gateway_idle_timeout") - if gateway_idle_timeout is None: - gateway_idle_timeout = host_vars.get("gateway_idle_timeout") or host_vars.get("ansible_platform_manager_idle_timeout") + # How long (seconds) the persistent manager process may sit idle — i.e. receive + # no RPC or API traffic — before it shuts itself down and removes its socket. + # This prevents orphaned manager processes from accumulating across playbook runs. + # Default: 3600 s (1 hour). Set to 0 to disable idle-based shutdown entirely. + # Accepted variable names (task arg takes priority over host var): + # gateway_idle_timeout / ansible_platform_manager_idle_timeout + gateway_idle_timeout = ( + task_args.get("gateway_idle_timeout") + or host_vars.get("gateway_idle_timeout") + or host_vars.get("ansible_platform_manager_idle_timeout") + ) # Connection mode: "standard" (default) or "experimental" (persistent manager) connection_mode = task_args.get("platform_connection_mode") or host_vars.get("platform_connection_mode") or "standard" @@ -127,7 +135,7 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars "Gateway config extracted: url=%s, auth_method=%s, verify_ssl=%s, timeout=%s", gateway_url, auth_method, gateway_validate_certs, gateway_request_timeout ) - config_kwargs = dict( + config = GatewayConfig( base_url=gateway_url or "", username=gateway_username, password=gateway_password, @@ -135,11 +143,8 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars verify_ssl=gateway_validate_certs, request_timeout=gateway_request_timeout, connection_mode=connection_mode, + idle_timeout=float(gateway_idle_timeout) if gateway_idle_timeout is not None else 3600.0, ) - if gateway_idle_timeout is not None: - config_kwargs["idle_timeout"] = float(gateway_idle_timeout) - - config = GatewayConfig(**config_kwargs) logger.debug("GatewayConfig created successfully") return config diff --git a/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py new file mode 100644 index 00000000..e414f71a --- /dev/null +++ b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py @@ -0,0 +1,240 @@ +# (c) 2026 Red Hat Inc. +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +"""Unit tests for manager_process._redact_argv — credential redaction. + +Sensitive argv positions (username=5, password=6, token=7) must never appear +in any log file regardless of which manager invocation code path is triggered. +""" + +from __future__ import absolute_import, division, print_function + +import io +import os +import sys +import unittest +from pathlib import Path +from unittest.mock import patch + +# --------------------------------------------------------------------------- +# Import the module under test. manager_process.py lives in plugin_utils so +# we need the collections parent on sys.path (conftest.py handles this when +# running via pytest; unittest needs it done here too). +# --------------------------------------------------------------------------- +_COLLECTIONS_PARENT = str(Path(__file__).resolve().parent.parent.parent.parent.parent.parent.parent.parent) +if _COLLECTIONS_PARENT not in sys.path: + sys.path.insert(0, _COLLECTIONS_PARENT) + +from ansible_collections.ansible.platform.plugins.plugin_utils.manager.manager_process import ( # noqa: E402 + _SENSITIVE_ARGV_POSITIONS, + _compute_poll_interval, + _redact_argv, +) + + +def _make_argv(username="admin", password="s3cr3t!", token="tok123"): + """Return a representative argv list matching manager_process argument layout.""" + return [ + "/path/to/manager_process.py", # 0: script + "/tmp/ap/manager.sock", # 1: socket_path + "/tmp/ap", # 2: socket_dir + "localhost", # 3: inventory_hostname + "https://gateway.example/", # 4: gateway_url + username, # 5: gateway_username ← sensitive + password, # 6: gateway_password ← sensitive + token, # 7: gateway_token ← sensitive + "true", # 8: validate_certs + "10.0", # 9: request_timeout + "3600.0", # 10: idle_timeout + ] + + +class TestComputePollInterval(unittest.TestCase): + """_compute_poll_interval derives the idle-monitor sleep from idle_timeout.""" + + def _call(self, idle_timeout): + """Call without the env-var override in effect.""" + with patch.dict(os.environ, {}, clear=False): + os.environ.pop("ANSIBLE_PLATFORM_IDLE_POLL_SECONDS", None) + return _compute_poll_interval(idle_timeout) + + # ------------------------------------------------------------------ + # Core formula: 10 % of idle_timeout, clamped to [5, 60] + # ------------------------------------------------------------------ + + def test_typical_default_timeout_gives_60s(self): + """3600 s timeout → 10 % = 360 s, capped at 60 s.""" + self.assertEqual(self._call(3600.0), 60) + + def test_300s_timeout_gives_30s(self): + """300 s timeout → 10 % = 30 s (within bounds).""" + self.assertEqual(self._call(300.0), 30) + + def test_short_timeout_is_floored_at_5s(self): + """20 s timeout → 10 % = 2 s, floored to 5 s.""" + self.assertEqual(self._call(20.0), 5) + + def test_exact_floor_boundary(self): + """50 s timeout → 10 % = 5 s, exactly at the floor.""" + self.assertEqual(self._call(50.0), 5) + + def test_exact_cap_boundary(self): + """600 s timeout → 10 % = 60 s, exactly at the cap.""" + self.assertEqual(self._call(600.0), 60) + + def test_large_timeout_capped_at_60s(self): + """Any timeout > 600 s caps at 60 s.""" + self.assertEqual(self._call(86400.0), 60) + + # ------------------------------------------------------------------ + # Disabled timeout (idle_timeout <= 0) + # ------------------------------------------------------------------ + + def test_zero_idle_timeout_returns_60(self): + """idle_timeout=0 (disabled) → interval is irrelevant, returns 60 s.""" + self.assertEqual(self._call(0), 60) + + def test_negative_idle_timeout_returns_60(self): + """Negative idle_timeout (also treated as disabled) → 60 s.""" + self.assertEqual(self._call(-1.0), 60) + + # ------------------------------------------------------------------ + # Env-var override (test harness) + # ------------------------------------------------------------------ + + def test_env_var_override_takes_precedence(self): + """ANSIBLE_PLATFORM_IDLE_POLL_SECONDS bypasses the formula entirely.""" + with patch.dict(os.environ, {"ANSIBLE_PLATFORM_IDLE_POLL_SECONDS": "2"}): + self.assertEqual(_compute_poll_interval(3600.0), 2) + + def test_env_var_override_works_for_short_timeout_too(self): + """Env-var overrides even when timeout is small (test speed-up).""" + with patch.dict(os.environ, {"ANSIBLE_PLATFORM_IDLE_POLL_SECONDS": "1"}): + self.assertEqual(_compute_poll_interval(5.0), 1) + + def test_no_env_var_uses_formula(self): + """Without the env var, the formula applies normally.""" + env = {k: v for k, v in os.environ.items() if k != "ANSIBLE_PLATFORM_IDLE_POLL_SECONDS"} + with patch.dict(os.environ, env, clear=True): + self.assertEqual(_compute_poll_interval(300.0), 30) + + +class TestSensitiveArgvPositions(unittest.TestCase): + def test_sensitive_positions_cover_username_password_token(self): + self.assertIn(5, _SENSITIVE_ARGV_POSITIONS) + self.assertIn(6, _SENSITIVE_ARGV_POSITIONS) + self.assertIn(7, _SENSITIVE_ARGV_POSITIONS) + + def test_non_sensitive_positions_not_in_set(self): + for pos in (0, 1, 2, 3, 4, 8, 9, 10): + self.assertNotIn(pos, _SENSITIVE_ARGV_POSITIONS) + + +class TestRedactArgv(unittest.TestCase): + def test_credentials_replaced_with_redacted(self): + argv = _make_argv(username="admin", password="s3cr3t!", token="tok123") + result = _redact_argv(argv) + self.assertEqual(result[5], "") + self.assertEqual(result[6], "") + self.assertEqual(result[7], "") + + def test_non_sensitive_positions_unchanged(self): + argv = _make_argv() + result = _redact_argv(argv) + self.assertEqual(result[0], argv[0]) + self.assertEqual(result[1], argv[1]) + self.assertEqual(result[2], argv[2]) + self.assertEqual(result[3], argv[3]) + self.assertEqual(result[4], argv[4]) + self.assertEqual(result[8], argv[8]) + self.assertEqual(result[9], argv[9]) + self.assertEqual(result[10], argv[10]) + + def test_plaintext_credentials_absent_from_result(self): + argv = _make_argv(username="admin", password="s3cr3t!", token="tok123") + result = _redact_argv(argv) + result_str = str(result) + self.assertNotIn("s3cr3t!", result_str) + self.assertNotIn("tok123", result_str) + + def test_original_argv_not_mutated(self): + argv = _make_argv(password="original") + original_copy = list(argv) + _redact_argv(argv) + self.assertEqual(argv, original_copy) + + def test_empty_credential_fields_not_leaked(self): + """Empty credentials are still replaced — length cannot be inferred.""" + argv = _make_argv(username="", password="", token="") + result = _redact_argv(argv) + self.assertEqual(result[5], "") # empty stays empty (nothing to reveal) + self.assertEqual(result[6], "") + self.assertEqual(result[7], "") + + def test_short_argv_does_not_raise(self): + """If argv is shorter than expected (early error path), no IndexError.""" + argv = ["manager_process.py", "/tmp/s.sock"] + result = _redact_argv(argv) + self.assertEqual(result, argv) + + def test_uses_sys_argv_when_no_argument_given(self): + """Called with no argument, _redact_argv() reads from sys.argv.""" + fake_argv = _make_argv(password="should_not_appear") + with patch.object(sys, "argv", fake_argv): + result = _redact_argv() + self.assertEqual(result[6], "") + self.assertNotIn("should_not_appear", str(result)) + + def test_partial_argv_missing_token_position(self): + """argv with only 7 entries: password redacted, token position absent.""" + argv = _make_argv()[:7] # indices 0-6, position 7 missing + result = _redact_argv(argv) + self.assertEqual(result[5], "") + self.assertEqual(result[6], "") + self.assertEqual(len(result), 7) + + +class TestRedactArgvInStartupLog(unittest.TestCase): + """Verify that the startup marker and error-path writes use redacted argv.""" + + def test_startup_marker_does_not_contain_password(self): + """The /tmp marker file must never contain plaintext credentials.""" + import tempfile + + fake_marker = Path(tempfile.mktemp(suffix="_test_marker.txt")) + fake_argv = _make_argv(password="plaintext_password", token="plaintext_token") + + try: + with patch.object(sys, "argv", fake_argv): + with patch( + "ansible_collections.ansible.platform.plugins.plugin_utils.manager.manager_process.Path", + side_effect=lambda p: fake_marker if "ansible_platform_manager_started" in str(p) else Path(p), + ): + with fake_marker.open("w") as _f: + _f.write(f"Script started with {len(sys.argv)} args\n") + _f.write(f"Args: {_redact_argv()}\n") + + content = fake_marker.read_text() + self.assertNotIn("plaintext_password", content) + self.assertNotIn("plaintext_token", content) + self.assertIn("", content) + finally: + try: + fake_marker.unlink() + except FileNotFoundError: + pass + + def test_stderr_error_path_does_not_contain_password(self): + """The early-exit print (too few args) must use _redact_argv().""" + fake_argv = _make_argv(password="plaintext_password")[:5] # too short → error path + captured = io.StringIO() + + # Simulate the error-path print + print(f"Args received: {_redact_argv(fake_argv)}", file=captured) + output = captured.getvalue() + + self.assertNotIn("plaintext_password", output) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py b/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py index 255de97c..2ef4657b 100644 --- a/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py +++ b/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py @@ -93,6 +93,153 @@ def test_record_activity_updates_timestamp(self): self.assertEqual(self.platform_service.seconds_since_last_activity(), 5.0) +class TestPlatformServiceIdleTokenExpiry(unittest.TestCase): + """Idle timeout behaviour when OAuth tokens expire. + + Key design invariant under test: + - should_exit_for_idle() is PURELY time-based — token state is irrelevant. + - record_activity() is called at the TOP of _make_request(), BEFORE the HTTP + call, so even a request that returns 401 (expired token) resets the idle clock. + - Internal token refresh / re-authentication does NOT call record_activity(), + so background auth work never keeps the manager alive artificially. + """ + + def setUp(self): + self.svc = _make_platform_service() + + # ------------------------------------------------------------------ + # 1. Expired token alone does not suppress idle exit + # ------------------------------------------------------------------ + + def test_expired_token_alone_does_not_suppress_idle_exit(self): + """If the token expires passively (no incoming request), idle timeout still fires.""" + self.svc.config.idle_timeout = 10.0 + with patch("time.monotonic", return_value=1000.0): + self.svc.record_activity() + + # Simulate the credential store reporting an expired token + with patch.object(self.svc, "_check_token_expiration", return_value=(True, -60.0)): + with patch("time.monotonic", return_value=1020.0): + self.assertTrue(self.svc.should_exit_for_idle()) + + def test_expired_token_does_not_prevent_idle_exit_when_no_traffic(self): + """No requests → no record_activity() → idle fires regardless of token state.""" + self.svc.config.idle_timeout = 5.0 + with patch("time.monotonic", return_value=500.0): + self.svc.record_activity() + + # Simulate oauth_token being wiped (e.g. after expiry) but no new request + self.svc.oauth_token = None + + with patch("time.monotonic", return_value=510.0): + self.assertTrue(self.svc.should_exit_for_idle()) + + # ------------------------------------------------------------------ + # 2. A request that hits a 401 still resets the idle timer + # ------------------------------------------------------------------ + + def test_401_response_still_resets_idle_timer(self): + """record_activity() fires before the HTTP call, so 401s reset the idle clock.""" + self.svc.config.idle_timeout = 10.0 + + # Anchor "last activity" far in the past + with patch("time.monotonic", return_value=0.0): + self.svc.record_activity() + + # Mock a 401 followed by a successful retry after re-auth + mock_401 = MagicMock() + mock_401.status_code = 401 + mock_401.text = "Unauthorized" + + mock_200 = MagicMock() + mock_200.status_code = 200 + mock_200.text = "" + + self.svc.session.get = MagicMock(side_effect=[mock_401, mock_200]) + + with patch.object(self.svc, "_handle_auth_error", return_value=True): + with patch("time.monotonic", return_value=999.0): + try: + self.svc._make_request("get", "https://gw/api/gateway/v1/users/") + except Exception: + pass + # record_activity() was called at t=999 inside _make_request + self.assertAlmostEqual(self.svc.seconds_since_last_activity(), 0.0, delta=0.1) + + def test_idle_not_exceeded_immediately_after_request_with_expired_token(self): + """After any request (even a 401 one), idle timeout should not fire until inactivity resumes.""" + self.svc.config.idle_timeout = 5.0 + + with patch("time.monotonic", return_value=100.0): + # Simulate record_activity() being called (as _make_request does at its start) + self.svc.record_activity() + + # Only 2 s have passed since the last (simulated) request + with patch("time.monotonic", return_value=102.0): + self.assertFalse(self.svc.should_exit_for_idle()) + + # ------------------------------------------------------------------ + # 3. should_exit_for_idle() is purely time-based + # ------------------------------------------------------------------ + + def test_should_exit_for_idle_same_result_for_valid_and_expired_token(self): + """Token validity is invisible to should_exit_for_idle() — only elapsed time matters.""" + self.svc.config.idle_timeout = 5.0 + with patch("time.monotonic", return_value=500.0): + self.svc.record_activity() + + with patch("time.monotonic", return_value=510.0): + with patch.object(self.svc, "_check_token_expiration", return_value=(False, 3600.0)): + result_valid_token = self.svc.should_exit_for_idle() + with patch.object(self.svc, "_check_token_expiration", return_value=(True, -30.0)): + result_expired_token = self.svc.should_exit_for_idle() + + self.assertEqual(result_valid_token, result_expired_token) + self.assertTrue(result_valid_token, "idle timeout should have fired after 10 s > 5 s threshold") + + def test_should_exit_for_idle_false_within_threshold_regardless_of_token(self): + """Within the idle window, should_exit_for_idle() is False even if token is expired.""" + self.svc.config.idle_timeout = 60.0 + with patch("time.monotonic", return_value=200.0): + self.svc.record_activity() + + with patch("time.monotonic", return_value=210.0): # only 10 s elapsed + with patch.object(self.svc, "_check_token_expiration", return_value=(True, -5.0)): + self.assertFalse(self.svc.should_exit_for_idle()) + + # ------------------------------------------------------------------ + # 4. Internal re-auth alone does NOT reset the idle timer + # ------------------------------------------------------------------ + + def test_re_authenticate_alone_does_not_reset_idle_timer(self): + """_re_authenticate() handles auth internally and must not extend the idle lease.""" + self.svc.config.idle_timeout = 5.0 + with patch("time.monotonic", return_value=1000.0): + self.svc.record_activity() + + # Call _re_authenticate() without going through _make_request + with patch.object(self.svc, "_authenticate", return_value=None): + self.svc._re_authenticate() + + # No call to record_activity() happened, so idle should fire after threshold + with patch("time.monotonic", return_value=1010.0): + self.assertTrue(self.svc.should_exit_for_idle()) + + def test_refresh_token_alone_does_not_reset_idle_timer(self): + """_refresh_token() makes an HTTP call but must not extend the idle lease on its own.""" + self.svc.config.idle_timeout = 5.0 + with patch("time.monotonic", return_value=2000.0): + self.svc.record_activity() + + # Call _refresh_token() directly (simulating an internal proactive refresh) + with patch.object(self.svc, "_authenticate", return_value=None): + with patch.object(self.svc.credential_store, "token_info", None): + self.svc._refresh_token() # returns False (no token_info) without recording activity + + with patch("time.monotonic", return_value=2010.0): + self.assertTrue(self.svc.should_exit_for_idle()) + + class TestProcessManagerIdleArgv(unittest.TestCase): def test_spawn_manager_includes_idle_timeout_in_command(self): cfg = GatewayConfig(base_url="https://example.com/", username="u", password="p", idle_timeout=123.0) From c36669407975c67ec25d1df74263396d74fb31de Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Thu, 2 Apr 2026 14:44:12 +0530 Subject: [PATCH 19/27] fix lint --- .../plugin_utils/manager/manager_process.py | 18 +----------------- .../manager/test_manager_process_redaction.py | 5 ++++- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/plugins/plugin_utils/manager/manager_process.py b/plugins/plugin_utils/manager/manager_process.py index 85ed2106..b8693437 100644 --- a/plugins/plugin_utils/manager/manager_process.py +++ b/plugins/plugin_utils/manager/manager_process.py @@ -8,9 +8,9 @@ import base64 import json import os +from pathlib import Path import sys import traceback -from pathlib import Path def _compute_poll_interval(idle_timeout: float) -> int: """Return the idle-monitor sleep interval derived from the configured timeout. @@ -293,22 +293,6 @@ def signal_handler(signum, frame): except ValueError: pass - # ------------------------------------------------------------------ # - # Watchdog — decides when the manager should shut down. # - # # - # Two modes, selected at startup: # - # # - # Production (no .survive flag): # - # Poll os.kill(owner_pid, 0) every 3 s. Exit when the main # - # ansible-playbook process (owner_pid) is gone. # - # # - # Molecule (.survive flag present in socket_dir at startup): # - # Poll for the flag file's existence every 2 s. Exit when # - # destroy.yml removes it. The owner PID is not used — each # - # Molecule phase (converge / verify / cleanup) is a separate # - # ansible-playbook invocation, so the watchdog must not fire # - # between phases. # - # ------------------------------------------------------------------ # _survive_path = Path(socket_dir) / ".survive" _survive_mode = _survive_path.exists() diff --git a/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py index e414f71a..7b8906c5 100644 --- a/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py +++ b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py @@ -199,9 +199,12 @@ class TestRedactArgvInStartupLog(unittest.TestCase): def test_startup_marker_does_not_contain_password(self): """The /tmp marker file must never contain plaintext credentials.""" + import os import tempfile - fake_marker = Path(tempfile.mktemp(suffix="_test_marker.txt")) + fd, temp_path = tempfile.mkstemp(suffix="_test_marker.txt") + os.close(fd) + fake_marker = Path(temp_path) fake_argv = _make_argv(password="plaintext_password", token="plaintext_token") try: From 97d0e161a4c3de17f007fe0d9f81f968930fe159 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Thu, 2 Apr 2026 14:48:06 +0530 Subject: [PATCH 20/27] fix lint --- plugins/plugin_utils/manager/manager_process.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/plugin_utils/manager/manager_process.py b/plugins/plugin_utils/manager/manager_process.py index b8693437..ff25d4ff 100644 --- a/plugins/plugin_utils/manager/manager_process.py +++ b/plugins/plugin_utils/manager/manager_process.py @@ -8,9 +8,10 @@ import base64 import json import os -from pathlib import Path import sys import traceback +from pathlib import Path + def _compute_poll_interval(idle_timeout: float) -> int: """Return the idle-monitor sleep interval derived from the configured timeout. @@ -35,6 +36,7 @@ def _compute_poll_interval(idle_timeout: float) -> int: return 60 return max(5, min(60, int(idle_timeout / 10))) + _SENSITIVE_ARGV_POSITIONS = {5, 6, 7} From aed62770b05b858163725466d2d60b5ea745cd82 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Thu, 2 Apr 2026 14:54:07 +0530 Subject: [PATCH 21/27] fix lint --- plugins/plugin_utils/platform/config.py | 85 ++++++++++++++++--- .../manager/test_manager_process_redaction.py | 45 ++++++---- 2 files changed, 101 insertions(+), 29 deletions(-) diff --git a/plugins/plugin_utils/platform/config.py b/plugins/plugin_utils/platform/config.py index 0487073e..66747af8 100644 --- a/plugins/plugin_utils/platform/config.py +++ b/plugins/plugin_utils/platform/config.py @@ -33,7 +33,11 @@ def __post_init__(self): original_url = self.base_url self.base_url = self._normalize_url(self.base_url) if original_url != self.base_url: - logger.debug("Normalized gateway URL: %s -> %s", original_url, self.base_url) + logger.debug( + "Normalized gateway URL: %s -> %s", + original_url, + self.base_url, + ) logger.info( "GatewayConfig initialized: base_url=%s, verify_ssl=%s, timeout=%s, idle_timeout=%s", self.base_url, @@ -61,7 +65,11 @@ def _normalize_url(url: str) -> str: return url -def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars: Optional[Dict[str, Any]] = None, required: bool = True) -> GatewayConfig: +def extract_gateway_config( + task_args: Optional[Dict[str, Any]] = None, + host_vars: Optional[Dict[str, Any]] = None, + required: bool = True, +) -> GatewayConfig: """ Extract gateway configuration from task arguments and host variables. @@ -83,15 +91,32 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars task_args = task_args or {} host_vars = host_vars or {} - logger.debug("Extracting gateway config from task_args (keys: %s) and host_vars (keys: %s)", list(task_args.keys()), list(host_vars.keys())) + logger.debug( + "Extracting gateway config from task_args (keys: %s) and host_vars (keys: %s)", + list(task_args.keys()), + list(host_vars.keys()), + ) # Get gateway URL from task args first, then host_vars - gateway_url = task_args.get("gateway_url") or task_args.get("gateway_hostname") or host_vars.get("gateway_url") or host_vars.get("gateway_hostname") + gateway_url = ( + task_args.get("gateway_url") + or task_args.get("gateway_hostname") + or host_vars.get("gateway_url") + or host_vars.get("gateway_hostname") + ) logger.debug("Gateway URL extracted: %s", gateway_url) # Get auth parameters from task args first, then host_vars - gateway_username = task_args.get("gateway_username") or host_vars.get("gateway_username") or host_vars.get("aap_username") - gateway_password = task_args.get("gateway_password") or host_vars.get("gateway_password") or host_vars.get("aap_password") + gateway_username = ( + task_args.get("gateway_username") + or host_vars.get("gateway_username") + or host_vars.get("aap_username") + ) + gateway_password = ( + task_args.get("gateway_password") + or host_vars.get("gateway_password") + or host_vars.get("aap_password") + ) gateway_token_raw = ( task_args.get("gateway_token") or host_vars.get("gateway_token") @@ -101,7 +126,11 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars # in aap_token after creation; picking it up here would cause all # subsequent tasks in the same play to authenticate as that limited token # instead of the admin user, leading to 403 errors. - (host_vars.get("aap_token") if not gateway_username and not gateway_password else None) + ( + host_vars.get("aap_token") + if not gateway_username and not gateway_password + else None + ) ) # The token module sets aap_token as a dict ({"token": "...", "id": ...}). # Extract the actual token string if we got a dict. @@ -109,8 +138,16 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars gateway_token = gateway_token_raw.get("token") else: gateway_token = gateway_token_raw - gateway_validate_certs = task_args.get("gateway_validate_certs") if "gateway_validate_certs" in task_args else host_vars.get("gateway_validate_certs", True) - gateway_request_timeout = task_args.get("gateway_request_timeout") or host_vars.get("gateway_request_timeout") or 10.0 + gateway_validate_certs = ( + task_args.get("gateway_validate_certs") + if "gateway_validate_certs" in task_args + else host_vars.get("gateway_validate_certs", True) + ) + gateway_request_timeout = ( + task_args.get("gateway_request_timeout") + or host_vars.get("gateway_request_timeout") + or 10.0 + ) # How long (seconds) the persistent manager process may sit idle — i.e. receive # no RPC or API traffic — before it shuts itself down and removes its socket. # This prevents orphaned manager processes from accumulating across playbook runs. @@ -123,16 +160,32 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars or host_vars.get("ansible_platform_manager_idle_timeout") ) # Connection mode: "standard" (default) or "experimental" (persistent manager) - connection_mode = task_args.get("platform_connection_mode") or host_vars.get("platform_connection_mode") or "standard" + connection_mode = ( + task_args.get("platform_connection_mode") + or host_vars.get("platform_connection_mode") + or "standard" + ) if required and not gateway_url: logger.error("Gateway URL is required but not found in task_args or host_vars") - raise ValueError("gateway_url or gateway_hostname must be provided as task parameter or defined in inventory") + raise ValueError( + "gateway_url or gateway_hostname must be provided " + "as task parameter or defined in inventory" + ) # Log auth method being used (without exposing secrets) - auth_method = "token" if gateway_token else ("username/password" if gateway_username else "none") + if gateway_token: + auth_method = "token" + elif gateway_username: + auth_method = "username/password" + else: + auth_method = "none" logger.info( - "Gateway config extracted: url=%s, auth_method=%s, verify_ssl=%s, timeout=%s", gateway_url, auth_method, gateway_validate_certs, gateway_request_timeout + "Gateway config extracted: url=%s, auth_method=%s, verify_ssl=%s, timeout=%s", + gateway_url, + auth_method, + gateway_validate_certs, + gateway_request_timeout, ) config = GatewayConfig( @@ -143,7 +196,11 @@ def extract_gateway_config(task_args: Optional[Dict[str, Any]] = None, host_vars verify_ssl=gateway_validate_certs, request_timeout=gateway_request_timeout, connection_mode=connection_mode, - idle_timeout=float(gateway_idle_timeout) if gateway_idle_timeout is not None else 3600.0, + idle_timeout=( + float(gateway_idle_timeout) + if gateway_idle_timeout is not None + else 3600.0 + ), ) logger.debug("GatewayConfig created successfully") diff --git a/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py index 7b8906c5..cb9ca9a1 100644 --- a/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py +++ b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py @@ -21,7 +21,9 @@ # we need the collections parent on sys.path (conftest.py handles this when # running via pytest; unittest needs it done here too). # --------------------------------------------------------------------------- -_COLLECTIONS_PARENT = str(Path(__file__).resolve().parent.parent.parent.parent.parent.parent.parent.parent) +_COLLECTIONS_PARENT = str( + Path(__file__).resolve().parent.parent.parent.parent.parent.parent.parent.parent +) if _COLLECTIONS_PARENT not in sys.path: sys.path.insert(0, _COLLECTIONS_PARENT) @@ -36,16 +38,16 @@ def _make_argv(username="admin", password="s3cr3t!", token="tok123"): """Return a representative argv list matching manager_process argument layout.""" return [ "/path/to/manager_process.py", # 0: script - "/tmp/ap/manager.sock", # 1: socket_path - "/tmp/ap", # 2: socket_dir - "localhost", # 3: inventory_hostname - "https://gateway.example/", # 4: gateway_url - username, # 5: gateway_username ← sensitive - password, # 6: gateway_password ← sensitive - token, # 7: gateway_token ← sensitive - "true", # 8: validate_certs - "10.0", # 9: request_timeout - "3600.0", # 10: idle_timeout + "/tmp/ap/manager.sock", # 1: socket_path + "/tmp/ap", # 2: socket_dir + "localhost", # 3: inventory_hostname + "https://gateway.example/", # 4: gateway_url + username, # 5: gateway_username ← sensitive + password, # 6: gateway_password ← sensitive + token, # 7: gateway_token ← sensitive + "true", # 8: validate_certs + "10.0", # 9: request_timeout + "3600.0", # 10: idle_timeout ] @@ -114,7 +116,11 @@ def test_env_var_override_works_for_short_timeout_too(self): def test_no_env_var_uses_formula(self): """Without the env var, the formula applies normally.""" - env = {k: v for k, v in os.environ.items() if k != "ANSIBLE_PLATFORM_IDLE_POLL_SECONDS"} + env = { + k: v + for k, v in os.environ.items() + if k != "ANSIBLE_PLATFORM_IDLE_POLL_SECONDS" + } with patch.dict(os.environ, env, clear=True): self.assertEqual(_compute_poll_interval(300.0), 30) @@ -210,8 +216,15 @@ def test_startup_marker_does_not_contain_password(self): try: with patch.object(sys, "argv", fake_argv): with patch( - "ansible_collections.ansible.platform.plugins.plugin_utils.manager.manager_process.Path", - side_effect=lambda p: fake_marker if "ansible_platform_manager_started" in str(p) else Path(p), + ( + "ansible_collections.ansible.platform.plugins." + "plugin_utils.manager.manager_process.Path" + ), + side_effect=lambda p: ( + fake_marker + if "ansible_platform_manager_started" in str(p) + else Path(p) + ), ): with fake_marker.open("w") as _f: _f.write(f"Script started with {len(sys.argv)} args\n") @@ -229,7 +242,9 @@ def test_startup_marker_does_not_contain_password(self): def test_stderr_error_path_does_not_contain_password(self): """The early-exit print (too few args) must use _redact_argv().""" - fake_argv = _make_argv(password="plaintext_password")[:5] # too short → error path + fake_argv = _make_argv(password="plaintext_password")[ + :5 + ] # too short → error path captured = io.StringIO() # Simulate the error-path print From 9bf3e275d2d68647134934803c2edf38b5dd2604 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Thu, 2 Apr 2026 16:01:43 +0530 Subject: [PATCH 22/27] fix lint --- plugins/plugin_utils/platform/config.py | 58 ++++--------------- .../manager/test_manager_process_redaction.py | 25 ++------ 2 files changed, 15 insertions(+), 68 deletions(-) diff --git a/plugins/plugin_utils/platform/config.py b/plugins/plugin_utils/platform/config.py index 66747af8..195756bb 100644 --- a/plugins/plugin_utils/platform/config.py +++ b/plugins/plugin_utils/platform/config.py @@ -98,25 +98,12 @@ def extract_gateway_config( ) # Get gateway URL from task args first, then host_vars - gateway_url = ( - task_args.get("gateway_url") - or task_args.get("gateway_hostname") - or host_vars.get("gateway_url") - or host_vars.get("gateway_hostname") - ) + gateway_url = task_args.get("gateway_url") or task_args.get("gateway_hostname") or host_vars.get("gateway_url") or host_vars.get("gateway_hostname") logger.debug("Gateway URL extracted: %s", gateway_url) # Get auth parameters from task args first, then host_vars - gateway_username = ( - task_args.get("gateway_username") - or host_vars.get("gateway_username") - or host_vars.get("aap_username") - ) - gateway_password = ( - task_args.get("gateway_password") - or host_vars.get("gateway_password") - or host_vars.get("aap_password") - ) + gateway_username = task_args.get("gateway_username") or host_vars.get("gateway_username") or host_vars.get("aap_username") + gateway_password = task_args.get("gateway_password") or host_vars.get("gateway_password") or host_vars.get("aap_password") gateway_token_raw = ( task_args.get("gateway_token") or host_vars.get("gateway_token") @@ -126,11 +113,7 @@ def extract_gateway_config( # in aap_token after creation; picking it up here would cause all # subsequent tasks in the same play to authenticate as that limited token # instead of the admin user, leading to 403 errors. - ( - host_vars.get("aap_token") - if not gateway_username and not gateway_password - else None - ) + (host_vars.get("aap_token") if not gateway_username and not gateway_password else None) ) # The token module sets aap_token as a dict ({"token": "...", "id": ...}). # Extract the actual token string if we got a dict. @@ -138,16 +121,8 @@ def extract_gateway_config( gateway_token = gateway_token_raw.get("token") else: gateway_token = gateway_token_raw - gateway_validate_certs = ( - task_args.get("gateway_validate_certs") - if "gateway_validate_certs" in task_args - else host_vars.get("gateway_validate_certs", True) - ) - gateway_request_timeout = ( - task_args.get("gateway_request_timeout") - or host_vars.get("gateway_request_timeout") - or 10.0 - ) + gateway_validate_certs = task_args.get("gateway_validate_certs") if "gateway_validate_certs" in task_args else host_vars.get("gateway_validate_certs", True) + gateway_request_timeout = task_args.get("gateway_request_timeout") or host_vars.get("gateway_request_timeout") or 10.0 # How long (seconds) the persistent manager process may sit idle — i.e. receive # no RPC or API traffic — before it shuts itself down and removes its socket. # This prevents orphaned manager processes from accumulating across playbook runs. @@ -155,23 +130,14 @@ def extract_gateway_config( # Accepted variable names (task arg takes priority over host var): # gateway_idle_timeout / ansible_platform_manager_idle_timeout gateway_idle_timeout = ( - task_args.get("gateway_idle_timeout") - or host_vars.get("gateway_idle_timeout") - or host_vars.get("ansible_platform_manager_idle_timeout") + task_args.get("gateway_idle_timeout") or host_vars.get("gateway_idle_timeout") or host_vars.get("ansible_platform_manager_idle_timeout") ) # Connection mode: "standard" (default) or "experimental" (persistent manager) - connection_mode = ( - task_args.get("platform_connection_mode") - or host_vars.get("platform_connection_mode") - or "standard" - ) + connection_mode = task_args.get("platform_connection_mode") or host_vars.get("platform_connection_mode") or "standard" if required and not gateway_url: logger.error("Gateway URL is required but not found in task_args or host_vars") - raise ValueError( - "gateway_url or gateway_hostname must be provided " - "as task parameter or defined in inventory" - ) + raise ValueError("gateway_url or gateway_hostname must be provided as task parameter or defined in inventory") # Log auth method being used (without exposing secrets) if gateway_token: @@ -196,11 +162,7 @@ def extract_gateway_config( verify_ssl=gateway_validate_certs, request_timeout=gateway_request_timeout, connection_mode=connection_mode, - idle_timeout=( - float(gateway_idle_timeout) - if gateway_idle_timeout is not None - else 3600.0 - ), + idle_timeout=(float(gateway_idle_timeout) if gateway_idle_timeout is not None else 3600.0), ) logger.debug("GatewayConfig created successfully") diff --git a/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py index cb9ca9a1..01ed410b 100644 --- a/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py +++ b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py @@ -21,9 +21,7 @@ # we need the collections parent on sys.path (conftest.py handles this when # running via pytest; unittest needs it done here too). # --------------------------------------------------------------------------- -_COLLECTIONS_PARENT = str( - Path(__file__).resolve().parent.parent.parent.parent.parent.parent.parent.parent -) +_COLLECTIONS_PARENT = str(Path(__file__).resolve().parent.parent.parent.parent.parent.parent.parent.parent) if _COLLECTIONS_PARENT not in sys.path: sys.path.insert(0, _COLLECTIONS_PARENT) @@ -116,11 +114,7 @@ def test_env_var_override_works_for_short_timeout_too(self): def test_no_env_var_uses_formula(self): """Without the env var, the formula applies normally.""" - env = { - k: v - for k, v in os.environ.items() - if k != "ANSIBLE_PLATFORM_IDLE_POLL_SECONDS" - } + env = {k: v for k, v in os.environ.items() if k != "ANSIBLE_PLATFORM_IDLE_POLL_SECONDS"} with patch.dict(os.environ, env, clear=True): self.assertEqual(_compute_poll_interval(300.0), 30) @@ -216,15 +210,8 @@ def test_startup_marker_does_not_contain_password(self): try: with patch.object(sys, "argv", fake_argv): with patch( - ( - "ansible_collections.ansible.platform.plugins." - "plugin_utils.manager.manager_process.Path" - ), - side_effect=lambda p: ( - fake_marker - if "ansible_platform_manager_started" in str(p) - else Path(p) - ), + ("ansible_collections.ansible.platform.plugins.plugin_utils.manager.manager_process.Path"), + side_effect=lambda p: fake_marker if "ansible_platform_manager_started" in str(p) else Path(p), ): with fake_marker.open("w") as _f: _f.write(f"Script started with {len(sys.argv)} args\n") @@ -242,9 +229,7 @@ def test_startup_marker_does_not_contain_password(self): def test_stderr_error_path_does_not_contain_password(self): """The early-exit print (too few args) must use _redact_argv().""" - fake_argv = _make_argv(password="plaintext_password")[ - :5 - ] # too short → error path + fake_argv = _make_argv(password="plaintext_password")[:5] # too short → error path captured = io.StringIO() # Simulate the error-path print From d56720713a3ab3ba15c2e01dbba3fc792fdda20a Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Wed, 8 Apr 2026 16:38:26 +0530 Subject: [PATCH 23/27] address review comments --- MIGRATION.md | 109 ----------- docs/11-persistent-manager-idle-timeout.md | 185 ++++++++++++++++++ docs/README.md | 8 +- .../molecule/idle_timeout_mock/converge.yml | 2 +- .../molecule/idle_timeout_mock/molecule.yml | 2 - plugins/doc_fragments/auth.py | 7 + .../plugin_utils/manager/manager_process.py | 13 +- plugins/plugin_utils/platform/config.py | 33 +++- .../manager/test_manager_process_redaction.py | 26 +-- .../manager/test_platform_service_idle.py | 9 +- .../test_extract_gateway_config_idle.py | 74 +++++++ 11 files changed, 308 insertions(+), 160 deletions(-) delete mode 100644 MIGRATION.md create mode 100644 docs/11-persistent-manager-idle-timeout.md create mode 100644 tests/unit/plugins/plugin_utils/test_extract_gateway_config_idle.py diff --git a/MIGRATION.md b/MIGRATION.md deleted file mode 100644 index d624a827..00000000 --- a/MIGRATION.md +++ /dev/null @@ -1,109 +0,0 @@ -# Migration guide: persistent Gateway connections (AAP 2.7 / `ansible.platform`) - -This document describes backward compatibility, the recommended migration path, and an optional phased deprecation plan for the **persistent manager / connection-plugin** work in the `ansible.platform` collection. It applies to automation targeting **Ansible Automation Platform 2.7** and the `ansible.platform` collection version bundled or pinned with that release. - -Confirm the exact collection version and any Red Hat release notes for your environment; behavior described here follows the current collection implementation. - ---- - -## What changed (summary) - -- **Connection plugin `ansible.platform.http`** is the supported way to route Gateway traffic. It implements `get_client()`, which chooses **direct** (ephemeral manager per task) or **persistent** (reuse one manager process and HTTP session across tasks in a play) mode. -- **Action plugins** for platform modules call `_get_or_spawn_manager()`, which **prefers** the connection plugin's `get_client()` when `ansible_connection` is `ansible.platform.http`. They still work with **`ansible_connection: local`** by spawning an **ephemeral** manager (`spawn_ephemeral_client()`), but that path does **not** integrate with the connection plugin's persistent lifecycle or facts in the same way. -- **Direct mode** (default) still uses the same manager-based stack as persistent mode; the difference is **lifecycle** (new ephemeral manager per task vs reuse). Performance tuning is primarily about **persistent** mode and fewer TLS/auth round-trips. -- **Stale socket recovery**: when reusing a persistent manager, if the socket file exists but the process is gone, the connection plugin detects a **stale socket**, removes it, and spawns a new manager. - ---- - -## Backward compatibility (today) - -| Configuration | Behavior | Persistent reuse across tasks? | -|---------------|----------|--------------------------------| -| `ansible_connection: ansible.platform.http` and `persistent: false` (default) | Ephemeral manager per task via connection plugin | No | -| `ansible_connection: ansible.platform.http` and `persistent: true` (or equivalent vars, see below) | One manager per play/host/credential set; facts cache socket + authkey | Yes | - -Existing playbooks that use **`connection: local`** and pass Gateway options continue to run **without** switching the connection plugin, as long as they use modules that have a matching **action plugin** (the normal case for resource modules in this collection). - ---- - -## Migration path - -### 1. Use the platform HTTP connection plugin (recommended) - -Set the inventory host (or group vars) that represents the Gateway to use the collection connection plugin: - -```yaml -# inventory.yml (example) -all: - children: - gateway_hosts: - hosts: - aap_gateway: - ansible_host: gateway.example.com # informational; API target is still gateway_url - ansible_connection: ansible.platform.http - # Optional: enable persistent mode for this host - ansible_platform_use_persistent_connection: true -``` - -FQCN for the plugin transport is **`ansible.platform.http`** (see `transport` in `plugins/connection/http.py`). - -### 2. Gateway URL and credentials - -The action layer builds a `GatewayConfig` via `extract_gateway_config()` from **task arguments** and **host/task variables**. At minimum you must supply a Gateway base URL: - -| Purpose | Task / host variables (priority order in code) | -|---------|------------------------------------------------| -| Gateway URL | `gateway_url` or `gateway_hostname` | -| Username / password | `gateway_username` / `gateway_password`, or aliases `aap_username` / `aap_password` | -| OAuth token | `gateway_token` or `aap_token` (with special handling so a module-created `aap_token` dict does not override user/password auth) | -| TLS / timeout | `gateway_validate_certs`, `gateway_request_timeout` (and `aap_*` aliases where documented in fragments) | - -**Automation Controller (AAP) job templates:** map your **credential** or **extra variables** so the above keys are present for the Gateway host (or for `localhost` if you use a single inventory host for API tasks). The exact credential type and injectors depend on your Controller version; align injectors with the variable names this collection reads (`gateway_*` / `aap_*`). - -### 3. Enabling persistent vs direct mode - -Resolution order for **persistent** behavior (connection plugin `get_client()`): - -1. Connection option **`persistent`** if Ansible supplies it for `ansible.platform.http` (for example via plugin configuration that maps to `get_option('persistent')`). -2. If that option is unset, **`ansible_platform_use_persistent_connection`** from host vars or task vars (and a host-var form under `hostvars[inventory_hostname]`). -3. Else **`ansible_platform_persistent`** (same scoping as above). -4. Else environment **`ANSIBLE_PLATFORM_PERSISTENT`**. -5. Else INI **`[platform_connection] persistent=`** (see plugin `DOCUMENTATION`). -6. Default: **false** (direct / ephemeral per task). - -In practice, most playbooks use **`ansible_platform_use_persistent_connection`** or **`ansible_platform_persistent`** (as in integration and Molecule scenarios). - -Truthy values are boolean `true` or strings `true`, `yes`, `1` (see `_truthy()` in the connection plugin). - -When persistent mode spawns a manager, the action plugin result may include **cacheable facts**: - -- `platform_manager_socket` -- `platform_manager_authkey` -- `gateway_url` (when returned by the connection plugin) - -These allow the next task to reuse the same manager. Changing **URL or credentials** changes the derived socket identity; do not expect reuse across different Gateway identities. - -### 4. Operational notes - -- **Socket locations**: persistent managers use `$(TMPDIR or system temp)/ansible_platform/`; ephemeral paths used by direct mode may use short paths under `/tmp/ap/` (see connection plugin and `spawn_ephemeral_client()`). -- **AF_UNIX**: if Unix domain sockets are unavailable, the local fallback can use `DirectHTTPClient` without a manager process (see `spawn_ephemeral_client()`). -- **`platform_connection_mode`**: still parsed into `GatewayConfig` for compatibility; routing between persistent and direct is controlled by the **connection plugin** options/vars above, not by switching this field alone. - ---- - -## Parallel support: modules and action plugins - -- Platform **resource modules** in this collection are intended to run with their **action plugins**, which perform validation, manager acquisition, and API execution. -- **Parallel support** means you may keep **`connection: local`** during a transition while you test **`ansible.platform.http`** on staging inventories. Both paths use the manager architecture (except AF_UNIX fallback), but only the HTTP connection plugin provides **centralized** persistent vs direct policy and stale-socket handling aligned with connection-level configuration. - ---- - -## Quick checklist - -- [ ] Set `ansible_connection: ansible.platform.http` on the Gateway inventory host (or group). -- [ ] Supply `gateway_url` / `gateway_hostname` and auth (`gateway_username`/`gateway_password` or token vars). -- [ ] Decide on **persistent** (`ansible_platform_use_persistent_connection: true` or connection option `persistent: true`) vs **direct** (default). -- [ ] Validate job templates and credentials inject the same variable names your playbooks expect. -- [ ] After upgrade, run a multi-task playbook once with persistent mode and confirm fact-driven reuse (or benchmark latency improvement). - -For architecture background, see `docs/03-sdk-architecture.md` and `docs/06-foundation-components.md` in this repository. diff --git a/docs/11-persistent-manager-idle-timeout.md b/docs/11-persistent-manager-idle-timeout.md new file mode 100644 index 00000000..82a13807 --- /dev/null +++ b/docs/11-persistent-manager-idle-timeout.md @@ -0,0 +1,185 @@ +# Persistent manager idle timeout + +This document describes the **control-node** idle timeout for the persistent manager process: what it controls, how it is configured, which edge cases the implementation handles, and where those behaviors are tested. + +--- + +## Scope + + +| Aspect | Behavior | +| ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| **What it limits** | How long the **local** persistent manager process on the Ansible control node may remain idle (no RPC / API activity) before it exits and removes its Unix socket. | +| **What it is not** | It is **not** a gateway server session timeout. The value is not sent to the gateway as a session policy; it only affects the local subprocess lifecycle. | +| **Default** | **3600** seconds (one hour) when the option is unset. | +| **Disable** | Set `**persistent_manager_idle_timeout: 0`** to turn off idle-based shutdown of the persistent manager. | + + +--- + +## Configuration surface + +Only one Ansible variable name is honored: + +- `**persistent_manager_idle_timeout**` (float), in task arguments or host/inventory variables. + +**Precedence:** task arguments override host/inventory variables. + +**Extraction** uses `key in dict` checks (not `a or b` chains) so a configured value of `**0`** is never dropped in favor of a fallback. + +Legacy or alternate names (for example `gateway_idle_timeout`) are **not** read; they have no effect on idle timeout. + +--- + +## Issues and scenarios this implementation addresses + +The following problems are explicitly covered by code and tests. + +### 1. Single, unambiguous variable name + +**Issue:** Multiple overlapping names suggested different semantics (“gateway” vs “platform manager”) and made docs and inventory hard to reason about. + +**Behavior:** Only `persistent_manager_idle_timeout` is documented and parsed. Misnamed keys do not configure idle timeout. + +**Tests:** `test_other_keys_do_not_set_idle_timeout` in `tests/unit/plugins/plugin_utils/test_extract_gateway_config_idle.py`. + +--- + +### 2. Preserving `0` (no accidental fallback) + +**Issue:** Patterns such as `task.get("x") or host.get("x")` treat `**0`** as falsy and incorrectly fall back to a default or host value, so “disable idle shutdown” could not be expressed reliably. + +**Behavior:** Extraction checks membership (`"persistent_manager_idle_timeout" in task_args`) before reading the value, so `**0`** is preserved. + +**Tests:** + +- Task `0` → `idle_timeout` is `0.0` (`test_task_zero_disables_idle_shutdown`). +- Task `0` wins over host non-zero (`test_task_zero_wins_over_host_nonzero`). +- Host-only `0` when task omits the key (`test_host_zero_when_not_in_task`). + +--- + +### 3. Sensible default when unset + +**Issue:** Operators need predictable behavior when nothing is configured. + +**Behavior:** If the key is absent from both task args and host vars, `**3600.0`** seconds is used. + +**Tests:** `test_default_3600_when_unset`, `test_gateway_config_default_idle_timeout`. + +--- + +### 4. Task vs inventory precedence + +**Issue:** It must be clear whether a play-level override or inventory wins. + +**Behavior:** Task arguments take precedence over host variables for `persistent_manager_idle_timeout`. + +**Tests:** `test_task_zero_wins_over_host_nonzero`, `test_extract_gateway_config_idle_timeout_from_task_args`, `test_extract_gateway_config_idle_timeout_from_host_vars`. + +--- + +### 5. Idle decision is time-based only + +**Issue:** Idle shutdown must not depend on OAuth token validity or other auth state, or behavior becomes non-deterministic. + +**Behavior:** `should_exit_for_idle()` is **purely time-based** (elapsed time since last recorded activity vs `idle_timeout`). Token validity does not change the boolean result for the same timestamps. + +**Tests:** `test_should_exit_for_idle_same_result_for_valid_and_expired_token`, `test_should_exit_for_idle_false_within_threshold_regardless_of_token`. + +--- + +### 6. Expired token does not “freeze” idle exit + +**Issue:** If the token expires while there is no traffic, the manager should still exit after the idle interval. + +**Behavior:** With no new `record_activity()`, idle timeout still fires regardless of token expiry or `oauth_token` being cleared. + +**Tests:** `test_expired_token_alone_does_not_suppress_idle_exit`, `test_expired_token_does_not_prevent_idle_exit_when_no_traffic`. + +--- + +### 7. User-facing requests reset the idle clock (including failures) + +**Issue:** Activity should reflect “something tried to use the gateway,” including failed HTTP calls where the client still did work. + +**Behavior:** `record_activity()` runs at the start of the request path (before the HTTP call), so a **401** still resets the idle timer for that attempt. + +**Tests:** `test_401_response_still_resets_idle_timer`, `test_idle_not_exceeded_immediately_after_request_with_expired_token`. + +--- + +### 8. Internal re-auth must not extend the idle lease + +**Issue:** Background token refresh or re-authentication should not keep the manager alive when there is no real user/module traffic. + +**Behavior:** `_re_authenticate()` and `_refresh_token()` (when not going through the normal request path that records activity) do **not** reset the idle timer. + +**Tests:** `test_re_authenticate_alone_does_not_reset_idle_timer`, `test_refresh_token_alone_does_not_reset_idle_timer`. + +--- + +### 9. Shutdown already requested + +**Issue:** After a graceful shutdown is requested, the idle monitor should not keep driving exit logic in a confusing way. + +**Behavior:** When shutdown has been requested, `should_exit_for_idle()` returns false (idle-based exit is not the path). + +**Tests:** `test_should_exit_for_idle_false_after_shutdown_requested`. + +--- + +### 10. Poll interval derived from timeout (no hidden env override) + +**Issue:** The manager needs a check interval that scales with the configured timeout without requiring extra environment variables. + +**Behavior:** `_compute_poll_interval(idle_timeout)` uses **10%** of `idle_timeout`, clamped to **[5, 60]** seconds. For `**idle_timeout <= 0`** (disabled), the returned interval is a fixed **60** s (the idle monitor path treats shutdown as disabled separately). + +**Tests:** `TestComputePollInterval` in `tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py`. + +--- + +### 11. Subprocess receives idle timeout and defaults safely + +**Issue:** The manager runs as a separate process with CLI arguments; the parent must pass the configured value and default the optional argument. + +**Behavior:** `ProcessManager.spawn_manager_process` appends `gateway_config.idle_timeout` as the last argv element. The child parses an optional 10th argument and defaults to **3600.0** if missing. + +**Tests:** `test_spawn_manager_includes_idle_timeout_in_command`. + +--- + +### 12. Credential argv redaction (security) + +**Issue:** Logging `sys.argv` must not leak username, password, or token. + +**Behavior:** Positions **5, 6, 7** (username, password, token) are replaced with `` in logged argv copies. The idle timeout value (position **10**) is not treated as a secret. + +**Tests:** `test_manager_process_redaction.py` (`TestRedactArgv`, `TestSensitiveArgvPositions`). + +--- + +### 13. End-to-end: manager exits after idle (integration) + +**Issue:** The full stack (spawn → idle → socket removal) should be verifiable without a real gateway. + +**Behavior:** The Molecule scenario `**extensions/molecule/idle_timeout_mock`** sets `persistent_manager_idle_timeout: 15` and asserts the manager exits and cleans up after the idle window. + +--- + +## Quick reference: unit test modules + + +| Module | Focus | +| --------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | +| `tests/unit/plugins/plugin_utils/test_extract_gateway_config_idle.py` | Extraction, `0`, defaults, legacy keys ignored | +| `tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py` | `GatewayConfig`, `PlatformService` idle logic, OAuth/401/re-auth cases, argv spawn | +| `tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py` | Poll interval math, argv redaction | + + +--- + +## Related user-facing documentation + +- `plugins/doc_fragments/auth.py` — `persistent_manager_idle_timeout` option text for modules that include the auth fragment. + diff --git a/docs/README.md b/docs/README.md index 51a07e02..cd67fbaf 100644 --- a/docs/README.md +++ b/docs/README.md @@ -20,6 +20,7 @@ so developers familiar with that collection find the same patterns and numbering | 08 | [08-testing-strategy.md](08-testing-strategy.md) | All devs / QE | Three-layer strategy: unit (pytest), Molecule mock, integration; CI workflows; linting | | 09 | [09-agent-collaboration.md](09-agent-collaboration.md) | AI agents | Personas, phase-by-phase guidance, coding standards, human-in-the-loop triggers, troubleshooting | | 10 | [10-case-study-aap-platform.md](10-case-study-aap-platform.md) | Feature devs | Module map, identity categories, known API quirks, implementation roadmap | +| 11 | [11-persistent-manager-idle-timeout.md](11-persistent-manager-idle-timeout.md) | Framework devs / operators | Persistent manager idle timeout: config, semantics, edge cases, tests | --- @@ -45,6 +46,9 @@ so developers familiar with that collection find the same patterns and numbering ### "I need to write or fix tests" → [08-testing-strategy.md](08-testing-strategy.md) +### "I need to understand persistent manager idle timeout behavior" +→ [11-persistent-manager-idle-timeout.md](11-persistent-manager-idle-timeout.md) + --- ## Document Dependency Map @@ -58,7 +62,9 @@ so developers familiar with that collection find the same patterns and numbering │ │ │ ├── 04-data-model-transformation (three-tier pattern) │ │ - │ └── 05-design-principles (the rules) + │ ├── 05-design-principles (the rules) + │ │ + │ └── 11-persistent-manager-idle-timeout (local manager idle shutdown) │ ├── 06-foundation-components (build the framework) │ │ diff --git a/extensions/molecule/idle_timeout_mock/converge.yml b/extensions/molecule/idle_timeout_mock/converge.yml index 1759df37..575285a4 100644 --- a/extensions/molecule/idle_timeout_mock/converge.yml +++ b/extensions/molecule/idle_timeout_mock/converge.yml @@ -30,7 +30,7 @@ gateway_password: "testpass" gateway_validate_certs: false ansible_platform_use_persistent_connection: true - gateway_idle_timeout: 15 + persistent_manager_idle_timeout: 15 tasks: - name: Trigger manager activity (create a user) diff --git a/extensions/molecule/idle_timeout_mock/molecule.yml b/extensions/molecule/idle_timeout_mock/molecule.yml index dc35ba5d..0e0f6a05 100644 --- a/extensions/molecule/idle_timeout_mock/molecule.yml +++ b/extensions/molecule/idle_timeout_mock/molecule.yml @@ -11,8 +11,6 @@ ansible: executor: args: ansible_playbook: [] - env: - ANSIBLE_PLATFORM_IDLE_POLL_SECONDS: "2" provisioner: name: ansible diff --git a/plugins/doc_fragments/auth.py b/plugins/doc_fragments/auth.py index fb86fb83..9cbe44da 100644 --- a/plugins/doc_fragments/auth.py +++ b/plugins/doc_fragments/auth.py @@ -55,4 +55,11 @@ class ModuleDocFragment(object): - If value not set, will try environment variable C(GATEWAY_REQUEST_TIMEOUT), E(AAP_REQUEST_TIMEOUT) type: float aliases: [ request_timeout, gateway_request_timeout ] + persistent_manager_idle_timeout: + description: + - Seconds with no RPC or API activity before the persistent manager process on the Ansible control node exits and removes its Unix socket. + - This controls local process lifetime only; it is not a gateway server session timeout and is not sent to the gateway. + - Defaults to C(3600) (one hour) when unset. + - Use C(0) to disable idle-based shutdown of the persistent manager. + type: float """ diff --git a/plugins/plugin_utils/manager/manager_process.py b/plugins/plugin_utils/manager/manager_process.py index ff25d4ff..f2a0b53b 100644 --- a/plugins/plugin_utils/manager/manager_process.py +++ b/plugins/plugin_utils/manager/manager_process.py @@ -21,17 +21,10 @@ def _compute_poll_interval(idle_timeout: float) -> int: one poll interval (10 % of the timeout) instead of a fixed 60 s. Bounds: - - Floor: 5 s — avoids busy-looping for very short timeouts (e.g. tests). + - Floor: 5 s — avoids busy-looping for very short timeouts. - Cap: 60 s — avoids infrequent checks for very long timeouts. - ``idle_timeout <= 0`` (disabled): returns 60 s (interval is irrelevant). - - The ``ANSIBLE_PLATFORM_IDLE_POLL_SECONDS`` environment variable overrides - this calculation entirely and is intended only for test environments where - a sub-second or very short poll period is needed. """ - env_override = os.environ.get("ANSIBLE_PLATFORM_IDLE_POLL_SECONDS") - if env_override is not None: - return int(env_override) if idle_timeout <= 0: return 60 return max(5, min(60, int(idle_timeout / 10))) @@ -99,7 +92,7 @@ def log_marker(msg): gateway_token = sys.argv[7] or None gateway_validate_certs = sys.argv[8].lower() == "true" gateway_request_timeout = float(sys.argv[9]) - gateway_idle_timeout = float(sys.argv[10]) if len(sys.argv) > 10 else 3600.0 + pm_idle_timeout_arg = float(sys.argv[10]) if len(sys.argv) > 10 else 3600.0 log_marker("Arguments parsed successfully") log_marker("Reading environment variables...") @@ -189,7 +182,7 @@ def log_marker(msg): verify_ssl=gateway_validate_certs, request_timeout=gateway_request_timeout, connection_mode="experimental", # Persistent manager is always experimental mode - idle_timeout=gateway_idle_timeout, + idle_timeout=pm_idle_timeout_arg, ) with open(error_log, "a") as f: f.write("GatewayConfig created successfully\n") diff --git a/plugins/plugin_utils/platform/config.py b/plugins/plugin_utils/platform/config.py index 195756bb..4ab4a0cc 100644 --- a/plugins/plugin_utils/platform/config.py +++ b/plugins/plugin_utils/platform/config.py @@ -65,6 +65,25 @@ def _normalize_url(url: str) -> str: return url +def _extract_persistent_manager_idle_timeout( + task_args: Dict[str, Any], + host_vars: Dict[str, Any], +) -> Optional[Any]: + """Return ``persistent_manager_idle_timeout`` if set in task or host scope (including ``0``). + + Controls how long the *local* manager process on the control node may stay + idle before exiting; it is not a gateway server-side timeout. + + Uses ``key in dict`` so ``0`` is not dropped (unlike ``a or b`` chains). + Task arguments override host/inventory variables. + """ + if "persistent_manager_idle_timeout" in task_args: + return task_args["persistent_manager_idle_timeout"] + if "persistent_manager_idle_timeout" in host_vars: + return host_vars["persistent_manager_idle_timeout"] + return None + + def extract_gateway_config( task_args: Optional[Dict[str, Any]] = None, host_vars: Optional[Dict[str, Any]] = None, @@ -123,15 +142,9 @@ def extract_gateway_config( gateway_token = gateway_token_raw gateway_validate_certs = task_args.get("gateway_validate_certs") if "gateway_validate_certs" in task_args else host_vars.get("gateway_validate_certs", True) gateway_request_timeout = task_args.get("gateway_request_timeout") or host_vars.get("gateway_request_timeout") or 10.0 - # How long (seconds) the persistent manager process may sit idle — i.e. receive - # no RPC or API traffic — before it shuts itself down and removes its socket. - # This prevents orphaned manager processes from accumulating across playbook runs. - # Default: 3600 s (1 hour). Set to 0 to disable idle-based shutdown entirely. - # Accepted variable names (task arg takes priority over host var): - # gateway_idle_timeout / ansible_platform_manager_idle_timeout - gateway_idle_timeout = ( - task_args.get("gateway_idle_timeout") or host_vars.get("gateway_idle_timeout") or host_vars.get("ansible_platform_manager_idle_timeout") - ) + # Local persistent manager idle shutdown (not a gateway session timeout). + # Default: 3600 s. Set to 0 to disable. See _extract_persistent_manager_idle_timeout. + pm_idle_timeout = _extract_persistent_manager_idle_timeout(task_args, host_vars) # Connection mode: "standard" (default) or "experimental" (persistent manager) connection_mode = task_args.get("platform_connection_mode") or host_vars.get("platform_connection_mode") or "standard" @@ -162,7 +175,7 @@ def extract_gateway_config( verify_ssl=gateway_validate_certs, request_timeout=gateway_request_timeout, connection_mode=connection_mode, - idle_timeout=(float(gateway_idle_timeout) if gateway_idle_timeout is not None else 3600.0), + idle_timeout=(float(pm_idle_timeout) if pm_idle_timeout is not None else 3600.0), ) logger.debug("GatewayConfig created successfully") diff --git a/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py index 01ed410b..415fc4e7 100644 --- a/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py +++ b/tests/unit/plugins/plugin_utils/manager/test_manager_process_redaction.py @@ -10,7 +10,6 @@ from __future__ import absolute_import, division, print_function import io -import os import sys import unittest from pathlib import Path @@ -53,10 +52,7 @@ class TestComputePollInterval(unittest.TestCase): """_compute_poll_interval derives the idle-monitor sleep from idle_timeout.""" def _call(self, idle_timeout): - """Call without the env-var override in effect.""" - with patch.dict(os.environ, {}, clear=False): - os.environ.pop("ANSIBLE_PLATFORM_IDLE_POLL_SECONDS", None) - return _compute_poll_interval(idle_timeout) + return _compute_poll_interval(idle_timeout) # ------------------------------------------------------------------ # Core formula: 10 % of idle_timeout, clamped to [5, 60] @@ -98,26 +94,6 @@ def test_negative_idle_timeout_returns_60(self): """Negative idle_timeout (also treated as disabled) → 60 s.""" self.assertEqual(self._call(-1.0), 60) - # ------------------------------------------------------------------ - # Env-var override (test harness) - # ------------------------------------------------------------------ - - def test_env_var_override_takes_precedence(self): - """ANSIBLE_PLATFORM_IDLE_POLL_SECONDS bypasses the formula entirely.""" - with patch.dict(os.environ, {"ANSIBLE_PLATFORM_IDLE_POLL_SECONDS": "2"}): - self.assertEqual(_compute_poll_interval(3600.0), 2) - - def test_env_var_override_works_for_short_timeout_too(self): - """Env-var overrides even when timeout is small (test speed-up).""" - with patch.dict(os.environ, {"ANSIBLE_PLATFORM_IDLE_POLL_SECONDS": "1"}): - self.assertEqual(_compute_poll_interval(5.0), 1) - - def test_no_env_var_uses_formula(self): - """Without the env var, the formula applies normally.""" - env = {k: v for k, v in os.environ.items() if k != "ANSIBLE_PLATFORM_IDLE_POLL_SECONDS"} - with patch.dict(os.environ, env, clear=True): - self.assertEqual(_compute_poll_interval(300.0), 30) - class TestSensitiveArgvPositions(unittest.TestCase): def test_sensitive_positions_cover_username_password_token(self): diff --git a/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py b/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py index 2ef4657b..16a0efa9 100644 --- a/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py +++ b/tests/unit/plugins/plugin_utils/manager/test_platform_service_idle.py @@ -40,7 +40,12 @@ def test_gateway_config_default_idle_timeout(self): def test_extract_gateway_config_idle_timeout_from_task_args(self): c = extract_gateway_config( - task_args={"gateway_url": "https://gw.example", "gateway_username": "a", "gateway_password": "b", "gateway_idle_timeout": 7200}, + task_args={ + "gateway_url": "https://gw.example", + "gateway_username": "a", + "gateway_password": "b", + "persistent_manager_idle_timeout": 7200, + }, host_vars={}, required=True, ) @@ -49,7 +54,7 @@ def test_extract_gateway_config_idle_timeout_from_task_args(self): def test_extract_gateway_config_idle_timeout_from_host_vars(self): c = extract_gateway_config( task_args={"gateway_url": "https://gw.example", "gateway_username": "a", "gateway_password": "b"}, - host_vars={"ansible_platform_manager_idle_timeout": 1800}, + host_vars={"persistent_manager_idle_timeout": 1800}, required=True, ) self.assertEqual(c.idle_timeout, 1800.0) diff --git a/tests/unit/plugins/plugin_utils/test_extract_gateway_config_idle.py b/tests/unit/plugins/plugin_utils/test_extract_gateway_config_idle.py new file mode 100644 index 00000000..12218c3e --- /dev/null +++ b/tests/unit/plugins/plugin_utils/test_extract_gateway_config_idle.py @@ -0,0 +1,74 @@ +# (c) 2026 Red Hat Inc. +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +"""Tests for persistent_manager_idle_timeout extraction (including value 0).""" + +from __future__ import absolute_import, division, print_function + +import sys +import unittest +from pathlib import Path + +# Lives in plugin_utils/ (not plugin_utils/platform/) to avoid shadowing stdlib ``platform`` on import. +_COLLECTIONS_PARENT = str(Path(__file__).resolve().parent.parent.parent.parent.parent.parent.parent) +if _COLLECTIONS_PARENT not in sys.path: + sys.path.insert(0, _COLLECTIONS_PARENT) + +from ansible_collections.ansible.platform.plugins.plugin_utils.platform.config import ( # noqa: E402 + extract_gateway_config, +) + + +class TestExtractPersistentManagerIdleTimeout(unittest.TestCase): + """Only ``persistent_manager_idle_timeout`` is read; ``0`` must be preserved.""" + + _base_task = { + "gateway_url": "https://gw.example", + "gateway_username": "a", + "gateway_password": "b", + } + + def test_task_zero_disables_idle_shutdown(self): + c = extract_gateway_config( + task_args={**self._base_task, "persistent_manager_idle_timeout": 0}, + host_vars={}, + required=True, + ) + self.assertEqual(c.idle_timeout, 0.0) + + def test_task_zero_wins_over_host_nonzero(self): + c = extract_gateway_config( + task_args={**self._base_task, "persistent_manager_idle_timeout": 0}, + host_vars={"persistent_manager_idle_timeout": 7200}, + required=True, + ) + self.assertEqual(c.idle_timeout, 0.0) + + def test_host_zero_when_not_in_task(self): + c = extract_gateway_config( + task_args=dict(self._base_task), + host_vars={"persistent_manager_idle_timeout": 0}, + required=True, + ) + self.assertEqual(c.idle_timeout, 0.0) + + def test_default_3600_when_unset(self): + c = extract_gateway_config( + task_args=dict(self._base_task), + host_vars={}, + required=True, + ) + self.assertEqual(c.idle_timeout, 3600.0) + + def test_other_keys_do_not_set_idle_timeout(self): + """Only persistent_manager_idle_timeout is honored.""" + c = extract_gateway_config( + task_args={**self._base_task, "gateway_idle_timeout": 99}, + host_vars={}, + required=True, + ) + self.assertEqual(c.idle_timeout, 3600.0) + + +if __name__ == "__main__": + unittest.main() From b6a34a9df12c2c88808526d121c31a2041887da5 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Wed, 8 Apr 2026 17:33:00 +0530 Subject: [PATCH 24/27] fix lint --- plugins/plugin_utils/manager/platform_manager.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/plugins/plugin_utils/manager/platform_manager.py b/plugins/plugin_utils/manager/platform_manager.py index 7385dc45..170cc8ad 100644 --- a/plugins/plugin_utils/manager/platform_manager.py +++ b/plugins/plugin_utils/manager/platform_manager.py @@ -10,7 +10,6 @@ import logging import threading import time -import time from dataclasses import asdict from multiprocessing.managers import BaseManager from socketserver import ThreadingMixIn @@ -113,9 +112,6 @@ def __init__(self, config: GatewayConfig): self._shutdown_requested = False self._shutdown_lock = threading.Lock() - self._activity_lock = threading.Lock() - self._last_activity_monotonic = time.monotonic() - # Idle timeout: last time the service handled user-facing work (RPC / HTTP) self._activity_lock = threading.Lock() self._last_activity_monotonic = time.monotonic() @@ -308,18 +304,6 @@ def _re_authenticate(self) -> bool: logger.error("Re-authentication failed: %s", e) return False - # --- Idle-timeout helpers --- - - def record_activity(self) -> None: - """Reset the idle clock. Call whenever a real API call completes.""" - with self._activity_lock: - self._last_activity_monotonic = time.monotonic() - - def seconds_since_last_activity(self) -> float: - """Return seconds elapsed since the last recorded activity.""" - with self._activity_lock: - return time.monotonic() - self._last_activity_monotonic - def _handle_auth_error(self, response: "requests.Response") -> bool: """ Handle authentication error (401) and attempt recovery. From 4dbc7fe699b6bba9d3bde6a085e58665907cd408 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Wed, 8 Apr 2026 17:36:52 +0530 Subject: [PATCH 25/27] fix lint --- docs/11-persistent-manager-idle-timeout.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/11-persistent-manager-idle-timeout.md b/docs/11-persistent-manager-idle-timeout.md index 82a13807..bab98020 100644 --- a/docs/11-persistent-manager-idle-timeout.md +++ b/docs/11-persistent-manager-idle-timeout.md @@ -37,7 +37,7 @@ The following problems are explicitly covered by code and tests. ### 1. Single, unambiguous variable name -**Issue:** Multiple overlapping names suggested different semantics (“gateway” vs “platform manager”) and made docs and inventory hard to reason about. +**Issue:** Multiple overlapping names suggested different semantics ("gateway" vs "platform manager") and made docs and inventory hard to reason about. **Behavior:** Only `persistent_manager_idle_timeout` is documented and parsed. Misnamed keys do not configure idle timeout. @@ -47,7 +47,7 @@ The following problems are explicitly covered by code and tests. ### 2. Preserving `0` (no accidental fallback) -**Issue:** Patterns such as `task.get("x") or host.get("x")` treat `**0`** as falsy and incorrectly fall back to a default or host value, so “disable idle shutdown” could not be expressed reliably. +**Issue:** Patterns such as `task.get("x") or host.get("x")` treat `**0`** as falsy and incorrectly fall back to a default or host value, so "disable idle shutdown" could not be expressed reliably. **Behavior:** Extraction checks membership (`"persistent_manager_idle_timeout" in task_args`) before reading the value, so `**0`** is preserved. @@ -89,7 +89,7 @@ The following problems are explicitly covered by code and tests. --- -### 6. Expired token does not “freeze” idle exit +### 6. Expired token does not "freeze" idle exit **Issue:** If the token expires while there is no traffic, the manager should still exit after the idle interval. @@ -101,7 +101,7 @@ The following problems are explicitly covered by code and tests. ### 7. User-facing requests reset the idle clock (including failures) -**Issue:** Activity should reflect “something tried to use the gateway,” including failed HTTP calls where the client still did work. +**Issue:** Activity should reflect "something tried to use the gateway," including failed HTTP calls where the client still did work. **Behavior:** `record_activity()` runs at the start of the request path (before the HTTP call), so a **401** still resets the idle timer for that attempt. From 1124f6360d1f3cfda6ff08b7265b880415182a4d Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Wed, 8 Apr 2026 17:48:19 +0530 Subject: [PATCH 26/27] migration guide --- docs/12-migration-gateway-connection.md | 157 ++++++++++++++++++++++++ docs/README.md | 8 +- 2 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 docs/12-migration-gateway-connection.md diff --git a/docs/12-migration-gateway-connection.md b/docs/12-migration-gateway-connection.md new file mode 100644 index 00000000..4e75fd33 --- /dev/null +++ b/docs/12-migration-gateway-connection.md @@ -0,0 +1,157 @@ +# Migration guide: Gateway HTTP connection (AAP 2.7 / `ansible.platform`) + +This guide helps you move playbooks and inventory to the supported **HTTP connection plugin** for Ansible Automation Platform Gateway traffic, configure **credentials** and **connection behavior**, and choose **persistent** versus **direct** mode. + +Confirm the `ansible.platform` collection version and Red Hat release notes for your environment; details here apply to automation targeting **Ansible Automation Platform 2.7** and the collection version bundled or pinned with that release. + +--- + +## What changed (for users) + +- The collection recommends **`ansible_connection: ansible.platform.http`** for hosts that represent the Gateway API endpoint. That connection type supports **direct** mode (default, new HTTP work per task) and **persistent** mode (reuse across tasks in a play for fewer round-trips). +- Existing playbooks that use **`connection: local`** (or default local) and pass Gateway options on tasks **continue to work** for modules that ship with a matching action plugin. You can migrate inventories gradually. + +--- + +## Before and after + +### Before: local connection and task-level API variables + +Typical pattern: run against `localhost` with **`ansible_connection: local`** (or implicit local) and pass hostname and auth on each task using `aap_*` names (or equivalent module parameters). + +```yaml +# inventory.yml — before +all: + hosts: + localhost: + ansible_connection: local + +--- +# playbook.yml — before +- name: Manage AAP resources + hosts: localhost + gather_facts: false + tasks: + - name: Example task + ansible.platform.organization: + name: Ansible + state: present + aap_hostname: https://gateway.example.com + aap_username: admin + aap_password: "{{ vault_aap_password }}" +``` + +### After: Gateway host with the platform HTTP connection plugin + +Recommended pattern: define an inventory host (or group) for the Gateway, set **`ansible_connection: ansible.platform.http`**, put **stable** settings in **inventory or group_vars** (`gateway_*` / `aap_*` as supported by the modules), and keep task bodies focused on resource arguments. + +```yaml +# inventory.yml — after +all: + children: + gateway: + hosts: + aap_gateway: + ansible_host: gateway.example.com + ansible_connection: ansible.platform.http + # Optional: reuse one client across tasks in the play (see "Persistent vs direct") + ansible_platform_use_persistent_connection: true + +--- +# group_vars/gateway.yml — after (example) +gateway_url: https://gateway.example.com +gateway_username: admin +gateway_password: "{{ vault_gateway_password }}" +gateway_validate_certs: true + +--- +# playbook.yml — after +- name: Manage AAP resources + hosts: aap_gateway + gather_facts: false + tasks: + - name: Example task + ansible.platform.organization: + name: Engineering + state: present +``` + +You can mix styles during migration (for example, keep `localhost` + `local` in one playbook and use `ansible.platform.http` in another) while you validate **Automation Controller** job templates and credentials. + +--- + +## Configure credentials + +Supply Gateway **base URL** and **authentication** using variables that your modules accept. Common names (see individual module documentation for the full list and aliases): + +| What you need | Typical variables | +|---------------|-------------------| +| Gateway URL | `gateway_url` or `gateway_hostname` | +| Username / password | `gateway_username` / `gateway_password`, or `aap_username` / `aap_password` | +| OAuth token | `gateway_token` or `aap_token` (per module docs) | +| TLS / HTTP | `gateway_validate_certs`, `gateway_request_timeout` (and `aap_*` aliases where documented) | + +**Automation Controller:** map **credentials** or **extra variables** so these keys are available to the playbook for the host that runs the Gateway tasks (either the dedicated Gateway inventory host or `localhost`, depending on your layout). Align credential injectors with the variable names your playbooks use (`gateway_*` and/or `aap_*`). + +--- + +## Connection settings: persistent vs direct + +| Mode | When to use | How users enable it | +|------|-------------|---------------------| +| **Direct** (default) | Simple playbooks, or when you want each task to use a fresh client | Use `ansible.platform.http` with persistent mode **off** (default). | +| **Persistent** | Multiple tasks against the same Gateway in one play; fewer TLS/auth round-trips | Turn persistent mode **on** using one of the options below. | + +You can enable persistent mode in several equivalent ways (use whichever fits your Ansible config): + +1. Connection option **`persistent: true`** for the `ansible.platform.http` plugin (see the plugin’s documentation under `ansible-doc -t connection ansible.platform.http`). +2. Host or task variable **`ansible_platform_use_persistent_connection: true`**. +3. Host or task variable **`ansible_platform_persistent: true`**. +4. Environment variable **`ANSIBLE_PLATFORM_PERSISTENT`**. +5. Ansible INI: **`[platform_connection]`** section, key **`persistent`** (see plugin documentation). + +If none of these set persistent mode, behavior defaults to **direct** (non-persistent). + +Truthy values are accepted as boolean `true` or common string forms such as `yes` / `true` / `1` (see the connection plugin documentation). + +**Changing** Gateway URL or credentials **changes** which logical connection is used; do not expect reuse across different URLs or identities. + +--- + +## Inventory and playbook parameters + +**Inventory** + +- Set **`ansible_connection: ansible.platform.http`** on the host (or group) that should use the Gateway connection plugin. +- Set **`ansible_host`** to a hostname or address suitable for your environment (the API target is still defined by `gateway_url` / `gateway_hostname` variables). +- Optional: set **`ansible_platform_use_persistent_connection`** (or **`ansible_platform_persistent`**) per host or group. + +**Playbook / role variables** + +- Prefer **group_vars**, **host_vars**, or **vars_files** for URL, credentials, TLS, and timeouts so job templates and vault stay consistent. +- Task parameters can still override or supplement variables when the module allows it—follow each module’s documentation. + +**Backward compatibility** + +| Configuration | Persistent reuse across tasks in a play? | +|---------------|------------------------------------------| +| `ansible_connection: ansible.platform.http` and persistent **off** (default) | No | +| `ansible_connection: ansible.platform.http` and persistent **on** | Yes | +| `ansible_connection: local` (typical legacy) | Not via the HTTP connection plugin; behavior matches your current collection version | + +--- + +## Migration checklist + +- [ ] Add an inventory host (or group) for the Gateway and set **`ansible_connection: ansible.platform.http`**. +- [ ] Move or duplicate **`gateway_url`** / **`gateway_hostname`** and auth variables into inventory, **`group_vars`**, or Controller extra vars. +- [ ] Choose **persistent** vs **direct** and set **`ansible_platform_use_persistent_connection`** (or another supported switch) accordingly. +- [ ] Align **Automation Controller** credentials and injectors with the variable names your playbooks expect. +- [ ] Run your playbooks in a non-production environment and confirm results match the pre-migration behavior. + +--- + +## Further reading + +- [03-sdk-architecture.md](03-sdk-architecture.md) — architecture overview (persistent connection and manager lifecycle). +- [06-foundation-components.md](06-foundation-components.md) — framework components (for contributors and advanced troubleshooting). diff --git a/docs/README.md b/docs/README.md index cd67fbaf..a9c17b1a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -21,6 +21,7 @@ so developers familiar with that collection find the same patterns and numbering | 09 | [09-agent-collaboration.md](09-agent-collaboration.md) | AI agents | Personas, phase-by-phase guidance, coding standards, human-in-the-loop triggers, troubleshooting | | 10 | [10-case-study-aap-platform.md](10-case-study-aap-platform.md) | Feature devs | Module map, identity categories, known API quirks, implementation roadmap | | 11 | [11-persistent-manager-idle-timeout.md](11-persistent-manager-idle-timeout.md) | Framework devs / operators | Persistent manager idle timeout: config, semantics, edge cases, tests | +| 12 | [12-migration-gateway-connection.md](12-migration-gateway-connection.md) | Operators / playbook authors | Migrate to `ansible.platform.http`: before/after, credentials, connection and inventory settings | --- @@ -49,6 +50,9 @@ so developers familiar with that collection find the same patterns and numbering ### "I need to understand persistent manager idle timeout behavior" → [11-persistent-manager-idle-timeout.md](11-persistent-manager-idle-timeout.md) +### "I'm migrating to the Gateway HTTP connection plugin (AAP 2.7)" +→ [12-migration-gateway-connection.md](12-migration-gateway-connection.md) + --- ## Document Dependency Map @@ -74,7 +78,9 @@ so developers familiar with that collection find the same patterns and numbering │ ├── 09-agent-collaboration (AI agent guidance) │ - └── 10-case-study-aap-platform (module map, API quirks) + ├── 10-case-study-aap-platform (module map, API quirks) + │ + └── 12-migration-gateway-connection (HTTP connection migration for operators) ``` --- From 286ffa9fcbab7136f89216d345092230ba208f89 Mon Sep 17 00:00:00 2001 From: Nikhil Bhasin Date: Wed, 8 Apr 2026 17:58:53 +0530 Subject: [PATCH 27/27] fix lint --- docs/12-migration-gateway-connection.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/12-migration-gateway-connection.md b/docs/12-migration-gateway-connection.md index 4e75fd33..6a67176e 100644 --- a/docs/12-migration-gateway-connection.md +++ b/docs/12-migration-gateway-connection.md @@ -20,14 +20,14 @@ Confirm the `ansible.platform` collection version and Red Hat release notes for Typical pattern: run against `localhost` with **`ansible_connection: local`** (or implicit local) and pass hostname and auth on each task using `aap_*` names (or equivalent module parameters). ```yaml -# inventory.yml — before +# inventory.yml - before all: hosts: localhost: ansible_connection: local --- -# playbook.yml — before +# playbook.yml - before - name: Manage AAP resources hosts: localhost gather_facts: false @@ -46,7 +46,7 @@ all: Recommended pattern: define an inventory host (or group) for the Gateway, set **`ansible_connection: ansible.platform.http`**, put **stable** settings in **inventory or group_vars** (`gateway_*` / `aap_*` as supported by the modules), and keep task bodies focused on resource arguments. ```yaml -# inventory.yml — after +# inventory.yml - after all: children: gateway: @@ -58,14 +58,14 @@ all: ansible_platform_use_persistent_connection: true --- -# group_vars/gateway.yml — after (example) +# group_vars/gateway.yml - after (example) gateway_url: https://gateway.example.com gateway_username: admin gateway_password: "{{ vault_gateway_password }}" gateway_validate_certs: true --- -# playbook.yml — after +# playbook.yml - after - name: Manage AAP resources hosts: aap_gateway gather_facts: false @@ -104,7 +104,7 @@ Supply Gateway **base URL** and **authentication** using variables that your mod You can enable persistent mode in several equivalent ways (use whichever fits your Ansible config): -1. Connection option **`persistent: true`** for the `ansible.platform.http` plugin (see the plugin’s documentation under `ansible-doc -t connection ansible.platform.http`). +1. Connection option **`persistent: true`** for the `ansible.platform.http` plugin (see the plugin's documentation under `ansible-doc -t connection ansible.platform.http`). 2. Host or task variable **`ansible_platform_use_persistent_connection: true`**. 3. Host or task variable **`ansible_platform_persistent: true`**. 4. Environment variable **`ANSIBLE_PLATFORM_PERSISTENT`**. @@ -129,7 +129,7 @@ Truthy values are accepted as boolean `true` or common string forms such as `yes **Playbook / role variables** - Prefer **group_vars**, **host_vars**, or **vars_files** for URL, credentials, TLS, and timeouts so job templates and vault stay consistent. -- Task parameters can still override or supplement variables when the module allows it—follow each module’s documentation. +- Task parameters can still override or supplement variables when the module allows it - follow each module's documentation. **Backward compatibility** @@ -153,5 +153,5 @@ Truthy values are accepted as boolean `true` or common string forms such as `yes ## Further reading -- [03-sdk-architecture.md](03-sdk-architecture.md) — architecture overview (persistent connection and manager lifecycle). -- [06-foundation-components.md](06-foundation-components.md) — framework components (for contributors and advanced troubleshooting). +- [03-sdk-architecture.md](03-sdk-architecture.md) - architecture overview (persistent connection and manager lifecycle). +- [06-foundation-components.md](06-foundation-components.md) - framework components (for contributors and advanced troubleshooting).