From 78255fe76e1c102fcdfb2bf700909f165bde5214 Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Tue, 22 Nov 2022 16:14:32 -0800 Subject: [PATCH 1/3] Tie `Environment` to a folder, not a file --- cli/azd/cmd/util.go | 2 +- .../commands/pipeline/azdo_provider_test.go | 4 +- .../pkg/environment/azdcontext/azdcontext.go | 13 ++++-- cli/azd/pkg/environment/environment.go | 40 +++++++++---------- cli/azd/test/functional/cli_test.go | 8 ++-- 5 files changed, 36 insertions(+), 31 deletions(-) diff --git a/cli/azd/cmd/util.go b/cli/azd/cmd/util.go index 6bcd5d69cab..5f5a9e5e95d 100644 --- a/cli/azd/cmd/util.go +++ b/cli/azd/cmd/util.go @@ -157,7 +157,7 @@ func loadOrInitEnvironment( return nil, false, err } - return environment.EmptyWithFile(azdCtx.GetEnvironmentFilePath(*environmentName)), true, nil + return environment.EmptyWithRoot(azdCtx.EnvironmentRoot(*environmentName)), true, nil } env, isNew, err := loadOrCreateEnvironment() diff --git a/cli/azd/pkg/commands/pipeline/azdo_provider_test.go b/cli/azd/pkg/commands/pipeline/azdo_provider_test.go index a7e992d7629..226cc0dfaba 100644 --- a/cli/azd/pkg/commands/pipeline/azdo_provider_test.go +++ b/cli/azd/pkg/commands/pipeline/azdo_provider_test.go @@ -161,11 +161,11 @@ func Test_saveEnvironmentConfig(t *testing.T) { value := "12345" provider := getEmptyAzdoScmProviderTestHarness() envPath := path.Join(tempDir, ".test.env") - provider.Env = environment.EmptyWithFile(envPath) + provider.Env = environment.EmptyWithRoot(tempDir) // act e := provider.saveEnvironmentConfig(key, value) // assert - writtenEnv, err := environment.FromFile(envPath) + writtenEnv, err := environment.FromRoot(envPath) require.NoError(t, err) require.EqualValues(t, writtenEnv.Values[key], value) diff --git a/cli/azd/pkg/environment/azdcontext/azdcontext.go b/cli/azd/pkg/environment/azdcontext/azdcontext.go index 3e50b8a7472..f551ce11a0d 100644 --- a/cli/azd/pkg/environment/azdcontext/azdcontext.go +++ b/cli/azd/pkg/environment/azdcontext/azdcontext.go @@ -14,6 +14,7 @@ import ( const ProjectFileName = "azure.yaml" const EnvironmentDirectoryName = ".azure" +const DotEnvFileName = ".env" const ConfigFileName = "config.json" const ConfigFileVersion = 1 const InfraDirectoryName = "infra" @@ -46,12 +47,16 @@ func (c *AzdContext) GetDefaultProjectName() string { return filepath.Base(c.ProjectDirectory()) } -func (c *AzdContext) GetEnvironmentFilePath(name string) string { - return filepath.Join(c.EnvironmentDirectory(), name, ".env") +func (c *AzdContext) EnvironmentDotEnvPath(name string) string { + return filepath.Join(c.EnvironmentDirectory(), name, DotEnvFileName) +} + +func (c *AzdContext) EnvironmentRoot(name string) string { + return filepath.Join(c.EnvironmentDirectory(), name) } func (c *AzdContext) GetEnvironmentWorkDirectory(name string) string { - return filepath.Join(c.GetEnvironmentFilePath(name), "wd") + return filepath.Join(c.EnvironmentRoot(name), "wd") } func (c *AzdContext) GetInfrastructurePath() string { @@ -75,7 +80,7 @@ func (c *AzdContext) ListEnvironments() ([]contracts.EnvListEnvironment, error) ev := contracts.EnvListEnvironment{ Name: ent.Name(), IsDefault: ent.Name() == defaultEnv, - DotEnvPath: c.GetEnvironmentFilePath(ent.Name()), + DotEnvPath: c.EnvironmentDotEnvPath(ent.Name()), } envs = append(envs, ev) } diff --git a/cli/azd/pkg/environment/environment.go b/cli/azd/pkg/environment/environment.go index 38b6b40a671..6b397ee7f0b 100644 --- a/cli/azd/pkg/environment/environment.go +++ b/cli/azd/pkg/environment/environment.go @@ -39,10 +39,10 @@ const ResourceGroupEnvVarName = "AZURE_RESOURCE_GROUP" type Environment struct { // Values is a map of setting names to values. Values map[string]string - // File is a path to the file that backs this environment. If empty, the Environment + // File is a path to the directory that backs this environment. If empty, the Environment // will not be persisted when `Save` is called. This allows the zero value to be used // for testing. - File string + Root string } // Same restrictions as a deployment name (ref: @@ -53,19 +53,19 @@ func IsValidEnvironmentName(name string) bool { return environmentNameRegexp.MatchString(name) } -// FromFile loads an environment from a file on disk. On error, +// FromRoot loads an environment located in a directory. On error, // an valid empty environment file, configured to persist its contents -// to file, is returned. -func FromFile(file string) (*Environment, error) { +// to this directory, is returned. +func FromRoot(root string) (*Environment, error) { env := &Environment{ - File: file, + Root: root, Values: make(map[string]string), } - e, err := godotenv.Read(file) + envPath := filepath.Join(root, azdcontext.DotEnvFileName) + e, err := godotenv.Read(envPath) if err != nil { - env.Values = make(map[string]string) - return env, fmt.Errorf("can't read %s: %w", file, err) + return EmptyWithRoot(root), fmt.Errorf("loading .env: %w", err) } env.Values = e @@ -73,14 +73,14 @@ func FromFile(file string) (*Environment, error) { } func GetEnvironment(azdContext *azdcontext.AzdContext, name string) (*Environment, error) { - return FromFile(azdContext.GetEnvironmentFilePath(name)) + return FromRoot(azdContext.EnvironmentRoot(name)) } -// EmptyWithFile returns an empty environment, which will be persisted -// to a given file when saved. -func EmptyWithFile(file string) *Environment { +// EmptyWithRoot returns an empty environment, which will be persisted +// to a given directory when saved. +func EmptyWithRoot(root string) *Environment { return &Environment{ - File: file, + Root: root, Values: make(map[string]string), } } @@ -107,21 +107,21 @@ func EphemeralWithValues(name string, values map[string]string) *Environment { return env } -// If `File` is set, Save writes the current contents of the environment to -// the given file, creating it and any intermediate directories as needed. +// If `Root` is set, Save writes the current contents of the environment to +// the given directory, creating it and any intermediate directories as needed. func (e *Environment) Save() error { - if e.File == "" { + if e.Root == "" { return nil } - err := os.MkdirAll(filepath.Dir(e.File), osutil.PermissionDirectory) + err := os.MkdirAll(e.Root, osutil.PermissionDirectory) if err != nil { return fmt.Errorf("failed to create a directory: %w", err) } - err = godotenv.Write(e.Values, e.File) + err = godotenv.Write(e.Values, filepath.Join(e.Root, azdcontext.DotEnvFileName)) if err != nil { - return fmt.Errorf("can't write '%s': %w", e.File, err) + return fmt.Errorf("saving .env: %w", err) } return nil diff --git a/cli/azd/test/functional/cli_test.go b/cli/azd/test/functional/cli_test.go index d0b30275af9..3d132ec84a5 100644 --- a/cli/azd/test/functional/cli_test.go +++ b/cli/azd/test/functional/cli_test.go @@ -134,8 +134,8 @@ func Test_CLI_InfraCreateAndDelete(t *testing.T) { _, err = cli.RunCommand(ctx, "infra", "create") require.NoError(t, err) - envFilePath := filepath.Join(dir, azdcontext.EnvironmentDirectoryName, envName, ".env") - env, err := environment.FromFile(envFilePath) + envPath := filepath.Join(dir, azdcontext.EnvironmentDirectoryName, envName) + env, err := environment.FromRoot(envPath) require.NoError(t, err) // AZURE_STORAGE_ACCOUNT_NAME is an output of the template, make sure it was added to the .env file. @@ -188,8 +188,8 @@ func Test_CLI_InfraCreateAndDeleteUpperCase(t *testing.T) { _, err = cli.RunCommand(ctx, "infra", "create") require.NoError(t, err) - envFilePath := filepath.Join(dir, azdcontext.EnvironmentDirectoryName, envName, ".env") - env, err := environment.FromFile(envFilePath) + envPath := filepath.Join(dir, azdcontext.EnvironmentDirectoryName, envName) + env, err := environment.FromRoot(envPath) require.NoError(t, err) // AZURE_STORAGE_ACCOUNT_NAME is an output of the template, make sure it was added to the .env file. From e60c05b8576bc984240c1158f161f9e08fa0b1e3 Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Tue, 22 Nov 2022 16:38:33 -0800 Subject: [PATCH 2/3] Add Config to Environment --- cli/azd/pkg/environment/environment.go | 28 +++++++++++++++++-- cli/azd/pkg/environment/environment_test.go | 31 +++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/cli/azd/pkg/environment/environment.go b/cli/azd/pkg/environment/environment.go index 6b397ee7f0b..819a09d908a 100644 --- a/cli/azd/pkg/environment/environment.go +++ b/cli/azd/pkg/environment/environment.go @@ -9,6 +9,7 @@ import ( "path/filepath" "regexp" + "github.com/azure/azure-dev/cli/azd/pkg/config" "github.com/azure/azure-dev/cli/azd/pkg/environment/azdcontext" "github.com/azure/azure-dev/cli/azd/pkg/osutil" "github.com/joho/godotenv" @@ -39,6 +40,10 @@ const ResourceGroupEnvVarName = "AZURE_RESOURCE_GROUP" type Environment struct { // Values is a map of setting names to values. Values map[string]string + + // Config is environment specific config + Config config.Config + // File is a path to the directory that backs this environment. If empty, the Environment // will not be persisted when `Save` is called. This allows the zero value to be used // for testing. @@ -58,8 +63,7 @@ func IsValidEnvironmentName(name string) bool { // to this directory, is returned. func FromRoot(root string) (*Environment, error) { env := &Environment{ - Root: root, - Values: make(map[string]string), + Root: root, } envPath := filepath.Join(root, azdcontext.DotEnvFileName) @@ -67,8 +71,17 @@ func FromRoot(root string) (*Environment, error) { if err != nil { return EmptyWithRoot(root), fmt.Errorf("loading .env: %w", err) } - env.Values = e + + cfgPath := filepath.Join(root, azdcontext.ConfigFileName) + + cfgMgr := config.NewManager() + cfg, err := cfgMgr.Load(cfgPath) + if err != nil { + return EmptyWithRoot(root), fmt.Errorf("loading config: %w", err) + } + env.Config = cfg + return env, nil } @@ -82,12 +95,14 @@ func EmptyWithRoot(root string) *Environment { return &Environment{ Root: root, Values: make(map[string]string), + Config: config.NewConfig(nil), } } func Ephemeral() *Environment { return &Environment{ Values: make(map[string]string), + Config: config.NewConfig(nil), } } @@ -124,6 +139,13 @@ func (e *Environment) Save() error { return fmt.Errorf("saving .env: %w", err) } + cfgMgr := config.NewManager() + + err = cfgMgr.Save(e.Config, filepath.Join(e.Root, azdcontext.ConfigFileName)) + if err != nil { + return fmt.Errorf("saving config: %w", err) + } + return nil } diff --git a/cli/azd/pkg/environment/environment_test.go b/cli/azd/pkg/environment/environment_test.go index 0c20b78f298..169a5932a42 100644 --- a/cli/azd/pkg/environment/environment_test.go +++ b/cli/azd/pkg/environment/environment_test.go @@ -4,9 +4,12 @@ package environment import ( + "errors" + "os" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestIsValidEnvironmentName(t *testing.T) { @@ -19,3 +22,31 @@ func TestIsValidEnvironmentName(t *testing.T) { assert.False(t, IsValidEnvironmentName("no spaces")) assert.False(t, IsValidEnvironmentName("12345678901234567890123456789012345678901234567890123456789012345")) } + +func TestConfigRoundTrips(t *testing.T) { + root := t.TempDir() + + // Create a new config from an empty root. We expect this to fail (because there is no configuration data), but + // to get back an empty configuration object we can use regardless. + e, err := FromRoot(root) + require.Error(t, err) + require.True(t, errors.Is(err, os.ErrNotExist)) + + // There should be no configuration since this is an empty environment. + require.True(t, e.Config.IsEmpty()) + + // Set a config value. + err = e.Config.Set("is.this.a.test", true) + require.NoError(t, err) + + // Save the environment + err = e.Save() + require.NoError(t, err) + + // Load the environment back up, we expect no error and for the config value we wrote to still exist. + e, err = FromRoot(root) + require.NoError(t, err) + v, has := e.Config.Get("is.this.a.test") + require.True(t, has) + require.Equal(t, true, v) +} From 694212ef2b461a5051c6627434593bd3821c0f7b Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Mon, 21 Nov 2022 16:23:20 -0800 Subject: [PATCH 3/3] Fix prompting for missing infrastructure values Our logic to prompt the user for missing infrastructure values was broken because we did not tell the spinner we wanted to interact with the user, so the spinner would prevent the prompt UI from being shown. The logic we had for trying to save these values was totally broken and in practice would end up generating files that the ARM control plane would reject. We now save these values in the newly added `Config` object on the environment, using `infra.prameters.` as the key name. As part of this work, we no longer materialize the full `.parameters.json` file to disk, we no longer need to do this now that we are using the SDK directly. Fixes #1176 Fixes #1191 --- cli/azd/pkg/azure/arm_template.go | 12 ++ .../commands/pipeline/azdo_provider_test.go | 6 +- .../provisioning/bicep/bicep_provider.go | 189 +++++++++--------- .../provisioning/bicep/bicep_provider_test.go | 25 +-- cli/azd/pkg/infra/scope.go | 10 +- cli/azd/pkg/infra/scope_test.go | 23 +-- cli/azd/pkg/tools/azcli/azcli.go | 5 +- cli/azd/pkg/tools/azcli/deployments.go | 38 ++-- 8 files changed, 141 insertions(+), 167 deletions(-) diff --git a/cli/azd/pkg/azure/arm_template.go b/cli/azd/pkg/azure/arm_template.go index 5cee8bf8241..6ca5f7dceed 100644 --- a/cli/azd/pkg/azure/arm_template.go +++ b/cli/azd/pkg/azure/arm_template.go @@ -5,3 +5,15 @@ package azure // ArmTemplate is a JSON encoded ARM template. type ArmTemplate string + +type ArmParameters map[string]ArmParameterValue + +type ArmParameterFile struct { + Schema string `json:"$schema"` + ContentVersion string `json:"contentVersion"` + Parameters ArmParameters `json:"parameters"` +} + +type ArmParameterValue struct { + Value any `json:"value"` +} diff --git a/cli/azd/pkg/commands/pipeline/azdo_provider_test.go b/cli/azd/pkg/commands/pipeline/azdo_provider_test.go index 226cc0dfaba..1ae652c897f 100644 --- a/cli/azd/pkg/commands/pipeline/azdo_provider_test.go +++ b/cli/azd/pkg/commands/pipeline/azdo_provider_test.go @@ -6,7 +6,7 @@ package pipeline import ( "context" "errors" - "path" + "path/filepath" "testing" "github.com/azure/azure-dev/cli/azd/pkg/azdo" @@ -160,8 +160,8 @@ func Test_saveEnvironmentConfig(t *testing.T) { key := "test" value := "12345" provider := getEmptyAzdoScmProviderTestHarness() - envPath := path.Join(tempDir, ".test.env") - provider.Env = environment.EmptyWithRoot(tempDir) + envPath := filepath.Join(tempDir, "test") + provider.Env = environment.EmptyWithRoot(envPath) // act e := provider.saveEnvironmentConfig(key, value) // assert diff --git a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go index 51359eb443c..92167fe41ce 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go +++ b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go @@ -30,9 +30,9 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/environment" "github.com/azure/azure-dev/cli/azd/pkg/exec" "github.com/azure/azure-dev/cli/azd/pkg/infra" + "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" . "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" "github.com/azure/azure-dev/cli/azd/pkg/input" - "github.com/azure/azure-dev/cli/azd/pkg/osutil" "github.com/azure/azure-dev/cli/azd/pkg/output" "github.com/azure/azure-dev/cli/azd/pkg/tools" "github.com/azure/azure-dev/cli/azd/pkg/tools/azcli" @@ -60,8 +60,7 @@ type BicepOutputParameter struct { } type BicepDeploymentDetails struct { - ParameterFilePath string - Template *azure.ArmTemplate + Template *azure.ArmTemplate } // BicepProvider exposes infrastructure provisioning using Azure Bicep templates @@ -136,7 +135,8 @@ func (p *BicepProvider) Plan( asyncContext.SetProgress( &DeploymentPlanningProgress{Message: "Generating Bicep parameters file", Timestamp: time.Now()}, ) - bicepTemplate, parameterFilePath, err := p.createParametersFile(ctx, asyncContext) + + parameters, err := p.loadParameters(ctx, asyncContext) if err != nil { asyncContext.SetError(fmt.Errorf("creating parameters file: %w", err)) return @@ -150,32 +150,23 @@ func (p *BicepProvider) Plan( return } - // Merge parameter values from template + // Assign values to parameters based on what was in the parameters file. for key, param := range deployment.Parameters { - if bicepParam, has := bicepTemplate.Parameters[key]; has { - param.Value = bicepParam.Value + if templateParam, has := parameters[key]; has { + param.Value = templateParam.Value deployment.Parameters[key] = param } } - updated, err := p.ensureParameters(ctx, deployment) - if err != nil { + if err := p.ensureParameters(ctx, asyncContext, deployment); err != nil { asyncContext.SetError(err) return } - if updated { - if err := p.updateParametersFile(ctx, deployment, parameterFilePath); err != nil { - asyncContext.SetError(fmt.Errorf("updating deployment parameters: %w", err)) - return - } - } - result := DeploymentPlan{ Deployment: *deployment, Details: BicepDeploymentDetails{ - ParameterFilePath: parameterFilePath, - Template: armTemplate, + Template: armTemplate, }, } @@ -230,7 +221,7 @@ func (p *BicepProvider) Deploy( // Start the deployment bicepDeploymentData := pd.Details.(BicepDeploymentDetails) deployResult, err := p.deployModule( - ctx, scope, bicepDeploymentData.Template, bicepDeploymentData.ParameterFilePath) + ctx, scope, bicepDeploymentData.Template, pd.Deployment.Parameters) if err != nil { asyncContext.SetError(err) @@ -669,34 +660,6 @@ func (p *BicepProvider) deleteDeployment( return nil } -// Converts the specified deployment to a bicep template parameters file and writes the file to disk. -func (p *BicepProvider) updateParametersFile(ctx context.Context, deployment *Deployment, parameterFilePath string) error { - bicepFile := BicepTemplate{ - Schema: "https://schema.management.azure.com/schemas/2019-04-01/deploymentParameters.json#", - ContentVersion: "1.0.0.0", - } - - parameters := make(map[string]BicepInputParameter) - - for key, param := range deployment.Parameters { - parameters[key] = BicepInputParameter(param) - } - - bicepFile.Parameters = parameters - - bytes, err := json.MarshalIndent(bicepFile, "", " ") - if err != nil { - return fmt.Errorf("marshaling parameters: %w", err) - } - - err = os.WriteFile(parameterFilePath, bytes, osutil.PermissionFile) - if err != nil { - return fmt.Errorf("writing parameters file: %w", err) - } - - return nil -} - func (p *BicepProvider) mapBicepTypeToInterfaceType(s string) ParameterType { switch s { case "String", "string": @@ -746,19 +709,17 @@ func (p *BicepProvider) createOutputParameters( return outputParams } -// createParametersFile will read the parameters file template for environment/module specified by Options, -// do environment and command substitutions, and write out the result into a temporary file. -// -// The caller of the method is responsible for deleting the file when it is no longer necessary. -func (p *BicepProvider) createParametersFile( +// loadParameters reads the parameters file template for environment/module specified by Options, +// doing environment and command substitutions, and returns the values. +func (p *BicepProvider) loadParameters( ctx context.Context, asyncContext *async.InteractiveTaskContextWithProgress[*DeploymentPlan, *DeploymentPlanningProgress], -) (*BicepTemplate, string, error) { +) (map[string]azure.ArmParameterValue, error) { parametersTemplateFilePath := p.parametersTemplateFilePath() log.Printf("Reading parameters template file from: %s", parametersTemplateFilePath) parametersBytes, err := os.ReadFile(parametersTemplateFilePath) if err != nil { - return nil, "", fmt.Errorf("reading parameter file template: %w", err) + return nil, fmt.Errorf("reading parameter file template: %w", err) } replaced, err := envsubst.Eval(string(parametersBytes), func(name string) string { @@ -768,35 +729,23 @@ func (p *BicepProvider) createParametersFile( return os.Getenv(name) }) if err != nil { - return nil, "", fmt.Errorf("substituting environment variables inside parameter file: %w", err) + return nil, fmt.Errorf("substituting environment variables inside parameter file: %w", err) } if cmdsubst.ContainsCommandInvocation(replaced, cmdsubst.SecretOrRandomPasswordCommandName) { cmdExecutor := cmdsubst.NewSecretOrRandomPasswordExecutor(p.azCli) replaced, err = cmdsubst.Eval(ctx, replaced, cmdExecutor) if err != nil { - return nil, "", fmt.Errorf("substituting command output inside parameter file: %w", err) + return nil, fmt.Errorf("substituting command output inside parameter file: %w", err) } } - var bicepTemplate BicepTemplate - if err := json.Unmarshal([]byte(replaced), &bicepTemplate); err != nil { - return nil, "", fmt.Errorf("error unmarshalling Bicep template parameters: %w", err) + var armParameters azure.ArmParameterFile + if err := json.Unmarshal([]byte(replaced), &armParameters); err != nil { + return nil, fmt.Errorf("error unmarshalling Bicep template parameters: %w", err) } - file, err := os.CreateTemp("", "deploymentParameters") - if err != nil { - return nil, "", err - } - - _, err = file.Write([]byte(replaced)) - file.Close() // Errors OK to ignore (see the docs) and we need to close the file whether Write() succeeded or not. - if err != nil { - os.Remove(file.Name()) // Error OK to ignore as well. - return nil, "", err - } - - return &bicepTemplate, file.Name(), nil + return armParameters.Parameters, nil } // Creates the compiled template from the specified module path @@ -848,13 +797,11 @@ func (p *BicepProvider) convertToDeployment(bicepTemplate BicepTemplate) (*Deplo // Deploys the specified Bicep module and parameters with the selected provisioning scope (subscription vs resource group) func (p *BicepProvider) deployModule( - ctx context.Context, scope infra.Scope, armTemplate *azure.ArmTemplate, parametersPath string) ( - *armresources.DeploymentExtended, error) { - // We've seen issues where `Deploy` completes but for a short while after, fetching the deployment fails with a - // `DeploymentNotFound` error. - // Since other commands of ours use the deployment, let's try to fetch it here and if we fail with `DeploymentNotFound`, - // ignore this error, wait a short while and retry. - + ctx context.Context, + scope infra.Scope, + armTemplate *azure.ArmTemplate, + parameters map[string]provisioning.InputParameter, +) (*armresources.DeploymentExtended, error) { // deployments API takes an ARM template. // At this point, the bicep file should have been already compiled and succeeded // do panic if the application tries to deploy a bicep file without compiling it first @@ -862,10 +809,27 @@ func (p *BicepProvider) deployModule( log.Panic("deployModule: received nil for arm template.") } - if err := scope.Deploy(ctx, armTemplate, parametersPath); err != nil { + armParameters := make(azure.ArmParameters, len(parameters)) + for k, v := range parameters { + + // Since we co-mingle parameter definitions and configurations, we need to ignore entires without values (they are + // un-configured and the expectation is we'll pick the default value. + if v.HasValue() { + armParameters[k] = azure.ArmParameterValue{ + Value: v.Value, + } + } + } + + if err := scope.Deploy(ctx, armTemplate, armParameters); err != nil { return nil, fmt.Errorf("failed deploying: %w", err) } + // We've seen issues where `Deploy` completes but for a short while after, fetching the deployment fails with a + // `DeploymentNotFound` error. + // Since other commands of ours use the deployment, let's try to fetch it here and if we fail with `DeploymentNotFound`, + // ignore this error, wait a short while and retry. + var deployment *armresources.DeploymentExtended if err := retry.Do(ctx, retry.WithMaxRetries(10, retry.NewExponential(1*time.Second)), func(ctx context.Context) error { deploymentResult, err := scope.GetDeployment(ctx) @@ -907,45 +871,70 @@ func (p *BicepProvider) modulePath() string { } // Ensures the provisioning parameters are valid and prompts the user for input as needed -func (p *BicepProvider) ensureParameters(ctx context.Context, deployment *Deployment) (bool, error) { +func (p *BicepProvider) ensureParameters( + ctx context.Context, + asyncContext *async.InteractiveTaskContextWithProgress[*DeploymentPlan, *DeploymentPlanningProgress], + deployment *Deployment, +) error { if len(deployment.Parameters) == 0 { - return false, nil + return nil } - updatedParameters := false for key, param := range deployment.Parameters { // If this parameter has a default, then there is no need for us to configure it if param.HasDefaultValue() { continue } if !param.HasValue() { - userValue, err := p.console.Prompt(ctx, input.ConsoleOptions{ - Message: fmt.Sprintf("Please enter a value for the '%s' deployment parameter:", key), - }) + configKey := fmt.Sprintf("infra.parameters.%s", key) - if err != nil { - return false, fmt.Errorf("prompting for deployment parameter: %w", err) + if v, has := p.env.Config.Get(configKey); has { + param.Value = v + deployment.Parameters[key] = param + continue } - param.Value = userValue + err := asyncContext.Interact(func() error { + userValue, err := p.console.Prompt(ctx, input.ConsoleOptions{ + Message: fmt.Sprintf("Please enter a value for the '%s' deployment parameter:", key), + }) - saveParameter, err := p.console.Confirm(ctx, input.ConsoleOptions{ - Message: "Save the value in the environment for future use", - }) + if err != nil { + return fmt.Errorf("prompting for deployment parameter: %w", err) + } - if err != nil { - return false, fmt.Errorf("prompting to save deployment parameter: %w", err) - } + saveParameter, err := p.console.Confirm(ctx, input.ConsoleOptions{ + Message: "Save the value in the environment for future use", + }) - if saveParameter { - p.env.Values[key] = userValue - } + if err != nil { + return fmt.Errorf("prompting to save deployment parameter: %w", err) + } - updatedParameters = true + if saveParameter { + if err := p.env.Config.Set(configKey, userValue); err == nil { + if err := p.env.Save(); err == nil { + // everything went as expected. + } else { + p.console.Message(ctx, fmt.Sprintf("warning: failed to save value: %v", err)) + } + } else { + p.console.Message(ctx, fmt.Sprintf("warning: failed to set value: %v", err)) + } + } + + param.Value = userValue + deployment.Parameters[key] = param + + return nil + }) + if err != nil { + return err + } } } - return updatedParameters, nil + return nil } // NewBicepProvider creates a new instance of a Bicep Infra provider diff --git a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go index cafaf32e8ce..57ae7a1bbed 100644 --- a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go +++ b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go @@ -9,8 +9,6 @@ import ( "encoding/json" "io" "net/http" - "os" - "path" "strings" "testing" @@ -25,7 +23,6 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/infra" . "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" "github.com/azure/azure-dev/cli/azd/pkg/input" - "github.com/azure/azure-dev/cli/azd/pkg/osutil" "github.com/azure/azure-dev/cli/azd/pkg/tools/azcli" "github.com/azure/azure-dev/cli/azd/test/mocks" execmock "github.com/azure/azure-dev/cli/azd/test/mocks/exec" @@ -139,15 +136,13 @@ func TestBicepDeploy(t *testing.T) { azCli := mockazcli.NewAzCliFromMockContext(mockContext) infraProvider := createBicepProvider(mockContext) - tmpPath := t.TempDir() - parametersPath := path.Join(tmpPath, "params.json") - createTmpFile := os.WriteFile(parametersPath, []byte(testArmParametersFile), osutil.PermissionFile) - require.NoError(t, createTmpFile) deploymentPlan := DeploymentPlan{ + Deployment: Deployment{ + Parameters: testArmParameters, + }, Details: BicepDeploymentDetails{ - ParameterFilePath: parametersPath, - Template: to.Ptr(azure.ArmTemplate("{}")), + Template: to.Ptr(azure.ArmTemplate("{}")), }, } @@ -553,13 +548,11 @@ func prepareDestroyMocks(mockContext *mocks.MockContext) { }) } -var testArmParametersFile string = `{ - "parameters": { - "location": { - "value": "West US" - } - } -}` +var testArmParameters = map[string]InputParameter{ + "location": { + Value: "West US", + }, +} func getKeyVaultMock(mockContext *mocks.MockContext, keyVaultString string, name string, location string) { mockContext.HttpClient.When(func(request *http.Request) bool { diff --git a/cli/azd/pkg/infra/scope.go b/cli/azd/pkg/infra/scope.go index 864a3fa0de4..59bb9af81fc 100644 --- a/cli/azd/pkg/infra/scope.go +++ b/cli/azd/pkg/infra/scope.go @@ -19,7 +19,7 @@ type Scope interface { // Gets the url to check deployment progress DeploymentUrl() string // Deploy a given template with a set of parameters. - Deploy(ctx context.Context, template *azure.ArmTemplate, parametersPath string) error + Deploy(ctx context.Context, template *azure.ArmTemplate, parameters azure.ArmParameters) error // GetDeployment fetches the result of the most recent deployment. GetDeployment(ctx context.Context) (*armresources.DeploymentExtended, error) // Gets the resource deployment operations for the current scope @@ -48,8 +48,8 @@ func (s *ResourceGroupScope) ResourceGroup() string { return s.resourceGroup } -func (s *ResourceGroupScope) Deploy(ctx context.Context, template *azure.ArmTemplate, parametersPath string) error { - _, err := s.azCli.DeployToResourceGroup(ctx, s.subscriptionId, s.resourceGroup, s.name, template, parametersPath) +func (s *ResourceGroupScope) Deploy(ctx context.Context, template *azure.ArmTemplate, parameters azure.ArmParameters) error { + _, err := s.azCli.DeployToResourceGroup(ctx, s.subscriptionId, s.resourceGroup, s.name, template, parameters) return err } @@ -107,8 +107,8 @@ func (s *SubscriptionScope) Location() string { } // Deploy a given template with a set of parameters. -func (s *SubscriptionScope) Deploy(ctx context.Context, template *azure.ArmTemplate, parametersPath string) error { - _, err := s.azCli.DeployToSubscription(ctx, s.subscriptionId, s.name, template, parametersPath, s.location) +func (s *SubscriptionScope) Deploy(ctx context.Context, template *azure.ArmTemplate, parameters azure.ArmParameters) error { + _, err := s.azCli.DeployToSubscription(ctx, s.subscriptionId, s.name, template, parameters, s.location) return err } diff --git a/cli/azd/pkg/infra/scope_test.go b/cli/azd/pkg/infra/scope_test.go index b011ce52474..b7bb464f7de 100644 --- a/cli/azd/pkg/infra/scope_test.go +++ b/cli/azd/pkg/infra/scope_test.go @@ -7,14 +7,11 @@ import ( "fmt" "io" "net/http" - "os" - "path" "strings" "testing" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" "github.com/azure/azure-dev/cli/azd/pkg/azure" - "github.com/azure/azure-dev/cli/azd/pkg/osutil" "github.com/azure/azure-dev/cli/azd/pkg/tools/azcli" "github.com/azure/azure-dev/cli/azd/test/mocks" "github.com/azure/azure-dev/cli/azd/test/mocks/mockazcli" @@ -114,10 +111,6 @@ func TestScopeGetDeployment(t *testing.T) { } func TestScopeDeploy(t *testing.T) { - tmpPath := t.TempDir() - parametersPath := path.Join(tmpPath, "params.json") - createTmpFile := os.WriteFile(parametersPath, []byte(testArmParametersFile), osutil.PermissionFile) - require.NoError(t, createTmpFile) t.Run("SubscriptionScopeSuccess", func(t *testing.T) { mockContext := mocks.NewMockContext(context.Background()) @@ -141,7 +134,7 @@ func TestScopeDeploy(t *testing.T) { scope := NewSubscriptionScope(azCli, "eastus2", "SUBSCRIPTION_ID", "DEPLOYMENT_NAME") armTemplate := azure.ArmTemplate(testArmTemplate) - err := scope.Deploy(*mockContext.Context, &armTemplate, parametersPath) + err := scope.Deploy(*mockContext.Context, &armTemplate, testArmParameters) require.NoError(t, err) }) @@ -168,7 +161,7 @@ func TestScopeDeploy(t *testing.T) { scope := NewResourceGroupScope(azCli, "SUBSCRIPTION_ID", "RESOURCE_GROUP", "DEPLOYMENT_NAME") armTemplate := azure.ArmTemplate(testArmTemplate) - err := scope.Deploy(*mockContext.Context, &armTemplate, parametersPath) + err := scope.Deploy(*mockContext.Context, &armTemplate, testArmParameters) require.NoError(t, err) }) } @@ -251,13 +244,11 @@ var testArmResponse string = `{ } ` -var testArmParametersFile string = `{ - "parameters": { - "location": { - "value": "West US" - } - } -}` +var testArmParameters = azure.ArmParameters{ + "location": { + Value: "West US", + }, +} var testArmTemplate string = `{ "$schema": "https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#", diff --git a/cli/azd/pkg/tools/azcli/azcli.go b/cli/azd/pkg/tools/azcli/azcli.go index 2a89b7aae6e..01ab62d7ff0 100644 --- a/cli/azd/pkg/tools/azcli/azcli.go +++ b/cli/azd/pkg/tools/azcli/azcli.go @@ -87,7 +87,8 @@ type AzCli interface { DeployToSubscription( ctx context.Context, subscriptionId, deploymentName string, armTemplate *azure.ArmTemplate, - parametersPath, location string) ( + parameters azure.ArmParameters, + location string) ( AzCliDeploymentResult, error) DeployToResourceGroup( ctx context.Context, @@ -95,7 +96,7 @@ type AzCli interface { resourceGroup, deploymentName string, armTemplate *azure.ArmTemplate, - parametersPath string, + parameters azure.ArmParameters, ) (AzCliDeploymentResult, error) DeleteSubscriptionDeployment(ctx context.Context, subscriptionId string, deploymentName string) error DeleteResourceGroup(ctx context.Context, subscriptionId string, resourceGroupName string) error diff --git a/cli/azd/pkg/tools/azcli/deployments.go b/cli/azd/pkg/tools/azcli/deployments.go index 907d68328dd..19f3fa361e2 100644 --- a/cli/azd/pkg/tools/azcli/deployments.go +++ b/cli/azd/pkg/tools/azcli/deployments.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "io" - "os" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" @@ -77,9 +76,12 @@ func (cli *azCli) createDeploymentsClient( } func (cli *azCli) DeployToSubscription( - ctx context.Context, subscriptionId, deploymentName string, - armTemplate *azure.ArmTemplate, parametersFile, location string) ( - AzCliDeploymentResult, error) { + ctx context.Context, + subscriptionId, deploymentName string, + armTemplate *azure.ArmTemplate, + parameters azure.ArmParameters, + location string, +) (AzCliDeploymentResult, error) { deploymentClient, err := cli.createDeploymentsClient(ctx, subscriptionId) if err != nil { return AzCliDeploymentResult{}, fmt.Errorf("creating deployments client: %w", err) @@ -89,17 +91,13 @@ func (cli *azCli) DeployToSubscription( if err != nil { return AzCliDeploymentResult{}, fmt.Errorf("reading template file: %w", err) } - parametersFileJsonAsMap, err := readJson(parametersFile) - if err != nil { - return AzCliDeploymentResult{}, fmt.Errorf("reading parameters file: %w", err) - } createFromTemplateOperation, err := deploymentClient.BeginCreateOrUpdateAtSubscriptionScope( ctx, deploymentName, armresources.Deployment{ Properties: &armresources.DeploymentProperties{ Template: templateJsonAsMap, - Parameters: parametersFileJsonAsMap["parameters"], + Parameters: parameters, Mode: to.Ptr(armresources.DeploymentModeIncremental), }, Location: to.Ptr(location), @@ -126,9 +124,11 @@ func (cli *azCli) DeployToSubscription( } func (cli *azCli) DeployToResourceGroup( - ctx context.Context, subscriptionId, resourceGroup, deploymentName string, - armTemplate *azure.ArmTemplate, parametersFile string) ( - AzCliDeploymentResult, error) { + ctx context.Context, + subscriptionId, resourceGroup, deploymentName string, + armTemplate *azure.ArmTemplate, + parameters azure.ArmParameters, +) (AzCliDeploymentResult, error) { deploymentClient, err := cli.createDeploymentsClient(ctx, subscriptionId) if err != nil { return AzCliDeploymentResult{}, fmt.Errorf("creating deployments client: %w", err) @@ -138,17 +138,13 @@ func (cli *azCli) DeployToResourceGroup( if err != nil { return AzCliDeploymentResult{}, fmt.Errorf("reading template file: %w", err) } - parametersFileJsonAsMap, err := readJson(parametersFile) - if err != nil { - return AzCliDeploymentResult{}, fmt.Errorf("reading parameters file: %w", err) - } createFromTemplateOperation, err := deploymentClient.BeginCreateOrUpdate( ctx, resourceGroup, deploymentName, armresources.Deployment{ Properties: &armresources.DeploymentProperties{ Template: templateJsonAsMap, - Parameters: parametersFileJsonAsMap["parameters"], + Parameters: parameters, Mode: to.Ptr(armresources.DeploymentModeIncremental), }, }, nil) @@ -193,14 +189,6 @@ func (cli *azCli) DeleteSubscriptionDeployment(ctx context.Context, subscription return nil } -func readJson(path string) (map[string]interface{}, error) { - templateFile, err := os.ReadFile(path) - if err != nil { - return nil, err - } - return readFromString(templateFile) -} - func readFromString(jsonBytes []byte) (map[string]interface{}, error) { template := make(map[string]interface{}) if err := json.Unmarshal(jsonBytes, &template); err != nil {