diff --git a/.envrc b/.envrc index 9d01ead4..a5ae4ac0 100644 --- a/.envrc +++ b/.envrc @@ -18,9 +18,22 @@ if [ -d ".venv" ]; then fi fi -python -m uv venv .venv --python $PYTHON_VERSION -python -m uv pip install -U pip uv -python -m uv pip install -e . +if [ ! -d ".venv" ] +then + # System python doesn't like us installing packages into it + # Test if uv is already installed (via brew or other package manager etc.) + # and use that if available. Otherwise fall back to previous behvaiour + if command -v uv + then + uv venv .venv --python $PYTHON_VERSION + uv pip install -U pip uv + uv pip install -e . + else + python -m uv venv .venv --python $PYTHON_VERSION + python -m uv pip install -U pip uv + python -m uv pip install -e . + fi +fi source ./.venv/bin/activate diff --git a/cloudsmith_cli/cli/commands/main.py b/cloudsmith_cli/cli/commands/main.py index d6586536..47390931 100644 --- a/cloudsmith_cli/cli/commands/main.py +++ b/cloudsmith_cli/cli/commands/main.py @@ -1,7 +1,12 @@ """Main command/entrypoint.""" +from functools import partial + import click +from cloudsmith_cli.cli import config +from cloudsmith_cli.cli.warnings import get_or_create_warnings + from ...core.api.version import get_version as get_api_version from ...core.utils import get_github_website, get_help_website from ...core.version import get_version as get_cli_version @@ -51,13 +56,37 @@ def print_version(): is_flag=True, is_eager=True, ) +@click.option( + "--no-warn", + help="Don't display auth or config warnings", + envvar="CLOUDSMITH_CLI_NO_WARN", + is_flag=True, + default=None, +) @decorators.common_cli_config_options @click.pass_context -def main(ctx, opts, version): +def main(ctx, opts, version, no_warn): """Handle entrypoint to CLI.""" # pylint: disable=unused-argument + if no_warn: + opts.no_warn = True + if version: + opts.no_warn = True print_version() elif ctx.invoked_subcommand is None: click.echo(ctx.get_help()) + + +@main.result_callback() +@click.pass_context +def result_callback(ctx, _, **kwargs): + """Callback for main function. Required for saving warnings til the end.""" + + warnings = get_or_create_warnings(ctx) + opts = config.get_or_create_options(ctx) + + if warnings and not opts.no_warn: + click_warn_partial = partial(click.secho, fg="yellow") + warnings.display(click_warn_partial) diff --git a/cloudsmith_cli/cli/config.py b/cloudsmith_cli/cli/config.py index e7638f8e..07a49e07 100644 --- a/cloudsmith_cli/cli/config.py +++ b/cloudsmith_cli/cli/config.py @@ -7,6 +7,8 @@ import click from click_configfile import ConfigFileReader, Param, SectionSchema, matches_section +from cloudsmith_cli.cli.warnings import ConfigLoadWarning, ProfileNotFoundWarning + from ..core.utils import get_data_path, read_file from . import utils, validators @@ -148,8 +150,9 @@ def has_default_file(cls): return False @classmethod - def load_config(cls, opts, path=None, profile=None): + def load_config(cls, opts, path=None, warnings=None, profile=None): """Load a configuration file into an options object.""" + if path and os.path.exists(path): if os.path.isdir(path): cls.config_searchpath.insert(0, path) @@ -159,10 +162,25 @@ def load_config(cls, opts, path=None, profile=None): config = cls.read_config() values = config.get("default", {}) cls._load_values_into_opts(opts, values) + existing_config_paths = { + path: os.path.exists(path) for path in cls.config_files + } if profile and profile != "default": - values = config.get("profile:%s" % profile, {}) - cls._load_values_into_opts(opts, values) + try: + values = config["profile:%s" % profile] + cls._load_values_into_opts(opts, values) + except KeyError: + warning = ProfileNotFoundWarning( + paths=existing_config_paths, profile=profile + ) + warnings.append(warning) + + if not any(list(existing_config_paths.values())): + config_load_warning = ConfigLoadWarning( + paths=existing_config_paths, + ) + warnings.append(config_load_warning) return values @@ -206,7 +224,31 @@ class CredentialsReader(ConfigReader): config_searchpath = list(_CFG_SEARCH_PATHS) config_section_schemas = [CredentialsSchema.Default, CredentialsSchema.Profile] + @classmethod + def load_config(cls, opts, path=None, warnings=None, profile=None): + """ + Load a credentials configuration file into an options object. + We overload the load_config command in CredentialsReader as + credentials files have their own specific default functionality. + """ + if path and os.path.exists(path): + if os.path.isdir(path): + cls.config_searchpath.insert(0, path) + else: + cls.config_files.insert(0, path) + + config = cls.read_config() + values = config.get("default", {}) + cls._load_values_into_opts(opts, values) + + if profile and profile != "default": + values = config.get("profile:%s" % profile, {}) + cls._load_values_into_opts(opts, values) + + return values + +# pylint: disable=too-many-public-methods class Options: """Options object that holds config for the application.""" @@ -227,15 +269,15 @@ def get_creds_reader(): """Get the credentials config reader class.""" return CredentialsReader - def load_config_file(self, path, profile=None): + def load_config_file(self, path, warnings=None, profile=None): """Load the standard config file.""" config_cls = self.get_config_reader() - return config_cls.load_config(self, path, profile=profile) + return config_cls.load_config(self, path, warnings=warnings, profile=profile) - def load_creds_file(self, path, profile=None): + def load_creds_file(self, path, warnings=None, profile=None): """Load the credentials config file.""" config_cls = self.get_creds_reader() - return config_cls.load_config(self, path, profile=profile) + return config_cls.load_config(self, path, warnings=warnings, profile=profile) @property def api_config(self): @@ -268,6 +310,16 @@ def api_host(self, value): """Set value for API host.""" self._set_option("api_host", value) + @property + def no_warn(self): + """Get value for API host.""" + return self._get_option("no_warn") + + @no_warn.setter + def no_warn(self, value): + """Set value for API host.""" + self._set_option("no_warn", value) + @property def api_key(self): """Get value for API key.""" diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index a8b41173..d7f38931 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -4,7 +4,10 @@ import click +from cloudsmith_cli.cli.warnings import ApiAuthenticationWarning, get_or_create_warnings + from ..core.api.init import initialise_api as _initialise_api +from ..core.api.user import get_user_brief from . import config, utils, validators @@ -91,11 +94,12 @@ def common_cli_config_options(f): def wrapper(ctx, *args, **kwargs): # pylint: disable=missing-docstring opts = config.get_or_create_options(ctx) + warnings = get_or_create_warnings(ctx) profile = kwargs.pop("profile") config_file = kwargs.pop("config_file") creds_file = kwargs.pop("credentials_file") - opts.load_config_file(path=config_file, profile=profile) - opts.load_creds_file(path=creds_file, profile=profile) + opts.load_config_file(path=config_file, profile=profile, warnings=warnings) + opts.load_creds_file(path=creds_file, profile=profile, warnings=warnings) kwargs["opts"] = opts return ctx.invoke(f, *args, **kwargs) @@ -305,6 +309,13 @@ def call_print_rate_limit_info_with_opts(rate_info): error_retry_cb=opts.error_retry_cb, ) + cloudsmith_host = kwargs["opts"].opts["api_config"].host + is_auth, _, _, _ = get_user_brief() + if not is_auth: + warnings = get_or_create_warnings(ctx) + auth_warning = ApiAuthenticationWarning(cloudsmith_host) + warnings.append(auth_warning) + kwargs["opts"] = opts return ctx.invoke(f, *args, **kwargs) diff --git a/cloudsmith_cli/cli/tests/commands/policy/test_licence.py b/cloudsmith_cli/cli/tests/commands/policy/test_licence.py index 75bc388c..7941c73e 100644 --- a/cloudsmith_cli/cli/tests/commands/policy/test_licence.py +++ b/cloudsmith_cli/cli/tests/commands/policy/test_licence.py @@ -85,7 +85,9 @@ def assert_output_matches_policy_config(output, config_file_path): return output_table["Identifier"] -@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.usefixtures( + "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var" +) def test_license_policy_commands(runner, organization, tmp_path): """Test CRUD operations for license policies.""" diff --git a/cloudsmith_cli/cli/tests/commands/policy/test_vulnerability.py b/cloudsmith_cli/cli/tests/commands/policy/test_vulnerability.py index de03d046..54d389d9 100644 --- a/cloudsmith_cli/cli/tests/commands/policy/test_vulnerability.py +++ b/cloudsmith_cli/cli/tests/commands/policy/test_vulnerability.py @@ -87,7 +87,9 @@ def assert_output_matches_policy_config(output, config_file_path): return output_table["Identifier"] -@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.usefixtures( + "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var" +) def test_vulnerability_policy_commands(runner, organization, tmp_path): """Test CRUD operations for vulnerability policies.""" diff --git a/cloudsmith_cli/cli/tests/commands/test_package_commands.py b/cloudsmith_cli/cli/tests/commands/test_package_commands.py index 0da9879d..3c9af02c 100644 --- a/cloudsmith_cli/cli/tests/commands/test_package_commands.py +++ b/cloudsmith_cli/cli/tests/commands/test_package_commands.py @@ -11,7 +11,9 @@ from ..utils import random_str -@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.usefixtures( + "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var" +) @pytest.mark.parametrize( "filesize", [ @@ -80,7 +82,9 @@ def test_push_and_delete_raw_package( assert len(data) == 0 -@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.usefixtures( + "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var" +) def test_list_packages_with_sort(runner, organization, tmp_repository, tmp_path): """Test listing packages with different sort options.""" org_repo = f'{organization}/{tmp_repository["slug"]}' diff --git a/cloudsmith_cli/cli/tests/commands/test_repos.py b/cloudsmith_cli/cli/tests/commands/test_repos.py index d879b197..cc601544 100644 --- a/cloudsmith_cli/cli/tests/commands/test_repos.py +++ b/cloudsmith_cli/cli/tests/commands/test_repos.py @@ -72,7 +72,9 @@ def assert_output_is_equal_to_repo_config(output, organisation, repo_config_file ) -@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.usefixtures( + "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var" +) def test_repos_commands(runner, organization, tmp_path): """Test CRUD operations for repositories.""" diff --git a/cloudsmith_cli/cli/tests/commands/test_upstream.py b/cloudsmith_cli/cli/tests/commands/test_upstream.py index 4559eef0..5283a9b1 100644 --- a/cloudsmith_cli/cli/tests/commands/test_upstream.py +++ b/cloudsmith_cli/cli/tests/commands/test_upstream.py @@ -6,7 +6,9 @@ from ..utils import random_str -@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.usefixtures( + "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var" +) @pytest.mark.parametrize("upstream_format", UPSTREAM_FORMATS) def test_upstream_commands( runner, organization, upstream_format, tmp_repository, tmp_path diff --git a/cloudsmith_cli/cli/tests/conftest.py b/cloudsmith_cli/cli/tests/conftest.py index 5e4936a7..b4e50a4f 100644 --- a/cloudsmith_cli/cli/tests/conftest.py +++ b/cloudsmith_cli/cli/tests/conftest.py @@ -73,3 +73,9 @@ def set_api_host_env_var(api_host): def set_api_key_env_var(api_key): """Set the CLOUDSMITH_API_KEY environment variable.""" os.environ["CLOUDSMITH_API_KEY"] = api_key + + +@pytest.fixture() +def set_no_warn_env_var(): + """Set the CLOUDSMITH_API_KEY environment variable.""" + os.environ["CLOUDSMITH_CLI_NO_WARN"] = "True" diff --git a/cloudsmith_cli/cli/tests/test_warnings.py b/cloudsmith_cli/cli/tests/test_warnings.py new file mode 100644 index 00000000..df57480a --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_warnings.py @@ -0,0 +1,26 @@ +from cloudsmith_cli.cli.warnings import ( + ApiAuthenticationWarning, + CliWarnings, + ConfigLoadWarning, + ProfileNotFoundWarning, +) + + +class TestWarnings: + def test_warning_append(self): + """Test appending warnings to the CliWarnings.""" + + config_load_warning_1 = ConfigLoadWarning({"test_path_1": False}) + config_load_warning_2 = ConfigLoadWarning({"test_path_2": True}) + profile_load_warning = ProfileNotFoundWarning( + {"test_path_1": False}, "test_profile" + ) + api_authentication_warning = ApiAuthenticationWarning("test.cloudsmith.io") + cli_warnings = CliWarnings() + cli_warnings.append(config_load_warning_1) + cli_warnings.append(config_load_warning_2) + cli_warnings.append(profile_load_warning) + cli_warnings.append(profile_load_warning) + cli_warnings.append(api_authentication_warning) + assert len(cli_warnings) == 5 + assert len(cli_warnings.__dedupe__()) == 4 diff --git a/cloudsmith_cli/cli/warnings.py b/cloudsmith_cli/cli/warnings.py new file mode 100644 index 00000000..dc230e7e --- /dev/null +++ b/cloudsmith_cli/cli/warnings.py @@ -0,0 +1,106 @@ +from abc import ABC +from typing import Dict, List + + +class CliWarning(ABC): + """ + Abstract base class for all Cloudsmith CLI warnings. + """ + + def __repr__(self): + return self.__str__() + + def __hash__(self): + return hash(self.__str__()) + + def __eq__(self, other): + if not isinstance(other, self.__class__): + return False + + return self.__dict__ == other.__dict__ + + +class ConfigLoadWarning(CliWarning): + """ + Warning for issues loading the configuration file. + """ + + def __init__(self, paths: Dict[str, bool]): + self.paths = paths + self.message = "Failed to load config files. Tried the following paths: \n" + for path, exists in paths.items(): + self.message += f" - {path} - exists: {exists}\n" + self.message += "You may need to run `cloudsmith login` to authenticate and create a config file. \n" + + def __str__(self): + return f"{self.__class__.__name__} - {self.paths}" + + +class ProfileNotFoundWarning(CliWarning): + """ + Warning for issues finding the requested profile. + """ + + def __init__(self, paths, profile): + self.path = paths + self.profile = profile + self.message = f"Failed to load profile {profile} from config. Tried the following paths: \n" + for path, exists in paths.items(): + self.message += f" - {path} - exists: {exists}\n" + + def __str__(self): + return f"{self.__class__.__name__} - {self.path} - {self.profile}" + + +class ApiAuthenticationWarning(CliWarning): + """ + Warning for issues with API authentication. + """ + + def __init__(self, cloudsmith_host): + self.cloudsmith_host = cloudsmith_host + self.message = "".join( + [ + "Failed to authenticate with Cloudsmith API ", + "Please check your credentials and try again.\n", + f"Host: {cloudsmith_host}\n", + ] + ) + + def __str__(self): + return f"{self.__class__.__name__} - {self.cloudsmith_host}" + + +class CliWarnings(list): + """ + A class to manage warnings in the CLI. + """ + + def __init__(self): + super().__init__() + self.warnings: List[CliWarning] = [] + + def append(self, warning: CliWarning): + self.warnings.append(warning) + + def __dedupe__(self) -> List[CliWarning]: + return list(set(self.warnings)) + + def display(self, display_fn): + for warning in self.__dedupe__(): + display_fn(warning.message) + + def __repr__(self) -> str: + return self.__str__() + + def __str__(self) -> str: + return ",".join([str(x) for x in self.warnings]) + + def __len__(self) -> int: + return len(self.warnings) + + +def get_or_create_warnings(ctx): + """Get or create the warnings object.""" + + return ctx.ensure_object(CliWarnings)