Added explicit configuration option storage-redirect-disable + fix …

…linter issues

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
+      https://docs.aws.amazon.com/sdk-for-go/api/aws/#LogLevelType

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)

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()
+        assert expected_storage['filesystem'].items() == actual_storage_config.items()