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.

Sign up for GitHub

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

Jump to bottom

azurerm_function_app - storage_connection_string can now rotate keys but still a require recreate when changing the account #5645

Closed
wants to merge 1 commit into from
Closed

azurerm_function_app - storage_connection_string can now rotate keys but still a require recreate when changing the account #5645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion azurerm/internal/services/web/resource_arm_function_app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ func resourceArmFunctionApp() *schema.Resource {
"storage_connection_string": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Sensitive: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

whilst this'd work it'd cause a bunch of runtime errors should someone opt to change the account being used - as such I'm wondering if we'd be better to split this field into two:

  1. "storage_account_id" for the ID of the Storage Account in question - which is ForceNew
  2. the "storage account access key" which is either the primary/secondary key and allows updating

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I love that idea! It makes more sense to me. Should I make this change with 2.0 in mind and remove the old property?

Copy link
Collaborator

Choose a reason for hiding this comment

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

now that we are post 2.0 what we will have to do is deprecate the old one and make them conflict.


Expand Down Expand Up @@ -429,6 +428,16 @@ func resourceArmFunctionAppUpdate(d *schema.ResourceData, meta interface{}) erro
}
}

if d.HasChange("storage_connection_string") {
oldValue, newValue := d.GetChange("storage_connection_string")
oldAccount := getAccountName(fmt.Sprintf("%s", oldValue))
newAccount := getAccountName(fmt.Sprintf("%s", newValue))

if oldAccount != newAccount {
return fmt.Errorf("Error updating storage_connection_string because the AccountName cannot be changed. To change the storage account the resource must be tainted.")
}
}

future, err := client.CreateOrUpdate(ctx, resGroup, name, siteEnvelope)
if err != nil {
return err
Expand Down Expand Up @@ -869,3 +878,14 @@ func flattenFunctionAppSiteCredential(input *web.UserProperties) []interface{} {

return append(results, result)
}

func getAccountName(storageConnectionString string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

in Golang functions are shared across the entire package - as such this method would probably be better named as:

Suggested change
func getAccountName(storageConnectionString string) string {
func getAccountNameFromStorageConnectionString(storageConnectionString string) string {

connectionStringArray := strings.SplitN(storageConnectionString, ";", -1)
connectionStringMap := make(map[string]string)
for _, pair := range connectionStringArray {
z := strings.Split(pair, "=")
connectionStringMap[z[0]] = z[1]
}

return connectionStringMap["AccountName"]
}
2 changes: 1 addition & 1 deletion website/docs/r/function_app.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ The following arguments are supported:

* `app_service_plan_id` - (Required) The ID of the App Service Plan within which to create this Function App.

* `storage_connection_string` - (Required) The connection string of the backend storage account which will be used by this Function App (such as the dashboard, logs).
* `storage_connection_string` - (Required) The connection string of the backend storage account which will be used by this Function App (such as the dashboard, logs). The access key can be updated, but changing the storage account requires a taint and a recreate to take place.

* `app_settings` - (Optional) A key-value pair of App Settings.

Expand Down