feat: add vault-kv relation #30

gboutry · 2023-08-21T13:17:38Z

Description

This PR introduces the Vault-kv relation to vault-k8s.

The vault-kv interface can be discussed here

This interface is a port from the machine charm interface with some note worthy differences:

Secret role id is no longer access by the requirer through a wrapped token
The provider side shall fill in the vault_url in the app databag
isolated is not allowed anymore as this concept is not really translatable to k8s charms

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have added tests that validate the behaviour of the software
I validated that new and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

gruyaume

Got a couple of comments + let's make sure we review/merge the charm-relation-interfaces pr before this one

lib/charms/vault_k8s/v0/vault_kv.py

tests/unit/test_charm.py

lib/charms/vault_k8s/v0/vault_kv.py

gboutry · 2023-08-22T13:01:24Z

Convert to draft to make sure interface PR is merged first

lib/charms/vault_k8s/v0/vault_kv.py

src/charm.py

gboutry · 2023-08-25T08:37:27Z

Force push is only rebasing on main, with no noticeable change (except the part on vault SVC to align with main).

Will now take into account the comments on interface, and on this pull request. Thank you for all your feedback.

lib/charms/vault_k8s/v0/vault_kv.py

src/vault.py

gboutry · 2023-09-15T16:17:00Z

Still WIP, mainly a rebase over main branch + integration of charm-relation-interface

Getting a secret by label only returns the short form of a secret id (without the model's UUID), which is fine when the two applications are inside the same model, but leads to error when in different models. Creating a secret always returns the long form of a secret id, therefore, storing the secret id in the peer relationship with key: label, value: secret id, to ensure long form of secret id.

gboutry · 2023-09-20T13:54:23Z

Rebase on main to get Ingress address into vault server's certificate.

Adding ca_certificate as relation data, cf: canonical/charm-relation-interfaces#105

Storing secret ids in peer relationship because getting secret by label only lead you (after a grant) to get a secret id in short form, which does not work over CMRs

src/charm.py

src/vault.py

Only enable the approle auth method if at least one vault-kv relation was created.

Remove any modification to unit status from the kv handler, replaced by logging calls. Move KV policy to dedicated file.

gruyaume

Overall looks good, got a couple more comments though. Feels like we're almost there :)

lib/charms/vault_k8s/v0/vault_kv.py

tests/unit/test_charm.py

Use schema defined in charm-relation-interface/vault-kv to validate schema. Rename property-like methods on Requirer object to getters.

gruyaume

:)

lib/charms/vault_k8s/v0/vault_kv.py

Simplify events, update documentation, and show how to use a secret to store the unit's nonce.

ghislainbourgeois

I'd like a small change, otherwise LGTM

lib/charms/vault_k8s/v0/vault_kv.py

src/charm.py

Loosen typing on remove_unit_credentials from Union[str, List[str]] to Union[str, Iterable[str]].

tests/unit/test_charm.py

Some tests were trying to do / assert many things outside the goal of the test, remove any superfluous testing.

gboutry requested a review from a team as a code owner August 21, 2023 13:17

gruyaume requested changes Aug 21, 2023

View reviewed changes

gruyaume requested a review from ghislainbourgeois August 21, 2023 15:08

gboutry marked this pull request as draft August 22, 2023 13:01

ghislainbourgeois requested changes Aug 24, 2023

View reviewed changes

lib/charms/vault_k8s/v0/vault_kv.py Outdated Show resolved Hide resolved

lib/charms/vault_k8s/v0/vault_kv.py Outdated Show resolved Hide resolved

lib/charms/vault_k8s/v0/vault_kv.py Outdated Show resolved Hide resolved

src/charm.py Outdated Show resolved Hide resolved

gboutry force-pushed the feat/secrets-backend branch from 72e5bb0 to 5d2f301 Compare August 25, 2023 08:34

gboutry commented Aug 25, 2023

View reviewed changes

lib/charms/vault_k8s/v0/vault_kv.py Show resolved Hide resolved

gboutry commented Aug 25, 2023

View reviewed changes

src/vault.py Show resolved Hide resolved

gboutry force-pushed the feat/secrets-backend branch from 1dd11d0 to c0c0c4b Compare September 15, 2023 16:16

gboutry force-pushed the feat/secrets-backend branch from c0c0c4b to 02f6a59 Compare September 15, 2023 16:17

gboutry changed the title ~~feat: add secrets backend relation~~ feat: add vault-kv relation Sep 15, 2023

gboutry force-pushed the feat/secrets-backend branch from 02f6a59 to 755a2c1 Compare September 20, 2023 13:51

gboutry added 3 commits September 20, 2023 15:52

[Feat] Implement vault-kv relation

3a4d31f

[Feat] Add ca_certificate to vault-kv relation

c564d67

gboutry force-pushed the feat/secrets-backend branch from 755a2c1 to 6ca9c1a Compare September 20, 2023 13:52

gboutry force-pushed the feat/secrets-backend branch from c1ca4e1 to ca40902 Compare September 21, 2023 13:22

gboutry marked this pull request as ready for review September 21, 2023 14:54

gruyaume requested changes Sep 21, 2023

View reviewed changes

src/charm.py Outdated Show resolved Hide resolved

src/charm.py Outdated Show resolved Hide resolved

src/charm.py Show resolved Hide resolved

src/vault.py Outdated Show resolved Hide resolved

gruyaume requested a review from ghislainbourgeois September 21, 2023 21:52

gboutry added 2 commits September 22, 2023 11:21

[Fix] Only enable approle if needed

2207c97

Only enable the approle auth method if at least one vault-kv relation was created.

[Fix] Improve logging and policy application

df0e267

Remove any modification to unit status from the kv handler, replaced by logging calls. Move KV policy to dedicated file.

gboutry force-pushed the feat/secrets-backend branch from af3b700 to df0e267 Compare September 22, 2023 09:21

[Fix] Set vault kv libid

87f9955

gruyaume requested changes Sep 25, 2023

View reviewed changes

[Fix] Implement schema validation with pydantic

7a4fc9d

Use schema defined in charm-relation-interface/vault-kv to validate schema. Rename property-like methods on Requirer object to getters.

gboutry force-pushed the feat/secrets-backend branch from a10cbfd to 7a4fc9d Compare September 25, 2023 16:26

[Fix] Update testing

33c6019

gboutry force-pushed the feat/secrets-backend branch from 7aa4342 to 33c6019 Compare September 26, 2023 07:23

gruyaume approved these changes Sep 27, 2023

View reviewed changes

lib/charms/vault_k8s/v0/vault_kv.py Outdated Show resolved Hide resolved

Update tests and docs, remove nonce from VaultKvRequires' init

eec4d3a

Simplify events, update documentation, and show how to use a secret to store the unit's nonce.

ghislainbourgeois requested changes Sep 27, 2023

View reviewed changes

lib/charms/vault_k8s/v0/vault_kv.py Outdated Show resolved Hide resolved

src/charm.py Outdated Show resolved Hide resolved

gboutry added 4 commits September 28, 2023 09:33

Loosen typing on remove_unit_credentials

452a3c1

Loosen typing on remove_unit_credentials from Union[str, List[str]] to Union[str, Iterable[str]].

Merge branch 'main' into feat/secrets-backend

a2fea0c

[Fix] Move kv_mount to templates

2a8ce12

[Docs] Remove type ignore comment in docs example

38cea6f

gboutry force-pushed the feat/secrets-backend branch from e9a751c to 38cea6f Compare September 28, 2023 08:54

gruyaume requested changes Sep 28, 2023

View reviewed changes

tests/unit/test_charm.py Show resolved Hide resolved

tests/unit/test_charm.py Outdated Show resolved Hide resolved

gruyaume requested changes Sep 28, 2023

View reviewed changes

tests/unit/test_charm.py Outdated Show resolved Hide resolved

[Tests] Reduce tests content

a4157da

Some tests were trying to do / assert many things outside the goal of the test, remove any superfluous testing.

gruyaume approved these changes Sep 28, 2023

View reviewed changes

gruyaume requested a review from ghislainbourgeois September 28, 2023 12:39

ghislainbourgeois approved these changes Sep 28, 2023

View reviewed changes

ghislainbourgeois merged commit dd47cec into canonical:main Sep 28, 2023

gboutry deleted the feat/secrets-backend branch September 28, 2023 13:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add vault-kv relation #30

feat: add vault-kv relation #30

gboutry commented Aug 21, 2023 •

edited

Loading

gruyaume left a comment

gboutry commented Aug 22, 2023

gboutry commented Aug 25, 2023

gboutry commented Sep 15, 2023

gboutry commented Sep 20, 2023

gruyaume left a comment

gruyaume left a comment

ghislainbourgeois left a comment

feat: add vault-kv relation #30

feat: add vault-kv relation #30

Conversation

gboutry commented Aug 21, 2023 • edited Loading

Description

Checklist:

gruyaume left a comment

Choose a reason for hiding this comment

gboutry commented Aug 22, 2023

gboutry commented Aug 25, 2023

gboutry commented Sep 15, 2023

gboutry commented Sep 20, 2023

gruyaume left a comment

Choose a reason for hiding this comment

gruyaume left a comment

Choose a reason for hiding this comment

ghislainbourgeois left a comment

Choose a reason for hiding this comment

gboutry commented Aug 21, 2023 •

edited

Loading