From 2891163d10d65189471364f97e5782c6d7b27bca Mon Sep 17 00:00:00 2001 From: yazansalti Date: Sat, 14 Sep 2024 14:47:34 +0200 Subject: [PATCH] Uses unit as part of validity string --- charmcraft.yaml | 65 ++++++++--- src/charm.py | 106 ++++++++++++------ tests/integration/test_integration.py | 9 +- tests/unit/test_charm_collect_status.py | 10 +- tests/unit/test_charm_configure.py | 31 +++-- .../test_charm_get_issued_certificates.py | 2 +- 6 files changed, 144 insertions(+), 79 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 86f45ae..be072f5 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -76,37 +76,70 @@ config: ca-common-name: type: string default: self-signed-certificates-operator - description: Common name to be used by the Certificate Authority. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. + description: > + Common name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-organization: type: string - description: Organization name to be used by the Certificate Authority. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. + description: > + Organization name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-organizational-unit: type: string - description: Organizational unit to be used by the Certificate Authority. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. + description: > + Organizational unit to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-email-address: type: string - description: Email address to be used by the Certificate Authority. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. + description: > + Email address to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-country-name: type: string - description: Country name to be used by the Certificate Authority. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. + description: > + Country name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-state-or-province-name: type: string - description: State or province name to be used by the Certificate Authority. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. + description: > + State or province name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. ca-locality-name: type: string - description: Locality name to be used by the Certificate Authority. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. + description: > + Locality name to be used by the Certificate Authority. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. root-ca-validity: - type: int - default: 365 - description: RootCA certificate validity (in days). This value should be longer than twice the certificate-validity. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. + type: string + default: 365d + description: > + RootCA certificate validity. + The given value must be followed by one of: "s" for seconds, "h" for hours, "d" for days and "y" for years. + For example, "15s" for 15 seconds, "10y" for 10 years. + If no units are given, the unit will be assumed as days. + Defaults to 365 days. + This value should be longer than twice the certificate-validity. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. certificate-validity: - type: int - default: 90 - description: Certificate validity (in days). This value must be shorter than half the root-ca-validity. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. - validity-unit: type: string - default: days - description: Validity unit weeks, days, hours, minutes or seconds, it is days by default and will be applied to both root-ca-validity and certificate-validity. Changing this value will trigger generation of a new CA certificate, revoking all previously issued certificates. + default: 90d + description: > + Signed certificate validity. + The given value must be followed by one of: "s" for seconds, "h" for hours, "d" for days and "y" for years. + For example, "15s" for 15 seconds, "10y" for 10 years. + If no units are given, the unit will be assumed as days. + Defaults to 365 days. + This value should be longer than twice the certificate-validity. + Changing this value will trigger generation of a new CA certificate, + revoking all previously issued certificates. actions: get-ca-certificate: diff --git a/src/charm.py b/src/charm.py index 314c203..3569b24 100755 --- a/src/charm.py +++ b/src/charm.py @@ -100,24 +100,39 @@ def _is_ca_cert_active(self) -> bool: return secret_info.expires > datetime.now(secret_info.expires.tzinfo) @property - def _config_root_ca_certificate_validity(self) -> timedelta: - """Return Root CA certificate validity (timedelta).""" - return self._int_config_to_timedelta( - int(self.model.config.get("root-ca-validity")), # type: ignore[arg-type] - self._config_validity_unit, - ) + def _config_root_ca_certificate_validity(self) -> timedelta | None: + """Return Root CA certificate validity. + + Returns: + int: Certificate validity + """ + try: + validity = self._parse_config_time_string( + str(self.model.config.get("root-ca-validity", "")) + ) + except ValueError: + logger.warning('config option "certificate-validity" is invalid.', exc_info=True) + return None + return validity @property - def _config_validity_unit(self) -> str: - """Return Validity unit. + def _config_certificate_validity(self) -> timedelta | None: + """Returns certificate validity (in seconds). Returns: - str: Validity unit. + int: Certificate validity (in seconds) """ - return str(self.model.config.get("validity-unit", "days")) + try: + validity = self._parse_config_time_string( + str(self.model.config.get("certificate-validity", "")) + ) + except ValueError: + logger.warning('config option "certificate-validity" is invalid.', exc_info=True) + return None + return validity @property - def _ca_certificate_renewal_threshold(self) -> timedelta: + def _ca_certificate_renewal_threshold(self) -> timedelta | None: """Return CA certificate renewal threshold. Which is the time difference between the validity of the root certificate @@ -127,6 +142,9 @@ def _ca_certificate_renewal_threshold(self) -> timedelta: the renewal threshold will be 275 days. This is important so the CA does not expire during the issued certificate validity. """ + if not self._config_root_ca_certificate_validity or not self._config_certificate_validity: + logger.warning("No root CA certificate validity or certificate validity set") + return None return self._config_root_ca_certificate_validity - self._config_certificate_validity def _on_get_issued_certificates(self, event: ActionEvent) -> None: @@ -167,36 +185,30 @@ def _on_rotate_private_key(self, event: ActionEvent): return event.set_results({"result": "New private key and CA certificate generated and stored."}) - @property - def _config_certificate_validity(self) -> timedelta: - """Returns certificate validity (timedelta).""" - return self._int_config_to_timedelta( - int(self.model.config.get("certificate-validity")), # type: ignore[arg-type] - self._config_validity_unit, - ) + def _parse_config_time_string(self, time_str: str) -> timedelta: + """Parse a given time string. - def _int_config_to_timedelta(self, value: int, unit: str) -> timedelta: - """Convert the config value to a timedelta. + It must be a number followed by either an + s for seconds, h for hours, d for days or y for years. Args: - value (int): Config value. - unit (str): Config unit. - + time_str: the input string. Ex: "15s", "365d", "10w" + or "10" and will be converted to days Returns: - timedelta: Timedelta. + timedelta object representing the given string """ - if unit == "weeks": - return timedelta(weeks=value) - elif unit == "days": - return timedelta(days=value) - elif unit == "hours": - return timedelta(hours=value) - elif unit == "minutes": - return timedelta(minutes=value) - elif unit == "seconds": + if time_str.isnumeric(): + return timedelta(days=int(time_str)) + value, unit = int(time_str[:-1]), time_str[-1] + if unit == "s": return timedelta(seconds=value) - else: + elif unit == "h": + return timedelta(hours=value) + elif unit == "d": return timedelta(days=value) + elif unit == "w": + return timedelta(weeks=value) + raise ValueError(f"unsupported time string format: {time_str}") @property def _config_ca_common_name(self) -> Optional[str]: @@ -246,8 +258,14 @@ def _generate_root_certificate(self) -> None: If the secret is already created, we simply update its content, else we create a new secret. """ - if not self._config_ca_common_name: - raise ValueError("CA common name should not be empty") + if ( + not self._config_ca_common_name + or not self._config_root_ca_certificate_validity + or not self._config_certificate_validity + or not self._ca_certificate_renewal_threshold + ): + logger.warning("Missing configuration for root CA certificate") + return private_key = generate_private_key() ca_certificate = generate_ca( private_key=private_key, @@ -315,6 +333,14 @@ def _move_active_ca_cert_to_expiring(self): The validity of the expiring CA can't be shorter than the validity of the issued certificates. """ + if ( + not self._config_ca_common_name + or not self._config_root_ca_certificate_validity + or not self._config_certificate_validity + or not self._ca_certificate_renewal_threshold + ): + logger.warning("Missing configuration for expiring CA certificate") + return try: current_active_ca_cert_secret = self.model.get_secret( label=CA_CERTIFICATES_SECRET_LABEL @@ -419,6 +445,14 @@ def _generate_self_signed_certificate( is_ca (bool): Whether the certificate is a CA relation_id (int): Relation id """ + if ( + not self._config_ca_common_name + or not self._config_root_ca_certificate_validity + or not self._config_certificate_validity + or not self._ca_certificate_renewal_threshold + ): + logger.warning("Missing configuration for self-signed certificate") + return ca_certificate_secret = self.model.get_secret(label=CA_CERTIFICATES_SECRET_LABEL) ca_certificate_secret_content = ca_certificate_secret.get_content(refresh=True) ca_certificate = Certificate.from_string(ca_certificate_secret_content["ca-certificate"]) diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 74acb9a..07f2bea 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -60,8 +60,8 @@ async def deploy(ops_test: OpsTest, request): trust=True, config={ "ca-common-name": CA_COMMON_NAME, - "root-ca-validity": 200, - "certificate-validity": 100, + "root-ca-validity": "200", + "certificate-validity": "100", "ca-email-address": "test@example.com", "ca-country-name": "US", "ca-state-or-province-name": "California", @@ -136,9 +136,8 @@ async def test_given_tls_requirer_is_integrated_when_certificate_expires_then_ne assert application await application.set_config( { - "root-ca-validity": 60, - "certificate-validity": 30, - "validity-unit": "seconds", + "root-ca-validity": "600s", + "certificate-validity": "30s", } ) await ops_test.model.wait_for_idle( diff --git a/tests/unit/test_charm_collect_status.py b/tests/unit/test_charm_collect_status.py index a858347..90126ab 100644 --- a/tests/unit/test_charm_collect_status.py +++ b/tests/unit/test_charm_collect_status.py @@ -21,7 +21,7 @@ def test_given_invalid_config_when_collect_unit_status_then_status_is_blocked(se state_in = scenario.State( config={ "ca-common-name": "", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, ) @@ -36,7 +36,7 @@ def test_given_invalid_validity_config_when_collect_unit_status_then_status_is_b state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 0, + "certificate-validity": "0", }, leader=True, ) @@ -53,8 +53,8 @@ def test_given_root_ca_validity_config_not_twice_the_certificate_validity_when_c state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, - "root-ca-validity": 0, + "certificate-validity": "100", + "root-ca-validity": "0", }, leader=True, ) @@ -77,7 +77,7 @@ def test_given_valid_config_when_collect_unit_status_then_status_is_active( state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, ) diff --git a/tests/unit/test_charm_configure.py b/tests/unit/test_charm_configure.py index 89298b8..e387ca2 100644 --- a/tests/unit/test_charm_configure.py +++ b/tests/unit/test_charm_configure.py @@ -50,8 +50,8 @@ def test_given_valid_config_when_config_changed_then_ca_certificate_is_pushed_to state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, - "root-ca-validity": 200, + "certificate-validity": "100", + "root-ca-validity": "200", }, leader=True, ) @@ -77,8 +77,8 @@ def test_given_valid_config_when_config_changed_then_ca_certificate_is_stored_in state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": certificate_validity, - "root-ca-validity": root_ca_validity, + "certificate-validity": str(certificate_validity), + "root-ca-validity": str(root_ca_validity), }, leader=True, ) @@ -111,7 +111,7 @@ def test_given_root_certificate_not_stored_when_config_changed_then_existing_cer state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, secrets=frozenset(), @@ -156,8 +156,8 @@ def test_given_new_root_ca_config_when_config_changed_then_new_root_ca_is_replac "ca-email-address": "abc@example.com", "ca-country-name": "CA", "ca-locality-name": "Montreal", - "certificate-validity": 100, - "root-ca-validity": 200, + "certificate-validity": "100", + "root-ca-validity": "200", }, leader=True, secrets={ca_certificate_secret}, @@ -214,9 +214,8 @@ def test_given_root_ca_about_to_expire_then_root_ca_is_marked_expiring_and_new_o state_in = scenario.State( config={ "ca-common-name": "example.com", - "root-ca-validity": 60, - "certificate-validity": 30, - "validity-unit": "seconds", + "root-ca-validity": "60s", + "certificate-validity": "30s", }, leader=True, secrets={ca_certificate_secret}, @@ -291,8 +290,8 @@ def test_given_outstanding_certificate_requests_when_config_changed_then_certifi state_in = scenario.State( config={ "ca-common-name": "example.com", - "certificate-validity": 100, - "root-ca-validity": 200, + "certificate-validity": "100", + "root-ca-validity": "200", }, leader=True, relations={tls_relation}, @@ -337,7 +336,7 @@ def test_given_valid_config_and_unit_is_leader_when_secret_expired_then_new_ca_c state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, secrets={ca_certificates_secret}, @@ -389,7 +388,7 @@ def test_given_initial_config_when_config_changed_then_stored_ca_common_name_use state_in = scenario.State( config={ "ca-common-name": "common-name-new.example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, secrets={ca_certificates_secret}, @@ -434,8 +433,8 @@ def test_given_certificate_transfer_relations_when_configure_then_ca_cert_is_adv leader=True, config={ "ca-common-name": "example.com", - "certificate-validity": 100, - "root-ca-validity": 200, + "certificate-validity": "100", + "root-ca-validity": "200", }, ) diff --git a/tests/unit/test_charm_get_issued_certificates.py b/tests/unit/test_charm_get_issued_certificates.py index 4c6244a..54b61f4 100644 --- a/tests/unit/test_charm_get_issued_certificates.py +++ b/tests/unit/test_charm_get_issued_certificates.py @@ -71,7 +71,7 @@ def test_given_certificates_issued_when_get_issued_certificates_action_then_acti state_in = scenario.State( config={ "ca-common-name": "example.com", - "certificate-validity": 100, + "certificate-validity": "100", }, leader=True, )