Add server certificate renewal for Edge Hub #412
Conversation
|
I'm not sure how to write tests for this? |
| { | ||
| logger.LogInformation("Scheduling server certificate renewal for {0}.", DateTime.UtcNow.Add(timeToExpire).ToString("o")); | ||
| this.cts = new CancellationTokenSource(timeToExpire); | ||
| this.cts.Token.Register(l => ((ILogger)l).LogInformation("Performing server certificate renewal."), logger); |
There was a problem hiding this comment.
Nit: We should probably also add "restarting edgeHub.." to the log..
There was a problem hiding this comment.
+1. Perhaps "Restarting Edge for certificate renewal..
There was a problem hiding this comment.
I think I can add a log in main to indicate we are shutting down. However, most of the logs are in ShutdownHandler and guarded by a semaphore. Do you know the history here?
There was a problem hiding this comment.
I added a couple logs to main?
| else | ||
| { | ||
| this.cts = new CancellationTokenSource(); | ||
| logger.LogWarning("Time to server certificate expiration is {0}. Not scheduling renewal.", timeToExpire.ToString("c")); |
There was a problem hiding this comment.
Should we instead say "Certificate is expired".. might be better?
Also, in this case we don't do anything?
There was a problem hiding this comment.
Yeah, I'm worried about it getting in a crash loop?
There was a problem hiding this comment.
Also if just started and it gets an expired certificate there isn't much a restart will accomplish.
There was a problem hiding this comment.
Hmm, I see.. I was thinking purely form this class's standpoint..
About the crash loop, isn't that what will happen anyways? Will the servers be able to start properly?
| { | ||
| if (disposing) | ||
| { | ||
| this.cts.Dispose(); |
There was a problem hiding this comment.
Does this cause the ugly "A task was cancelled" exception?
There was a problem hiding this comment.
Do you think this handles it?
| { | ||
| logger.LogInformation("Scheduling server certificate renewal for {0}.", DateTime.UtcNow.Add(timeToExpire).ToString("o")); | ||
| this.cts = new CancellationTokenSource(timeToExpire); | ||
| this.cts.Token.Register(l => ((ILogger)l).LogInformation("Performing server certificate renewal."), logger); |
There was a problem hiding this comment.
+1. Perhaps "Restarting Edge for certificate renewal..
| Preconditions.CheckNotNull(logger, nameof(logger)); | ||
|
|
||
| TimeSpan timeToExpire = certificates.ServerCertificate.NotAfter - DateTime.UtcNow; | ||
| if (timeToExpire > TimeSpan.Zero) |
There was a problem hiding this comment.
I'd prefer if we leave a small buffer here of a few mins before expiration instead of waiting till it is expired. This is so that clients log any TLS validation errors and so on.
Adds server cert renewal for the Edge Hub.
Triggers a restart of the Edge Hub when expiration of the cert is about to occur.