Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLS fixes #245

Merged
merged 4 commits into from
Sep 19, 2023
Merged

TLS fixes #245

merged 4 commits into from
Sep 19, 2023

Conversation

sed-i
Copy link
Contributor

@sed-i sed-i commented Sep 15, 2023

Issue

Curling prometheus via ingress url results in Internal Server Error.

Solution

  1. Drop incorrect rootCAs section for e2e TLS.
  2. On upgrade, make sure to push certs received via the "recv-ca-certs" relation.

Context

On upgrade, no relation events are emitted (unless you modify relation data as part of the upgrade sequence). So if, for example, you get certificates over relation data, they would all be lost after an upgrade. And you do not want persistent storage because then you would need to figure out which ones you need to delete after an upgrade.

So on upgrade you’d need to iterate over all relations and write those certs to disk. But not quite on upgrade-charm. We need the container to be there, so this must happen of pebble-ready.

Testing Instructions

Follow testing instructions in canonical/cos-lite-bundle#80 and curl prometheus.

Until we have cert_transfer impl'd in prom and grafana, copy traefik's ca cert manually:

juju run external-ca/0 get-ca-certificate --format json | jq -r '."external-ca/0".results."ca-certificate"' > external-ca.crt
openssl x509 -noout -text -in external-ca.crt

echo | openssl s_client -showcerts -servername 10.43.8.206 -connect 10.43.8.206:443 | openssl x509 -text -noout

juju scp --container prometheus external-ca.crt prometheus/0:/usr/local/share/ca-certificates
juju ssh --container prometheus prometheus/0 update-ca-certificates --fresh

juju scp --container grafana external-ca.crt grafana/0:/usr/local/share/ca-certificates
juju ssh --container grafana grafana/0 update-ca-certificates --fresh

juju ssh --container grafana grafana/0 curl https://10.43.8.206/trfk-prometheus-0/api/v1/targets | jq | grep -E "scrapeUrl|health"

Release Notes

Fix cert handling.

@PietroPasotti
Copy link
Contributor

datasources are still broken in grafana after refreshing traefik to use this source
image

@sed-i
Copy link
Contributor Author

sed-i commented Sep 16, 2023

datasources are still broken in grafana after refreshing traefik to use this source image

I think that's because prometheus is announcing its ingress address to grafana. The problem is that grafana cannot validate traefik's cert because it doesn't have the external-ca cert.

  • I think if you relate traefik to the local ca it would work (but would be wrong).
  • Until Add cross_model property to model.Relation operator#970 is resolved, we could use only fqdns for grafana source urls (this would break datasources over CMRs), or add a "receive-ca-cert" to grafana and relate it to the external-ca 🤮.

@sed-i
Copy link
Contributor Author

sed-i commented Sep 19, 2023

I tested locally with external_hostname for traefik and it works.
After some QA by the team I believe this PR could be merged even if bare IP does not work yet.

@sed-i sed-i marked this pull request as ready for review September 19, 2023 04:56
@sed-i sed-i merged commit 4086e3b into main Sep 19, 2023
@sed-i sed-i deleted the feature/fix-tls branch September 19, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants