[NET-8091] Add file-system-certificate config entry for API gateway#20873
[NET-8091] Add file-system-certificate config entry for API gateway#20873missylbytes merged 21 commits intomainfrom
Conversation
This validation makes sense for inline certificates since Consul server is holding the certificate; however, for file system certificates, Consul server never actually sees the certificate.
08744bb to
2a44afa
Compare
Add new doc to nav
95be27a to
76483f1
Compare
| case structs.InlineCertificate: | ||
| if cert, ok := cfgSnap.APIGateway.Certificates.Get(certRef); ok { | ||
| certs = append(certs, cert) | ||
| } |
There was a problem hiding this comment.
Did you intentionally omit this check in both cases?
if !ok {
continue
}
There was a problem hiding this comment.
Yes because I think this would only be a programmer error, not something that people should run into.
There was a problem hiding this comment.
they're functionally the same unless I'm missing something.
if ok {
// do something
}vs.
if !ok {
continue
}
// do somethingThere was a problem hiding this comment.
I guess it just depends on the behaviour we want
if ok {
// do something
}
using the above code, we successfully retrieve the certificate and move on to the next thing.
if !ok {
continue
}
// do something
This code here indicates that what we'll like to do if the cert isn't found, we can skip the rest of the iteration and move onto the next thing. Just depends on how we want to handle it. I saw that we used if !ok { continue} in the previous implementation and was wondering why we didn't here.
There was a problem hiding this comment.
ohhhh I see what you're saying. I just went with the current flavor because it simplified the code and had the same outcome, no more reasoning that
website/content/docs/connect/config-entries/file-system-certificate.mdx
Outdated
Show resolved
Hide resolved
website/content/docs/connect/gateways/api-gateway/configuration/index.mdx
Show resolved
Hide resolved
NiniOak
left a comment
There was a problem hiding this comment.
Small error check comment but mostly looks good. The work done here is MASSIVE and IMPRESSIVE. Very well done 🚀
…20873) * Define file-system-certificate config entry * Collect file-system-certificate(s) referenced by api-gateway onto snapshot * Add file-system-certificate to config entry kind allow lists * Remove inapplicable validation This validation makes sense for inline certificates since Consul server is holding the certificate; however, for file system certificates, Consul server never actually sees the certificate. * Support file-system-certificate as source for listener TLS certificate * Add more required mappings for the new config entry type * Construct proper TLS context based on certificate kind * Add support or SDS in xdscommon * Remove unused param * Adds back verification of certs for inline-certificates * Undo tangential changes to TLS config consumption * Remove stray curly braces * Undo some more tangential changes * Improve function name for generating API gateway secrets * Add changelog entry * Update .changelog/20873.txt Co-authored-by: Jared Kirschner <85913323+jkirschner-hashicorp@users.noreply.github.com> * Add some nil-checking, remove outdated TODO * Update test assertions to include file-system-certificate * Add documentation for file-system-certificate config entry Add new doc to nav * Fix grammar mistake * Rename watchmaps, remove outdated TODO --------- Co-authored-by: Melisa Griffin <melisa.griffin@hashicorp.com> Co-authored-by: Jared Kirschner <85913323+jkirschner-hashicorp@users.noreply.github.com>
|
This will not be back-ported at this time. |
* Initial rebrand for HCP Terraform * Apply suggestions from code review Co-authored-by: Rose M Koron <32436232+rkoron007@users.noreply.github.com> * path fix and redirect * reintroduce nav from #20873 and #20994 --------- Co-authored-by: Rose M Koron <32436232+rkoron007@users.noreply.github.com> Co-authored-by: Krastin Krastev <krastin@hashicorp.com>
…20873) * Define file-system-certificate config entry * Collect file-system-certificate(s) referenced by api-gateway onto snapshot * Add file-system-certificate to config entry kind allow lists * Remove inapplicable validation This validation makes sense for inline certificates since Consul server is holding the certificate; however, for file system certificates, Consul server never actually sees the certificate. * Support file-system-certificate as source for listener TLS certificate * Add more required mappings for the new config entry type * Construct proper TLS context based on certificate kind * Add support or SDS in xdscommon * Remove unused param * Adds back verification of certs for inline-certificates * Undo tangential changes to TLS config consumption * Remove stray curly braces * Undo some more tangential changes * Improve function name for generating API gateway secrets * Add changelog entry * Update .changelog/20873.txt Co-authored-by: Jared Kirschner <85913323+jkirschner-hashicorp@users.noreply.github.com> * Add some nil-checking, remove outdated TODO * Update test assertions to include file-system-certificate * Add documentation for file-system-certificate config entry Add new doc to nav * Fix grammar mistake * Rename watchmaps, remove outdated TODO --------- Co-authored-by: Melisa Griffin <melisa.griffin@hashicorp.com> Co-authored-by: Jared Kirschner <85913323+jkirschner-hashicorp@users.noreply.github.com> # Conflicts: # agent/consul/usagemetrics/usagemetrics_test.go # agent/structs/structs.deepcopy.go # proto/private/pbconfigentry/config_entry.pb.go
Description
Today,
inline-certificateis the only supported way of specifying a TLS certificate + private key for an api-gateway. As the name suggests, the certificate + private key values are "inlined" in the configuration entry.This PR adds a new
file-system-certificateconfiguration entry that instead accepts file paths to a certificate + private key that are accessible in the local file system of the api-gateway at runtime. This functions in the same way that terminating-gateway does today (docs) but adds support for rotation by watching the file system for changes to the referenced certificate + private key.Ideally, we will be able to add this same support for rotation in terminating-gateway in the future as rotating the certificate and private key today require a restart of the terminating-gateway, which is not ideal (docs).
Testing & Reproduction steps
Testing on VMs
Check out this repo from @nathancoleman
dynamic_active_secrets.echo secret_val | base64 -dand you should see the value of cert.crtReplace the value of
cert.crtwith this certificate:dynamic_active_secretsagain, it should match the above cert.Testing on K8S
You cannot do the same type of replacement for K8S, so if you would like to test K8S check out the K8S PR. You will mainly verify that the certificate is a
file-system-certificateas opposed to aninline-certificate.Links
PR Checklist