From 66698503b878459ed5d7d208c7ec37e995a54f28 Mon Sep 17 00:00:00 2001 From: Vini Brasil Date: Thu, 31 Oct 2024 15:00:58 -0300 Subject: [PATCH] Replace .netrc with uv environment variables (#1541) This commit replaces .netrc with uv environment variables for installing tools from private repositories. To store credentials, I created a new and reusable settings file for the CLI in `$HOME/.config/crewai/settings.json`. The issue with .netrc files is that they are applied system-wide and are scoped by hostname, meaning we can't differentiate tool repositories requests from regular requests to CrewAI's API. --- src/crewai/cli/config.py | 38 ++++++++++++ src/crewai/cli/tools/main.py | 40 +++++++------ tests/cli/config_test.py | 109 +++++++++++++++++++++++++++++++++++ tests/cli/tools/test_main.py | 1 + 4 files changed, 169 insertions(+), 19 deletions(-) create mode 100644 src/crewai/cli/config.py create mode 100644 tests/cli/config_test.py diff --git a/src/crewai/cli/config.py b/src/crewai/cli/config.py new file mode 100644 index 000000000..000f1e6d0 --- /dev/null +++ b/src/crewai/cli/config.py @@ -0,0 +1,38 @@ +import json +from pathlib import Path +from pydantic import BaseModel, Field +from typing import Optional + +DEFAULT_CONFIG_PATH = Path.home() / ".config" / "crewai" / "settings.json" + +class Settings(BaseModel): + tool_repository_username: Optional[str] = Field(None, description="Username for interacting with the Tool Repository") + tool_repository_password: Optional[str] = Field(None, description="Password for interacting with the Tool Repository") + config_path: Path = Field(default=DEFAULT_CONFIG_PATH, exclude=True) + + def __init__(self, config_path: Path = DEFAULT_CONFIG_PATH, **data): + """Load Settings from config path""" + config_path.parent.mkdir(parents=True, exist_ok=True) + + file_data = {} + if config_path.is_file(): + try: + with config_path.open("r") as f: + file_data = json.load(f) + except json.JSONDecodeError: + file_data = {} + + merged_data = {**file_data, **data} + super().__init__(config_path=config_path, **merged_data) + + def dump(self) -> None: + """Save current settings to settings.json""" + if self.config_path.is_file(): + with self.config_path.open("r") as f: + existing_data = json.load(f) + else: + existing_data = {} + + updated_data = {**existing_data, **self.model_dump(exclude_unset=True)} + with self.config_path.open("w") as f: + json.dump(updated_data, f, indent=4) diff --git a/src/crewai/cli/tools/main.py b/src/crewai/cli/tools/main.py index c875229c3..35652a1ec 100644 --- a/src/crewai/cli/tools/main.py +++ b/src/crewai/cli/tools/main.py @@ -1,17 +1,15 @@ import base64 import os -import platform import subprocess import tempfile from pathlib import Path -from netrc import netrc -import stat import click from rich.console import Console from crewai.cli import git from crewai.cli.command import BaseCommand, PlusAPIMixin +from crewai.cli.config import Settings from crewai.cli.utils import ( get_project_description, get_project_name, @@ -153,26 +151,16 @@ class ToolCommand(BaseCommand, PlusAPIMixin): raise SystemExit login_response_json = login_response.json() - self._set_netrc_credentials(login_response_json["credential"]) + + settings = Settings() + settings.tool_repository_username = login_response_json["credential"]["username"] + settings.tool_repository_password = login_response_json["credential"]["password"] + settings.dump() console.print( "Successfully authenticated to the tool repository.", style="bold green" ) - def _set_netrc_credentials(self, credentials, netrc_path=None): - if not netrc_path: - netrc_filename = "_netrc" if platform.system() == "Windows" else ".netrc" - netrc_path = Path.home() / netrc_filename - netrc_path.touch(mode=stat.S_IRUSR | stat.S_IWUSR, exist_ok=True) - - netrc_instance = netrc(file=netrc_path) - netrc_instance.hosts["app.crewai.com"] = (credentials["username"], "", credentials["password"]) - - with open(netrc_path, 'w') as file: - file.write(str(netrc_instance)) - - console.print(f"Added credentials to {netrc_path}", style="bold green") - def _add_package(self, tool_details): tool_handle = tool_details["handle"] repository_handle = tool_details["repository"]["handle"] @@ -187,7 +175,11 @@ class ToolCommand(BaseCommand, PlusAPIMixin): tool_handle, ] add_package_result = subprocess.run( - add_package_command, capture_output=False, text=True, check=True + add_package_command, + capture_output=False, + env=self._build_env_with_credentials(repository_handle), + text=True, + check=True ) if add_package_result.stderr: @@ -206,3 +198,13 @@ class ToolCommand(BaseCommand, PlusAPIMixin): "[bold yellow]Tip:[/bold yellow] Navigate to a different directory and try again." ) raise SystemExit + + def _build_env_with_credentials(self, repository_handle: str): + repository_handle = repository_handle.upper().replace("-", "_") + settings = Settings() + + env = os.environ.copy() + env[f"UV_INDEX_{repository_handle}_USERNAME"] = str(settings.tool_repository_username or "") + env[f"UV_INDEX_{repository_handle}_PASSWORD"] = str(settings.tool_repository_password or "") + + return env diff --git a/tests/cli/config_test.py b/tests/cli/config_test.py new file mode 100644 index 000000000..1065d4730 --- /dev/null +++ b/tests/cli/config_test.py @@ -0,0 +1,109 @@ +import unittest +import json +import tempfile +import shutil +from pathlib import Path +from crewai.cli.config import Settings + +class TestSettings(unittest.TestCase): + def setUp(self): + self.test_dir = Path(tempfile.mkdtemp()) + self.config_path = self.test_dir / "settings.json" + + def tearDown(self): + shutil.rmtree(self.test_dir) + + def test_empty_initialization(self): + settings = Settings(config_path=self.config_path) + self.assertIsNone(settings.tool_repository_username) + self.assertIsNone(settings.tool_repository_password) + + def test_initialization_with_data(self): + settings = Settings( + config_path=self.config_path, + tool_repository_username="user1" + ) + self.assertEqual(settings.tool_repository_username, "user1") + self.assertIsNone(settings.tool_repository_password) + + def test_initialization_with_existing_file(self): + self.config_path.parent.mkdir(parents=True, exist_ok=True) + with self.config_path.open("w") as f: + json.dump({"tool_repository_username": "file_user"}, f) + + settings = Settings(config_path=self.config_path) + self.assertEqual(settings.tool_repository_username, "file_user") + + def test_merge_file_and_input_data(self): + self.config_path.parent.mkdir(parents=True, exist_ok=True) + with self.config_path.open("w") as f: + json.dump({ + "tool_repository_username": "file_user", + "tool_repository_password": "file_pass" + }, f) + + settings = Settings( + config_path=self.config_path, + tool_repository_username="new_user" + ) + self.assertEqual(settings.tool_repository_username, "new_user") + self.assertEqual(settings.tool_repository_password, "file_pass") + + def test_dump_new_settings(self): + settings = Settings( + config_path=self.config_path, + tool_repository_username="user1" + ) + settings.dump() + + with self.config_path.open("r") as f: + saved_data = json.load(f) + + self.assertEqual(saved_data["tool_repository_username"], "user1") + + def test_update_existing_settings(self): + self.config_path.parent.mkdir(parents=True, exist_ok=True) + with self.config_path.open("w") as f: + json.dump({"existing_setting": "value"}, f) + + settings = Settings( + config_path=self.config_path, + tool_repository_username="user1" + ) + settings.dump() + + with self.config_path.open("r") as f: + saved_data = json.load(f) + + self.assertEqual(saved_data["existing_setting"], "value") + self.assertEqual(saved_data["tool_repository_username"], "user1") + + def test_none_values(self): + settings = Settings( + config_path=self.config_path, + tool_repository_username=None + ) + settings.dump() + + with self.config_path.open("r") as f: + saved_data = json.load(f) + + self.assertIsNone(saved_data.get("tool_repository_username")) + + def test_invalid_json_in_config(self): + self.config_path.parent.mkdir(parents=True, exist_ok=True) + with self.config_path.open("w") as f: + f.write("invalid json") + + try: + settings = Settings(config_path=self.config_path) + self.assertIsNone(settings.tool_repository_username) + except json.JSONDecodeError: + self.fail("Settings initialization should handle invalid JSON") + + def test_empty_config_file(self): + self.config_path.parent.mkdir(parents=True, exist_ok=True) + self.config_path.touch() + + settings = Settings(config_path=self.config_path) + self.assertIsNone(settings.tool_repository_username) diff --git a/tests/cli/tools/test_main.py b/tests/cli/tools/test_main.py index e4fc19be3..3804fb9b4 100644 --- a/tests/cli/tools/test_main.py +++ b/tests/cli/tools/test_main.py @@ -82,6 +82,7 @@ def test_install_success(mock_get, mock_subprocess_run): capture_output=False, text=True, check=True, + env=unittest.mock.ANY ) assert "Succesfully installed sample-tool" in output