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

Azdevify azure-sql-db-session-recommender. #7

Merged
merged 9 commits into from
Nov 24, 2023
Merged

Azdevify azure-sql-db-session-recommender. #7

merged 9 commits into from
Nov 24, 2023

Conversation

Menghua1
Copy link
Member

@Menghua1 Menghua1 commented Nov 10, 2023

This pr is all changes to convert azure-sql-db-session-recommender application into an Azd template.

You can currently deploy this application using azd by following the steps below:

  1. Execute the command: azd init -t Menghua1/azure-sql-db-session-recommender -b azdevify
  2. Before testing, modify the supportingScriptUris parameter in the infra\app\sqlserver.bicep file to https://raw.githubusercontent.com/Menghua1/azure-sql-db-session-recommender/azdevify/database/setup-database.sh.
  3. Execute the command: azd up.
    Wait a moment for the resource deployment to complete, click the Website endpoint and you will see:
image

@jongio and @yorek for notification.

@Menghua1 Menghua1 marked this pull request as ready for review November 10, 2023 06:06
azure.yaml Outdated Show resolved Hide resolved
infra/app/applicationInsights.bicep Outdated Show resolved Hide resolved
infra/app/functions.bicep Show resolved Hide resolved
infra/app/functions.bicep Outdated Show resolved Hide resolved
infra/app/functions.bicep Outdated Show resolved Hide resolved
infra/main.bicep Outdated Show resolved Hide resolved
infra/main.bicep Outdated
keyVaultName: keyVault.outputs.name
}
}
resource kv 'Microsoft.KeyVault/vaults@2023-02-01' existing = {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this ref?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ref key vault to get service keys. Next, we will use AUTH_TYPE (RBAC or KEYS) to replace it.

infra/main.bicep Outdated Show resolved Hide resolved
infra/main.bicep Outdated Show resolved Hide resolved
infra/main.bicep Outdated Show resolved Hide resolved
Modify format and content

Modify format and content
Modify function.bicep
@Menghua1
Copy link
Member Author

@jongio - Updated based on all your comments, except for getting the keys for related services. Next, we will continue to modify and no longer use key vault to store keys. Instead, we will use AUTH_TYPE (RBAC or KEYS) according to the comments in chat PR. Just to sync the progress of the current PR with you.

@jongio
Copy link
Member

jongio commented Nov 20, 2023

@jongio - Updated based on all your comments, except for getting the keys for related services. Next, we will continue to modify and no longer use key vault to store keys. Instead, we will use AUTH_TYPE (RBAC or KEYS) according to the comments in chat PR. Just to sync the progress of the current PR with you.

Can you please add a 3rd parameter to main.bicep called "USE_KEY_VAULT"?

Default it to true and store and use the keys from KV. If false, then directly use the listKeys function.

Thanks

Add USE_KEY_VAULT parameter
@Menghua1
Copy link
Member Author

Menghua1 commented Nov 21, 2023

@jongio We have added the USE_KEY_VAULT parameter to the main.bicep file. Please re-review this PR. Thanks.

In the updated code, we cannot directly use the listkeys method in the main.bicep file, and an error will be reported: The xxx resource cannot be found, because the xxx resource has not been created yet; so, in the main.bicep file, the resource key's value is empty when useKeyVault is false, and the listkeys method is used under the Infra/app folder.

The reason about the following files are not directly used in the "infra/core" folder:

  • infra/app/sqlserver.bicep:
    -> Update startIpAddress: '0.0.0.1', endIpAddress: '255.255.255.254' tostartIpAddress: '0.0.0.0, endIpAddress: '0.0.0.0' : Because the firewall needs to be set up to allow Azure resources to access sqlservice
    -> Add the Microsoft.Resources/deploymentScripts@2020-10-01 resource to run the sql command.
    -> Add the Microsoft.Sql/servers/administrators@2022-05-01-preview resource to set the Microsoft Entra admin for Azure SQL server.

  • infra\app\openai.bicep and infra\app\storageaccount.bicep:
    Ref Key vault to store openAIKey or storageAccountKey.

  • infra\app\keyvault.bicep:
    Add the properties enabledForTemplateDeployment: true to allow getSecret in main.bicep.

@yorek
Copy link
Collaborator

yorek commented Nov 21, 2023

I just tested it and I got the following error:

ERROR: deployment failed: failing invoking action 'provision', error deploying infrastructure: deploying to resource group:

Deployment Error Details:
DeploymentScriptDownloadFailure: The deployment script execution failed because the primary or supporting scripts could not be downloaded successfully due to the following error:
Uri: https://raw.githubusercontent.com/Menghua1/azure-sql-db-session-recommender/main/database/setup-database.sh failed to download. Detail: curl: (22) The requested URL returned error: 404. Please refer to https://aka.ms/DeploymentScriptsTroubleshoot for more deployment script information.

@Menghua1
Copy link
Member Author

@yorek According to the error message, the link you are using is the main branch. However, the file setup-database.sh does not exist in the main branch, it is in the azdevify branch.
The full link of supportingScriptUris is https://raw.githubusercontent.com/Menghua1/azure-sql-db-session-recommender/azdevify/database/setup-database.sh. Please update this link in your code.

@yorek
Copy link
Collaborator

yorek commented Nov 22, 2023

Just tested, it works just like magic! Impressive :)

One thing I noticed, the first time I run azd up I had an error and I had to run azd config set alpha.resourceGroupDeployments on before being able to run azd up successfully.

After that, everything worked perfectly. I think I just fell in love with AZD :)

Two requests:

  1. would it be possible to put in the .env file in the .azure folder then database connection string using the application user and password?
  2. would be possible to return also the sql server administrative login and password?

Otherwise it is quite complex to connect and query the database :)

@yorek
Copy link
Collaborator

yorek commented Nov 22, 2023

Even better if you go set the Microsoft Entra admin for Azure SQL server to be the identity of who created/deployed the solution

@Menghua1
Copy link
Member Author

Menghua1 commented Nov 23, 2023

@yorek Yeah, need to run azd config set alpha.resourceGroupDeployments on before running azd up, because Resource Group Scoped Deployment is currently an alpha feature. We also haved added a note in Readme.md.

Additionally, we haved added code here to set the Microsoft Entra admin for Azure SQL server. Please re-review.

@yorek yorek merged commit e2b95b0 into Azure-Samples:main Nov 24, 2023
1 check passed
@yorek
Copy link
Collaborator

yorek commented Nov 24, 2023

Thanks a lot, it all works flawlessly! Thanks a lot for the contribution, merged now :)

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.

3 participants