From bf497c3a99b9f8d32184d97c018a2b51575257d6 Mon Sep 17 00:00:00 2001 From: Daniel Caires Date: Mon, 3 Nov 2025 16:47:28 -0300 Subject: [PATCH] Adjust get issuer url code and unit tests This change reorganizes, and adjust code and tests merged recently [1] regarding the retrievel of the issuer_url DEX parameter. [1] - https://review.opendev.org/c/starlingx/openstack-armada-app/+/965219 Test Plan: PASS - Build all packages and STX-O tarball PASS - Upload and apply tarball under 3 situations: 1 - OIDC not applied and parameters not configured 2 - OIDC not applied and parameters configured 3 - OIDC applied PASS - With parameters not configured and DEX integration enabled, Keystone plugin throws exception PASS - issue_url retrieved in scenarios 2 and 3 regardless of DEX integration being enabled PASS - App remove and delete Story: 2011517 Task: 52959 Change-Id: I804528d2b04545642a3e20deecff4db44cc8f008 Signed-off-by: Daniel Caires --- .../k8sapp_openstack/helm/keystone.py | 39 ++++++++-------- .../tests/utils/test_utils.py | 45 ++++++++++++++----- .../k8sapp_openstack/utils.py | 40 ++++++++++++++--- 3 files changed, 90 insertions(+), 34 deletions(-) diff --git a/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/helm/keystone.py b/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/helm/keystone.py index 13d90805..84ac2e2e 100644 --- a/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/helm/keystone.py +++ b/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/helm/keystone.py @@ -1,5 +1,5 @@ # -# Copyright (c) 2019-2024 Wind River Systems, Inc. +# Copyright (c) 2019-2025 Wind River Systems, Inc. # # SPDX-License-Identifier: Apache-2.0 # @@ -8,13 +8,13 @@ import os from oslo_log import log as logging from six.moves import configparser -from sysinv.common import constants from sysinv.common import exception from sysinv.db import api as dbapi from sysinv.helm import common from k8sapp_openstack.common import constants as app_constants from k8sapp_openstack.helm import openstack +from k8sapp_openstack.utils import get_dex_issuer_url from k8sapp_openstack.utils import is_dex_enabled LOG = logging.getLogger(__name__) @@ -323,27 +323,28 @@ class KeystoneHelm(openstack.OpenstackBaseHelm): } def _get_oidc_overrides(self): + """ + Generate OIDC override values for Dex integration. + + This function builds the OIDC override dictionary containing the + `provider_remote_id`, which is derived from the system's Dex issuer URL. + The value is added even if the OIDC application itself is not applied, + since it is only used when `dex_idp.enabled` is set to True. + + Returns: + dict: A dictionary with the Dex OIDC override in the format: + { + 'dex_idp': { + 'provider_remote_id': + } + } + """ db = dbapi.get_instance() dex_enabled = is_dex_enabled() - # since this will only be used if dex_idp.enabled is true, it can be ammended to the + # Because this will only be used if dex_idp.enabled is true, it can be ammended to the # overrides even if oidc is not applied return { 'dex_idp': { - 'provider_remote_id': self.get_dex_issuer_url(db, dex_enabled) + 'provider_remote_id': get_dex_issuer_url(db, dex_enabled) } } - - def get_dex_issuer_url(self, db, dex_enabled): - - try: - oidc_issuer_url = db.service_parameter_get_one( - service=constants.SERVICE_TYPE_KUBERNETES, - section=constants.SERVICE_PARAM_SECTION_KUBERNETES_APISERVER, - name=constants.SERVICE_PARAM_NAME_OIDC_ISSUER_URL) - return oidc_issuer_url.value - except Exception as e: - if dex_enabled: - LOG.error(f"Failed to retrieve OIDC issuer URL: {e}") - raise exception.NotFound("Failed to retrieve OIDC issuer URL") - else: - return "" diff --git a/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/tests/utils/test_utils.py b/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/tests/utils/test_utils.py index fc33a408..099aeb75 100644 --- a/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/tests/utils/test_utils.py +++ b/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/tests/utils/test_utils.py @@ -1218,27 +1218,52 @@ class UtilsTest(dbbase.ControllerHostTestCase): assert result == "" @mock.patch("k8sapp_openstack.utils._get_value_from_application") - def test_returns_true_when_enabled_true(self, mock_get_value): - mock_get_value.return_value = "true" + def test_is_dex_enabled_returns_true(self, mock_get_value): + mock_get_value.return_value = True result = app_utils.is_dex_enabled() self.assertTrue(result) mock_get_value.assert_called_once_with( - default_value="false", + default_value=False, chart_name=app_constants.HELM_CHART_KEYSTONE, override_name="conf.federation.dex_idp.enabled", ) @mock.patch("k8sapp_openstack.utils._get_value_from_application") - def test_returns_false_when_enabled_false(self, mock_get_value): - mock_get_value.return_value = "false" + def test_is_dex_enabled_returns_false(self, mock_get_value): + """ test is_dex_enabled for when dex_ipd.enabled equals false + """ + mock_get_value.return_value = False result = app_utils.is_dex_enabled() self.assertFalse(result) - @mock.patch("k8sapp_openstack.utils._get_value_from_application") - def test_returns_false_when_enabled_other(self, mock_get_value): - mock_get_value.return_value = "anything_else" + def test_get_dex_issuer_url_enabled_success(self): + """ Test get_dex_issuer_url with successfully retrieving parameter + """ + db_mock = mock.Mock() + db_mock.service_parameter_get_one.return_value.value = "https://dex.example.com" - result = app_utils.is_dex_enabled() - self.assertFalse(result) + result = app_utils.get_dex_issuer_url(db_mock, dex_enabled=True) + assert result == "https://dex.example.com" + + def test_get_dex_issuer_url_enabled_not_found(self): + """ Test get_dex_issuer_url with dex enabled but not configured + """ + db_mock = mock.Mock() + db_mock.service_parameter_get_one.side_effect = Exception("DB error") + + self.assertRaises( + exception.NotFound, + app_utils.get_dex_issuer_url, + db_mock, + dex_enabled=True) + + def test_get_dex_issuer_url_disabled_not_found(self): + """ Test get_dex_issuer_url with dex disabled + """ + db_mock = mock.Mock() + db_mock.service_parameter_get_one.side_effect = Exception("DB error") + + result = app_utils.get_dex_issuer_url(db_mock, dex_enabled=False) + assert result == "" diff --git a/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/utils.py b/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/utils.py index 2bada3f5..bacbb3a3 100644 --- a/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/utils.py +++ b/python3-k8sapp-openstack/k8sapp_openstack/k8sapp_openstack/utils.py @@ -1536,14 +1536,44 @@ def get_server_list() -> str: def is_dex_enabled() -> bool: - """ Retrieves if DEX integration has been enabled by user + """ + Determine whether DEX integration is enabled in Keystone overrides. Returns: - bool: Whether user has enabled or not DEX integration. + bool: True if DEX integration is enabled, False otherwise. """ enabled = _get_value_from_application( - default_value="false", + default_value=False, chart_name=app_constants.HELM_CHART_KEYSTONE, - override_name="conf.federation.dex_idp.enabled").lower() + override_name="conf.federation.dex_idp.enabled") - return enabled == 'true' + return enabled + + +def get_dex_issuer_url(db, dex_enabled) -> str: + """ + Retrieve the OIDC issuer URL from system parameters. + + Args: + db: The system database instance. + dex_enabled (bool): Indicates if Dex is enabled via user overrides. + + Returns: + str: The OIDC issuer URL if it exists. Returns an empty string if Dex is disabled + and the parameter is not found. + + Raises: + NotFound: If Dex is enabled but the OIDC issuer URL cannot be retrieved. + """ + try: + oidc_issuer_url = db.service_parameter_get_one( + service=constants.SERVICE_TYPE_KUBERNETES, + section=constants.SERVICE_PARAM_SECTION_KUBERNETES_APISERVER, + name=constants.SERVICE_PARAM_NAME_OIDC_ISSUER_URL) + return oidc_issuer_url.value + except Exception as e: + if dex_enabled: + LOG.error(f"Failed to retrieve OIDC issuer URL: {e}") + raise exception.NotFound("Failed to retrieve OIDC issuer URL") + else: + return ""