Skip to content

azurerm_function_app_flex_consumption - add function app backend storage option#29099

Open
xiaxyi wants to merge 6 commits intohashicorp:mainfrom
xiaxyi:flex-consumption/storage-setting
Open

azurerm_function_app_flex_consumption - add function app backend storage option#29099
xiaxyi wants to merge 6 commits intohashicorp:mainfrom
xiaxyi:flex-consumption/storage-setting

Conversation

@xiaxyi
Copy link
Copy Markdown
Collaborator

@xiaxyi xiaxyi commented Mar 18, 2025

Community Note

  • Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
  • Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

There are two types of storage account in flex consumption apps:
- Backend storage
- Deployment storage

In 4.0, there are 5 storage relate properties
storage_container_type
storage_container_endpoint
storage_authentication_type
storage_access_key
storage_user_assigned_identity_id
Both backend and deployment storage is defined by them, they will be used to calculate the AzureWebJobStorage which is used by backend storage and DEPLOYMENT_STORAGE_CONNECTION_STRING which is used by deployment storage.

In 4.0, if user doesn't specify the backend_storage, the AzureWebJobStorage will be calculated by
storage_access_key and storage_container_endpoint, otherwise, be calculated based in backend_storage block.
In 4.0 user can only choose deployment_storage or the old 5 storage related properties.

In 5.0, backend_storage and deployment_storage will be introduced and the old storage account properties will be deprecated.

PR Checklist

  • I have followed the guidelines in our Contributing Documentation.
  • I have checked to ensure there aren't other open Pull Requests for the same update/change.
  • I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
  • I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
  • I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
    For example: “resource_name_here - description of change e.g. adding property new_property_name_here

Changes to existing Resource / Data Source

  • I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
  • I have written new tests for my resource or datasource changes & updated any relevant documentation.
  • I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
  • (For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

  • My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

  • azurerm_resource - support for the thing1 property [GH-00000]

This is a (please select all that apply):

  • Bug Fix
  • New Feature (ie adding a service, resource, or data source)
  • Enhancement
  • Breaking Change

Related Issue(s)

Related to #28199 enhancement

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

@nzthiago
Copy link
Copy Markdown

@stephybun in case you can help review this PR. We have a policy for all our samples for Flex Consumption to use managed identity and I am waiting for this PR to get reviewed before updating them.

@nzthiago
Copy link
Copy Markdown

nzthiago commented Apr 2, 2025

@rcskosir appreciate any help getting this one moving. We need to configure managed indentity in all our samples so waiting for this to complete before updating them on https://aka.ms/flexconsumption/samples

Copy link
Copy Markdown
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Looks like all tests are not failing with

------- Stdout: -------
=== RUN   TestAccFunctionAppFlexConsumption_appSettings
=== PAUSE TestAccFunctionAppFlexConsumption_appSettings
=== CONT  TestAccFunctionAppFlexConsumption_appSettings
    testcase.go:173: Step 1/3 error: After applying this test step, the non-refresh plan was not empty.
        stdout:
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # azurerm_application_insights.test will be updated in-place
          ~ resource "azurerm_application_insights" "test" {
                id                                    = "/subscriptions/*******/resourceGroups/acctestRG-LFA-250421185620141720/providers/Microsoft.Insights/components/acctestappinsights-250421185620141720"
                name                                  = "acctestappinsights-250421185620141720"
              - workspace_id                          = "/subscriptions/*******/resourceGroups/ai_acctestappinsights-250421185620141720_20fdd8c2-be14-42fa-9625-31aa0b60e41b_managed/providers/Microsoft.OperationalInsights/workspaces/managed-acctestappinsights-250421185620141720-ws" -> null
                # (15 unchanged attributes hidden)
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccFunctionAppFlexConsumption_appSettings (238.55s)
FAIL

due to i presume a service change, this will need to be fixed before this pr can be merged

@xiaxyi
Copy link
Copy Markdown
Collaborator Author

xiaxyi commented Apr 23, 2025

@katbyte The failure was due to api update from the app insight side, I updated the test cases.

@lukasz-mitka
Copy link
Copy Markdown

@katbyte could you please take a look again?

@katbyte katbyte removed their assignment Jun 30, 2025
Copy link
Copy Markdown
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Hi @xiaxyi thanks for your work on this PR. I had a look through and suggested a couple of changes, but once those are addressed I can take another look. Thanks!

Description: "The ID of the App Service Plan within which to create this Function App",
},

"function_app_storage_account_name": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we remove function_app prefix from this property as this should be evident from the resource name?

Suggested change
"function_app_storage_account_name": {
"storage_account_name": {

Copy link
Copy Markdown

@nzthiago nzthiago Jul 1, 2025

Choose a reason for hiding this comment

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

There are two storage accounts that can be configured - one for the function app's own workings - translates to AzureWebJobsStorage app setting(s) - and there's a separate configuration for the storage account and container for the deployment where the customer zip file would be placed. They can be the same or they can be different.

@xiaxyi - my recommendation would be:
storage_account_name, storage_account_access_key, storage_uses_managed_identity, storage_key_vault_secret_id, storage_user_assigned_identity_id (as recommended by @catriona-m ) - would apply to the first one related to AzureWebJobsStorage.

Then we rename the deployment ones to include deployment_ prefix:
deployment_storage_container_type, deployment_storage_container_endpoint, deployment_storage_authentication_type, deployment_storage_access_key, deployment_storage_user_assigned_identity_id (note this last one doesn't seem to exist already @xiaxyi , we need it as the user identity for deployment can be different from the main function identity given you can assign multiple identities to the same app)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@nzthiago Thanks for the comment, your suggestions totally make sense. However, I'm afraid that we can't change the naming of an existing property because it will cause breaking changes for existing users, unless in a major release. can we @catriona-m ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@catriona-m , Thanks for the comment, as @nzthiago mentioned, there are two types of storage account that can be configured for the flex consumption app; One is the storage account that will be accessed by the function app for the runtime related tasks, the other is the deployment storage account that holds user's function app's deployment related tasks. These two storage accounts can be different, so we need to differentiate them. I think what @nzthiago suggested may be the way, but we can't change the existing name due to breaking changes, so I'm suggesting, we stick to the current naming and I will add the 6.0 flag to these two storage account name, WDYT?

},
},

"function_app_storage_account_access_key": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"function_app_storage_account_access_key": {
"storage_account_access_key": {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

commented above

Description: "The access key which will be used to access the storage account for the Function App.",
},

"function_app_storage_uses_managed_identity": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"function_app_storage_uses_managed_identity": {
"storage_uses_managed_identity": {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

commented above

Description: "Should the Function App use its Managed Identity to access storage?",
},

"function_app_storage_key_vault_secret_id": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"function_app_storage_key_vault_secret_id": {
"storage_key_vault_secret_id": {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

commented above

"key_vault_reference_identity_id": {
Type: pluginsdk.TypeString,
Optional: true,
Computed: true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if a computed value is returned for this property, could we leave a comment explaining why this is computed using // NOTE: O+C or remove Computed: true, if this is not necessary

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated locally

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it looks like this comment wasn't addressed?

storage_container_endpoint = "${azurerm_storage_account.test.primary_blob_endpoint}${azurerm_storage_container.test.name}"
storage_authentication_type = "StorageAccountConnectionString"
storage_access_key = azurerm_storage_account.test.primary_access_key
storage_access_key = azurerm_storage_account.test.primary_connection_string
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is there a reason for changing these in the existing tests?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated locally

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it looks like this comment wasn't addressed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@catriona-m for this one, the actually value being used is connection string not the access key. I will make another PR to address the name

@xiaxyi
Copy link
Copy Markdown
Collaborator Author

xiaxyi commented Jul 9, 2025

@catriona-m my response is added inline, let me know your thoughts

Copy link
Copy Markdown
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

thanks @xiaxyi - it looks like a couple of my original comments got missed if you could take another look? Thanks!

"key_vault_reference_identity_id": {
Type: pluginsdk.TypeString,
Optional: true,
Computed: true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it looks like this comment wasn't addressed?

storage_container_endpoint = "${azurerm_storage_account.test.primary_blob_endpoint}${azurerm_storage_container.test.name}"
storage_authentication_type = "StorageAccountConnectionString"
storage_access_key = azurerm_storage_account.test.primary_access_key
storage_access_key = azurerm_storage_account.test.primary_connection_string
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it looks like this comment wasn't addressed?

@xiaxyi
Copy link
Copy Markdown
Collaborator Author

xiaxyi commented Jul 17, 2025

@catriona-m Thanks for the comment, I've modified the storage account name per your suggestion and I will make another PR to address the name better.

--- PASS: TestAccFunctionAppFlexConsumption_backendStorageUsingKeyValutString (1239.22s)
PASS
ok github.com/hashicorp/terraform-provider-azurerm/internal/services/appservice 1266.925s

Comment on lines +716 to +723
functionAppStorageAccountString := functionAppFlexConsumption.StorageAccountName
if !functionAppFlexConsumption.StorageAccountUsesMSI {
if functionAppFlexConsumption.StorageAccountKeyVaultSecretID != "" {
functionAppStorageAccountString = fmt.Sprintf(helpers.StorageStringFmtKV, functionAppFlexConsumption.StorageAccountKeyVaultSecretID)
} else if functionAppStorageAccountString != "" {
functionAppStorageAccountString = fmt.Sprintf(helpers.StorageStringFmt, functionAppFlexConsumption.StorageAccountName, functionAppFlexConsumption.StorageAccountAccessKey, *storageDomainSuffix)
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This may be more readable / maintainable as a switch statement.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @wyattfry for the comment, Would you mind be more specific about which variable to put in the switch condition? This code is calculating the storage string based on the other two properties, just like azurerm_linux_function_app did.

Type: &storageAuthType,
}

endpoint := strings.TrimPrefix(deploymentSaEndpoint, "https://")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The manual string parsing I feel would be very easy to break. What if we used "net/url"? Something maybe like this:

	u, err := url.Parse(deploymentSaEndpoint)
	if err != nil {
		return (err)
	}
    storageName := strings.Split(u.Host, ".")[0]

You could also move the parsing implementation to the package's parsing sub-package. Then you could add validation and unit tests to make sure it works as expected.

}

endpoint := strings.TrimPrefix(deploymentSaEndpoint, "https://")
var deploymentSaConncString string
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The name seems a little unconventional, maybe this?

Suggested change
var deploymentSaConncString string
var deploymentSaConnString string


if functionAppFlexConsumption.StorageAuthType == string(webapps.AuthenticationTypeStorageAccountConnectionString) {
if functionAppFlexConsumption.StorageAccessKey == "" {
if storageAuthType == webapps.AuthenticationTypeStorageAccountConnectionString {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For these two checks, I feel like a CustomizeDiff() would be a better place for them to live.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @wyattfry for the comment, the storage_authentication_type is defined as computed, can we still define the checks in CustomizeDiff ()?

Comment on lines +1142 to +1145
appSettingsResp, err := client.ListApplicationSettings(ctx, *id)
if err != nil {
return fmt.Errorf("retrieving App Settings for %s: %+v", id, err)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could this be moved closer to where it's used? Around line 1221?

Comment on lines +1207 to +1209
storageConnStringForFCApp := ""
storageConnStringForFcAppValue := ""
var deploymentSaName, deploymentSaKey, backendStorageString string
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could condense / neaten even further...

Suggested change
storageConnStringForFCApp := ""
storageConnStringForFcAppValue := ""
var deploymentSaName, deploymentSaKey, backendStorageString string
var (
storageConnStringForFCApp,
storageConnStringForFcAppValue,
deploymentSaName,
deploymentSaKey,
backendStorageString string
)

deploymentSaKey = state.DeploymentStorageAccessKey
}

endpoint := strings.TrimPrefix(deploymentSaEndpoint, "https://")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See above about URL parsing.

}
if state.StorageAuthType == string(webapps.AuthenticationTypeStorageAccountConnectionString) {
if state.StorageAccessKey == "" {
if storageAuthType == webapps.AuthenticationTypeStorageAccountConnectionString {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See note above about CustomizeDiff

if StorageUserAssignedIdentityID == "" {
return fmt.Errorf("the user assigned identity id must be specified when using the user assigned identity to access the storage account")
}
storageAuth.UserAssignedIdentityResourceId = &StorageUserAssignedIdentityID
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know you declare this variable a couple lines above, but for safety, it may be worth using pointer.To

Suggested change
storageAuth.UserAssignedIdentityResourceId = &StorageUserAssignedIdentityID
storageAuth.UserAssignedIdentityResourceId = pointer.To(StorageUserAssignedIdentityID)

storageAuth := webapps.FunctionsDeploymentStorageAuthentication{
Type: &storageAuthType,
}
if state.DeploymentStorageAuthType == string(webapps.AuthenticationTypeStorageAccountConnectionString) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See note above about CustomizeDiff


// Note: We process this regardless to give us a "clean" view of service-side app_settings, so we can reconcile the user-defined entries later
siteConfig, err := helpers.ExpandSiteConfigFunctionFlexConsumptionApp(state.SiteConfig, model.Properties.SiteConfig, metadata, false, storageString, storageConnStringForFCApp)
siteConfig, err := helpers.ExpandSiteConfigFunctionFlexConsumptionApp(state.SiteConfig, model.Properties.SiteConfig, metadata, state.StorageAccountUsesMSI, backendStorageString, storageConnStringForFCApp, storageConnStringForFcAppValue)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought it was a nice move when previously you put each argument on its own line, maybe here too?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @wyattfry for this comment, would you mind be more specific about it?

Copy link
Copy Markdown
Contributor

@wyattfry wyattfry left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I left some notes for you to consider. About the acceptance tests, I had a couple suggestions:

  1. you can remove the check.That(...)s other than the ExistsInAzure(r), the test runner automatically detects differences between the actual resource and the configuration.
  2. rather than writing two functions for every test condition, one for the beta, one not; a pattern I like to use is to have only one test function, but in the configuration function, have it return either version depending on the flag.
    Example: https://github.com/hashicorp/terraform-provider-azurerm/blob/main/internal/services/loganalytics/log_analytics_workspace_resource_test.go#L802

@xiaxyi
Copy link
Copy Markdown
Collaborator Author

xiaxyi commented Oct 7, 2025

Thanks @wyattfry for the comment! The concern of remodeling the test cases as what you suggested would be the deployment_storage_* won't be covered in 4.0. As deployment_storage_* can be set in 4.0, we need to also test it.

Copy link
Copy Markdown
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Hi @xiaxyi it looks like there are some comments from a previous review still waiting to be addressed here and some conflicts to be resolved. Once those are fixed up, I can take another look, thanks!

@xiaxyi
Copy link
Copy Markdown
Collaborator Author

xiaxyi commented Nov 12, 2025

Thanks @wyattfry for the review! I updated the code and some of the comments, would you mind taking another look and let me know your suggestions?

Copy link
Copy Markdown
Collaborator

@sreallymatt sreallymatt left a comment

Choose a reason for hiding this comment

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

Hi @xiaxyi, before I give this a more thorough review, I had a question/suggestion regarding the schema. Based on the description and documentation, there are 2 sets of storage related properties, one for backend storage and another for deployment storage.

Instead of having many top level properties, this may be easier to use if there are 2 storage blocks and the properties are contained within, e.g. deployment_storage and backend_storage, WDYT?

@xiaxyi
Copy link
Copy Markdown
Collaborator Author

xiaxyi commented Nov 30, 2025

Thanks @sreallymatt , I think your suggestion totally makes sense. Actually, this was my first thought, however, during my previous practices, I was suggested to use the top-level for this kind of propertice. Would you like me to change to use the block?

@sreallymatt
Copy link
Copy Markdown
Collaborator

Hi @xiaxyi - apologies for the delayed response, I missed this mention.

From a UX perspective, having a logical grouping for the properties by using blocks is a good approach, the main difficulty I can see is the fact that we currently have top-level properties for part of it, so this may need a DiffSuppressFunc or O+C for 4.x on some of those, and the top-level args can be deprecated and removed in 5.0.

@xiaxyi
Copy link
Copy Markdown
Collaborator Author

xiaxyi commented Apr 16, 2026

thanks @sreallymatt , I remodelled the schema and now there are two storage blocksdeployment_storage and backend_storage. The rest of the 4.0 storage_* properties will be deprecated in 5.0.

Copy link
Copy Markdown
Collaborator

@wuxu92 wuxu92 left a comment

Choose a reason for hiding this comment

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

Thanks for the update! I left some comments once these are addressed we can have another look.

"tags": commonschema.Tags(),
}
if !features.FivePointOh() {
schema["backend_storage"] = &pluginsdk.Schema{
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

backend_storage is the new feature to add, we don't have to mark it as Computed in 4.x, rigth?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's a new feature to add, but we still need to make it as computed because it's calculated from AzureWebJobStorage.

},
},
},
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
}
}

"container_endpoint": {
Type: pluginsdk.TypeString,
Required: true,
Description: "The endpoint of the storage container where the function app's code is hosted.",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add a validation?

Suggested change
Description: "The endpoint of the storage container where the function app's code is hosted.",
Description: "The endpoint of the storage container where the function app's code is hosted.",
ValidateFunc: validation.StringIsNotEmpty,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

added

Comment on lines +501 to +548
schema["deployment_storage"] = &pluginsdk.Schema{
Type: pluginsdk.TypeList,
Optional: true,
Computed: true,
MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"container_type": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice([]string{
string(webapps.FunctionsDeploymentStorageTypeBlobContainer),
}, false),
Description: "The type of the storage container where the function app's code is hosted. Only `blobContainer` is supported currently.",
},

"container_endpoint": {
Type: pluginsdk.TypeString,
Required: true,
Description: "The endpoint of the storage container where the function app's code is hosted.",
},

"authentication_type": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice([]string{
string(webapps.AuthenticationTypeSystemAssignedIdentity),
string(webapps.AuthenticationTypeStorageAccountConnectionString),
string(webapps.AuthenticationTypeUserAssignedIdentity),
}, false),
},

"access_key": {
Type: pluginsdk.TypeString,
Optional: true,
Sensitive: true,
ValidateFunc: validation.StringIsNotEmpty,
},

"user_assigned_identity_id": {
Type: pluginsdk.TypeString,
Optional: true,
ValidateFunc: commonids.ValidateUserAssignedIdentityID,
},
},
},
ExactlyOneOf: []string{"storage_container_type", "deployment_storage"},
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why not directly set Computed:

Suggested change
schema["deployment_storage"] = &pluginsdk.Schema{
Type: pluginsdk.TypeList,
Optional: true,
Computed: true,
MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"container_type": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice([]string{
string(webapps.FunctionsDeploymentStorageTypeBlobContainer),
}, false),
Description: "The type of the storage container where the function app's code is hosted. Only `blobContainer` is supported currently.",
},
"container_endpoint": {
Type: pluginsdk.TypeString,
Required: true,
Description: "The endpoint of the storage container where the function app's code is hosted.",
},
"authentication_type": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice([]string{
string(webapps.AuthenticationTypeSystemAssignedIdentity),
string(webapps.AuthenticationTypeStorageAccountConnectionString),
string(webapps.AuthenticationTypeUserAssignedIdentity),
}, false),
},
"access_key": {
Type: pluginsdk.TypeString,
Optional: true,
Sensitive: true,
ValidateFunc: validation.StringIsNotEmpty,
},
"user_assigned_identity_id": {
Type: pluginsdk.TypeString,
Optional: true,
ValidateFunc: commonids.ValidateUserAssignedIdentityID,
},
},
},
ExactlyOneOf: []string{"storage_container_type", "deployment_storage"},
}
schema["deployment_storage"].Computed = true
schema["deployment_storage"].Required = false
schema["deployment_storage"].Optional = true
schema["deployment_storage"].ExactlyOneOf = []string{"storage_container_type", "deployment_storage"}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated

Comment on lines +1454 to +1457
endpoint, err := url.Parse(v.ContainerEndPoint)
if err != nil {
return nil, deploymentSaConnectionStr, fmt.Errorf("parsing storage container endpoint error, the expected format is https://storagename.blob.core.windows.net/containername, the received value is %s", v.ContainerEndPoint)
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if ContainerEndpoint has to be an url, we need to update the Schema.ValidateFunc rather than validate in the exapnd function. so we don't need the err return value in this method.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated

Comment on lines +1484 to +1487
deploymentStorageList := make([]DeploymentStorage, 0)
if deploymentSa == nil {
return deploymentStorageList
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
deploymentStorageList := make([]DeploymentStorage, 0)
if deploymentSa == nil {
return deploymentStorageList
}
if deploymentSa == nil {
return []DeploymentStorage{}
}

Comment on lines +1492 to +1500
if deploymentSa.Authentication != nil && deploymentSa.Authentication.Type != nil {
ds.AuthenticationType = string(pointer.From(deploymentSa.Authentication.Type))
if deploymentSa.Authentication.Type != nil && pointer.From(deploymentSa.Authentication.Type) == webapps.AuthenticationTypeStorageAccountConnectionString {
_, ds.AccessKey = helpers.ParseWebJobsStorageString(deploymentSaString)
}
if deploymentSa.Authentication != nil && deploymentSa.Authentication.UserAssignedIdentityResourceId != nil {
ds.UserAssignedIdentityId = pointer.From(deploymentSa.Authentication.UserAssignedIdentityResourceId)
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nil check for deploymentSa.Authentication is done in the first if condition, we can make this simple:

Suggested change
if deploymentSa.Authentication != nil && deploymentSa.Authentication.Type != nil {
ds.AuthenticationType = string(pointer.From(deploymentSa.Authentication.Type))
if deploymentSa.Authentication.Type != nil && pointer.From(deploymentSa.Authentication.Type) == webapps.AuthenticationTypeStorageAccountConnectionString {
_, ds.AccessKey = helpers.ParseWebJobsStorageString(deploymentSaString)
}
if deploymentSa.Authentication != nil && deploymentSa.Authentication.UserAssignedIdentityResourceId != nil {
ds.UserAssignedIdentityId = pointer.From(deploymentSa.Authentication.UserAssignedIdentityResourceId)
}
}
if deploymentSa.Authentication != nil {
ds.AuthenticationType = string(pointer.From(deploymentSa.Authentication.Type))
if pointer.From(deploymentSa.Authentication.Type) == webapps.AuthenticationTypeStorageAccountConnectionString {
_, ds.AccessKey = helpers.ParseWebJobsStorageString(deploymentSaString)
}
if uai := deploymentSa.Authentication.UserAssignedIdentityResourceId; uai != nil {
ds.UserAssignedIdentityId = pointer.From(uai)
}
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated


func FlattenBackendStorage(input string, backendStorageUseMsi bool) []BackendStorage {
backendStorageList := make([]BackendStorage, 0)
backendStorageString := input
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why not directly name the first parameter as storageString

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated, removed the variable backendStorageString and used input directly


func FlattenStorageConnectionStrings(input webapps.StringDictionary) (backendStorageConnString string, backendStorageUseMsi bool, deploymentStorageConnString string) {
if input.Properties == nil {
return "", false, ""
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

simplify named return value with default value

Suggested change
return "", false, ""
return

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated

deploymentStorageConnString = v
}
}
return backendStorageConnString, backendStorageUseMsi, deploymentStorageConnString
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

return with named return values

Suggested change
return backendStorageConnString, backendStorageUseMsi, deploymentStorageConnString
return

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

updated

@xiaxyi
Copy link
Copy Markdown
Collaborator Author

xiaxyi commented Apr 17, 2026

Thanks @wuxu92 for review! All the comments are updated and acc tests passed:


--- PASS: TestAccFunctionAppFlexConsumption_alwaysReadyInstanceCountError (169.56s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhAppSettings (339.04s)
--- PASS: TestAccFunctionAppFlexConsumption_runtimeNode (341.58s)
--- PASS: TestAccFunctionAppFlexConsumption_runtimePowerShell (346.21s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhComplete (348.77s)
--- PASS: TestAccFunctionAppFlexConsumption_complete (358.37s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhStickySettings (359.07s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhBackendStorageUseMsi (229.13s)
--- PASS: TestAccFunctionAppFlexConsumption_vNetIntegrationWithVnetProperties ()
--- PASS: 
--- PASS: TestAccFunctionAppFlexConsumption_userAssignedIdentityUpdate (595.86s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhConnectionString (306.50s)
--- PASS: TestAccFunctionAppFlexConsumption_runtimeDotNet (294.20s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhDeploymentStorageSchemaUpdate (742.55s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhDeploymentStorageSystemIdentitySchemaUpdate (753.55s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhConnectionStringUpdate (758.25s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhMaxInstanceCount (821.25s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhAlwaysReadyUpdate (535.25s)
--- PASS: TestAccTestAccFunctionAppFlexConsumption_authV2Update (965.72s)
--- PASS: TestAccFunctionAppFlexConsumption_alwaysReadyUpdateName (983.03s)
--- PASS: TestAccFunctionAppFlexConsumption_runtimeJava (590.29s)
--- PASS: TestAccFunctionAppFlexConsumption_stickySettings (304.00s)
--- PASS: TestAccFunctionAppFlexConsumption_backendStorageUsingKeyVaultString (1048.90s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhDeploymentStorageUpdate (781.33s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhAddBackendStorage (538.28s)
--- PASS: TestAccFunctionAppFlexConsumption_runtimePython (316.29s)
--- PASS: TestAccFunctionAppFlexConsumption_runtimeJavaUpdate (752.40s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhAlwaysReadyUpdateName (951.05s)
--- PASS: TestAccFunctionAppFlexConsumption_connectionString (319.19s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhBackendStorageUseKeyVault (962.81s)
--- PASS: TestAccFunctionAppFlexConsumption_userAssignedIdentity (291.22s)
--- PASS: TestAccFunctionAppFlexConsumption_systemAssignedIdentity (307.22s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhBackendStorageUpdate (745.61s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhBasic (318.81s)
--- PASS: TestAccFunctionAppFlexConsumption_connectionStringUpdate (737.34s)
--- PASS: TestAccFunctionAppFlexConsumption_appSettingsUpdate (750.57s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhRuntimePython (296.92s)
--- PASS: TestAccFunctionAppFlexConsumption_appSettings (307.21s)
--- PASS: TestAccFunctionAppFlexConsumption_basic (305.48s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhDeploymentStorageUaiSchemaUpdate (964.78s)
--- PASS: TestAccFunctionAppFlexConsumption_backendStorageUseMsi (351.74s)
--- PASS: TestAccFunctionAppFlexConsumption_alwaysReadyUpdate (619.55s)
--- PASS: TestAccFunctionAppFlexConsumption_basicEnableAiDetector (912.18s)
--- PASS: TestAccFunctionAppFlexConsumption_maxInstanceCount (839.48s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhvNetIntegrationWithVnetProperties (412.04s)
--- PASS: TestAccFunctionAppFlexConsumption_httpsOnlyUpdate (797.06s)
--- PASS: TestAccFunctionAppFlexConsumption_httpConcurrencyUpdate (1022.97s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhInstanceMemoryUpdate (783.13s)
--- PASS: TestAccFunctionAppFlexConsumption_backendStorageUpdate (605.80s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhAppSettingsUpdate (770.66s)
--- PASS: TestAccFunctionAppFlexConsumption_instanceMemoryUpdate (836.68s)
--- PASS: TestAccFunctionAppFlexConsumption_withAuthSettings (1400.65s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhHttpsOnlyUpdate (750.22s)
--- PASS: TestAccFunctionAppFlexConsumption_deploymentStorageUpdate (736.69s)
--- PASS: TestAccFunctionAppFlexConsumption_FourPointOhHttpConcurrencyUpdate (911.93s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/appservice    2533.351s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

azurerm_function_app_flex_consumption re-injects AzureWebJobsStorage into app_settings

9 participants