azurerm_function_app - storage_connection_string can now rotate keys but still a require recreate when changing the account
#5645
Conversation
ghost
added
kraihn
force-pushed
the
bugfix/function-app-storage-credentials
branch
from
fcbcbb6
to
9bb3ce6
Compare
There was a problem hiding this comment.
hey @kraihn
Thanks for this PR :)
I've taken a look through and left a couple of comments inline about the design of this resource - ultimately I'm wondering if it'd be worth splitting the storage_connection_string field into two to be able to apply different rules to each of the segments - WDYT?
Thanks!
| @@ -78,7 +78,6 @@ func resourceArmFunctionApp() *schema.Resource { | |||
| "storage_connection_string": { | |||
| Type: schema.TypeString, | |||
| Required: true, | |||
| ForceNew: true, | |||
| Sensitive: true, | |||
| }, | |||
There was a problem hiding this comment.
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:
- "storage_account_id" for the ID of the Storage Account in question - which is ForceNew
- the "storage account access key" which is either the primary/secondary key and allows updating
WDYT?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
now that we are post 2.0 what we will have to do is deprecate the old one and make them conflict.
| @@ -869,3 +878,14 @@ func flattenFunctionAppSiteCredential(input *web.UserProperties) []interface{} { | |||
|
|
|||
| return append(results, result) | |||
| } | |||
|
|
|||
| func getAccountName(storageConnectionString string) string { | |||
There was a problem hiding this comment.
in Golang functions are shared across the entire package - as such this method would probably be better named as:
| func getAccountName(storageConnectionString string) string { | |
| func getAccountNameFromStorageConnectionString(storageConnectionString string) string { |
| @@ -78,7 +78,6 @@ func resourceArmFunctionApp() *schema.Resource { | |||
| "storage_connection_string": { | |||
| Type: schema.TypeString, | |||
| Required: true, | |||
| ForceNew: true, | |||
| Sensitive: true, | |||
| }, | |||
There was a problem hiding this comment.
now that we are post 2.0 what we will have to do is deprecate the old one and make them conflict.
|
@kraihnm, going to close this in favour of #6304. thanks for opening the PR! |
|
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! |
Fixes #5435