The Wayback Machine - https://web.archive.org/web/20201124004813/https://github.com/Azure/azure-quickstart-templates/pull/8054
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

WebApp for Containers template #8054

Open
wants to merge 17 commits into
base: master
from
Open

Conversation

@mgnsm
Copy link

@mgnsm mgnsm commented Aug 30, 2020

PR Checklist

Check these items before submitting a PR...

Contribution Guide

Best Practice Guide

  • - Please check this box once you've submitted the PR if you've read through the Contribution Guide and best practices checklist.

Changelog

  • A new quickstart template that deploys a WebApp for Containers app service that authenticates to an Azure Container Registry (ACR) using a service principal.
  • The service principal must be created before using the template. There is a sample Bash script included in 101-webapp-for-containers\scripts that demonstrates how to deploy all required resources.
  • There is a prereqs template included for creating the ACR.
@microsoft-cla
Copy link

@microsoft-cla microsoft-cla bot commented Aug 30, 2020

CLA assistant check
All CLA requirements met.

@mgnsm
Copy link
Author

@mgnsm mgnsm commented Aug 30, 2020

Are templates that require the existence of a service principal supposed to be validated manually?

The "Deploy Template (prereqs)" step fails with a PrincipalNotFound code which makes sense as the service principal must be created before the Azure Container Registry (ACR) resource.

Please advise how to proceed to get this PR accepted and merged.

@MCKLMT
Copy link
Contributor

@MCKLMT MCKLMT commented Aug 31, 2020

You should be able to use GEN-AZUREAD-OBJECTID to replace GEN-GUID for parameter servicePrincipalObjectId in prereq.azuredeploy.parameters.json.
This value will be replaced by the CI before deploying the template. I hope this help.

@mgnsm
Copy link
Author

@mgnsm mgnsm commented Aug 31, 2020

Thanks for the suggestion. This placeholder should probably be mentioned in the contribution guidelines.

Now the validation of the prereqs template succeeds. The deployment validation of the main template however fails with an AccessToContainerRegistryDenied error code.

How are the appId and password of the required service principal supposed to be passed to/used in the build pipeline?

@MCKLMT
Copy link
Contributor

@MCKLMT MCKLMT commented Aug 31, 2020

You can find all the placeholders there: https://github.com/Azure/azure-quickstart-templates/blob/master/test/ci-gen-setup/.config.json
They will be replaced by the CI before deploying the template.

@mgnsm
Copy link
Author

@mgnsm mgnsm commented Sep 1, 2020

@MCKLMT Yes, using the GEN-AZUREAD-OBJECTID placeholder works for the prereqs template.

It still doens't create an actual service principal which is required for the validation of the main template to succeed.

So the question remains, should I set the validation type to "Manual" in the metadata.json file for the PR to be accepted and merged or how to proceed?

@MCKLMT
Copy link
Contributor

@MCKLMT MCKLMT commented Sep 1, 2020

@bmoore-msft Can you provide guidance to @mgnsm to get his PR validated please?

bmoore-msft added 3 commits Sep 26, 2020
@bmoore-msft
Copy link
Collaborator

@bmoore-msft bmoore-msft commented Sep 26, 2020

I updated the param files to use the existing SP, there is another failure now that looks like configuration (that I'm not familiar with...

@mgnsm
Copy link
Author

@mgnsm mgnsm commented Sep 28, 2020

@bmoore-msft

It fails becase there is no container uploaded to the ACR.

So in between the deployment of the prereqs template (ACR) and the actual template (WebApp), an image has to be uploaded to the ACR for the WebApp to be able to fetch it. The script does this:

az acr build . \
  --registry acrName  \
  --file Dockerfile \
  --image "imageName:latest"

How to do the same in the build pipeline, i.e. run a custom Bash script in between the deployment of the prereqs template and the main template?

bmoore-msft added 2 commits Sep 28, 2020
101-webapp-for-containers/scripts/Dockerfile Outdated Show resolved Hide resolved
password=$(az ad sp create-for-rbac --name $servicePrincipalName --skip-assignment --query password --output tsv)
appId=$(az ad sp show --id $servicePrincipalName --query appId --output tsv)
servicePrincipalObjectId=$(az ad sp show --id $appId --query objectId --output tsv)

This comment has been minimized.

@bmoore-msft

bmoore-msft Sep 28, 2020
Collaborator

remove this file, or if you want to keep if for svc principal creation, remove everything after line 11

This comment has been minimized.

@mgnsm

mgnsm Sep 29, 2020
Author

The Bash script is provided as an example of how to deploy the template including the prereqs outside the context of this repository. It's not meant to be used in the validation of the template itself.

Should I perhaps rename the folder to something else than scripts?

This comment has been minimized.

@bmoore-msft

bmoore-msft Nov 6, 2020
Collaborator

All templates in the repo are deployed the same inside and outside of the repo - having every sample provide their own deployment script creates confusion and a maintenance problem...

This comment has been minimized.

@mgnsm

mgnsm Nov 23, 2020
Author

I removed the script and added instructions for how to deploy the resources in the README file. The build fails for some reason?

101-webapp-for-containers/azuredeploy.json Outdated Show resolved Hide resolved
101-webapp-for-containers/azuredeploy.json Outdated Show resolved Hide resolved
101-webapp-for-containers/azuredeploy.json Outdated Show resolved Hide resolved

Before you use it, you should create a service principal and an ACR using the [pre-requisite template](prereqs/prereq.azuredeploy.json).

You can use [this Bash script](scripts/deploy.sh) to deploy all required resources. The script uses the Azure CLI to do the following:

This comment has been minimized.

@bmoore-msft

bmoore-msft Sep 28, 2020
Collaborator

update this when you remove the script

@mgnsm mgnsm requested a review from bmoore-msft Nov 6, 2020
password=$(az ad sp create-for-rbac --name $servicePrincipalName --skip-assignment --query password --output tsv)
appId=$(az ad sp show --id $servicePrincipalName --query appId --output tsv)
servicePrincipalObjectId=$(az ad sp show --id $appId --query objectId --output tsv)

This comment has been minimized.

@bmoore-msft

bmoore-msft Nov 6, 2020
Collaborator

All templates in the repo are deployed the same inside and outside of the repo - having every sample provide their own deployment script creates confusion and a maintenance problem...

@msftbot
Copy link

@msftbot msftbot bot commented Nov 21, 2020

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.