New Resource: `azurerm_app_service` #344

tombuildsstuff · 2017-09-18T13:35:41Z

This PR extends the work done by @lstolyarov in #2 to support provisioning App Services within an App Service Plan.

There's a couple of limitations with this PR due to API issues (which I'll open Rest API Specs issues about shortly):

it's currently not possible to update three fields: client_affinity_enabled, enabled and tags - as the Update endpoint has issues - such I've made them ForceNew for the moment
it's currently only possible to provision Windows App Services, so I've removed references to Linux App Services for the moment (until the correct API fields are returned)

Fixes #35

tombuildsstuff · 2017-09-18T14:09:13Z

Tests pass:

stack72

@tombuildsstuff this is looking awesome - I left a comments inline for you to think of before it get's merged

One thing I am really not particularly a fan of is these types of validations - just for info, Tom and I spoke on the phone about this so he understands my concerns and I understand his reasoning - it's just nice to let it be known that we have spoken

stack72 · 2017-09-19T11:49:02Z

azurerm/import_arm_app_service_test.go

@@ -0,0 +1,468 @@
+package azurerm


Really nice to see lots of coverage of import functionality! I think it's usually something that's forgotten about :) So <3 for that!

stack72 · 2017-09-19T11:51:49Z

azurerm/resource_arm_app_service.go

+							Type:     schema.TypeString,
+							Optional: true,
+							Default:  "v4.0",
+							ValidateFunc: validation.StringInSlice([]string{


So I am beginning to disagree with validations of this type - by doing this, if Azure release new support, then we need to rerelease the Terraform provider

I am not against it, I just think it limits us - I know it's good for plan time validation but look at what we have to do with diffSuppress

This bit us with another provider so I'd agree with Stack here

Given this refers to versions of .net, I don't think this is likely to change often (if at all, now .net core is out), as such I think the validation is safe enough. Submitting v2.0 to the API actually provisions the latest version of .net 2.0 (which is currently .net 3.5.x) and submitting v4.0 uses the latest version of 4.x (currently .net 4.7).

Given this (and any new version of .net being supported being likely to be a big announcement) - I'm going to suggest we leave this in for the moment, until it's proven to be a pain

stack72 · 2017-09-19T11:53:44Z

azurerm/resource_arm_app_service.go

+				Type:     schema.TypeBool,
+				Optional: true,
+				Default:  true,
+				ForceNew: true, // due to a bug in the Azure API :(


if there is a bug in the API and this relates to a bug report, it would be nice to link back right here for future ref

Updated to link to Azure/azure-rest-api-specs#1697

stack72 · 2017-09-19T11:56:11Z

azurerm/resource_arm_app_service.go

+	enabled := d.Get("enabled").(bool)
+	tags := d.Get("tags").(map[string]interface{})
+
+	siteConfig := expandAppServiceSiteConfig(d)


can this error out? If so, should we try to catch the error?

nope, the schema validation should handle this (so I don't think there's anything needed here)

stack72 · 2017-09-19T11:56:21Z

azurerm/resource_arm_app_service.go

+	tags := d.Get("tags").(map[string]interface{})
+
+	siteConfig := expandAppServiceSiteConfig(d)
+	appSettings := expandAppServiceAppSettings(d)


Same question as per siteConfig

(as above, there's no error thrown here - so not at this time)

stack72 · 2017-09-19T12:00:20Z

azurerm/resource_arm_app_service.go

+			return fmt.Errorf("Error updating Connection Strings for App Service %q: %+v", name, err)
+		}
+	}
+


do any of these need to check to make sure it's Not a new resource?

nope, they're all applicable here

stack72 · 2017-09-19T12:00:36Z

azurerm/resource_arm_app_service.go

+	resp, err := client.Get(resGroup, name)
+	if err != nil {
+		if utils.ResponseWasNotFound(resp.Response) {
+			// TODO: debug logging


if it's a to-do, then.... ;)

stack72 · 2017-09-19T12:02:37Z

website/docs/r/app_service.html.markdown

+
+---
+
+# azurerm\_app\_service


we don't need the \ - this is a fallacy about markdown :)

stack72 · 2017-09-19T12:03:35Z

website/docs/r/app_service.html.markdown

+
+Manages an App Service (within an App Service Plan).
+
+## Example Usage


I would really like to see 1 or 2 other example usages here - something maybe a little more complex with a connection_string struct maybe?

updated this to be a .net 4.x example with a sql connection string, and added a new java example

stack72 · 2017-09-19T12:03:59Z

website/docs/r/app_service.html.markdown

+
+* `connection_string` - (Optional) An `connection_string` block as defined below.
+
+* `client_affinity_enabled` - (Optional) TODO: COMPLETE ME. Changing this forces a new resource to be created.


/me whistles....

mbfrahry

Agreeing with Stack on quite a few things but other than that it looks really good.

mbfrahry · 2017-09-19T14:51:00Z

azurerm/resource_arm_app_service.go

+							Type:     schema.TypeString,
+							Optional: true,
+							Default:  "v4.0",
+							ValidateFunc: validation.StringInSlice([]string{


This bit us with another provider so I'd agree with Stack here

mbfrahry · 2017-09-19T14:59:08Z

website/docs/r/app_service.html.markdown

+
+* `client_affinity_enabled` - (Optional) TODO: COMPLETE ME. Changing this forces a new resource to be created.
+
+* `enabled` - (Optional) TODO: COMPLETE ME. Changing this forces a new resource to be created.


tombuildsstuff · 2017-09-19T17:16:13Z

Updated - tests still pass:

mbfrahry

LGTM!

elemantalcode · 2017-09-26T15:14:59Z

If I could add more +1 or +<3 I would... thank you!!!

pixelicous · 2018-02-20T11:12:15Z

@tombuildsstuff can we add support for msi, custom domain name, support for https only etc?

Currently i need to do that through arm template.. but i get "ttlInSeconds object is not present in the request body"

tombuildsstuff · 2018-02-20T17:47:22Z

@pixelicous

can we add support for msi, custom domain name, support for https only etc?

Issue #756 is tracking the feature request for these items - which we're hoping to look into in the near future.

"ttlInSeconds object is not present in the request body

Unfortunately this is a bug in the Swagger/API which has been raised with Microsoft/Azure here: Azure/azure-rest-api-specs#1697

pixelicous · 2018-02-20T18:54:53Z

@tombuildsstuff thank you! i actually manage to get around the msi problem with an arm template, i had a problem getting the identity.principalid though, that made me get stuck because spns cannot be created using arm template, and using powershell with external provider is just ugly code, you need to export profile of spn, and use it, and remove it.. becomes very ugly.

On another note, Tom i was wondering if you can shade some light, i have been in a couple of hashicorp/ms conferences and they speak about the great cooperation between the companies. now this has been said for months on end now, still, not much change on the azurerm provider.. it seems like you are the only one working on it, and in terms of module registries, well aws has around 200 while azure has around 20, so whats the gig.. what is the workforce behind this provider.. this is very lagging behind to the point that i am thinking of finding another tool to get the job done

thanks man, hopefully you could provide some answer for this, and sorry for hijacking the reply 😋

ghost · 2020-03-31T15:31:32Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

leostolyarov and others added 28 commits September 18, 2017 14:21

Adding azure app service resource

cf0726f

adding importer

ceaf0d7

adding some tests

6f8869b

updating tests to use testlocation()

27c3626

setting tags

fbcb783

Resolve App Service Import Acceptance test failure

2165bd0

removed additional spaces which were inserted by editor

92b89a0

removing duplicates

74f9069

adding documentation

f5f45a1

running fmt

58be15a

updating app service resource

5141e2a

updating app service code

1ebf2e5

updating tests

be6793d

updating documentation

2c8b16e

setting site config

007c21b

adding more tests

f2d292d

making use of utils module

2a8813f

WIP

dd51d15

Reserved requires an App Service Environment

5b0cf5c

Fixing the Java apps

0fc5a96

Adding tests for remote debugging

bd30db1

remote debugging version needs to be computed

e43b775

Fixing the tests

f60e228

Fixing the jetty tests

83b8a61

Removing linux support for the moment

7dd051d

Adding import tests

8e3ac2e

Handling setting the appSettings/connectionStrings correctly

61d6167

Updating the documentation

976e2b4

tombuildsstuff added enhancement new-resource labels Sep 18, 2017

tombuildsstuff mentioned this pull request Sep 18, 2017

Create azure App Service resource #2

Closed

tombuildsstuff requested a review from mbfrahry September 18, 2017 13:48

stack72 reviewed Sep 19, 2017

View reviewed changes

Removing the updates from the create

978c976

tombuildsstuff force-pushed the azurerm/app-service-resource branch from c58554b to 978c976 Compare September 19, 2017 13:56

mbfrahry reviewed Sep 19, 2017

View reviewed changes

Updating the documentation

50f1a06

tombuildsstuff force-pushed the azurerm/app-service-resource branch from 3822292 to 50f1a06 Compare September 19, 2017 17:22

mbfrahry approved these changes Sep 19, 2017

View reviewed changes

tombuildsstuff mentioned this pull request Sep 19, 2017

[App Service] Update API is Broken Azure/azure-rest-api-specs#1697

Closed

Updating the TODO's

91f71fd

tombuildsstuff merged commit 5979359 into master Sep 19, 2017

tombuildsstuff added a commit that referenced this pull request Sep 19, 2017

Updating to include #344

af81d44

tombuildsstuff deleted the azurerm/app-service-resource branch October 2, 2017 12:28

ghost locked and limited conversation to collaborators Mar 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_app_service` #344

New Resource: `azurerm_app_service` #344

tombuildsstuff commented Sep 18, 2017 •

edited

Loading

tombuildsstuff commented Sep 18, 2017

stack72 left a comment

stack72 Sep 19, 2017

stack72 Sep 19, 2017

mbfrahry Sep 19, 2017

tombuildsstuff Sep 19, 2017

stack72 Sep 19, 2017

tombuildsstuff Sep 19, 2017

stack72 Sep 19, 2017

tombuildsstuff Sep 19, 2017

stack72 Sep 19, 2017

tombuildsstuff Sep 19, 2017

stack72 Sep 19, 2017

tombuildsstuff Sep 19, 2017

stack72 Sep 19, 2017

tombuildsstuff Sep 19, 2017

stack72 Sep 19, 2017

tombuildsstuff Sep 19, 2017

stack72 Sep 19, 2017

tombuildsstuff Sep 19, 2017

stack72 Sep 19, 2017

tombuildsstuff Sep 19, 2017 •

edited

Loading

mbfrahry left a comment

mbfrahry Sep 19, 2017

mbfrahry Sep 19, 2017

tombuildsstuff Sep 19, 2017

tombuildsstuff commented Sep 19, 2017

mbfrahry left a comment

elemantalcode commented Sep 26, 2017

pixelicous commented Feb 20, 2018

tombuildsstuff commented Feb 20, 2018

pixelicous commented Feb 20, 2018

ghost commented Mar 31, 2020


		Manages an App Service (within an App Service Plan).

		## Example Usage


		* `connection_string` - (Optional) An `connection_string` block as defined below.

		* `client_affinity_enabled` - (Optional) TODO: COMPLETE ME. Changing this forces a new resource to be created.


		* `client_affinity_enabled` - (Optional) TODO: COMPLETE ME. Changing this forces a new resource to be created.

		* `enabled` - (Optional) TODO: COMPLETE ME. Changing this forces a new resource to be created.


		---

		# azurerm\_app\_service

New Resource: azurerm_app_service #344

New Resource: azurerm_app_service #344

Conversation

tombuildsstuff commented Sep 18, 2017 • edited Loading

tombuildsstuff commented Sep 18, 2017

stack72 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff Sep 19, 2017 • edited Loading

Choose a reason for hiding this comment

mbfrahry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff commented Sep 19, 2017

mbfrahry left a comment

Choose a reason for hiding this comment

elemantalcode commented Sep 26, 2017

pixelicous commented Feb 20, 2018

tombuildsstuff commented Feb 20, 2018

pixelicous commented Feb 20, 2018

ghost commented Mar 31, 2020

New Resource: `azurerm_app_service` #344

New Resource: `azurerm_app_service` #344

tombuildsstuff commented Sep 18, 2017 •

edited

Loading

tombuildsstuff Sep 19, 2017 •

edited

Loading