[WebApp] Use onlyIfNotExits to support slots #13669
Conversation
…sts with pipeline steps and mutating bicep on runtime
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13669Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13669" |
There was a problem hiding this comment.
Pull request overview
This PR removes the runtime WebApp slot parent existence check and replaces it with a declarative approach using the @onlyIfNotExists() bicep decorator. This is a temporary workaround while waiting for native Azure SDK support (azure-sdk-for-net PR #54632).
Key changes:
- Eliminates the "check-api-exists" pipeline step that used Azure ARM API to verify WebApp existence
- Adds post-processing to inject
@onlyIfNotExists()decorators into generated bicep templates - Enables the same bicep manifest to work for both first deployment (creates parent + slot) and subsequent deployments (skips existing parent, updates slot only)
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
ConditionalProvisionableResource.cs |
Adds ConditionalResourceInfo class (currently unused) to track conditional deployment info |
BicepDecoratorWriter.cs |
Implements regex-based post-processing to inject @onlyIfNotExists() decorators into bicep |
AzureAppServiceWebsiteContext.cs |
Adds BuildWebSiteAndSlotForManifest() method and tracking list for resources needing decorators |
AzureAppServiceWebSiteResource.cs |
Removes ARM API existence check logic; adds bicep post-processing in GetBicepTemplateFile() and GetBicepTemplateString() |
AzureAppServiceEnvironmentContext.cs |
Adds TryGetWebsiteContext() helper method |
| Test snapshots (3 files) | Updates pipeline step counts from 36→35, 39→38, 29→28 due to removed check step |
| Bicep snapshots (2 files) | Shows @onlyIfNotExists() decorator applied to webapp and mainContainer resources |
| Playground files | Demonstrates slot configuration with WithDeploymentSlot("staging") |
| var hostName = $"{TargetResource.Name.ToLowerInvariant()}-{websiteSuffix}"; | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(deploymentSlot)) | ||
| { | ||
| hostName += $"-{deploymentSlot}"; | ||
| } | ||
|
|
||
| if (hostName.Length > 60) | ||
| { | ||
| hostName = hostName.Substring(0, 60); | ||
| } | ||
|
|
There was a problem hiding this comment.
Using Substring to truncate the hostname when it exceeds 60 characters can result in cutting off in the middle of a meaningful component (like a deployment slot name). Consider using a more robust truncation strategy that preserves the structure, such as using the 'take' function pattern shown elsewhere in the codebase, or ensuring critical parts like the deployment slot suffix are preserved.
| var hostName = $"{TargetResource.Name.ToLowerInvariant()}-{websiteSuffix}"; | |
| if (!string.IsNullOrWhiteSpace(deploymentSlot)) | |
| { | |
| hostName += $"-{deploymentSlot}"; | |
| } | |
| if (hostName.Length > 60) | |
| { | |
| hostName = hostName.Substring(0, 60); | |
| } | |
| const int maxHostNameLength = 60; | |
| var baseHostName = $"{TargetResource.Name.ToLowerInvariant()}-{websiteSuffix}"; | |
| string hostName; | |
| if (!string.IsNullOrWhiteSpace(deploymentSlot)) | |
| { | |
| var slotPart = $"-{deploymentSlot}"; | |
| var maxBaseLength = maxHostNameLength - slotPart.Length; | |
| if (maxBaseLength <= 0) | |
| { | |
| var combined = baseHostName + slotPart; | |
| hostName = combined.Length > maxHostNameLength | |
| ? combined[..maxHostNameLength] | |
| : combined; | |
| } | |
| else | |
| { | |
| var truncatedBase = baseHostName.Length > maxBaseLength | |
| ? baseHostName[..maxBaseLength] | |
| : baseHostName; | |
| if (truncatedBase.Length == maxBaseLength && baseHostName.Length > maxBaseLength) | |
| { | |
| var lastHyphen = truncatedBase.LastIndexOf('-'); | |
| if (lastHyphen > 0) | |
| { | |
| truncatedBase = truncatedBase[..lastHyphen]; | |
| } | |
| } | |
| hostName = $"{truncatedBase}{slotPart}"; | |
| } | |
| } | |
| else | |
| { | |
| hostName = baseHostName; | |
| if (hostName.Length > maxHostNameLength) | |
| { | |
| var truncated = hostName[..maxHostNameLength]; | |
| var lastHyphen = truncated.LastIndexOf('-'); | |
| hostName = lastHyphen > 0 | |
| ? truncated[..lastHyphen] | |
| : truncated; | |
| } | |
| } |
| /// This is a temporary solution until Azure.Provisioning SDK natively supports conditional resources. | ||
| /// Once the SDK adds native support, this class can be deprecated and removed. | ||
| /// </remarks> | ||
| internal sealed class ConditionalResourceInfo |
There was a problem hiding this comment.
The filename 'ConditionalProvisionableResource.cs' does not match the class name 'ConditionalResourceInfo' defined within it. According to C# naming conventions, the filename should match the primary class name in the file. Consider renaming the file to 'ConditionalResourceInfo.cs'.
| /// <summary> | ||
| /// Tracks conditional deployment information for resources. | ||
| /// This allows resources to be deployed conditionally using the bicep 'if' syntax. | ||
| /// </summary> |
There was a problem hiding this comment.
The summary says this class "allows resources to be deployed conditionally using the bicep 'if' syntax", but based on the actual implementation and usage throughout the code, this class is specifically for tracking resources that need the @onlyIfNotExists() decorator, not the general bicep 'if' syntax. The summary should be updated to accurately reflect that this tracks resources requiring the @onlyIfNotExists() decorator annotation.
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| namespace Aspire.Hosting.Azure.AppService; | ||
|
|
||
| /// <summary> | ||
| /// Tracks conditional deployment information for resources. | ||
| /// This allows resources to be deployed conditionally using the bicep 'if' syntax. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This is a temporary solution until Azure.Provisioning SDK natively supports conditional resources. | ||
| /// Once the SDK adds native support, this class can be deprecated and removed. | ||
| /// </remarks> | ||
| internal sealed class ConditionalResourceInfo | ||
| { | ||
| /// <summary> | ||
| /// Gets the resource identifier that will be conditionally deployed. | ||
| /// </summary> | ||
| public string ResourceIdentifier { get; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the condition expression string that determines whether the resource should be deployed. | ||
| /// This should be a valid bicep boolean expression (e.g., "firstDeployment" or "not(firstDeployment)"). | ||
| /// </summary> | ||
| public string ConditionExpression { get; } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="ConditionalResourceInfo"/> class. | ||
| /// </summary> | ||
| /// <param name="resourceIdentifier">The bicep identifier of the resource to deploy conditionally.</param> | ||
| /// <param name="conditionExpression">The bicep boolean expression string that determines if the resource should be deployed.</param> | ||
| public ConditionalResourceInfo(string resourceIdentifier, string conditionExpression) | ||
| { | ||
| ResourceIdentifier = resourceIdentifier ?? throw new ArgumentNullException(nameof(resourceIdentifier)); | ||
| ConditionExpression = conditionExpression ?? throw new ArgumentNullException(nameof(conditionExpression)); | ||
| } | ||
| } |
There was a problem hiding this comment.
The XML documentation comment describes what the parameter will contain (bicep boolean expression), but the actual property name 'ConditionExpression' and the class being defined are unused in the codebase. The class appears to be a placeholder that was never integrated. Only the 'OnlyIfNotExistsResources' lists are actually used to track resource identifiers, not ConditionalResourceInfo instances. Consider removing this unused class definition.
| // Licensed to the .NET Foundation under one or more agreements. | |
| // The .NET Foundation licenses this file to you under the MIT license. | |
| namespace Aspire.Hosting.Azure.AppService; | |
| /// <summary> | |
| /// Tracks conditional deployment information for resources. | |
| /// This allows resources to be deployed conditionally using the bicep 'if' syntax. | |
| /// </summary> | |
| /// <remarks> | |
| /// This is a temporary solution until Azure.Provisioning SDK natively supports conditional resources. | |
| /// Once the SDK adds native support, this class can be deprecated and removed. | |
| /// </remarks> | |
| internal sealed class ConditionalResourceInfo | |
| { | |
| /// <summary> | |
| /// Gets the resource identifier that will be conditionally deployed. | |
| /// </summary> | |
| public string ResourceIdentifier { get; } | |
| /// <summary> | |
| /// Gets the condition expression string that determines whether the resource should be deployed. | |
| /// This should be a valid bicep boolean expression (e.g., "firstDeployment" or "not(firstDeployment)"). | |
| /// </summary> | |
| public string ConditionExpression { get; } | |
| /// <summary> | |
| /// Initializes a new instance of the <see cref="ConditionalResourceInfo"/> class. | |
| /// </summary> | |
| /// <param name="resourceIdentifier">The bicep identifier of the resource to deploy conditionally.</param> | |
| /// <param name="conditionExpression">The bicep boolean expression string that determines if the resource should be deployed.</param> | |
| public ConditionalResourceInfo(string resourceIdentifier, string conditionExpression) | |
| { | |
| ResourceIdentifier = resourceIdentifier ?? throw new ArgumentNullException(nameof(resourceIdentifier)); | |
| ConditionExpression = conditionExpression ?? throw new ArgumentNullException(nameof(conditionExpression)); | |
| } | |
| } | |
| // Licensed to the .NET Foundation under one or more agreements. | |
| // The .NET Foundation licenses this file to you under the MIT license. | |
| namespace Aspire.Hosting.Azure.AppService; |
| /// <summary> | ||
| /// Builds both the main website and a deployment slot for manifest publishing. | ||
| /// Uses @onlyIfNotExistsDecorator to create webapp and mainContainer only if they don't exist. | ||
| /// Slot and slot container are always created. |
There was a problem hiding this comment.
The XML documentation comment says "Uses @onlyIfNotExistsDecorator" but should say "Uses @onlyIfNotExists() decorator" to match the actual bicep syntax. The decorator name should include the parentheses to accurately represent the bicep syntax.
This PR:
@onlyIfNotExists. This is a temp-fix while Azure provisioning can support setting this directly (PR opened: Add Bicep metadata support for resources Azure/azure-sdk-for-net#54632). Once this is supported by CDK, we would remove this temp-fix and let the CDK to set the annotation.Benefits from this changes:
Note:
The @onlyIfNotExists() decorator: