functionapp: handle .funcignore more gracefully

cli/azd/pkg/project/service_target_functionapp.go

@@ -5,7 +5,9 @@ package project
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
@@ -16,9 +18,67 @@ import (
 	"github.com/azure/azure-dev/cli/azd/pkg/azapi"
 	"github.com/azure/azure-dev/cli/azd/pkg/environment"
 	"github.com/azure/azure-dev/cli/azd/pkg/input"
+	"github.com/azure/azure-dev/cli/azd/pkg/output"
 	"github.com/azure/azure-dev/cli/azd/pkg/tools"
+	"github.com/denormal/go-gitignore"
 )
 
+const functionAppRemoteBuildDocURL = "https://aka.ms/azd-functionapp-remote-build"
+
+// resolveFunctionAppRemoteBuild returns the appropriate remote build setting for function apps.
+func resolveFunctionAppRemoteBuild(serviceConfig *ServiceConfig) (remoteBuild bool, err error) {
+	switch serviceConfig.Language {
+	case ServiceLanguageJavaScript, ServiceLanguageTypeScript:
+		ignore, err := gitignore.NewFromFile(filepath.Join(serviceConfig.Path(), serviceConfig.Host.IgnoreFile()))
+		if errors.Is(err, fs.ErrNotExist) {
+			// no ignore file, default to true
+			return true, nil
+		}
+
+		if err != nil {
+			return false, fmt.Errorf("reading ignore file: %w", err)
+		}
+
+		nodeModulesExcluded := false
+		if match := ignore.Relative("node_modules", true); match != nil && match.Ignore() {
+			nodeModulesExcluded = true
+		}
+
+		if serviceConfig.RemoteBuild == nil { // remoteBuild option unset
+			// enable remote build only if 'node_modules' is excluded
+			return nodeModulesExcluded, nil
+		}
+
+		if *serviceConfig.RemoteBuild && !nodeModulesExcluded {
+			return false, &internal.ErrorWithSuggestion{
+				Err: fmt.Errorf("'remoteBuild: true' requires '.funcignore' to exclude node_modules"),
+				Suggestion: fmt.Sprintf(
+					"Update '.funcignore' to exclude node_modules, or set 'remoteBuild: false'. Learn more: %s",
+					output.WithLinkFormat(functionAppRemoteBuildDocURL),
+				),
+			}
+		}
+
+		if !*serviceConfig.RemoteBuild && nodeModulesExcluded {
+			return false, &internal.ErrorWithSuggestion{
+				Err: fmt.Errorf("'remoteBuild: false' cannot be used when '.funcignore' excludes node_modules"),
+				Suggestion: fmt.Sprintf(
+					"Set 'remoteBuild: true', or remove node_modules from '.funcignore'. Learn more: %s",
+					output.WithLinkFormat(functionAppRemoteBuildDocURL),
+				),
+			}
+		}
+
+		return *serviceConfig.RemoteBuild, nil
+	default:
+		if serviceConfig.RemoteBuild != nil {
+			return *serviceConfig.RemoteBuild, nil
+		}
+
+		return serviceConfig.Language == ServiceLanguagePython, nil
+	}
+}
+
 // functionAppTarget specifies an Azure Function to deploy to.
 // Implements `project.ServiceTarget`
 type functionAppTarget struct {
@@ -156,17 +216,14 @@ func (f *functionAppTarget) Deploy(
 	}
 
 	progress.SetProgress(NewServiceProgress("Uploading deployment package"))
-	var remoteBuild bool
-	if serviceConfig.RemoteBuild != nil {
-		remoteBuild = *serviceConfig.RemoteBuild
-	} else {
-		remoteBuild = serviceConfig.Language == ServiceLanguageJavaScript ||
-			serviceConfig.Language == ServiceLanguageTypeScript ||
-			serviceConfig.Language == ServiceLanguagePython
-	}
 
 	// Deploy to appropriate plan type
 	if isFlexConsumption {
+		remoteBuild, buildErr := resolveFunctionAppRemoteBuild(serviceConfig)
+		if buildErr != nil {
+			return nil, buildErr
+		}
+
 		_, err = f.cli.DeployFunctionAppUsingZipFileFlexConsumption(
 			ctx,
 			targetResource.SubscriptionId(),

cli/azd/pkg/project/service_target_functionapp_test.go

@@ -4,9 +4,12 @@
 package project
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/azure/azure-dev/cli/azd/internal"
 	"github.com/azure/azure-dev/cli/azd/pkg/azapi"
 	"github.com/azure/azure-dev/cli/azd/pkg/environment"
 	"github.com/stretchr/testify/require"
@@ -45,3 +48,109 @@ func TestNewFunctionAppTargetTypeValidation(t *testing.T) {
 		})
 	}
 }
+
+func TestResolveFunctionAppRemoteBuild_JavaScriptMatrix(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name              string
+		remoteBuild       *bool
+		funcIgnoreContent string
+		expectRemoteBuild bool
+		expectError       string
+	}{
+		{
+			name:              "NoRemoteBuildAndFuncIgnoreExcludesNodeModules_RemoteBuildEnabled",
+			remoteBuild:       nil,
+			funcIgnoreContent: "node_modules\n",
+			expectRemoteBuild: true,
+		},
+		{
+			name:              "NoRemoteBuildAndFuncIgnoreDoesNotExcludeNodeModules_RemoteBuildDisabled",
+			remoteBuild:       nil,
+			funcIgnoreContent: "dist\n",
+			expectRemoteBuild: false,
+		},
+		{
+			name:              "NoRemoteBuildAndMissingFuncIgnore_RemoteBuildEnabled",
+			remoteBuild:       nil,
+			funcIgnoreContent: "",
+			expectRemoteBuild: true,
+		},
+		{
+			name:              "RemoteBuildFalseAndFuncIgnoreExcludesNodeModules_Errors",
+			remoteBuild:       new(false),
+			funcIgnoreContent: "node_modules\n",
+			expectError:       "'remoteBuild: false' cannot be used when '.funcignore' excludes node_modules",
+		},
+		{
+			name:              "RemoteBuildFalseAndFuncIgnoreDoesNotExcludeNodeModules_Succeeds",
+			remoteBuild:       new(false),
+			funcIgnoreContent: "dist\n",
+			expectRemoteBuild: false,
+		},
+		{
+			name:              "RemoteBuildTrueAndFuncIgnoreExcludesNodeModules_Succeeds",
+			remoteBuild:       new(true),
+			funcIgnoreContent: "node_modules\n",
+			expectRemoteBuild: true,
+		},
+		{
+			name:              "RemoteBuildTrueAndFuncIgnoreDoesNotExcludeNodeModules_Errors",
+			remoteBuild:       new(true),
+			funcIgnoreContent: "dist\n",
+			expectError:       "'remoteBuild: true' requires '.funcignore' to exclude node_modules",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			serviceConfig := createTestServiceConfig(t.TempDir(), AzureFunctionTarget, ServiceLanguageJavaScript)
+			serviceConfig.RemoteBuild = tt.remoteBuild
+
+			if tt.funcIgnoreContent != "" {
+				err := os.WriteFile(
+					filepath.Join(serviceConfig.Path(), ".funcignore"),
+					[]byte(tt.funcIgnoreContent),
+					0600,
+				)
+				require.NoError(t, err)
+			}
+
+			remoteBuild, err := resolveFunctionAppRemoteBuild(serviceConfig)
+			if tt.expectError != "" {
+				require.Error(t, err)
+				require.ErrorContains(t, err, tt.expectError)
+
+				var suggestionErr *internal.ErrorWithSuggestion
+				require.ErrorAs(t, err, &suggestionErr)
+				require.Contains(t, suggestionErr.Suggestion, functionAppRemoteBuildDocURL)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tt.expectRemoteBuild, remoteBuild)
+		})
+	}
+}
+
+func TestResolveFunctionAppRemoteBuild_NonJavaScriptDefaults(t *testing.T) {
+	t.Parallel()
+
+	pythonConfig := createTestServiceConfig(t.TempDir(), AzureFunctionTarget, ServiceLanguagePython)
+	remoteBuild, err := resolveFunctionAppRemoteBuild(pythonConfig)
+	require.NoError(t, err)
+	require.True(t, remoteBuild)
+
+	pythonConfig.RemoteBuild = new(false)
+	remoteBuild, err = resolveFunctionAppRemoteBuild(pythonConfig)
+	require.NoError(t, err)
+	require.False(t, remoteBuild)
+
+	csharpConfig := createTestServiceConfig(t.TempDir(), AzureFunctionTarget, ServiceLanguageCsharp)
+	remoteBuild, err = resolveFunctionAppRemoteBuild(csharpConfig)
+	require.NoError(t, err)
+	require.False(t, remoteBuild)
+}