Fixed integration with Openshift API

It was discovered that Openshift integration did not
function because the Openshift allocation would use
the same url to make calls to both the account
manager and the Openshift API. This resulted in the
Openshift API never actually being called. Due to
poor error handling by the integration code (more details below),
this big went undetected by the functional tests.
Appropriate fixes have been added tests, and the
Openshift resource type now requires two URLs, one
for the account manager, and one for the Openshift API.

Regarding the poor error handling, none of the functional
Openshift test cases actually checked if the call to
`get_federated_user()` returned the expected output.
The `get_federated_user()` function itself only emits a
log message if the user is not found. This meant that
even though `get_federated_user()` never called the
Openshift API and would therefore never find the user, the test cases still passed.

Additionally, while `_openshift_user_exists()`, the function
which was supposed to call the Openshift API, does
catch the `kexc.NotFoundError`, this error is not specific
enough, as it could be caused by a 404 response made
by ANY server (in the case of our tests, the account manager).

Author: QuanMPhm

ci/run_functional_tests_openshift.sh

@@ -17,6 +17,8 @@ fi
 export DJANGO_SETTINGS_MODULE="local_settings"
 export FUNCTIONAL_TESTS="True"
 export OS_AUTH_URL="https://onboarding-onboarding.cluster.local"
+export OS_API_URL="https://onboarding-onboarding.cluster.local:6443"
+
 
 coverage run --source="." -m django test coldfront_plugin_cloud.tests.functional.openshift
 coverage report

src/coldfront_plugin_cloud/attributes.py

@@ -18,6 +18,7 @@ class CloudAllocationAttribute:
     is_changeable: bool = True
 
 
+RESOURCE_API_URL = 'OpenShift API Endpoint URL'
 RESOURCE_AUTH_URL = 'Identity Endpoint URL'
 RESOURCE_IDENTITY_NAME = 'OpenShift Identity Provider Name'
 RESOURCE_ROLE = 'Role for User in Project'
@@ -32,6 +33,7 @@ class CloudAllocationAttribute:
 RESOURCE_EULA_URL = "EULA URL"
 
 RESOURCE_ATTRIBUTES = [
+    CloudResourceAttribute(name=RESOURCE_API_URL),
     CloudResourceAttribute(name=RESOURCE_AUTH_URL),
     CloudResourceAttribute(name=RESOURCE_IDENTITY_NAME),
     CloudResourceAttribute(name=RESOURCE_FEDERATION_PROTOCOL),

src/coldfront_plugin_cloud/management/commands/add_openshift_resource.py

@@ -17,6 +17,8 @@ def add_arguments(self, parser):
                             help='Name of OpenShift resource')
         parser.add_argument('--auth-url', type=str, required=True,
                             help='URL of the openshift-acct-mgt endpoint')
+        parser.add_argument('--api-url', type=str, required=True,
+                            help='API URL of the openshift cluster')
         parser.add_argument('--role', type=str, default='edit',
                             help='Role for user when added to project (default: edit)')
 
@@ -37,6 +39,12 @@ def handle(self, *args, **options):
             resource=openshift,
             value=options['auth_url']
         )
+        ResourceAttribute.objects.get_or_create(
+            resource_attribute_type=ResourceAttributeType.objects.get(
+                name=attributes.RESOURCE_API_URL),
+            resource=openshift,
+            value=options['api_url']
+        )
         ResourceAttribute.objects.get_or_create(
             resource_attribute_type=ResourceAttributeType.objects.get(
                 name=attributes.RESOURCE_ROLE),

src/coldfront_plugin_cloud/openshift.py

@@ -77,7 +77,7 @@ def __init__(self, resource, allocation):
     def k8_client(self):
         # Load Endpoint URL and Auth token for new k8 client
         openshift_token = os.getenv(f"OPENSHIFT_{self.safe_resource_name}_TOKEN")
-        openshift_url = self.resource.get_attribute(attributes.RESOURCE_AUTH_URL)
+        openshift_url = self.resource.get_attribute(attributes.RESOURCE_API_URL)
 
         k8_config = kubernetes.client.Configuration()
         k8_config.api_key["authorization"] = openshift_token
@@ -181,7 +181,7 @@ def reactivate_project(self, project_id):
     def get_federated_user(self, username):
         if (
             self._openshift_user_exists(username)
-            and self._openshift_get_identity(username)
+            and self._openshift_identity_exists(username)
             and self._openshift_useridentitymapping_exists(username, username)
         ):
             return {'username': username}
@@ -264,22 +264,33 @@ def _openshift_get_identity(self, id_user):
     def _openshift_user_exists(self, user_name):
         try:
             self._openshift_get_user(user_name)
-        except kexc.NotFoundError:
-            return False
+        except kexc.NotFoundError as e:
+            # Ensures error raise because resource not found, 
+            # not because of other reasons, like incorrect url
+            e_info = json.loads(e.body)
+            if e_info.get("reason") == "NotFound":
+                return False
+            raise e
         return True
 
     def _openshift_identity_exists(self, id_user):
         try:
             self._openshift_get_identity(id_user)
-        except kexc.NotFoundError:
-            return False
+        except kexc.NotFoundError as e:
+            e_info = json.loads(e.body)
+            if e_info.get("reason") == "NotFound":
+                return False
+            raise e
         return True
 
     def _openshift_useridentitymapping_exists(self, user_name, id_user):
         try:
             user = self._openshift_get_user(user_name)
-        except kexc.NotFoundError:
-            return False
+        except kexc.NotFoundError as e:
+            e_info = json.loads(e.body)
+            if e_info.get("reason") == "NotFound":
+                return False
+            raise e
 
         return any(
             identity == self.qualified_id_user(id_user)

src/coldfront_plugin_cloud/tests/base.py

@@ -81,13 +81,14 @@ def new_openstack_resource(name=None, auth_url=None) -> Resource:
         return Resource.objects.get(name=resource_name)
 
     @staticmethod
-    def new_openshift_resource(name=None, auth_url=None) -> Resource:
+    def new_openshift_resource(name=None, auth_url=None, api_url=None) -> Resource:
         resource_name = name or uuid.uuid4().hex
 
         call_command(
             'add_openshift_resource',
             name=resource_name,
             auth_url=auth_url or 'https://onboarding-onboarding.cluster.local',
+            api_url=api_url or 'https://onboarding-onboarding.cluster.local:6443',
         )
         return Resource.objects.get(name=resource_name)
 

src/coldfront_plugin_cloud/tests/functional/openshift/test_allocation.py

@@ -15,7 +15,8 @@ def setUp(self) -> None:
         super().setUp()
         self.resource = self.new_openshift_resource(
             name='Microshift',
-            auth_url=os.getenv('OS_AUTH_URL')
+            auth_url=os.getenv('OS_AUTH_URL'),
+            api_url=os.getenv('OS_API_URL'),
         )
 
     def test_new_allocation(self):
@@ -36,7 +37,8 @@ def test_new_allocation(self):
         allocator._get_project(project_id)
 
         # Check user and roles
-        allocator.get_federated_user(user.username)
+        user_info = allocator.get_federated_user(user.username)
+        self.assertEqual(user_info, {'username': user.username})
 
         allocator._get_role(user.username, project_id)
 
@@ -73,7 +75,8 @@ def test_add_remove_user(self):
         tasks.add_user_to_allocation(allocation_user2.pk)
         allocator._get_role(user.username, project_id)
 
-        allocator.get_federated_user(user2.username)
+        user_info = allocator.get_federated_user(user.username)
+        self.assertEqual(user_info, {'username': user.username})
 
         allocator._get_role(user.username, project_id)
         allocator._get_role(user2.username, project_id)