Ensure using latest webhook url for HTTP management APIs (#1716)

Previously, we store the value of the webhook URL during extension
initialization with the assumption that this webhook URL never changes
after startup. This stored value is used for various IDurableClient
methods, along with the out-of-proc clients when local RPC endpoints are
disabled.

However, it turns out that in the case of slot-swap operations,
WEBSITE_HOSTNAME uses the staging slot hostname at site startup, and
only after the slot swap does the hostname change. The function host
listens for a new header value sent as a part of some ping, updating the
source of ExtensionConfigContext.GetWebhookUrl() with the new value once
the slot swap is complete. This PR makes sure we always fetch the value
from the ExtensionConfigContext instead of caching it to ensure we have
the most up-to-date value.

Author: Connor McMahon

release_notes.md

@@ -1,5 +1,6 @@
 ## New Features
 - Added support to select a storage backend provider when multiple are installed (#1702): Select which storage backend to use by setting the `type` field under `durableTask/storageProvider` in host.json. If this field isn't set, then the storage backend will default to using Azure Storage.
-
 - Improved concurrency defaults for the App Service Consumption plan (https://github.com/Azure/azure-functions-durable-extension/pull/1706)
 
+## Bug Fixes:
+- Properly used update management API URLs after a successful slot swap (#1716)

src/WebJobs.Extensions.DurableTask/DurableTaskExtension.cs

@@ -310,13 +310,17 @@ void IExtensionConfigProvider.Initialize(ExtensionConfigContext context)
             // Throw if any of the configured options are invalid
             this.Options.Validate(this.nameResolver, this.TraceHelper);
 
-            // For 202 support
-            if (this.Options.NotificationUrl == null)
-            {
 #pragma warning disable CS0618 // Type or member is obsolete
-                this.Options.NotificationUrl = context.GetWebhookHandler();
+
+            // Invoke webhook handler to make functions runtime register extension endpoints.
+            context.GetWebhookHandler();
+
+            // This line ensure every time we need the webhook URI, we get it directly from the
+            // function runtime, which has the most up-to-date knowledge about the site hostname.
+            this.HttpApiHandler.RegisterWebhookProvider(
+                this.Options.WebhookUriProviderOverride ??
+                context.GetWebhookHandler);
 #pragma warning restore CS0618 // Type or member is obsolete
-            }
 
             this.TraceConfigurationSettings();
 

src/WebJobs.Extensions.DurableTask/HttpApiHandler.cs

@@ -71,6 +71,7 @@ internal class HttpApiHandler : IDisposable
         private readonly EndToEndTraceHelper traceHelper;
         private readonly DurableTaskOptions durableTaskOptions;
         private readonly DurableTaskExtension config;
+        private Func<Uri> webhookUrlProvider;
 
         public HttpApiHandler(
             EndToEndTraceHelper traceHelper,
@@ -83,6 +84,8 @@ public HttpApiHandler(
             this.durableTaskOptions = durableTaskOptions;
             this.traceHelper = traceHelper;
 
+            this.webhookUrlProvider = this.durableTaskOptions.WebhookUriProviderOverride;
+
             // The listen URL must not include the path.
             this.localHttpListener = new LocalHttpListener(
                 this.traceHelper,
@@ -118,6 +121,11 @@ internal HttpResponseMessage CreateCheckStatusResponse(
                 httpManagementPayload.RestartPostUri);
         }
 
+        public void RegisterWebhookProvider(Func<Uri> webhookProvider)
+        {
+            this.webhookUrlProvider = webhookProvider;
+        }
+
         // /orchestrators/{functionName}/{instanceId?}
         private static TemplateMatcher GetStartOrchestrationRoute()
         {
@@ -264,13 +272,9 @@ public async Task<HttpResponseMessage> HandleRequestAsync(HttpRequestMessage req
                 {
                     basePath = this.localHttpListener.InternalRpcUri.AbsolutePath;
                 }
-                else if (this.durableTaskOptions.NotificationUrl != null)
-                {
-                    basePath = this.durableTaskOptions.NotificationUrl.AbsolutePath;
-                }
                 else
                 {
-                    throw new InvalidOperationException($"Don't know how to handle request to {request.RequestUri}.");
+                    basePath = this.GetWebhookUri().AbsolutePath;
                 }
 
                 string path = "/" + request.RequestUri.AbsolutePath.Substring(basePath.Length).Trim('/');
@@ -1021,19 +1025,15 @@ protected virtual IDurableClient GetClient(DurableClientAttribute attribute)
 
         internal string GetBaseUrl()
         {
-            this.ThrowIfWebhooksNotConfigured();
-
-            Uri notificationUri = this.durableTaskOptions.NotificationUrl;
+            Uri notificationUri = this.GetWebhookUri();
 
             string hostUrl = notificationUri.GetLeftPart(UriPartial.Authority);
             return hostUrl + notificationUri.AbsolutePath.TrimEnd('/');
         }
 
         internal string GetUniversalQueryStrings()
         {
-            this.ThrowIfWebhooksNotConfigured();
-
-            Uri notificationUri = this.durableTaskOptions.NotificationUrl;
+            Uri notificationUri = this.GetWebhookUri();
 
             return !string.IsNullOrEmpty(notificationUri.Query)
                 ? notificationUri.Query.TrimStart('?')
@@ -1063,9 +1063,7 @@ private HttpManagementPayload GetClientResponseLinks(
              string connectionName,
              bool returnInternalServerErrorOnFailure = false)
         {
-            this.ThrowIfWebhooksNotConfigured();
-
-            Uri notificationUri = this.durableTaskOptions.NotificationUrl;
+            Uri notificationUri = this.GetWebhookUri();
             Uri baseUri = request?.RequestUri ?? notificationUri;
 
             // e.g. http://{host}/runtime/webhooks/durabletask?code={systemKey}
@@ -1130,12 +1128,9 @@ private HttpResponseMessage CreateCheckStatusResponseMessage(
             return response;
         }
 
-        private void ThrowIfWebhooksNotConfigured()
+        private Uri GetWebhookUri()
         {
-            if (this.durableTaskOptions.NotificationUrl == null)
-            {
-                throw new InvalidOperationException("Webhooks are not configured");
-            }
+            return this.webhookUrlProvider?.Invoke() ?? throw new InvalidOperationException("Webhooks are not configured");
         }
 
         internal bool TryGetRpcBaseUrl(out Uri rpcBaseUrl)

src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask-net461.xml

@@ -98,17 +98,6 @@ public string HubName
         /// </value>
         public int? MaxConcurrentOrchestratorFunctions { get; set; } = null;
 
-        /// <summary>
-        /// Gets or sets the base URL for the HTTP APIs managed by this extension.
-        /// </summary>
-        /// <remarks>
-        /// This property is intended for use only by runtime hosts.
-        /// </remarks>
-        /// <value>
-        /// A URL pointing to the hosted function app that responds to status polling requests.
-        /// </value>
-        public Uri NotificationUrl { get; set; }
-
         /// <summary>
         /// Gets or sets a value indicating whether to enable the local RPC endpoint managed by this extension.
         /// </summary>
@@ -210,6 +199,10 @@ public string HubName
         // Used for mocking the lifecycle notification helper.
         internal HttpMessageHandler NotificationHandler { get; set; }
 
+        // This is just a way for tests to overwrite the webhook url, since there is no easy way
+        // to mock the value from ExtensionConfigContext. It should not be used in production code paths.
+        internal Func<Uri> WebhookUriProviderOverride { get; set; }
+
         /// <summary>
         /// Sets HubName to a value that is considered a default value.
         /// </summary>
@@ -226,14 +219,6 @@ internal void TraceConfiguration(EndToEndTraceHelper traceHelper, JObject storag
             // We make updates to the clone rather than to JSON to ensure we're updating what we think we're updating.
             DurableTaskOptions clone = JObject.FromObject(this).ToObject<DurableTaskOptions>();
 
-            // Don't trace the notification URL query string since it may contain secrets.
-            // This is the only property which we expect to contain secrets. Everything else should be *names*
-            // of secrets that are resolved later from environment variables, etc.
-            if (clone.NotificationUrl != null)
-            {
-                clone.NotificationUrl = new Uri(clone.NotificationUrl.GetLeftPart(UriPartial.Path));
-            }
-
             // At this stage the task hub name is expected to have been resolved. However, we want to know
             // what the original value was in addition to the resolved value, so we're updating the JSON
             // blob property to use the original, unresolved value.

src/WebJobs.Extensions.DurableTask/Microsoft.Azure.WebJobs.Extensions.DurableTask.xml

@@ -322,7 +322,7 @@ private static DurableTaskExtension GetDurableTaskConfig()
         {
             var options = new DurableTaskOptions();
             options.HubName = "DurableTaskHub";
-            options.NotificationUrl = new Uri("https://sampleurl.net");
+            options.WebhookUriProviderOverride = () => new Uri("https://sampleurl.net");
             var wrappedOptions = new OptionsWrapper<DurableTaskOptions>(options);
             var nameResolver = TestHelpers.GetTestNameResolver();
             var connectionStringResolver = new TestConnectionStringResolver();

src/WebJobs.Extensions.DurableTask/Options/DurableTaskOptions.cs

@@ -33,7 +33,10 @@ public void CreateCheckStatusResponse_Throws_Exception_When_NotificationUrl_Miss
             {
                 Notifications = new NotificationOptions(),
             };
-            options.NotificationUrl = null;
+
+            // With a null override, and the production code path returning null webhook path,
+            // this simulates a non-configured webhook url.
+            options.WebhookUriProviderOverride = null;
             options.HubName = "DurableTaskHub";
 
             var httpApiHandler = new HttpApiHandler(GetTestExtension(options), null);
@@ -62,6 +65,45 @@ public async Task WaitForCompletionOrCreateCheckStatusResponseAsync_Throws_Excep
             Assert.Equal($"Total timeout 0 should be bigger than retry timeout 100", ex.Message);
         }
 
+        [Fact]
+        [Trait("Category", PlatformSpecificHelpers.TestCategory)]
+        public void OutOfProcEndpoints_UpdateWithNewWebhookUri()
+        {
+            var httpApiHandler = new HttpApiHandler(GetTestExtension(), null);
+            var webhookProvider = new ChangingWebhookProvider() { WebhookUri = new Uri(TestConstants.NotificationUrl) };
+            httpApiHandler.RegisterWebhookProvider(() => webhookProvider.WebhookUri);
+
+            AssertApisUsingCorrectWebhookUri(httpApiHandler, TestConstants.NotificationUrlBase);
+
+            string newWebhookUri = TestConstants.NotificationUrl.Replace("localhost:7071", "localhost:5050");
+            string newBaseUri = TestConstants.NotificationUrlBase.Replace("localhost:7071", "localhost:5050");
+            webhookProvider.WebhookUri = new Uri(newWebhookUri);
+
+            AssertApisUsingCorrectWebhookUri(httpApiHandler, newBaseUri);
+        }
+
+        // Validate the expected uris are used for CreateHttpManagementPayload(), GetBaseUrl(), and GetInstanceCreationLinks()
+        private static void AssertApisUsingCorrectWebhookUri(HttpApiHandler httpApiHandler, string expectedBaseUri)
+        {
+            HttpManagementPayload managementPayload = httpApiHandler.CreateHttpManagementPayload(
+                 TestConstants.InstanceId,
+                 null,
+                 null);
+
+            Assert.StartsWith(expectedBaseUri, managementPayload.StatusQueryGetUri);
+            Assert.StartsWith(expectedBaseUri, managementPayload.SendEventPostUri);
+            Assert.StartsWith(expectedBaseUri, managementPayload.PurgeHistoryDeleteUri);
+            Assert.StartsWith(expectedBaseUri, managementPayload.RestartPostUri);
+            Assert.StartsWith(expectedBaseUri, managementPayload.TerminatePostUri);
+
+            string baseUri = httpApiHandler.GetBaseUrl();
+            Assert.Equal(expectedBaseUri, baseUri);
+
+            HttpCreationPayload creationPayload = httpApiHandler.GetInstanceCreationLinks();
+            Assert.StartsWith(expectedBaseUri, creationPayload.CreateNewInstancePostUri);
+            Assert.StartsWith(expectedBaseUri, creationPayload.CreateAndWaitOnNewInstancePostUri);
+        }
+
         [Fact]
         [Trait("Category", PlatformSpecificHelpers.TestCategory)]
         public async Task CreateCheckStatusResponse_Returns_Correct_HTTP_202_Response()
@@ -1354,7 +1396,7 @@ public async Task SignalEntity_Is_Success(bool hasKey, bool hasOp, bool hasConte
         private static DurableTaskExtension GetTestExtension()
         {
             var options = new DurableTaskOptions();
-            options.NotificationUrl = new Uri(TestConstants.NotificationUrl);
+            options.WebhookUriProviderOverride = () => new Uri(TestConstants.NotificationUrl);
             options.HubName = "DurableFunctionsHub";
 
             return GetTestExtension(options);
@@ -1382,6 +1424,11 @@ protected override IDurableClient GetClient(DurableClientAttribute attribute)
             }
         }
 
+        private class ChangingWebhookProvider
+        {
+            public Uri WebhookUri { get; set; }
+        }
+
         private class MockDurableTaskExtension : DurableTaskExtension
         {
             public MockDurableTaskExtension(DurableTaskOptions options)

test/Common/DurableClientBaseTests.cs

@@ -104,7 +104,7 @@ public static ITestHost GetJobHost(
             {
                 DefaultAsyncRequestSleepTimeMilliseconds = httpAsyncSleepTime,
             };
-            options.NotificationUrl = notificationUrl;
+            options.WebhookUriProviderOverride = () => notificationUrl;
             options.ExtendedSessionsEnabled = enableExtendedSessions;
             options.MaxConcurrentOrchestratorFunctions = 200;
             options.MaxConcurrentActivityFunctions = 200;

test/Common/HttpApiHandlerTests.cs

@@ -54,7 +54,7 @@ private static DurableTaskExtension GetDurableTaskConfig()
         {
             var options = new DurableTaskOptions();
             options.HubName = "DurableTaskHub";
-            options.NotificationUrl = new Uri("https://sampleurl.net");
+            options.WebhookUriProviderOverride = () => new Uri("https://sampleurl.net");
             var wrappedOptions = new OptionsWrapper<DurableTaskOptions>(options);
             var nameResolver = TestHelpers.GetTestNameResolver();
             var connectionStringResolver = new TestConnectionStringResolver();