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

Add GitHub and Azure DevOps provision&deploy pipelines for azd pipeline config #488

Closed
wants to merge 0 commits into from

Conversation

vhvb1989
Copy link
Contributor

@vhvb1989 vhvb1989 commented Aug 1, 2023

  • Updates main.bicep to support role-assigment for a ServicePrincipal when running from a pipelines.
  • Setting role-assigment mode to ServicePrincipal when running from a pipeline (Github or Azure DevOps).
  • Adding documentation on ReadMe for how to enable this provision&deploy pipelines.

@pamelafox
Copy link
Collaborator

sweet, is this ready for review?

@vhvb1989
Copy link
Contributor Author

vhvb1989 commented Aug 2, 2023

sweet, is this ready for review?

Yes.
We can either just let this PR open as a reference for folks on how to add pipelines, or we can merge.
GitHub pipeline is disabled and needs to be renamed to enable (mentioned in the Readme)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@pamelafox pamelafox left a comment

Choose a reason for hiding this comment

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

Overall question: Can they both deploy manually and via CI, or only one or the other? (Particularly in regards to that ServicePrincipal user role change?)

@pamelafox
Copy link
Collaborator

@vhvb1989 Given the number of issues around DevOps, I personally think we should get it in the repo, as long as it doesn't compromise the existing flow.

@vhvb1989
Copy link
Contributor Author

vhvb1989 commented Aug 2, 2023

Overall question: Can they both deploy manually and via CI, or only one or the other? (Particularly in regards to that ServicePrincipal user role change?)

They can both be deployed.
For local/manual, the role access is set to User, which gives the current user access to the resources for running the prepdocs scripts
On CI, the pipeline definition set an ENV VAR to change User to ServicePrincipal. That's because on CI, the SP is the one which will run the prepdocs script.

This new param for main.bicep also allows someone running azd locally with a ServicePrincipal to set the ENV VAR and make this scenario work (it is currently expected to fail)

@pamelafox
Copy link
Collaborator

@vhvb1989 Awesome!

Copy link
Collaborator

@pamelafox pamelafox left a comment

Choose a reason for hiding this comment

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

Nice! I will try it on my fork after you merge.

@vhvb1989
Copy link
Contributor Author

vhvb1989 commented Aug 3, 2023

@pamelafox , I will wait and not merge. I think there would be an issue for folks if they want to add CI pipeline for a current project if they are using more than one resource group.
This project allows folks to set multiple resource groups (and also existing resources) by using ENV VARS. But azd is not currently setting all the values from the .env into GitHub/Azdo secrets/variables. This would result in CI trying to do a new deployment instead of using what's set up locally.

I thought about adding notes here about setting the variables and fixing the pipeline definition. But I would better wait for azd to support setting all the CI variables.

@namratam30
Copy link

I want to apply the condition on bicep if the resource exist. It should not create a new one and use existing.

@vhvb1989
Copy link
Contributor Author

I want to apply the condition on bicep if the resource exist. It should not create a new one and use existing.

Since azd is not putting all the env-vars into github secrets/variables, when azd provision runs on github, azd doesn't find the existing resources.

That's why I didn't want to merge this yet, until azd supports putting all the fields from the env.

If you want this to work, however, you can manually set the github variables and update the workflow definition to use the those variables as env-vars.

@merryHunter
Copy link

Hi, is there any ongoing work regarding this PR or other? Our team needs to have automated CI deployment from Azure DevOps, would be nice to avoid reinventing the wheels...

@mattjames1978
Copy link

mattjames1978 commented Sep 19, 2023

I have had a go at deploying this. One issue that has come up for me is permissions to execute the prepdocs.sh script using the MS hosted ubuntu container. Some how need to allow it to execute chmod 755 etc.

ERROR: failed running post hooks: 'postprovision' hook failed with exit code: '126', Path: './scripts/prepdocs.sh'. : exit code: 126
/bin/sh: 1: ./scripts/prepdocs.sh: Permission denied
##[error]Script failed with exit code: 1

To resolve I added this to azure.yaml file that does the hook post deployment
posix:
shell: sh
run: chmod 755 ./scripts/prepdocs.sh

@mattjames1978
Copy link

@vhvb1989 Hi Victor, sorry to message you directly about this but I am sure you will know the answer :) When deploying this solution via DevOps I have added some additional Bicep resources for our use case such as Private Endpoint but I would like it to display on the handy deployment plan where it puts a tick and Done message. Do you know where the resources are referenced from, in the main repository? I tried adding an output but it didn't make any difference?
thanks in advance.
Matt

Screen shot below with id;s hidden.
Deployment Plan

@pamelafox
Copy link
Collaborator

@mattjames1978 That's determined by azd, there's a set of resource kinds that it displays. You can request that private endpoint be added to that list by filing an issue here: https://github.com/azure/azure-dev

@mattjames1978
Copy link

thanks @pamelafox , will log a request,
many thanks for your help.

@vhvb1989
Copy link
Contributor Author

vhvb1989 commented Oct 1, 2023

@mattjames1978 Can you share some context about what you are trying to do?

Is it the Azure Private endpoint that uses Bastion? (the one from this docs: https://learn.microsoft.com/en-us/azure/private-link/create-private-endpoint-portal?tabs=dynamic-ip ??)

Can you share the extra bicep code you used to set this up?

@mattjames1978
Copy link

mattjames1978 commented Oct 2, 2023

@vhvb1989 sure so we have set this solution up in one of our subscriptions via Azure Devops pipelines using the example you shared and it is all working well. I have made some improvements to the pipeline yaml files so we can select whether to deploy infrastructure or app settings as well as add templates and stages, jobs. Happy to share these as well. To meet our security requirements we wanted to put the app service backend web app behind a private endpoint as well as enable VNET integration. So I have create a module for private endpoint and place the code here Openai\infra\core\pe\private-endpoint.bicep. Then the module is called like the others from main.bicep. Also added an output to appservice.bicep to output the app id which the private endpoint attaches too. Lastly added some additional parameters for the private endpoint resources which we pickup from the pipeline using variable groups in ADO.

VNET integration was simple to do and just added some additional config to the appservice resource such as subnet ID and enabling vnetRouteAllEnabled to true.

In this solution we consume an existing VNET and subnet for Private Endpoint but can easily add a new VNET and subnet resource if you wish.

So the above is all working well and the web app for Open AI can no longer be reached publicly it throws a 403 error by design. We can then reach it on the private IP address from the VNET and internal network via private DNS. The bicep code creates private endpoint with a network interface, private link service connection, new private DNS zone and private DNS Zone Network Link as well as private endpoint DNS group. However in my next version I need to attach to an existing private DNS zone and group in a different subscription but that is just to allow our internal DNS routing to resolve the IP.

One other change I made as well is added identity and AD authentication settings to the web app which uses an existing App registration Client ID and secret. This is mentioned as an additional component in the read me file. But I want to automate as much as possible and this is setup from the start as long as the App registration is done before hand. This is tricky to automate as Bicep does not fully support it and you have to use a deployment script plus you need tenant level rights such as Application Administrator which I don't have in our environment.

So the reason I reached out is I thought it would be nice to have private endpoint listed as a resource on the pipeline deployment screen when azd deploy is running which sounds like is managed via the azd engine. It is not a necessity though as the code still deploys and works, just don't get any output of it. We just see the 8 resources that come with the code version.

Below is the PE code.

pe.zip

Add below to main.bicep

// Private Endpoint
module privateEndpoint 'core/pe/private-endpoint.bicep' = {
  name: 'private-endpoint'
  scope: privateEndpointResourceGroup
  params: {
    location: privateEndpointResourceGroupLocation
    privateEndpointName: privateEndpointName
    appServiceid: backend.outputs.id 
    vnetId: vnet.id 
    subnetId: subnet.id 
    privateDnsZoneName: privateDnsZoneName 
    pvtEndpointDnsGroupName: pvtEndpointDnsGroupName 
  }
}

add below for PE parameters

 "privateEndpointResourceGroupName": {
      "value": "${PRIVATE_ENDPOINT_RESOURCE_GROUP}"
    },
    "privateEndpointName":{
      "value": "${PRIVATE_ENDPOINT_NAME}"
    },
    "privateDnsZoneName":{
      "value": "${PRIVATE_DNS_ZONE_NAME}"
    },
    "pvtEndpointDnsGroupName":{
      "value": "${PRIVATE_ENDPOINT_DNS_GROUP_NAME}"
    },

also added appsettings.bicep with my improvements.

Many thanks
Matt
appservice.zip

@vhvb1989
Copy link
Contributor Author

vhvb1989 commented Oct 2, 2023

@mattjames1978 , thanks for the great explanation.

I've created a PR for you: Azure/azure-dev#2812
It would make azd to display something like this:

image

I'd would be awesome adding your private_endpoint.bicep module to the /core modules from https://github.com/Azure/azure-dev/tree/main/templates/common/infra/bicep/core

Probably to /networking or security.

I was going to make another PR for that, but it looks like the module currently depends on a VNET and subnet to already exists. If we could have the module to automatically create the resources (if the parameters for existing resources are not set), this would become a general-use module for other folks to use.

IIRC, @jongio did (or were doing) some vnet modules in the past... Adding him here as FYI.

@mattjames1978
Copy link

@vhvb1989 thanks very much Victor for adding the Private Endpoint resource. So once it has been approved and publicly available do I just need to update the version of AZD I am using? I am more than happy for you to add the private endpoint module to the code. Makes sense for it to go under networking. I will have a go at creating a condition for the deployment of the VNET and subnet based on parameter set. This should be easy enough. Will share the bicep modules and code once I have tested it.
I also just read that Microsoft doesn't recommend creating subnets as child resources of the VNET as it can cause issues when redeployed so can create the subnet under the VNET resource but conditionally create it if none exists.

I was also going to re visit the storage account as well and potentially add a PE there too. Plus set the firewall on it to the VNET. So lots of development in the pipeline. We should be able to call the PE module twice but need to parametrize the group id to a different sub resource for storage.

thanks
Matt

@vhvb1989
Copy link
Contributor Author

vhvb1989 commented Oct 3, 2023

So once it has been approved and publicly available do I just need to update the version of AZD I am using?

Yes, there's a release schedule for this week. azd will give display an update-available note onc e it is published.

@mattjames1978
Copy link

mattjames1978 commented Oct 4, 2023

thanks @vhvb1989 will install the latest update once released. I have had a go at creating a VNET module which conditionally creates the resources based on a parameter switch. I have deployed both scenarios and seems to work ok. I have uploaded the module zip for you to review and share in your repo.

The module takes a parameter called newOrExisting. Set it to new in main.bicep if you want to deploy new resources. Set to existing if you want to deploy to existing VNET and subnets. However you have to supply the additional parameter names of the existing resources to pick them up. If deploying 'new' then supply the names that you want them to be called.

The module creates a new VNET with two subnets. One for the web app private endpoint, the second for the web app VNET integration. This also adds an NSG to each subnet and a route table. However as these will be different per use case there are no rules or routes. These can be added to the module if required. The properties are commented out. The NSG does deploy the default NSG rules which allows interVNET traffic.

The VNET module outputs the vnetid to consume by the private endpoint and app service, as well as the subnet1 and subnet2 resource id. This has conditions set to output new IDs or the existing IDs.

In main.bicep provide the following parameters:

param newOrExisting string = 'new' //accepts new or existing
param vnetRG string = resourceGroupName
param vnetName string = 'openai-vnet'
param vnetAddressPrefix string = '10.0.0.0/16'
param subnet1Name string = 'subnet1' // for Private Endpoint
param subnet1Prefix string = '10.0.0.0/24'
param subnet2Name string = 'subnet2' //for VNET Integration app service
param subnet2Prefix string = '10.0.1.0/24'
param subnet1nsgname string = 'subnet1nsg'
param subnet1routetablename = 'subnet1rt'
param subnet2nsgname string = 'subnet2nsg'
param subnet2routetablename = 'subnet2rt'

Call the VNET module in main.bicep with following code supplying parameters

module vnet 'core/networking/vnet.bicep' = {
  name: vnetName
  scope:  resourceGroup
  params: {
    location: location
    vnetRG: vnetRG
    vnetName: vnetName
    vnetAddressPrefix: vnetAddressPrefix
    subnet1Name: subnet1Name
    subnet2Name: subnet2Name
    subnet1Prefix: subnet1Prefix
    subnet2Prefix: subnet2Prefix
    subnet1nsgname: subnet1nsgname
    subnet1routetablename: subnet1routetablename
    subnet2nsgname: subnet2nsgname
    subnet2routetablename: subnet2routetablename
    newOrExisting: newOrExisting
  }
}

One thing to note, is if deploying to existing VNET then you don't have to worry about the VNET prefix, subnet prefixes, nsg names or route table names as they will be ignored but the following params should be set for existing with the correct names otherwise the deployment will fail.

param newOrExisting string = 'existing' 
param vnetRG string = existingRGName
param vnetName string = 'existingVNETName'
param subnet1Name string = 'existingSubnetinVNET'
param subnet2Name string = 'existingSubnetinVNET'

Finally set the following Subnet Param on the backed module

subnet2Id: vnet.outputs.subnet2Resourceid

As well as the Private Endpoint Module call

subnet1Id: vnet.outputs.subnet1Resourceid

This will pickup the output of the existing resources or new resources in the VNET module.

Some observations from my testing. I was deploying this using an ADO self hosted agent in an existing VNET and subnet. When deploying the solution to a different VNET I needed to add my existing VNET as a VNET link on the private endpoint so I can communicate to the app. Otherwise you lock yourself out. So worth mentioning this in the Read Me that there are connectivity considerations when deploying to a new VNET. VNET peering and NSG rules may also be required. I added these between the VNET I was building from and the new VNET. I didn't need any routes adding between the two as VNET peering took care of it.

The VM agent needs connectivity to the private endpoint to deploy the application deployment. This can easily be added to the Private Endpoint module to add an extra link to the build VNET

   virtualNetwork: {
      id: vnetId
    }

Let me know if you have any thoughts or improvements on the above.
Hope it is useful
cheers
vnet.zip
Matt

@vhvb1989
Copy link
Contributor Author

vhvb1989 commented Oct 4, 2023

@mattjames1978 , just brilliant.

I'll work on making that part of the /core modules soon. Thank you very much

@tonybaloney
Copy link
Contributor

@mattjames1978 I've worked on a branch based on the resources you shared, please can you look at main...tonybaloney:azure-search-openai-demo:private_endpoint and let me know if I've got this right based on what you said. I had a couple of comments in the bicep where I'm not sure if it's right.

I'll update this and put conditional statements in so it can be proposed as an optional feature.

I removed the AAD app settings because that seemed like a separate requirement.

@mattjames1978
Copy link

mattjames1978 commented Oct 12, 2023

Hi @tonybaloney thanks for taking a look at this and I think we are nearly there. So you have added two boolean values for useVnet and usePrivateEndpoint. That's great. I think if you conditionally call the module in main.bicep based on whether true or false that should work fine.

module vnet 'core/networking/vnet.bicep' = if (usePrivateEndpoint) {

In regards to your comments around the subnets, Subnet1 should be consumed by Private Endpoint. We could name these better to make them clearer so call them PrivateEndpointSubnet and WebAppSubnet if it is better to understand. The difference between the two is subnet2 has delegation enabled for server farms.

So this is correct for private endpoint
subnet1Id: usePrivateEndpoint ? vnet.outputs.subnet1Resourceid : '' // TODO: Verify this is the right subnet

For the backend module it should be set to subnet2ResourceId as follows:

subnet1Id: usePrivateEndpoint ? vnet.outputs.subnet2Resourceid : '' // TODO: Verify this is the right subnet

In my code I have it set to
subnet2Id: vnet.outputs.subnet2Resourceid

Then pass subnet2Id as a param to the appservice.bicep module.

Once you have tested and happy I will update my code also with conditional logic to create VNET and private endpoint based on these two params

Let me know if you have any more queries.
cheers

@pamelafox
Copy link
Collaborator

@mattjames1978 I'm just chatting with another developer who wants to deploy into a private endpoint. Did you need to put OpenAI in a private endpoint? Is that possible? It looks like your Bicep doesn't change anything about OpenAI so I assume you didn't, but wanted to check.

@mattjames1978
Copy link

Hi @pamelafox, thanks for reaching out and you raise a very good point. An oversight on my part as I was focusing on protecting the web front end that the users connect to. We should definitely look at enabling private endpoint for the Open AI service too. This will be easy enough to do and is supported. We should be able to reuse the private endpoint module. Just need to add a parameter for the subresource ID as it will be different and needs to be set to "account"

In this instance it will have a resource type of
Microsoft.CognitiveServices/accounts.

Microsoft also recommends choosing Disabled on the Allow access from. This doesn't block all access and still allows connectivity from the private endpoint.

image

The search service needs configuring to connect to it. Even the storage account supports private endpoints.

One other thing to note is adding a private endpoint for openAI only blocks the chat service. OpenAI Studio is a cloud based service that must be accessed through the public internet.

If you leave it with me, today I will update the code, deploy it in our environment and let you know how it goes. Then I will share the code with you. Just in case there are any connectivity issues with the other components.

@tonybaloney
Copy link
Contributor

@mattjames1978

I've made a number of changes to that template to make it generic in the branch I linked so that private endpoints are off by default, but can be enabled via an environment variable. I'm still just working through provisioning errors.

@mattjames1978
Copy link

thanks for the update @tonybaloney, some further considerations I have made is to enable private endpoint for the following services.

Web App - attach to web sites dns zone
Open AI Service - attach to openai dns zone. Sub resource account
Search service - attach to cognitive services DNS zone. Public network access can then be disabled. Sub resource searchService
Document Intelligence - attach to the cognitive services DNS zone. Sub resource account

Open AI can also be integrated into VNET.

So I ended up building 4 private endpoints in total with three DNS zones.

privatelink.azurewebsites.net
privatelink.openai.azure.com
privatelink.cognitiveservices.azure.com

Hope that helps

@vhvb1989
Copy link
Contributor Author

@mattjames1978 , take a look to https://github.com/Azure/gpt-rag,

If you set the input parameter network isolation = true, everything is provisioned using private endpoints
https://github.com/Azure/GPT-RAG/blob/main/infra/main.bicep#L21

@mattjames1978
Copy link

looks awesome @vhvb1989, thanks for sharing. Will take a look at the repo.

cheers

@pamelafox
Copy link
Collaborator

I added the GitHub action workflow to a branch of mine, and I had to take the following steps to get it to work:

  1. Hardcode the openAiResourceGroup location (since that's usually prompted for, but prompts wont work in CI)
  2. Comment out the user roles, since it doesn't make sense to assign those to a service principal (and it results in error)
  3. Remove the postprovision hook that did data ingestion, as it won't work without user roles, and reingestion on CI isn't needed

I don't know the right approach to fix #1 since we intentionally don't set that via an env parameter, to make sure developers have to pick.

For #2, I think the fix is similar to what's needed for #962

For #3, it could fail more gracefully perhaps.

You can see my workarounds in the final commits of this branch:
main...pamelafox:azure-search-openai-demo:playwrightdata

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.

6 participants