From f775b3d53daca2104736d2a4ff7692eda4848f47 Mon Sep 17 00:00:00 2001 From: Guillaume Boutry Date: Thu, 20 Aug 2026 15:29:26 +0200 Subject: [PATCH] feat(storage): align TLS backend UX contracts Pass certificate and CA material through existing charm configuration. Keep the private key routed through the Juju secret mapping. Expose supported generic certificate and TLS-enable options through the existing model-generated CLI, manifest, and Terraform path. Describe Nimble certificate input as PEM bundle content and cover public, secret, boolean, and path placement across backends. Depends-On: https://github.com/canonical/snap-cinder-volume/pull/97 Depends-On: https://review.opendev.org/c/openstack/sunbeam-charms/+/1001791 Partial-Bug: #2161732 Assisted-By: Codex (gpt-5-6-sol) Signed-off-by: Guillaume Boutry --- .../storage/backends/hpe3par/backend.py | 1 - .../storage/backends/infinidat/backend.py | 8 + .../storage/backends/netapp/backend.py | 23 +-- .../storage/backends/nimble/backend.py | 7 +- .../sunbeam/storage/backends/qnap/backend.py | 4 + .../storage/backends/solidfire/backend.py | 4 + .../sunbeam/storage/backends/stx/backend.py | 9 + .../sunbeam/storage/backends/test_netapp.py | 72 ++++++++ .../storage/backends/test_tls_contract.py | 171 ++++++++++++++++++ 9 files changed, 279 insertions(+), 20 deletions(-) create mode 100644 sunbeam-python/tests/unit/sunbeam/storage/backends/test_tls_contract.py diff --git a/sunbeam-python/sunbeam/storage/backends/hpe3par/backend.py b/sunbeam-python/sunbeam/storage/backends/hpe3par/backend.py index 89a7b5029..db831bf08 100644 --- a/sunbeam-python/sunbeam/storage/backends/hpe3par/backend.py +++ b/sunbeam-python/sunbeam/storage/backends/hpe3par/backend.py @@ -52,7 +52,6 @@ class HPEthreeparConfig(StorageBackendConfig): ), SecretDictField(field="hpe3par-password"), ] = None - # Replication settings replication_device: Annotated[ str | None, diff --git a/sunbeam-python/sunbeam/storage/backends/infinidat/backend.py b/sunbeam-python/sunbeam/storage/backends/infinidat/backend.py index 63893c1de..24455feee 100644 --- a/sunbeam-python/sunbeam/storage/backends/infinidat/backend.py +++ b/sunbeam-python/sunbeam/storage/backends/infinidat/backend.py @@ -31,6 +31,14 @@ class InfinidatConfig(StorageBackendConfig): use_chap_auth: Annotated[ bool | None, Field(description="Use CHAP authentication") ] = None + driver_use_ssl: Annotated[ + bool | None, + Field( + description=( + "Use HTTPS for connections to the InfiniBox management interface." + ) + ), + ] = None # Secrets san_login: Annotated[ diff --git a/sunbeam-python/sunbeam/storage/backends/netapp/backend.py b/sunbeam-python/sunbeam/storage/backends/netapp/backend.py index 82012ec9a..7bfda7491 100644 --- a/sunbeam-python/sunbeam/storage/backends/netapp/backend.py +++ b/sunbeam-python/sunbeam/storage/backends/netapp/backend.py @@ -121,12 +121,7 @@ class NetAppConfig(StorageBackendConfig): netapp_ssl_cert_path: Annotated[ str | None, - Field( - description=( - "The path to a CA_BUNDLE file or directory with certificates of " - "trusted CA." - ) - ), + Field(description="CA bundle PEM content for trusted certificates."), ] = None netapp_login: Annotated[ @@ -155,8 +150,8 @@ class NetAppConfig(StorageBackendConfig): str | None, Field( description=( - "Absolute path to the file containing the private key " - "associated with the certificate." + "Private key PEM content associated with the certificate, " + "supplied through a Juju secret." ) ), SecretDictField(field="netapp-private-key-file"), @@ -164,20 +159,12 @@ class NetAppConfig(StorageBackendConfig): netapp_certificate_file: Annotated[ str | None, - Field( - description="Absolute path to the file containing the digital certificate." - ), - SecretDictField(field="netapp-certificate-file"), + Field(description="Digital certificate PEM content."), ] = None netapp_ca_certificate_file: Annotated[ str | None, - Field( - description=( - "Absolute path to the file containing the public key " - "certificate of the trusted CA." - ) - ), + Field(description="Trusted CA certificate PEM content."), ] = None netapp_certificate_host_validation: Annotated[ diff --git a/sunbeam-python/sunbeam/storage/backends/nimble/backend.py b/sunbeam-python/sunbeam/storage/backends/nimble/backend.py index dfc452e2f..3a821025b 100644 --- a/sunbeam-python/sunbeam/storage/backends/nimble/backend.py +++ b/sunbeam-python/sunbeam/storage/backends/nimble/backend.py @@ -64,7 +64,12 @@ class NimbleConfig(StorageBackendConfig): ] = None nimble_verify_cert_path: Annotated[ str | None, - Field(description="Path to Nimble Array SSL certificate"), + Field( + description=( + "PEM-encoded certificate or CA bundle content used to verify " + "the Nimble array." + ) + ), ] = None san_thin_provision: Annotated[ bool | None, diff --git a/sunbeam-python/sunbeam/storage/backends/qnap/backend.py b/sunbeam-python/sunbeam/storage/backends/qnap/backend.py index 13489f550..fa1a18517 100644 --- a/sunbeam-python/sunbeam/storage/backends/qnap/backend.py +++ b/sunbeam-python/sunbeam/storage/backends/qnap/backend.py @@ -48,6 +48,10 @@ class QnapConfig(StorageBackendConfig): Protocol | None, Field(description="Protocol selector: fc, iscsi."), ] = None + driver_ssl_cert_verify: Annotated[ + bool | None, + Field(description="Verify HTTPS certificates using the system trust store."), + ] = None # Optional backend configuration qnap_management_url: Annotated[ diff --git a/sunbeam-python/sunbeam/storage/backends/solidfire/backend.py b/sunbeam-python/sunbeam/storage/backends/solidfire/backend.py index 9591c31fa..6563d1702 100644 --- a/sunbeam-python/sunbeam/storage/backends/solidfire/backend.py +++ b/sunbeam-python/sunbeam/storage/backends/solidfire/backend.py @@ -52,6 +52,10 @@ class SolidFireConfig(StorageBackendConfig): description="Front-end protocol (Cinder SolidFire driver uses iSCSI).", ), ] = None + driver_ssl_cert_verify: Annotated[ + bool | None, + Field(description="Verify HTTPS certificates using the system trust store."), + ] = None sf_emulate_512: Annotated[ bool | None, Field(description="Set 512 byte emulation on volume creation."), diff --git a/sunbeam-python/sunbeam/storage/backends/stx/backend.py b/sunbeam-python/sunbeam/storage/backends/stx/backend.py index 91094337f..ec5dcbc22 100644 --- a/sunbeam-python/sunbeam/storage/backends/stx/backend.py +++ b/sunbeam-python/sunbeam/storage/backends/stx/backend.py @@ -46,6 +46,15 @@ class StxConfig(StorageBackendConfig): Protocol | None, Field(description="Protocol selector: iscsi."), ] = None + driver_ssl_cert: Annotated[ + str | None, + Field( + description=( + "PEM-encoded SSL certificate content for HTTPS connections to " + "the storage array." + ) + ), + ] = None # Optional backend configuration seagate_pool_name: Annotated[ diff --git a/sunbeam-python/tests/unit/sunbeam/storage/backends/test_netapp.py b/sunbeam-python/tests/unit/sunbeam/storage/backends/test_netapp.py index abb202f9e..d26b2bd87 100644 --- a/sunbeam-python/tests/unit/sunbeam/storage/backends/test_netapp.py +++ b/sunbeam-python/tests/unit/sunbeam/storage/backends/test_netapp.py @@ -5,6 +5,7 @@ import pytest +from sunbeam.storage.models import SecretDictField from tests.unit.sunbeam.storage.backends.test_common import BaseBackendTests @@ -23,3 +24,74 @@ def test_backend_type_is_netapp(self, backend): def test_charm_name_is_netapp_charm(self, backend): """Test that charm name is cinder-volume-netapp.""" assert backend.charm_name == "cinder-volume-netapp" + + def test_certificate_options_accept_pem_content(self, backend): + """Test that public certificate options describe PEM content input.""" + fields = backend.config_type().model_fields + for field_name in ( + "netapp_ssl_cert_path", + "netapp_certificate_file", + "netapp_ca_certificate_file", + ): + assert "PEM content" in fields[field_name].description + + def test_only_private_tls_material_is_secret(self, backend): + """Test that only the private key uses Juju secret handling.""" + fields = backend.config_type().model_fields + public_fields = ( + "netapp_ssl_cert_path", + "netapp_certificate_file", + "netapp_ca_certificate_file", + ) + for field_name in public_fields: + assert not any( + isinstance(metadata, SecretDictField) + for metadata in fields[field_name].metadata + ) + + private_key_metadata = fields["netapp_private_key_file"].metadata + private_key_secret = next( + metadata + for metadata in private_key_metadata + if isinstance(metadata, SecretDictField) + ) + assert private_key_secret.field == "netapp-private-key-file" + + def test_private_key_accepts_pem_content_via_juju_secret(self, backend): + """Test that the private key UX describes its content and transport.""" + description = ( + backend.config_type().model_fields["netapp_private_key_file"].description + ) + assert "PEM content" in description + assert "Juju secret" in description + + def test_tls_material_terraform_mapping( + self, backend, mock_deployment, mock_manifest + ): + """Test public PEM content and the private key use their intended paths.""" + ca_bundle = "-----BEGIN CERTIFICATE-----\nbundle\n-----END CERTIFICATE-----" + certificate = "-----BEGIN CERTIFICATE-----\nclient\n-----END CERTIFICATE-----" + ca_certificate = "-----BEGIN CERTIFICATE-----\nca\n-----END CERTIFICATE-----" + private_key = "-----BEGIN PRIVATE KEY-----\nkey\n-----END PRIVATE KEY-----" + config = backend.config_type().model_validate( + { + "san-ip": "192.0.2.10", + "protocol": "iscsi", + "netapp-ssl-cert-path": ca_bundle, + "netapp-certificate-file": certificate, + "netapp-ca-certificate-file": ca_certificate, + "netapp-private-key-file": private_key, + } + ) + + tfvars = backend.build_terraform_vars( + mock_deployment, mock_manifest, "test-netapp", config + ) + + assert tfvars["charm_config"]["netapp-ssl-cert-path"] == ca_bundle + assert tfvars["charm_config"]["netapp-certificate-file"] == certificate + assert tfvars["charm_config"]["netapp-ca-certificate-file"] == ca_certificate + assert tfvars["secrets"]["netapp-private-key-file"] == "netapp-private-key-file" + assert "netapp-certificate-file" not in tfvars["secrets"] + assert "netapp-ssl-cert-path" not in tfvars["secrets"] + assert "netapp-ca-certificate-file" not in tfvars["secrets"] diff --git a/sunbeam-python/tests/unit/sunbeam/storage/backends/test_tls_contract.py b/sunbeam-python/tests/unit/sunbeam/storage/backends/test_tls_contract.py new file mode 100644 index 000000000..e59c13b58 --- /dev/null +++ b/sunbeam-python/tests/unit/sunbeam/storage/backends/test_tls_contract.py @@ -0,0 +1,171 @@ +# SPDX-FileCopyrightText: 2026 - Canonical Ltd +# SPDX-License-Identifier: Apache-2.0 + +"""Cross-backend storage TLS UX contract tests.""" + +from dataclasses import dataclass + +import click +import pytest +from click.testing import CliRunner + +from sunbeam.storage.cli_base import StorageBackendCLIBase +from sunbeam.storage.models import SecretDictField +from tests.unit.sunbeam.storage.backends.conftest import BACKENDS + +GENERIC_CERT_BACKENDS = ( + "dellpowerstore", + "dellpowervault", + "hitachi", + "purestorage", + "stx", +) + + +@dataclass(frozen=True) +class TLSField: + """Expected user-facing and transport contract for a TLS-related field.""" + + backend: str + field: str + is_boolean: bool = False + is_secret: bool = False + is_path: bool = False + + @property + def alias(self) -> str: + """Return the charm configuration alias.""" + return self.field.replace("_", "-") + + +TLS_FIELDS = ( + *(TLSField(backend, "driver_ssl_cert") for backend in GENERIC_CERT_BACKENDS), + TLSField("hitachi", "hitachi_mirror_ssl_cert"), + TLSField("nimble", "nimble_verify_certificate", is_boolean=True), + TLSField("nimble", "nimble_verify_cert_path"), + TLSField("netapp", "netapp_ssl_cert_path"), + TLSField("netapp", "netapp_private_key_file", is_secret=True), + TLSField("netapp", "netapp_certificate_file"), + TLSField("netapp", "netapp_ca_certificate_file"), + TLSField("netapp", "netapp_certificate_host_validation", is_boolean=True), + TLSField("infinidat", "driver_use_ssl", is_boolean=True), + TLSField("qnap", "driver_ssl_cert_verify", is_boolean=True), + TLSField("solidfire", "driver_ssl_cert_verify", is_boolean=True), + TLSField("dellsc", "dell_sc_verify_cert", is_boolean=True), + TLSField("synology", "synology_ssl_verify", is_boolean=True), + TLSField("zadara", "zadara_vpsa_use_ssl", is_boolean=True), + TLSField("zadara", "zadara_ssl_cert_verify", is_boolean=True), + TLSField("dellsc", "san_private_key", is_path=True), + TLSField("fujitsueternusdx", "fujitsu_private_key_path", is_path=True), + TLSField("ibmgpfs", "gpfs_private_key", is_path=True), + TLSField("ibmgpfs", "gpfs_hosts_key_file", is_path=True), +) + + +def _backend(name): + """Return a backend instance by registry name.""" + return BACKENDS[name]() + + +def _field_is_secret(field_info) -> bool: + """Return whether a model field uses Juju secret mapping.""" + return any( + isinstance(metadata, SecretDictField) for metadata in field_info.metadata + ) + + +def _add_options(backend) -> dict[str, click.Option]: + """Return generated add-command options keyed by Click parameter name.""" + params = StorageBackendCLIBase(backend)._build_add_params() + return {param.name: param for param in params if isinstance(param, click.Option)} + + +@pytest.mark.parametrize("backend_name", GENERIC_CERT_BACKENDS) +def test_generic_certificate_is_pem_content_in_manifest(backend_name): + """Generic certificate fields accept content and appear in the schema.""" + backend = _backend(backend_name) + field = backend.config_type().model_fields["driver_ssl_cert"] + + assert "PEM" in field.description + assert "path" not in field.description.lower() + assert ( + "driver-ssl-cert" + in backend.config_type().model_json_schema(by_alias=True)["properties"] + ) + + +def test_nimble_certificate_bundle_is_described_as_content(): + """Nimble asks for PEM bundle content instead of a local path.""" + backend = _backend("nimble") + description = ( + backend.config_type().model_fields["nimble_verify_cert_path"].description + ) + + assert "PEM" in description + assert "content" in description.lower() + assert "path" not in description.lower() + + +@pytest.mark.parametrize("contract", TLS_FIELDS) +def test_tls_field_manifest_type_and_secret_classification(contract): + """Manifest schemas and secret markers match the TLS contract.""" + backend = _backend(contract.backend) + model = backend.config_type() + field = model.model_fields[contract.field] + schema = model.model_json_schema(by_alias=True)["properties"][contract.alias] + schema_text = str(schema) + + assert ("boolean" in schema_text) is contract.is_boolean + assert _field_is_secret(field) is contract.is_secret + if contract.is_path: + assert any(word in field.description.lower() for word in ("file", "path")) + + +@pytest.mark.parametrize("contract", TLS_FIELDS) +def test_generated_cli_exposes_tls_field_with_model_help(contract): + """Generated add commands preserve model types and descriptions.""" + backend = _backend(contract.backend) + field = backend.config_type().model_fields[contract.field] + option = _add_options(backend)[contract.field] + + expected_type = click.BOOL if contract.is_boolean else click.STRING + assert option.type is expected_type + assert option.help == field.description + if not contract.is_boolean and not contract.is_path: + assert "PEM" in option.help + assert "path" not in option.help.lower() + + +@pytest.mark.parametrize("contract", TLS_FIELDS) +def test_registered_cli_help_exposes_tls_field(contract): + """Registered add commands render each TLS option in actual Click help.""" + backend = _backend(contract.backend) + + @click.group() + def add(): + """Test storage add group.""" + + StorageBackendCLIBase(backend).register_add_cli(add) + result = CliRunner().invoke(add, [backend.backend_type, "--help"]) + + assert result.exit_code == 0, result.output + assert f"--{contract.alias}" in result.output + + +@pytest.mark.parametrize("contract", TLS_FIELDS) +def test_terraform_places_tls_field_in_config_or_secret( + contract, mock_deployment, mock_manifest +): + """Terraform uses ordinary config except for raw private-key material.""" + backend = _backend(contract.backend) + marker = True if contract.is_boolean else f"{contract.alias}-value" + config = backend.config_type().model_construct(**{contract.field: marker}) + + tfvars = backend.build_terraform_vars( + mock_deployment, mock_manifest, "tls-audit", config + ) + + assert tfvars["charm_config"][contract.alias] == marker + assert (contract.alias in tfvars["secrets"]) is contract.is_secret + if contract.is_secret: + assert tfvars["secrets"][contract.alias] == contract.alias