Revert "Ensure SyncTriggers clears secrets cache (#7698) (#7744)" (#7915)

* Revert "Ensure SyncTriggers clears secrets cache (#7698) (#7744)"

This reverts commit d16b14d.

* missing ScriptHostManager

Author: VpOfEngineering

src/WebJobs.Script.WebHost/Management/FunctionsSyncManager.cs

@@ -106,8 +106,6 @@ public async Task<SyncTriggersResult> TrySyncTriggersAsync(bool isBackgroundSync
             {
                 await _syncSemaphore.WaitAsync();
 
-                PrepareSyncTriggers();
-
                 var hashBlobClient = await GetHashBlobAsync();
                 if (isBackgroundSync && hashBlobClient == null && !_environment.IsKubernetesManagedHosting())
                 {
@@ -162,28 +160,6 @@ public async Task<SyncTriggersResult> TrySyncTriggersAsync(bool isBackgroundSync
             return result;
         }
 
-        /// <summary>
-        /// SyncTriggers is performed whenever deployments or other changes are made to the application.
-        /// There are some operations we want to perform whenever this happens.
-        /// </summary>
-        private void PrepareSyncTriggers()
-        {
-            // We clear cache to ensure that secrets are reloaded. This is important because secrets are part
-            // of the StartupContext payload (see StartupContextProvider) and that payload comes from the
-            // SyncTriggers operation. So there's a chicken and egg situation here. Consider the following scenario:
-            //   - app is using blob storage for keys
-            //   - a SyncTriggers operation has happened previously and the StartupContext has key info
-            //   - app instances initialize keys from StartupContext (keys aren't loaded from storage)
-            //   - user updates the app to use a new storage account
-            //   - a SyncTriggers operation is performed
-            //   - the app initializes from StartupContext, and **previous old key info is loaded**
-            //   - the SyncTriggers operation uses this old key info, so trigger cache is never updated with new key info
-            //   - Portal/ARM APIs will continue to show old key info.
-            // By clearing cache, we ensure that this host instance reloads keys when they're requested, and the SyncTriggers
-            // operation will contain current keys.
-            _secretManagerProvider.Current.ClearCache();
-        }
-
         internal static bool IsSyncTriggersEnvironment(IScriptWebHostEnvironment webHostEnvironment, IEnvironment environment)
         {
             if (environment.IsCoreTools())

src/WebJobs.Script.WebHost/Security/KeyManagement/ISecretManager.cs

@@ -68,11 +68,5 @@ public interface ISecretManager
         /// <param name="rootScriptPath">The root function directory.</param>
         /// <param name="logger">The ILogger to log to.</param>
         Task PurgeOldSecretsAsync(string rootScriptPath, ILogger logger);
-
-        /// <summary>
-        /// If secrets have been loaded and cached, clear all cached secrets so they'll
-        /// be reloaded next time they're requested.
-        /// </summary>
-        void ClearCache();
     }
 }

src/WebJobs.Script.WebHost/Security/KeyManagement/SecretManager.cs

@@ -651,13 +651,6 @@ private void ClearCacheOnChange(ScriptSecretsType secretsType, string functionNa
             }
         }
 
-        public void ClearCache()
-        {
-            _authorizationCache.Clear();
-            _hostSecrets = null;
-            _functionSecrets.Clear();
-        }
-
         private async Task<string> AnalyzeSnapshots(string[] secretBackups)
         {
             string analyzeResult = string.Empty;

test/WebJobs.Script.Tests.Integration/Management/FunctionsSyncManagerTests.cs

@@ -46,7 +46,6 @@ public class FunctionsSyncManagerTests : IDisposable
         private readonly Mock<IScriptWebHostEnvironment> _mockWebHostEnvironment;
         private readonly Mock<IEnvironment> _mockEnvironment;
         private readonly HostNameProvider _hostNameProvider;
-        private readonly Mock<ISecretManager> _secretManagerMock;
         private readonly TestScriptHostService _scriptHostManager; // To refresh underlying IConfiguration for IAzureBlobStorageProvider
         private string _function1;
         private bool _emptyContent;
@@ -92,8 +91,8 @@ public FunctionsSyncManagerTests()
             var changeTokens = new[] { tokenSource };
             var optionsMonitor = new OptionsMonitor<ScriptApplicationHostOptions>(factory, changeTokens, factory);
             var secretManagerProviderMock = new Mock<ISecretManagerProvider>(MockBehavior.Strict);
-            _secretManagerMock = new Mock<ISecretManager>(MockBehavior.Strict);
-            secretManagerProviderMock.SetupGet(p => p.Current).Returns(_secretManagerMock.Object);
+            var secretManagerMock = new Mock<ISecretManager>(MockBehavior.Strict);
+            secretManagerProviderMock.SetupGet(p => p.Current).Returns(secretManagerMock.Object);
             var hostSecretsInfo = new HostSecretsInfo();
             hostSecretsInfo.MasterKey = "aaa";
             hostSecretsInfo.FunctionKeys = new Dictionary<string, string>
@@ -106,14 +105,13 @@ public FunctionsSyncManagerTests()
                     { "TestSystemKey1", "aaa" },
                     { "TestSystemKey2", "bbb" }
                 };
-            _secretManagerMock.Setup(p => p.GetHostSecretsAsync()).ReturnsAsync(hostSecretsInfo);
+            secretManagerMock.Setup(p => p.GetHostSecretsAsync()).ReturnsAsync(hostSecretsInfo);
             Dictionary<string, string> functionSecretsResponse = new Dictionary<string, string>()
                 {
                     { "TestFunctionKey1", "aaa" },
                     { "TestFunctionKey2", "bbb" }
                 };
-            _secretManagerMock.Setup(p => p.GetFunctionSecretsAsync("function1", false)).ReturnsAsync(functionSecretsResponse);
-            _secretManagerMock.Setup(p => p.ClearCache());
+            secretManagerMock.Setup(p => p.GetFunctionSecretsAsync("function1", false)).ReturnsAsync(functionSecretsResponse);
 
             var configuration = ScriptSettingsManager.BuildDefaultConfiguration();
             var hostIdProviderMock = new Mock<IHostIdProvider>(MockBehavior.Strict);
@@ -318,18 +316,6 @@ public async Task TrySyncTriggers_BackgroundSync_DoesNotPostsEmptyContent()
             }
         }
 
-        [Fact]
-        public async Task TrySyncTriggers_ClearsSecretsCache()
-        {
-            using (var env = new TestScopedEnvironmentVariable(_vars))
-            {
-                var result = await _functionsSyncManager.TrySyncTriggersAsync();
-                Assert.True(result.Success);
-
-                _secretManagerMock.Verify(p => p.ClearCache(), Times.Once);
-            }
-        }
-
         [Fact]
         public async Task TrySyncTriggers_BackgroundSync_PostsExpectedContent()
         {

test/WebJobs.Script.Tests.Shared/TestSecretManager.cs

@@ -90,10 +90,6 @@ public void Reset()
             };
         }
 
-        public void ClearCache()
-        {
-        }
-
         public async Task<(string, AuthorizationLevel)> GetAuthorizationLevelOrNullAsync(string key, string functionName = null)
         {
             return await SecretManager.GetAuthorizationLevelAsync(this, key, functionName);