-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
azurerm_function_app
- add support for the always_on
and connection_string
fields
#695
azurerm_function_app
- add support for the always_on
and connection_string
fields
#695
Conversation
da0d72d
to
29ae3a0
Compare
29ae3a0
to
2121c1a
Compare
I've added also the support for custom |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @cloudify
Thanks for this PR - I've taken a look through and left a couple of comments in-line but this otherwise LGTM :)
Thanks!
azurerm/resource_arm_function_app.go
Outdated
@@ -106,6 +159,7 @@ func resourceArmFunctionAppCreate(d *schema.ResourceData, meta interface{}) erro | |||
ServerFarmID: utils.String(appServicePlanID), | |||
Enabled: utils.Bool(enabled), | |||
SiteConfig: &web.SiteConfig{ | |||
AlwaysOn: siteConfig.AlwaysOn, | |||
AppSettings: &basicAppSettings, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this object is actually being returned, rather than mapping each individual object across can we assign the existing AppSettings object to the returned siteConfig
object (for instance):
siteConfig := expandFunctionAppSiteConfig(d)
siteConfig.AppSettings = basicAppSettings
# ...
siteEnvelope := web.Site{
# ...
Enabled: utils.Bool(enabled),
SiteConfig: &siteConfig,
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -94,6 +94,52 @@ func TestAccAzureRMFunctionApp_appSettings(t *testing.T) { | |||
}) | |||
} | |||
|
|||
func TestAccAzureRMFunctionApp_siteConfig(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add an additional test to set an app setting, then enable always on (as a separate step) and then add a connection string to have an end-to-end test? Multiple test steps can be achieved by specifying multiple steps below, like so:
Steps: []resource.TestStep{
{
Config: config,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMFunctionAppExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "site_config.0.always_on", "true"),
),
},
{
Config: updatedConfig,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMFunctionAppExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "site_config.0.always_on", "false"),
),
},
},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
817022b
to
2780a7d
Compare
…to function_app
2780a7d
to
02fef14
Compare
@tombuildsstuff I've fixed the conflict and taken care of your suggestions |
Hey @cloudify Thanks for pushing those updates - apologies for the delayed re-review here, I'm just taking a look into this now. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @cloudify
Thanks for pushing those updates - I'll kick off the test suite now - but this now LGTM 👍
Thanks!
azurerm_function_app
- add support for the always_on
and connection_string
fields
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! |
Hello @tombuildsstuff,
my attempt to fix #691, I've added the
always_on
attribute for now, if my approach is good for you I'll add the rest,thanks