Skip to content

Commit aa4aeb2

Browse files
Copilotvhvb1989
andauthored
Fix bicep CLI uninitialized path in container app deployments (#6610)
* fix security issue with playwright/test 1.49.1 (#6592) * Checkpoint from VS Code for cloud agent session * Fix bicep CLI initialization by restoring EnsureInstalled calls - Restore public EnsureInstalled() method in bicep.go - Remove automatic installation from Build() and BuildBicepParam() - Add explicit EnsureInstalled() calls in service_target_containerapp.go and bicep_provider.go - Update all test files to use public EnsureInstalled() method - Reverts incorrect changes from previous checkpoint commit This fix follows the same pattern as PR #6593 for GitHub CLI. Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com> * Checkpoint from VS Code for cloud agent session --------- Co-authored-by: Victor Vazquez <vhvb1989@gmail.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
1 parent 2fc6e71 commit aa4aeb2

File tree

7 files changed

+23
-25
lines changed

7 files changed

+23
-25
lines changed

cli/azd/internal/scaffold/scaffold_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,6 @@ func TestExecInfra(t *testing.T) {
257257

258258
ctx := context.Background()
259259
cli := bicep.NewCli(mockinput.NewMockConsole(), exec.NewCommandRunner(nil))
260-
err = cli.EnsureInstalled(ctx)
261-
require.NoError(t, err)
262260

263261
res, err := cli.Build(ctx, filepath.Join(dir, "main.bicep"))
264262
require.NoError(t, err)

cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,6 @@ func (p *BicepProvider) Name() string {
9595
// Initialize initializes provider state from the options.
9696
// It also calls EnsureEnv, which ensures the client-side state is ready for provisioning.
9797
func (p *BicepProvider) Initialize(ctx context.Context, projectPath string, opt provisioning.Options) error {
98-
// Ensure bicep CLI is installed before any operations
99-
if err := p.bicepCli.EnsureInstalled(ctx); err != nil {
100-
return fmt.Errorf("ensuring bicep is installed: %w", err)
101-
}
102-
10398
infraOptions, err := opt.GetWithDefaults()
10499
if err != nil {
105100
return err

cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1093,7 +1093,6 @@ func TestUserDefinedTypes(t *testing.T) {
10931093

10941094
azCli := mockazapi.NewAzureClientFromMockContext(mockContext)
10951095
bicepCli := bicep.NewCli(mockContext.Console, mockContext.CommandRunner)
1096-
require.NoError(t, bicepCli.EnsureInstalled(*mockContext.Context))
10971096
env := environment.NewWithValues("test-env", map[string]string{})
10981097

10991098
mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool {

cli/azd/pkg/project/service_target_containerapp.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,11 +229,7 @@ func (at *containerAppTarget) Deploy(
229229
fetchBicepCli := at.bicepCli
230230
if fetchBicepCli == nil {
231231
fetchBicepCli = func() (*bicep.Cli, error) {
232-
cli := bicep.NewCli(at.console, at.commandRunner)
233-
if err := cli.EnsureInstalled(ctx); err != nil {
234-
return nil, err
235-
}
236-
return cli, nil
232+
return bicep.NewCli(at.console, at.commandRunner), nil
237233
}
238234
}
239235

cli/azd/pkg/tools/bicep/bicep.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,14 @@ import (
2828
// user).
2929
var Version semver.Version = semver.MustParse("0.39.26")
3030

31-
// Cli is a wrapper around the bicep CLI. Call EnsureInstalled before using Build or BuildBicepParam.
31+
// Cli is a wrapper around the bicep CLI.
32+
// The CLI automatically ensures bicep is installed before executing commands.
33+
//
34+
// Concurrency notes: The sync.Once is per-instance, not global. In normal operation, the IoC
35+
// container registers Cli as a singleton, so one shared instance is used. However, some code paths
36+
// (e.g., service_target_containerapp.go) create new instances inline. If multiple instances race to
37+
// install bicep concurrently, this is safe because downloadBicep uses atomic file operations
38+
// (temp file + rename). The only downside is potentially redundant downloads, which is rare and harmless.
3239
type Cli struct {
3340
path string
3441
runner exec.CommandRunner
@@ -39,8 +46,8 @@ type Cli struct {
3946
installErr error
4047
}
4148

42-
// NewCli creates a new Bicep CLI wrapper. The CLI is not yet installed; call EnsureInstalled
43-
// before using Build or BuildBicepParam methods.
49+
// NewCli creates a new Bicep CLI wrapper.
50+
// The CLI automatically ensures bicep is installed when Build or BuildBicepParam is called.
4451
func NewCli(console input.Console, commandRunner exec.CommandRunner) *Cli {
4552
return newCliWithTransporter(console, commandRunner, http.DefaultClient)
4653
}
@@ -58,10 +65,9 @@ func newCliWithTransporter(
5865
}
5966
}
6067

61-
// EnsureInstalled checks if bicep is available and downloads/upgrades if needed.
68+
// ensureInstalledOnce checks if bicep is available and downloads/upgrades if needed.
6269
// This is safe to call multiple times; installation only happens once.
63-
// Should be called with a request-scoped context before first use.
64-
func (cli *Cli) EnsureInstalled(ctx context.Context) error {
70+
func (cli *Cli) ensureInstalledOnce(ctx context.Context) error {
6571
cli.installOnce.Do(func() {
6672
cli.installErr = cli.ensureInstalled(ctx)
6773
})
@@ -270,6 +276,10 @@ type BuildResult struct {
270276
}
271277

272278
func (cli *Cli) Build(ctx context.Context, file string) (BuildResult, error) {
279+
if err := cli.ensureInstalledOnce(ctx); err != nil {
280+
return BuildResult{}, fmt.Errorf("ensuring bicep is installed: %w", err)
281+
}
282+
273283
args := []string{"build", file, "--stdout"}
274284
buildRes, err := cli.runCommand(ctx, nil, args...)
275285

@@ -287,6 +297,10 @@ func (cli *Cli) Build(ctx context.Context, file string) (BuildResult, error) {
287297
}
288298

289299
func (cli *Cli) BuildBicepParam(ctx context.Context, file string, env []string) (BuildResult, error) {
300+
if err := cli.ensureInstalledOnce(ctx); err != nil {
301+
return BuildResult{}, fmt.Errorf("ensuring bicep is installed: %w", err)
302+
}
303+
290304
args := []string{"build-params", file, "--stdout"}
291305
buildRes, err := cli.runCommand(ctx, env, args...)
292306

cli/azd/pkg/tools/bicep/bicep_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestNewBicepCli(t *testing.T) {
4848
cli := newCliWithTransporter(
4949
mockContext.Console, mockContext.CommandRunner, mockContext.HttpClient,
5050
)
51-
err := cli.EnsureInstalled(*mockContext.Context)
51+
err := cli.ensureInstalledOnce(*mockContext.Context)
5252
require.NoError(t, err)
5353
require.NotNil(t, cli)
5454

@@ -121,7 +121,7 @@ func TestNewBicepCliWillUpgrade(t *testing.T) {
121121
cli := newCliWithTransporter(
122122
mockContext.Console, mockContext.CommandRunner, mockContext.HttpClient,
123123
)
124-
err = cli.EnsureInstalled(*mockContext.Context)
124+
err = cli.ensureInstalledOnce(*mockContext.Context)
125125
require.NoError(t, err)
126126
require.NotNil(t, cli)
127127

cli/azd/test/functional/aspire_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,6 @@ func Test_CLI_Aspire_DetectGen(t *testing.T) {
179179
require.NoError(t, err)
180180

181181
bicepCli := bicep.NewCli(mockinput.NewMockConsole(), exec.NewCommandRunner(nil))
182-
err = bicepCli.EnsureInstalled(ctx)
183-
require.NoError(t, err)
184182

185183
// Validate bicep builds without errors
186184
// cdk lint errors are expected
@@ -232,8 +230,6 @@ func Test_CLI_Aspire_DetectGen(t *testing.T) {
232230
require.NoError(t, err)
233231

234232
bicepCli := bicep.NewCli(mockinput.NewMockConsole(), exec.NewCommandRunner(nil))
235-
err = bicepCli.EnsureInstalled(ctx)
236-
require.NoError(t, err)
237233

238234
// Validate bicep builds without errors
239235
// cdk lint errors are expected

0 commit comments

Comments
 (0)