Remove infrastructure outputs from .env on azd down

The tool had logic to this, but it had been broken for a while due to
a change in how `Environment.Save()` worked. In Azure#1667 we made a change
such that when you save an environment, we would merge in new values
from the existing .env file into the file we were saving, in case the
.env file had been modified by an external hook.

However, that logic did not take into account that we may have
intentionally removed some values (to explicitly delete them) and if
they already existed in the `.env` files, they should be removed as
part of the save operation.

To address this, we now track the deletes that we have preformed so we
can replay them during saving.

To prevent regressing again we also add an end to end test.

Fixes Azure#2217

cli/azd/cmd/down.go

@@ -114,21 +114,10 @@ func (a *downAction) Run(ctx context.Context) (*actions.ActionResult, error) {
 	startTime := time.Now()
 
 	destroyOptions := provisioning.NewDestroyOptions(a.flags.forceDelete, a.flags.purgeDelete)
-	destroyResult, err := infraManager.Destroy(ctx, destroyOptions)
-	if err != nil {
+	if _, err = infraManager.Destroy(ctx, destroyOptions); err != nil {
 		return nil, fmt.Errorf("deleting infrastructure: %w", err)
 	}
 
-	// Remove any outputs from the template from the environment since destroying the infrastructure
-	// invalidated them all.
-	for outputName := range destroyResult.Outputs {
-		a.env.DotenvDelete(outputName)
-	}
-
-	if err := a.env.Save(); err != nil {
-		return nil, fmt.Errorf("saving environment: %w", err)
-	}
-
 	return &actions.ActionResult{
 		Message: &actions.ResultMessage{
 			Header: fmt.Sprintf("Your application was removed from Azure in %s.", ux.DurationAsText(time.Since(startTime))),

cli/azd/pkg/environment/environment.go

@@ -53,7 +53,7 @@ type Environment struct {
 
 	// dotEnvDeleted keeps track of deleted keys to be reapplied before a merge operation
 	// happens in Save
-	dotenvDeleted map[string]bool
+	dotenvDeleted map[string]struct{}
 
 	// Config is environment specific config
 	Config config.Config
@@ -181,7 +181,7 @@ func (e *Environment) LookupEnv(key string) (string, bool) {
 // does not exist. [Save] should be called to ensure this change is persisted.
 func (e *Environment) DotenvDelete(key string) {
 	delete(e.dotenv, key)
-	e.dotenvDeleted[key] = true
+	e.dotenvDeleted[key] = struct{}{}
 }
 
 // Dotenv returns a copy of the key value pairs from the .env file in the environment.
@@ -193,6 +193,7 @@ func (e *Environment) Dotenv() map[string]string {
 // called to ensure this change is persisted.
 func (e *Environment) DotenvSet(key string, value string) {
 	e.dotenv[key] = value
+	delete(e.dotenvDeleted, key)
 }
 
 // Reloads environment variables and configuration
@@ -201,12 +202,12 @@ func (e *Environment) Reload() error {
 	envPath := filepath.Join(e.Root, azdcontext.DotEnvFileName)
 	if envMap, err := godotenv.Read(envPath); errors.Is(err, os.ErrNotExist) {
 		e.dotenv = make(map[string]string)
-		e.dotenvDeleted = make(map[string]bool)
+		e.dotenvDeleted = make(map[string]struct{})
 	} else if err != nil {
 		return fmt.Errorf("loading .env: %w", err)
 	} else {
 		e.dotenv = envMap
-		e.dotenvDeleted = make(map[string]bool)
+		e.dotenvDeleted = make(map[string]struct{})
 	}
 
 	// Reload env config
@@ -282,7 +283,7 @@ func (e *Environment) GetEnvName() string {
 
 // SetEnvName is shorthand for DotenvSet(EnvNameEnvVarName, envname)
 func (e *Environment) SetEnvName(envname string) {
-	e.dotenv[EnvNameEnvVarName] = envname
+	e.DotenvSet(EnvNameEnvVarName, envname)
 }
 
 // GetSubscriptionId is shorthand for Getenv(SubscriptionIdEnvVarName)
@@ -297,7 +298,7 @@ func (e *Environment) GetTenantId() string {
 
 // SetLocation is shorthand for DotenvSet(SubscriptionIdEnvVarName, location)
 func (e *Environment) SetSubscriptionId(id string) {
-	e.dotenv[SubscriptionIdEnvVarName] = id
+	e.DotenvSet(SubscriptionIdEnvVarName, id)
 }
 
 // GetLocation is shorthand for Getenv(LocationEnvVarName)
@@ -307,7 +308,7 @@ func (e *Environment) GetLocation() string {
 
 // SetLocation is shorthand for DotenvSet(LocationEnvVarName, location)
 func (e *Environment) SetLocation(location string) {
-	e.dotenv[LocationEnvVarName] = location
+	e.DotenvSet(LocationEnvVarName, location)
 }
 
 func normalize(key string) string {
@@ -321,7 +322,7 @@ func (e *Environment) GetServiceProperty(serviceName string, propertyName string
 
 // Sets the value of a service-namespaced property in the environment.
 func (e *Environment) SetServiceProperty(serviceName string, propertyName string, value string) {
-	e.dotenv[fmt.Sprintf("SERVICE_%s_%s", normalize(serviceName), propertyName)] = value
+	e.DotenvSet(fmt.Sprintf("SERVICE_%s_%s", normalize(serviceName), propertyName), value)
 }
 
 // Creates a slice of key value pairs, based on the entries in the `.env` file like `KEY=VALUE` that

cli/azd/pkg/environment/environment_test.go

@@ -157,6 +157,19 @@ func Test_SaveAndReload(t *testing.T) {
 	// Verify the property is deleted
 	_, ok := env.LookupEnv("SERVICE_WEB_ENDPOINT_URL")
 	require.False(t, ok)
+
+	// Delete an existing key, then add it with a different value and save the environment, to ensure we
+	// don't drop the existing key even though it was deleted in an earlier operation.
+	env.DotenvDelete("SERVICE_API_ENDPOINT_URL")
+	env.DotenvSet("SERVICE_API_ENDPOINT_URL", "http://api.example.com/updated")
+
+	err = env.Save()
+	require.NoError(t, err)
+
+	// Verify the property still exists, and has the updated value.
+	value, ok := env.LookupEnv("SERVICE_API_ENDPOINT_URL")
+	require.True(t, ok)
+	require.Equal(t, "http://api.example.com/updated", value)
 }
 
 func TestCleanName(t *testing.T) {

cli/azd/test/functional/up_test.go

@@ -260,6 +260,14 @@ func Test_CLI_Up_Down_ContainerApp(t *testing.T) {
 
 	_, err = cli.RunCommand(ctx, "infra", "delete", "--force", "--purge")
 	require.NoError(t, err)
+
+	// As part of deleting the infrastructure, outputs of the infrastructure such as "WEBSITE_URL" should
+	// have been removed from the environment.
+	env, err = godotenv.Read(filepath.Join(dir, azdcontext.EnvironmentDirectoryName, envName, ".env"))
+	require.NoError(t, err)
+
+	_, has = env["WEBSITE_URL"]
+	require.False(t, has, "WEBSITE_URL should have been removed from the environment as part of infrastructure removal")
 }
 
 func Test_CLI_Up_ResourceGroupScope(t *testing.T) {