From 20e3ffb6a2b6ac5acfe6640e49a2858189350823 Mon Sep 17 00:00:00 2001
From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com>
Date: Tue, 9 Jun 2020 14:37:57 +0530
Subject: [PATCH] Make sure project namespace is used instead of project name

---
 .../helmsman/tests/data/helmsman_config.yaml  |  8 ++--
 cloudman/helmsman/tests/mock_helm.py          | 28 +++++++++++---
 cloudman/helmsman/tests/test_helmsman_api.py  |  4 +-
 cloudman/projman/api.py                       | 12 ++++--
 .../projman/tests/data/helmsman_config.yaml   |  8 ++--
 cloudman/projman/tests/test_project_api.py    | 37 +++++++++++++++----
 6 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/cloudman/helmsman/tests/data/helmsman_config.yaml b/cloudman/helmsman/tests/data/helmsman_config.yaml
index 3cba753a..4815ce95 100644
--- a/cloudman/helmsman/tests/data/helmsman_config.yaml
+++ b/cloudman/helmsman/tests/data/helmsman_config.yaml
@@ -23,9 +23,9 @@ install_templates:
       template: |
         ingress:
           enabled: true
-          path: '/{{context.project}}/jupyterhub'
+          path: '{{context.project.access_path}}/jupyterhub'
         hub:
-          baseUrl: '/{{context.project}}/jupyterhub'
+          baseUrl: '{{context.project.access_path}}/jupyterhub'
         proxy:
           secretToken: '{{random_alphanumeric(65)}}'
     galaxy:
@@ -41,7 +41,7 @@ install_templates:
                     <url>https://ngkc4.cloudve.org/auth</url>
                     <client_id>galaxy-auth</client_id>
                     <client_secret>{{random_alphanumeric(8)}}-{{random_alphanumeric(4)}}-{{random_alphanumeric(4)}}-{{random_alphanumeric(12)}}</client_secret>
-                    <redirect_uri>https://ngkc4.cloudve.org/{{context.project}}/galaxy/authnz/custos/callback</redirect_uri>
+                    <redirect_uri>https://ngkc4.cloudve.org{{context.project.access_path}}/galaxy/authnz/custos/callback</redirect_uri>
                     <realm>master</realm>
                 </provider>
             </OIDC>
@@ -65,7 +65,7 @@ install_templates:
           enabled: true
           hosts:
             - ngkc4.cloudve.org
-          path: /{{context.project}}/galaxy
+          path: {{context.project.access_path}}/galaxy
           tls:
             - hosts:
                 - ngkc4.cloudve.org
diff --git a/cloudman/helmsman/tests/mock_helm.py b/cloudman/helmsman/tests/mock_helm.py
index 380d55aa..7d00185f 100644
--- a/cloudman/helmsman/tests/mock_helm.py
+++ b/cloudman/helmsman/tests/mock_helm.py
@@ -1,6 +1,7 @@
 import argparse
 import csv
 from io import StringIO
+import re
 import uuid
 import yaml
 
@@ -13,6 +14,8 @@ class MockHelm(object):
     to simulate helm commands.
     """
 
+    DNS_SUBDOMAIN_REGEX = re.compile('^(?![0-9]+$)(?!-)[a-zA-Z0-9-]{,63}(?<!-)$')
+
     def __init__(self):
         self.revision_history = [
             {
@@ -60,6 +63,13 @@ def __init__(self):
         self.repo_search_field_names = ["NAME", "CHART VERSION", "APP VERSION", "DESCRIPTION"]
         self.parser = self._create_parser()
 
+    def validate_namespace(self, value):
+        value = str(value)
+        if not self.DNS_SUBDOMAIN_REGEX.match(value):
+            raise argparse.ArgumentTypeError(
+                "Namespace: \"%s\" must be a valid subdomain name which is RFC1123 compliant." % value)
+        return value
+
     def _create_parser(self):
         parser = argparse.ArgumentParser(prog='helm')
 
@@ -69,7 +79,8 @@ def _create_parser(self):
         parser_list = subparsers.add_parser('list', help='list releases')
         parser_list.add_argument('--all-namespaces', action='store_true',
                                  help='list releases from all namespaces')
-        parser_list.add_argument('--namespace', type=str, help='namespace')
+        parser_list.add_argument('--namespace', type=self.validate_namespace,
+                                 help='namespace')
         parser_list.set_defaults(func=self._helm_list)
 
         # Helm install
@@ -80,7 +91,8 @@ def _create_parser(self):
             'chart', type=str, help='chart name')
         parser_inst.add_argument(
             '--generate-name', action='store_true', help='generate random name')
-        parser_inst.add_argument('--namespace', type=str, help='namespace')
+        parser_inst.add_argument('--namespace', type=self.validate_namespace,
+                                 help='namespace')
         parser_inst.add_argument('--version', type=str, help='version')
         parser_inst.add_argument(
             '-f', '--values', type=str, help='value files', nargs="*", action="append")
@@ -108,14 +120,16 @@ def _create_parser(self):
         parser_rollback.add_argument(
             'revision', type=int, help='revision number')
         parser_rollback.add_argument(
-            '--namespace', type=str, help='namespace of release')
+            '--namespace', type=self.validate_namespace,
+            help='namespace of release')
         parser_rollback.set_defaults(func=self._helm_rollback)
 
         # Helm history
         parser_history = subparsers.add_parser('history', help='prints historical revisions for a given release')
         parser_history.add_argument(
             'release', type=str, help='release name')
-        parser_history.add_argument('--namespace', type=str, help='namespace')
+        parser_history.add_argument('--namespace', type=self.validate_namespace,
+                                    help='namespace')
         parser_history.set_defaults(func=self._helm_history)
 
         # Helm repo commands
@@ -142,7 +156,8 @@ def _create_parser(self):
         p_get_values.add_argument(
             '--all', action='store_true', help='dump all values')
         p_get_values.add_argument(
-            '--namespace', type=str, help='namespace of release')
+            '--namespace', type=self.validate_namespace,
+            help='namespace of release')
         p_get_values.set_defaults(func=self._helm_get_values)
 
 
@@ -153,7 +168,8 @@ def _create_parser(self):
         # Helm delete
         parser_delete = subparsers.add_parser('delete', help='delete a release')
         parser_delete.add_argument('release', type=str, help='release name')
-        parser_delete.add_argument('--namespace', type=str, help='namespace')
+        parser_delete.add_argument('--namespace', type=self.validate_namespace,
+                                   help='namespace')
         parser_delete.set_defaults(func=self._helm_delete)
 
         # Helm search
diff --git a/cloudman/helmsman/tests/test_helmsman_api.py b/cloudman/helmsman/tests/test_helmsman_api.py
index d71e1cb0..2521940c 100644
--- a/cloudman/helmsman/tests/test_helmsman_api.py
+++ b/cloudman/helmsman/tests/test_helmsman_api.py
@@ -252,9 +252,9 @@ class InstallTemplateServiceTests(HelmsManServiceTestBase):
         'context': {'project': 'test'},
         'template': """ingress:
               enabled: true
-              path: '/{{context.project}}/galaxy'
+              path: '{{context.project.access_path}}/galaxy'
             hub:
-              baseUrl: '/{{context.project}}/galaxy'
+              baseUrl: '{{context.project.access_path}}/galaxy'
             proxy:
               secretToken: '{{random_alphanumeric(65)}}'"""
     }
diff --git a/cloudman/projman/api.py b/cloudman/projman/api.py
index fd3cb106..3d2df6e6 100644
--- a/cloudman/projman/api.py
+++ b/cloudman/projman/api.py
@@ -113,7 +113,7 @@ def _init_default_project_charts(self, project):
         if chart_template:
             project.charts.create(chart_template.name)
         else:
-            client.charts.create("cloudve", "projman", project.name)
+            client.charts.create("cloudve", "projman", project.namespace)
 
     def create(self, name):
         self.check_permissions('projman.add_project')
@@ -178,14 +178,14 @@ def find(self, name):
 
     def list(self):
         return [self._to_proj_chart(chart) for chart
-                in self._get_helmsman_api().charts.list(self.project.name)
+                in self._get_helmsman_api().charts.list(self.project.namespace)
                 if self.has_permissions('projman.view_chart', chart)]
 
     def get(self, chart_id):
         chart = self._get_helmsman_api().charts.get(chart_id)
         self.check_permissions('projman.view_chart', chart)
         return (self._to_proj_chart(chart)
-                if chart and chart.namespace == self.project.name else None)
+                if chart and chart.namespace == self.project.namespace else None)
 
     def create(self, template_name, release_name=None,
                values=None, context=None):
@@ -193,7 +193,11 @@ def create(self, template_name, release_name=None,
         template = self._get_helmsman_api().templates.get(template_name)
         if not context:
             context = {}
-        context.update({'project': self.project.name})
+        context.update({'project': {
+            'name': self.project.name,
+            'namespace': self.project.namespace,
+            'access_path': f"/{self.project.namespace}",
+        }})
         return self._to_proj_chart(
             template.install(self.project.name, release_name,
                              values, context=context))
diff --git a/cloudman/projman/tests/data/helmsman_config.yaml b/cloudman/projman/tests/data/helmsman_config.yaml
index 25db37c6..07354290 100644
--- a/cloudman/projman/tests/data/helmsman_config.yaml
+++ b/cloudman/projman/tests/data/helmsman_config.yaml
@@ -10,9 +10,9 @@ install_templates:
       template: |
         ingress:
           enabled: true
-          path: '/{{context.project}}/jupyterhub'
+          path: '{{context.project.access_path}}/jupyterhub'
         hub:
-          baseUrl: '/{{context.project}}/jupyterhub'
+          baseUrl: '{{context.project.access_path}}/jupyterhub'
         proxy:
           secretToken: '{{random_alphanumeric(65)}}'
     galaxy:
@@ -30,7 +30,7 @@ install_templates:
                     <url>https://ngkc4.cloudve.org/auth</url>
                     <client_id>galaxy-auth</client_id>
                     <client_secret>{{random_alphanumeric(8)}}-{{random_alphanumeric(4)}}-{{random_alphanumeric(4)}}-{{random_alphanumeric(12)}}</client_secret>
-                    <redirect_uri>https://ngkc4.cloudve.org/{{context.project}}/galaxy/authnz/custos/callback</redirect_uri>
+                    <redirect_uri>https://ngkc4.cloudve.org{{context.project.access_path}}/galaxy/authnz/custos/callback</redirect_uri>
                     <realm>master</realm>
                 </provider>
             </OIDC>
@@ -54,7 +54,7 @@ install_templates:
           enabled: true
           hosts:
             - ngkc4.cloudve.org
-          path: /{{context.project}}/galaxy
+          path: {{context.project.access_path}}/galaxy
           tls:
             - hosts:
                 - ngkc4.cloudve.org
diff --git a/cloudman/projman/tests/test_project_api.py b/cloudman/projman/tests/test_project_api.py
index ac9c2443..8382d0f8 100644
--- a/cloudman/projman/tests/test_project_api.py
+++ b/cloudman/projman/tests/test_project_api.py
@@ -34,25 +34,25 @@ class ProjectServiceTests(ProjManManServiceTestBase):
         'name': 'GVL'
     }
 
-    def _create_project(self):
+    def _create_project(self, project_data=None):
         # create the object
         url = reverse('projman:projects-list')
-        return self.client.post(url, self.PROJECT_DATA, format='json')
+        return self.client.post(url, project_data or self.PROJECT_DATA, format='json')
 
-    def _list_project(self):
+    def _list_project(self, project_data=None):
         # list existing objects
         url = reverse('projman:projects-list')
         response = self.client.get(url)
         self.assertEqual(response.status_code, status.HTTP_200_OK, response.data)
-        self.assertDictContainsSubset(self.PROJECT_DATA, response.data['results'][0])
+        self.assertDictContainsSubset(project_data or self.PROJECT_DATA, response.data['results'][0])
         return response.data['results'][0]['id']
 
-    def _check_project_exists(self, project_id):
+    def _check_project_exists(self, project_id, project_data=None):
         # check it exists
         url = reverse('projman:projects-detail', args=[project_id])
         response = self.client.get(url)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertDictContainsSubset(self.PROJECT_DATA, response.data)
+        self.assertDictContainsSubset(project_data or self.PROJECT_DATA, response.data)
         return response.data['id']
 
     def _delete_project(self, project_id):
@@ -139,6 +139,27 @@ def test_namespace_tied_to_project(self):
         obj = client.namespaces.get(namespace)
         self.assertIsNone(obj, "Deleting the project should delete namespace")
 
+    def test_project_name_special_chars(self):
+        """
+        When creating a project with special chars, the corresponding namespace
+        should be RFC1123 compliant.
+        """
+        PROJECT_DATA = {
+            "name": "A non RFC1123 compli'ant name"
+        }
+        response = self._create_project(project_data=PROJECT_DATA)
+        # Namespace should be slugified version of project name
+        project_id = response.data['id']
+        namespace = response.data['namespace']
+        self.assertEquals(namespace, "a-non-rfc1123-compliant-name")
+        admin = User.objects.filter(is_superuser=True).first()
+        client = HelmsManAPI(HMServiceContext(user=admin))
+        obj = client.namespaces.get(namespace)
+        assert obj
+        self._delete_project(project_id)
+        obj = client.namespaces.get(namespace)
+        self.assertIsNone(obj, "Deleting the project should delete namespace")
+
 
 class ProjectChartServiceTests(ProjManManServiceTestBase):
 
@@ -196,11 +217,11 @@ def _list_project_chart(self, project_id):
 
         return response.data['results'][1]['id']
 
-    def _check_project_chart_exists(self, project_id, chart_id):
+    def _check_project_chart_exists(self, project_id, chart_id, project_data=None):
         url = reverse('projman:chart-detail', args=[project_id, chart_id])
         response = self.client.get(url)
         self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertDictContainsSubset(self.PROJECT_DATA, response.data['project'])
+        self.assertDictContainsSubset(project_data or self.PROJECT_DATA, response.data['project'])
         # Flatten dicts because assertDictContainsSubset doesn't handle nested dicts
         response_chart = hm_helpers.flatten_dict(response.data)
         expected_chart = hm_helpers.flatten_dict(self.CHART_DATA)