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

Adding an azure-nextgen example for placing app service behind vnet #909

Merged
merged 11 commits into from
Feb 16, 2021

Conversation

viveklak
Copy link
Contributor

import * as random from "@pulumi/random";

const config = new pulumi.Config();
const resourceGroupNameParam = config.require("resourceGroupNameParam");
Copy link
Member

Choose a reason for hiding this comment

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

Why require the resource group name? Should we add a default? Otherwise, we need add a command to set it to the readme.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a default and cleaned up a lot of the parameter cruft from the arm template conversion.

@@ -0,0 +1,174 @@
// Copyright 2016-2021, Pulumi Corporation. All rights reserved.

import * as azure_nextgen from "@pulumi/azure-nextgen";
Copy link
Member

Choose a reason for hiding this comment

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

Other examples use a different import style (e.g. https://github.com/pulumi/examples/blob/master/azure-nextgen-ts-aks/index.ts#L7-L8). Should we do the same here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Member

@mikhailshilkov mikhailshilkov left a comment

Choose a reason for hiding this comment

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

This looks great! A couple of small questions.
It would be nice to deploy a demo app to show that this actually works, but I understand that's beyond the scope for now.
What's our plan for porting to other languages? Manually or via PCL?

// Copyright 2016-2021, Pulumi Corporation. All rights reserved.

// pvt DNS zone and virtual network link are only available on this version
import * as pvtnetwork from "@pulumi/azure-nextgen/network/v20180901";
Copy link
Member

Choose a reason for hiding this comment

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

Does it have to be this old version? Have you tried latest for all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest works.

resourceGroupName: resourceGroup.name,
serverFarmId: serverfarm.id,
siteConfig: {
ftpsState: web.FtpsState.AllAllowed,
Copy link
Member

Choose a reason for hiding this comment

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

Do you know if we need this setting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No don't think so, came from the arm template.

Co-authored-by: Mikhail Shilkov <github@mikhail.io>
@viveklak
Copy link
Contributor Author

viveklak commented Feb 11, 2021

This looks great! A couple of small questions.
It would be nice to deploy a demo app to show that this actually works, but I understand that's beyond the scope for now.

I was playing around with something but didn't get it to work yet. Will update the PR with a clear demonstration of the front-end -> backend interaction via the private endpoint as soon as I get it working.

What's our plan for porting to other languages? Manually or via PCL?

Most of this is convertible with PCL now with some cleanup. Will look to get that out shortly.

Copy link
Member

@mikhailshilkov mikhailshilkov left a comment

Choose a reason for hiding this comment

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

Looks great! Just one remark with a possible simplification.


const serverfarm = new web.AppServicePlan("serverfarm", {
kind: "app",
location: location,
Copy link
Member

Choose a reason for hiding this comment

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

If you update to 0.6.1, you can omit location property everywhere by setting the azure-nextgen:location config value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@viveklak viveklak merged commit 8e11d06 into master Feb 16, 2021
@pulumi-bot pulumi-bot deleted the vl/VNetInteg branch February 16, 2021 17:45
dixler pushed a commit that referenced this pull request Jan 21, 2022
Adding an azure-nextgen example for placing app service behind vnet
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.

Hiding App Service behind a VNet
2 participants