New Beta Resources - azurerm_linux_function_app_slot and azurerm_windows_function_app_slot
#14940
Conversation
jackofallops
force-pushed
the
f/win-linux-func-app-slots
branch
from
47aa6ba to
0e3b379
Compare
| }, | ||
| "retention_period_days": { | ||
| Type: pluginsdk.TypeInt, | ||
| Optional: true, | ||
| ValidateFunc: validation.IntBetween(0, 99999), | ||
| Description: "The retention period for logs in days. Valid values are between `0` and `99999`. Defaults to `0` (never delete).", |
There was a problem hiding this comment.
Do we want to add Default: 0? Or is that taken care of because ints already default to 0? It might be worth adding just to keep it in line with how we normally default things?
There was a problem hiding this comment.
iirc the default is "not set", so defining a default would possibly change the behaviour of the service... I'll add some tests around this to confirm.
|
|
||
| "scm_ip_restriction": IpRestrictionSchema(), | ||
|
|
||
| "load_balancing_mode": { // Supported on Function Apps? |
There was a problem hiding this comment.
Let's check that this works before merging
There was a problem hiding this comment.
I've tested this manually, and it does work (or at least is successfully reflected in the resource configuration), just needs coverage in the acceptance tests I think.
|
|
||
| "scm_ip_restriction": IpRestrictionSchema(), | ||
|
|
||
| "load_balancing_mode": { // Supported on Function Apps? |
| Optional: true, | ||
| Computed: true, | ||
| ValidateFunc: validation.IntBetween(2, 10), | ||
| Description: "The amount of time in minutes that a node is unhealthy before being removed from the load balancer. Possible values are between `2` and `10`. Defaults to `10`. Only valid in conjunction with `health_check_path`", |
There was a problem hiding this comment.
I think we can add RequiredWith: []string{"health_check_path"} for one extra layer of validation
There was a problem hiding this comment.
these are not tied together,health_check_path provides an optional "thing" to check for health, otherwise the check just makes sure it gets a 200 from the deployed site.
| Description: "Should all outbound traffic to have Virtual Network Security Groups and User Defined Routes applied? Defaults to `false`.", | ||
| }, | ||
|
|
||
| "detailed_error_logging_enabled": { |
There was a problem hiding this comment.
Just want to confirm that this can't be set
There was a problem hiding this comment.
More that it shouldn't be set. This value is calculated from other configuration elsewhere (specifically the logs blocks)
|
|
||
| "detailed_error_logging_enabled": { | ||
| Type: pluginsdk.TypeBool, | ||
| Computed: true, |
There was a problem hiding this comment.
Looking to confirm we can't set this
|
|
||
| "linux_fx_version": { | ||
| Type: pluginsdk.TypeString, | ||
| Computed: true, |
There was a problem hiding this comment.
And looking to confirm if we can't set this
There was a problem hiding this comment.
as per windows_fx_version above
| switch tier := *planSku.Tier; strings.ToLower(tier) { | ||
| case "dynamic": // Consumption Plan modifications to request | ||
| sendContentSettings = false | ||
| case "elastic": // ElasticPremium Plan modifications to request? |
There was a problem hiding this comment.
Do we need to do anything for this case?
There was a problem hiding this comment.
This switch is being used to call out that there are potentially differences in data sent for different plan types, the intention of leaving in those currently without special modifications is to provide readiness / hints about where/how to do so if needed in future. Since this case is empty, it just drops through, so I felt it was low enough cost to provide hints to future development.
| if _, ok := metadata.ResourceData.GetOk("app_settings.WEBSITE_CONTENTSHARE"); ok { | ||
| appSettings[k] = utils.NormalizeNilableString(v) | ||
| } | ||
| case "WEBSITE_HTTPLOGGING_RETENTION_DAYS": |
There was a problem hiding this comment.
Do we need to do anything here?
There was a problem hiding this comment.
In this case, the switch is used to filter out settings that are configured elsewhere and would otherwise make app_settings impossible to manage correctly in state. Some cases this information is set into the respective locations in the schema, and others (like this) are simply filtered out as the service has a different primary attribute with this information in.
| m.FunctionExtensionsVersion = utils.NormalizeNilableString(v) | ||
|
|
||
| case "WEBSITE_NODE_DEFAULT_VERSION": // Note - This is only set if it's not the default of 12, but we collect it from WindowsFxVersion so can discard it here | ||
| case "WEBSITE_CONTENTAZUREFILECONNECTIONSTRING": |
There was a problem hiding this comment.
Do we need to do anything for these next 4 cases?
jackofallops
force-pushed
the
f/win-linux-func-app-slots
branch
from
0fae3a5 to
a1deeb7
Compare
|
Tests looking good:
|
|
This functionality has been released in v2.95.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Also updates docs and fixes a number of related issues in the [windows|linux]_function_app resources uncovered.