[DPE-2677] Avoid setting secret upon tls relation broken if using juju secrets

[shayancanonical]

Issue

We do not have access to a juju secret upon relation-broken event. However, the tls relation code in MySQL is setting tls related secrets to None. This is necessary for secrets stored in the databag (as they are reset to None), but causes the trace in the Jira ticket as the secret is deleted from the secrets backend

Solution

Do not attempt to reset the tls secrets upon relation broken if using juju secrets

[codecov]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (f8f49dd) 65.43% compared to head (870ef74) 65.90%.
Report is 2 commits behind head on main.

Files	Patch %	Lines
lib/charms/mysql/v0/mysql.py	86.79%	2 Missing and 5 partials ⚠️
lib/charms/mysql/v0/tls.py	33.33%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #360      +/-   ##
==========================================
+ Coverage   65.43%   65.90%   +0.47%     
==========================================
  Files          17       17              
  Lines        3127     3121       -6     
  Branches      415      411       -4     
==========================================
+ Hits         2046     2057      +11     
+ Misses        950      936      -14     
+ Partials      131      128       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[carlcsaposs-canonical]

We do not have access to a juju secret upon relation-broken event.

wait what? is this intended behavior by juju?

also, should we be cleaning up the secret on relation broken?

[paulomach]

We do not have access to a juju secret upon relation-broken event.

wait what? is this intended behavior by juju?

it make sense to do so for relation secrets, could not find documentation for that. @tmihoc here's a shameless plug for you!

also, should we be cleaning up the secret on relation broken?

Do you see any reason to keep it?

imo we should go fwd with the solution, and refactor if needed.

[tmihoc]

@paulomach

We do not have access to a juju secret upon relation-broken event.

wait what? is this intended behavior by juju?

it make sense to do so for relation secrets, could not find documentation for that. @tmihoc here's a shameless plug for you!

Here's the documentation we have on secret events: https://juju.is/docs/sdk/secret-events . What do you think we should add to it to make it clearer for what you need here?

[paulomach]

Given the behavior, that's an appropriated solution

[paulomach]

Here's the documentation we have on secret events: https://juju.is/docs/sdk/secret-events . What do you think we should add to it to make it clearer for what you need here?

Hi @tmihoc , basically the secret availability under a relation departure. See convo

[carlcsaposs-canonical]

I'm still quite confused—do we own this secret? if so, why are we losing access to it during relation broken?

[carlcsaposs-canonical]

https://chat.charmhub.io/charmhub/pl/cdejbq4yotbmfmiifninijm9so

[paulomach]

So, from the chat, since the charm owns the secret, it should not lose access? That does not seem to be what was observed

[carlcsaposs-canonical]

So, from the chat, since the charm owns the secret, it should not lose access?

yes, that's my understanding

was the unit/app also dying?

[taurus-forever]

@paulomach please merge to include the next stable. Tnx!

[carlcsaposs-canonical]

checking behavior with juju team here: https://chat.charmhub.io/charmhub/pl/8u7d138khpdypmbxfw4uetxjbo

if the behavior is expected, I think this PR is probably a simpler workaround than checking if the unit is dying

@paulomach will the changes in this PR be okay if the relation with tls is removed and re-added? (the secret won't be cleaned up) if so, PR lgtm

[carlcsaposs-canonical]

for history, @taurus-forever tried removing the relation instead of removing the unit and this error did not occur—seems to indicate this error is from setting the secret while the unit is dying (relation breaking not relevant)

[paulomach]

checking behavior with juju team here: https://chat.charmhub.io/charmhub/pl/8u7d138khpdypmbxfw4uetxjbo

if the behavior is expected, I think this PR is probably a simpler workaround than checking if the unit is dying

@paulomach will the changes in this PR be okay if the relation with tls is removed and re-added? (the secret won't be cleaned up) if so, PR lgtm

It should, as the secret will always be overwritten (or written for 2.9) on relation re-added, with a new value.
The tls usage in the workload will be disabled when the relation is gone and re-enabled after receiving the new certificates.