canonical / vault-k8s-operator

Vault secure, store and tightly control access to tokens, passwords, certificates, encryption keys for protecting secrets and other sensitive data.
https://charmhub.io/vault-k8s
Apache License 2.0
8 stars 6 forks source link

Changing the common name after it has been set in vault-k8s (PKI) has no effect #367

Closed saltiyazan closed 3 months ago

saltiyazan commented 3 months ago

Bug Description

When the common name used to configure the PKI engine in the charm changes, the PKI role is not updated to allow creating certificates for the new common name

To Reproduce

juju deploy vault-k8s juju deploy self-signed-certificates juju integrate vault:tls-certificates-pki self-signed-certificates juju deploy tls-certificates-requirer juju config vault common_name=mydomain.com juju config tls-certificates-requirer common_name=example.com juju config vault common_name=example.com juju integrate tls-certificates-requirer vault:vault-pki

Environment

Juju 3.4 and Vault-k8s 1.15 beta

Relevant log output

unit-vault-0: 13:42:42 INFO unit.vault/0.juju-log [TLS] Sent CA certificate to other relations
....
unit-vault-0: 13:47:38 WARNING unit.vault/0.juju-log Error while signing PKI certificate: common name admin not allowed by this role, on post https://10.122.32.33:8200/v1/charm-pki/sign/charm-pki

Additional context

No response

DanielArndt commented 3 months ago

In addition to this, the logic for checking if the old CA certificate matches is incorrect.

https://github.com/canonical/vault-k8s-operator/blob/e5e7f5bb27615609bd00b18f35e7b4177303896e/src/charm.py#L474-L478

In the code above, we return None if the cert doesn't match.

https://github.com/canonical/vault-k8s-operator/blob/e5e7f5bb27615609bd00b18f35e7b4177303896e/src/charm.py#L383-L386

In the caller [above], we return early if the return value is None, so we return before updating the certificate