fix(controller): revert release.check_image_access for now

Author: duanhongyi

rootfs/api/models/app.py

@@ -596,12 +596,8 @@ def deploy(self, release, force_deploy=False, rollback_on_failure=True):  # noqa
             # create the application config in k8s (secret in this case) for all deploy objects
             self.set_application_config(release)
             # only buildpack apps need access to object storage
-            # only docker apps need check access to the image, so users can't exploit the k8s
-            # image cache to gain access to other users' images
             if release.build.type == 'buildpack':
                 self.create_object_store_secret()
-            else:
-                release.check_image_access()
 
             # gather all proc types to be deployed
             tasks = [

rootfs/api/models/release.py

@@ -3,7 +3,7 @@
 from django.conf import settings
 from django.db import models
 
-from registry import publish_release, get_port as docker_get_port, check_access as docker_check_access, RegistryException # noqa
+from registry import publish_release, get_port as docker_get_port, RegistryException # noqa
 from api.utils import dict_diff
 from api.models import UuidAuditedModel
 from api.exceptions import DryccException, AlreadyExists
@@ -136,13 +136,6 @@ def publish(self):
         drycc_registry = bool(self.build.source_based)
         publish_release(source_image, self.image, drycc_registry, self.get_registry_auth())
 
-    def check_image_access(self):
-        try:
-            creds = self.get_registry_auth()
-            docker_check_access(self.image, creds)
-        except Exception as e:
-            raise DryccException(str(e)) from e
-
     def get_port(self):
         """
         Get application port for a given release. If pulling from private registry

rootfs/api/tests/test_app.py

@@ -35,7 +35,6 @@ def _mock_run(*args, **kwargs):
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
 @mock.patch('api.models.release.docker_get_port', mock_port)
-@mock.patch('api.models.release.docker_check_access', lambda *args: None)
 class AppTest(DryccTestCase):
     """Tests creation of applications"""
 

rootfs/api/tests/test_build.py

@@ -23,7 +23,6 @@
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
 @mock.patch('api.models.release.docker_get_port', mock_port)
-@mock.patch('api.models.release.docker_check_access', lambda *args: None)
 class BuildTest(DryccTransactionTestCase):
 
     """Tests build notification from build system"""
@@ -603,44 +602,6 @@ def test_build_image_in_registry_with_auth(self, mock_requests):
         response = self.client.post(url, body)
         self.assertEqual(response.status_code, 201, response.data)
 
-    def test_build_image_no_registry_password(self, mock_requests):
-        """build with image from private registry, but no password given"""
-        app_id = self.create_app()
-
-        # post an image as a build
-        with mock.patch('api.models.release.docker_check_access') as mock_check_access:
-            mock_check_access.side_effect = Exception('no no no')  # let the image access fail
-            url = "/v2/apps/{app_id}/builds".format(**locals())
-            image = 'autotest/example'
-            response = self.client.post(url, {'image': image, 'stack': 'container'})
-            self.assertEqual(response.status_code, 400, response.data)
-
-    def test_build_image_wrong_registry_password(self, mock_requests):
-        """build with image from private registry, but wrong password given"""
-        app_id = self.create_app()
-
-        # post an image as a build using registry hostname
-        url = "/v2/apps/{app_id}/builds".format(**locals())
-        image = 'autotest/example'
-        response = self.client.post(url, {'image': image, 'stack': 'container'})
-        self.assertEqual(response.status_code, 201, response.data)
-
-        # add the required PORT information
-        url = '/v2/apps/{app_id}/config'.format(**locals())
-        body = {'values': json.dumps({'PORT': '80'})}
-        response = self.client.post(url, body)
-        self.assertEqual(response.status_code, 201, response.data)
-
-        # set some registry information
-        with mock.patch('api.models.release.docker_check_access') as mock_check_access:
-            mock_check_access.side_effect = Exception('no no no')  # let the image access fail
-            url = '/v2/apps/{app_id}/config'.format(**locals())
-            body = {'registry': json.dumps({'username': 'bob', 'password': 'zoomzoom'})}
-            response = self.client.post(url, body)
-            self.assertEqual(response.status_code, 400, response.data)
-            mock_check_access.assert_called_with(
-                image, {'username': 'bob', 'password': 'zoomzoom', 'email': 'autotest@drycc.cc'})
-
     def test_build_image_in_registry_with_auth_no_port(self, mock_requests):
         """add authentication to the build but with no PORT config"""
         app_id = self.create_app()

rootfs/api/tests/test_config.py

@@ -21,7 +21,6 @@
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
 @mock.patch('api.models.release.docker_get_port', mock_port)
-@mock.patch('api.models.release.docker_check_access', lambda *args: None)
 class ConfigTest(DryccTransactionTestCase):
     """Tests setting and updating config values"""
 

rootfs/api/tests/test_healthchecks.py

@@ -13,7 +13,6 @@
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
 @mock.patch('api.models.release.docker_get_port', mock_port)
-@mock.patch('api.models.release.docker_check_access', lambda *args: None)
 class TestHealthchecks(DryccTransactionTestCase):
     """Tests setting and updating config values"""
 

rootfs/api/tests/test_hooks.py

@@ -50,7 +50,6 @@
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
 @mock.patch('api.models.release.docker_get_port', mock_port)
-@mock.patch('api.models.release.docker_check_access', lambda *args: None)
 class HookTest(DryccTransactionTestCase):
 
     """Tests API hooks used to trigger actions from external components"""

rootfs/api/tests/test_pods.py

@@ -21,7 +21,6 @@
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
 @mock.patch('api.models.release.docker_get_port', mock_port)
-@mock.patch('api.models.release.docker_check_access', lambda *args: None)
 class PodTest(DryccTransactionTestCase):
     """Tests creation of pods on nodes"""
 

rootfs/api/tests/test_registry.py

@@ -1,6 +1,5 @@
 import json
 import requests_mock
-from unittest import mock
 
 from django.core.cache import cache
 from django.contrib.auth.models import User
@@ -135,13 +134,3 @@ def test_registry_deploy(self, mock_requests):
         self.assertIn('password', response.data['registry'])
         self.assertEqual(response.data['registry']['username'], 'bob')
         self.assertEqual(response.data['registry']['password'], 's3cur3pw1')
-
-        # post a new build
-        with mock.patch('api.models.release.docker_check_access') as mock_check_access:
-            url = "/v2/apps/{app_id}/builds".format(**locals())
-            body = {'image': 'autotest/example', 'stack': 'container'}
-            response = self.client.post(url, body)
-            self.assertEqual(response.status_code, 201, response.data)
-            mock_check_access.assert_called_with(
-                'autotest/example',
-                {'password': 's3cur3pw1', 'username': 'bob', 'email': 'autotest@drycc.cc'})

rootfs/api/tests/test_release.py

@@ -21,7 +21,6 @@
 @requests_mock.Mocker(real_http=True, adapter=adapter)
 @mock.patch('api.models.release.publish_release', lambda *args: None)
 @mock.patch('api.models.release.docker_get_port', mock_port)
-@mock.patch('api.models.release.docker_check_access', lambda *args: None)
 class ReleaseTest(DryccTransactionTestCase):
 
     """Tests push notification from build system"""