address PR review: extract bicepparam compile to helper, temp-file cleanup, docs fixes, exact assertion count

Author: bgdnext64

cmd/bicepCmd.go

@@ -33,6 +33,7 @@ import (
 	"github.com/Azure/mpf/pkg/domain"
 	"github.com/Azure/mpf/pkg/infrastructure/ARMTemplateShared"
 	"github.com/Azure/mpf/pkg/infrastructure/authorizationCheckers/ARMTemplateDeployment"
+	"github.com/Azure/mpf/pkg/infrastructure/bicepUtils"
 	"github.com/Azure/mpf/pkg/infrastructure/mpfSharedUtils"
 	resourceGroupManager "github.com/Azure/mpf/pkg/infrastructure/resourceGroupManager"
 	sproleassignmentmanager "github.com/Azure/mpf/pkg/infrastructure/spRoleAssignmentManager"
@@ -129,18 +130,25 @@ func getMPFBicep(cmd *cobra.Command, args []string) {
 		log.Errorf("Error getting absolute path for parameters file: %v\n", err)
 	}
 
-	// If the parameters file is a .bicepparam file, compile it to ARM JSON format
-	if strings.HasSuffix(strings.ToLower(flgParametersFilePath), ".bicepparam") {
+	// If the parameters file is a .bicepparam file, compile it to ARM JSON format.
+	// Compilation goes to a temp file (rather than next to the source) so we never
+	// silently clobber an existing "<name>.parameters.json" the user already has,
+	// and so the artifact is reliably cleaned up on exit.
+	if bicepUtils.IsBicepParamFile(flgParametersFilePath) {
 		log.Infoln("Detected .bicepparam file, compiling to ARM parameters JSON format")
-		compiledParamsPath := strings.TrimSuffix(flgParametersFilePath, filepath.Ext(flgParametersFilePath)) + ".parameters.json"
-		buildParamsCmd := exec.Command(flgBicepExecPath, "build-params", flgParametersFilePath, "--outfile", compiledParamsPath)
-		buildParamsCmd.Dir = filepath.Dir(flgParametersFilePath)
 
-		output, err := buildParamsCmd.CombinedOutput()
+		compiledParamsPath, err := bicepUtils.CompileBicepParamsToTempFile(flgBicepExecPath, flgParametersFilePath)
 		if err != nil {
-			log.Fatalf("error running bicep build-params: %s\n%s", err, string(output))
+			log.Fatalf("error compiling .bicepparam file: %v", err)
 		}
-		log.Infoln("Bicep parameters compiled successfully, ARM Parameters JSON created at:", compiledParamsPath)
+
+		defer func(path string) {
+			if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+				log.Warnf("error removing temporary ARM parameters file %q: %v", path, err)
+			}
+		}(compiledParamsPath)
+
+		log.Infoln("Bicep parameters compiled successfully, temporary ARM Parameters JSON created at:", compiledParamsPath)
 		flgParametersFilePath = compiledParamsPath
 	}
 

docs/installation-and-quickstart.md

@@ -230,7 +230,9 @@ $env:MPF_SPOBJECTID = "YOUR_SP_OBJECT_ID"
 $env:MPF_BICEPEXECPATH = (Get-Command bicep).Source # Dynamically resolves to the Bicep executable path, works across different installation locations
 
 .\azmpf.exe bicep --bicepFilePath .\samples\bicep\storage-account-simple.bicep --parametersFilePath .\samples\bicep\storage-account-simple-params.json --jsonOutput --verbose
-```#### Bicep with .bicepparam Parameters File
+```
+
+#### Bicep with .bicepparam Parameters File
 
 MPF also supports Bicep-native `.bicepparam` parameter files. These are automatically compiled to ARM JSON format using `bicep build-params` before deployment. The `.bicepparam` file uses a `using` directive to reference the Bicep template:
 
@@ -263,7 +265,7 @@ $env:MPF_SPCLIENTSECRET = "YOUR_SP_CLIENT_SECRET"
 $env:MPF_SPOBJECTID = "YOUR_SP_OBJECT_ID"
 $env:MPF_BICEPEXECPATH = (Get-Command bicep).Source
 
-.\.azmpf.exe bicep --bicepFilePath .\samples\bicep\storage-account-simple.bicep --parametersFilePath .\samples\bicep\storage-account-simple-params.bicepparam --jsonOutput --verbose
+.\azmpf.exe bicep --bicepFilePath .\samples\bicep\storage-account-simple.bicep --parametersFilePath .\samples\bicep\storage-account-simple-params.bicepparam --jsonOutput --verbose
 ```
 
 ### Terraform

e2eTests/e2eBicep_test.go

@@ -32,6 +32,7 @@ import (
 
 	"github.com/Azure/mpf/pkg/infrastructure/ARMTemplateShared"
 	"github.com/Azure/mpf/pkg/infrastructure/authorizationCheckers/ARMTemplateDeployment"
+	"github.com/Azure/mpf/pkg/infrastructure/bicepUtils"
 	mpfSharedUtils "github.com/Azure/mpf/pkg/infrastructure/mpfSharedUtils"
 	rgm "github.com/Azure/mpf/pkg/infrastructure/resourceGroupManager"
 	spram "github.com/Azure/mpf/pkg/infrastructure/spRoleAssignmentManager"
@@ -198,33 +199,37 @@ func TestBicepWithBicepparamFile(t *testing.T) {
 	}
 
 	bicepExecPath := os.Getenv("MPF_BICEPEXECPATH")
-	bicepFilePath := "../samples/bicep/storage-account-simple.bicep"
-	parametersFilePath := "../samples/bicep/storage-account-simple-params.bicepparam"
 
-	bicepFilePath, _ = getAbsolutePath(bicepFilePath)
-	parametersFilePath, _ = getAbsolutePath(parametersFilePath)
-
-	// Compile .bicepparam to ARM JSON parameters
-	compiledParamsPath := strings.TrimSuffix(parametersFilePath, filepath.Ext(parametersFilePath)) + ".parameters.json"
-	buildParamsCmd := exec.Command(bicepExecPath, "build-params", parametersFilePath, "--outfile", compiledParamsPath)
-	buildParamsCmd.Dir = filepath.Dir(parametersFilePath)
+	bicepFilePath, err := getAbsolutePath("../samples/bicep/storage-account-simple.bicep")
+	if err != nil {
+		t.Fatalf("failed to resolve absolute path for bicep file: %v", err)
+	}
+	parametersFilePath, err := getAbsolutePath("../samples/bicep/storage-account-simple-params.bicepparam")
+	if err != nil {
+		t.Fatalf("failed to resolve absolute path for parameters file: %v", err)
+	}
 
-	output, err := buildParamsCmd.CombinedOutput()
+	// Exercise the same compile helper used by the bicep CLI command so this
+	// test covers the auto-compile-on-.bicepparam code path.
+	if !bicepUtils.IsBicepParamFile(parametersFilePath) {
+		t.Fatalf("expected %q to be detected as a .bicepparam file", parametersFilePath)
+	}
+	compiledParamsPath, err := bicepUtils.CompileBicepParamsToTempFile(bicepExecPath, parametersFilePath)
 	if err != nil {
-		log.Errorf("error running bicep build-params: %s\n%s", err, string(output))
-		t.Fatal(err)
+		t.Fatalf("error compiling .bicepparam file: %v", err)
 	}
 	t.Cleanup(func() { _ = os.Remove(compiledParamsPath) })
 
 	// Build bicep to ARM template
 	armTemplatePath := strings.TrimSuffix(bicepFilePath, ".bicep") + ".json"
+	t.Cleanup(func() { _ = os.Remove(armTemplatePath) })
+
 	bicepCmd := exec.Command(bicepExecPath, "build", bicepFilePath, "--outfile", armTemplatePath)
 	bicepCmd.Dir = filepath.Dir(bicepFilePath)
 
-	_, err = bicepCmd.CombinedOutput()
+	output, err := bicepCmd.CombinedOutput()
 	if err != nil {
-		log.Error(err)
-		t.Error(err)
+		t.Fatalf("error running bicep build: %s\n%s", err, string(output))
 	}
 
 	ctx := t.Context()
@@ -256,13 +261,13 @@ func TestBicepWithBicepparamFile(t *testing.T) {
 		t.Error(err)
 	}
 
-	// Storage account deployment requires these permissions:
+	// Storage account deployment requires exactly these 4 permissions:
 	// Microsoft.Resources/deployments/read
 	// Microsoft.Resources/deployments/write
 	// Microsoft.Storage/storageAccounts/read
 	// Microsoft.Storage/storageAccounts/write
 	assert.NotEmpty(t, mpfResult.RequiredPermissions)
-	assert.GreaterOrEqual(t, len(mpfResult.RequiredPermissions[mpfConfig.SubscriptionID]), 4)
+	assert.Equal(t, 4, len(mpfResult.RequiredPermissions[mpfConfig.SubscriptionID]))
 }
 
 func getAbsolutePath(path string) (string, error) {

pkg/infrastructure/bicepUtils/compileBicepParams.go

@@ -0,0 +1,69 @@
+//     MIT License
+//
+//     Copyright (c) Microsoft Corporation.
+//
+//     Permission is hereby granted, free of charge, to any person obtaining a copy
+//     of this software and associated documentation files (the "Software"), to deal
+//     in the Software without restriction, including without limitation the rights
+//     to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+//     copies of the Software, and to permit persons to whom the Software is
+//     furnished to do so, subject to the following conditions:
+//
+//     The above copyright notice and this permission notice shall be included in all
+//     copies or substantial portions of the Software.
+//
+//     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+//     IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+//     FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+//     AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+//     LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+//     OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+//     SOFTWARE
+
+// Package bicepUtils contains helpers for working with Bicep input files,
+// such as compiling .bicepparam files to ARM JSON parameter files.
+package bicepUtils
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+)
+
+// IsBicepParamFile reports whether the given path refers to a .bicepparam file.
+func IsBicepParamFile(path string) bool {
+	return strings.HasSuffix(strings.ToLower(path), ".bicepparam")
+}
+
+// CompileBicepParamsToTempFile compiles the given .bicepparam file to an ARM
+// JSON parameters file using `bicep build-params`. The output is written to a
+// freshly-created temp file, so any pre-existing "<name>.parameters.json" next
+// to the source file is left untouched.
+//
+// The caller is responsible for removing the returned path when no longer
+// needed (typically via defer / t.Cleanup). If an error occurs, the temp file
+// is removed before returning.
+func CompileBicepParamsToTempFile(bicepExecPath, paramsFilePath string) (string, error) {
+	tempFile, err := os.CreateTemp("", "mpf-bicepparam-*.parameters.json")
+	if err != nil {
+		return "", fmt.Errorf("creating temporary ARM parameters file: %w", err)
+	}
+	compiledPath := tempFile.Name()
+	if err := tempFile.Close(); err != nil {
+		_ = os.Remove(compiledPath)
+		return "", fmt.Errorf("closing temporary ARM parameters file: %w", err)
+	}
+
+	cmd := exec.Command(bicepExecPath, "build-params", paramsFilePath, "--outfile", compiledPath)
+	cmd.Dir = filepath.Dir(paramsFilePath)
+
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		_ = os.Remove(compiledPath)
+		return "", fmt.Errorf("running bicep build-params: %w\n%s", err, string(output))
+	}
+
+	return compiledPath, nil
+}

pkg/infrastructure/bicepUtils/compileBicepParams_test.go

@@ -0,0 +1,46 @@
+//     MIT License
+//
+//     Copyright (c) Microsoft Corporation.
+//
+//     Permission is hereby granted, free of charge, to any person obtaining a copy
+//     of this software and associated documentation files (the "Software"), to deal
+//     in the Software without restriction, including without limitation the rights
+//     to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+//     copies of the Software, and to permit persons to whom the Software is
+//     furnished to do so, subject to the following conditions:
+//
+//     The above copyright notice and this permission notice shall be included in all
+//     copies or substantial portions of the Software.
+//
+//     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+//     IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+//     FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+//     AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+//     LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+//     OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+//     SOFTWARE
+
+package bicepUtils
+
+import "testing"
+
+func TestIsBicepParamFile(t *testing.T) {
+	cases := []struct {
+		path string
+		want bool
+	}{
+		{"params.bicepparam", true},
+		{"PARAMS.BICEPPARAM", true},
+		{"/some/path/Params.BicepParam", true},
+		{"params.json", false},
+		{"main.bicep", false},
+		{"", false},
+		{"params.bicepparam.bak", false},
+	}
+
+	for _, tc := range cases {
+		if got := IsBicepParamFile(tc.path); got != tc.want {
+			t.Errorf("IsBicepParamFile(%q) = %v, want %v", tc.path, got, tc.want)
+		}
+	}
+}