From 94d4856ab3f98c605d3de631e6ed26e450a1e675 Mon Sep 17 00:00:00 2001 From: Bence Kovacs Date: Fri, 8 Dec 2023 11:38:46 +0100 Subject: [PATCH] fix: minor fixes based on comments on pr #278 --- .github/workflows/pull-request.yaml | 2 +- .github/workflows/release.yaml | 2 +- src/charm.py | 4 +--- tests/integration/oauth_tools/constants.py | 9 -------- tests/integration/test_grafana_oauth.py | 24 ++++++++-------------- tests/unit/test_charm.py | 19 ++++++++++------- 6 files changed, 23 insertions(+), 37 deletions(-) diff --git a/.github/workflows/pull-request.yaml b/.github/workflows/pull-request.yaml index f301da85..5d23c2e1 100644 --- a/.github/workflows/pull-request.yaml +++ b/.github/workflows/pull-request.yaml @@ -12,4 +12,4 @@ jobs: secrets: inherit with: ip-range-start: 10.64.140.43 - ip-range-end: 10.64.140.49 + ip-range-end: 10.64.140.46 diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index c9f2a172..10b355a1 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -11,4 +11,4 @@ jobs: secrets: inherit with: ip-range-start: 10.64.140.43 - ip-range-end: 10.64.140.49 \ No newline at end of file + ip-range-end: 10.64.140.46 \ No newline at end of file diff --git a/src/charm.py b/src/charm.py index d77be537..71636cbb 100755 --- a/src/charm.py +++ b/src/charm.py @@ -304,8 +304,6 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None: Args: event: a :class:`ConfigChangedEvent` to signal that something happened """ - self.oauth.update_client_config(client_config=self._oauth_client_config) - self._configure() self._configure_replication() @@ -740,7 +738,7 @@ def _generate_grafana_config(self) -> str: if self.has_db: configs.append(self._generate_database_config()) - if self.model.relations[OAUTH]: + if self.oauth.is_client_created(): configs.append(self._generate_oauth_refresh_config()) return "\n".join(configs) diff --git a/tests/integration/oauth_tools/constants.py b/tests/integration/oauth_tools/constants.py index 9f2de9bf..5123eb73 100644 --- a/tests/integration/oauth_tools/constants.py +++ b/tests/integration/oauth_tools/constants.py @@ -35,15 +35,6 @@ SELF_SIGNED_CERTIFICATES="self-signed-certificates", ) -OAUTH_RELATION = collections.namedtuple( - "OAUTH_RELATION", ["OAUTH_APPLICATION", "OAUTH_INTERFACE", "OAUTH_PROXY", "OAUTH_CERTIFICATES"] -)( - OAUTH_APPLICATION="hydra", - OAUTH_INTERFACE="oauth", - OAUTH_PROXY="traefik-public", - OAUTH_CERTIFICATES="self-signed-certificates", -) - IDENTITY_BUNDLE = collections.namedtuple("IDENTITY_BUNDLE", ["NAME", "CHANNEL"])( NAME="identity-platform", CHANNEL="0.1/edge", diff --git a/tests/integration/test_grafana_oauth.py b/tests/integration/test_grafana_oauth.py index 975a9add..d1a70c96 100644 --- a/tests/integration/test_grafana_oauth.py +++ b/tests/integration/test_grafana_oauth.py @@ -27,7 +27,7 @@ verify_page_loads, get_cookie_from_browser_by_name, ) -from oauth_tools.constants import OAUTH_RELATION, EXTERNAL_USER_EMAIL +from oauth_tools.constants import EXTERNAL_USER_EMAIL, APPS logger = logging.getLogger(__name__) @@ -59,20 +59,16 @@ async def test_build_and_deploy(ops_test: OpsTest, grafana_charm): ) # Integrate grafana with the identity bundle - await ops_test.model.integrate( - f"grafana:{OAUTH_RELATION.OAUTH_INTERFACE}", OAUTH_RELATION.OAUTH_APPLICATION - ) - await ops_test.model.integrate("grafana:ingress", f"{OAUTH_RELATION.OAUTH_PROXY}") - await ops_test.model.integrate( - "grafana:receive-ca-cert", f"{OAUTH_RELATION.OAUTH_CERTIFICATES}" - ) + await ops_test.model.integrate("grafana:oauth", APPS.HYDRA) + await ops_test.model.integrate("grafana:ingress", APPS.TRAEFIK_PUBLIC) + await ops_test.model.integrate("grafana:receive-ca-cert", APPS.SELF_SIGNED_CERTIFICATES) await ops_test.model.wait_for_idle( apps=[ - OAUTH_RELATION.OAUTH_APPLICATION, + APPS.HYDRA, + APPS.TRAEFIK_PUBLIC, + APPS.SELF_SIGNED_CERTIFICATES, "grafana", - OAUTH_RELATION.OAUTH_PROXY, - OAUTH_RELATION.OAUTH_CERTIFICATES, ], status="active", raise_on_blocked=False, @@ -86,9 +82,7 @@ async def test_oauth_login_with_identity_bundle( ) -> None: external_idp_manager = ExternalIdpManager(ops_test=ops_test) - grafana_proxy = await get_reverse_proxy_app_url( - ops_test, OAUTH_RELATION.OAUTH_PROXY, "grafana" - ) + grafana_proxy = await get_reverse_proxy_app_url(ops_test, APPS.TRAEFIK_PUBLIC, "grafana") redirect_login = os.path.join(grafana_proxy, "login") await access_application_login_page( @@ -117,8 +111,6 @@ async def test_oauth_login_with_identity_bundle( verify=False, ) assert request.status_code == 200 - assert request.status_code == 200 - request.raise_for_status() assert request.json()["email"] == EXTERNAL_USER_EMAIL external_idp_manager.remove_idp_service() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index e05c99da..a19f1bcb 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -79,6 +79,9 @@ OAUTH_CONFIG_INI = """[feature_toggles] accessTokenExpirationCheck = true """ +OAUTH_CLIENT_ID = "grafana_client_id" +OAUTH_CLIENT_SECRET = "s3cR#T" + AUTH_PROVIDER_APPLICATION = "auth_provider" @@ -401,13 +404,13 @@ def test_config_is_updated_with_oauth_relation_data(self): ) # update databag with client details - received once a grafana client is created in hydra - secret_id = self.harness.add_model_secret("hydra", {"secret": "s3cR#T"}) + secret_id = self.harness.add_model_secret("hydra", {"secret": OAUTH_CLIENT_SECRET}) self.harness.grant_secret(secret_id, "grafana-k8s") self.harness.update_relation_data( rel_id, "hydra", { - "client_id": "grafana_client_id", + "client_id": OAUTH_CLIENT_ID, "client_secret_id": secret_id, }, ) @@ -422,23 +425,25 @@ def test_config_is_updated_with_oauth_relation_data(self): services["environment"]["GF_AUTH_GENERIC_OAUTH_NAME"], "external identity provider" ) self.assertEqual( - services["environment"]["GF_AUTH_GENERIC_OAUTH_CLIENT_ID"], "grafana_client_id" + services["environment"]["GF_AUTH_GENERIC_OAUTH_CLIENT_ID"], OAUTH_CLIENT_ID + ) + self.assertEqual( + services["environment"]["GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET"], OAUTH_CLIENT_SECRET ) - self.assertEqual(services["environment"]["GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET"], "s3cR#T") self.assertEqual( services["environment"]["GF_AUTH_GENERIC_OAUTH_SCOPES"], "openid email offline_access" ) self.assertEqual( services["environment"]["GF_AUTH_GENERIC_OAUTH_AUTH_URL"], - "https://example.oidc.com/oauth2/auth", + oauth_provider_info["authorization_endpoint"], ) self.assertEqual( services["environment"]["GF_AUTH_GENERIC_OAUTH_TOKEN_URL"], - "https://example.oidc.com/oauth2/token", + oauth_provider_info["token_endpoint"], ) self.assertEqual( services["environment"]["GF_AUTH_GENERIC_OAUTH_API_URL"], - "https://example.oidc.com/userinfo", + oauth_provider_info["userinfo_endpoint"], ) self.assertEqual( services["environment"]["GF_AUTH_GENERIC_OAUTH_USE_REFRESH_TOKEN"],