From b7ba5402dd11b9e3ec39e45032ff7031643c78ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vykintas=20Baltru=C5=A1aitis?= Date: Tue, 28 Nov 2023 13:48:31 +0200 Subject: [PATCH 1/4] Config dependabot to only request security updates --- .github/dependabot.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index b38df29f4..5589d2f24 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,3 +4,5 @@ updates: directory: "/" schedule: interval: "daily" + # This setting does not affect security updates + open-pull-requests-limit: 0 \ No newline at end of file From b838c90b6fa44670b3ca8de67e0a70d2ce717234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vykintas=20Baltru=C5=A1aitis?= Date: Tue, 28 Nov 2023 13:52:37 +0200 Subject: [PATCH 2/4] Add town crier update --- .../+dependabot-disable-non-security-updates.infrastructure.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/+dependabot-disable-non-security-updates.infrastructure.md diff --git a/changelog.d/+dependabot-disable-non-security-updates.infrastructure.md b/changelog.d/+dependabot-disable-non-security-updates.infrastructure.md new file mode 100644 index 000000000..49dc84bac --- /dev/null +++ b/changelog.d/+dependabot-disable-non-security-updates.infrastructure.md @@ -0,0 +1 @@ +Disable dependabot requests for updates unrelated to security issues. \ No newline at end of file From 80e0a2d193db4b01e59a7d01a4082c6981994542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Nowacki?= Date: Thu, 23 Nov 2023 21:29:20 +0100 Subject: [PATCH 3/4] fix resource_tracker warning on OSX --- .github/workflows/ci.yml | 2 +- b2/console_tool.py | 169 +++++++++++---------- changelog.d/+fix_leaking_semaphores.fix.md | 1 + test/integration/helpers.py | 11 +- test/integration/test_tqdm_closer.py | 40 +++++ test/unit/conftest.py | 8 + 6 files changed, 145 insertions(+), 86 deletions(-) create mode 100644 changelog.d/+fix_leaking_semaphores.fix.md create mode 100644 test/integration/test_tqdm_closer.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cc21f907a..10cc36130 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: - name: Run linters run: nox -vs lint - name: Validate new changelog entries - if: contains(github.event.pull_request.labels.*.name, '-changelog') == false + if: (contains(github.event.pull_request.labels.*.name, '-changelog') == false) && (github.event.pull_request.base.ref != '') run: if [ -z "$(git diff --diff-filter=A --name-only origin/${{ github.event.pull_request.base.ref }} changelog.d)" ]; then echo no changelog item added; exit 1; fi diff --git a/b2/console_tool.py b/b2/console_tool.py index 05bc97963..f802ca29a 100644 --- a/b2/console_tool.py +++ b/b2/console_tool.py @@ -13,6 +13,7 @@ import argparse import base64 +import contextlib import csv import dataclasses import datetime @@ -84,6 +85,7 @@ ScanPoliciesManager, Synchronizer, SyncReport, + TqdmProgressListener, UploadMode, current_time_millis, get_included_sources, @@ -683,7 +685,31 @@ def _set_threads_from_args(self, args): self.api.services.download_manager.set_thread_pool_size(threads) -class Command(Described): +class _TqdmCloser: + """ + On OSX using Tqdm with b2sdk causes semaphore leaks. This fix is located here and not in b2sdk, because after this + cleanup Tqdm might not work properly, therefore it's best to do it when exiting a python process. + """ + + def __init__(self, progress_listener): + self.progress_listener = progress_listener + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + if sys.platform != "darwin" or os.environ.get('B2_TEST_DISABLE_TQDM_CLOSER'): + return + try: + from multiprocessing.synchronize import SemLock + tqdm_lock = self.progress_listener.tqdm.get_lock() + if tqdm_lock.mp_lock._semlock.name is not None: + SemLock._cleanup(tqdm_lock.mp_lock._semlock.name) + except Exception as ex: + logger.debug('Error encountered during Tqdm cleanup', exc_info=ex) + + +class Command(Described, metaclass=ABCMeta): # Set to True for commands that receive sensitive information in arguments FORBID_LOGGING_ARGUMENTS = False @@ -701,6 +727,14 @@ def __init__(self, console_tool): self.stdout = console_tool.stdout self.stderr = console_tool.stderr self.quiet = False + self.exit_stack = contextlib.ExitStack() + + def make_progress_listener(self, file_name: str, quiet: bool): + progress_listener = make_progress_listener(file_name, quiet) + self.exit_stack.enter_context(progress_listener) + if isinstance(progress_listener, TqdmProgressListener): + self.exit_stack.enter_context(_TqdmCloser(progress_listener)) + return progress_listener @classmethod def name_and_alias(cls): @@ -792,7 +826,12 @@ def create_parser( def run(self, args): self.quiet = args.quiet - return 0 + with self.exit_stack: + return self._run(args) + + @abstractmethod + def _run(self, args) -> int: + ... @classmethod def _setup_parser(cls, parser): @@ -947,8 +986,7 @@ class B2(Command): def name_and_alias(cls): return NAME, None - def run(self, args): - super().run(args) + def _run(self, args): # Commands could be named via name or alias, so we fetch # the command from args assigned during parser preparation. return args.command_class @@ -1003,8 +1041,7 @@ def _setup_parser(cls, parser): parser.add_argument('applicationKey', nargs='?') super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): # Handle internal options for testing inside Backblaze. # These are not documented in the usage string. realm = self._get_realm(args) @@ -1091,8 +1128,7 @@ def _setup_parser(cls, parser): parser.add_argument('bucketName').completer = bucket_name_completer super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): bucket = self.api.get_bucket_by_name(args.bucketName) for file_version in bucket.list_unfinished_large_files(): bucket.cancel_large_file(file_version.file_id) @@ -1118,8 +1154,7 @@ def _setup_parser(cls, parser): parser.add_argument('fileId') super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): self.api.cancel_large_file(args.fileId) self._print(args.fileId, 'canceled') return 0 @@ -1141,8 +1176,7 @@ class ClearAccount(Command): REQUIRES_AUTH = False - def run(self, args): - super().run(args) + def _run(self, args): self.api.account_info.clear() return 0 @@ -1205,8 +1239,7 @@ def _setup_parser(cls, parser): super()._setup_parser(parser) # add parameters from the mixins - def run(self, args): - super().run(args) + def _run(self, args): file_infos = None if args.info: file_infos = self._parse_file_infos(args.info) @@ -1326,8 +1359,7 @@ def _setup_parser(cls, parser): super()._setup_parser(parser) # add parameters from the mixins - def run(self, args): - super().run(args) + def _run(self, args): encryption_setting = self._get_default_sse_setting(args) bucket = self.api.create_bucket( args.bucketName, @@ -1382,8 +1414,7 @@ def _setup_parser(cls, parser): capabilities.add_argument('--allCapabilities', action='store_true') super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): # Translate the bucket name into a bucketId if args.bucket is None: bucket_id_or_none = None @@ -1420,8 +1451,7 @@ def _setup_parser(cls, parser): parser.add_argument('bucketName').completer = bucket_name_completer super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): bucket = self.api.get_bucket_by_name(args.bucketName) self.api.delete_bucket(bucket) return 0 @@ -1452,8 +1482,7 @@ def _setup_parser(cls, parser): super()._setup_parser(parser) parser.add_argument('--bypassGovernance', action='store_true', default=False) - def run(self, args): - super().run(args) + def _run(self, args): file_name = self._get_file_name_from_args(args) file_info = self.api.delete_file_version(args.fileId, file_name, args.bypassGovernance) self._print_json(file_info) @@ -1475,15 +1504,19 @@ def _setup_parser(cls, parser): parser.add_argument('applicationKeyId') super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): application_key = self.api.delete_key_by_id(application_key_id=args.applicationKeyId) self._print(application_key.id_) return 0 class DownloadCommand( - ProgressMixin, SourceSseMixin, WriteBufferSizeMixin, SkipHashVerificationMixin, Command + ProgressMixin, + SourceSseMixin, + WriteBufferSizeMixin, + SkipHashVerificationMixin, + Command, + metaclass=ABCMeta ): """ helper methods for returning results from download commands """ @@ -1589,9 +1622,8 @@ class DownloadFileBase( - **readFiles** """ - def run(self, args): - super().run(args) - progress_listener = make_progress_listener( + def _run(self, args): + progress_listener = self.make_progress_listener( args.localFileName, args.noProgress or args.quiet ) encryption_setting = self._get_source_sse_setting(args) @@ -1660,10 +1692,11 @@ class Cat(B2URIFileArgMixin, DownloadCommand): - **readFiles** """ - def run(self, args): - super().run(args) + def _run(self, args): target_filename = '-' - progress_listener = make_progress_listener(target_filename, args.noProgress or args.quiet) + progress_listener = self.make_progress_listener( + target_filename, args.noProgress or args.quiet + ) encryption_setting = self._get_source_sse_setting(args) file_request = self.api.download_file_by_uri( args.B2_URI, progress_listener=progress_listener, encryption=encryption_setting @@ -1680,8 +1713,7 @@ class GetAccountInfo(Command): the current application keys has. """ - def run(self, args): - super().run(args) + def _run(self, args): account_info = self.api.account_info data = dict( accountId=account_info.get_account_id(), @@ -1735,8 +1767,7 @@ def _setup_parser(cls, parser): parser.add_argument('bucketName').completer = bucket_name_completer super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): # This always wants up-to-date info, so it does not use # the bucket cache. for b in self.api.list_buckets(args.bucketName): @@ -1774,8 +1805,7 @@ class FileInfoBase(Command): - **readFiles** """ - def run(self, args): - super().run(args) + def _run(self, args): b2_uri = self.get_b2_uri_from_arg(args) file_version = self.api.get_file_info_by_uri(b2_uri) self._print_json(file_version) @@ -1818,8 +1848,7 @@ def _setup_parser(cls, parser): parser.add_argument('bucketName').completer = bucket_name_completer super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): bucket = self.api.get_bucket_by_name(args.bucketName) auth_token = bucket.get_download_authorization( file_name_prefix=args.prefix, valid_duration_in_seconds=args.duration @@ -1854,8 +1883,7 @@ def _setup_parser(cls, parser): parser.add_argument('fileName').completer = file_name_completer super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): bucket = self.api.get_bucket_by_name(args.bucketName) auth_token = bucket.get_download_authorization( file_name_prefix=args.fileName, valid_duration_in_seconds=args.duration @@ -1882,8 +1910,7 @@ def _setup_parser(cls, parser): parser.add_argument('fileName').completer = file_name_completer super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): bucket = self.api.get_bucket_by_name(args.bucketName) file_info = bucket.hide_file(args.fileName) self._print_json(file_info) @@ -1915,8 +1942,7 @@ def _setup_parser(cls, parser): parser.add_argument('--json', action='store_true') super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): buckets = self.api.list_buckets() if args.json: self._print_json(list(buckets)) @@ -1962,8 +1988,7 @@ def __init__(self, console_tool): super().__init__(console_tool) self.bucket_id_to_bucket_name = None - def run(self, args): - super().run(args) + def _run(self, args): for key in self.api.list_keys(): self.print_key(key, args.long) @@ -2027,8 +2052,7 @@ def _setup_parser(cls, parser): parser.add_argument('largeFileId') super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): for part in self.api.list_parts(args.largeFileId): self._print('%5d %9d %s' % (part.part_number, part.content_length, part.content_sha1)) return 0 @@ -2050,8 +2074,7 @@ def _setup_parser(cls, parser): parser.add_argument('bucketName').completer = bucket_name_completer super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): bucket = self.api.get_bucket_by_name(args.bucketName) for unfinished in bucket.list_unfinished_large_files(): file_info_text = ' '.join( @@ -2184,8 +2207,7 @@ def _setup_parser(cls, parser): parser.add_argument('--replication', action='store_true') super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): if args.json: i = -1 for i, (file_version, _) in enumerate(self._get_ls_generator(args)): @@ -2412,8 +2434,7 @@ def _setup_parser(cls, parser): parser.add_argument('--failFast', action='store_true') super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): if args.dryRun: self._print_files(args) return 0 @@ -2454,8 +2475,7 @@ class GetUrlBase(Command): it is public. """ - def run(self, args): - super().run(args) + def _run(self, args): b2_uri = self.get_b2_uri_from_arg(args) self._print(self.api.get_download_url_by_uri(b2_uri)) return 0 @@ -2689,8 +2709,7 @@ def _setup_parser(cls, parser): del_keep_group.add_argument('--delete', action='store_true') del_keep_group.add_argument('--keepDays', type=float, metavar='DAYS') - def run(self, args): - super().run(args) + def _run(self, args): policies_manager = self.get_policies_manager_from_args(args) if args.threads is not None: @@ -2903,8 +2922,7 @@ def _setup_parser(cls, parser): super()._setup_parser(parser) # add parameters from the mixins and the parent class - def run(self, args): - super().run(args) + def _run(self, args): if args.defaultRetentionMode is not None: if args.defaultRetentionMode == 'none': default_retention = NO_RETENTION_BUCKET_SETTING @@ -3003,8 +3021,7 @@ def _setup_parser(cls, parser): super()._setup_parser(parser) # add parameters from the mixins - def run(self, args): - super().run(args) + def _run(self, args): self._set_threads_from_args(args) upload_kwargs = self.get_execute_kwargs(args) file_info = self.execute_operation(**upload_kwargs) @@ -3053,7 +3070,7 @@ def get_execute_kwargs(self, args) -> dict: "min_part_size": args.minPartSize, "progress_listener": - make_progress_listener(args.localFilePath, args.noProgress or args.quiet), + self.make_progress_listener(args.localFilePath, args.noProgress or args.quiet), "sha1_sum": args.sha1, "threads": @@ -3261,8 +3278,7 @@ def _setup_parser(cls, parser): super()._setup_parser(parser) parser.add_argument('legalHold', choices=(LegalHold.ON.value, LegalHold.OFF.value)) - def run(self, args): - super().run(args) + def _run(self, args): file_name = self._get_file_name_from_args(args) legal_hold = LegalHold(args.legalHold) self.api.update_file_legal_hold(args.fileId, file_name, legal_hold) @@ -3312,8 +3328,7 @@ def _setup_parser(cls, parser): ) parser.add_argument('--bypassGovernance', action='store_true', default=False) - def run(self, args): - super().run(args) + def _run(self, args): file_name = self._get_file_name_from_args(args) if args.retentionMode == 'none': @@ -3371,8 +3386,7 @@ def _setup_parser(cls, parser): help='if given, also replicates files uploaded prior to creation of the replication rule' ) - def run(self, args): - super().run(args) + def _run(self, args): if args.destination_profile is None: destination_api = self.api else: @@ -3398,8 +3412,7 @@ def _setup_parser(cls, parser): parser.add_argument('source', metavar='SOURCE_BUCKET_NAME') parser.add_argument('rule_name', metavar='REPLICATION_RULE_NAME') - def run(self, args): - super().run(args) + def _run(self, args): bucket = self.api.get_bucket_by_name(args.source).get_fresh_state() found, altered = self.alter_rule_by_name(bucket, args.rule_name) if not found: @@ -3545,8 +3558,7 @@ def _setup_parser(cls, parser): metavar='COLUMN ONE,COLUMN TWO' ) - def run(self, args): - super().run(args) + def _run(self, args): destination_api = args.destination_profile and _get_b2api_for_profile( args.destination_profile ) @@ -3687,8 +3699,7 @@ def _setup_parser(cls, parser): parser.add_argument('--short', action='store_true') super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): if args.short: self._print(VERSION) else: @@ -3751,8 +3762,7 @@ def _setup_parser(cls, parser): ) super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): if self.LICENSE_OUTPUT_FILE.exists() and not args.dump: self._print(self.LICENSE_OUTPUT_FILE.read_text(encoding='utf8')) return 0 @@ -3906,8 +3916,7 @@ def _setup_parser(cls, parser): parser.add_argument('--shell', choices=SUPPORTED_SHELLS, default=None) super()._setup_parser(parser) - def run(self, args): - super().run(args) + def _run(self, args): shell = args.shell or detect_shell() if shell not in SUPPORTED_SHELLS: self._print_stderr( diff --git a/changelog.d/+fix_leaking_semaphores.fix.md b/changelog.d/+fix_leaking_semaphores.fix.md new file mode 100644 index 000000000..57b1f39f5 --- /dev/null +++ b/changelog.d/+fix_leaking_semaphores.fix.md @@ -0,0 +1 @@ +fix an error that caused multiprocessing semaphores to leak on OSX \ No newline at end of file diff --git a/test/integration/helpers.py b/test/integration/helpers.py index 264f15045..a8777072e 100755 --- a/test/integration/helpers.py +++ b/test/integration/helpers.py @@ -30,7 +30,6 @@ from os import environ, linesep, path from pathlib import Path from tempfile import gettempdir, mkdtemp, mktemp -from unittest.mock import MagicMock import backoff from b2sdk.v2 import ( @@ -286,10 +285,8 @@ def print_text_indented(text): """ Prints text that may include weird characters, indented four spaces. """ - mock_console_tool = MagicMock() - cmd_instance = Command(console_tool=mock_console_tool) for line in text.split(linesep): - cmd_instance._print_standard_descriptor(sys.stdout, ' ', repr(line)[1:-1]) + Command._print_helper(sys.stdout, sys.stdout.encoding, ' ', repr(line)[1:-1]) def print_output(status, stdout, stderr): @@ -426,6 +423,7 @@ def should_succeed( args: list[str] | None, expected_pattern: str | None = None, additional_env: dict | None = None, + expected_stderr_pattern: str | re.Pattern = None, ) -> str: """ Runs the command-line with the given arguments. Raises an exception @@ -435,7 +433,10 @@ def should_succeed( status, stdout, stderr = self.execute(args, additional_env) assert status == 0, f'FAILED with status {status}, stderr={stderr}' - if stderr != '': + if expected_stderr_pattern: + assert expected_stderr_pattern.search(stderr), \ + f'stderr did not match pattern="{expected_stderr_pattern}", stderr="{stderr}"' + elif stderr != '': for line in (s.strip() for s in stderr.split(os.linesep)): assert any(p.match(line) for p in self.EXPECTED_STDERR_PATTERNS), \ f'Unexpected stderr line: {repr(line)}' diff --git a/test/integration/test_tqdm_closer.py b/test/integration/test_tqdm_closer.py new file mode 100644 index 000000000..a97a43102 --- /dev/null +++ b/test/integration/test_tqdm_closer.py @@ -0,0 +1,40 @@ +###################################################################### +# +# File: test/integration/test_tqdm_closer.py +# +# Copyright 2023 Backblaze Inc. All Rights Reserved. +# +# License https://www.backblaze.com/using_b2_code.html +# +###################################################################### +import re +import sys + +import pytest + + +@pytest.mark.skipif( + (sys.platform != 'darwin') or ((sys.version_info.major, sys.version_info.minor) < (3, 11)), + reason='Tqdm closing error only occurs on OSX and python 3.11 or newer', +) +def test_tqdm_closer(b2_tool, bucket, file_name): + # test that stderr doesn't contain any warning, in particular warnings about multiprocessing resource tracker + # leaking semaphores + b2_tool.should_succeed([ + 'cat', + f'b2://{bucket.name}/{file_name}', + ]) + + # test that disabling _TqdmCloser does produce a resource tracker warning. Should the following check ever fail, + # that would mean that either Tqdm or python fixed the issue and _TqdmCloser can be disabled for fixed versions + b2_tool.should_succeed( + [ + 'cat', + f'b2://{bucket.name}/{file_name}', + ], + additional_env={'B2_TEST_DISABLE_TQDM_CLOSER': '1'}, + expected_stderr_pattern=re.compile( + r'UserWarning: resource_tracker: There appear to be \d+ leaked semaphore' + r' objects to clean up at shutdown' + ), + ) diff --git a/test/unit/conftest.py b/test/unit/conftest.py index c21344b75..edcba882a 100644 --- a/test/unit/conftest.py +++ b/test/unit/conftest.py @@ -13,6 +13,8 @@ import pytest from b2sdk.raw_api import REALM_URLS +from b2.console_tool import _TqdmCloser + @pytest.fixture(autouse=True, scope='session') def mock_realm_urls(): @@ -25,3 +27,9 @@ def bg_executor(): """Executor for running background tasks in tests""" with RunOrDieExecutor() as executor: yield executor + + +@pytest.fixture(autouse=True) +def disable_tqdm_closer_cleanup(): + with mock.patch.object(_TqdmCloser, '__exit__'): + yield From e8114e39480e9b16ec4fbc8788cad367baaae3fa Mon Sep 17 00:00:00 2001 From: Maciej Urbanski Date: Tue, 21 Nov 2023 13:37:50 +0100 Subject: [PATCH 4/4] don't print 'Using realm' unless realm was explicitly changed by user --- b2/console_tool.py | 23 ++-- changelog.d/949.fixed.md | 1 + test/unit/console_tool/conftest.py | 17 +++ .../console_tool/test_authorize_account.py | 124 ++++++++++++++++++ test/unit/test_console_tool.py | 121 +---------------- 5 files changed, 163 insertions(+), 123 deletions(-) create mode 100644 changelog.d/949.fixed.md create mode 100644 test/unit/console_tool/test_authorize_account.py diff --git a/b2/console_tool.py b/b2/console_tool.py index f802ca29a..8fe8b4bc7 100644 --- a/b2/console_tool.py +++ b/b2/console_tool.py @@ -1044,7 +1044,7 @@ def _setup_parser(cls, parser): def _run(self, args): # Handle internal options for testing inside Backblaze. # These are not documented in the usage string. - realm = self._get_realm(args) + realm = self._get_user_requested_realm(args) if args.applicationKeyId is None: args.applicationKeyId = ( @@ -1060,17 +1060,21 @@ def _run(self, args): return self.authorize(args.applicationKeyId, args.applicationKey, realm) - def authorize(self, application_key_id, application_key, realm): + def authorize(self, application_key_id, application_key, realm: str | None): """ Perform the authorization and capability checks, report errors. :param application_key_id: application key ID used to authenticate :param application_key: application key - :param realm: authorization realm + :param realm: authorization realm; if None, production is used :return: exit status """ + verbose_realm = bool(realm) + realm = realm or 'production' url = REALM_URLS.get(realm, realm) - self._print_stderr(f"Using {url}") + logger.info(f"Using {url}") + if verbose_realm: + self._print_stderr(f'Using {url}') try: self.api.authorize_account(realm, application_key_id, application_key) @@ -1099,7 +1103,10 @@ def authorize(self, application_key_id, application_key, realm): return 1 @classmethod - def _get_realm(cls, args): + def _get_user_requested_realm(cls, args) -> str | None: + """ + Determine the realm to use for authorization. + """ if args.dev: return 'dev' if args.staging: @@ -1107,7 +1114,7 @@ def _get_realm(cls, args): if args.environment: return args.environment - return os.environ.get(B2_ENVIRONMENT_ENV_VAR, 'production') + return os.environ.get(B2_ENVIRONMENT_ENV_VAR) @B2.register_subcommand @@ -4032,9 +4039,9 @@ def authorize_from_env(self, command_class): f'Please provide both "{B2_APPLICATION_KEY_ENV_VAR}" and "{B2_APPLICATION_KEY_ID_ENV_VAR}" environment variables or none of them' ) return 1 - realm = os.environ.get(B2_ENVIRONMENT_ENV_VAR, 'production') + realm = os.environ.get(B2_ENVIRONMENT_ENV_VAR) - if self.api.account_info.is_same_key(key_id, realm): + if self.api.account_info.is_same_key(key_id, realm or 'production'): return 0 logger.info('authorize-account is being run from env variables') diff --git a/changelog.d/949.fixed.md b/changelog.d/949.fixed.md new file mode 100644 index 000000000..72b54cf28 --- /dev/null +++ b/changelog.d/949.fixed.md @@ -0,0 +1 @@ +Don't print `Using https://REALM" in stderr unless explicitly set by user. diff --git a/test/unit/console_tool/conftest.py b/test/unit/console_tool/conftest.py index aae3562fc..cd9bae128 100644 --- a/test/unit/console_tool/conftest.py +++ b/test/unit/console_tool/conftest.py @@ -13,6 +13,8 @@ import pytest +import b2.console_tool + class ConsoleToolTester(BaseConsoleToolTest): def authorize(self): @@ -22,6 +24,21 @@ def run(self, *args, **kwargs): return self._run_command(*args, **kwargs) +@pytest.fixture +def cwd_path(tmp_path): + """Set up a test directory and return its path.""" + prev_cwd = os.getcwd() + os.chdir(tmp_path) + yield tmp_path + os.chdir(prev_cwd) + + +@pytest.fixture +def b2_cli_log_fix(caplog): + caplog.set_level(0) # prevent pytest from blocking logs + b2.console_tool.logger.setLevel(0) # reset logger level to default + + @pytest.fixture def b2_cli(): cli_tester = ConsoleToolTester() diff --git a/test/unit/console_tool/test_authorize_account.py b/test/unit/console_tool/test_authorize_account.py new file mode 100644 index 000000000..85201b047 --- /dev/null +++ b/test/unit/console_tool/test_authorize_account.py @@ -0,0 +1,124 @@ +###################################################################### +# +# File: test/unit/console_tool/test_authorize_account.py +# +# Copyright 2023 Backblaze Inc. All Rights Reserved. +# +# License https://www.backblaze.com/using_b2_code.html +# +###################################################################### +from unittest import mock + +import pytest + +from b2._cli.const import ( + B2_APPLICATION_KEY_ENV_VAR, + B2_APPLICATION_KEY_ID_ENV_VAR, + B2_ENVIRONMENT_ENV_VAR, +) + + +@pytest.fixture +def b2_cli_is_authorized_afterwards(b2_cli): + assert b2_cli.account_info.get_account_auth_token() is None + yield b2_cli + assert b2_cli.account_info.get_account_auth_token() is not None + + +def test_authorize_with_bad_key(b2_cli): + expected_stdout = "" + expected_stderr = """ + ERROR: unable to authorize account: Invalid authorization token. Server said: secret key is wrong (unauthorized) + """ + + b2_cli._run_command( + ["authorize-account", b2_cli.account_id, "bad-app-key"], + expected_stdout, + expected_stderr, + 1, + ) + assert b2_cli.account_info.get_account_auth_token() is None + + +@pytest.mark.parametrize( + "command", + [ + "authorize-account", + "authorize_account", + ], +) +def test_authorize_with_good_key(b2_cli, b2_cli_is_authorized_afterwards, command): + assert b2_cli.account_info.get_account_auth_token() is None + + expected_stderr = """ + """ + + b2_cli._run_command([command, b2_cli.account_id, b2_cli.master_key], "", expected_stderr, 0) + + assert b2_cli.account_info.get_account_auth_token() is not None + + +def test_authorize_using_env_variables(b2_cli): + assert b2_cli.account_info.get_account_auth_token() is None + + expected_stderr = """ + """ + + with mock.patch.dict( + "os.environ", + { + B2_APPLICATION_KEY_ID_ENV_VAR: b2_cli.account_id, + B2_APPLICATION_KEY_ENV_VAR: b2_cli.master_key, + }, + ): + b2_cli._run_command(["authorize-account"], "", expected_stderr, 0) + + assert b2_cli.account_info.get_account_auth_token() is not None + + +@pytest.mark.parametrize( + "flags,realm_url", + [ + ([], "http://production.example.com"), + (["--debugLogs"], "http://production.example.com"), + (["--environment", "http://custom.example.com"], "http://custom.example.com"), + (["--environment", "production"], "http://production.example.com"), + (["--dev"], "http://api.backblazeb2.xyz:8180"), + (["--staging"], "https://api.backblaze.net"), + ], +) +def test_authorize_towards_realm( + b2_cli, b2_cli_is_authorized_afterwards, flags, realm_url, cwd_path, b2_cli_log_fix +): + expected_stderr = f"Using {realm_url}\n" if any(f != "--debugLogs" for f in flags) else "" + + b2_cli._run_command( + ["authorize-account", *flags, b2_cli.account_id, b2_cli.master_key], + "", + expected_stderr, + 0, + ) + log_path = cwd_path / "b2_cli.log" + if "--debugLogs" in flags: + assert f"Using {realm_url}\n" in log_path.read_text() + else: + assert not log_path.exists() + + +def test_authorize_towards_custom_realm_using_env(b2_cli, b2_cli_is_authorized_afterwards): + expected_stderr = """ + Using http://custom2.example.com + """ + + with mock.patch.dict( + "os.environ", + { + B2_ENVIRONMENT_ENV_VAR: "http://custom2.example.com", + }, + ): + b2_cli._run_command( + ["authorize-account", b2_cli.account_id, b2_cli.master_key], + "", + expected_stderr, + 0, + ) diff --git a/test/unit/test_console_tool.py b/test/unit/test_console_tool.py index dd58ac162..8593797e0 100644 --- a/test/unit/test_console_tool.py +++ b/test/unit/test_console_tool.py @@ -256,106 +256,6 @@ def _upload_multiple_files(cls, bucket): class TestConsoleTool(BaseConsoleToolTest): - def test_authorize_with_bad_key(self): - expected_stdout = '' - expected_stderr = ''' - Using http://production.example.com - ERROR: unable to authorize account: Invalid authorization token. Server said: secret key is wrong (unauthorized) - ''' - - self._run_command( - ['authorize-account', self.account_id, 'bad-app-key'], expected_stdout, expected_stderr, - 1 - ) - - def test_authorize_with_good_key_using_hyphen(self): - # Initial condition - assert self.account_info.get_account_auth_token() is None - - # Authorize an account with a good api key. - expected_stderr = """ - Using http://production.example.com - """ - - self._run_command( - ['authorize-account', self.account_id, self.master_key], '', expected_stderr, 0 - ) - - # Auth token should be in account info now - assert self.account_info.get_account_auth_token() is not None - - def test_authorize_with_good_key_using_underscore(self): - # Initial condition - assert self.account_info.get_account_auth_token() is None - - # Authorize an account with a good api key. - expected_stderr = """ - Using http://production.example.com - """ - self._run_command( - ['authorize_account', self.account_id, self.master_key], '', expected_stderr, 0 - ) - - # Auth token should be in account info now - assert self.account_info.get_account_auth_token() is not None - - def test_authorize_using_env_variables(self): - # Initial condition - assert self.account_info.get_account_auth_token() is None - - # Authorize an account with a good api key. - expected_stderr = """ - Using http://production.example.com - """ - - # Setting up environment variables - with mock.patch.dict( - 'os.environ', { - B2_APPLICATION_KEY_ID_ENV_VAR: self.account_id, - B2_APPLICATION_KEY_ENV_VAR: self.master_key, - } - ): - assert B2_APPLICATION_KEY_ID_ENV_VAR in os.environ - assert B2_APPLICATION_KEY_ENV_VAR in os.environ - - self._run_command(['authorize-account'], '', expected_stderr, 0) - - # Auth token should be in account info now - assert self.account_info.get_account_auth_token() is not None - - def test_authorize_towards_custom_realm(self): - # Initial condition - assert self.account_info.get_account_auth_token() is None - - # Authorize an account with a good api key. - expected_stderr = """ - Using http://custom.example.com - """ - - # realm provided with args - self._run_command( - [ - 'authorize-account', '--environment', 'http://custom.example.com', self.account_id, - self.master_key - ], '', expected_stderr, 0 - ) - - # Auth token should be in account info now - assert self.account_info.get_account_auth_token() is not None - - expected_stderr = """ - Using http://custom2.example.com - """ - # realm provided with env var - with mock.patch.dict( - 'os.environ', { - B2_ENVIRONMENT_ENV_VAR: 'http://custom2.example.com', - } - ): - self._run_command( - ['authorize-account', self.account_id, self.master_key], '', expected_stderr, 0 - ) - def test_create_key_and_authorize_with_it(self): # Start with authorizing with the master key self._authorize_account() @@ -372,7 +272,7 @@ def test_create_key_and_authorize_with_it(self): self._run_command( ['authorize-account', 'appKeyId0', 'appKey0'], '', - 'Using http://production.example.com\n', + '', 0, ) @@ -396,7 +296,7 @@ def test_create_key_with_authorization_from_env_vars(self): self._run_command( ['create-key', 'key1', 'listBuckets,listKeys'], 'appKeyId0 appKey0\n', - 'Using http://production.example.com\n', + '', 0, ) @@ -418,7 +318,7 @@ def test_create_key_with_authorization_from_env_vars(self): self._run_command( ['create-key', 'key1', 'listBuckets,listKeys'], 'appKeyId2 appKey2\n', - 'Using http://production.example.com\n', + '', 0, ) @@ -445,7 +345,6 @@ def test_authorize_key_without_list_buckets(self): self._run_command( ['authorize-account', 'appKeyId0', 'appKey0'], '', - 'Using http://production.example.com\n' 'ERROR: application key has no listBuckets capability, which is required for the b2 command-line tool\n', 1, ) @@ -519,7 +418,7 @@ def test_create_bucket_key_and_authorize_with_it(self): self._run_command( ['authorize-account', 'appKeyId0', 'appKey0'], '', - 'Using http://production.example.com\n', + '', 0, ) @@ -770,10 +669,7 @@ def test_keys(self): self._run_command(['list-keys', '--long'], expected_list_keys_out_long, '', 0) # authorize and make calls using application key with no restrictions - self._run_command( - ['authorize-account', 'appKeyId0', 'appKey0'], '', - 'Using http://production.example.com\n', 0 - ) + self._run_command(['authorize-account', 'appKeyId0', 'appKey0'], '', '', 0) self._run_command( ['list-buckets'], 'bucket_0 allPublic my-bucket-a\nbucket_2 allPublic my-bucket-c\n', '', 0 @@ -796,10 +692,7 @@ def test_keys(self): self._run_command(['get-bucket', 'my-bucket-a'], expected_json_in_stdout=expected_json) # authorize and make calls using an application key with bucket restrictions - self._run_command( - ['authorize-account', 'appKeyId1', 'appKey1'], '', - 'Using http://production.example.com\n', 0 - ) + self._run_command(['authorize-account', 'appKeyId1', 'appKey1'], '', '', 0) self._run_command( ['list-buckets'], '', 'ERROR: Application key is restricted to bucket: my-bucket-a\n', 1 @@ -2350,7 +2243,6 @@ def test_list_buckets_not_allowed_for_app_key(self): self._run_command( ['authorize-account', 'appKeyId0', 'appKey0'], '', - 'Using http://production.example.com\n' 'ERROR: application key has no listBuckets capability, which is required for the b2 command-line tool\n', 1, ) @@ -2379,7 +2271,6 @@ def test_bucket_missing_for_bucket_key(self): self._run_command( ['authorize-account', 'appKeyId0', 'appKey0'], '', - "Using http://production.example.com\n" "ERROR: unable to authorize account: Application key is restricted to a bucket that doesn't exist\n", 1, )