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.

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_linux_function_app, azurerm_linux_function_app_slot - support for the vnet_image_pull_enabled property. #25249

Closed
wants to merge 25 commits into from
Closed
Changes from 18 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5bf320a
adding vnetImagePullEnabled property to linux function app
xiaxyi Mar 14, 2024
4fa4d96
change the default value for ase
xiaxyi Mar 18, 2024
206124e
fix error
xiaxyi Mar 18, 2024
4f7253b
update customize diff func
xiaxyi Mar 18, 2024
62d0f52
update for ase
xiaxyi Mar 19, 2024
f444765
Merge remote-tracking branch 'upstream/main' into appService/addVnetP…
xiaxyi Mar 26, 2024
d958e2c
Merge remote-tracking branch 'upstream/main' into appservice/addVnetP…
xiaxyi Mar 29, 2024
cbbaee5
Merge remote-tracking branch 'upstream/main' into appservice/addVnetP…
xiaxyi Apr 2, 2024
585746d
update test cases to remove the vnetimagepull property
xiaxyi Apr 3, 2024
b3a8079
add 4.0 flag and windows function apps
xiaxyi Apr 3, 2024
c94b55d
fix fmt
xiaxyi Apr 3, 2024
5a6cdf1
fix wrong type issue
xiaxyi Apr 3, 2024
805288d
Merge remote-tracking branch 'upstream/main' into appService/addVnetP…
xiaxyi Apr 5, 2024
e83ec27
fix 4.0 comment for linux function app
xiaxyi Apr 5, 2024
7b5dd68
merging
xiaxyi Apr 5, 2024
36b4b3e
update commit and test case per reviewer's comments
xiaxyi Apr 5, 2024
552e769
sorting imports
xiaxyi Apr 5, 2024
99d5023
fmt
xiaxyi Apr 5, 2024
472f314
Merge remote-tracking branch 'upstream/main' into appservice/addVnetP…
xiaxyi May 28, 2024
1c36adb
add new test cases for the vnet_image_pull_enable property with 4.0 f…
xiaxyi May 30, 2024
ca319a5
Merge remote-tracking branch 'upstream/main' into appservice/addVnetP…
xiaxyi May 30, 2024
caa7c04
Merge remote-tracking branch 'origin/appService/addVnetProperties' in…
xiaxyi May 31, 2024
432dfe5
add 4.0 flag for test cases based on the reviewer's comments
xiaxyi May 31, 2024
a1fa339
add unparam linter
xiaxyi May 31, 2024
4ee6865
Merge remote-tracking branch 'upstream/main' into appservice/addVnetP…
xiaxyi May 31, 2024
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
52 changes: 51 additions & 1 deletion internal/services/appservice/linux_function_app_resource.go
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ import (
"github.com/hashicorp/go-azure-helpers/resourcemanager/location"
"github.com/hashicorp/go-azure-sdk/resource-manager/web/2023-01-01/resourceproviders"
"github.com/hashicorp/go-azure-sdk/resource-manager/web/2023-01-01/webapps"
"github.com/hashicorp/terraform-provider-azurerm/internal/features"
"github.com/hashicorp/terraform-provider-azurerm/internal/sdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/appservice/helpers"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/appservice/migration"
@@ -71,6 +72,8 @@ type LinuxFunctionAppModel struct {
PublishingFTPBasicAuthEnabled bool `tfschema:"ftp_publish_basic_authentication_enabled"`
Identity []identity.ModelSystemAssignedUserAssigned `tfschema:"identity"`

// VnetImagePullEnabled bool `tfschema:"vnet_image_pull_enabled"` // TODO 4.0 not supported on Consumption plans
Copy link
Member

Choose a reason for hiding this comment

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

This can be included now using the additional struct tag

Suggested change
// VnetImagePullEnabled bool `tfschema:"vnet_image_pull_enabled"` // TODO 4.0 not supported on Consumption plans
VnetImagePullEnabled bool `tfschema:"vnet_image_pull_enabled,addedInNextMajorVersion"` // TODO 4.0 not supported on Consumption plans

Can you also look to address the 4.0 TODO here within this PR?


// Computed
CustomDomainVerificationId string `tfschema:"custom_domain_verification_id"`
DefaultHostname string `tfschema:"default_hostname"`
@@ -105,7 +108,7 @@ func (r LinuxFunctionAppResource) IDValidationFunc() pluginsdk.SchemaValidateFun
}

func (r LinuxFunctionAppResource) Arguments() map[string]*pluginsdk.Schema {
return map[string]*pluginsdk.Schema{
s := map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
@@ -305,6 +308,15 @@ func (r LinuxFunctionAppResource) Arguments() map[string]*pluginsdk.Schema {
Description: "The local path and filename of the Zip packaged application to deploy to this Linux Function App. **Note:** Using this value requires either `WEBSITE_RUN_FROM_PACKAGE=1` or `SCM_DO_BUILD_DURING_DEPLOYMENT=true` to be set on the App in `app_settings`.",
},
}
if features.FourPointOhBeta() {
s["vnet_image_pull_enabled"] = &pluginsdk.Schema{
Type: pluginsdk.TypeBool,
Optional: true,
Default: false,
Description: "Is container image pull over virtual network enabled? Defaults to `false`.",
}
}
return s
}

func (r LinuxFunctionAppResource) Attributes() map[string]*pluginsdk.Schema {
@@ -425,6 +437,11 @@ func (r LinuxFunctionAppResource) Create() sdk.ResourceFunc {

availabilityRequest.Name = fmt.Sprintf("%s.%s", functionApp.Name, nameSuffix)
availabilityRequest.IsFqdn = pointer.To(true)
if features.FourPointOhBeta() {
if !metadata.ResourceData.Get("vnet_image_pull_enabled").(bool) {
return fmt.Errorf("`vnet_image_pull_enabled` cannot be disabled for app running in an app service environment.")
}
}
}
}
// Only send for ElasticPremium and Consumption plan
@@ -525,6 +542,9 @@ func (r LinuxFunctionAppResource) Create() sdk.ResourceFunc {
VnetRouteAllEnabled: siteConfig.VnetRouteAllEnabled,
},
}
if features.FourPointOhBeta() {
siteEnvelope.Properties.VnetImagePullEnabled = pointer.To(metadata.ResourceData.Get("vnet_image_pull_enabled").(bool))
}

pna := helpers.PublicNetworkAccessEnabled
if !functionApp.PublicNetworkAccess {
@@ -764,6 +784,9 @@ func (r LinuxFunctionAppResource) Read() sdk.ResourceFunc {
state.DefaultHostname = pointer.From(props.DefaultHostName)
state.PublicNetworkAccess = !strings.EqualFold(pointer.From(props.PublicNetworkAccess), helpers.PublicNetworkAccessDisabled)

if features.FourPointOhBeta() {
metadata.ResourceData.Set("vnet_image_pull_enabled", pointer.From(props.VnetImagePullEnabled))
}
servicePlanId, err := commonids.ParseAppServicePlanIDInsensitively(*props.ServerFarmId)
if err != nil {
return err
@@ -929,6 +952,10 @@ func (r LinuxFunctionAppResource) Update() sdk.ResourceFunc {
}
}

if metadata.ResourceData.HasChange("vnet_image_pull_enabled") && features.FourPointOhBeta() {
model.Properties.VnetImagePullEnabled = pointer.To(metadata.ResourceData.Get("vnet_image_pull_enabled").(bool))
}

if metadata.ResourceData.HasChange("client_certificate_enabled") {
model.Properties.ClientCertEnabled = pointer.To(state.ClientCertEnabled)
}
@@ -1210,6 +1237,29 @@ func (r LinuxFunctionAppResource) CustomizeDiff() sdk.ResourceFunc {
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
client := metadata.Client.AppService.ServicePlanClient
rd := metadata.ResourceDiff
if rd.HasChange("vnet_image_pull_enabled") {
Copy link
Member

Choose a reason for hiding this comment

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

Although this shouldn't be problematic, can we still add the FourPointOhBeta flag here to make it clear and consistent that this is a 4.0 property

Suggested change
if rd.HasChange("vnet_image_pull_enabled") {
if rd.HasChange("vnet_image_pull_enabled") && features.FourPointOhBeta() {

Please apply this to the other resources too

planId := rd.Get("service_plan_id")
// the plan id is known after apply during the initial creation
if planId.(string) == "" {
return nil
}
_, newValue := rd.GetChange("vnet_image_pull_enabled")
servicePlanId, err := commonids.ParseAppServicePlanID(planId.(string))
if err != nil {
return err
}

asp, err := client.Get(ctx, *servicePlanId)
if err != nil {
return fmt.Errorf("retrieving %s: %+v", servicePlanId, err)
}
if aspModel := asp.Model; aspModel != nil {
if aspModel.Properties != nil && aspModel.Properties.HostingEnvironmentProfile != nil &&
aspModel.Properties.HostingEnvironmentProfile.Id != nil && *(aspModel.Properties.HostingEnvironmentProfile.Id) != "" && !newValue.(bool) {
return fmt.Errorf("`vnet_image_pull_enabled` cannot be disabled for app running in an app service environment.")
}
}
}
if rd.HasChange("service_plan_id") {
currentPlanIdRaw, newPlanIdRaw := rd.GetChange("service_plan_id")
if newPlanIdRaw.(string) == "" {
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@ import (
"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance"
"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
"github.com/hashicorp/terraform-provider-azurerm/internal/features"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
)

@@ -1680,9 +1681,15 @@ func TestAccLinuxFunctionApp_vNetIntegration(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_linux_function_app", "test")
r := LinuxFunctionAppResource{}

var vnetIntegrationProperties string
if features.FourPointOhBeta() {
vnetIntegrationProperties = r.vNetIntegration_subnet1WithVnetProperties(data, SkuStandardPlan)
} else {
vnetIntegrationProperties = r.vNetIntegration_subnet1(data, SkuStandardPlan)
}
data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.vNetIntegration_subnet1(data, SkuStandardPlan),
Config: vnetIntegrationProperties,
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("virtual_network_subnet_id").MatchesOtherKey(
@@ -4100,6 +4107,7 @@ resource "azurerm_linux_function_app" "test" {
`, r.template(data, planSku), data.RandomInteger, data.RandomInteger)
}

// TODO 4.0 remove this test case as it's replaced by vNetIntegration_subnet1WithVnetProperties
func (r LinuxFunctionAppResource) vNetIntegration_subnet1(data acceptance.TestData, planSku string) string {
return fmt.Sprintf(`
provider "azurerm" {
@@ -4224,6 +4232,70 @@ resource "azurerm_linux_function_app" "test" {
`, r.template(data, planSku), data.RandomInteger, data.RandomInteger)
}

func (r LinuxFunctionAppResource) vNetIntegration_subnet1WithVnetProperties(data acceptance.TestData, planSku string) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}

%s

resource "azurerm_virtual_network" "test" {
name = "vnet-%d"
address_space = ["10.0.0.0/16"]
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
}

resource "azurerm_subnet" "test1" {
name = "subnet1"
resource_group_name = azurerm_resource_group.test.name
virtual_network_name = azurerm_virtual_network.test.name
address_prefixes = ["10.0.1.0/24"]

delegation {
name = "delegation"

service_delegation {
name = "Microsoft.Web/serverFarms"
actions = ["Microsoft.Network/virtualNetworks/subnets/action"]
}
}
}

resource "azurerm_subnet" "test2" {
name = "subnet2"
resource_group_name = azurerm_resource_group.test.name
virtual_network_name = azurerm_virtual_network.test.name
address_prefixes = ["10.0.2.0/24"]

delegation {
name = "delegation"

service_delegation {
name = "Microsoft.Web/serverFarms"
actions = ["Microsoft.Network/virtualNetworks/subnets/action"]
}
}
}

resource "azurerm_linux_function_app" "test" {
name = "acctest-LFA-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
service_plan_id = azurerm_service_plan.test.id
virtual_network_subnet_id = azurerm_subnet.test1.id

storage_account_name = azurerm_storage_account.test.name
storage_account_access_key = azurerm_storage_account.test.primary_access_key

vnet_image_pull_enabled = true
site_config {}
}
`, r.template(data, planSku), data.RandomInteger, data.RandomInteger)
}

// TODO 4.0 enable the vnet_image_pull_enabled property for app running in ase env
func (r LinuxFunctionAppResource) withASEV3(data acceptance.TestData) string {
return fmt.Sprintf(`
%s
@@ -4244,6 +4316,7 @@ resource "azurerm_linux_function_app" "test" {
storage_account_name = azurerm_storage_account.test.name
storage_account_access_key = azurerm_storage_account.test.primary_access_key

// vnet_image_pull_enabled = true
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// vnet_image_pull_enabled = true

Copy link
Member

Choose a reason for hiding this comment

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

When we hook the feature flag FourPointOhBeta up to an environment variable to be able to enable the beta and to test that the 4.0 features work, this will be problematic and require manual intervention to uncomment the line in order to test. This needs to be removed and added to a new test that is skipped prior to 4.0 e.g.

	if !features.FourPointOhBeta() {
		t.Skip("this test requires 4.0 mode")
	}

Please make this change to the other resource tests too

site_config {
vnet_route_all_enabled = true
}
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ import (
"github.com/hashicorp/go-azure-helpers/resourcemanager/location"
"github.com/hashicorp/go-azure-sdk/resource-manager/web/2023-01-01/resourceproviders"
"github.com/hashicorp/go-azure-sdk/resource-manager/web/2023-01-01/webapps"
"github.com/hashicorp/terraform-provider-azurerm/internal/features"
"github.com/hashicorp/terraform-provider-azurerm/internal/locks"
"github.com/hashicorp/terraform-provider-azurerm/internal/sdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/appservice/helpers"
@@ -73,6 +74,7 @@ type LinuxFunctionAppSlotModel struct {
SiteCredentials []helpers.SiteCredential `tfschema:"site_credential"`
StorageAccounts []helpers.StorageAccount `tfschema:"storage_account"`
Identity []identity.ModelSystemAssignedUserAssigned `tfschema:"identity"`
// VnetImagePullEnabled bool `tfschema:"vnet_image_pull_enabled"` // TODO 4.0 not supported on Consumption plans
}

var _ sdk.ResourceWithUpdate = LinuxFunctionAppSlotResource{}
@@ -92,7 +94,7 @@ func (r LinuxFunctionAppSlotResource) IDValidationFunc() pluginsdk.SchemaValidat
}

func (r LinuxFunctionAppSlotResource) Arguments() map[string]*pluginsdk.Schema {
return map[string]*pluginsdk.Schema{
s := map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
@@ -281,6 +283,15 @@ func (r LinuxFunctionAppSlotResource) Arguments() map[string]*pluginsdk.Schema {
ValidateFunc: commonids.ValidateSubnetID,
},
}
if features.FourPointOhBeta() {
s["vnet_image_pull_enabled"] = &pluginsdk.Schema{
Type: pluginsdk.TypeBool,
Optional: true,
Default: false,
Description: "Is container image pull over virtual network enabled? Defaults to `false`.",
}
}
return s
}

func (r LinuxFunctionAppSlotResource) Attributes() map[string]*pluginsdk.Schema {
@@ -427,6 +438,11 @@ func (r LinuxFunctionAppSlotResource) Create() sdk.ResourceFunc {

availabilityRequest.Name = fmt.Sprintf("%s.%s", functionAppSlot.Name, nameSuffix)
availabilityRequest.IsFqdn = pointer.To(true)
if features.FourPointOhBeta() {
if !metadata.ResourceData.Get("vnet_image_pull_enabled").(bool) {
return fmt.Errorf("`vnet_image_pull_enabled` cannot be disabled for app running in an app service environment.")
}
}
}

}
@@ -520,6 +536,10 @@ func (r LinuxFunctionAppSlotResource) Create() sdk.ResourceFunc {
},
}

if features.FourPointOhBeta() {
siteEnvelope.Properties.VnetImagePullEnabled = pointer.To(metadata.ResourceData.Get("vnet_image_pull_enabled").(bool))
}

pan := helpers.PublicNetworkAccessEnabled
if !functionAppSlot.PublicNetworkAccess {
pan = helpers.PublicNetworkAccessDisabled
@@ -736,6 +756,9 @@ func (r LinuxFunctionAppSlotResource) Read() sdk.ResourceFunc {
state.DefaultHostname = pointer.From(props.DefaultHostName)
state.PublicNetworkAccess = !strings.EqualFold(pointer.From(props.PublicNetworkAccess), helpers.PublicNetworkAccessDisabled)

if features.FourPointOhBeta() {
metadata.ResourceData.Set("vnet_image_pull_enabled", pointer.From(props.VnetImagePullEnabled))
}
if hostingEnv := props.HostingEnvironmentProfile; hostingEnv != nil {
state.HostingEnvId = pointer.From(hostingEnv.Id)
}
@@ -923,6 +946,10 @@ func (r LinuxFunctionAppSlotResource) Update() sdk.ResourceFunc {
}
}

if metadata.ResourceData.HasChange("vnet_image_pull_enabled") && features.FourPointOhBeta() {
model.Properties.VnetImagePullEnabled = pointer.To(metadata.ResourceData.Get("vnet_image_pull_enabled").(bool))
}

storageString := state.StorageAccountName
if !state.StorageUsesMSI {
if state.StorageKeyVaultSecretID != "" {
@@ -1186,3 +1213,35 @@ func (r LinuxFunctionAppSlotResource) StateUpgraders() sdk.StateUpgradeData {
},
}
}

func (r LinuxFunctionAppSlotResource) CustomizeDiff() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 5 * time.Minute,
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
appClient := metadata.Client.AppService.WebAppsClient
rd := metadata.ResourceDiff
if rd.HasChange("vnet_image_pull_enabled") {
appId := rd.Get("function_app_id")
if appId.(string) == "" {
return nil
}
_, newValue := rd.GetChange("vnet_image_pull_enabled")
functionAppId, err := commonids.ParseAppServiceID(appId.(string))
if err != nil {
return err
}

functionApp, err := appClient.Get(ctx, *functionAppId)
if err != nil {
return fmt.Errorf("retrieving %s: %+v", functionAppId, err)
}
if functionAppModel := functionApp.Model; functionAppModel != nil && functionAppModel.Properties != nil {
if ase := functionAppModel.Properties.HostingEnvironmentProfile; ase != nil && ase.Id != nil && *(ase.Id) != "" && !newValue.(bool) {
return fmt.Errorf("`vnet_image_pull_enabled` cannot be disabled for app slot running in an app service environment.")
}
}
}
return nil
},
}
}
Loading
Loading