SNOW-873456 config file slicing (#1660)

Author: sfc-gh-mkeller

DESCRIPTION.md

@@ -9,15 +9,14 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
 # Release Notes
 
 - v3.0.5(TBD)
-  - Added a feature that lets you add connection definitions to the `config.toml` configuration file. A connection definition refers to a collection of connection parameters. The connection configuration name must begin with **connections**, similar to the following that defines the parameters for the `prod` connection:
-
+  - Added a feature that lets you add connection definitions to the `connections.toml` configuration file. A connection definition refers to a collection of connection parameters, for example, if you wanted to define a connection named `prod``:
     ```toml
-    [connections.prod]
+    [prod]
     account = "my_account"
     user = "my_user"
     password = "my_password"
     ```
-    By default, we look for the `config.toml` file in the location specified in the `SNOWFLAKE_HOME` environment variable (default: `~/.snowflake`). If this folder does not exist, the Python connector looks for the file in the `platformdirs` location, as follows:
+    By default, we look for the `connections.toml` file in the location specified in the `SNOWFLAKE_HOME` environment variable (default: `~/.snowflake`). If this folder does not exist, the Python connector looks for the file in the [platformdirs](https://github.com/platformdirs/platformdirs/blob/main/README.rst) location, as follows:
 
     - On Linux: `~/.config/snowflake/`,  but follows XDG settings
     - On Mac: `~/Library/Application Support/snowflake/`
@@ -26,7 +25,7 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
     You can determine which file is used by running the following command:
 
     ```
-    python -c "from snowflake.connector.constants import CONFIG_FILE; print(str(CONFIG_FILE))"
+    python -c "from snowflake.connector.constants import CONNECTIONS_FILE; print(str(CONNECTIONS_FILE))"
     ```
   - Bumped cryptography dependency from <41.0.0,>=3.1.0 to >=3.1.0,<42.0.0.
   - Improved OCSP response caching to remove tmp cache files on Windows.

src/snowflake/connector/config_manager.py

@@ -4,27 +4,45 @@
 
 from __future__ import annotations
 
+import itertools
 import logging
 import os
 import stat
 from collections.abc import Iterable
 from operator import methodcaller
 from pathlib import Path
-from typing import Any, Callable, Literal, TypeVar
+from typing import Any, Callable, Literal, NamedTuple, TypeVar
 from warnings import warn
 
 import tomlkit
 from tomlkit.items import Table
 
-from snowflake.connector.constants import CONFIG_FILE
-from snowflake.connector.errors import ConfigManagerError, ConfigSourceError
+from snowflake.connector.constants import CONFIG_FILE, CONNECTIONS_FILE
+from snowflake.connector.errors import (
+    ConfigManagerError,
+    ConfigSourceError,
+    MissingConfigOptionError,
+)
 
 _T = TypeVar("_T")
 
 LOGGER = logging.getLogger(__name__)
 READABLE_BY_OTHERS = stat.S_IRGRP | stat.S_IROTH
 
 
+class ConfigSliceOptions(NamedTuple):
+    """Class that defines settings individual configuration files."""
+
+    check_permissions: bool = True
+    only_in_slice: bool = False
+
+
+class ConfigSlice(NamedTuple):
+    path: Path
+    options: ConfigSliceOptions
+    section: str
+
+
 class ConfigOption:
     """ConfigOption represents a flag/setting.
 
@@ -138,10 +156,9 @@ def _get_env(self) -> tuple[bool, str | _T | None]:
 
     def _get_config(self) -> Any:
         """Get value from the cached config file."""
-        if self._root_manager.conf_file_cache is None and (
-            self._root_manager.file_path is not None
-            and self._root_manager.file_path.exists()
-            and self._root_manager.file_path.is_file()
+        if (
+            self._root_manager.conf_file_cache is None
+            and self._root_manager.file_path is not None
         ):
             self._root_manager.read_config()
         e = self._root_manager.conf_file_cache
@@ -150,7 +167,15 @@ def _get_config(self) -> Any:
                 f"Root parser '{self._root_manager.name}' is missing file_path",
             )
         for k in self._nest_path[1:]:
-            e = e[k]
+            try:
+                e = e[k]
+            except tomlkit.exceptions.NonExistentKey:
+                raise MissingConfigOptionError(  # TOOO: maybe a child Exception for missing option?
+                    f"Configuration option '{self.option_name}' is not defined anywhere, "
+                    "have you forgotten to set it in a configuration file, "
+                    "or environmental variable?"
+                )
+
         if isinstance(e, (Table, tomlkit.TOMLDocument)):
             # If we got a TOML table we probably want it in dictionary form
             return e.value
@@ -190,6 +215,7 @@ def __init__(
         *,
         name: str,
         file_path: Path | None = None,
+        _slices: list[ConfigSlice] | None = None,
     ):
         """Create a new ConfigManager.
 
@@ -198,8 +224,11 @@ def __init__(
             file_path: File this parser should read values from. Can be omitted
               for all child parsers.
         """
+        if _slices is None:
+            _slices = list()
         self.name = name
         self.file_path = file_path
+        self._slices = _slices
         # Objects holding subparsers and options
         self._options: dict[str, ConfigOption] = dict()
         self._sub_parsers: dict[str, ConfigManager] = dict()
@@ -225,29 +254,42 @@ def read_config(
                 "ConfigManager is trying to read config file, but it doesn't "
                 "have one"
             )
-        if not self.file_path.exists():
-            raise ConfigSourceError(
-                f"The config file '{self.file_path}' does not exist"
-            )
-        if (
-            # Same check as openssh does for permissions
-            #  https://github.com/openssh/openssh-portable/blob/2709809fd616a0991dc18e3a58dea10fb383c3f0/readconf.c#LL2264C1-L2264C1
-            self.file_path.stat().st_mode & READABLE_BY_OTHERS != 0
-            or (
-                # TODO: Windows doesn't have getuid, skip checking
-                hasattr(os, "getuid")
-                and self.file_path.stat().st_uid != 0
-                and self.file_path.stat().st_uid != os.getuid()
-            )
+        read_config_file = tomlkit.TOMLDocument()
+
+        # Read in all of the config slices
+        for filep, sliceoptions, section in itertools.chain(
+            ((self.file_path, ConfigSliceOptions(), None),),
+            self._slices,
         ):
-            warn(f"Bad owner or permissions on {self.file_path}")
-        LOGGER.debug(f"reading configuration file from {str(self.file_path)}")
-        try:
-            self.conf_file_cache = tomlkit.parse(self.file_path.read_text())
-        except Exception as e:
-            raise ConfigSourceError(
-                "An unknown error happened while loading " f"'{str(self.file_path)}'"
-            ) from e
+            if sliceoptions.only_in_slice:
+                del read_config_file[section]
+            if not filep.exists():
+                continue
+            if (
+                sliceoptions.check_permissions  # Skip checking if this file couldn't hold sensitive information
+                # Same check as openssh does for permissions
+                #  https://github.com/openssh/openssh-portable/blob/2709809fd616a0991dc18e3a58dea10fb383c3f0/readconf.c#LL2264C1-L2264C1
+                and filep.stat().st_mode & READABLE_BY_OTHERS != 0
+                or (
+                    # Windows doesn't have getuid, skip checking
+                    hasattr(os, "getuid")
+                    and filep.stat().st_uid != 0
+                    and filep.stat().st_uid != os.getuid()
+                )
+            ):
+                warn(f"Bad owner or permissions on {str(filep)}")
+            LOGGER.debug(f"reading configuration file from {str(filep)}")
+            try:
+                read_config_piece = tomlkit.parse(filep.read_text())
+            except Exception as e:
+                raise ConfigSourceError(
+                    "An unknown error happened while loading " f"'{str(filep)}'"
+                ) from e
+            if section is None:
+                read_config_file = read_config_piece
+            else:
+                read_config_file[section] = read_config_piece
+        self.conf_file_cache = read_config_file
 
     def add_option(
         self,
@@ -331,6 +373,15 @@ def __getitem__(self, name: str) -> ConfigOption | ConfigManager:
 CONFIG_PARSER = ConfigManager(
     name="CONFIG_PARSER",
     file_path=CONFIG_FILE,
+    _slices=[
+        ConfigSlice(  # Optional connections file to read in connections from
+            CONNECTIONS_FILE,
+            ConfigSliceOptions(
+                check_permissions=True,  # connections could live here, check permissions
+            ),
+            "connections",
+        ),
+    ],
 )
 CONFIG_PARSER.add_option(
     name="connections",

src/snowflake/connector/connection.py

@@ -297,7 +297,7 @@ class SnowflakeConnection:
     def __init__(
         self,
         connection_name: str | None = None,
-        config_file_path: pathlib.Path | None = None,
+        connections_file_path: pathlib.Path | None = None,
         **kwargs,
     ) -> None:
         self._lock_sequence_counter = Lock()
@@ -332,10 +332,13 @@ def __init__(
         self.converter = None
         self.query_context_cache: QueryContextCache | None = None
         self.query_context_cache_size = 5
-        if config_file_path is not None:
+        if connections_file_path is not None:
             # Change config file path and force update cache
-            CONFIG_PARSER.file_path = config_file_path
-            CONFIG_PARSER.read_config()
+            for i, s in enumerate(CONFIG_PARSER._slices):
+                if s.section == "connections":
+                    CONFIG_PARSER._slices[i] = s._replace(path=connections_file_path)
+                    CONFIG_PARSER.read_config()
+                    break
         if connection_name is not None:
             connections = CONFIG_PARSER["connections"]
             if connection_name not in connections:
@@ -366,7 +369,8 @@ def ocsp_fail_open(self) -> bool:
         return self._ocsp_fail_open
 
     def _ocsp_mode(self) -> OCSPMode:
-        """OCSP mode. INSECURE, FAIL_OPEN or FAIL_CLOSED."""
+        """OCSP mode. INSEC
+        URE, FAIL_OPEN or FAIL_CLOSED."""
         if self.insecure_mode:
             return OCSPMode.INSECURE
         elif self.ocsp_fail_open:

src/snowflake/connector/constants.py

@@ -21,11 +21,12 @@
 # defaults. Please see comments in sf_dir.py for more information.
 DIRS = _resolve_platform_dirs()
 
-# Snowflake's central configuration file. By default, platformdirs will resolve
+# Snowflake's configuration files. By default, platformdirs will resolve
 # them to these places depending on OS:
-#   * Linux: `~/.config/snowflake/config.toml` but can be updated with XDG vars
-#   * Windows: `%USERPROFILE%\AppData\Local\snowflake\config.toml`
-#   * Mac: `~/Library/Application Support/snowflake/config.toml`
+#   * Linux: `~/.config/snowflake/filename` but can be updated with XDG vars
+#   * Windows: `%USERPROFILE%\AppData\Local\snowflake\filename`
+#   * Mac: `~/Library/Application Support/snowflake/filename`
+CONNECTIONS_FILE = DIRS.user_config_path / "connections.toml"
 CONFIG_FILE = DIRS.user_config_path / "config.toml"
 
 DBAPI_TYPE_STRING = 0

src/snowflake/connector/errors.py

@@ -612,6 +612,13 @@ class ConfigSourceError(Error):
     """
 
 
+class MissingConfigOptionError(ConfigSourceError):
+    """When a configuration option is missing from the final, resolved configurations.
+
+    This is a special-case of ConfigSourceError.
+    """
+
+
 class ConfigManagerError(Error):
     """Configuration parser related errors.
 

test/integ/test_connection.py

@@ -1231,18 +1231,16 @@ def test_connection_name_loading(monkeypatch, db_parameters, tmp_path, mode):
         # If anything unexpected fails here, don't want to expose password
         for k, v in db_parameters.items():
             default_con[k] = v
+        doc["default"] = default_con
         with monkeypatch.context() as m:
             if mode == "env":
-                doc["default"] = default_con
                 m.setenv("SF_CONNECTIONS", tomlkit.dumps(doc))
             else:
-                doc["connections"] = tomlkit.table()
-                doc["connections"]["default"] = default_con
-                tmp_config_file = tmp_path / "config.toml"
+                tmp_config_file = tmp_path / "connections.toml"
                 tmp_config_file.write_text(tomlkit.dumps(doc))
             with snowflake.connector.connect(
                 connection_name="default",
-                config_file_path=tmp_config_file,
+                connections_file_path=tmp_config_file,
             ) as conn:
                 with conn.cursor() as cur:
                     assert cur.execute("select 1;").fetchall() == [