-
Notifications
You must be signed in to change notification settings - Fork 40
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
[nexus] Add HTTPS support, plumbing x509 certificates #1500
Changes from 15 commits
1f843f1
920ad13
494a6f1
3ede000
0104f49
1cf097a
b58e10a
2926fe0
edea7b9
8137419
eb1ea85
95c10a0
6502c44
0f0afbb
31a93ea
cf4864b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,13 @@ This script requires Omicron be uninstalled, e.g., with `pfexec | |
that is not the case. The script will then remove the file-based vdevs and the | ||
VNICs created by `create_virtual_hardware.sh`. | ||
|
||
=== Make me a certificate! | ||
|
||
Nexus's external interface will typically be served using public-facing x.509 | ||
certificate. While we are still configuring the mechanism to integrate this real | ||
certificate into the package system, `./tools/create_self_signed_cert.sh` can be | ||
used to generate an equivalent self-signed certificate. | ||
Comment on lines
+58
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO(me): I wonder if I can fix #1398 and make this easier at the same time? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See: #1528. We're gonna need to grab / insert certs from somewhere. I've updating the packaging hints to make this more obvious if an error is encountered. |
||
|
||
== Deploying Omicron | ||
|
||
The control plane repository contains a packaging tool which bundles binaries | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ zone = true | |
setup_hint = """ | ||
- Run `./tools/ci_download_console` to download the web console assets | ||
- Run `pkg install library/postgresql-13` to download Postgres libraries | ||
- Run `./tools/create_self_signed_cert.sh` to generate a certificate | ||
""" | ||
|
||
[[package.omicron-nexus.paths]] | ||
|
@@ -30,6 +31,14 @@ to = "/var/svc/manifest/site/nexus" | |
[[package.omicron-nexus.paths]] | ||
from = "out/console-assets" | ||
to = "/var/nexus/static" | ||
# Note, we could just map the whole "out/certs" directory, but this ensures | ||
# both files exist. | ||
[[package.omicron-nexus.paths]] | ||
from = "out/certs/cert.pem" | ||
to = "/var/nexus/certs/cert.pem" | ||
[[package.omicron-nexus.paths]] | ||
from = "out/certs/key.pem" | ||
to = "/var/nexus/certs/key.pem" | ||
Comment on lines
+34
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be a longer-term issue to resolve, but I'd like for this to not necessarily be bundled with the package. However, this may require a different mechanism for acquiring / rotation of the certificate. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I followed up today and met with Rick to discuss this. My takeaways:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is tracked by #1528 |
||
|
||
[package.oximeter-collector] | ||
rust.binary_names = ["oximeter"] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an analogous change for
how-to-run-simulated
? In general, the test suite matches whathow-to-run-simulated
does so if you've updated that then things should work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is not, because the implementation within Nexus checks for the existence of the certificate files before deciding whether or not to launch the HTTPS server.
For the simulated server, they won't exist, so only the HTTP server will be launched.
I definitely think this would be required before moving to "HTTPS only", but considered this PR an intermediate step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mentioned in the other comment, but that doesn't seem like the right behavior. It seems like in production, if they don't exist, that should be a fatal error. And in development, if they do exist, we should start the HTTP server. If this were part of configuration, then whoever's running Nexus can express their intent. (I'm not saying we have to do all that in this PR, but if we decide that is what we want, then I think this behavior is not really an intermediate step.)