Updates self-hosted db discover to use 2190h ttl for certificate#47081
Merged
stevenGravy merged 5 commits intomasterfrom Oct 3, 2024
Merged
Updates self-hosted db discover to use 2190h ttl for certificate#47081stevenGravy merged 5 commits intomasterfrom
stevenGravy merged 5 commits intomasterfrom
Conversation
stevenGravy
changed the title
| <Text mt={1}> | ||
| Restart the database server to apply the configuration. | ||
| Restart the database server to apply the configuration. The | ||
| certificate is by default 90 days so this will require installing an |
Contributor
There was a problem hiding this comment.
Suggested change
| certificate is by default 90 days so this will require installing an | |
| certificate is valid for 90 days so this will require installing an |
I guess the next question is how can they generate a new one? Should we add the tctl sign ... command here? Or just link to docs?
Contributor
Author
There was a problem hiding this comment.
I think docs as there are things like is the user set up to impersonate right. I'll add that link
GavinFrazar
reviewed
Oct 2, 2024
Co-authored-by: Gavin Frazar <gavin.frazar@goteleport.com>
kimlisa
approved these changes
Oct 3, 2024
Contributor
There was a problem hiding this comment.
hrm, i don't think we should define the database TTL here, but instead define it as a const in the file it is used (see other comment)
i don't see it being used in other places atm
Comment on lines
+116
to
+117
| const ttl = cfg.getDatabaseCertificateTTL(); | ||
| const requestData = JSON.stringify({ hostname, ttl }); |
Contributor
There was a problem hiding this comment.
at the top of this file, i would define
// the length of the certificate to request for the database
const DEFAULT_DATABASE_TTL = '2190h'then
Suggested change
| const ttl = cfg.getDatabaseCertificateTTL(); | |
| const requestData = JSON.stringify({ hostname, ttl }); | |
| const requestData = JSON.stringify({ hostname, ttl: DEFAULT_DATABASE_TTL }); |
Contributor
There was a problem hiding this comment.
took liberty to refactor this page a bit
GavinFrazar
approved these changes
Oct 3, 2024
public-teleport-github-review-bot
Bot
removed request for
klizhentas,
r0mant and
rosstimothy
|
@stevenGravy See the table below for backport results.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #47069
changelog: Updates self-hosted db discover flow to generate 2190h TTL certs, not 12h