From c4f76d5a891ab311a3b2df22c3164ca9a230026a Mon Sep 17 00:00:00 2001 From: Taru Garg Date: Tue, 26 Aug 2025 18:37:15 +0530 Subject: [PATCH 1/7] feat: config class for tfe --- Makefile | 4 +- pyproject.toml | 12 +++- tests/{ => units}/test_client.py | 2 +- tests/units/test_config.py | 88 +++++++++++++++++++++++++++++ {src/python_tfe => tfe}/__init__.py | 5 +- {src/python_tfe => tfe}/client.py | 0 tfe/config.py | 66 ++++++++++++++++++++++ 7 files changed, 169 insertions(+), 8 deletions(-) rename tests/{ => units}/test_client.py (84%) create mode 100644 tests/units/test_config.py rename {src/python_tfe => tfe}/__init__.py (67%) rename {src/python_tfe => tfe}/client.py (100%) create mode 100644 tfe/config.py diff --git a/Makefile b/Makefile index 6b405940..11c59d73 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ .PHONY: help fmt fmt-check lint check test install dev-install type-check clean all venv activate PYTHON := python3 -SRC_DIR := src +SRC_DIR := tfe TEST_DIR := tests VENV := .venv VENV_PYTHON := $(VENV)/bin/python @@ -59,7 +59,7 @@ type-check: $(VENV_PYTHON) -m mypy $(SRC_DIR) test: - $(VENV_PYTHON) -m pytest + $(VENV_PYTHON) -m pytest -v clean: find . -type f -name "*.pyc" -delete diff --git a/pyproject.toml b/pyproject.toml index f2aeba35..387a66df 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,16 +13,22 @@ requires-python = ">=3.10" dependencies = ["requests>=2.25.0"] [project.optional-dependencies] -dev = ["pytest>=7.0.0", "mypy>=1.17.1", "ruff>=0.12.10"] +dev = [ + "pytest>=7.0.0", + "pytest-mock>=3.14.1", + "mypy>=1.17.1", + "ruff>=0.12.10", + "types-requests>=2.32.4.20250809", +] [project.urls] Repository = "https://github.com/hashicorp/python-tfe" [tool.setuptools.packages.find] -where = ["src"] +where = ["tfe"] [tool.setuptools.package-dir] -"" = "src" +"" = "tfe" # Ruff configuration [tool.ruff] diff --git a/tests/test_client.py b/tests/units/test_client.py similarity index 84% rename from tests/test_client.py rename to tests/units/test_client.py index 08deb132..fe48a55f 100644 --- a/tests/test_client.py +++ b/tests/units/test_client.py @@ -2,7 +2,7 @@ Simple tests for the client. """ -from python_tfe.client import TerraformEnterpriseClient +from tfe.client import TerraformEnterpriseClient def test_foo(): diff --git a/tests/units/test_config.py b/tests/units/test_config.py new file mode 100644 index 00000000..ba9a48cd --- /dev/null +++ b/tests/units/test_config.py @@ -0,0 +1,88 @@ +import pytest +import requests + +from tfe import config + + +@pytest.fixture(autouse=True) +def reset_environment(monkeypatch): + """Reset environment variables before each test.""" + monkeypatch.delenv("TFE_ADDRESS", raising=False) + monkeypatch.delenv("TFE_TOKEN", raising=False) + monkeypatch.delenv("TFE_HOST", raising=False) + yield + + +@pytest.fixture +def cfg(): + """Provide a fresh Config instance with clean environment.""" + return config.Config() + + +@pytest.fixture +def test_session(): + """Provide a clean requests session without default headers.""" + session = requests.Session() + session.headers["User-Agent"] = "test" + session.headers["Authorization"] = "Bearer test" + return session + + +class TestConfig: + def test_default_config(self, cfg): + """Test that default configuration values are set correctly.""" + assert cfg.address == config.DEFAULT_ADDRESS + assert cfg.base_path == config.DEFAULT_BASE_PATH + assert cfg.registry_base_path == config.DEFAULT_REGISTRY_PATH + assert cfg.token is None + assert isinstance(cfg.http_client, requests.Session) + assert "User-Agent" in cfg.http_client.headers + assert "Authorization" not in cfg.http_client.headers + assert cfg.retry_log_hook is None + assert cfg.retry_server_errors is False + + def test_env_address_and_token(self, monkeypatch): + """Test that environment variables TFE_ADDRESS and TFE_TOKEN are read correctly.""" + monkeypatch.setenv("TFE_ADDRESS", "https://custom.tfe") + monkeypatch.setenv("TFE_TOKEN", "abc123") + cfg = config.Config() + assert cfg.address == "https://custom.tfe" + assert cfg.token == "abc123" + + def test_env_host_fallback(self, monkeypatch): + """Test that TFE_HOST is used as fallback when TFE_ADDRESS is not set.""" + monkeypatch.setenv("TFE_HOST", "host.tfe") + cfg = config.Config() + assert cfg.address == "https://host.tfe" + + def test_explicit_address_override(self): + """Test that explicitly passed address overrides environment variables.""" + cfg = config.Config(address="https://explicit.tfe") + assert cfg.address == "https://explicit.tfe" + + def test_headers_update(self): + """Test that custom headers are properly merged with default headers.""" + custom_headers = {"Authorization": "Bearer testtoken", "X-Test": "yes"} + cfg = config.Config(headers=custom_headers) + assert "Authorization" in cfg.http_client.headers + assert cfg.http_client.headers["Authorization"] == "Bearer testtoken" + assert "X-Test" in cfg.http_client.headers + assert cfg.http_client.headers["X-Test"] == "yes" + assert "User-Agent" in cfg.http_client.headers + + def test_retry_log_hook_and_server_errors(self): + """Test that retry configuration is properly set.""" + + def dummy_hook(retries, response): + pass + + cfg = config.Config(retry_log_hook=dummy_hook, retry_server_errors=True) + assert cfg.retry_log_hook == dummy_hook + assert cfg.retry_server_errors is True + + def test_custom_session(self, test_session): + """Test that User-Agent is set when session has no default User-Agent.""" + cfg = config.Config(http_client=test_session) + assert "User-Agent" in cfg.http_client.headers + assert cfg.http_client.headers["User-Agent"] == "test" + assert cfg.http_client.headers["Authorization"] == "Bearer test" diff --git a/src/python_tfe/__init__.py b/tfe/__init__.py similarity index 67% rename from src/python_tfe/__init__.py rename to tfe/__init__.py index 516d34bf..4c2a3698 100644 --- a/src/python_tfe/__init__.py +++ b/tfe/__init__.py @@ -6,6 +6,7 @@ workspaces, runs, state files, and other TFE/TFC resources. """ -from .client import TerraformEnterpriseClient +from tfe.client import TerraformEnterpriseClient +from tfe.config import Config -__all__ = ["TerraformEnterpriseClient"] +__all__ = ["TerraformEnterpriseClient", "Config"] diff --git a/src/python_tfe/client.py b/tfe/client.py similarity index 100% rename from src/python_tfe/client.py rename to tfe/client.py diff --git a/tfe/config.py b/tfe/config.py new file mode 100644 index 00000000..919f8391 --- /dev/null +++ b/tfe/config.py @@ -0,0 +1,66 @@ +import logging +import os +from collections.abc import Callable +from dataclasses import dataclass, field + +import requests + +logger = logging.getLogger(__name__) + +DEFAULT_ADDRESS = "https://app.terraform.io" +DEFAULT_BASE_PATH = "/api/v2/" +DEFAULT_REGISTRY_PATH = "/api/registry" + + +@dataclass +class Config: + # Address of the Terraform Enterprise API + address: str | None = field(default_factory=lambda: os.getenv("TFE_ADDRESS")) + + # Base path for which the API is served + base_path: str | None = DEFAULT_BASE_PATH + + # Base path for the Terraform Enterprise Registry API + registry_base_path: str | None = DEFAULT_REGISTRY_PATH + + # API token used to access the terraform enterprise API + token: str | None = field(default_factory=lambda: os.getenv("TFE_TOKEN")) + + # Headers to include in API requests + # TODO: Do we need headers ? we can pass them directly to http_client, but this will differ from the go-tfe module + headers: dict[str, str] | None = None + + # Custom request session which needs to be used + http_client: requests.Session = field(default_factory=requests.Session) + + # Callable to run before any request is retried + retry_log_hook: Callable[[int, requests.Response], None] | None = None + + # Enable/Disable retry logic + retry_server_errors: bool = False + + def __post_init__(self) -> None: + if not self.address: + host = os.getenv("TFE_HOST") + if host: + self.address = f"https://{host}" + else: + self.address = DEFAULT_ADDRESS + + if self.headers is None: + self.headers = {} + + if ( + "User-Agent" not in self.http_client.headers + and "User-Agent" not in self.headers + ): + self.headers["User-Agent"] = "python-tfe" + + if ( + self.token + and "Authorization" not in self.http_client.headers + and "Authorization" not in self.headers + ): + self.headers["Authorization"] = f"Bearer {self.token}" + + self.http_client.headers.update(self.headers) From 57ec2a0b53e576b79edd9025deabd43ff78274c0 Mon Sep 17 00:00:00 2001 From: Taru Garg Date: Tue, 26 Aug 2025 23:08:20 +0530 Subject: [PATCH 2/7] feat: add client --- tests/units/test_client.py | 16 ---- tfe/__init__.py | 4 +- tfe/client.py | 149 ++++++++++++++++++++++++++++++++----- tfe/config.py | 15 ++-- 4 files changed, 142 insertions(+), 42 deletions(-) diff --git a/tests/units/test_client.py b/tests/units/test_client.py index fe48a55f..8b137891 100644 --- a/tests/units/test_client.py +++ b/tests/units/test_client.py @@ -1,17 +1 @@ -""" -Simple tests for the client. -""" -from tfe.client import TerraformEnterpriseClient - - -def test_foo(): - """Test foo returns foo.""" - client = TerraformEnterpriseClient("token") - assert client.foo() == "foo" - - -def test_bar(): - """Test bar returns bar.""" - client = TerraformEnterpriseClient("token") - assert client.bar() == "bar" diff --git a/tfe/__init__.py b/tfe/__init__.py index 4c2a3698..60fbccb2 100644 --- a/tfe/__init__.py +++ b/tfe/__init__.py @@ -6,7 +6,7 @@ workspaces, runs, state files, and other TFE/TFC resources. """ -from tfe.client import TerraformEnterpriseClient +from tfe.client import Client, TFEClientError from tfe.config import Config -__all__ = ["TerraformEnterpriseClient", "Config"] +__all__ = ["Client", "TFEClientError", "Config"] diff --git a/tfe/client.py b/tfe/client.py index ebfecede..4733fbf0 100644 --- a/tfe/client.py +++ b/tfe/client.py @@ -2,27 +2,140 @@ Main client class for Terraform Enterprise/Cloud API. """ +import logging +from typing import cast +from urllib.parse import urljoin, urlparse -class TerraformEnterpriseClient: - """Simple client for Terraform Enterprise/Cloud API.""" +from tfe.config import Config - def __init__(self, token: str, base_url: str = "https://app.terraform.io/api/v2/"): - """Initialize the client with API token and base URL.""" - self.token = token - self.base_url = base_url.rstrip("/") + "/" +logger = logging.getLogger(__name__) - def get_base_url(self) -> str: - """Get the base URL.""" - return self.base_url - def foo(self) -> str: - """Simple foo method for testing.""" - return "foo" +class TFEClientError(Exception): + """Base exception for TFE client errors.""" - def bar(self) -> str: - """Simple bar method for testing.""" - return "bar" + pass - def foobar(self) -> str: - """Combine foo and bar.""" - return f"{self.foo()}{self.bar()}" + +class Client: + """ + Client is the Terraform Enterprise API client. It provides the basic + functionality to interact with the Terraform API. + """ + + def __init__(self, config: Config | None = None) -> None: + self.config = Config() + + if config is not None: + self._merge_config(config) + + self._validate_config() + self._setup_urls() + + self._api_version = "" + self._tfe_version = "" + self._app_name = "" + self._fetch_api_metadata() + + def _merge_config(self, config: Config) -> None: + """Merge provided config with defaults, preserving non-empty values.""" + if config.address: + self.config.address = config.address + if config.base_path: + self.config.base_path = config.base_path + if config.registry_base_path: + self.config.registry_base_path = config.registry_base_path + if config.token: + self.config.token = config.token + if config.headers: + if self.config.headers is None: + self.config.headers = {} + self.config.headers.update(config.headers) + if config.http_client: + self.config.http_client = config.http_client + if config.retry_log_hook: + self.config.retry_log_hook = config.retry_log_hook + self.config.retry_server_errors = config.retry_server_errors + + def _validate_config(self) -> None: + """Validate required configuration.""" + if not self.config.token: + raise TFEClientError("API token is required") + + if not self.config.address: + raise TFEClientError("API address is required") + + # Type narrowing: after validation, we know these are not None + assert self.config.address is not None + assert self.config.token is not None + + def _setup_urls(self) -> None: + """Parse and setup base URLs.""" + try: + # After validation, we know address is not None + parsed_url = urlparse(self.config.address) + if not parsed_url.scheme: + raise ValueError("Address must include protocol (http/https)") + + # Ensure base path ends with / + base_path = self.config.base_path + if not base_path.endswith("/"): + base_path += "/" + + registry_path = self.config.registry_base_path + if not registry_path.endswith("/"): + registry_path += "/" + + self.base_url = urljoin(self.config.address, base_path) + self.registry_base_url = urljoin(self.config.address, registry_path) + + except Exception as e: + raise TFEClientError(f"Invalid address '{self.config.address}': {e}") from e + + def _fetch_api_metadata(self) -> None: + """Fetch API metadata from the server.""" + ping_url = urljoin(self.base_url, "ping") + + # After validation, we know token is not None + token = cast(str, self.config.token) + headers = { + "Accept": "application/vnd.api+json", + "Authorization": f"Bearer {token}", + } + if self.config.headers: + headers.update(self.config.headers) + + response = self.config.http_client.get(ping_url, headers=headers) + response.raise_for_status() + + # Extract metadata from headers + self._api_version = response.headers.get("TFP-API-Version", "") + self._tfe_version = response.headers.get("X-TFE-Version", "") + self._app_name = response.headers.get("TFP-AppName", "") + + @property + def remote_api_version(self) -> str: + """Return the server's declared API version string.""" + return self._api_version + + @property + def remote_tfe_version(self) -> str: + """Return the server's declared TFE version string.""" + return self._tfe_version + + @property + def app_name(self) -> str: + """Return the name of the instance.""" + return self._app_name + + def is_cloud(self) -> bool: + """Return True if the client is configured against HCP Terraform.""" + return self._app_name == "HCP Terraform" + + def is_enterprise(self) -> bool: + """Return True if the client is configured against Terraform Enterprise.""" + return not self.is_cloud() + + def set_fake_remote_api_version(self, version: str) -> None: + """Set a fake API version for testing purposes.""" + self._api_version = version diff --git a/tfe/config.py b/tfe/config.py index 919f8391..aeb8dec4 100644 --- a/tfe/config.py +++ b/tfe/config.py @@ -15,13 +15,13 @@ @dataclass class Config: # Address of the Terraform Enterprise API - address: str | None = field(default_factory=lambda: os.getenv("TFE_ADDRESS")) + address: str = "" # Base path for which the API is served - base_path: str | None = DEFAULT_BASE_PATH + base_path: str = DEFAULT_BASE_PATH # Base path for the Terraform Enterprise Registry API - registry_base_path: str | None = DEFAULT_REGISTRY_PATH + registry_base_path: str = DEFAULT_REGISTRY_PATH # API token used to access the terraform enterprise API token: str | None = field(default_factory=lambda: os.getenv("TFE_TOKEN")) @@ -40,10 +40,13 @@ class Config: retry_server_errors: bool = False def __post_init__(self) -> None: + tfe_address = os.getenv("TFE_ADDRESS", "") + if tfe_address: + self.address = tfe_address + if not self.address: - host = os.getenv("TFE_HOST") - if host: - self.address = f"https://{host}" + if os.getenv("TFE_HOST"): + self.address = f"https://{os.getenv('TFE_HOST')}" else: self.address = DEFAULT_ADDRESS From 3d661e115acf89306d255243011528a784998c78 Mon Sep 17 00:00:00 2001 From: Taru Garg Date: Tue, 26 Aug 2025 23:23:23 +0530 Subject: [PATCH 3/7] fix: cleanup types --- tests/units/test_config.py | 2 +- tfe/client.py | 4 +--- tfe/config.py | 7 +++++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/units/test_config.py b/tests/units/test_config.py index ba9a48cd..56c84445 100644 --- a/tests/units/test_config.py +++ b/tests/units/test_config.py @@ -34,7 +34,7 @@ def test_default_config(self, cfg): assert cfg.address == config.DEFAULT_ADDRESS assert cfg.base_path == config.DEFAULT_BASE_PATH assert cfg.registry_base_path == config.DEFAULT_REGISTRY_PATH - assert cfg.token is None + assert cfg.token == "" assert isinstance(cfg.http_client, requests.Session) assert "User-Agent" in cfg.http_client.headers assert "Authorization" not in cfg.http_client.headers diff --git a/tfe/client.py b/tfe/client.py index 4733fbf0..c0e43a4a 100644 --- a/tfe/client.py +++ b/tfe/client.py @@ -3,7 +3,6 @@ """ import logging -from typing import cast from urllib.parse import urljoin, urlparse from tfe.config import Config @@ -97,10 +96,9 @@ def _fetch_api_metadata(self) -> None: ping_url = urljoin(self.base_url, "ping") # After validation, we know token is not None - token = cast(str, self.config.token) headers = { "Accept": "application/vnd.api+json", - "Authorization": f"Bearer {token}", + "Authorization": f"Bearer {self.config.token}", } if self.config.headers: headers.update(self.config.headers) diff --git a/tfe/config.py b/tfe/config.py index aeb8dec4..65558a8e 100644 --- a/tfe/config.py +++ b/tfe/config.py @@ -15,7 +15,7 @@ @dataclass class Config: # Address of the Terraform Enterprise API - address: str = "" + address: str = field(default="") # Base path for which the API is served base_path: str = DEFAULT_BASE_PATH @@ -24,7 +24,7 @@ class Config: registry_base_path: str = DEFAULT_REGISTRY_PATH # API token used to access the terraform enterprise API - token: str | None = field(default_factory=lambda: os.getenv("TFE_TOKEN")) + token: str = field(default="") # Headers to include in API requests # TODO: Do we need headers ? we can pass them directly to http_client, but this will differ from the go-tfe module @@ -50,6 +50,9 @@ def __post_init__(self) -> None: else: self.address = DEFAULT_ADDRESS + if not self.token: + self.token = os.getenv("TFE_TOKEN", "") + if self.headers is None: self.headers = {} From fafa55d2465555734f29ca4659c29bc4c69ad62d Mon Sep 17 00:00:00 2001 From: Taru Garg Date: Wed, 27 Aug 2025 12:21:35 +0530 Subject: [PATCH 4/7] tests: improve tests for client and config --- tests/units/test_client.py | 102 +++++++++++++++++++++++++++++++++++++ tests/units/test_config.py | 15 ++++-- tfe/client.py | 71 ++++---------------------- tfe/config.py | 34 +++++++++---- 4 files changed, 149 insertions(+), 73 deletions(-) diff --git a/tests/units/test_client.py b/tests/units/test_client.py index 8b137891..ccaac3e6 100644 --- a/tests/units/test_client.py +++ b/tests/units/test_client.py @@ -1 +1,103 @@ +import pytest +from unittest.mock import Mock, patch +from tfe import client, config + + +@pytest.fixture +def test_config(): + return config.Config( + address="https://app.terraform.io", + token="test-token" + ) + + +@pytest.fixture +def mock_response(): + response = Mock() + response.headers = { + "TFP-API-Version": "2.5.0", + "X-TFE-Version": "v202308-1", + "TFP-AppName": "HCP Terraform" + } + response.raise_for_status.return_value = None + return response + + +class TestClient: + @patch('requests.Session.get') + def test_client_initialization(self, mock_get, test_config, mock_response): + """Test basic client setup works.""" + mock_get.return_value = mock_response + + client_instance = client.Client(config=test_config) + + assert client_instance.config.address == "https://app.terraform.io" + assert client_instance.config.token == "test-token" + assert client_instance.base_url == "https://app.terraform.io/api/v2/" + assert client_instance.registry_base_url == "https://app.terraform.io/api/registry/" + + @patch('requests.Session.get') + def test_url_normalization(self, mock_get, mock_response): + """Test that paths get normalized with trailing slashes.""" + mock_get.return_value = mock_response + + cfg = config.Config( + address="https://example.com", + token="test", + base_path="/custom/api", # no trailing slash + registry_base_path="/registry" # no trailing slash + ) + + client_instance = client.Client(config=cfg) + + assert client_instance.base_url == "https://example.com/custom/api/" + assert client_instance.registry_base_url == "https://example.com/registry/" + + @patch('requests.Session.get') + def test_api_metadata_extraction(self, mock_get, test_config, mock_response): + """Test that API metadata gets extracted from response headers.""" + mock_get.return_value = mock_response + + client_instance = client.Client(config=test_config) + + assert client_instance.remote_api_version == "2.5.0" + assert client_instance.remote_tfe_version == "v202308-1" + assert client_instance.app_name == "HCP Terraform" + + @patch('requests.Session.get') + def test_cloud_vs_enterprise_detection(self, mock_get, test_config): + """Test detection between cloud and enterprise instances.""" + # Test HCP Terraform (cloud) + cloud_response = Mock() + cloud_response.headers = {"TFP-AppName": "HCP Terraform"} + cloud_response.raise_for_status.return_value = None + mock_get.return_value = cloud_response + + cloud_client = client.Client(config=test_config) + assert cloud_client.is_cloud() is True + assert cloud_client.is_enterprise() is False + + # Test Terraform Enterprise + enterprise_response = Mock() + enterprise_response.headers = {"TFP-AppName": "Terraform Enterprise"} + enterprise_response.raise_for_status.return_value = None + mock_get.return_value = enterprise_response + + enterprise_client = client.Client(config=test_config) + assert enterprise_client.is_cloud() is False + assert enterprise_client.is_enterprise() is True + + @patch('requests.Session.get') + def test_fake_api_version_for_testing(self, mock_get, test_config, mock_response): + """Test the fake API version setter for testing scenarios.""" + mock_get.return_value = mock_response + + client_instance = client.Client(config=test_config) + + # Original version from mock + assert client_instance.remote_api_version == "2.5.0" + + # Set fake version + client_instance.set_fake_remote_api_version("3.0.0") + assert client_instance.remote_api_version == "3.0.0" diff --git a/tests/units/test_config.py b/tests/units/test_config.py index 56c84445..ba692c04 100644 --- a/tests/units/test_config.py +++ b/tests/units/test_config.py @@ -10,6 +10,7 @@ def reset_environment(monkeypatch): monkeypatch.delenv("TFE_ADDRESS", raising=False) monkeypatch.delenv("TFE_TOKEN", raising=False) monkeypatch.delenv("TFE_HOST", raising=False) + monkeypatch.setenv("TFE_TOKEN", "abc123") yield @@ -34,17 +35,14 @@ def test_default_config(self, cfg): assert cfg.address == config.DEFAULT_ADDRESS assert cfg.base_path == config.DEFAULT_BASE_PATH assert cfg.registry_base_path == config.DEFAULT_REGISTRY_PATH - assert cfg.token == "" assert isinstance(cfg.http_client, requests.Session) assert "User-Agent" in cfg.http_client.headers - assert "Authorization" not in cfg.http_client.headers assert cfg.retry_log_hook is None assert cfg.retry_server_errors is False def test_env_address_and_token(self, monkeypatch): """Test that environment variables TFE_ADDRESS and TFE_TOKEN are read correctly.""" monkeypatch.setenv("TFE_ADDRESS", "https://custom.tfe") - monkeypatch.setenv("TFE_TOKEN", "abc123") cfg = config.Config() assert cfg.address == "https://custom.tfe" assert cfg.token == "abc123" @@ -86,3 +84,14 @@ def test_custom_session(self, test_session): assert "User-Agent" in cfg.http_client.headers assert cfg.http_client.headers["User-Agent"] == "test" assert cfg.http_client.headers["Authorization"] == "Bearer test" + + def test_validate_config(self, monkeypatch): + """Test that configuration validation works as expected.""" + with pytest.raises(ValueError, match="API token is required") as _: + monkeypatch.setenv("TFE_TOKEN", "") + cfg = config.Config(token="") + + with pytest.raises(ValueError, match="Address must include protocol") as _: + monkeypatch.setenv("TFE_TOKEN", "test-token") + monkeypatch.setenv("TFE_ADDRESS", "test.foo.bar") + cfg = config.Config() \ No newline at end of file diff --git a/tfe/client.py b/tfe/client.py index c0e43a4a..94c5c279 100644 --- a/tfe/client.py +++ b/tfe/client.py @@ -3,7 +3,7 @@ """ import logging -from urllib.parse import urljoin, urlparse +from urllib.parse import urljoin from tfe.config import Config @@ -23,12 +23,7 @@ class Client: """ def __init__(self, config: Config | None = None) -> None: - self.config = Config() - - if config is not None: - self._merge_config(config) - - self._validate_config() + self.config = config or Config() self._setup_urls() self._api_version = "" @@ -36,69 +31,25 @@ def __init__(self, config: Config | None = None) -> None: self._app_name = "" self._fetch_api_metadata() - def _merge_config(self, config: Config) -> None: - """Merge provided config with defaults, preserving non-empty values.""" - if config.address: - self.config.address = config.address - if config.base_path: - self.config.base_path = config.base_path - if config.registry_base_path: - self.config.registry_base_path = config.registry_base_path - if config.token: - self.config.token = config.token - if config.headers: - if self.config.headers is None: - self.config.headers = {} - self.config.headers.update(config.headers) - if config.http_client: - self.config.http_client = config.http_client - if config.retry_log_hook: - self.config.retry_log_hook = config.retry_log_hook - self.config.retry_server_errors = config.retry_server_errors - - def _validate_config(self) -> None: - """Validate required configuration.""" - if not self.config.token: - raise TFEClientError("API token is required") - - if not self.config.address: - raise TFEClientError("API address is required") - - # Type narrowing: after validation, we know these are not None - assert self.config.address is not None - assert self.config.token is not None - def _setup_urls(self) -> None: """Parse and setup base URLs.""" - try: - # After validation, we know address is not None - parsed_url = urlparse(self.config.address) - if not parsed_url.scheme: - raise ValueError("Address must include protocol (http/https)") + # Ensure base path ends with / + base_path = self.config.base_path + if not base_path.endswith("/"): + base_path += "/" - # Ensure base path ends with / - base_path = self.config.base_path - if not base_path.endswith("/"): - base_path += "/" + registry_path = self.config.registry_base_path + if not registry_path.endswith("/"): + registry_path += "/" - registry_path = self.config.registry_base_path - if not registry_path.endswith("/"): - registry_path += "/" - - self.base_url = urljoin(self.config.address, base_path) - self.registry_base_url = urljoin(self.config.address, registry_path) - - except Exception as e: - raise TFEClientError(f"Invalid address '{self.config.address}': {e}") from e + self.base_url = urljoin(self.config.address, base_path) + self.registry_base_url = urljoin(self.config.address, registry_path) def _fetch_api_metadata(self) -> None: """Fetch API metadata from the server.""" ping_url = urljoin(self.base_url, "ping") - - # After validation, we know token is not None headers = { "Accept": "application/vnd.api+json", - "Authorization": f"Bearer {self.config.token}", } if self.config.headers: headers.update(self.config.headers) diff --git a/tfe/config.py b/tfe/config.py index 65558a8e..f412f885 100644 --- a/tfe/config.py +++ b/tfe/config.py @@ -2,6 +2,7 @@ import os from collections.abc import Callable from dataclasses import dataclass, field +from urllib.parse import urlparse import requests @@ -28,7 +29,7 @@ class Config: # Headers to include in API requests # TODO: Do we need headers ? we can pass them directly to http_client, but this will differ from the go-tfe module - headers: dict[str, str] | None = None + headers: dict[str, str] = field(default_factory=dict) # Custom request session which needs to be used http_client: requests.Session = field(default_factory=requests.Session) @@ -39,7 +40,7 @@ class Config: # Enable/Disable retry logic retry_server_errors: bool = False - def __post_init__(self) -> None: + def _set_address(self) -> None: tfe_address = os.getenv("TFE_ADDRESS", "") if tfe_address: self.address = tfe_address @@ -50,23 +51,36 @@ def __post_init__(self) -> None: else: self.address = DEFAULT_ADDRESS + def _set_token(self) -> None: if not self.token: self.token = os.getenv("TFE_TOKEN", "") - if self.headers is None: - self.headers = {} + if ( + self.token + and "Authorization" not in self.http_client.headers + and "Authorization" not in self.headers + ): + self.headers["Authorization"] = f"Bearer {self.token}" + def _set_user_agent(self) -> None: if ( "User-Agent" not in self.http_client.headers and "User-Agent" not in self.headers ): self.headers["User-Agent"] = "python-tfe" - if ( - self.token - and "Authorization" not in self.http_client.headers - and "Authorization" not in self.headers - ): - self.headers["Authorization"] = f"Bearer {self.token}" + def _validate_config(self) -> None: + if not self.http_client.headers.get("Authorization"): + raise ValueError( + "API token is required, please set the TFE_TOKEN environment variable or the token field in the configuration." + ) + parsed_url = urlparse(self.address) + if not parsed_url.scheme: + raise ValueError("Address must include protocol (http/https)") + def __post_init__(self) -> None: + self._set_address() + self._set_token() + self._set_user_agent() self.http_client.headers.update(self.headers) + self._validate_config() From 5ac2dbf1a3193edea90c16ebeb187c01b296adf4 Mon Sep 17 00:00:00 2001 From: Taru Garg Date: Wed, 27 Aug 2025 12:23:28 +0530 Subject: [PATCH 5/7] tests: make lint happy --- tests/units/test_client.py | 55 +++++++++++++++++++------------------- tests/units/test_config.py | 4 +-- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/tests/units/test_client.py b/tests/units/test_client.py index ccaac3e6..591bf500 100644 --- a/tests/units/test_client.py +++ b/tests/units/test_client.py @@ -1,15 +1,13 @@ -import pytest from unittest.mock import Mock, patch +import pytest + from tfe import client, config @pytest.fixture def test_config(): - return config.Config( - address="https://app.terraform.io", - token="test-token" - ) + return config.Config(address="https://app.terraform.io", token="test-token") @pytest.fixture @@ -17,55 +15,58 @@ def mock_response(): response = Mock() response.headers = { "TFP-API-Version": "2.5.0", - "X-TFE-Version": "v202308-1", - "TFP-AppName": "HCP Terraform" + "X-TFE-Version": "v202308-1", + "TFP-AppName": "HCP Terraform", } response.raise_for_status.return_value = None return response class TestClient: - @patch('requests.Session.get') + @patch("requests.Session.get") def test_client_initialization(self, mock_get, test_config, mock_response): """Test basic client setup works.""" mock_get.return_value = mock_response - + client_instance = client.Client(config=test_config) - + assert client_instance.config.address == "https://app.terraform.io" assert client_instance.config.token == "test-token" assert client_instance.base_url == "https://app.terraform.io/api/v2/" - assert client_instance.registry_base_url == "https://app.terraform.io/api/registry/" + assert ( + client_instance.registry_base_url + == "https://app.terraform.io/api/registry/" + ) - @patch('requests.Session.get') + @patch("requests.Session.get") def test_url_normalization(self, mock_get, mock_response): """Test that paths get normalized with trailing slashes.""" mock_get.return_value = mock_response - + cfg = config.Config( address="https://example.com", token="test", base_path="/custom/api", # no trailing slash - registry_base_path="/registry" # no trailing slash + registry_base_path="/registry", # no trailing slash ) - + client_instance = client.Client(config=cfg) - + assert client_instance.base_url == "https://example.com/custom/api/" assert client_instance.registry_base_url == "https://example.com/registry/" - @patch('requests.Session.get') + @patch("requests.Session.get") def test_api_metadata_extraction(self, mock_get, test_config, mock_response): """Test that API metadata gets extracted from response headers.""" mock_get.return_value = mock_response - + client_instance = client.Client(config=test_config) - + assert client_instance.remote_api_version == "2.5.0" assert client_instance.remote_tfe_version == "v202308-1" assert client_instance.app_name == "HCP Terraform" - @patch('requests.Session.get') + @patch("requests.Session.get") def test_cloud_vs_enterprise_detection(self, mock_get, test_config): """Test detection between cloud and enterprise instances.""" # Test HCP Terraform (cloud) @@ -73,31 +74,31 @@ def test_cloud_vs_enterprise_detection(self, mock_get, test_config): cloud_response.headers = {"TFP-AppName": "HCP Terraform"} cloud_response.raise_for_status.return_value = None mock_get.return_value = cloud_response - + cloud_client = client.Client(config=test_config) assert cloud_client.is_cloud() is True assert cloud_client.is_enterprise() is False - + # Test Terraform Enterprise enterprise_response = Mock() enterprise_response.headers = {"TFP-AppName": "Terraform Enterprise"} enterprise_response.raise_for_status.return_value = None mock_get.return_value = enterprise_response - + enterprise_client = client.Client(config=test_config) assert enterprise_client.is_cloud() is False assert enterprise_client.is_enterprise() is True - @patch('requests.Session.get') + @patch("requests.Session.get") def test_fake_api_version_for_testing(self, mock_get, test_config, mock_response): """Test the fake API version setter for testing scenarios.""" mock_get.return_value = mock_response - + client_instance = client.Client(config=test_config) - + # Original version from mock assert client_instance.remote_api_version == "2.5.0" - + # Set fake version client_instance.set_fake_remote_api_version("3.0.0") assert client_instance.remote_api_version == "3.0.0" diff --git a/tests/units/test_config.py b/tests/units/test_config.py index ba692c04..107413ff 100644 --- a/tests/units/test_config.py +++ b/tests/units/test_config.py @@ -89,9 +89,9 @@ def test_validate_config(self, monkeypatch): """Test that configuration validation works as expected.""" with pytest.raises(ValueError, match="API token is required") as _: monkeypatch.setenv("TFE_TOKEN", "") - cfg = config.Config(token="") + _ = config.Config(token="") with pytest.raises(ValueError, match="Address must include protocol") as _: monkeypatch.setenv("TFE_TOKEN", "test-token") monkeypatch.setenv("TFE_ADDRESS", "test.foo.bar") - cfg = config.Config() \ No newline at end of file + _ = config.Config() From aaeb3722ce104817425e47e3e6dadae010c0bfca Mon Sep 17 00:00:00 2001 From: Taru Garg Date: Wed, 27 Aug 2025 13:18:27 +0530 Subject: [PATCH 6/7] chore: add git hooks for pre-commit formatting --- .gitignore | 1 + .hooks/pre-commit | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100755 .hooks/pre-commit diff --git a/.gitignore b/.gitignore index 6c125433..7e44cac2 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ htmlcov .pytest_cache/ .mypy_cache/ .ruff_cache/ +*.egg-info # Visual Studio Code .vscode/ diff --git a/.hooks/pre-commit b/.hooks/pre-commit new file mode 100755 index 00000000..4697f867 --- /dev/null +++ b/.hooks/pre-commit @@ -0,0 +1,34 @@ +set -euo pipefail + +# Call block to block the commit with a message. +block() { + echo "$@" + echo "Commit blocked - see errors above." + exit 1 +} + +# Add all check functions to this space separated list. +# They are executed in this order (see end of file). +CHECKS="fmt lint" + +# Run fmt against changed files compared to origin/main +fmt() { + echo "==> Running fmt on all files" + make fmt || block "Formatting failed" + + # Re-add any files that were changed by the fixers + git add -u +} + +lint() { + echo "==> Running lint on all files" + make lint || block "Linting failed" + + # Re-add any files that were changed by the fixers + git add -u +} + +for CHECK in $CHECKS; do + # Force each check into a subshell to avoid crosstalk. + ( $CHECK ) || exit $? +done \ No newline at end of file From 795f98963d478fb23f5794ef39833e223471f0c5 Mon Sep 17 00:00:00 2001 From: Taru Garg Date: Wed, 27 Aug 2025 13:19:55 +0530 Subject: [PATCH 7/7] fix: add shebang to precommit hook --- .hooks/pre-commit | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.hooks/pre-commit b/.hooks/pre-commit index 4697f867..d9ad6fc9 100755 --- a/.hooks/pre-commit +++ b/.hooks/pre-commit @@ -1,3 +1,5 @@ +#!/usr/bin/env bash + set -euo pipefail # Call block to block the commit with a message.