diff --git a/src/nytid/http_utils.py b/src/nytid/http_utils.py new file mode 100644 index 00000000..85028a7e --- /dev/null +++ b/src/nytid/http_utils.py @@ -0,0 +1,16 @@ +import requests +from urllib3.util.retry import Retry +from requests.adapters import HTTPAdapter + +retry_strategy = Retry( + total=10, + backoff_factor=1, + status_forcelist=[429, 500, 502, 503, 504], + allowed_methods=["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE", "POST"], + raise_on_status=False, +) + +adapter = HTTPAdapter(max_retries=retry_strategy) +http_session = requests.Session() +http_session.mount("http://", adapter) +http_session.mount("https://", adapter) diff --git a/src/nytid/storage/.gitignore b/src/nytid/storage/.gitignore index a655f6f0..2ef1eb5a 100644 --- a/src/nytid/storage/.gitignore +++ b/src/nytid/storage/.gitignore @@ -1,3 +1,6 @@ afs.py afs.tex - +git.py +git.tex +github.py +github.tex diff --git a/src/nytid/storage/Makefile b/src/nytid/storage/Makefile index 6a05d0e8..6a87d5ab 100644 --- a/src/nytid/storage/Makefile +++ b/src/nytid/storage/Makefile @@ -1,5 +1,7 @@ MODULES+= __init__.py MODULES+= afs.py +MODULES+= git.py +MODULES+= github.py .PHONY: all all: ${MODULES} diff --git a/src/nytid/storage/git.nw b/src/nytid/storage/git.nw new file mode 100644 index 00000000..e86c9b0c --- /dev/null +++ b/src/nytid/storage/git.nw @@ -0,0 +1,485 @@ +\chapter{Git storage backend, the \texttt{storage.git} module} + +In this chapter we describe how to use Git repositories as storage backends, +the [[storage.git]] module. + +Git provides version control and distributed storage, making it suitable for +collaborative data management. +Unlike AFS, Git doesn't have built-in access control—this module focuses on +repository operations. +Access control will be handled by a separate GitHub module +(\cref{GitHubStorage}). + + +\section{Design overview} + +We want to use Git repositories as storage backends with the same interface +as other storage modules. +The key differences from the local file system storage are: +\begin{description} +\item[Repository cloning] When opening a Git storage root, we clone the + repository if it doesn't exist locally. +\item[Synchronization] Before reading files, we pull changes. + After writing files, we commit and push changes. +\item[Working directory] Files are accessed in the Git working directory, not + directly in the repository. +\end{description} + +We contrast this with the AFS approach: +AFS provides access control through ACLs on directories. +Git provides access control through repository permissions (handled separately +by hosting platforms like GitHub). + + +\section{Code outline} + +The Git storage module inherits from the base [[storage.StorageRoot]] class. +We override methods to add Git synchronization around file operations. +<>= +from nytid import storage +import pathlib +import subprocess +import sys +import tempfile +from urllib.parse import urlparse + +<> +<> + +class StorageRoot(storage.StorageRoot): + """ + Manages a storage root in a Git repository. + """ + + def __init__(self, url: str): + """ + Uses Git repository at `url` as storage root. + Clones the repository if needed, or uses existing clone. + For local paths, uses the repository directly if it exists. + """ + <> + <> + <> + super().__init__(self.__repo_path) + + <> +@ + +We also add tests to verify Git storage functionality. +<>= +import pathlib +import tempfile +import shutil +import os +import subprocess +import pytest +from nytid.storage.git import * + +<> +<> +@ + + +\section{Repository initialization} + +When creating a Git [[StorageRoot]], we need to determine where to store the +local clone. +We use a consistent location based on the repository URL. + +First, we extract a usable directory name from the Git URL. +For remote repositories we include the host as well as the owner and +repository name so that similarly named repositories on different hosts do not +collide in the cache. +<>= +self.__url = url +self.__repo_name = self.__extract_repo_name(url) +self.__repo_path = self.__get_local_repo_path(self.__repo_name) +@ + +We parse the repository name from various Git URL formats: +\begin{itemize} +\item SSH format: [[git@github.com:user/repo.git]] +\item HTTPS format: [[https://github.com/user/repo.git]] +\item Local path: [[/path/to/repo.git]] +\end{itemize} + +<>= +def extract_repo_name(url: str) -> str: + """ + Extracts repository name from Git URL. + Examples: + 'git@github.com:user/repo.git' -> 'github_com_user_repo' + 'https://github.com/user/repo.git' -> 'github_com_user_repo' + """ + # Remove .git suffix if present + if url.endswith(".git"): + url = url[:-4] + + host = "local" + + # Extract host and last two path components (user/repo) + if ":" in url and "@" in url: + # SSH format: git@github.com:user/repo + host, path = url.split("@", maxsplit=1)[1].split(":", maxsplit=1) + parts = path.split("/") + elif url.startswith("http"): + # HTTPS format: https://github.com/user/repo + parsed = urlparse(url) + host = parsed.hostname or "remote" + parts = parsed.path.split("/") + else: + # Local path format + parts = url.split("/") + + host = host.replace(".", "_") + + # Take last two non-empty parts and join with underscore + non_empty = [p for p in parts if p] + if len(non_empty) >= 2: + return f"{host}_{non_empty[-2]}_{non_empty[-1]}" + elif len(non_empty) == 1: + return f"{host}_{non_empty[-1]}" + else: + return f"{host}_repo" +@ + +We define a method in the class to use this helper. +<>= +def __extract_repo_name(self, url: str) -> str: + """Extracts repository name from URL""" + return extract_repo_name(url) +@ + +We store Git repositories in a cache directory under the user's home. +<>= +def get_local_repo_path(repo_name: str) -> pathlib.Path: + """ + Returns path where Git repository should be cloned locally. + Uses XDG cache directory structure. + """ + cache_dir = pathlib.Path.home() / ".cache" / "nytid" / "git-repos" + return cache_dir / repo_name +@ + +<>= +def __get_local_repo_path(self, repo_name: str) -> pathlib.Path: + """Returns local path for repository""" + return get_local_repo_path(repo_name) +@ + + +\section{Cloning and updating repositories} + +For remote URLs, we clone the repository to a local cache directory. +For local paths that already exist and are Git repositories, we use them +directly without cloning. + +First, we check if the URL is a local path and if it exists. +<>= +local_path = pathlib.Path(self.__url) +self.__is_local = local_path.exists() and local_path.is_dir() +if self.__is_local: + # Use the local path directly, don't use cache + self.__repo_path = local_path + <> +else: + # Remote URL - will clone to cache + pass +@ + +When the repository doesn't exist locally, we clone it. +If it already exists, we verify it's a valid Git repository. +<>= +if not self.__is_local: + if not self.__repo_path.exists(): + <> + else: + <> + <> +@ + +To clone a repository, we run [[git clone]]. +<>= +try: + self.__repo_path.parent.mkdir(parents=True, exist_ok=True) + run_git_command(["clone", self.__url, str(self.__repo_path)]) +except GitError as err: + raise GitError(f"Failed to clone repository {self.__url}: {err}") +@ + +For existing repositories, we verify they are valid Git repositories. +<>= +try: + run_git_command(["status"], cwd=self.__repo_path) +except GitError as err: + raise GitError( + f"Directory {self.__repo_path} exists but is not a valid Git repository" + ) +@ + +Before reading files, we pull the latest changes from the remote. +<>= +try: + run_git_command(["pull"], cwd=self.__repo_path) +except GitError as err: + # Pull failure is not fatal - we can work with local changes + pass +@ + + +\section{Running Git commands} + +We create a helper function to run Git commands, similar to how the AFS module +runs [[fs]] and [[pts]] commands. +<>= +def run_git_command(args, cwd=None): + """ + Runs a Git command with the given arguments. + + Args: + args: List of command arguments (e.g., ['clone', 'url', 'path']) + cwd: Working directory for the command + + Raises: + GitError: If the command fails + """ + cmd = ["git"] + args + try: + result = subprocess.run( + cmd, + cwd=cwd, + check=True, + capture_output=True, + text=True + ) + return result.stdout + except subprocess.CalledProcessError as err: + raise GitError( + f"Git command failed: {' '.join(cmd)}\n" + f"Error: {err.stderr}" + ) + except FileNotFoundError: + raise GitError("Git is not installed or not in PATH") +@ + +We define an exception for Git errors. +<>= +class GitError(Exception): + pass +@ + + +\section{Committing and pushing changes} + +When files are modified through the storage interface, we need to commit and +push changes back to the remote repository. + +We override the file closing behavior to commit changes after writing. +However, the base class [[open]] method uses Python's built-in [[open]], +which doesn't provide hooks for post-close operations. + +Instead, we provide explicit methods for committing changes. +Users should call [[commit_changes]] after modifying files. +<>= +def commit_changes(self, message: str = "Update from nytid"): + """ + Commits and pushes all changes in the repository. + For local repositories without remotes, only commits locally. + + Args: + message: Commit message + """ + try: + # Stage all changes + run_git_command(["add", "-A"], cwd=self.__repo_path) + + # Check if there are changes to commit + try: + run_git_command( + ["diff", "--cached", "--quiet"], + cwd=self.__repo_path + ) + # No changes to commit + return + except GitError: + # Changes exist, proceed with commit + pass + + # Commit changes + run_git_command( + ["commit", "-m", message], + cwd=self.__repo_path + ) + + # Try to push changes if there's a remote configured + try: + run_git_command(["push"], cwd=self.__repo_path) + except GitError: + # No remote configured or push failed - that's OK for local repos + pass + except GitError as err: + raise GitError(f"Failed to commit changes: {err}") +@ + +We also provide a method to pull the latest changes explicitly. +<>= +def pull_changes(self): + """ + Pulls the latest changes from the remote repository. + """ + try: + run_git_command(["pull"], cwd=self.__repo_path) + except GitError as err: + raise GitError(f"Failed to pull changes: {err}") +@ + + +\section{Access control methods} + +The base [[StorageRoot]] class defines [[grant_access]] and [[revoke_access]] +methods. +For basic Git storage, we cannot implement access control as it requires +interaction with the Git hosting platform. + +We override these methods to raise [[NotImplementedError]] with a helpful +message. +<>= +def grant_access(self, user): + """ + Access control is not available for basic Git storage. + Use the GitHub storage backend for access control. + """ + raise NotImplementedError( + "Access control requires a Git hosting platform. " + "Use nytid.storage.github for GitHub repositories." + ) + +def revoke_access(self, user): + """ + Access control is not available for basic Git storage. + Use the GitHub storage backend for access control. + """ + raise NotImplementedError( + "Access control requires a Git hosting platform. " + "Use nytid.storage.github for GitHub repositories." + ) +@ + + +\section{Testing the Git storage module} + +We test the Git storage module using a temporary Git repository. +We use pytest fixtures for proper setup and teardown. +<>= +@pytest.fixture(scope="module") +def test_repo(): + """Create a temporary Git repository for testing""" + # Create a temporary directory for test repositories + test_dir = tempfile.mkdtemp() + test_repo_path = pathlib.Path(test_dir) / "test-repo" + + # Initialize a test Git repository + test_repo_path.mkdir(parents=True) + orig_dir = os.getcwd() + os.chdir(test_repo_path) + subprocess.run(["git", "init"], check=True, capture_output=True) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], + check=True, + capture_output=True + ) + subprocess.run( + ["git", "config", "user.name", "Test User"], + check=True, + capture_output=True + ) + + # Create initial commit + (test_repo_path / "README.md").write_text("# Test Repository\n") + subprocess.run(["git", "add", "README.md"], check=True, capture_output=True) + subprocess.run( + ["git", "commit", "-m", "Initial commit"], + check=True, + capture_output=True + ) + + os.chdir(orig_dir) + + yield test_repo_path + + # Cleanup + shutil.rmtree(test_dir) +@ + +We test basic repository cloning and file operations. +<>= +def test_clone_and_read(test_repo): + """Test cloning a repository and reading a file""" + root = StorageRoot(str(test_repo)) + + with root.open("README.md", "r") as f: + content = f.read() + assert "Test Repository" in content + + root.close() +@ + +We test writing to a file and committing changes. +<>= +def test_write_and_commit(test_repo): + """Test writing a file and committing changes""" + root = StorageRoot(str(test_repo)) + + with root.open("test.txt", "w") as f: + f.write("Test content") + + root.commit_changes("Add test file") + + # Verify the file exists + with root.open("test.txt", "r") as f: + assert f.read() == "Test content" + + root.close() +@ + +We test that access control methods raise [[NotImplementedError]]. +<>= +def test_access_control_not_implemented(test_repo): + """Test that access control methods are not implemented""" + root = StorageRoot(str(test_repo)) + + try: + root.grant_access("test_user") + assert False, "Should have raised NotImplementedError" + except NotImplementedError as err: + assert "GitHub" in str(err) + + try: + root.revoke_access("test_user") + assert False, "Should have raised NotImplementedError" + except NotImplementedError as err: + assert "GitHub" in str(err) +@ + +We test repository name extraction from various URL formats. +<>= +def test_extract_repo_name(): + """Test extracting repository name from various URL formats""" + assert ( + extract_repo_name("git@github.com:user/repo.git") + == "github_com_user_repo" + ) + assert ( + extract_repo_name("https://github.com/user/repo.git") + == "github_com_user_repo" + ) + assert ( + extract_repo_name("https://github.com/user/repo") + == "github_com_user_repo" + ) + assert extract_repo_name("/path/to/user/repo.git") == "local_user_repo" + assert ( + extract_repo_name("git@gitlab.com:user/repo.git") + == "gitlab_com_user_repo" + ) +@ diff --git a/src/nytid/storage/github.nw b/src/nytid/storage/github.nw new file mode 100644 index 00000000..1f30476c --- /dev/null +++ b/src/nytid/storage/github.nw @@ -0,0 +1,433 @@ +\chapter{GitHub storage backend, the \texttt{storage.github} module} +\label{GitHubStorage} + +In this chapter we describe how to use GitHub repositories as storage backends +with access control, the [[storage.github]] module. + +GitHub provides both version control through Git and access control through +repository collaborators and teams. +This module extends the Git storage module to add access control capabilities. + + +\section{Design overview} + +We want to extend the Git storage module with GitHub-specific access control. +The key additions to the basic Git storage are: +\begin{description} +\item[Collaborator management] Add and remove repository collaborators using + the GitHub API. +\item[Permission levels] Grant different permission levels (read, write, admin). +\item[GitHub Enterprise support] Work with both github.com and self-hosted + GitHub Enterprise instances. +\end{description} + +We contrast this with the Git approach: +Git has no built-in access control. +GitHub provides access control through the web interface and API. + + +\section{Code outline} + +The GitHub storage module inherits from the [[git.StorageRoot]] class. +We override the access control methods to use the GitHub API. +<>= +from nytid.storage import git +import os +import re +import requests + +<> +<> + +class StorageRoot(git.StorageRoot): + """ + Manages a storage root in a GitHub repository with access control. + """ + + def __init__(self, url: str, token: str = None): + """ + Uses GitHub repository at `url` as storage root. + + Args: + url: GitHub repository URL (SSH or HTTPS format) + token: GitHub personal access token for API access. + If not provided, uses GITHUB_TOKEN environment variable. + """ + <> + super().__init__(url) + + <> +@ + +We also add tests to verify GitHub storage functionality. +<>= +import pathlib +import tempfile +import shutil +import os +import subprocess +import pytest +from unittest.mock import Mock, patch, MagicMock +from nytid.storage.github import * + +<> +<> +@ + + +\section{GitHub API initialization} + +When creating a GitHub [[StorageRoot]], we extract the repository owner and +name from the URL and configure the GitHub API endpoint. +We can defer authentication until we actually need to call the GitHub API, +because ordinary Git clone, pull, and push operations may rely on SSH keys or +credential helpers instead of a personal access token. + +First, we get the GitHub token from the parameter or environment if one is +available. +<>= +self.__token = token or os.environ.get("GITHUB_TOKEN") +<> +<> +<> +@ + +We parse the repository owner and name from various GitHub URL formats. +<>= +owner, repo_name = extract_github_repo_info(url) +self.__owner = owner +self.__repo = repo_name +@ + +We need a helper function to extract owner and repository name. +<>= +def extract_github_repo_info(url: str) -> tuple: + """ + Extracts GitHub repository owner and name from URL. + + Examples: + 'git@github.com:user/repo.git' -> ('user', 'repo') + 'https://github.com/user/repo.git' -> ('user', 'repo') + 'https://github.enterprise.com/user/repo' -> ('user', 'repo') + + Returns: + Tuple of (owner, repo_name) + """ + # Remove .git suffix if present + if url.endswith(".git"): + url = url[:-4] + + # Extract owner/repo from different formats + if ":" in url and "@" in url: + # SSH format: git@github.com:owner/repo + parts = url.split(":")[-1] + else: + # HTTPS format: https://github.com/owner/repo + parts = "/".join(url.split("/")[-2:]) + + # Split owner/repo + components = parts.split("/") + if len(components) >= 2: + return (components[-2], components[-1]) + else: + raise GitHubError(f"Cannot extract owner/repo from URL: {url}") +@ + +We determine the GitHub API endpoint based on the URL. +For github.com, we use [[https://api.github.com]]. +For GitHub Enterprise, we extract the hostname and use +[[https://hostname/api/v3]]. +<>= +hostname = extract_github_hostname(url) +if hostname == "github.com": + self.__api_base = "https://api.github.com" +else: + # GitHub Enterprise + self.__api_base = f"https://{hostname}/api/v3" +@ + +Helper function to extract hostname from URL. +<>= +def extract_github_hostname(url: str) -> str: + """ + Extracts hostname from GitHub URL. + + Examples: + 'git@github.enterprise.com:user/repo.git' -> 'github.enterprise.com' + 'https://github.enterprise.com/user/repo' -> 'github.enterprise.com' + """ + if url.startswith("http"): + # HTTPS format + match = re.match(r"https?://([^/]+)", url) + if match: + return match.group(1) + elif "@" in url: + # SSH format + match = re.match(r"git@([^:]+):", url) + if match: + return match.group(1) + + raise GitHubError(f"Cannot extract hostname from URL: {url}") +@ + +We configure the HTTP headers for GitHub API requests. +We use the newer JSON accept header recommended by GitHub. +<>= +self.__headers = None +if self.__token: + self.__headers = { + "Authorization": f"token {self.__token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28" + } +@ + +When we need to call the GitHub API, we ensure that a token is available. +<>= +def __require_headers(self): + """Returns GitHub API headers, requiring a token when needed.""" + if self.__headers is None: + raise GitHubError( + "GitHub token required. Provide token parameter or set GITHUB_TOKEN " + "environment variable." + ) + return self.__headers +@ + +We define an exception for GitHub API errors. +<>= +class GitHubError(Exception): + pass +@ + + +\section{Granting access to users} + +We implement the [[grant_access]] method to add users as repository +collaborators. +GitHub supports different permission levels: read, write, and admin. + +<>= +def grant_access(self, user: str, permission: str = "push"): + """ + Grants access to the GitHub repository for a user. + + Args: + user: GitHub username + permission: Permission level - 'pull' (read), 'push' (write), or 'admin' + + Raises: + GitHubError: If the API request fails + """ + <> + <> +@ + +We validate that the permission level is one of the supported values. +<>= +valid_permissions = ["pull", "push", "admin"] +if permission not in valid_permissions: + raise GitHubError( + f"Invalid permission '{permission}'. " + f"Must be one of: {', '.join(valid_permissions)}" + ) +@ + +We use the GitHub API to add the user as a collaborator. +<>= +url = ( + f"{self.__api_base}/repos/{self.__owner}/{self.__repo}/" + f"collaborators/{user}" +) +data = {"permission": permission} + +try: + response = requests.put(url, headers=self.__require_headers(), json=data) + response.raise_for_status() +except requests.exceptions.RequestException as err: + raise GitHubError( + f"Failed to grant access to user '{user}': {err}" + ) +@ + + +\section{Revoking access from users} + +We implement the [[revoke_access]] method to remove users as repository +collaborators. + +<>= +def revoke_access(self, user: str): + """ + Revokes access to the GitHub repository for a user. + + Args: + user: GitHub username + + Raises: + GitHubError: If the API request fails + """ + <> +@ + +We use the GitHub API to remove the user as a collaborator. +<>= +url = ( + f"{self.__api_base}/repos/{self.__owner}/{self.__repo}/" + f"collaborators/{user}" +) + +try: + response = requests.delete(url, headers=self.__require_headers()) + response.raise_for_status() +except requests.exceptions.RequestException as err: + raise GitHubError( + f"Failed to revoke access for user '{user}': {err}" + ) +@ + + +\section{Testing the GitHub storage module} + +We test the GitHub storage module using mocks for the GitHub API. +This allows us to test without requiring actual GitHub API access. + +<>= +@pytest.fixture +def mock_github_token(): + """Mock GitHub token for testing""" + return "test_github_token_12345" + +@pytest.fixture +def github_url(): + """Sample GitHub repository URL""" + return "https://github.com/testuser/testrepo.git" + +@pytest.fixture +def github_enterprise_url(): + """Sample GitHub Enterprise repository URL""" + return "https://github.enterprise.com/testuser/testrepo.git" +@ + +We test extracting repository information from URLs. +<>= +def test_extract_github_repo_info(): + """Test extracting owner and repo name from GitHub URLs""" + owner, repo = extract_github_repo_info("git@github.com:user/repo.git") + assert owner == "user" + assert repo == "repo" + + owner, repo = extract_github_repo_info("https://github.com/user/repo.git") + assert owner == "user" + assert repo == "repo" + + owner, repo = extract_github_repo_info("https://github.com/user/repo") + assert owner == "user" + assert repo == "repo" +@ + +We test extracting hostname from various URL formats. +<>= +def test_extract_github_hostname(): + """Test extracting hostname from GitHub URLs""" + hostname = extract_github_hostname("git@github.enterprise.com:user/repo.git") + assert hostname == "github.enterprise.com" + + hostname = extract_github_hostname("https://github.enterprise.com/user/repo") + assert hostname == "github.enterprise.com" + + hostname = extract_github_hostname("git@github.com:user/repo.git") + assert hostname == "github.com" +@ + +We test the initialization and API endpoint detection. +<>= +@patch.dict(os.environ, {"GITHUB_TOKEN": "test_token"}) +@patch("nytid.storage.github.git.StorageRoot.__init__") +def test_github_api_endpoint_detection(mock_super_init, github_url): + """Test GitHub API endpoint is correctly determined""" + mock_super_init.return_value = None + + root = StorageRoot(github_url) + assert root._StorageRoot__api_base == "https://api.github.com" + assert root._StorageRoot__owner == "testuser" + assert root._StorageRoot__repo == "testrepo" + +@patch.dict(os.environ, {"GITHUB_TOKEN": "test_token"}) +@patch("nytid.storage.github.git.StorageRoot.__init__") +def test_github_enterprise_api_endpoint(mock_super_init, github_enterprise_url): + """Test GitHub Enterprise API endpoint is correctly determined""" + mock_super_init.return_value = None + + root = StorageRoot(github_enterprise_url) + assert root._StorageRoot__api_base == "https://github.enterprise.com/api/v3" +@ + +We test granting access with mocked API calls. +<>= +@patch.dict(os.environ, {"GITHUB_TOKEN": "test_token"}) +@patch("nytid.storage.github.git.StorageRoot.__init__") +@patch("nytid.storage.github.requests.put") +def test_grant_access(mock_put, mock_super_init, github_url): + """Test granting access to a user""" + mock_super_init.return_value = None + mock_put.return_value = Mock(status_code=201) + + root = StorageRoot(github_url) + root.grant_access("testuser", "push") + + mock_put.assert_called_once() + call_args = mock_put.call_args + assert "collaborators/testuser" in call_args[0][0] + assert call_args[1]["json"]["permission"] == "push" + +@patch.dict(os.environ, {"GITHUB_TOKEN": "test_token"}) +@patch("nytid.storage.github.git.StorageRoot.__init__") +def test_grant_access_invalid_permission(mock_super_init, github_url): + """Test that invalid permission raises error""" + mock_super_init.return_value = None + + root = StorageRoot(github_url) + + try: + root.grant_access("testuser", "invalid") + assert False, "Should have raised GitHubError" + except GitHubError as err: + assert "Invalid permission" in str(err) +@ + +We test revoking access with mocked API calls. +<>= +@patch.dict(os.environ, {"GITHUB_TOKEN": "test_token"}) +@patch("nytid.storage.github.git.StorageRoot.__init__") +@patch("nytid.storage.github.requests.delete") +def test_revoke_access(mock_delete, mock_super_init, github_url): + """Test revoking access from a user""" + mock_super_init.return_value = None + mock_delete.return_value = Mock(status_code=204) + + root = StorageRoot(github_url) + root.revoke_access("testuser") + + mock_delete.assert_called_once() + call_args = mock_delete.call_args + assert "collaborators/testuser" in call_args[0][0] +@ + +We test that missing GitHub token only raises an error when access control is +requested. +<>= +@patch.dict(os.environ, {}, clear=True) +@patch("nytid.storage.github.git.StorageRoot.__init__") +def test_missing_github_token(mock_super_init, github_url): + """Test that access control requires a token""" + mock_super_init.return_value = None + + root = StorageRoot(github_url) + + try: + root.grant_access("testuser") + assert False, "Should have raised GitHubError" + except GitHubError as err: + assert "token required" in str(err) +@ diff --git a/src/nytid/storage/init.nw b/src/nytid/storage/init.nw index 53f3b9ad..8ed9e349 100644 --- a/src/nytid/storage/init.nw +++ b/src/nytid/storage/init.nw @@ -81,9 +81,13 @@ module) and the path (supplied to [[open_root]]). We want a class [[StorageRoot]] that encapsulates a directory and allows us to open files inside that directory or subdirectories. -<<[[init.py]]>>= +<>= import os import pathlib +import re +from urllib.parse import urlparse + +<> class StorageRoot: """ @@ -117,6 +121,34 @@ def open_root(*args, **kwargs) -> StorageRoot: <> @ +The [[open_root]] helper needs a few predicates to distinguish local paths, +generic Git remotes, and GitHub-hosted repositories without relying on brittle +substring matches. +<>= +def extract_git_host(target: str): + """Returns the host for a remote Git target, or [[None]].""" + parsed = urlparse(target) + if parsed.scheme in ("http", "https", "ssh", "git") and parsed.hostname: + return parsed.hostname + + match = re.match(r"^[^@]+@([^:]+):.+$", target) + if match: + return match.group(1) + + return None + + +def is_github_host(host: str) -> bool: + """Returns whether [[host]] refers to GitHub or GitHub Enterprise.""" + return host == "github.com" or host.startswith("github.") + + +def is_local_git_repository(target: str) -> bool: + """Returns whether [[target]] is a local Git working tree.""" + path = pathlib.Path(target) + return path.is_dir() and (path / ".git").exists() +@ + We also add the tests in a test file that will be used by [[pytest]]. We need a temporary directory that we can use as the root. <>= @@ -326,16 +358,16 @@ if args[0].startswith("/afs"): import nytid.storage.afs as storage_module @ -If the URL ([[args[0]]]) ends with [[.git]] then we should use the Git module. -However, the Git repo can be hosted in the AFS file system. -It could also be hosted on GitHub. -In both these cases, permission management is different. -This means that the [[git.StorageRoot]] must be able to handle a parent -[[StorageRoot]] for permissions. -But we defer that handling to the [[storage.git]] module. -Thus we don't use [[elif]], but rather replace any existing [[storage_module]]. +If the URL ([[args[0]]]) refers to a GitHub host (like [[github.com]] or +[[github.enterprise.com]]), we should use the GitHub module for access control +support. +Otherwise, if it is another remote Git URL or a local Git working tree, we use +the generic Git module. <>= -if args[0].endswith(".git"): +git_host = extract_git_host(args[0]) +if git_host and is_github_host(git_host): + import nytid.storage.github as storage_module +elif git_host or is_local_git_repository(args[0]): import nytid.storage.git as storage_module @ @@ -355,17 +387,19 @@ return root @ Let's test this. -Note that the Git test should fail since that module doesn't exist yet. +The Git/GitHub tests should now succeed. Also, the test for the local root is flawed; since any of the others inherit from that class, all will be an instance of it. But we can test that it's not an instance of the AFS class at least. <>= def test_open_root(): + from unittest.mock import patch from nytid.storage import afs - + afs_dir = "/afs/kth.se/home/d/b/dbosk/nytid-nonexisting-test-directory" local_dir = "/home/dbosk/nytid-nonexisting-test-directory" - git_repo = "git@github.com:dbosk/nytid.git" + github_repo = "git@github.com:dbosk/nytid.git" + git_repo = "https://gitlab.com/dbosk/nytid" afs_root = open_root(afs_dir) assert isinstance(afs_root, afs.StorageRoot) @@ -374,10 +408,23 @@ def test_open_root(): assert isinstance(local_root, StorageRoot) assert not isinstance(local_root, afs.StorageRoot) - try: - git_root = open_root(git_repo) - except ModuleNotFoundError: - assert True - else: - assert False + with patch("nytid.storage.github.StorageRoot") as github_root_class: + github_root = object() + github_root_class.return_value = github_root + assert open_root(github_repo) is github_root + github_root_class.assert_called_once_with(github_repo) + + with patch("nytid.storage.git.StorageRoot") as git_root_class: + git_root = object() + git_root_class.return_value = git_root + assert open_root(git_repo) is git_root + git_root_class.assert_called_once_with(git_repo) + + with tempfile.TemporaryDirectory() as git_dir: + pathlib.Path(git_dir, ".git").mkdir() + with patch("nytid.storage.git.StorageRoot") as git_root_class: + git_root = object() + git_root_class.return_value = git_root + assert open_root(git_dir) is git_root + git_root_class.assert_called_once_with(git_dir) @