From ca64050f6afa617d3cfefe3ceb9d2a3673f3e1a8 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Tue, 1 Apr 2025 13:23:28 -0700 Subject: [PATCH] config:url_fetch_method: allow curl args (#49712) Signed-off-by: Gregory Becker --- lib/spack/docs/config_yaml.rst | 7 ++++--- lib/spack/spack/fetch_strategy.py | 9 +++++---- lib/spack/spack/schema/config.py | 2 +- lib/spack/spack/test/url_fetch.py | 20 ++++++++++++++++++++ lib/spack/spack/util/web.py | 11 +++++------ 5 files changed, 35 insertions(+), 14 deletions(-) diff --git a/lib/spack/docs/config_yaml.rst b/lib/spack/docs/config_yaml.rst index e98fc55c5e4..54c659caf29 100644 --- a/lib/spack/docs/config_yaml.rst +++ b/lib/spack/docs/config_yaml.rst @@ -148,15 +148,16 @@ this can expose you to attacks. Use at your own risk. ``ssl_certs`` -------------------- -Path to custom certificats for SSL verification. The value can be a +Path to custom certificats for SSL verification. The value can be a filesytem path, or an environment variable that expands to an absolute file path. The default value is set to the environment variable ``SSL_CERT_FILE`` to use the same syntax used by many other applications that automatically detect custom certificates. When ``url_fetch_method:curl`` the ``config:ssl_certs`` should resolve to a single file. Spack will then set the environment variable ``CURL_CA_BUNDLE`` -in the subprocess calling ``curl``. -If ``url_fetch_method:urllib`` then files and directories are supported i.e. +in the subprocess calling ``curl``. If additional ``curl`` arguments are required, +they can be set in the config, e.g. ``url_fetch_method:'curl -k -q'``. +If ``url_fetch_method:urllib`` then files and directories are supported i.e. ``config:ssl_certs:$SSL_CERT_FILE`` or ``config:ssl_certs:$SSL_CERT_DIR`` will work. In all cases the expanded path must be absolute for Spack to use the certificates. diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index a7cfaaf2930..dd09a6d3dd1 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -295,8 +295,9 @@ def fetch(self): ) def _fetch_from_url(self, url): - if spack.config.get("config:url_fetch_method") == "curl": - return self._fetch_curl(url) + fetch_method = spack.config.get("config:url_fetch_method", "urllib") + if fetch_method.startswith("curl"): + return self._fetch_curl(url, config_args=fetch_method.split()[1:]) else: return self._fetch_urllib(url) @@ -345,7 +346,7 @@ def _fetch_urllib(self, url): self._check_headers(str(response.headers)) @_needs_stage - def _fetch_curl(self, url): + def _fetch_curl(self, url, config_args=[]): save_file = None partial_file = None if self.stage.save_filename: @@ -374,7 +375,7 @@ def _fetch_curl(self, url): timeout = self.extra_options.get("timeout") base_args = web_util.base_curl_fetch_args(url, timeout) - curl_args = save_args + base_args + cookie_args + curl_args = config_args + save_args + base_args + cookie_args # Run curl but grab the mime type from the http headers curl = self.curl diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py index 8c35b80e5f7..b1f9a2e9bd0 100644 --- a/lib/spack/spack/schema/config.py +++ b/lib/spack/spack/schema/config.py @@ -100,7 +100,7 @@ "allow_sgid": {"type": "boolean"}, "install_status": {"type": "boolean"}, "binary_index_root": {"type": "string"}, - "url_fetch_method": {"type": "string", "enum": ["urllib", "curl"]}, + "url_fetch_method": {"type": "string", "pattern": r"^urllib$|^curl( .*)*"}, "additional_external_search_paths": {"type": "array", "items": {"type": "string"}}, "binary_index_ttl": {"type": "integer", "minimum": 0}, "aliases": {"type": "object", "patternProperties": {r"\w[\w-]*": {"type": "string"}}}, diff --git a/lib/spack/spack/test/url_fetch.py b/lib/spack/spack/test/url_fetch.py index f186a7b5a06..9a205e074c6 100644 --- a/lib/spack/spack/test/url_fetch.py +++ b/lib/spack/spack/test/url_fetch.py @@ -109,6 +109,26 @@ def test_fetch_options(tmp_path, mock_archive): assert filecmp.cmp(fetcher.archive_file, mock_archive.archive_file) +def test_fetch_curl_options(tmp_path, mock_archive, monkeypatch): + with spack.config.override("config:url_fetch_method", "curl -k -q"): + fetcher = fs.URLFetchStrategy( + url=mock_archive.url, fetch_options={"cookie": "True", "timeout": 10} + ) + + def check_args(*args, **kwargs): + # Raise StopIteration to avoid running the rest of the fetch method + # args[0] is `which curl`, next two are our config options + assert args[1:3] == ("-k", "-q") + raise StopIteration + + monkeypatch.setattr(type(fetcher.curl), "__call__", check_args) + + with Stage(fetcher, path=str(tmp_path)): + assert fetcher.archive_file is None + with pytest.raises(StopIteration): + fetcher.fetch() + + @pytest.mark.parametrize("_fetch_method", ["curl", "urllib"]) def test_archive_file_errors(tmp_path, mutable_config, mock_archive, _fetch_method): """Ensure FetchStrategy commands may only be used as intended""" diff --git a/lib/spack/spack/util/web.py b/lib/spack/spack/util/web.py index c1c58c02731..5d094640c30 100644 --- a/lib/spack/spack/util/web.py +++ b/lib/spack/spack/util/web.py @@ -388,9 +388,9 @@ def fetch_url_text(url, curl: Optional[Executable] = None, dest_dir="."): fetch_method = spack.config.get("config:url_fetch_method") tty.debug("Using '{0}' to fetch {1} into {2}".format(fetch_method, url, path)) - if fetch_method == "curl": + if fetch_method.startswith("curl"): curl_exe = curl or require_curl() - curl_args = ["-O"] + curl_args = fetch_method.split()[1:] + ["-O"] curl_args.extend(base_curl_fetch_args(url)) # Curl automatically downloads file contents as filename @@ -436,15 +436,14 @@ def url_exists(url, curl=None): url_result = urllib.parse.urlparse(url) # Use curl if configured to do so - use_curl = spack.config.get( - "config:url_fetch_method", "urllib" - ) == "curl" and url_result.scheme not in ("gs", "s3") + fetch_method = spack.config.get("config:url_fetch_method", "urllib") + use_curl = fetch_method.startswith("curl") and url_result.scheme not in ("gs", "s3") if use_curl: curl_exe = curl or require_curl() # Telling curl to fetch the first byte (-r 0-0) is supposed to be # portable. - curl_args = ["--stderr", "-", "-s", "-f", "-r", "0-0", url] + curl_args = fetch_method.split()[1:] + ["--stderr", "-", "-s", "-f", "-r", "0-0", url] if not spack.config.get("config:verify_ssl"): curl_args.append("-k") _ = curl_exe(*curl_args, fail_on_error=False, output=os.devnull)