-
Notifications
You must be signed in to change notification settings - Fork 0
[DPE-9663] Support error propagation in TLS relation #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
908c338
3f88d8f
1a72f26
6f3c7da
0ab8640
8c80e7b
daee81c
86e2690
b50a3f9
3189d2f
78ac2f4
82535c6
4831012
c9a9cd4
2d51194
0f0af2d
152377b
4534b55
0dc94b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -220,9 +220,9 @@ async def test_user_secret_permissions(juju: jubilant.Juju) -> None: | |
| ) | ||
|
|
||
| logger.info("Secret access will be granted now - wait for updated password") | ||
| juju.grant_secret(identifier=secret_name, app=APP_NAME) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this to fix the flakiness we've been noticing on CI?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I was investigating these tests locally and found that the |
||
| # deferred `config_changed` event will be retried before `update_status` | ||
| with fast_forward(juju): | ||
| juju.grant_secret(identifier=secret_name, app=APP_NAME) | ||
| juju.wait( | ||
| lambda status: are_apps_active_and_agents_idle(status, APP_NAME, idle_period=10), | ||
| timeout=1200, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,14 @@ | |
| # Copyright 2026 Canonical Ltd. | ||
| # See LICENSE file for licensing details. | ||
| import logging | ||
| import os | ||
| import re | ||
| import subprocess | ||
| from pathlib import Path | ||
|
|
||
| import jubilant | ||
|
|
||
| from literals import Substrate | ||
| from literals import CharmUsers, Substrate | ||
| from statuses import TLSStatuses | ||
| from tests.integration.helpers import ( | ||
| APP_NAME, | ||
|
|
@@ -18,27 +21,49 @@ | |
| are_apps_active_and_agents_idle, | ||
| does_status_match, | ||
| download_client_certificate_from_unit, | ||
| get_cluster_hostnames, | ||
| get_password, | ||
| set_key, | ||
| ) | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| NUM_UNITS = 3 | ||
| TEST_KEY = "test_key" | ||
| TEST_VALUE = "test_value" | ||
| VAULT_NAME = "vault" | ||
|
|
||
|
|
||
| def test_build_and_deploy(charm: str, juju: jubilant.Juju, substrate: Substrate) -> None: | ||
| """Deploy the charm under test and a TLS provider.""" | ||
| logger.info("Installing vault cli client") | ||
| subprocess.run( | ||
| ["sudo", "snap", "install", "vault"], check=True, text=True, capture_output=True | ||
| ) | ||
|
|
||
| juju.deploy( | ||
| charm, | ||
| resources=IMAGE_RESOURCE if substrate == Substrate.K8S else None, | ||
| num_units=NUM_UNITS, | ||
| trust=True, | ||
| ) | ||
| juju.deploy(TLS_NAME, channel=TLS_CHANNEL) | ||
| juju.deploy( | ||
| "vault-k8s" if substrate == Substrate.K8S else "vault", | ||
| app=VAULT_NAME, | ||
| channel="1.18/edge", | ||
| config={ | ||
| "pki_ca_common_name": "mydomain.com", | ||
| "pki_allow_any_name": False, | ||
| "pki_allow_ip_sans": False, | ||
| }, | ||
| ) | ||
| juju.integrate(f"{APP_NAME}:client-certificates", TLS_NAME) | ||
| juju.wait( | ||
| lambda status: are_agents_idle(status, APP_NAME, idle_period=30, unit_count=NUM_UNITS), | ||
| timeout=600, | ||
| ) | ||
| juju.wait(lambda status: jubilant.all_blocked(status, VAULT_NAME)) | ||
|
|
||
|
|
||
| def test_extra_sans_config_option(juju: jubilant.Juju) -> None: | ||
|
|
@@ -98,3 +123,157 @@ def test_extra_sans_config_option(juju: jubilant.Juju) -> None: | |
| assert expected_sans not in client_cert_sans, ( | ||
| f"sans value {expected_sans} found in certificate sans {client_cert_sans}" | ||
| ) | ||
|
|
||
| logger.info("Remove relation with %s", TLS_NAME) | ||
| juju.remove_relation(f"{APP_NAME}:client-certificates", f"{TLS_NAME}:certificates") | ||
| juju.wait( | ||
| lambda status: are_agents_idle(status, APP_NAME, idle_period=30, unit_count=NUM_UNITS), | ||
| timeout=600, | ||
| ) | ||
|
|
||
|
|
||
| def test_initialize_vault(juju: jubilant.Juju, substrate: Substrate) -> None: | ||
reneradoi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| """Initialize Vault and wait for it to be ready.""" | ||
| # follows the procedure for initializing and unsealing Vault as described in | ||
| # https://canonical-vault-charms.readthedocs-hosted.com/en/latest/tutorial/getting_started_k8s/#deploy-vault | ||
| logger.info("Initializing Vault") | ||
|
|
||
| logger.info("Getting the Vault address") | ||
| vault_units = juju.status().get_units(VAULT_NAME) | ||
| vault_unit = next(iter(vault_units.values())) | ||
| vault_ip = ( | ||
| juju.status().apps[VAULT_NAME].address | ||
| if substrate == Substrate.K8S | ||
| else vault_unit.public_address | ||
| ) | ||
| secrets = juju.secrets() | ||
|
|
||
| logger.info("Extracting Vault's CA certificate") | ||
| vault_ca = None | ||
| for secret in secrets: | ||
| if secret.label == "self-signed-vault-ca-certificate": | ||
| vault_ca = juju.show_secret(identifier=secret.uri, reveal=True).content.get( | ||
| "certificate" | ||
| ) | ||
| assert vault_ca, "Vault CA certificate not found in secrets" | ||
| Path("./vault_ca.pem").write_text(vault_ca) | ||
|
|
||
| # point the locally installed Vault client to the Vault deployment | ||
| vault_env = os.environ.copy() | ||
| vault_env["VAULT_CACERT"] = "./vault_ca.pem" | ||
| vault_env["VAULT_ADDR"] = f"https://{vault_ip}:8200" | ||
|
|
||
| # initialize the deployed Vault | ||
| logger.info("Running vault operator init") | ||
| init_cmd = [ | ||
| "vault", | ||
| "operator", | ||
| "init", | ||
| "-key-shares=1", | ||
| "-key-threshold=1", | ||
| ] | ||
| init_result = subprocess.run( | ||
| init_cmd, check=True, text=True, capture_output=True, env=vault_env | ||
| ) | ||
| logger.info(f"Vault operator init output: {init_result.stdout}") | ||
| init_results_list = [line.strip() for line in init_result.stdout.splitlines() if line.strip()] | ||
|
|
||
| # on init, Vault returns the root token and a key that are required for unsealing Vault | ||
| unseal_key = init_results_list[0].split(":")[1].strip() | ||
| root_token = init_results_list[1].split(":")[1].strip() | ||
| vault_env["VAULT_TOKEN"] = root_token | ||
|
|
||
| # unseal the deployed Vault | ||
| logger.info("Running vault operator unseal") | ||
| unseal_cmd = [ | ||
| "vault", | ||
| "operator", | ||
| "unseal", | ||
| unseal_key, | ||
| ] | ||
| unseal_result = subprocess.run( | ||
| unseal_cmd, check=True, text=True, capture_output=True, env=vault_env | ||
| ) | ||
| logger.info(f"Vault operator unseal output: {unseal_result.stdout}") | ||
|
|
||
| # authorize Vault charm | ||
| # create a one-time token and store it as a secret | ||
| logger.info("Creating Vault token for the vault charm") | ||
| create_token_cmd = [ | ||
| "vault", | ||
| "token", | ||
| "create", | ||
| "-ttl=60m", | ||
| ] | ||
| create_token_result = subprocess.run( | ||
| create_token_cmd, check=True, text=True, capture_output=True, env=vault_env | ||
| ) | ||
| logger.info(f"Vault token create output: {create_token_result.stdout}") | ||
| token_regex = r"token\s+([\w\.]+)" | ||
|
|
||
| # extract token using regex | ||
| match = re.search(token_regex, create_token_result.stdout) | ||
| assert match, "Failed to extract token from Vault token create output" | ||
| charm_vault_token = match.group(1) | ||
| secret_id = juju.add_secret( | ||
| "vault-token", | ||
| { | ||
| "token": charm_vault_token, | ||
| }, | ||
| ) | ||
|
|
||
| assert secret_id, "Failed to create vault-token secret" | ||
| juju.grant_secret("vault-token", VAULT_NAME) | ||
|
|
||
| # authorize the charm to interact with Vault using the token value from the secret | ||
| vault_unit_name = next(iter(vault_units)) | ||
| action = juju.run( | ||
| unit=vault_unit_name, | ||
| action="authorize-charm", | ||
| params={ | ||
| "secret-id": str(secret_id), | ||
| }, | ||
| ) | ||
|
|
||
| assert action.status == "completed", "Action should succeed" | ||
| juju.wait(lambda status: are_apps_active_and_agents_idle(status, VAULT_NAME)) | ||
|
|
||
|
|
||
| async def test_certificate_denied(juju: jubilant.Juju) -> None: | ||
| """Process denied certificate request.""" | ||
| logger.info("Integrate %s with %s for Intermediate CA", VAULT_NAME, TLS_NAME) | ||
| juju.integrate(f"{VAULT_NAME}:tls-certificates-pki", TLS_NAME) | ||
| juju.wait(lambda status: are_agents_idle(status, VAULT_NAME, idle_period=30), timeout=600) | ||
|
|
||
| logger.info("Integrate Valkey with Vault for client TLS") | ||
| logger.info("Certificate requests should be denied because Vault does not allow IP SANs") | ||
| juju.integrate(f"{APP_NAME}:client-certificates", VAULT_NAME) | ||
| juju.wait( | ||
| lambda status: does_status_match( | ||
| status, | ||
| expected_unit_statuses={APP_NAME: [TLSStatuses.CERTIFICATE_DENIED.value]}, | ||
| num_units={APP_NAME: NUM_UNITS}, | ||
| ), | ||
| timeout=600, | ||
| ) | ||
|
|
||
| logger.info("Ensure access without TLS is still possible") | ||
| hostnames = get_cluster_hostnames(juju, APP_NAME) | ||
| result = await set_key( | ||
| hostnames=hostnames, | ||
| username=CharmUsers.VALKEY_ADMIN.value, | ||
| password=get_password(juju, user=CharmUsers.VALKEY_ADMIN), | ||
| tls_enabled=False, | ||
| key=TEST_KEY, | ||
| value=TEST_VALUE, | ||
| ) | ||
| assert result == "OK", "Failed to write data without TLS" | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add an additional test where you remove the relation with Vault and the status goes away?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea - I found a nice improvement for the |
||
| logger.info("Removing TLS relation again") | ||
| juju.remove_relation(f"{APP_NAME}:client-certificates", VAULT_NAME) | ||
| juju.wait( | ||
| lambda status: are_apps_active_and_agents_idle( | ||
| status, APP_NAME, idle_period=30, unit_count=NUM_UNITS | ||
| ), | ||
| timeout=100, | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have 2 questions here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory we should not, but as we have the same safeguard in place for the certificate available event ... I've added some error logging for this case.