fix(registry): tell a user they need PORT when using off-cluster native iaas registry (#988)

registry:set does the same kind of warning but since this is a whole platform setting then we need to warn on deploy as well

Fixes #986

Author: helgi

rootfs/api/models/release.py

@@ -152,8 +152,10 @@ def get_port(self):
             # application has registry auth - $PORT is required
             if (creds is not None) or (settings.REGISTRY_LOCATION != 'on-cluster'):
                 if envs.get('PORT', None) is None:
-                    self.app.log('Private registry detected but no $PORT defined. Defaulting to $PORT 5000', logging.WARNING)  # noqa
-                    return 5000
+                    raise DeisException(
+                        'PORT needs to be set in the config '
+                        'when using a private registry'
+                    )
 
                 # User provided PORT
                 return int(envs.get('PORT'))

rootfs/api/tests/test_release.py

@@ -10,12 +10,13 @@
 
 from django.contrib.auth.models import User
 from django.core.cache import cache
-from django.conf import settings
+from django.test.utils import override_settings
 from unittest import mock
 from rest_framework.authtoken.models import Token
 
 from api.models import App, Release
 from scheduler import KubeHTTPException
+from api.exceptions import DeisException
 from api.tests import adapter, mock_port, DeisTransactionTestCase
 import requests_mock
 
@@ -380,20 +381,19 @@ def test_release_get_port(self, mock_requests):
 
         # TODO(bacongobbler): test dockerfile ports
 
+    @override_settings(REGISTRY_LOCATION="off-cluster")
     def test_release_external_registry(self, mock_requests):
         """
         Test that get_port always returns the proper value.
         """
-        app_id = "test"
-        body = {'id': app_id}
-        response = self.client.post('/v2/apps', body,)
-        self.assertEqual(response.status_code, 201, response.data)
+        app_id = self.create_app()
+
+        # set the required port for external registries
         body = {'values': json.dumps({'PORT': '3000'})}
-        config_response = self.client.post('/v2/apps/test/config', body)
+        config_response = self.client.post('/v2/apps/{}/config'.format(app_id), body)
         self.assertEqual(config_response.status_code, 201, config_response.data)
 
         app = App.objects.get(id=app_id)
-        settings.REGISTRY_LOCATION = "off-cluster"
         url = '/v2/apps/{app_id}/builds'.format(**locals())
         body = {'image': 'test/autotest/example'}
         response = self.client.post(url, body)
@@ -403,3 +403,24 @@ def test_release_external_registry(self, mock_requests):
         self.assertEqual(release.get_port(), 3000)
 
         self.assertEqual(release.image, 'test/autotest/example')
+
+    @override_settings(REGISTRY_LOCATION="off-cluster")
+    def test_release_external_registry_no_port(self, mock_requests):
+        """
+        Test that an exception is raised when registry if off-cluster but
+        no port is provided.
+        """
+        app_id = self.create_app()
+
+        app = App.objects.get(id=app_id)
+        url = '/v2/apps/{app_id}/builds'.format(**locals())
+        body = {'image': 'test/autotest/example'}
+        response = self.client.post(url, body)
+        self.assertEqual(response.status_code, 400, response.data)
+
+        with self.assertRaises(
+            DeisException,
+            msg='PORT needs to be set in the config when using a private registry'
+        ):
+            release = app.release_set.latest()
+            release.get_port()

rootfs/scheduler/tests/test_scheduler.py

@@ -5,7 +5,7 @@
 """
 from django.core.cache import cache
 from django.test import TestCase
-from django.conf import settings
+from django.test.utils import override_settings
 
 from scheduler import mock
 import base64
@@ -143,15 +143,21 @@ def test_get_private_registry_config(self):
         self.assertEqual(secret_name, "private-registry")
         self.assertEqual(secret_create, True)
 
-        settings.REGISTRY_LOCATION = "ecr"
+    @override_settings(REGISTRY_LOCATION="ecr")
+    def test_get_private_registry_config_ecr(self):
         registry = {}
         image = "test.com/test/test"
         docker_config, secret_name, secret_create = self.scheduler._get_private_registry_config(registry, image)  # noqa
         self.assertEqual(docker_config, None)
         self.assertEqual(secret_name, "private-registry-ecr")
         self.assertEqual(secret_create, False)
 
-        settings.REGISTRY_LOCATION = "off-cluster"
+    @override_settings(REGISTRY_LOCATION="off-cluster")
+    def test_get_private_registry_config_off_cluster(self):
+        registry = {}
+        auth = bytes('{}:{}'.format("test", "test"), 'UTF-8')
+        encAuth = base64.b64encode(auth).decode(encoding='UTF-8')
+        image = "test.com/test/test"
         docker_config, secret_name, secret_create = self.scheduler._get_private_registry_config(registry, image)  # noqa
         dockerConfig = json.loads(docker_config)
         expected = {"https://index.docker.io/v1/": {
@@ -161,7 +167,9 @@ def test_get_private_registry_config(self):
         self.assertEqual(secret_name, "private-registry-off-cluster")
         self.assertEqual(secret_create, True)
 
-        settings.REGISTRY_LOCATION = "ecra"
+    @override_settings(REGISTRY_LOCATION="ecra")
+    def test_get_private_registry_config_bad_registry_location(self):
+        registry = {}
         image = "test.com/test/test"
         docker_config, secret_name, secret_create = self.scheduler._get_private_registry_config(registry, image)  # noqa
         self.assertEqual(docker_config, None)