Skip to content

feat(key-server): use TLS certificates with nginx#21243

Merged
sfoster1 merged 8 commits intoedgefrom
exec-2475-nginx-integration
Apr 13, 2026
Merged

feat(key-server): use TLS certificates with nginx#21243
sfoster1 merged 8 commits intoedgefrom
exec-2475-nginx-integration

Conversation

@sfoster1
Copy link
Copy Markdown
Member

@sfoster1 sfoster1 commented Apr 9, 2026

Overview

The point of having TLS certificates is to give them to nginx so that it can serve HTTPS traffic. This PR enables that!

We now actually use our fancy TLS EE and CA managers in the application lifecycle, so certs are generated and rotated automatically as the system comes up. This needed a little bit of config fettling; that's now a dependency and gets parsed in the lifecycle as well.

The other fun bit is that nginx needs to be explicitly told when a cert changes out from under it, because nginx aggressively caches them. We do this with systemd, but of course we don't have systemd in test or dev, so we have a new setting to just not do that.

There's a lot more to this, to do with nginx configuration, but that's over in oe-core: Opentrons/oe-core#302

Test Plan and Hands on Testing

This is the first thing that can be reasonably tested! You'll need an OE core build to do it, for reasons that become apparent in the OE core pr, but once you have that you can do the following:

  • fetch the CA cert from the robot; it should be in /var/lib/opentrons-key-server/tls/ot-robot-tls-ca-SOMEDATE.cer. note that this is not in the secure volume; it is a public key and is public data. scp this cert to your computer
  • Add that CA to your browser's trust store, in the way that seems best to you
  • Navigate to https://robotip:32313/health and note that the browser (a) displays a lock and (b) displays our json error text: {"message":"header.opentrons-version: Field required","errorCode":"4000"}
  • Note that the above may not reaaaly work because we don't have the robot IPs baked into the certs yet; but you should definitely get it doing HTTPS traffic even if it's not trusted, and the system should trace the end entity cert back to the CA cert you installed.
  • If you can tell me why that's the port I picked you will feel very clever

Review requests

Not that much code change over here, though there is in OE-core; still, take a look especially about the config dependency.

Risk assessment

Low, not used yet.

Closes EXEC-2475

sfoster1 added 8 commits April 9, 2026 14:59
We need to manage the CA and TLS certificates with the lifecycle as the
app as a whole, so make a FastAPI dependency and use it in the app
lifecycle manager.
There are some things in the config that had defaults, and if multiple
things depend on the defaults then who's supposed to set them? Now, the
config has its own little dependency store that sets up things like base
temporary directories when the server starts, so both the secure volume
manager and the tls manager can rely on setting up tempdirs in a safe
and sane way.
This will be useful for reloading nginx
When set, this will talk to nginx. We need this because we definitely
will not have either nginx or systemd in dev and test setups.
Nginx will very aggressively cache TLS certs, so when we rotate certs we
need to let it know to reload them. This is done by sending SIGHUP to
the main process; this would need the pid, which is annoying, but
luckily systemd has systemctl reload which just does that. Hooray!
Copy link
Copy Markdown
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooray!

Copy link
Copy Markdown
Contributor

@TamarZanzouri TamarZanzouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! cool!

@sfoster1 sfoster1 merged commit 82b719e into edge Apr 13, 2026
6 checks passed
@sfoster1 sfoster1 deleted the exec-2475-nginx-integration branch April 13, 2026 15:13
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.

3 participants