From 529c39d4b917e1f712508194e540892721851085 Mon Sep 17 00:00:00 2001
From: ike thecoder <ikethecoder@copeconsulting.ca>
Date: Mon, 18 Nov 2024 09:19:34 -0800
Subject: [PATCH] enforce validation on upstream services (#129)

---
 microservices/gatewayApi/README.md            |  3 +-
 microservices/gatewayApi/config/test.json     |  7 +-
 microservices/gatewayApi/tests/conftest.py    | 37 ++++++++--
 .../routes/v2/test_gateway_err_validations.py | 31 +++++++++
 .../v1 => utils}/test_validate_upstream.py    | 30 +++++++-
 microservices/gatewayApi/utils/validators.py  | 56 +++++++++++++++
 microservices/gatewayApi/v1/routes/gateway.py | 60 +++-------------
 microservices/gatewayApi/v2/routes/gateway.py | 68 +++----------------
 8 files changed, 172 insertions(+), 120 deletions(-)
 rename microservices/gatewayApi/tests/{routes/v1 => utils}/test_validate_upstream.py (79%)

diff --git a/microservices/gatewayApi/README.md b/microservices/gatewayApi/README.md
index 7b1aa3b..c2d7793 100644
--- a/microservices/gatewayApi/README.md
+++ b/microservices/gatewayApi/README.md
@@ -53,7 +53,8 @@ The `data_planes_config.json` example:
   "data_planes": {
     "dp-silver-kong-proxy": {
       "kube-api": "https://api.cloud",
-      "kube-ns": "xxxxxx-dev"
+      "kube-ns": "xxxxxx-dev",
+      "validate-upstreams": false
     }
   }
 }
diff --git a/microservices/gatewayApi/config/test.json b/microservices/gatewayApi/config/test.json
index 232fdd5..0ed2926 100644
--- a/microservices/gatewayApi/config/test.json
+++ b/microservices/gatewayApi/config/test.json
@@ -19,10 +19,15 @@
     "test-default-dp": {
       "kube-api": "http://kube-api",
       "kube-ns": "abcd-1234"
+    },
+    "strict-dp": {
+      "kube-api": "http://kube-api",
+      "kube-ns": "abcd-1234",
+      "validate-upstreams": true
     }
   },
   "kubeApiCreds": {
     "kubeApiPass": "password",
     "kubeApiUser": "username"
   }
-}
+}
\ No newline at end of file
diff --git a/microservices/gatewayApi/tests/conftest.py b/microservices/gatewayApi/tests/conftest.py
index d3c1149..736b536 100644
--- a/microservices/gatewayApi/tests/conftest.py
+++ b/microservices/gatewayApi/tests/conftest.py
@@ -60,15 +60,38 @@ def decorated_function(*args, **kwargs):
 def mock_keycloak(mocker):
     class mock_kc_admin:
         def get_group_by_path(path, search_in_subgroups):
-            return {
-                "id": "g001"
-            }
+            if path == "/ns/mytest":
+                return {"id": "g001"}
+            elif path == "/ns/mytest2":
+                return {"id": "g002"}
+            elif path == "/ns/mytest3":
+                return {"id": "g003"}
+            else:
+                return {"id": "g001"}
         def get_group(id):
-            return {
-                "attributes": {
-                    "perm-domains": [ ".api.gov.bc.ca", ".cluster.local" ]
+            if id == "g001":
+                return {
+                    "attributes": {
+                        "perm-domains": [ ".api.gov.bc.ca", ".cluster.local" ]
+                    }
                 }
-            }
+            elif id == "g002":
+                return {
+                    "attributes": {
+                        "perm-data-plane": ["strict-dp"],
+                        "perm-upstreams": [],
+                        "perm-domains": [ ".api.gov.bc.ca", ".cluster.local" ]
+                    }
+                }
+            elif id == "g003":
+                return {
+                    "attributes": {
+                        "perm-data-plane": ["strict-dp"],
+                        "perm-upstreams": ['ns1'],
+                        "perm-domains": [ ".api.gov.bc.ca", ".cluster.local" ]
+                    }
+                }
+
     mocker.patch("v2.services.namespaces.admin_api", return_value=mock_kc_admin)
 
 def mock_kong(mocker):
diff --git a/microservices/gatewayApi/tests/routes/v2/test_gateway_err_validations.py b/microservices/gatewayApi/tests/routes/v2/test_gateway_err_validations.py
index 8723229..eca517b 100644
--- a/microservices/gatewayApi/tests/routes/v2/test_gateway_err_validations.py
+++ b/microservices/gatewayApi/tests/routes/v2/test_gateway_err_validations.py
@@ -114,3 +114,34 @@ def test_invalid_upstream(client):
     response = client.put('/v2/namespaces/mytest/gateway', json=data)
     assert response.status_code == 400
     assert json.dumps(response.json) == '{"error": "Validation Errors:\\nservice upstream is invalid (e1)"}'
+
+def test_invalid_strict_dp_upstream(client):
+    configFile = '''
+        services:
+        - name: my-service
+          host: myservice.ns1.svc
+          tags: ["ns.mytest2", "another"]
+        '''
+
+    data={
+        "configFile": configFile,
+        "dryRun": True
+    }
+    response = client.put('/v2/namespaces/mytest2/gateway', json=data)
+    assert response.status_code == 400
+    assert json.dumps(response.json) == '{"error": "Validation Errors:\\nservice upstream is invalid (e6)"}'
+
+def test_valid_strict_dp_upstream(client):
+    configFile = '''
+        services:
+        - name: my-service
+          host: myservice.ns1.svc
+          tags: ["ns.mytest3", "another"]
+        '''
+
+    data={
+        "configFile": configFile,
+        "dryRun": True
+    }
+    response = client.put('/v2/namespaces/mytest3/gateway', json=data)
+    assert response.status_code == 200
diff --git a/microservices/gatewayApi/tests/routes/v1/test_validate_upstream.py b/microservices/gatewayApi/tests/utils/test_validate_upstream.py
similarity index 79%
rename from microservices/gatewayApi/tests/routes/v1/test_validate_upstream.py
rename to microservices/gatewayApi/tests/utils/test_validate_upstream.py
index 8107275..8aec218 100644
--- a/microservices/gatewayApi/tests/routes/v1/test_validate_upstream.py
+++ b/microservices/gatewayApi/tests/utils/test_validate_upstream.py
@@ -6,7 +6,7 @@
 
 import yaml
 import pytest
-from v1.routes.gateway import validate_upstream
+from utils.validators import validate_upstream
 
 def test_upstream_good(app):
     payload = '''
@@ -126,3 +126,31 @@ def test_upstream_protected_service_allow(app):
     y = yaml.load(payload, Loader=yaml.FullLoader)
     validate_upstream (y, { "perm-protected-ns": ["deny"]}, ['my-namespace']) 
 
+def test_upstream_pass_validation(app):
+    payload = '''
+services:
+  - name: my-service
+    tags: ["ns.mytest", "another"]
+    host: myapi.my-namespace.svc
+'''
+    y = yaml.load(payload, Loader=yaml.FullLoader)
+
+    validate_upstream (y, { "perm-upstreams": ["my-namespace"]}, [], True) 
+
+def test_upstream_fail_validation(app):
+    payload = '''
+services:
+  - name: my-service
+    tags: ["ns.mytest", "another"]
+    host: myapi.my-namespace.svc
+'''
+    y = yaml.load(payload, Loader=yaml.FullLoader)
+
+    with pytest.raises(Exception, match=r"service upstream is invalid \(e6\)"):
+        validate_upstream (y, {}, [], True)
+
+    with pytest.raises(Exception, match=r"service upstream is invalid \(e6\)"):
+        validate_upstream (y, { "perm-upstreams": ["other-namespace"]}, [], True)
+
+    with pytest.raises(Exception, match=r"service upstream is invalid \(e6\)"):
+        validate_upstream (y, { "perm-upstreams": [""]}, [], True)
diff --git a/microservices/gatewayApi/utils/validators.py b/microservices/gatewayApi/utils/validators.py
index bfbf752..09c3b01 100644
--- a/microservices/gatewayApi/utils/validators.py
+++ b/microservices/gatewayApi/utils/validators.py
@@ -1,5 +1,6 @@
 
 import re
+from urllib.parse import urlparse
 
 namespace_validation_rule='^[a-z][a-z0-9-]{4,14}$'
 
@@ -16,3 +17,58 @@ def host_valid(input_string):
     # match = regex.match(str(input_string))
     # return bool(match is not None)
 
+def validate_upstream(yaml, ns_attributes, protected_kube_namespaces, do_validate_upstreams: bool = False):
+    errors = []
+
+    perm_upstreams = ns_attributes.get('perm-upstreams', [])
+
+    allow_protected_ns = ns_attributes.get('perm-protected-ns', ['deny'])[0] == 'allow'
+
+    # A service host must not contain a list of protected
+    if 'services' in yaml:
+        for service in yaml['services']:
+            if 'url' in service:
+                try:
+                    u = urlparse(service["url"])
+                    if u.hostname is None:
+                        errors.append("service upstream has invalid url specified (e1)")
+                    else:
+                        validate_upstream_host(u.hostname, errors, allow_protected_ns, protected_kube_namespaces, do_validate_upstreams, perm_upstreams)
+                except Exception as e:
+                    errors.append("service upstream has invalid url specified (e2)")
+
+            if 'host' in service:
+                host = service["host"]
+                validate_upstream_host(host, errors, allow_protected_ns, protected_kube_namespaces, do_validate_upstreams, perm_upstreams)
+
+    if len(errors) != 0:
+        raise Exception('\n'.join(errors))
+
+
+def validate_upstream_host(_host, errors, allow_protected_ns, protected_kube_namespaces, do_validate_upstreams, perm_upstreams):
+    host = _host.lower()
+
+    restricted = ['localhost', '127.0.0.1', '0.0.0.0']
+
+    if host in restricted:
+        errors.append("service upstream is invalid (e1)")
+    elif host.endswith('svc'):
+        partials = host.split('.')
+        # get the namespace, and make sure it is not in the protected_kube_namespaces list
+        if len(partials) != 3:
+            errors.append("service upstream is invalid (e2)")
+        elif partials[1] in protected_kube_namespaces and allow_protected_ns is False:
+            errors.append("service upstream is invalid (e3)")
+        elif do_validate_upstreams and (partials[1] in perm_upstreams) is False:
+            errors.append("service upstream is invalid (e6)")
+    elif host.endswith('svc.cluster.local'):
+        partials = host.split('.')
+        # get the namespace, and make sure it is not in the protected_kube_namespaces list
+        if len(partials) != 5:
+            errors.append("service upstream is invalid (e4)")
+        elif partials[1] in protected_kube_namespaces and allow_protected_ns is False:
+            errors.append("service upstream is invalid (e5)")
+        elif do_validate_upstreams and (partials[1] in perm_upstreams) is False:
+            errors.append("service upstream is invalid (e6)")
+    elif do_validate_upstreams:
+        errors.append("service upstream is invalid (e6)")
diff --git a/microservices/gatewayApi/v1/routes/gateway.py b/microservices/gatewayApi/v1/routes/gateway.py
index 71f8f67..ba94ad5 100644
--- a/microservices/gatewayApi/v1/routes/gateway.py
+++ b/microservices/gatewayApi/v1/routes/gateway.py
@@ -24,7 +24,7 @@
 from clients.ocp_routes import get_host_list, get_route_overrides
 from clients.ocp_gateway_secret import prep_submitted_config, prep_and_apply_secret, write_submitted_config
 
-from utils.validators import host_valid
+from utils.validators import host_valid, validate_upstream
 from utils.transforms import plugins_transformations
 from utils.masking import mask
 
@@ -270,7 +270,14 @@ def write_config(namespace: str) -> object:
         # Validate upstream URLs are valid
         try:
             protected_kube_namespaces = json.loads(app.config['protectedKubeNamespaces'])
-            validate_upstream(gw_config, ns_attributes, protected_kube_namespaces)
+
+            dp = get_data_plane(ns_attributes)
+
+            do_validate_upstreams = app.config['data_planes'][dp].get("validate-upstreams", False)
+
+            log.debug("Validate upstreams %s %s" % (dp, do_validate_upstreams))
+
+            validate_upstream(gw_config, ns_attributes, protected_kube_namespaces, do_validate_upstreams)
         except Exception as ex:
             traceback.print_exc()
             log.error("%s - %s" % (namespace, " Upstream Validation Errors: %s" % ex))
@@ -476,55 +483,6 @@ def transform_host(host):
     else:
         return host
 
-
-def validate_upstream(yaml, ns_attributes, protected_kube_namespaces):
-    errors = []
-
-    allow_protected_ns = ns_attributes.get('perm-protected-ns', ['deny'])[0] == 'allow'
-
-    # A host must not contain a list of protected
-    if 'services' in yaml:
-        for service in yaml['services']:
-            if 'url' in service:
-                try:
-                    u = urlparse(service["url"])
-                    if u.hostname is None:
-                        errors.append("service upstream has invalid url specified (e1)")
-                    else:
-                        validate_upstream_host(u.hostname, errors, allow_protected_ns, protected_kube_namespaces)
-                except Exception as e:
-                    errors.append("service upstream has invalid url specified (e2)")
-
-            if 'host' in service:
-                host = service["host"]
-                validate_upstream_host(host, errors, allow_protected_ns, protected_kube_namespaces)
-
-    if len(errors) != 0:
-        raise Exception('\n'.join(errors))
-
-
-def validate_upstream_host(_host, errors, allow_protected_ns, protected_kube_namespaces):
-    host = _host.lower()
-
-    restricted = ['localhost', '127.0.0.1', '0.0.0.0']
-
-    if host in restricted:
-        errors.append("service upstream is invalid (e1)")
-    if host.endswith('svc'):
-        partials = host.split('.')
-        # get the namespace, and make sure it is not in the protected_kube_namespaces list
-        if len(partials) != 3:
-            errors.append("service upstream is invalid (e2)")
-        elif partials[1] in protected_kube_namespaces and allow_protected_ns is False:
-            errors.append("service upstream is invalid (e3)")
-    if host.endswith('svc.cluster.local'):
-        partials = host.split('.')
-        # get the namespace, and make sure it is not in the protected_kube_namespaces list
-        if len(partials) != 5:
-            errors.append("service upstream is invalid (e4)")
-        elif partials[1] in protected_kube_namespaces and allow_protected_ns is False:
-            errors.append("service upstream is invalid (e5)")
-
 # Handle the two cases:
 # - pass in an empty config expecting all routes to be deleted ('upstreams' not in yaml)
 # - pass in a config with services ('services' in yaml)
diff --git a/microservices/gatewayApi/v2/routes/gateway.py b/microservices/gatewayApi/v2/routes/gateway.py
index 6b04711..702216c 100644
--- a/microservices/gatewayApi/v2/routes/gateway.py
+++ b/microservices/gatewayApi/v2/routes/gateway.py
@@ -6,26 +6,20 @@
 from urllib.parse import urlparse
 from subprocess import Popen, PIPE, STDOUT
 import uuid
-import logging
 import json
 import requests
 import yaml
-from werkzeug.exceptions import HTTPException, NotFound
-from flask import Blueprint, config, jsonify, request, Response, make_response, abort, g, current_app as app
-from io import TextIOWrapper
+from werkzeug.exceptions import HTTPException
+from flask import Blueprint, jsonify, request, make_response, abort, current_app as app
 from clients.ocp_routes import get_host_list, get_route_overrides
 
 from v2.auth.auth import admin_jwt, uma_enforce
-
 from v2.services.namespaces import NamespaceService
 
 from clients.portal import record_gateway_event
 from clients.kong import get_routes, register_kong_certs
-from clients.ocp_networksecuritypolicy import get_ocp_service_namespaces, check_nsp, apply_nsp, delete_nsp
-from clients.ocp_routes import prepare_apply_routes, prepare_delete_routes, apply_routes, delete_routes
-from clients.ocp_gateway_secret import prep_submitted_config, prep_and_apply_secret, write_submitted_config
-
-from utils.validators import host_valid
+from clients.ocp_gateway_secret import prep_submitted_config
+from utils.validators import host_valid, validate_upstream
 from utils.transforms import plugins_transformations
 from utils.masking import mask
 
@@ -273,7 +267,11 @@ def write_config(namespace: str) -> object:
         # Validate upstream URLs are valid
         try:
             protected_kube_namespaces = json.loads(app.config['protectedKubeNamespaces'])
-            validate_upstream(gw_config, ns_attributes, protected_kube_namespaces)
+
+            do_validate_upstreams = app.config['data_planes'][dp].get("validate-upstreams", False)
+
+            log.debug("Validate upstreams %s %s" % (dp, do_validate_upstreams))
+            validate_upstream(gw_config, ns_attributes, protected_kube_namespaces, do_validate_upstreams)
         except Exception as ex:
             traceback.print_exc()
             log.error("%s - %s" % (namespace, " Upstream Validation Errors: %s" % ex))
@@ -509,54 +507,6 @@ def transform_host(host):
     else:
         return host
 
-def validate_upstream(yaml, ns_attributes, protected_kube_namespaces):
-    errors = []
-
-    allow_protected_ns = ns_attributes.get('perm-protected-ns', ['deny'])[0] == 'allow'
-
-    # A host must not contain a list of protected
-    if 'services' in yaml:
-        for service in yaml['services']:
-            if 'url' in service:
-                try:
-                    u = urlparse(service["url"])
-                    if u.hostname is None:
-                        errors.append("service upstream has invalid url specified (e1)")
-                    else:
-                        validate_upstream_host(u.hostname, errors, allow_protected_ns, protected_kube_namespaces)
-                except Exception as e:
-                    errors.append("service upstream has invalid url specified (e2)")
-
-            if 'host' in service:
-                host = service["host"]
-                validate_upstream_host(host, errors, allow_protected_ns, protected_kube_namespaces)
-
-    if len(errors) != 0:
-        raise Exception('\n'.join(errors))
-
-
-def validate_upstream_host(_host, errors, allow_protected_ns, protected_kube_namespaces):
-    host = _host.lower()
-
-    restricted = ['localhost', '127.0.0.1', '0.0.0.0']
-
-    if host in restricted:
-        errors.append("service upstream is invalid (e1)")
-    if host.endswith('svc'):
-        partials = host.split('.')
-        # get the namespace, and make sure it is not in the protected_kube_namespaces list
-        if len(partials) != 3:
-            errors.append("service upstream is invalid (e2)")
-        elif partials[1] in protected_kube_namespaces and allow_protected_ns is False:
-            errors.append("service upstream is invalid (e3)")
-    if host.endswith('svc.cluster.local'):
-        partials = host.split('.')
-        # get the namespace, and make sure it is not in the protected_kube_namespaces list
-        if len(partials) != 5:
-            errors.append("service upstream is invalid (e4)")
-        elif partials[1] in protected_kube_namespaces and allow_protected_ns is False:
-            errors.append("service upstream is invalid (e5)")
-
 def update_routes_check(yaml):
     if 'services' in yaml or 'upstreams' not in yaml:
         return True