From 49cb548d510c58798d7b53acc10b4f38360f4a1f Mon Sep 17 00:00:00 2001
From: Liam Young <liam.young@canonical.com>
Date: Tue, 12 Sep 2023 15:09:46 +0000
Subject: [PATCH] Add add_explcit_port method

Add add_explcit_port method which adds and explicit port to a url.
This is to fix keystone auth parsing of endpoints. Use this method
to update admin, internal and public endpoint properties.

Change-Id: Ic4c7e980c1c454006f636ddfa048594d0a00dc4d
---
 ops-sunbeam/ops_sunbeam/charm.py    | 37 +++++++++++++++++++++++----
 ops-sunbeam/unit_tests/test_core.py | 39 +++++++++++++++++++++++++++--
 2 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/ops-sunbeam/ops_sunbeam/charm.py b/ops-sunbeam/ops_sunbeam/charm.py
index a57e033e..c2d2287f 100644
--- a/ops-sunbeam/ops_sunbeam/charm.py
+++ b/ops-sunbeam/ops_sunbeam/charm.py
@@ -31,6 +31,7 @@ containers and managing the service running in the container.
 
 import ipaddress
 import logging
+import urllib
 from typing import (
     List,
     Mapping,
@@ -690,11 +691,13 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharmK8S):
                     "ingress-public.url of: %s",
                     self.ingress_public.url,
                 )
-                return self.ingress_public.url
+                return self.add_explicit_port(self.ingress_public.url)
         except (AttributeError, KeyError):
             pass
 
-        return self.service_url(self.public_ingress_address)
+        return self.add_explicit_port(
+            self.service_url(self.public_ingress_address)
+        )
 
     @property
     def admin_url(self) -> str:
@@ -702,7 +705,7 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharmK8S):
         hostname = self.model.get_binding(
             "identity-service"
         ).network.ingress_address
-        return self.service_url(hostname)
+        return self.add_explicit_port(self.service_url(hostname))
 
     @property
     def internal_url(self) -> str:
@@ -714,14 +717,14 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharmK8S):
                     "ingress_internal.url of: %s",
                     self.ingress_internal.url,
                 )
-                return self.ingress_internal.url
+                return self.add_explicit_port(self.ingress_internal.url)
         except (AttributeError, KeyError):
             pass
 
         hostname = self.model.get_binding(
             "identity-service"
         ).network.ingress_address
-        return self.service_url(hostname)
+        return self.add_explicit_port(self.service_url(hostname))
 
     def get_pebble_handlers(self) -> List[sunbeam_chandlers.PebbleHandler]:
         """Pebble handlers for the service."""
@@ -813,3 +816,27 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharmK8S):
     def open_ports(self):
         """Register ports in underlying cloud."""
         self.unit.open_port("tcp", self.default_public_ingress_port)
+
+    def add_explicit_port(self, org_url: str) -> str:
+        """Update a url to add an explicit port.
+
+        Keystone auth endpoint parsing can give odd results if
+        an explicit port is missing.
+        """
+        url = urllib.parse.urlparse(org_url)
+        new_netloc = url.netloc
+        if not url.port:
+            if url.scheme == "http":
+                new_netloc = url.netloc + ":80"
+            elif url.scheme == "https":
+                new_netloc = url.netloc + ":443"
+        return urllib.parse.urlunparse(
+            (
+                url.scheme,
+                new_netloc,
+                url.path,
+                url.params,
+                url.query,
+                url.fragment,
+            )
+        )
diff --git a/ops-sunbeam/unit_tests/test_core.py b/ops-sunbeam/unit_tests/test_core.py
index db0f5c4e..04d7283c 100644
--- a/ops-sunbeam/unit_tests/test_core.py
+++ b/ops-sunbeam/unit_tests/test_core.py
@@ -319,9 +319,9 @@ class TestOSBaseOperatorAPICharm(_TestOSBaseOperatorAPICharm):
         # Add ingress relation
         test_utils.add_complete_ingress_relation(self.harness)
         self.assertEqual(
-            self.harness.charm.internal_url, "http://internal-url"
+            self.harness.charm.internal_url, "http://internal-url:80"
         )
-        self.assertEqual(self.harness.charm.public_url, "http://public-url")
+        self.assertEqual(self.harness.charm.public_url, "http://public-url:80")
 
     @mock.patch("ops_sunbeam.charm.Client")
     def test_endpoint_urls_no_ingress(self, mock_client: mock.patch) -> None:
@@ -398,6 +398,41 @@ class TestOSBaseOperatorAPICharm(_TestOSBaseOperatorAPICharm):
         )
         self.assertTrue(self.harness.charm.relation_handlers_ready())
 
+    def test_add_explicit_port(self):
+        """Test add_explicit_port method."""
+        self.assertEqual(
+            self.harness.charm.add_explicit_port("http://test.org/something"),
+            "http://test.org:80/something",
+        )
+        self.assertEqual(
+            self.harness.charm.add_explicit_port(
+                "http://test.org:80/something"
+            ),
+            "http://test.org:80/something",
+        )
+        self.assertEqual(
+            self.harness.charm.add_explicit_port("https://test.org/something"),
+            "https://test.org:443/something",
+        )
+        self.assertEqual(
+            self.harness.charm.add_explicit_port(
+                "https://test.org:443/something"
+            ),
+            "https://test.org:443/something",
+        )
+        self.assertEqual(
+            self.harness.charm.add_explicit_port(
+                "http://test.org:8080/something"
+            ),
+            "http://test.org:8080/something",
+        )
+        self.assertEqual(
+            self.harness.charm.add_explicit_port(
+                "https://test.org:8443/something"
+            ),
+            "https://test.org:8443/something",
+        )
+
 
 class TestOSBaseOperatorMultiSVCAPICharm(_TestOSBaseOperatorAPICharm):
     """Test Charm with multiple services."""