Improve Common.Certificates internals and tests (#1523)

* Fix certificate options configuration bugs and update tests, now using both default and named options

Co-authored-by: Bart Koelman <[email protected]>

Author: TimHess

src/Common/src/Certificates/CertificateConfigurationExtensions.cs

@@ -10,49 +10,6 @@ public static class CertificateConfigurationExtensions
 {
     internal const string AppInstanceIdentityCertificateName = "AppInstanceIdentity";
 
-    /// <summary>
-    /// Adds file path information for a certificate and (optional) private key to configuration, for use with <see cref="CertificateOptions" />.
-    /// </summary>
-    /// <param name="builder">
-    /// The <see cref="IConfigurationBuilder" /> to add configuration to.
-    /// </param>
-    /// <param name="certificateName">
-    /// Name of the certificate, or <see cref="string.Empty" /> for an unnamed certificate.
-    /// </param>
-    /// <param name="certificateFilePath">
-    /// The path on disk to locate a valid certificate file.
-    /// </param>
-    /// <param name="privateKeyFilePath">
-    /// The path on disk to locate a valid PEM-encoded RSA key file.
-    /// </param>
-    /// <returns>
-    /// The incoming <paramref name="builder" /> so that additional calls can be chained.
-    /// </returns>
-    internal static IConfigurationBuilder AddCertificate(this IConfigurationBuilder builder, string certificateName, string certificateFilePath,
-        string? privateKeyFilePath = null)
-    {
-        ArgumentNullException.ThrowIfNull(builder);
-        ArgumentNullException.ThrowIfNull(certificateName);
-        ArgumentException.ThrowIfNullOrEmpty(certificateFilePath);
-
-        string keyPrefix = certificateName.Length == 0
-            ? $"{CertificateOptions.ConfigurationKeyPrefix}{ConfigurationPath.KeyDelimiter}"
-            : $"{CertificateOptions.ConfigurationKeyPrefix}{ConfigurationPath.KeyDelimiter}{certificateName}{ConfigurationPath.KeyDelimiter}";
-
-        var keys = new Dictionary<string, string?>
-        {
-            [$"{keyPrefix}CertificateFilePath"] = certificateFilePath
-        };
-
-        if (!string.IsNullOrEmpty(privateKeyFilePath))
-        {
-            keys[$"{keyPrefix}PrivateKeyFilePath"] = privateKeyFilePath;
-        }
-
-        builder.AddInMemoryCollection(keys);
-        return builder;
-    }
-
     /// <summary>
     /// Adds PEM certificate files representing application identity to the application configuration. When running outside of Cloud Foundry-based platforms,
     /// this method will create certificates resembling those found on the platform.
@@ -152,7 +109,15 @@ public static IConfigurationBuilder AddAppInstanceIdentityCertificate(this IConf
 
         if (certificateFile != null && keyFile != null)
         {
-            builder.AddCertificate(AppInstanceIdentityCertificateName, certificateFile, keyFile);
+            const string keyPrefix = $"{CertificateOptions.ConfigurationKeyPrefix}:{AppInstanceIdentityCertificateName}:";
+
+            var keys = new Dictionary<string, string?>
+            {
+                [$"{keyPrefix}{nameof(CertificateSettings.CertificateFilePath)}"] = certificateFile,
+                [$"{keyPrefix}{nameof(CertificateSettings.PrivateKeyFilePath)}"] = keyFile
+            };
+
+            builder.AddInMemoryCollection(keys);
         }
 
         return builder;

src/Common/src/Certificates/CertificateServiceCollectionExtensions.cs

@@ -32,9 +32,13 @@ public static IServiceCollection ConfigureCertificateOptions(this IServiceCollec
             ? CertificateOptions.ConfigurationKeyPrefix
             : ConfigurationPath.Combine(CertificateOptions.ConfigurationKeyPrefix, certificateName);
 
-        services.AddOptions<CertificateOptions>().BindConfiguration(configurationKey);
-        services.WatchFilePathInOptions<CertificateOptions>(configurationKey, certificateName, "CertificateFileName");
-        services.WatchFilePathInOptions<CertificateOptions>(configurationKey, certificateName, "PrivateKeyFileName");
+        services.AddOptions<CertificateOptions>(certificateName).BindConfiguration(configurationKey);
+
+        services.WatchFilePathInOptions<CertificateOptions>(CertificateOptions.ConfigurationKeyPrefix, certificateName,
+            nameof(CertificateSettings.CertificateFilePath));
+
+        services.WatchFilePathInOptions<CertificateOptions>(CertificateOptions.ConfigurationKeyPrefix, certificateName,
+            nameof(CertificateSettings.PrivateKeyFilePath));
 
         services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<CertificateOptions>, ConfigureCertificateOptions>());
         return services;

src/Common/src/Certificates/FilePathInOptionsChangeTokenSource.cs

@@ -32,9 +32,6 @@ public void ChangePath(string filePath)
         {
             _filePath = filePath;
 
-            // Wait until the file is fully written to disk.
-            Thread.Sleep(500);
-
             ConfigurationReloadToken previousToken = Interlocked.Exchange(ref _changeFilePathToken, new ConfigurationReloadToken());
             previousToken.OnReload();
         }
@@ -44,8 +41,12 @@ public IChangeToken GetChangeToken()
     {
         IChangeToken watcherChangeToken = _fileWatcher.GetChangeToken(_filePath);
 
+        // Wrap the watcher token to delay signaling to the options monitor
+        // -- avoids IOException when certificate and key change around the same time.
+        IChangeToken debouncedToken = new DebouncedChangeToken(watcherChangeToken, TimeSpan.FromMilliseconds(200));
+
         return new CompositeChangeToken([
-            watcherChangeToken,
+            debouncedToken,
             _changeFilePathToken
         ]);
     }
@@ -126,4 +127,30 @@ private static string EnsureTrailingSlash(string path)
             return path.Length > 0 && path[^1] != Path.DirectorySeparatorChar ? $"{path}{Path.DirectorySeparatorChar}" : path;
         }
     }
+
+    private sealed class DebouncedChangeToken(IChangeToken inner, TimeSpan delay) : IChangeToken
+    {
+        private readonly IChangeToken _inner = inner;
+        private readonly TimeSpan _delay = delay;
+
+        public bool HasChanged => _inner.HasChanged;
+
+        public bool ActiveChangeCallbacks => _inner.ActiveChangeCallbacks;
+
+        public IDisposable RegisterChangeCallback(Action<object?> callback, object? state)
+        {
+            return _inner.RegisterChangeCallback(async void (_) =>
+            {
+                try
+                {
+                    await Task.Delay(_delay).ConfigureAwait(false);
+                    callback(state);
+                }
+                catch
+                {
+                    // Swallow exceptions to avoid crashing the options infrastructure
+                }
+            }, state);
+        }
+    }
 }

src/Common/test/Certificates.Test/CertificateConfigurationExtensionsTest.cs

@@ -3,18 +3,32 @@
 // See the LICENSE file in the project root for more information.
 
 using Microsoft.Extensions.Configuration;
+using Steeltoe.Common.TestResources;
 
 namespace Steeltoe.Common.Certificates.Test;
 
 public sealed class CertificateConfigurationExtensionsTest
 {
-    private const string CertificateName = "test";
+    [Fact]
+    public void AddAppInstanceIdentityCertificate_SetsPaths_RunningLocal()
+    {
+        IConfiguration configuration = new ConfigurationBuilder().AddAppInstanceIdentityCertificate().Build();
+
+        configuration[$"Certificates:{CertificateConfigurationExtensions.AppInstanceIdentityCertificateName}:certificateFilePath"].Should()
+            .EndWith($"{LocalCertificateWriter.CertificateFilenamePrefix}Cert.pem");
+
+        configuration[$"Certificates:{CertificateConfigurationExtensions.AppInstanceIdentityCertificateName}:privateKeyFilePath"].Should()
+            .EndWith($"{LocalCertificateWriter.CertificateFilenamePrefix}Key.pem");
+    }
 
     [Fact]
-    public void AddCertificate_SetsPaths()
+    public void AddAppInstanceIdentityCertificate_SetsPaths_RunningOnCloudFoundry()
     {
-        IConfigurationRoot configurationRoot = new ConfigurationBuilder().AddCertificate(CertificateName, "instance.crt", "instance.key").Build();
-        configurationRoot[$"Certificates:{CertificateName}:certificateFilePath"].Should().Be("instance.crt");
-        configurationRoot[$"Certificates:{CertificateName}:privateKeyFilePath"].Should().Be("instance.key");
+        using var vcapScope = new EnvironmentVariableScope("VCAP_APPLICATION", "{}");
+        using var certificateScope = new EnvironmentVariableScope("CF_INSTANCE_CERT", "instance.crt");
+        using var privateKeyScope = new EnvironmentVariableScope("CF_INSTANCE_KEY", "instance.key");
+        IConfiguration configuration = new ConfigurationBuilder().AddAppInstanceIdentityCertificate().Build();
+        configuration[$"Certificates:{CertificateConfigurationExtensions.AppInstanceIdentityCertificateName}:certificateFilePath"].Should().Be("instance.crt");
+        configuration[$"Certificates:{CertificateConfigurationExtensions.AppInstanceIdentityCertificateName}:privateKeyFilePath"].Should().Be("instance.key");
     }
 }