Mismatched bindings lead to missing vault_url and incomplete relation

[David Ames]

Since commit db22a4652c6ec4bfbaf1b7cbd529f38a60a138df [0] for LP Bug#1826892 [1] if the bindings do not match on both ends of the secrets storage (vault-kv) relation the vault_url is not published even though role ids and tokens are. This leads to the relation being incomplete and is very difficult to diagnose.

We have now seen this in the wild on multiple occasions.

We need a more robust solution that either errors out or very clearly communicates to the end user what needs fixing. The solution needs to resolve this bug and LP Bug#1826892.

I recall discussing the commit above at a sprint. I question the requirement to have matching bindings and it seems to me even if the bindings match but it is a routed environment (same space but different IP subnets) the current code would not work.

[0] https://github.com/openstack-charmers/charm-interface-vault-kv/commit/db22a4652c6ec4bfbaf1b7cbd529f38a60a138df
[1] https://bugs.launchpad.net/vault-charm/+bug/1826892

[David Ames (thedac)]

Also adding the vault charm itself as better reporting may be required.

[Brad Marshall (brad-marshall)]

FWIW its not as simple as having the secret-storage and vault:secrets bindings the same, I had to update the vault:access binding to match as well. This was with cs:vault-40.

[David Ames (thedac)]

A closer look at the code shows that Brad is 100% correct. The vault charm completely ignores the secrets relation binding and is checking either or both access and external extra bindings.

See [0] which originally introduced the external extra binding.

Triage:

1) Update vault-kv interface to stop verifying the binding. The charm not the interface should be responsible for this.
2) Revamp vault.get_vip
3) Revamp the handler for send_vault_url_and_ca
Priority in this order:
* External binding
* Check the secrets relation binding
* Check access binding

Keep the original goal of [0] in mind. Enabling publishing an external access vip.

[0] https://github.com/openstack/charm-vault/commit/c7e2c531ec9038c0f9b4b8405b76624a6c271558

[Cory Johns (johnsca)]

Related older bug and PR for the same issue:

https://bugs.launchpad.net/vault-charm/+bug/1843809
https://github.com/openstack-charmers/charm-interface-vault-kv/pull/6

[Cory Johns (johnsca)]

Oh, I also filed a Juju bug related to this because fundamentally it comes down to Juju not providing the charm enough info about how the bindings are being used (and / or not providing for a given relation to bound to a space vs the entire relation endpoint).

https://bugs.launchpad.net/juju/+bug/1913495

[David Ames (thedac)]

First interface PR for after 21.01 release [0] will stop the bleeding on this bug but not resolve the need for an "external" binding concept [1] [2].

Ultimately, the solution will need to be implemented in the vault charm itself with an address selection function (potentially get_vault_url) that can take in quite a few considerations.

We have a few primary problems for the original intention of external binding that need consensus on solutions

* Juju does not have the primitives to determine if a request is coming from an internal or external source
* Juju does not have a per relation *ID* binding, i.e. "secrets:2" rather than "secrets"
* It is not that we have too few possible bindings but too many: "access", "external", "certificates", "secrets", "cluster"
  * It is very likely if not inevitable that the relation, secrets, could be bound to a different space than the extra binding, access. How do we choose?

Possible solutions

* It is possible the "external" binding concept needs to be abandoned.
* The vault-kv interface can add an "external" variable to the request such that the vault charm's get_vault_url can then be guaranteed to use its "external" binding.

[0] https://github.com/openstack-charmers/charm-interface-vault-kv/pull/13
[1] https://github.com/openstack-charmers/charm-interface-vault-kv/pull/5/files
[2] https://bugs.launchpad.net/vault-charm/+bug/1826892

[David Ames (thedac)]

[0] Has landed marking Fix Committed for the interface. Leaving the primary charm open as we could still do some refactoring there.

[0] https://github.com/openstack-charmers/charm-interface-vault-kv/pull/13