Fix the section name in wait_for_oidc_server function (#3415)

* Fix the section name in the wait_for_oidc_server function

- Fix leftover openid-connect -> openid
- Add test coverage for wait_for_oidc_server
- Add test coverage for the remaining parts of shell.py

Author: npalaska

lib/pbench/cli/server/shell.py

@@ -110,7 +110,16 @@ def run_gunicorn(server_config: PbenchServerConfig, logger: Logger) -> int:
     except OpenIDClient.NotConfigured as exc:
         logger.warning("OpenID Connect client not configured, {}", exc)
         notifier.notify("STOPPING=1")
+        notifier.notify("STATUS=OPENID broker not configured")
+        return 1
+    except (
+        OpenIDClient.ServerConnectionError,
+        OpenIDClient.ServerConnectionTimedOut,
+    ) as exc:
+        logger.warning("OpenID Connect client not reachable, {}", exc)
+        notifier.notify("STOPPING=1")
         notifier.notify("STATUS=OPENID broker not responding")
+        return 1
     else:
         logger.info("Pbench server using OIDC server {}", oidc_server)
 

lib/pbench/server/api/resources/endpoint_configure.py

@@ -1,4 +1,3 @@
-from configparser import NoOptionError, NoSectionError
 from http import HTTPStatus
 import re
 from typing import Any, Dict
@@ -50,7 +49,7 @@ def get(self):
         Return server configuration information required by web clients
         including the Pbench dashboard UI. This includes:
 
-        openid-connect: A JSON object containing the OpenID Connect parameters
+        openid: A JSON object containing the OpenID Connect parameters
                 required for the web client to use OIDC authentication.
         identification: The Pbench server name and version
         uri:    A dict of server API templates, where each template defines a
@@ -150,18 +149,15 @@ def get(self):
             "uri": templates,
         }
 
-        try:
-            client = self.server_config.get("openid", "client")
-            realm = self.server_config.get("openid", "realm")
-            server = self.server_config.get("openid", "server_url")
-        except (NoOptionError, NoSectionError):
-            pass
-        else:
-            endpoints["openid"] = {
-                "client": client,
-                "realm": realm,
-                "server": server,
-            }
+        client = self.server_config.get("openid", "client")
+        realm = self.server_config.get("openid", "realm")
+        server = self.server_config.get("openid", "server_url")
+
+        endpoints["openid"] = {
+            "client": client,
+            "realm": realm,
+            "server": server,
+        }
 
         try:
             response = jsonify(endpoints)

lib/pbench/server/auth/__init__.py

@@ -208,7 +208,7 @@ def wait_for_oidc_server(
                 times out.
         """
         try:
-            oidc_server = server_config.get("openid-connect", "server_url")
+            oidc_server = server_config.get("openid", "server_url")
         except (NoOptionError, NoSectionError) as exc:
             raise OpenIDClient.NotConfigured() from exc
 

lib/pbench/test/unit/server/auth/test_auth.py

@@ -8,6 +8,7 @@
 import pytest
 import requests
 from requests.structures import CaseInsensitiveDict
+import responses
 
 from pbench.server import JSON
 import pbench.server.auth
@@ -289,6 +290,64 @@ def get(self, path: str, **kwargs) -> MockResponse:
 class TestOpenIDClient:
     """Verify the OpenIDClient class."""
 
+    @responses.activate
+    def test_wait_for_oidc_server_fail(self, make_logger):
+        """Verify .wait_for_oidc_server() failure mode"""
+        # Start with an empty configuration, no openid section
+        config = configparser.ConfigParser()
+        with pytest.raises(OpenIDClient.NotConfigured):
+            OpenIDClient.wait_for_oidc_server(config, make_logger)
+        # Missing "server_url"
+        section = {}
+        config["openid"] = section
+        with pytest.raises(OpenIDClient.NotConfigured):
+            OpenIDClient.wait_for_oidc_server(config, make_logger)
+
+        section = {"server_url": "https://example.com"}
+        config["openid"] = section
+
+        # Keycloak health returning response but status is not UP
+        responses.add(
+            responses.GET,
+            "https://example.com/health",
+            body='{"status": "DOWN","checks": []}',
+            content_type="application/json",
+        )
+
+        with pytest.raises(OpenIDClient.ServerConnectionTimedOut):
+            OpenIDClient.wait_for_oidc_server(config, make_logger)
+
+        # Keycloak health returning network exception and no content
+        responses.add(
+            responses.GET,
+            "https://example.com/health",
+            body=Exception("some network exception"),
+        )
+
+        with pytest.raises(OpenIDClient.ServerConnectionError):
+            OpenIDClient.wait_for_oidc_server(config, make_logger)
+
+    @responses.activate
+    def test_wait_for_oidc_server_succ(self, make_logger):
+        """Verify .wait_for_oidc_server() success mode"""
+
+        config = configparser.ConfigParser()
+        section = {"server_url": "https://example.com"}
+        config["openid"] = section
+
+        # Keycloak health returning response with status UP
+        responses.add(
+            responses.GET,
+            "https://example.com/health",
+            body='{"status": "UP","checks": []}',
+            content_type="application/json",
+        )
+
+        assert (
+            OpenIDClient.wait_for_oidc_server(config, make_logger)
+            == "https://example.com"
+        )
+
     def test_construct_oidc_client_fail(self):
         """Verfiy .construct_oidc_client() failure mode"""
 

lib/pbench/test/unit/server/test_shell_cli.py

@@ -71,10 +71,7 @@ def test_find_the_unicorn(monkeypatch, make_logger):
 
     @staticmethod
     @pytest.mark.parametrize("user_site", [False, True])
-    @pytest.mark.parametrize("oidc_conf", [False, True])
-    def test_main(
-        monkeypatch, make_logger, mock_get_server_config, user_site, oidc_conf
-    ):
+    def test_main(monkeypatch, make_logger, mock_get_server_config, user_site):
         called = []
 
         def find_the_unicorn(logger: logging.Logger):
@@ -89,10 +86,8 @@ def init_indexing(*args, **kwargs):
         def wait_for_oidc_server(
             server_config: PbenchServerConfig, logger: logging.Logger
         ) -> str:
-            if oidc_conf:
-                return "https://oidc.example.com"
-            else:
-                raise OpenIDClient.NotConfigured()
+            called.append("wait_for_oidc_server")
+            return "https://oidc.example.com"
 
         commands = []
 
@@ -117,6 +112,7 @@ def run(args, cwd: Optional[str] = None) -> subprocess.CompletedProcess:
         expected_called += [
             "wait_for_uri(sqlite:///:memory:,120)",
             "wait_for_uri(http://elasticsearch.example.com:7080,120)",
+            "wait_for_oidc_server",
             "init_indexing",
         ]
         assert called == expected_called
@@ -211,8 +207,17 @@ def init_db(*args, **kwargs) -> int:
         assert ret_val == 1
 
     @staticmethod
+    @pytest.mark.parametrize(
+        "exc",
+        [
+            Exception("oidc exception"),
+            OpenIDClient.NotConfigured,
+            OpenIDClient.ServerConnectionTimedOut,
+            OpenIDClient.ServerConnectionError,
+        ],
+    )
     def test_main_wait_for_oidc_server_exc(
-        monkeypatch, make_logger, mock_get_server_config
+        monkeypatch, make_logger, mock_get_server_config, exc
     ):
         def immediate_success(*args, **kwargs):
             pass
@@ -223,7 +228,7 @@ def wait_for_oidc_server(
             server_config: PbenchServerConfig, logger: logging.Logger
         ) -> str:
             called[0] = True
-            raise Exception("oidc exception")
+            raise exc
 
         monkeypatch.setattr(shell.site, "ENABLE_USER_SITE", False)
         monkeypatch.setattr(shell, "wait_for_uri", immediate_success)

server/pbenchinacan/load_keycloak.sh

@@ -22,7 +22,7 @@ KEYCLOAK_REDIRECT_URI=${KEYCLOAK_REDIRECT_URI:-"http://localhost:8080/*"}
 ADMIN_USERNAME=${ADMIN_USERNAME:-"admin"}
 ADMIN_PASSWORD=${ADMIN_PASSWORD:-"admin"}
 # These values must match the options "realm" and "client in the
-# "openid-connect" section of the pbench server configuration file.
+# "openid" section of the pbench server configuration file.
 REALM=${KEYCLOAK_REALM:-"pbench-server"}
 CLIENT=${KEYCLOAK_CLIENT:-"pbench-client"}