-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Watch file-based certificates for changes #49979
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
0f6d97d
Watch file-based certificates for changes
amcasey 428df83
Annotate CertificateConfig with MemberNotNullWhenAttribute
amcasey eada2be
Index log messages from 1
amcasey 02f3e09
Make log message placeholders PascalCase
amcasey 5cf296d
Append 'Unsynchronized' to test hook names
amcasey 7bc7dc2
Add explanatory comment
amcasey 8350cb3
Fix tests now that structured logging property names are PascalCase
amcasey a50c2a7
Add a test at the KestrelConfigurationLoader level
amcasey fdb7c6d
Try a different way of updating a file's last write time to address C…
amcasey 0b4e8f4
Speculatively use deterministically distinct names in flaky tests
amcasey 8551da1
Make more aggressive changes to files
amcasey 6cee51a
Format log messages consistently
amcasey 59702e4
Remove redundant @s
amcasey 87eca27
Add an abstraction layer between CertificatePathWatcher and the file …
amcasey e6ccd62
Clarify use of delays in KestrelConfigurationLoaderTests.CertificateC…
amcasey c731b28
Address PR feedback
amcasey ca372a7
Preemptively add CertificateChangedOnDisk to the helix retry list
amcasey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
328 changes: 328 additions & 0 deletions
328
src/Servers/Kestrel/Core/src/Internal/CertificatePathWatcher.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,328 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Diagnostics; | ||
| using Microsoft.Extensions.Configuration; | ||
| using Microsoft.Extensions.FileProviders; | ||
| using Microsoft.Extensions.FileProviders.Physical; | ||
| using Microsoft.Extensions.Hosting; | ||
| using Microsoft.Extensions.Logging; | ||
| using Microsoft.Extensions.Primitives; | ||
|
|
||
| namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal; | ||
|
|
||
| internal sealed partial class CertificatePathWatcher : IDisposable | ||
| { | ||
| private readonly Func<string, IFileProvider?> _fileProviderFactory; | ||
| private readonly string _contentRootDir; | ||
| private readonly ILogger<CertificatePathWatcher> _logger; | ||
|
|
||
| private readonly object _metadataLock = new(); | ||
|
|
||
| /// <remarks>Acquire <see cref="_metadataLock"/> before accessing.</remarks> | ||
| private readonly Dictionary<string, DirectoryWatchMetadata> _metadataForDirectory = new(); | ||
| /// <remarks>Acquire <see cref="_metadataLock"/> before accessing.</remarks> | ||
| private readonly Dictionary<string, FileWatchMetadata> _metadataForFile = new(); | ||
|
|
||
| private ConfigurationReloadToken _reloadToken = new(); | ||
| private bool _disposed; | ||
|
|
||
| public CertificatePathWatcher(IHostEnvironment hostEnvironment, ILogger<CertificatePathWatcher> logger) | ||
| : this( | ||
| hostEnvironment.ContentRootPath, | ||
| logger, | ||
| dir => Directory.Exists(dir) ? new PhysicalFileProvider(dir, ExclusionFilters.None) : null) | ||
| { | ||
| } | ||
|
|
||
| /// <remarks> | ||
| /// For testing. | ||
| /// </remarks> | ||
| internal CertificatePathWatcher(string contentRootPath, ILogger<CertificatePathWatcher> logger, Func<string, IFileProvider?> fileProviderFactory) | ||
| { | ||
| _contentRootDir = contentRootPath; | ||
| _logger = logger; | ||
| _fileProviderFactory = fileProviderFactory; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Returns a token that will fire when any watched <see cref="CertificateConfig"/> is changed on disk. | ||
| /// The affected <see cref="CertificateConfig"/> will have <see cref="CertificateConfig.FileHasChanged"/> | ||
| /// set to <code>true</code>. | ||
| /// </summary> | ||
| public IChangeToken GetChangeToken() | ||
| { | ||
| return _reloadToken; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Update the set of <see cref="CertificateConfig"/>s being watched for file changes. | ||
| /// If a given <see cref="CertificateConfig"/> appears in both lists, it is first removed and then added. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Does not consider targets when watching files that are symlinks. | ||
| /// </remarks> | ||
| public void UpdateWatches(List<CertificateConfig> certificateConfigsToRemove, List<CertificateConfig> certificateConfigsToAdd) | ||
| { | ||
| var addSet = new HashSet<CertificateConfig>(certificateConfigsToAdd, ReferenceEqualityComparer.Instance); | ||
| var removeSet = new HashSet<CertificateConfig>(certificateConfigsToRemove, ReferenceEqualityComparer.Instance); | ||
|
|
||
| // Don't remove anything we're going to re-add anyway. | ||
| // Don't remove such items from addSet to guard against the (hypothetical) possibility | ||
| // that a caller might remove a config that isn't already present. | ||
| removeSet.ExceptWith(certificateConfigsToAdd); | ||
|
|
||
| if (addSet.Count == 0 && removeSet.Count == 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| lock (_metadataLock) | ||
| { | ||
| // Adds before removes to increase the chances of watcher reuse. | ||
| // Since removeSet doesn't contain any of these configs, this won't change the semantics. | ||
| foreach (var certificateConfig in addSet) | ||
| { | ||
| AddWatchUnsynchronized(certificateConfig); | ||
| } | ||
|
|
||
| foreach (var certificateConfig in removeSet) | ||
| { | ||
| RemoveWatchUnsynchronized(certificateConfig); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Start watching a certificate's file path for changes. | ||
| /// <paramref name="certificateConfig"/> must have <see cref="CertificateConfig.IsFileCert"/> set to <code>true</code>. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Internal for testing. | ||
| /// </remarks> | ||
| internal void AddWatchUnsynchronized(CertificateConfig certificateConfig) | ||
| { | ||
| Debug.Assert(certificateConfig.IsFileCert, "AddWatch called on non-file cert"); | ||
|
|
||
| var path = Path.Combine(_contentRootDir, certificateConfig.Path); | ||
| var dir = Path.GetDirectoryName(path)!; | ||
|
|
||
| if (!_metadataForDirectory.TryGetValue(dir, out var dirMetadata)) | ||
| { | ||
| // If we wanted to detected deletions of this whole directory (which we don't since we ignore deletions), | ||
| // we'd probably need to watch the whole directory hierarchy | ||
|
|
||
| var fileProvider = _fileProviderFactory(dir); | ||
| if (fileProvider is null) | ||
| { | ||
| _logger.DirectoryDoesNotExist(dir, path); | ||
| return; | ||
| } | ||
|
|
||
| dirMetadata = new DirectoryWatchMetadata(fileProvider); | ||
| _metadataForDirectory.Add(dir, dirMetadata); | ||
|
|
||
| _logger.CreatedDirectoryWatcher(dir); | ||
| } | ||
|
|
||
| if (!_metadataForFile.TryGetValue(path, out var fileMetadata)) | ||
| { | ||
| // PhysicalFileProvider appears to be able to tolerate non-existent files, as long as the directory exists | ||
|
|
||
| var disposable = ChangeToken.OnChange( | ||
| () => dirMetadata.FileProvider.Watch(Path.GetFileName(path)), | ||
| static tuple => tuple.Item1.OnChange(tuple.Item2), | ||
| ValueTuple.Create(this, path)); | ||
|
|
||
| fileMetadata = new FileWatchMetadata(disposable); | ||
| _metadataForFile.Add(path, fileMetadata); | ||
| dirMetadata.FileWatchCount++; | ||
|
|
||
| // We actually don't care if the file doesn't exist - we'll watch in case it is created | ||
| fileMetadata.LastModifiedTime = GetLastModifiedTimeOrMinimum(path, dirMetadata.FileProvider); | ||
|
|
||
| _logger.CreatedFileWatcher(path); | ||
| } | ||
|
|
||
| if (!fileMetadata.Configs.Add(certificateConfig)) | ||
| { | ||
| _logger.ReusedObserver(path); | ||
| return; | ||
| } | ||
|
|
||
| _logger.AddedObserver(path); | ||
|
|
||
| _logger.ObserverCount(path, fileMetadata.Configs.Count); | ||
| _logger.FileCount(dir, dirMetadata.FileWatchCount); | ||
| } | ||
|
|
||
| private DateTimeOffset GetLastModifiedTimeOrMinimum(string path, IFileProvider fileProvider) | ||
| { | ||
| try | ||
| { | ||
| return fileProvider.GetFileInfo(Path.GetFileName(path)).LastModified; | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| _logger.LastModifiedTimeError(path, e); | ||
| } | ||
|
|
||
| return DateTimeOffset.MinValue; | ||
| } | ||
|
|
||
| private void OnChange(string path) | ||
| { | ||
| // Block until any in-progress updates are complete | ||
| lock (_metadataLock) | ||
| { | ||
| if (!_metadataForFile.TryGetValue(path, out var fileMetadata)) | ||
| { | ||
| _logger.UntrackedFileEvent(path); | ||
| return; | ||
| } | ||
|
|
||
| // Existence implied by the fact that we're tracking the file | ||
| var dirMetadata = _metadataForDirectory[Path.GetDirectoryName(path)!]; | ||
|
|
||
| // We ignore file changes that don't advance the last modified time. | ||
| // For example, if we lose access to the network share the file is | ||
| // stored on, we don't notify our listeners because no one wants | ||
| // their endpoint/server to shutdown when that happens. | ||
| // We also anticipate that a cert file might be renamed to cert.bak | ||
| // before a new cert is introduced with the old name. | ||
| // This also helps us in scenarios where the underlying file system | ||
| // reports more than one change for a single logical operation. | ||
| var lastModifiedTime = GetLastModifiedTimeOrMinimum(path, dirMetadata.FileProvider); | ||
| if (lastModifiedTime > fileMetadata.LastModifiedTime) | ||
| { | ||
| fileMetadata.LastModifiedTime = lastModifiedTime; | ||
| } | ||
| else | ||
| { | ||
| if (lastModifiedTime == DateTimeOffset.MinValue) | ||
| { | ||
| _logger.EventWithoutLastModifiedTime(path); | ||
| } | ||
| else if (lastModifiedTime == fileMetadata.LastModifiedTime) | ||
| { | ||
| _logger.RedundantEvent(path); | ||
| } | ||
| else | ||
| { | ||
| _logger.OutOfOrderEvent(path); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| var configs = fileMetadata.Configs; | ||
| foreach (var config in configs) | ||
| { | ||
| config.FileHasChanged = true; | ||
| } | ||
| } | ||
|
|
||
| // AddWatch and RemoveWatch don't affect the token, so this doesn't need to be under the semaphore. | ||
| // It does however need to be synchronized, since there could be multiple concurrent events. | ||
| var previousToken = Interlocked.Exchange(ref _reloadToken, new()); | ||
| previousToken.OnReload(); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Stop watching a certificate's file path for changes (previously started by <see cref="AddWatchUnsynchronized"/>. | ||
| /// <paramref name="certificateConfig"/> must have <see cref="CertificateConfig.IsFileCert"/> set to <code>true</code>. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Internal for testing. | ||
| /// </remarks> | ||
| internal void RemoveWatchUnsynchronized(CertificateConfig certificateConfig) | ||
| { | ||
| Debug.Assert(certificateConfig.IsFileCert, "RemoveWatch called on non-file cert"); | ||
|
|
||
| var path = Path.Combine(_contentRootDir, certificateConfig.Path); | ||
| var dir = Path.GetDirectoryName(path)!; | ||
|
|
||
| if (!_metadataForFile.TryGetValue(path, out var fileMetadata)) | ||
| { | ||
| _logger.UnknownFile(path); | ||
| return; | ||
| } | ||
|
|
||
| var configs = fileMetadata.Configs; | ||
|
|
||
| if (!configs.Remove(certificateConfig)) | ||
| { | ||
| _logger.UnknownObserver(path); | ||
| return; | ||
| } | ||
|
|
||
| _logger.RemovedObserver(path); | ||
|
|
||
| // If we found fileMetadata, there must be a containing/corresponding dirMetadata | ||
| var dirMetadata = _metadataForDirectory[dir]; | ||
amcasey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (configs.Count == 0) | ||
| { | ||
| fileMetadata.Dispose(); | ||
| _metadataForFile.Remove(path); | ||
| dirMetadata.FileWatchCount--; | ||
|
|
||
| _logger.RemovedFileWatcher(path); | ||
|
|
||
| if (dirMetadata.FileWatchCount == 0) | ||
| { | ||
| dirMetadata.Dispose(); | ||
| _metadataForDirectory.Remove(dir); | ||
|
|
||
| _logger.RemovedDirectoryWatcher(dir); | ||
| } | ||
| } | ||
|
|
||
| _logger.ObserverCount(path, configs.Count); | ||
| _logger.FileCount(dir, dirMetadata.FileWatchCount); | ||
| } | ||
|
|
||
| /// <remarks>Test hook</remarks> | ||
| internal int TestGetDirectoryWatchCountUnsynchronized() => _metadataForDirectory.Count; | ||
|
|
||
| /// <remarks>Test hook</remarks> | ||
| internal int TestGetFileWatchCountUnsynchronized(string dir) => _metadataForDirectory.TryGetValue(dir, out var metadata) ? metadata.FileWatchCount : 0; | ||
|
|
||
| /// <remarks>Test hook</remarks> | ||
| internal int TestGetObserverCountUnsynchronized(string path) => _metadataForFile.TryGetValue(path, out var metadata) ? metadata.Configs.Count : 0; | ||
|
|
||
| void IDisposable.Dispose() | ||
| { | ||
| if (_disposed) | ||
| { | ||
| return; | ||
| } | ||
| _disposed = true; | ||
|
|
||
| foreach (var dirMetadata in _metadataForDirectory.Values) | ||
| { | ||
| dirMetadata.Dispose(); | ||
| } | ||
|
|
||
| foreach (var fileMetadata in _metadataForFile.Values) | ||
| { | ||
| fileMetadata.Dispose(); | ||
| } | ||
| } | ||
|
|
||
| private sealed class DirectoryWatchMetadata(IFileProvider fileProvider) : IDisposable | ||
| { | ||
| public readonly IFileProvider FileProvider = fileProvider; | ||
| public int FileWatchCount; | ||
|
|
||
| public void Dispose() => (FileProvider as IDisposable)?.Dispose(); | ||
| } | ||
|
|
||
| private sealed class FileWatchMetadata(IDisposable disposable) : IDisposable | ||
| { | ||
| public readonly IDisposable Disposable = disposable; | ||
| public readonly HashSet<CertificateConfig> Configs = new(ReferenceEqualityComparer.Instance); | ||
| public DateTimeOffset LastModifiedTime = DateTimeOffset.MinValue; | ||
|
|
||
| public void Dispose() => Disposable.Dispose(); | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.