Skip to content

[in-proc] Fix sync trigger error when AzureWebjobsStorage is not set in Managed #11214

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

Open
wants to merge 4 commits into
base: in-proc
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@

<!-- Please add your release notes in the following format:
- My change description (#PR)
-->
-->
- Allow sync trigger to happen in managed environment when `AzureWebJobsStorage` is not set (#11214)
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public async Task<SyncTriggersResult> TrySyncTriggersAsync(bool isBackgroundSync
PrepareSyncTriggers();

var hashBlobClient = await GetHashBlobAsync();
if (isBackgroundSync && hashBlobClient == null && !_environment.IsKubernetesManagedHosting())
if (isBackgroundSync && hashBlobClient == null && !_environment.IsAnyKubernetesEnvironment())
{
// short circuit before doing any work in background sync
// cases where we need to check/update hash but don't have
Expand All @@ -135,7 +135,7 @@ public async Task<SyncTriggersResult> TrySyncTriggersAsync(bool isBackgroundSync

bool shouldSyncTriggers = true;
string newHash = null;
if (isBackgroundSync && !_environment.IsKubernetesManagedHosting())
if (isBackgroundSync && hashBlobClient != null)
{
newHash = await CheckHashAsync(hashBlobClient, payload.Content);
shouldSyncTriggers = newHash != null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,62 @@ public async Task TrySyncTriggers_LocalEnvironment_ReturnsFalse()
}
}

[Fact]
public async Task TrySyncTriggers_ManagedAppEnv_WithNo_AzureWebJobsStorage_ReturnsTrue()
{
_vars.Add("AzureWebJobsStorage", null);
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting a dictionary value to null may not have the intended effect of removing the environment variable. Consider using an empty string or a specific test utility method to simulate the absence of this environment variable.

Suggested change
_vars.Add("AzureWebJobsStorage", null);
_vars.Add("AzureWebJobsStorage", string.Empty);

Copilot uses AI. Check for mistakes.

_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteArmCacheEnabled)).Returns("0");

using (var env = new TestScopedEnvironmentVariable(_vars))
{
_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.ManagedEnvironment)).Returns("true");
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("FUNCTIONS_API_SERVER")).Returns("https://appname.azurewebsites.net");
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("CONTAINER_APP_NAME")).Returns("appname");
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("CONTAINER_APP_NAMESPACE")).Returns("appns");
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("CONTAINER_APP_REVISION")).Returns("appname--r1");

// _functionsSyncManager is initialized in the constructor with all the secrets from environment,
// so HostAzureBlobStorageProvider will have AzureWebJobsStorage defined in both ActiveHostConfigurationSource
// and the WebHost IConfiguration source from DI.
// The TestScopedEnvironmentVariable only changes the WebHost level IConfiguration
// When it is set to empty/null, the connection string from the ActiveHostConfigurationSource wins (never changed since it is set in
// constructor as mentioned).
// Therefore, we need to force refresh the configuration with an ActiveHostChanged event. This is because setting an empty/null environment variable
// removes it, but will not remove it from the ActiveHostConfigurationSource.
_scriptHostManager.OnActiveHostChanged();
var result = await _functionsSyncManager.TrySyncTriggersAsync(isBackgroundSync: true);
Assert.True(result.Success);
VerifyResultWithCacheOff(durableVersion: "V1");
}
}

[Fact]
public async Task TrySyncTriggers_KubernetesManagedEnv_WithNo_AzureWebJobsStorage_ReturnsTrue()
{
_vars.Add("AzureWebJobsStorage", null);
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting a dictionary value to null may not have the intended effect of removing the environment variable. Consider using an empty string or a specific test utility method to simulate the absence of this environment variable.

Suggested change
_vars.Add("AzureWebJobsStorage", null);
_vars.Add("AzureWebJobsStorage", "");

Copilot uses AI. Check for mistakes.

_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteArmCacheEnabled)).Returns("0");

using (var env = new TestScopedEnvironmentVariable(_vars))
{
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("FUNCTIONS_API_SERVER")).Returns("https://appname.azurewebsites.net");
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("KUBERNETES_SERVICE_HOST")).Returns("kubhost");
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("POD_NAMESPACE")).Returns("podns");

// _functionsSyncManager is initialized in the constructor with all the secrets from environment,
// so HostAzureBlobStorageProvider will have AzureWebJobsStorage defined in both ActiveHostConfigurationSource
// and the WebHost IConfiguration source from DI.
// The TestScopedEnvironmentVariable only changes the WebHost level IConfiguration
// When it is set to empty/null, the connection string from the ActiveHostConfigurationSource wins (never changed since it is set in
// constructor as mentioned).
// Therefore, we need to force refresh the configuration with an ActiveHostChanged event. This is because setting an empty/null environment variable
// removes it, but will not remove it from the ActiveHostConfigurationSource.
_scriptHostManager.OnActiveHostChanged();
var result = await _functionsSyncManager.TrySyncTriggersAsync(isBackgroundSync: true);
Assert.True(result.Success);
VerifyResultWithCacheOff(durableVersion: "V1");
}
}

[Fact]
public void ArmCacheEnabled_VerifyDefault()
{
Expand Down Expand Up @@ -1147,4 +1203,4 @@ public void Reset()
}
}
}
}
}