Skip to content
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

fix: Fixes various issue in the web app deployment script #4050

Merged
merged 3 commits into from
Sep 13, 2020

Conversation

ltwlf
Copy link
Contributor

@ltwlf ltwlf commented Sep 6, 2020

Description

The azure webapp deployment script has various issues e.g. bf luis:build has wrong --dialog argument.
appsettings.deployment.json will never be used by the published bot and exposes local secrets to the remote folder.

Task Item

fixes #4049

if (-not $botPath) {
# If don't provide bot path, then try to copy all dialogs except the runtime folder in parent folder to the publishing folder (bin\Realse\ Folder)
$botPath = '..'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrong default path. Must be on level higher.

}

$botPath = $(Join-Path $botPath '*')
Write-Host "Publishing dialogs from external bot project: $($botPath)"
Copy-Item -Path (Get-Item -Path $botPath -Exclude ('runtime', 'generated')).FullName -Destination $remoteBotPath -Recurse -Force -Container

# Try to get luis config from appsettings
$settings = Get-Content $(Join-Path $projFolder appsettings.deployment.json) | ConvertFrom-Json
Copy link
Contributor Author

Choose a reason for hiding this comment

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

appsettings.deployment.json has no effect to the published bot, should be appsettings.json

$luisSettings = $settings.luis

if (-not $luisAuthoringKey) {
$luisAuthoringKey = $luisSettings.authoringKey
}

if (-not $luisEndpointKey) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Secrets/key should not be published to the remote host -> should be configured via azure setting: luis__endpointkey

if (-not $luisAuthoringRegion) {
$luisAuthoringRegion = $luisSettings.region
}

# set feature configuration
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the local settings features if configured

# create generated folder if not exists
if (!(Test-Path generated)) {
New-Item -ItemType Directory -Force -Path generated
}

bf luis:build --luConfig $(Join-Path $remoteBotPath luconfig.json) --botName $name --authoringKey $luisAuthoringKey --dialog --out .\generated --suffix $customizedEnv -f --region $luisAuthoringRegion
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the argument --dialog without value causes an error. In the latest branch it mus be crosstrained


bf luis:build --luConfig $(Join-Path $remoteBotPath luconfig.json) --botName $name --authoringKey $luisAuthoringKey --dialog --out .\generated --suffix $customizedEnv -f --region $luisAuthoringRegion

bf luis:build --luConfig $(Join-Path $remoteBotPath luconfig.json) --botName $name --authoringKey $luisAuthoringKey --dialog crosstrained --out .\generated --suffix $environment -f --region $luisAuthoringRegion
}
else {
Write-Host "bf luis:build does not exist, use the following command to install:"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Meanwhile there is a published version of the CLI

$settings | Add-Member -Type NoteProperty -Force -Name 'feature' -Value $featureConfig
$settings | ConvertTo-Json -depth 100 | Out-File $(Join-Path $publishFolder appsettings.deployment.json)
$settings | ConvertTo-Json -depth 100 | Out-File $settingsPath
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replace settings in composer settings folder. The settings will only includes the luis app id mappings and feature configuration. All other settings must be configured via the azure web app settings

@coveralls
Copy link

coveralls commented Sep 6, 2020

Coverage Status

Coverage remained the same at 55.494% when pulling 1aa9d6b on ltwlf:deploy into 9cbec03 on microsoft:main.

@luhan2017
Copy link
Contributor

@ltwlf , we are deprecating this scripts, and use the scripts generated in your bot folder instead, could you please try that one? sorry for the confusion.

@ltwlf
Copy link
Contributor Author

ltwlf commented Sep 10, 2020

I optimized the deploy script for CI/CD deployment and updated the readme accordingly. And I deleted the obsolete scripts. As soon this is merged I'm going to record a screen cast on how to do CI/CD pipelines with composer projects.
@VanyLaw can you review the PR?

@cwhitten
Copy link
Member

@ltwlf please update/rebase your branches to represent the appropriate diff. Many commits outside this change are now included and the surface area is much too large.

@ltwlf
Copy link
Contributor Author

ltwlf commented Sep 13, 2020

@cwhitten I rebased on the current main. The file changes are now correct again.

@cwhitten cwhitten merged commit 8d7187c into microsoft:main Sep 13, 2020
@cwhitten cwhitten mentioned this pull request Nov 13, 2020
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
…4050)

* Fixes various issue in the web app deployment script

deploy improvements

fixes missing empty models; removes accidentally committed yarn.lock

* Deletes obsolete scripts

* Optimises deployment script for CI/CD pipeline; readme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with azure webapp deployment script
4 participants