sync: Support pluggable remote helpers for smart sync manifest server. Introduce support for pluggable remote helpers (declared via the optional 'helper' attribute in <manifest-server>) to dynamically resolve proxy addresses. Route the XML-RPC manifest server connection through the resolved proxy. Bug: b/517477903 Change-Id: I3b6b8ea2640bb077521df4b4a9e8a34a8c6ecdad Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/591642 Tested-by: Rahul Yadav <yadavrah@google.com> Commit-Queue: Rahul Yadav <yadavrah@google.com> Reviewed-by: Gavin Mak <gavinmak@google.com>
diff --git a/docs/manifest-format.md b/docs/manifest-format.md index 42fb1bf..e128a65 100644 --- a/docs/manifest-format.md +++ b/docs/manifest-format.md
@@ -58,6 +58,7 @@ <!ELEMENT manifest-server EMPTY> <!ATTLIST manifest-server url CDATA #REQUIRED> + <!ATTLIST manifest-server helper CDATA #IMPLIED> <!ELEMENT submanifest EMPTY> <!ATTLIST submanifest name ID #REQUIRED> @@ -239,6 +240,11 @@ See the [smart sync documentation](./smart-sync.md) for more details. +Attribute `url`: The URL of the manifest server. + +Attribute `helper`: Optional name of a remote helper binary to execute to +resolve proxying or authentication for the manifest server. + ### Element submanifest
diff --git a/docs/smart-sync.md b/docs/smart-sync.md index 1769572..9f65945 100644 --- a/docs/smart-sync.md +++ b/docs/smart-sync.md
@@ -27,13 +27,44 @@ element. This is how the client knows what service to talk to. ```xml - <manifest-server url="https://example.com/your/manifest/server/url" /> + <manifest-server url="https://example.com/your/manifest/server/url" + helper="repo-remote-helper-name" /> ``` If the URL starts with `persistent-`, then the [`git-remote-persistent-https` helper](https://github.com/git/git/blob/HEAD/contrib/persistent-https/README) is used to communicate with the server. +### Pluggable Remote Helpers + +For custom proxying or authentication, Repo supports pluggable remote helpers. +You can declare a helper binary via the optional `helper` attribute on the +`<manifest-server>` element. + +If the `helper` attribute is present in `<manifest-server>`: +1. Repo searches your system `PATH` for the specified helper binary (e.g., + `repo-remote-sso`). If the helper cannot be found in `PATH`, Repo will raise + a `SmartSyncError` and abort the sync. +2. The helper is executed with the manifest server URL as its first argument: + ```bash + <helper-binary-name> <url> + ``` +3. The helper must output a single-line JSON object on stdout and exit: + * On success, return `status: "ok"` and a loopback proxy URL (including + scheme, e.g., `http://127.0.0.1:999`): + ```json + {"status": "ok", "message": "http://127.0.0.1:999"} + ``` + * On failure, return `status: "error"` and a detailed error message: + ```json + {"status": "error", "message": "unauthorized"} + ``` +4. Repo routes the XML-RPC request through the returned proxy address. + +**Timeout Constraint**: The remote helper must complete and exit within 10 +seconds. If it hangs or exceeds this limit, Repo will force-terminate (`kill`) +the process and abort the synchronization. + ## Credentials Credentials may be specified directly in typical `username:password`
diff --git a/man/repo-manifest.1 b/man/repo-manifest.1 index 75c9fa9..caa912d 100644 --- a/man/repo-manifest.1 +++ b/man/repo-manifest.1
@@ -1,5 +1,5 @@ .\" DO NOT MODIFY THIS FILE! It was generated by help2man. -.TH REPO "1" "April 2026" "repo manifest" "Repo Manual" +.TH REPO "1" "June 2026" "repo manifest" "Repo Manual" .SH NAME repo \- repo manifest - manual page for repo manifest .SH SYNOPSIS @@ -138,6 +138,7 @@ .IP <!ELEMENT manifest\-server EMPTY> <!ATTLIST manifest\-server url CDATA #REQUIRED> +<!ATTLIST manifest\-server helper CDATA #IMPLIED> .IP <!ELEMENT submanifest EMPTY> <!ATTLIST submanifest name ID #REQUIRED> @@ -347,6 +348,11 @@ .PP See the [smart sync documentation](./smart\-sync.md) for more details. .PP +Attribute `url`: The URL of the manifest server. +.PP +Attribute `helper`: Optional name of a remote helper binary to execute to +resolve proxying or authentication for the manifest server. +.PP Element submanifest .PP One or more submanifest elements may be specified. Each element describes a
diff --git a/manifest_xml.py b/manifest_xml.py index 8f72949..dd00868 100644 --- a/manifest_xml.py +++ b/manifest_xml.py
@@ -651,6 +651,8 @@ if self._manifest_server: e = doc.createElement("manifest-server") e.setAttribute("url", self._manifest_server) + if self._manifest_server_helper: + e.setAttribute("helper", self._manifest_server_helper) root.appendChild(e) root.appendChild(doc.createTextNode("")) @@ -1000,6 +1002,11 @@ return self._manifest_server @property + def manifest_server_helper(self): + self._Load() + return self._manifest_server_helper + + @property def CloneBundle(self): clone_bundle = self.manifestProject.clone_bundle if clone_bundle is None: @@ -1157,6 +1164,7 @@ self._notice = None self.branch = None self._manifest_server = None + self._manifest_server_helper = None def Load(self): """Read the manifest into memory.""" @@ -1426,11 +1434,13 @@ for node in itertools.chain(*node_list): if node.nodeName == "manifest-server": url = self._reqatt(node, "url") + helper = node.getAttribute("helper") or None if self._manifest_server is not None: raise ManifestParseError( "duplicate manifest-server in %s" % (self.manifestFile) ) self._manifest_server = url + self._manifest_server_helper = helper def recursively_add_projects(project): projects = self._projects.setdefault(project.name, [])
diff --git a/subcmds/sync.py b/subcmds/sync.py index 8c53327..2b9cafe 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py
@@ -23,6 +23,8 @@ import optparse import os from pathlib import Path +import shutil +import subprocess import sys import tempfile import time @@ -1738,16 +1740,104 @@ return True - def _SmartSyncSetup(self, opt, smart_sync_manifest_path, manifest): - if not manifest.manifest_server: - raise SmartSyncError( - "error: cannot smart sync: no manifest server defined in " - "manifest" - ) + def _ResolveManifestServerTransport(self, opt, manifest): + """Resolves the manifest server URL and transport. + Returns: + Tuple[str, xmlrpc.client.Transport]: The resolved server URL and + transport. + + Raises: + SmartSyncError: If resolution fails (e.g. helper missing, helper + error, unsupported scheme). + """ manifest_server = manifest.manifest_server - if not opt.quiet: - print("Using manifest server %s" % manifest_server) + helper_binary = manifest.manifest_server_helper + + if helper_binary: + if not shutil.which(helper_binary): + raise SmartSyncError( + f"error: helper binary '{helper_binary}' declared in " + "manifest was not found in your PATH." + ) + + if not opt.quiet: + print(f"Using remote helper {helper_binary}") + + p = None + try: + p = subprocess.Popen( + [helper_binary, manifest_server], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + stdout, stderr = p.communicate(timeout=10) + output = stdout.strip() + stderr_content = stderr.strip() if stderr else "" + except subprocess.TimeoutExpired as e: + err_msg = f"helper {helper_binary} timed out after 10 seconds" + timeout_stderr = e.stderr.strip() if e.stderr else "" + if timeout_stderr: + err_msg += f". Stderr: {timeout_stderr}" + raise SmartSyncError(err_msg) + except OSError as e: + raise SmartSyncError( + "failed to start or communicate with helper " + f"{helper_binary}: {e}" + ) + finally: + if p and p.poll() is None: + p.kill() + p.wait() + + if p.returncode != 0: + err_msg = ( + f"helper {helper_binary} exited with exit code " + f"{p.returncode}." + ) + if stderr_content: + err_msg += f" Stderr: {stderr_content}" + raise SmartSyncError(err_msg) + + try: + res = json.loads(output) + status = res.get("status") + msg = res.get("message") + except json.JSONDecodeError as e: + err_msg = ( + f"failed to parse JSON from helper {helper_binary}: {e}. " + f"Output was: {output}" + ) + if stderr_content: + err_msg += f"\nStderr was: {stderr_content}" + raise SmartSyncError(err_msg) + + if status != "ok": + err_msg = f"helper {helper_binary} returned error: {msg}" + if stderr_content: + err_msg += f"\nStderr was: {stderr_content}" + raise SmartSyncError(err_msg) + + proxy_url = msg + transport = PersistentTransport(manifest_server, proxy=proxy_url) + server_url = manifest_server + if server_url.startswith("persistent-"): + server_url = server_url[len("persistent-") :] + return server_url, transport + + # Fallback path if helper isn't specified + scheme = urllib.parse.urlparse(manifest_server).scheme + if scheme not in ( + "http", + "https", + "persistent-http", + "persistent-https", + ): + raise SmartSyncError( + f"error: unsupported manifest server scheme '{scheme}'." + ) if "@" not in manifest_server: username = None @@ -1782,20 +1872,34 @@ ) transport = PersistentTransport(manifest_server) - if manifest_server.startswith("persistent-"): - manifest_server = manifest_server[len("persistent-") :] + server_url = manifest_server + if server_url.startswith("persistent-"): + server_url = server_url[len("persistent-") :] + + return server_url, transport + + def _SmartSyncSetup(self, opt, smart_sync_manifest_path, manifest): + if not manifest.manifest_server: + raise SmartSyncError( + "error: cannot smart sync: no manifest server defined in " + "manifest" + ) + + if not opt.quiet: + print("Using manifest server %s" % manifest.manifest_server) + + server_url, transport = self._ResolveManifestServerTransport( + opt, manifest + ) # Changes in behavior should update docs/smart-sync.md accordingly. try: - server = xmlrpc.client.Server(manifest_server, transport=transport) + server = xmlrpc.client.Server(server_url, transport=transport) if opt.smart_sync: branch = self._GetBranch(manifest.manifestProject) - + target = None if "SYNC_TARGET" in os.environ: target = os.environ["SYNC_TARGET"] - [success, manifest_str] = server.GetApprovedManifest( - branch, target - ) elif ( "TARGET_PRODUCT" in os.environ and "TARGET_BUILD_VARIANT" in os.environ @@ -1806,9 +1910,6 @@ os.environ["TARGET_RELEASE"], os.environ["TARGET_BUILD_VARIANT"], ) - [success, manifest_str] = server.GetApprovedManifest( - branch, target - ) elif ( "TARGET_PRODUCT" in os.environ and "TARGET_BUILD_VARIANT" in os.environ @@ -1817,6 +1918,8 @@ os.environ["TARGET_PRODUCT"], os.environ["TARGET_BUILD_VARIANT"], ) + + if target: [success, manifest_str] = server.GetApprovedManifest( branch, target ) @@ -1838,25 +1941,34 @@ aggregate_errors=[e], ) self._ReloadManifest(manifest_name, manifest) - else: - raise SmartSyncError( - "error: manifest server RPC call failed: %s" % manifest_str - ) + return manifest_name + + raise SmartSyncError( + "error: manifest server RPC call failed: %s" % manifest_str + ) except (OSError, xmlrpc.client.Fault) as e: + if manifest.manifest_server_helper: + raise SmartSyncError( + "error: failed to communicate with manifest server via " + f"helper: {e}" + ) raise SmartSyncError( "error: cannot connect to manifest server %s:\n%s" % (manifest.manifest_server, e), aggregate_errors=[e], ) except xmlrpc.client.ProtocolError as e: + if manifest.manifest_server_helper: + raise SmartSyncError( + "error: failed to communicate with manifest server via " + f"helper: {e}" + ) raise SmartSyncError( "error: cannot connect to manifest server %s:\n%d %s" % (manifest.manifest_server, e.errcode, e.errmsg), aggregate_errors=[e], ) - return manifest_name - def _UpdateAllManifestProjects(self, opt, mp, manifest_name, errors): """Fetch & update the local manifest project. @@ -3094,14 +3206,15 @@ # request to request like the normal transport, the real url # is passed during initialization. class PersistentTransport(xmlrpc.client.Transport): - def __init__(self, orig_host): + def __init__(self, orig_host, proxy=None): super().__init__() self.orig_host = orig_host + self.proxy = proxy def request(self, host, handler, request_body, verbose=False): with GetUrlCookieFile(self.orig_host, not verbose) as ( cookiefile, - proxy, + cookie_proxy, ): # Python doesn't understand cookies with the #HttpOnly_ prefix # Since we're only using them for HTTP, copy the file temporarily, @@ -3127,10 +3240,11 @@ else: cookiejar = cookielib.CookieJar() + active_proxy = self.proxy or cookie_proxy proxyhandler = urllib.request.ProxyHandler - if proxy: + if active_proxy: proxyhandler = urllib.request.ProxyHandler( - {"http": proxy, "https": proxy} + {"http": active_proxy, "https": active_proxy} ) opener = urllib.request.build_opener( @@ -3143,13 +3257,16 @@ scheme = parse_results.scheme if scheme == "persistent-http": scheme = "http" - if scheme == "persistent-https": + elif scheme == "persistent-https": # If we're proxying through persistent-https, use http. The # proxy itself will do the https. - if proxy: + if active_proxy: scheme = "http" else: scheme = "https" + elif scheme not in ("http", "https"): + if active_proxy: + scheme = "http" # Parse out any authentication information using the base class. host, extra_headers, _ = self.get_host_info(parse_results.netloc)
diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index 70b9684..f858846 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py
@@ -1514,3 +1514,232 @@ ) mock_sync_to_rev.assert_not_called() mock_update_manifest.assert_called_once() + + +class TestSmartSyncSetupRemoteHelper(unittest.TestCase): + """Tests for _SmartSyncSetup with remote helpers.""" + + def setUp(self): + self.cmd = sync.Sync() + self.opt = mock.MagicMock() + self.opt.quiet = False + self.opt.smart_sync = True + self.manifest = mock.MagicMock() + self.smart_sync_manifest_path = "/fake/path/to/manifest.xml" + + @mock.patch("shutil.which") + @mock.patch("subprocess.Popen") + @mock.patch("xmlrpc.client.Server") + @mock.patch("subcmds.sync.PersistentTransport") + def test_smart_sync_setup_with_helper( + self, mock_transport_class, mock_server_class, mock_popen, mock_which + ): + """Test _SmartSyncSetup when a helper is present and succeeds.""" + import subprocess + + self.manifest.manifest_server = ( + "persistent-https://android-smartsync.corp.google.com/" + "manifestserver" + ) + self.manifest.manifest_server_helper = "repo-remote-sso" + mock_which.return_value = "/fake/bin/repo-remote-sso" + + # Mock subprocess to return a JSON with status ok and proxy address + mock_process = mock.MagicMock() + mock_process.communicate.return_value = ( + '{"status":"ok","message":"http://127.0.0.1:999"}\n', + "", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + # Mock XML-RPC server call + mock_server = mock.MagicMock() + mock_server.GetApprovedManifest.return_value = [ + True, + "<manifest></manifest>", + ] + mock_server_class.return_value = mock_server + + # Mock manifest project branch + self.cmd._GetBranch = mock.MagicMock(return_value="main") + self.cmd._ReloadManifest = mock.MagicMock() + + # Mock open to avoid writing to disk + with mock.patch("builtins.open", mock.mock_open()): + manifest_name = self.cmd._SmartSyncSetup( + self.opt, self.smart_sync_manifest_path, self.manifest + ) + + # Assertions + mock_which.assert_called_once_with("repo-remote-sso") + mock_popen.assert_called_once_with( + ["repo-remote-sso", self.manifest.manifest_server], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + + # Verify transport was created with the proxy returned by helper (with + # http:// prepended) + mock_transport_class.assert_called_once_with( + self.manifest.manifest_server, proxy="http://127.0.0.1:999" + ) + + # Verify Server was created with the same URL, with persistent- stripped + mock_server_class.assert_called_once_with( + "https://android-smartsync.corp.google.com/manifestserver", + transport=mock_transport_class.return_value, + ) + + self.assertEqual(manifest_name, "manifest.xml") + + @mock.patch("shutil.which") + @mock.patch("subprocess.Popen") + def test_smart_sync_setup_helper_error(self, mock_popen, mock_which): + """Test _SmartSyncSetup when helper returns an error status.""" + self.manifest.manifest_server = ( + "http://android-smartsync.corp.google.com/manifestserver" + ) + self.manifest.manifest_server_helper = "repo-remote-sso" + mock_which.return_value = "/fake/bin/repo-remote-sso" + + # Mock subprocess to return a JSON with status error + mock_process = mock.MagicMock() + mock_process.communicate.return_value = ( + '{"status":"error","message":"uplink-helper failed"}\n', + "", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + with self.assertRaises(sync.SmartSyncError) as context: + self.cmd._SmartSyncSetup( + self.opt, self.smart_sync_manifest_path, self.manifest + ) + + self.assertIn( + "helper repo-remote-sso returned error: uplink-helper failed", + str(context.exception), + ) + + @mock.patch("shutil.which") + def test_smart_sync_setup_missing_declared_helper(self, mock_which): + """Test _SmartSyncSetup when helper declared in manifest is missing.""" + self.manifest.manifest_server = ( + "http://android-smartsync.corp.google.com/manifestserver" + ) + self.manifest.manifest_server_helper = "repo-remote-sso" + mock_which.return_value = None + + with self.assertRaises(sync.SmartSyncError) as context: + self.cmd._SmartSyncSetup( + self.opt, self.smart_sync_manifest_path, self.manifest + ) + + self.assertIn( + "helper binary 'repo-remote-sso' declared in manifest was not " + "found", + str(context.exception), + ) + + @mock.patch("shutil.which") + @mock.patch("subprocess.Popen") + def test_smart_sync_setup_helper_exit_code_error( + self, mock_popen, mock_which + ): + """Test _SmartSyncSetup when helper exits with non-zero and stderr.""" + self.manifest.manifest_server = ( + "http://android-smartsync.corp.google.com/manifestserver" + ) + self.manifest.manifest_server_helper = "repo-remote-sso" + mock_which.return_value = "/fake/bin/repo-remote-sso" + + # Mock subprocess: exit code 1, stderr, and no JSON on stdout + mock_process = mock.MagicMock() + mock_process.communicate.return_value = ( + "", + "internal binary error occurred\n", + ) + mock_process.returncode = 1 + mock_popen.return_value = mock_process + + with self.assertRaises(sync.SmartSyncError) as context: + self.cmd._SmartSyncSetup( + self.opt, self.smart_sync_manifest_path, self.manifest + ) + + self.assertIn( + "helper repo-remote-sso exited with exit code 1. " + "Stderr: internal binary error occurred", + str(context.exception), + ) + + @mock.patch("shutil.which") + @mock.patch("subprocess.Popen") + def test_smart_sync_setup_helper_json_decode_error_with_stderr( + self, mock_popen, mock_which + ): + """Test _SmartSyncSetup when helper returns invalid JSON and stderr.""" + self.manifest.manifest_server = ( + "http://android-smartsync.corp.google.com/manifestserver" + ) + self.manifest.manifest_server_helper = "repo-remote-sso" + mock_which.return_value = "/fake/bin/repo-remote-sso" + + mock_process = mock.MagicMock() + mock_process.communicate.return_value = ( + "not a json", + "some warning messages\n", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + with self.assertRaises(sync.SmartSyncError) as context: + self.cmd._SmartSyncSetup( + self.opt, self.smart_sync_manifest_path, self.manifest + ) + + self.assertIn( + "failed to parse JSON from helper repo-remote-sso", + str(context.exception), + ) + self.assertIn( + "Stderr was: some warning messages", + str(context.exception), + ) + + @mock.patch("shutil.which") + @mock.patch("subprocess.Popen") + def test_smart_sync_setup_helper_error_with_stderr( + self, mock_popen, mock_which + ): + """Test _SmartSyncSetup when helper returns error status and stderr.""" + self.manifest.manifest_server = ( + "http://android-smartsync.corp.google.com/manifestserver" + ) + self.manifest.manifest_server_helper = "repo-remote-sso" + mock_which.return_value = "/fake/bin/repo-remote-sso" + + mock_process = mock.MagicMock() + mock_process.communicate.return_value = ( + '{"status":"error","message":"uplink-helper failed"}\n', + "debugging logs\n", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + with self.assertRaises(sync.SmartSyncError) as context: + self.cmd._SmartSyncSetup( + self.opt, self.smart_sync_manifest_path, self.manifest + ) + + self.assertIn( + "helper repo-remote-sso returned error: uplink-helper failed", + str(context.exception), + ) + self.assertIn( + "Stderr was: debugging logs", + str(context.exception), + )