microsoft / azure-pipelines-tasks

Tasks for Azure Pipelines
https://aka.ms/tfbuild
MIT License
3.51k stars 2.62k forks source link

[BUG]: `AzureWebAppContainer@1` should error if App Service Resource Kind is not a container (such as "app,linux") #19785

Open qqii opened 7 months ago

qqii commented 7 months ago

New issue checklist

Task name

AzureWebAppContainer

Task version

1

Issue Description

This is related, but not the same as https://github.com/microsoft/azure-pipelines-tasks/issues/14805.

AzureWebAppContainer should validate and error if the App Service Resource Kind is not within the supported osTypeMap

https://github.com/microsoft/azure-pipelines-tasks/blob/e9730867881919d29d809863248eb5f668287ab7/Tasks/AzureWebAppContainerV1/taskparameters.ts#L8-L11

I have just spent a day tracking down why AzureWebAppContainer doesn't work for our configuration. We are migrating to use containers with App Service. Since the docs makes no mention of changing the resource kind, I had foolishly left it as app,linux.

What caused the most confusion was the successful task, and the Azure Portal which showed the correct container image.

image

I dug into this a little bit, and I've figured out what is going wrong. When the App Service Resource Kind is not within the osTypeMap, the getWebAppKind returns osType as undefined.

https://github.com/microsoft/azure-pipelines-tasks/blob/e9730867881919d29d809863248eb5f668287ab7/Tasks/AzureWebAppContainerV1/taskparameters.ts#L53-L74

This propagates uncaught to deployWebAppImage because it uses properties: any, which in turn calls _updateConfigurationDetails.

https://github.com/microsoft/azure-pipelines-tasks/blob/e9730867881919d29d809863248eb5f668287ab7/Tasks/AzureWebAppContainerV1/operations/ContainerBasedDeploymentUtility.ts#L56-L73

Here, since isLinuxApp = undefined and undefined is falsy, windowsFxVersion gets set to the specified image. This in turn is displayed under the Azure Portal under "Properties > Web App > {Publishing model, Container Image}", fooling me into thinking something else was going wrong.

It would be really useful if AzureWebAppContainer validated the kind parameter and creates a runtime error instead of silently continuing!

Environment type (Please select at least one enviroment where you face this issue)

Azure DevOps Server type

dev.azure.com (formerly visualstudio.com)

Relevant log output

Starting: *snip*
==============================================================================
Task         : Azure Web App for Containers
Description  : Deploy containers to Azure App Service
Version      : 1.233.1
Author       : Microsoft Corporation
Help         : https://aka.ms/azurewebappcontainertroubleshooting
==============================================================================
Got service connection details for Azure App Service:'*snip*'
Single-container Deployment to the webapp '*snip*' as only the image detail was specified.
Trying to update App Service Configuration settings. Data: {"appCommandLine":null,"windowsFxVersion":"DOCKER|****snip*"}
Updated App Service Configuration settings.
Restarting App Service : *snip*
App Service '*snip*' restarted successfully.
Successfully updated deployment History at *snip*
App Service Application URL: *snip*
Finishing: *snip*

Repro steps

1. Create an app service with kind "app,linux"
2. Run a pipeline with AzureWebAppContainer
3. It succeeds, and the UI now shows "Publishing model" and "Container Image"
qqii commented 7 months ago

A further bug, when taskParameters.DeployToSlotOrASEFlag = true, getWebAppKind assumes that all slots of the app service has the same kind. This may not be the case.

https://github.com/microsoft/azure-pipelines-tasks/blob/e9730867881919d29d809863248eb5f668287ab7/Tasks/AzureWebAppContainerV1/taskparameters.ts#L53-L74

drdamour commented 1 month ago

@qqii i don't think it resolves as undefined...i think it resolves as the kind string in this case "app,linux" and then

 taskParameters.isLinuxContainerApp = taskParameters.OSType && taskParameters.OSType.indexOf("Linux") !=-1;

get sets to false as "linux" != "Linux" so it becomes windows

i agree this task shoudl fail if the os type can't be definitively found, but i would like it to figure out it should be linux if it encouters "app,linux"