From 8c6967beef21dc5cc1f159e4a3f13a89bfa21856 Mon Sep 17 00:00:00 2001 From: Alex Ramanau Date: Wed, 11 Sep 2024 10:58:19 +0200 Subject: [PATCH] Added explicit configuration option `storage-redirect-disable` + fix linter issues --- config.yaml | 10 ++++- lib/charms/layer/docker_registry.py | 15 ++++--- tests/unit/test_docker_registry.py | 66 ++++++++++++++++++++++++++++- 3 files changed, 83 insertions(+), 8 deletions(-) diff --git a/config.yaml b/config.yaml index 197ebca..d7008bb 100644 --- a/config.yaml +++ b/config.yaml @@ -96,6 +96,14 @@ options: description: | Enable/disable the "delete" storage option. False, the default, disables this option in the registry config file. + storage-redirect-disable: + type: boolean + default: true + description: | + For backends that support it(swift, s3), redirecting is disabled by default. + All data routed through the Registry, rather than redirecting to the backend. + If you want to redirect client requests directly to content storage, + set this option to false. storage-read-only: type: boolean default: false @@ -268,4 +276,4 @@ options: Valid values are: off (default), debug, debugwithsigning, debugwithhttpbody, debugwithrequestretries, debugwithrequesterrors and debugwitheventstreambody. See the AWS SDK for Go API reference for details: - https://docs.aws.amazon.com/sdk-for-go/api/aws/#LogLevelType \ No newline at end of file + https://docs.aws.amazon.com/sdk-for-go/api/aws/#LogLevelType diff --git a/lib/charms/layer/docker_registry.py b/lib/charms/layer/docker_registry.py index 8abee11..78d0393 100644 --- a/lib/charms/layer/docker_registry.py +++ b/lib/charms/layer/docker_registry.py @@ -142,12 +142,10 @@ def configure_registry(): val = charm_config.get('storage-swift-domain', '') if val != '': storage['swift'].update({'domain': val}) - - storage['redirect'] = {'disable': True} elif ( - charm_config.get('storage-s3-region') and - charm_config.get('storage-s3-bucket') - ): + charm_config.get('storage-s3-region') and + charm_config.get('storage-s3-bucket') + ): storage['s3'] = { 'region': charm_config.get('storage-s3-region'), 'bucket': charm_config.get('storage-s3-bucket'), @@ -199,6 +197,13 @@ def configure_registry(): storage['maintenance'] = {'readonly': {'enabled': True}} if charm_config.get('storage-cache', 'inmemory') == 'inmemory': storage['cache'] = {'blobdescriptor': 'inmemory'} + + storage_redirect_disable = charm_config.get('storage-redirect-disable', True) + + # by default redirect is disabled for S3 and Swift backends mostly because this + # charm is used in private clouds with private object storage + storage['redirect'] = {'disable': storage_redirect_disable} + registry_config['storage'] = storage os.makedirs(os.path.dirname(registry_config_file), exist_ok=True) diff --git a/tests/unit/test_docker_registry.py b/tests/unit/test_docker_registry.py index 7034ff2..271c468 100644 --- a/tests/unit/test_docker_registry.py +++ b/tests/unit/test_docker_registry.py @@ -114,12 +114,14 @@ def test_configure_registry_s3_storage_smoke(config, *args): layer.docker_registry.configure_registry() args, _ = mock_yaml.safe_dump.call_args_list[0] assert 'storage' in args[0] + redirect_config = args[0]['storage'].get('redirect', None) + assert redirect_config['disable'] is True + assert 's3' in args[0]['storage'] actual_storage_config = args[0]['storage']['s3'] assert expected_storage['s3'].items() == actual_storage_config.items() - @mock.patch("os.makedirs", mock.Mock(return_value=0)) @mock.patch("charms.layer.docker_registry._write_tls_blobs_to_files") @mock.patch("charms.layer.docker_registry._configure_local_client") @@ -157,6 +159,9 @@ def test_configure_registry_s3_storage_region_endpoint(config, *args): layer.docker_registry.configure_registry() args, _ = mock_yaml.safe_dump.call_args_list[0] assert 'storage' in args[0] + redirect_config = args[0]['storage'].get('redirect', None) + assert redirect_config['disable'] is True + assert 's3' in args[0]['storage'] actual_storage_config = args[0]['storage']['s3'] assert expected_storage['s3'].items() == actual_storage_config.items() @@ -204,6 +209,63 @@ def test_configure_registry_s3_storage_override_default(config, *args): assert expected_storage['s3'].items() == actual_storage_config.items() +@mock.patch("os.makedirs", mock.Mock(return_value=0)) +@mock.patch("charms.layer.docker_registry._write_tls_blobs_to_files") +@mock.patch("charms.layer.docker_registry._configure_local_client") +@mock.patch("charms.layer.docker_registry._write_tls_blobs_to_files") +@mock.patch("charms.layer.docker_registry.unitdata") +@mock.patch("charmhelpers.core.hookenv.config") +def test_configure_registry_s3_storage_enable_storage_redirect(config, *args): + config.return_value = { + "log-level": "info", + "storage-redirect-disable": False + } + with mock.patch("charms.layer.docker_registry.yaml") as mock_yaml: + layer.docker_registry.configure_registry() + args, _ = mock_yaml.safe_dump.call_args_list[0] + assert 'storage' in args[0] + + redirect_config = args[0]['storage'].get('redirect', None) + assert redirect_config['disable'] is False + + +@mock.patch("os.makedirs", mock.Mock(return_value=0)) +@mock.patch("charms.layer.docker_registry._write_tls_blobs_to_files") +@mock.patch("charms.layer.docker_registry._configure_local_client") +@mock.patch("charms.layer.docker_registry._write_tls_blobs_to_files") +@mock.patch("charms.layer.docker_registry.unitdata") +@mock.patch("charmhelpers.core.hookenv.config") +def test_configure_registry_swift_storage_redirect_disabled(config, *args): + config.return_value = { + "log-level": "info", + "storage-swift-authurl": "https://ns1-region-swift.internal", + "storage-swift-username": "test_user", + "storage-swift-password": "secret", + "storage-swift-region": "test", + } + expected_storage = { + "swift": { + "authurl": "https://ns1-region-swift.internal", + "username": "test_user", + "password": "secret", + "region": "test", + "container": "", + "tenant": "" + }, + } + with mock.patch("charms.layer.docker_registry.yaml") as mock_yaml: + layer.docker_registry.configure_registry() + args, _ = mock_yaml.safe_dump.call_args_list[0] + assert 'storage' in args[0] + redirect_config = args[0]['storage'].get('redirect', None) + # redirect has to be disabled by default for swift storage + assert redirect_config['disable'] is True + + assert 'swift' in args[0]['storage'] + actual_storage_config = args[0]['storage']['swift'] + assert expected_storage['swift'].items() == actual_storage_config.items() + + @mock.patch("os.makedirs", mock.Mock(return_value=0)) @mock.patch("charms.layer.docker_registry._write_tls_blobs_to_files") @mock.patch("charms.layer.docker_registry._configure_local_client") @@ -225,4 +287,4 @@ def test_configure_registry_default_file_storage(config, *args): assert 'storage' in args[0] assert 'filesystem' in args[0]['storage'] actual_storage_config = args[0]['storage']['filesystem'] - assert expected_storage['filesystem'].items() == actual_storage_config.items() \ No newline at end of file + assert expected_storage['filesystem'].items() == actual_storage_config.items()