diff --git a/AGENTS.md b/AGENTS.md index 700a163..dd963bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,11 +28,12 @@ When asked to analyze Azure IaC for minimum permissions, follow this process: ### Environment Variables (Required) ```bash -export MPF_SUBSCRIPTIONID="" -export MPF_TENANTID="" -export MPF_SPCLIENTID="" -export MPF_SPCLIENTSECRET="" -export MPF_SPOBJECTID="" +# Preferred snake_case form (legacy concatenated names like MPF_SUBSCRIPTIONID still work) +export MPF_SUBSCRIPTION_ID="" +export MPF_TENANT_ID="" +export MPF_SP_CLIENT_ID="" +export MPF_SP_CLIENT_SECRET="" +export MPF_SP_OBJECT_ID="" ``` ### Service Principal Setup @@ -41,11 +42,11 @@ Create a dedicated Service Principal for MPF analysis (it should have NO roles a ```bash MPF_SP=$(az ad sp create-for-rbac --name "MPF-Analyzer-SP" --skip-assignment) -export MPF_SPCLIENTID=$(echo $MPF_SP | jq -r .appId) -export MPF_SPCLIENTSECRET=$(echo $MPF_SP | jq -r .password) -export MPF_SPOBJECTID=$(az ad sp show --id $MPF_SPCLIENTID --query id -o tsv) -export MPF_TENANTID=$(az account show --query tenantId -o tsv) -export MPF_SUBSCRIPTIONID=$(az account show --query id -o tsv) +export MPF_SP_CLIENT_ID=$(echo $MPF_SP | jq -r .appId) +export MPF_SP_CLIENT_SECRET=$(echo $MPF_SP | jq -r .password) +export MPF_SP_OBJECT_ID=$(az ad sp show --id $MPF_SP_CLIENT_ID --query id -o tsv) +export MPF_TENANT_ID=$(az account show --query tenantId -o tsv) +export MPF_SUBSCRIPTION_ID=$(az account show --query id -o tsv) ``` --- @@ -90,7 +91,7 @@ azmpf arm \ ### Bicep Analysis ```bash -export MPF_BICEPEXECPATH=$(which bicep) +export MPF_BICEP_EXEC_PATH=$(which bicep) azmpf bicep \ --bicepFilePath ./path/to/main.bicep \ @@ -102,7 +103,7 @@ azmpf bicep \ ### Terraform Analysis ```bash -export MPF_TFPATH=$(which terraform) +export MPF_TF_PATH=$(which terraform) # Ensure terraform is initialized cd ./terraform-module-dir @@ -177,7 +178,7 @@ For least-privilege access, generate a custom role: After analysis, delete the Service Principal: ```bash -az ad sp delete --id "$MPF_SPCLIENTID" +az ad sp delete --id "$MPF_SP_CLIENT_ID" ``` --- diff --git a/README.md b/README.md index efafca7..c659d61 100644 --- a/README.md +++ b/README.md @@ -103,21 +103,21 @@ To run the unit tests, run `task testunit`. To run the end-to-end tests for ARM, you need to have the following environment variables set, and then execute `task teste2e:arm`: ```shell -# bash -export MPF_SUBSCRIPTIONID="YOUR_SUBSCRIPTION_ID" -export MPF_TENANTID="YOUR_TENANT_ID" -export MPF_SPCLIENTID="YOUR_SP_CLIENT_ID" -export MPF_SPCLIENTSECRET="YOUR_SP_CLIENT_SECRET" -export MPF_SPOBJECTID="YOUR_SP_OBJECT_ID" +# bash (snake_case preferred; legacy names like MPF_SUBSCRIPTIONID still work for the CLI) +export MPF_SUBSCRIPTION_ID="YOUR_SUBSCRIPTION_ID" +export MPF_TENANT_ID="YOUR_TENANT_ID" +export MPF_SP_CLIENT_ID="YOUR_SP_CLIENT_ID" +export MPF_SP_CLIENT_SECRET="YOUR_SP_CLIENT_SECRET" +export MPF_SP_OBJECT_ID="YOUR_SP_OBJECT_ID" ``` ```powershell # powershell -$env:MPF_SUBSCRIPTIONID="YOUR_SUBSCRIPTION_ID" -$env:MPF_TENANTID="YOUR_TENANT_ID" -$env:MPF_SPCLIENTID="YOUR_SP_CLIENT_ID" -$env:MPF_SPCLIENTSECRET="YOUR_SP_CLIENT_SECRET" -$env:MPF_SPOBJECTID="YOUR_SP_OBJECT_ID" +$env:MPF_SUBSCRIPTION_ID="YOUR_SUBSCRIPTION_ID" +$env:MPF_TENANT_ID="YOUR_TENANT_ID" +$env:MPF_SP_CLIENT_ID="YOUR_SP_CLIENT_ID" +$env:MPF_SP_CLIENT_SECRET="YOUR_SP_CLIENT_SECRET" +$env:MPF_SP_OBJECT_ID="YOUR_SP_OBJECT_ID" ``` ```shell @@ -130,22 +130,22 @@ To run the end-to-end tests for Bicep, you need to have the following environmen ```shell # bash -export MPF_SUBSCRIPTIONID="YOUR_SUBSCRIPTION_ID" -export MPF_TENANTID="YOUR_TENANT_ID" -export MPF_SPCLIENTID="YOUR_SP_CLIENT_ID" -export MPF_SPCLIENTSECRET="YOUR_SP_CLIENT_SECRET" -export MPF_SPOBJECTID="YOUR_SP_OBJECT_ID" -export MPF_BICEPEXECPATH="/opt/homebrew/bin/bicep" # Path to the Bicep executable +export MPF_SUBSCRIPTION_ID="YOUR_SUBSCRIPTION_ID" +export MPF_TENANT_ID="YOUR_TENANT_ID" +export MPF_SP_CLIENT_ID="YOUR_SP_CLIENT_ID" +export MPF_SP_CLIENT_SECRET="YOUR_SP_CLIENT_SECRET" +export MPF_SP_OBJECT_ID="YOUR_SP_OBJECT_ID" +export MPF_BICEP_EXEC_PATH="/opt/homebrew/bin/bicep" # Path to the Bicep executable ``` ```powershell # powershell -$env:MPF_SUBSCRIPTIONID="YOUR_SUBSCRIPTION_ID" -$env:MPF_TENANTID="YOUR_TENANT_ID" -$env:MPF_SPCLIENTID="YOUR_SP_CLIENT_ID" -$env:MPF_SPCLIENTSECRET="YOUR_SP_CLIENT_SECRET" -$env:MPF_SPOBJECTID="YOUR_SP_OBJECT_ID" -$env:MPF_BICEPEXECPATH=$(where.exe bicep) +$env:MPF_SUBSCRIPTION_ID="YOUR_SUBSCRIPTION_ID" +$env:MPF_TENANT_ID="YOUR_TENANT_ID" +$env:MPF_SP_CLIENT_ID="YOUR_SP_CLIENT_ID" +$env:MPF_SP_CLIENT_SECRET="YOUR_SP_CLIENT_SECRET" +$env:MPF_SP_OBJECT_ID="YOUR_SP_OBJECT_ID" +$env:MPF_BICEP_EXEC_PATH=$(where.exe bicep) ``` ```shell @@ -158,22 +158,22 @@ The Terraform end-to-end tests can take a long time to execute, depending on the ```shell # bash -export MPF_SUBSCRIPTIONID="YOUR_SUBSCRIPTION_ID" -export MPF_TENANTID="YOUR_TENANT_ID" -export MPF_SPCLIENTID="YOUR_SP_CLIENT_ID" -export MPF_SPCLIENTSECRET="YOUR_SP_CLIENT_SECRET" -export MPF_SPOBJECTID="YOUR_SP_OBJECT_ID" -export MPF_TFPATH=$(which terraform) # Path to the Terraform executable +export MPF_SUBSCRIPTION_ID="YOUR_SUBSCRIPTION_ID" +export MPF_TENANT_ID="YOUR_TENANT_ID" +export MPF_SP_CLIENT_ID="YOUR_SP_CLIENT_ID" +export MPF_SP_CLIENT_SECRET="YOUR_SP_CLIENT_SECRET" +export MPF_SP_OBJECT_ID="YOUR_SP_OBJECT_ID" +export MPF_TF_PATH=$(which terraform) # Path to the Terraform executable ``` ```powershell # powershell -$env:MPF_SUBSCRIPTIONID="YOUR_SUBSCRIPTION_ID" -$env:MPF_TENANTID="YOUR_TENANT_ID" -$env:MPF_SPCLIENTID="YOUR_SP_CLIENT_ID" -$env:MPF_SPCLIENTSECRET="YOUR_SP_CLIENT_SECRET" -$env:MPF_SPOBJECTID="YOUR_SP_OBJECT_ID" -$env:MPF_TFPATH=$(where.exe terraform) +$env:MPF_SUBSCRIPTION_ID="YOUR_SUBSCRIPTION_ID" +$env:MPF_TENANT_ID="YOUR_TENANT_ID" +$env:MPF_SP_CLIENT_ID="YOUR_SP_CLIENT_ID" +$env:MPF_SP_CLIENT_SECRET="YOUR_SP_CLIENT_SECRET" +$env:MPF_SP_OBJECT_ID="YOUR_SP_OBJECT_ID" +$env:MPF_TF_PATH=$(where.exe terraform) ``` ```shell diff --git a/cmd/envbinding.go b/cmd/envbinding.go new file mode 100644 index 0000000..e928295 --- /dev/null +++ b/cmd/envbinding.go @@ -0,0 +1,119 @@ +// 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 main + +import ( + "fmt" + "strings" + "unicode" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/spf13/viper" + + log "github.com/sirupsen/logrus" +) + +// camelToSnakeCase converts a camelCase or PascalCase identifier to snake_case. +// Consecutive uppercase acronyms are handled so that "subscriptionID" becomes +// "subscription_id" and "spClientID" becomes "sp_client_id". +func camelToSnakeCase(s string) string { + if s == "" { + return s + } + + runes := []rune(s) + var b strings.Builder + b.Grow(len(s) + 4) + + for i, r := range runes { + if unicode.IsUpper(r) { + // Insert underscore before this uppercase rune when it starts a new word: + // - previous rune is lowercase (e.g. nID -> n_id), or + // - previous is uppercase and the next is lowercase (end of acronym before a new word). + if i > 0 { + prev := runes[i-1] + nextIsLower := i+1 < len(runes) && unicode.IsLower(runes[i+1]) + if unicode.IsLower(prev) || (unicode.IsUpper(prev) && nextIsLower) { + b.WriteByte('_') + } + } + b.WriteRune(unicode.ToLower(r)) + continue + } + b.WriteRune(r) + } + + return b.String() +} + +// envNamesForFlag returns the full environment variable names accepted for a +// given cobra/viper flag name. +// +// Two forms are supported for backward compatibility: +// 1. Legacy concatenated form derived from the flag name as-is, e.g. subscriptionID -> MPF_SUBSCRIPTIONID +// 2. Snake_case form, e.g. subscriptionID -> MPF_SUBSCRIPTION_ID +// +// When both resolve to the same name (single-word flags like "verbose"), only one entry is returned. +// Order is legacy first, then snake_case, so existing deployments keep their current value if both are set. +func envNamesForFlag(flagName string) []string { + legacy := envPrefix + "_" + strings.ToUpper(flagName) + snake := envPrefix + "_" + strings.ToUpper(camelToSnakeCase(flagName)) + + if legacy == snake { + return []string{legacy} + } + return []string{legacy, snake} +} + +// bindFlags applies viper config and environment values to cobra flags that +// were not set on the command line. Each flag accepts both the legacy +// concatenated env var name and the snake_case form (see envNamesForFlag). +func bindFlags(cmd *cobra.Command, v *viper.Viper) { + cmd.Flags().VisitAll(func(f *pflag.Flag) { + configName := f.Name + // If using camelCase in the config file, replace hyphens with a camelCased string. + // Since viper does case-insensitive comparisons, we don't need to bother fixing the case, and only need to remove the hyphens. + if replaceHyphenWithCamelCase { + configName = strings.ReplaceAll(f.Name, "-", "") + } + + // Explicit BindEnv is required so both legacy and snake_case env names are + // recognized. When BindEnv is given explicit names they are used as-is + // (prefix is not re-applied). + envNames := envNamesForFlag(configName) + bindArgs := append([]string{configName}, envNames...) + if err := v.BindEnv(bindArgs...); err != nil { + log.Errorf("Error binding env vars for flag %s: %v\n", f.Name, err) + } + + // Apply the viper config value to the flag when the flag is not set and viper has a value + if !f.Changed && v.IsSet(configName) { + val := v.Get(configName) + err := cmd.Flags().Set(f.Name, fmt.Sprintf("%v", val)) + if err != nil { + log.Errorf("Error setting flag %s: %v\n", f.Name, err) + } + } + }) +} diff --git a/cmd/envbinding_test.go b/cmd/envbinding_test.go new file mode 100644 index 0000000..827b1f6 --- /dev/null +++ b/cmd/envbinding_test.go @@ -0,0 +1,389 @@ +// 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 main + +import ( + "os" + "reflect" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/viper" +) + +func TestCamelToSnakeCase(t *testing.T) { + tests := []struct { + in string + want string + }{ + {"", ""}, + {"verbose", "verbose"}, + {"debug", "debug"}, + {"location", "location"}, + {"subscriptionID", "subscription_id"}, + {"tenantID", "tenant_id"}, + {"spClientID", "sp_client_id"}, + {"spObjectID", "sp_object_id"}, + {"spClientSecret", "sp_client_secret"}, + {"showDetailedOutput", "show_detailed_output"}, + {"jsonOutput", "json_output"}, + {"initialPermissions", "initial_permissions"}, + {"templateFilePath", "template_file_path"}, + {"parametersFilePath", "parameters_file_path"}, + {"resourceGroupNamePfx", "resource_group_name_pfx"}, + {"deploymentNamePfx", "deployment_name_pfx"}, + {"bicepFilePath", "bicep_file_path"}, + {"bicepExecPath", "bicep_exec_path"}, + {"tfPath", "tf_path"}, + {"workingDir", "working_dir"}, + {"varFilePath", "var_file_path"}, + {"importExistingResourcesToState", "import_existing_resources_to_state"}, + {"targetModule", "target_module"}, + // Acronym edge cases + {"JSONOutput", "json_output"}, + {"parseURLToHTML", "parse_url_to_html"}, + } + + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + got := camelToSnakeCase(tt.in) + if got != tt.want { + t.Errorf("camelToSnakeCase(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +func TestEnvNamesForFlag(t *testing.T) { + tests := []struct { + flag string + want []string + }{ + {"verbose", []string{"MPF_VERBOSE"}}, + {"subscriptionID", []string{"MPF_SUBSCRIPTIONID", "MPF_SUBSCRIPTION_ID"}}, + {"spClientID", []string{"MPF_SPCLIENTID", "MPF_SP_CLIENT_ID"}}, + {"showDetailedOutput", []string{"MPF_SHOWDETAILEDOUTPUT", "MPF_SHOW_DETAILED_OUTPUT"}}, + {"importExistingResourcesToState", []string{"MPF_IMPORTEXISTINGRESOURCESTOSTATE", "MPF_IMPORT_EXISTING_RESOURCES_TO_STATE"}}, + {"templateFilePath", []string{"MPF_TEMPLATEFILEPATH", "MPF_TEMPLATE_FILE_PATH"}}, + } + + for _, tt := range tests { + t.Run(tt.flag, func(t *testing.T) { + got := envNamesForFlag(tt.flag) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("envNamesForFlag(%q) = %v, want %v", tt.flag, got, tt.want) + } + }) + } +} + +// unsetEnvForTest removes key for the duration of the test and restores it afterward. +// Empty-string values are not used because LookupEnv treats "" as set, which would +// make the legacy env name shadow snake_case values (legacy is checked first). +func unsetEnvForTest(t *testing.T, key string) { + t.Helper() + orig, had := os.LookupEnv(key) + if err := os.Unsetenv(key); err != nil { + t.Fatalf("unsetenv %s: %v", key, err) + } + t.Cleanup(func() { + if had { + _ = os.Setenv(key, orig) + } else { + _ = os.Unsetenv(key) + } + }) +} + +// newTestViper mirrors initializeConfig's viper setup for unit tests. +func newTestViper() *viper.Viper { + v := viper.New() + v.SetEnvPrefix(envPrefix) + v.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) + v.AutomaticEnv() + return v +} + +func TestBindFlags_SnakeCaseEnvVars(t *testing.T) { + // Ensure legacy names do not leak from the developer environment. + for _, name := range []string{ + "MPF_SUBSCRIPTIONID", + "MPF_TENANTID", + "MPF_SPCLIENTID", + "MPF_SPOBJECTID", + "MPF_SPCLIENTSECRET", + "MPF_SHOWDETAILEDOUTPUT", + "MPF_TEMPLATEFILEPATH", + } { + unsetEnvForTest(t, name) + } + + t.Setenv("MPF_SUBSCRIPTION_ID", "snake-sub-id") + t.Setenv("MPF_TENANT_ID", "snake-tenant-id") + t.Setenv("MPF_SP_CLIENT_ID", "snake-sp-client-id") + t.Setenv("MPF_SP_OBJECT_ID", "snake-sp-object-id") + t.Setenv("MPF_SP_CLIENT_SECRET", "snake-sp-secret") + t.Setenv("MPF_SHOW_DETAILED_OUTPUT", "true") + t.Setenv("MPF_TEMPLATE_FILE_PATH", "/tmp/template.json") + t.Setenv("MPF_VERBOSE", "true") + + var ( + subscriptionID string + tenantID string + spClientID string + spObjectID string + spClientSecret string + showDetailedOutput bool + templateFilePath string + verbose bool + ) + + cmd := &cobra.Command{Use: "test"} + cmd.Flags().StringVar(&subscriptionID, "subscriptionID", "", "") + cmd.Flags().StringVar(&tenantID, "tenantID", "", "") + cmd.Flags().StringVar(&spClientID, "spClientID", "", "") + cmd.Flags().StringVar(&spObjectID, "spObjectID", "", "") + cmd.Flags().StringVar(&spClientSecret, "spClientSecret", "", "") + cmd.Flags().BoolVar(&showDetailedOutput, "showDetailedOutput", false, "") + cmd.Flags().StringVar(&templateFilePath, "templateFilePath", "", "") + cmd.Flags().BoolVar(&verbose, "verbose", false, "") + + bindFlags(cmd, newTestViper()) + + assertEq := func(name, got, want string) { + t.Helper() + if got != want { + t.Errorf("%s = %q, want %q", name, got, want) + } + } + assertEq("subscriptionID", subscriptionID, "snake-sub-id") + assertEq("tenantID", tenantID, "snake-tenant-id") + assertEq("spClientID", spClientID, "snake-sp-client-id") + assertEq("spObjectID", spObjectID, "snake-sp-object-id") + assertEq("spClientSecret", spClientSecret, "snake-sp-secret") + assertEq("templateFilePath", templateFilePath, "/tmp/template.json") + if !showDetailedOutput { + t.Errorf("showDetailedOutput = false, want true") + } + if !verbose { + t.Errorf("verbose = false, want true") + } +} + +func TestBindFlags_LegacyEnvVarsStillWork(t *testing.T) { + // Clear snake_case variants so only legacy names are used. + for _, name := range []string{ + "MPF_SUBSCRIPTION_ID", + "MPF_TENANT_ID", + "MPF_SP_CLIENT_ID", + "MPF_TEMPLATE_FILE_PATH", + } { + unsetEnvForTest(t, name) + } + + t.Setenv("MPF_SUBSCRIPTIONID", "legacy-sub-id") + t.Setenv("MPF_TENANTID", "legacy-tenant-id") + t.Setenv("MPF_SPCLIENTID", "legacy-sp-client-id") + t.Setenv("MPF_TEMPLATEFILEPATH", "/legacy/template.json") + + var ( + subscriptionID string + tenantID string + spClientID string + templateFilePath string + ) + + cmd := &cobra.Command{Use: "test"} + cmd.Flags().StringVar(&subscriptionID, "subscriptionID", "", "") + cmd.Flags().StringVar(&tenantID, "tenantID", "", "") + cmd.Flags().StringVar(&spClientID, "spClientID", "", "") + cmd.Flags().StringVar(&templateFilePath, "templateFilePath", "", "") + + bindFlags(cmd, newTestViper()) + + if subscriptionID != "legacy-sub-id" { + t.Errorf("subscriptionID = %q, want %q", subscriptionID, "legacy-sub-id") + } + if tenantID != "legacy-tenant-id" { + t.Errorf("tenantID = %q, want %q", tenantID, "legacy-tenant-id") + } + if spClientID != "legacy-sp-client-id" { + t.Errorf("spClientID = %q, want %q", spClientID, "legacy-sp-client-id") + } + if templateFilePath != "/legacy/template.json" { + t.Errorf("templateFilePath = %q, want %q", templateFilePath, "/legacy/template.json") + } +} + +func TestBindFlags_LegacyTakesPrecedenceWhenBothSet(t *testing.T) { + // Legacy is checked first so existing deployments are not surprised if both forms are present. + t.Setenv("MPF_SUBSCRIPTIONID", "from-legacy") + t.Setenv("MPF_SUBSCRIPTION_ID", "from-snake") + + var subscriptionID string + cmd := &cobra.Command{Use: "test"} + cmd.Flags().StringVar(&subscriptionID, "subscriptionID", "", "") + + bindFlags(cmd, newTestViper()) + + if subscriptionID != "from-legacy" { + t.Errorf("subscriptionID = %q, want legacy value %q when both env forms are set", subscriptionID, "from-legacy") + } +} + +func TestBindFlags_CLIFlagTakesPrecedenceOverEnv(t *testing.T) { + t.Setenv("MPF_SUBSCRIPTION_ID", "from-env") + t.Setenv("MPF_SUBSCRIPTIONID", "from-legacy-env") + + var subscriptionID string + cmd := &cobra.Command{Use: "test"} + cmd.Flags().StringVar(&subscriptionID, "subscriptionID", "", "") + + // Simulate the flag being set on the CLI before bindFlags runs. + if err := cmd.Flags().Set("subscriptionID", "from-cli"); err != nil { + t.Fatalf("failed to set CLI flag: %v", err) + } + + bindFlags(cmd, newTestViper()) + + if subscriptionID != "from-cli" { + t.Errorf("subscriptionID = %q, want CLI value %q", subscriptionID, "from-cli") + } +} + +func TestBindFlags_TerraformProviderFlags(t *testing.T) { + for _, name := range []string{ + "MPF_TFPATH", + "MPF_WORKINGDIR", + "MPF_VARFILEPATH", + "MPF_IMPORTEXISTINGRESOURCESTOSTATE", + "MPF_TARGETMODULE", + } { + unsetEnvForTest(t, name) + } + + t.Setenv("MPF_TF_PATH", "/usr/bin/terraform") + t.Setenv("MPF_WORKING_DIR", "/work/tf") + t.Setenv("MPF_VAR_FILE_PATH", "/work/tf/dev.tfvars") + t.Setenv("MPF_IMPORT_EXISTING_RESOURCES_TO_STATE", "false") + t.Setenv("MPF_TARGET_MODULE", "module.law") + + var ( + tfPath string + workingDir string + varFilePath string + importExistingResourcesToState bool + targetModule string + ) + + cmd := &cobra.Command{Use: "terraform"} + cmd.Flags().StringVar(&tfPath, "tfPath", "", "") + cmd.Flags().StringVar(&workingDir, "workingDir", "", "") + cmd.Flags().StringVar(&varFilePath, "varFilePath", "", "") + // Default true mirrors the real terraform command flag. + cmd.Flags().BoolVar(&importExistingResourcesToState, "importExistingResourcesToState", true, "") + cmd.Flags().StringVar(&targetModule, "targetModule", "", "") + + bindFlags(cmd, newTestViper()) + + if tfPath != "/usr/bin/terraform" { + t.Errorf("tfPath = %q, want %q", tfPath, "/usr/bin/terraform") + } + if workingDir != "/work/tf" { + t.Errorf("workingDir = %q, want %q", workingDir, "/work/tf") + } + if varFilePath != "/work/tf/dev.tfvars" { + t.Errorf("varFilePath = %q, want %q", varFilePath, "/work/tf/dev.tfvars") + } + if importExistingResourcesToState { + t.Errorf("importExistingResourcesToState = true, want false") + } + if targetModule != "module.law" { + t.Errorf("targetModule = %q, want %q", targetModule, "module.law") + } +} + +func TestInitializeConfig_BindsRootAndSubcommandSnakeCaseEnv(t *testing.T) { + // Clear legacy forms that may be present in the environment. + for _, name := range []string{ + "MPF_SUBSCRIPTIONID", "MPF_TENANTID", "MPF_SPCLIENTID", + "MPF_SPOBJECTID", "MPF_SPCLIENTSECRET", "MPF_TEMPLATEFILEPATH", + "MPF_PARAMETERSFILEPATH", + } { + unsetEnvForTest(t, name) + } + + t.Setenv("MPF_SUBSCRIPTION_ID", "init-sub") + t.Setenv("MPF_TENANT_ID", "init-tenant") + t.Setenv("MPF_SP_CLIENT_ID", "init-client") + t.Setenv("MPF_SP_OBJECT_ID", "init-object") + t.Setenv("MPF_SP_CLIENT_SECRET", "init-secret") + t.Setenv("MPF_TEMPLATE_FILE_PATH", "/arm/template.json") + t.Setenv("MPF_PARAMETERS_FILE_PATH", "/arm/params.json") + + // Reset package-level flag vars that NewRootCommand binds to. + flgSubscriptionID = "" + flgTenantID = "" + flgSPClientID = "" + flgSPObjectID = "" + flgSPClientSecret = "" + flgTemplateFilePath = "" + flgParametersFilePath = "" + + root := NewRootCommand() + // Locate the arm subcommand (includes inherited persistent flags after merge). + armCmd, _, err := root.Find([]string{"arm"}) + if err != nil { + t.Fatalf("Find arm: %v", err) + } + + // Merge inherited persistent flags the same way cobra does before PersistentPreRun. + armCmd.Flags().AddFlagSet(root.PersistentFlags()) + + if err := initializeConfig(armCmd); err != nil { + t.Fatalf("initializeConfig: %v", err) + } + + if flgSubscriptionID != "init-sub" { + t.Errorf("flgSubscriptionID = %q, want %q", flgSubscriptionID, "init-sub") + } + if flgTenantID != "init-tenant" { + t.Errorf("flgTenantID = %q, want %q", flgTenantID, "init-tenant") + } + if flgSPClientID != "init-client" { + t.Errorf("flgSPClientID = %q, want %q", flgSPClientID, "init-client") + } + if flgSPObjectID != "init-object" { + t.Errorf("flgSPObjectID = %q, want %q", flgSPObjectID, "init-object") + } + if flgSPClientSecret != "init-secret" { + t.Errorf("flgSPClientSecret = %q, want %q", flgSPClientSecret, "init-secret") + } + if flgTemplateFilePath != "/arm/template.json" { + t.Errorf("flgTemplateFilePath = %q, want %q", flgTemplateFilePath, "/arm/template.json") + } + if flgParametersFilePath != "/arm/params.json" { + t.Errorf("flgParametersFilePath = %q, want %q", flgParametersFilePath, "/arm/params.json") + } +} diff --git a/cmd/rootCmd.go b/cmd/rootCmd.go index bfcf871..dcc5c40 100644 --- a/cmd/rootCmd.go +++ b/cmd/rootCmd.go @@ -33,7 +33,6 @@ import ( "github.com/Azure/mpf/pkg/infrastructure/mpfSharedUtils" "github.com/google/uuid" "github.com/spf13/cobra" - "github.com/spf13/pflag" "github.com/spf13/viper" log "github.com/sirupsen/logrus" @@ -142,33 +141,13 @@ func initializeConfig(cmd *cobra.Command) error { v.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) v.AutomaticEnv() + // Bind flags to both legacy concatenated env names (e.g. MPF_SUBSCRIPTIONID) + // and snake_case names (e.g. MPF_SUBSCRIPTION_ID). See envbinding.go. bindFlags(cmd, v) return nil } -// Bind each cobra flag to its associated viper configuration (config file and environment variable) -func bindFlags(cmd *cobra.Command, v *viper.Viper) { - cmd.Flags().VisitAll(func(f *pflag.Flag) { - // Determine the naming convention of the flags when represented in the config file - configName := f.Name - // If using camelCase in the config file, replace hyphens with a camelCased string. - // Since viper does case-insensitive comparisons, we don't need to bother fixing the case, and only need to remove the hyphens. - if replaceHyphenWithCamelCase { - configName = strings.ReplaceAll(f.Name, "-", "") - } - - // Apply the viper config value to the flag when the flag is not set and viper has a value - if !f.Changed && v.IsSet(configName) { - val := v.Get(configName) - err := cmd.Flags().Set(f.Name, fmt.Sprintf("%v", val)) - if err != nil { - log.Errorf("Error setting flag %s: %v\n", f.Name, err) - } - } - }) -} - func setLogLevel() { if flgVerbose { log.SetLevel(log.InfoLevel) diff --git a/docs/commandline-flags-and-env-variables.md b/docs/commandline-flags-and-env-variables.md index eb8a4e7..b1c8413 100644 --- a/docs/commandline-flags-and-env-variables.md +++ b/docs/commandline-flags-and-env-variables.md @@ -1,54 +1,70 @@ # MPF command line flags and environment variables -**Note**: Environment variables can be set using bash/shell syntax (e.g., `export MPF_SUBSCRIPTIONID=value`) on Linux/macOS, or using PowerShell syntax (e.g., `$env:MPF_SUBSCRIPTIONID = "value"`) on Windows. +**Note**: Environment variables can be set using bash/shell syntax (e.g., `export MPF_SUBSCRIPTION_ID=value`) on Linux/macOS, or using PowerShell syntax (e.g., `$env:MPF_SUBSCRIPTION_ID = "value"`) on Windows. + +## Environment variable naming + +Each flag accepts **two** environment variable name styles (prefix is always `MPF_`): + +| Style | Example for `subscriptionID` | Recommended | +|-------|------------------------------|-------------| +| **Snake_case** (preferred) | `MPF_SUBSCRIPTION_ID` | Yes — clearer and conventional for env vars | +| **Legacy concatenated** | `MPF_SUBSCRIPTIONID` | Still supported for backward compatibility | + +Rules: + +- Snake_case inserts underscores at camelCase word boundaries, including acronyms such as `ID` (`spClientID` → `MPF_SP_CLIENT_ID`). +- Single-word flags (`verbose`, `debug`, `location`) have the same name in both styles (`MPF_VERBOSE`, etc.). +- If both forms are set, the **legacy concatenated** value wins (so existing deployments are not changed accidentally). +- Explicit CLI flags always override environment variables. ## Global Flags (Common to all providers) -| Flag | Environment Variable | Required / Optional | Description | -|--------------------|------------------------|---------------------|-----------------------------------------------------------------------------------------------------------------------------------| -| subscriptionID | MPF_SUBSCRIPTIONID | Required | | -| tenantID | MPF_TENANTID | Required | | -| spClientID | MPF_SPCLIENTID | Required | | -| spObjectID | MPF_SPOBJECTID | Required | Note this is the SP Object id and is different from the Client ID | -| spClientSecret | MPF_SPCLIENTSECRET | Required | | -| showDetailedOutput | MPF_SHOWDETAILEDOUTPUT | Optional | If set to true, the output shows details of permissions resource wise as well. This is not needed if --jsonOutput is specified | -| jsonOutput | MPF_JSONOUTPUT | Optional | If set to true, the detailed output is printed in JSON format | -| verbose | MPF_VERBOSE | Optional | If set to true, verbose output with informational messages is displayed | -| debug | MPF_DEBUG | Optional | If set to true, output with detailed debug messages is displayed. The debug messages may contain sensitive tokens | -| initialPermissions | MPF_INITIALPERMISSIONS | Optional | Initial permissions to seed the custom role with before MPF analysis. See [Initial Permissions](#initial-permissions) for details | +| Flag | Environment Variable (preferred) | Legacy Environment Variable | Required / Optional | Description | +|--------------------|----------------------------------|-----------------------------|---------------------|-----------------------------------------------------------------------------------------------------------------------------------| +| subscriptionID | MPF_SUBSCRIPTION_ID | MPF_SUBSCRIPTIONID | Required | | +| tenantID | MPF_TENANT_ID | MPF_TENANTID | Required | | +| spClientID | MPF_SP_CLIENT_ID | MPF_SPCLIENTID | Required | | +| spObjectID | MPF_SP_OBJECT_ID | MPF_SPOBJECTID | Required | Note this is the SP Object id and is different from the Client ID | +| spClientSecret | MPF_SP_CLIENT_SECRET | MPF_SPCLIENTSECRET | Required | | +| showDetailedOutput | MPF_SHOW_DETAILED_OUTPUT | MPF_SHOWDETAILEDOUTPUT | Optional | If set to true, the output shows details of permissions resource wise as well. This is not needed if --jsonOutput is specified | +| jsonOutput | MPF_JSON_OUTPUT | MPF_JSONOUTPUT | Optional | If set to true, the detailed output is printed in JSON format | +| verbose | MPF_VERBOSE | MPF_VERBOSE | Optional | If set to true, verbose output with informational messages is displayed | +| debug | MPF_DEBUG | MPF_DEBUG | Optional | If set to true, output with detailed debug messages is displayed. The debug messages may contain sensitive tokens | +| initialPermissions | MPF_INITIAL_PERMISSIONS | MPF_INITIALPERMISSIONS | Optional | Initial permissions to seed the custom role with before MPF analysis. See [Initial Permissions](#initial-permissions) for details | When used for Terraform, the verbose and debug flags show detailed logs from Terraform. ## ARM Flags -| Flag | Environment Variable | Required / Optional | Description | -|----------------------|--------------------------|---------------------|---------------------------------------------------------------------------------------------------------------------------------------------------| -| templateFilePath | MPF_TEMPLATEFILEPATH | Required | ARM template file with path | -| parametersFilePath | MPF_PARAMETERSFILEPATH | Required | ARM template parameters file with path | -| resourceGroupNamePfx | MPF_RESOURCEGROUPNAMEPFX | Optional | Prefix for the resource group name. If not provided, default prefix is testdeployrg. For ARM deployments this temporary resource group is created | -| deploymentNamePfx | MPF_DEPLOYMENTNAMEPFX | Optional | Prefix for the deployment name. If not provided, default prefix is testDeploy. For ARM deployments this temporary deployment is created | -| location | MPF_LOCATION | Optional | Location for the resource group. If not provided, default location is eastus2 | +| Flag | Environment Variable (preferred) | Legacy Environment Variable | Required / Optional | Description | +|----------------------|----------------------------------|-----------------------------|---------------------|---------------------------------------------------------------------------------------------------------------------------------------------------| +| templateFilePath | MPF_TEMPLATE_FILE_PATH | MPF_TEMPLATEFILEPATH | Required | ARM template file with path | +| parametersFilePath | MPF_PARAMETERS_FILE_PATH | MPF_PARAMETERSFILEPATH | Required | ARM template parameters file with path | +| resourceGroupNamePfx | MPF_RESOURCE_GROUP_NAME_PFX | MPF_RESOURCEGROUPNAMEPFX | Optional | Prefix for the resource group name. If not provided, default prefix is testdeployrg. For ARM deployments this temporary resource group is created | +| deploymentNamePfx | MPF_DEPLOYMENT_NAME_PFX | MPF_DEPLOYMENTNAMEPFX | Optional | Prefix for the deployment name. If not provided, default prefix is testDeploy. For ARM deployments this temporary deployment is created | +| location | MPF_LOCATION | MPF_LOCATION | Optional | Location for the resource group. If not provided, default location is eastus2 | ### Bicep Flags -| Flag | Environment Variable | Required / Optional | Description | -|----------------------|--------------------------|---------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------| -| bicepFilePath | MPF_BICEPFILEPATH | Required | Bicep file with path | -| parametersFilePath | MPF_PARAMETERSFILEPATH | Required | Bicep parameters file with path (.json or .bicepparam). When a .bicepparam file is provided, it is automatically compiled to ARM JSON format | -| bicepExecPath | MPF_BICEPEXECPATH | Required | Path to the Bicep executable | -| resourceGroupNamePfx | MPF_RESOURCEGROUPNAMEPFX | Optional | Prefix for the resource group name. If not provided, default prefix is testdeployrg. For Bicep deployments this temporary resource group is created | -| deploymentNamePfx | MPF_DEPLOYMENTNAMEPFX | Optional | Prefix for the deployment name. If not provided, default prefix is testDeploy. For Bicep deployments this temporary deployment is created | -| location | MPF_LOCATION | Optional | Location for the resource group. If not provided, default location is eastus2 | +| Flag | Environment Variable (preferred) | Legacy Environment Variable | Required / Optional | Description | +|----------------------|----------------------------------|-----------------------------|---------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------| +| bicepFilePath | MPF_BICEP_FILE_PATH | MPF_BICEPFILEPATH | Required | Bicep file with path | +| parametersFilePath | MPF_PARAMETERS_FILE_PATH | MPF_PARAMETERSFILEPATH | Required | Bicep parameters file with path (.json or .bicepparam). When a .bicepparam file is provided, it is automatically compiled to ARM JSON format | +| bicepExecPath | MPF_BICEP_EXEC_PATH | MPF_BICEPEXECPATH | Required | Path to the Bicep executable | +| resourceGroupNamePfx | MPF_RESOURCE_GROUP_NAME_PFX | MPF_RESOURCEGROUPNAMEPFX | Optional | Prefix for the resource group name. If not provided, default prefix is testdeployrg. For Bicep deployments this temporary resource group is created | +| deploymentNamePfx | MPF_DEPLOYMENT_NAME_PFX | MPF_DEPLOYMENTNAMEPFX | Optional | Prefix for the deployment name. If not provided, default prefix is testDeploy. For Bicep deployments this temporary deployment is created | +| location | MPF_LOCATION | MPF_LOCATION | Optional | Location for the resource group. If not provided, default location is eastus2 | ## Terraform Flags -| Flag | Environment Variable | Required / Optional | Description | -|--------------------------------|------------------------------------|---------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| tfPath | MPF_TFPATH | Required | Path to the Terraform executable | -| workingDir | MPF_WORKINGDIR | Required | Path to the Terraform module directory | -| varFilePath | MPF_VARFILEPATH | Optional | Path to the Terraform variables file | -| importExistingResourcesToState | MPF_IMPORTEXISTINGRESOURCESTOSTATE | Optional | Default Value is true. This is required for some scenarios as described in the [Known Issues - Import Errors](./known-issues-and-workarounds.MD#existing-resource--import-errors) | -| targetModule | MPF_TARGETMODULE | Optional | Target module to be used for the Terraform deployment | +| Flag | Environment Variable (preferred) | Legacy Environment Variable | Required / Optional | Description | +|--------------------------------|-------------------------------------------|------------------------------------|---------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| tfPath | MPF_TF_PATH | MPF_TFPATH | Required | Path to the Terraform executable | +| workingDir | MPF_WORKING_DIR | MPF_WORKINGDIR | Required | Path to the Terraform module directory | +| varFilePath | MPF_VAR_FILE_PATH | MPF_VARFILEPATH | Optional | Path to the Terraform variables file | +| importExistingResourcesToState | MPF_IMPORT_EXISTING_RESOURCES_TO_STATE | MPF_IMPORTEXISTINGRESOURCESTOSTATE | Optional | Default Value is true. This is required for some scenarios as described in the [Known Issues - Import Errors](./known-issues-and-workarounds.MD#existing-resource--import-errors) | +| targetModule | MPF_TARGET_MODULE | MPF_TARGETMODULE | Optional | Target module to be used for the Terraform deployment | ### Example: Terraform Module Targeting @@ -57,12 +73,12 @@ When a Terraform configuration contains multiple modules, you can use `--targetM The following example uses the `module-test-with-targetting` sample which defines two modules (`law` and `law2`). To find minimum permissions for only the `law` module: ```bash -export MPF_SUBSCRIPTIONID="YOUR_SUBSCRIPTION_ID" -export MPF_TENANTID="YOUR_TENANT_ID" -export MPF_SPCLIENTID="YOUR_SP_CLIENT_ID" -export MPF_SPCLIENTSECRET="YOUR_SP_CLIENT_SECRET" -export MPF_SPOBJECTID="YOUR_SP_OBJECT_ID" -export MPF_TFPATH=$(which terraform) +export MPF_SUBSCRIPTION_ID="YOUR_SUBSCRIPTION_ID" +export MPF_TENANT_ID="YOUR_TENANT_ID" +export MPF_SP_CLIENT_ID="YOUR_SP_CLIENT_ID" +export MPF_SP_CLIENT_SECRET="YOUR_SP_CLIENT_SECRET" +export MPF_SP_OBJECT_ID="YOUR_SP_OBJECT_ID" +export MPF_TF_PATH=$(which terraform) cd samples/terraform/module-test-with-targetting terraform init @@ -73,12 +89,12 @@ azmpf terraform --workingDir $(pwd) --targetModule module.law --verbose On Windows (PowerShell): ```powershell -$env:MPF_SUBSCRIPTIONID = "YOUR_SUBSCRIPTION_ID" -$env:MPF_TENANTID = "YOUR_TENANT_ID" -$env:MPF_SPCLIENTID = "YOUR_SP_CLIENT_ID" -$env:MPF_SPCLIENTSECRET = "YOUR_SP_CLIENT_SECRET" -$env:MPF_SPOBJECTID = "YOUR_SP_OBJECT_ID" -$env:MPF_TFPATH = (Get-Command terraform).Source +$env:MPF_SUBSCRIPTION_ID = "YOUR_SUBSCRIPTION_ID" +$env:MPF_TENANT_ID = "YOUR_TENANT_ID" +$env:MPF_SP_CLIENT_ID = "YOUR_SP_CLIENT_ID" +$env:MPF_SP_CLIENT_SECRET = "YOUR_SP_CLIENT_SECRET" +$env:MPF_SP_OBJECT_ID = "YOUR_SP_OBJECT_ID" +$env:MPF_TF_PATH = (Get-Command terraform).Source cd samples\terraform\module-test-with-targetting terraform init diff --git a/e2eTests/e2eArm_test.go b/e2eTests/e2eArm_test.go index fefb94b..35cf02f 100644 --- a/e2eTests/e2eArm_test.go +++ b/e2eTests/e2eArm_test.go @@ -25,7 +25,6 @@ package e2etests import ( "errors" "fmt" - "os" "testing" "github.com/Azure/mpf/pkg/domain" @@ -86,11 +85,12 @@ func getMPFConfig(mpfArgs MpfCLIArgs) domain.MPFConfig { } func getTestingMPFArgs() (MpfCLIArgs, error) { - subscriptionID := os.Getenv("MPF_SUBSCRIPTIONID") - servicePrincipalClientID := os.Getenv("MPF_SPCLIENTID") - servicePrincipalObjectID := os.Getenv("MPF_SPOBJECTID") - servicePrincipalClientSecret := os.Getenv("MPF_SPCLIENTSECRET") - tenantID := os.Getenv("MPF_TENANTID") + // Accept legacy concatenated names and snake_case (legacy first for compatibility). + subscriptionID := mpfEnv("MPF_SUBSCRIPTIONID", "MPF_SUBSCRIPTION_ID") + servicePrincipalClientID := mpfEnv("MPF_SPCLIENTID", "MPF_SP_CLIENT_ID") + servicePrincipalObjectID := mpfEnv("MPF_SPOBJECTID", "MPF_SP_OBJECT_ID") + servicePrincipalClientSecret := mpfEnv("MPF_SPCLIENTSECRET", "MPF_SP_CLIENT_SECRET") + tenantID := mpfEnv("MPF_TENANTID", "MPF_TENANT_ID") resourceGroupNamePfx := "e2eTest" deploymentNamePfx := "e2eTest" location := "eastus2" diff --git a/e2eTests/e2eBicepInvalid_test.go b/e2eTests/e2eBicepInvalid_test.go index 7052955..ec7cec3 100644 --- a/e2eTests/e2eBicepInvalid_test.go +++ b/e2eTests/e2eBicepInvalid_test.go @@ -25,7 +25,6 @@ package e2etests import ( "errors" "fmt" - "os" "os/exec" "path/filepath" "testing" @@ -53,7 +52,7 @@ import ( // t.Skip("required environment variables not set, skipping end to end test") // } -// bicepExecPath := os.Getenv("MPF_BICEPEXECPATH") +// bicepExecPath := mpfEnv("MPF_BICEPEXECPATH", "MPF_BICEP_EXEC_PATH") // bicepFilePath := "../samples/bicep/aks-private-subnet.bicep" // parametersFilePath := "../samples/bicep/aks-invalid-params.json" @@ -114,7 +113,7 @@ func TestBicepInvalidResourceFile(t *testing.T) { t.Skip("required environment variables not set, skipping end to end test") } - bicepExecPath := os.Getenv("MPF_BICEPEXECPATH") + bicepExecPath := mpfEnv("MPF_BICEPEXECPATH", "MPF_BICEP_EXEC_PATH") bicepFilePath := "../samples/bicep/invalid-bicep.bicep" bicepFilePath, err = getAbsolutePath(bicepFilePath) @@ -146,7 +145,7 @@ func TestBicepInvalidParamsFullDeployment(t *testing.T) { t.Skip("required environment variables not set, skipping end to end test") } - bicepExecPath := os.Getenv("MPF_BICEPEXECPATH") + bicepExecPath := mpfEnv("MPF_BICEPEXECPATH", "MPF_BICEP_EXEC_PATH") bicepFilePath := "../samples/bicep/aks-private-subnet.bicep" parametersFilePath := "../samples/bicep/aks-invalid-params.json" diff --git a/e2eTests/e2eBicep_test.go b/e2eTests/e2eBicep_test.go index d4c7049..5654c25 100644 --- a/e2eTests/e2eBicep_test.go +++ b/e2eTests/e2eBicep_test.go @@ -42,7 +42,7 @@ import ( ) func checkBicepTestEnvVars() bool { - return os.Getenv("MPF_BICEPEXECPATH") == "" + return mpfEnv("MPF_BICEPEXECPATH", "MPF_BICEP_EXEC_PATH") == "" } // func TestBicepAks(t *testing.T) { @@ -56,7 +56,7 @@ func checkBicepTestEnvVars() bool { // t.Skip("required environment variables not set, skipping end to end test") // } -// bicepExecPath := os.Getenv("MPF_BICEPEXECPATH") +// bicepExecPath := mpfEnv("MPF_BICEPEXECPATH", "MPF_BICEP_EXEC_PATH") // bicepFilePath := "../samples/bicep/aks-private-subnet.bicep" // parametersFilePath := "../samples/bicep/aks-private-subnet-params.json" @@ -127,7 +127,7 @@ func TestBicepAksFullDeployment(t *testing.T) { t.Skip("required environment variables not set, skipping end to end test") } - bicepExecPath := os.Getenv("MPF_BICEPEXECPATH") + bicepExecPath := mpfEnv("MPF_BICEPEXECPATH", "MPF_BICEP_EXEC_PATH") bicepFilePath := "../samples/bicep/aks-private-subnet.bicep" parametersFilePath := "../samples/bicep/aks-private-subnet-params.json" @@ -198,7 +198,7 @@ func TestBicepWithBicepparamFile(t *testing.T) { t.Skip("required environment variables not set, skipping end to end test") } - bicepExecPath := os.Getenv("MPF_BICEPEXECPATH") + bicepExecPath := mpfEnv("MPF_BICEPEXECPATH", "MPF_BICEP_EXEC_PATH") bicepFilePath, err := getAbsolutePath("../samples/bicep/storage-account-simple.bicep") if err != nil { diff --git a/e2eTests/e2eTerraformAuthPermissionMismatch_test.go b/e2eTests/e2eTerraformAuthPermissionMismatch_test.go index 26415a2..8fdf545 100644 --- a/e2eTests/e2eTerraformAuthPermissionMismatch_test.go +++ b/e2eTests/e2eTerraformAuthPermissionMismatch_test.go @@ -23,7 +23,6 @@ package e2etests import ( - "os" "path" "runtime" "testing" @@ -48,10 +47,10 @@ func TestTerraformAuthorizationPermissionMismatch(t *testing.T) { mpfArgs.MPFMode = "terraform" var tfpath string - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath = mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath = os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) diff --git a/e2eTests/e2eTerraformAuthorizationRequestDenied_test.go b/e2eTests/e2eTerraformAuthorizationRequestDenied_test.go index 8590d68..f74f177 100644 --- a/e2eTests/e2eTerraformAuthorizationRequestDenied_test.go +++ b/e2eTests/e2eTerraformAuthorizationRequestDenied_test.go @@ -23,7 +23,6 @@ package e2etests import ( - "os" "path" "runtime" "strings" @@ -49,10 +48,10 @@ func TestTerraformAuthorizationRequestDenied(t *testing.T) { } mpfArgs.MPFMode = "terraform" - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath := mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath := os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) diff --git a/e2eTests/e2eTerraformInvalid_test.go b/e2eTests/e2eTerraformInvalid_test.go index 0be62b9..3b77376 100644 --- a/e2eTests/e2eTerraformInvalid_test.go +++ b/e2eTests/e2eTerraformInvalid_test.go @@ -23,7 +23,6 @@ package e2etests import ( - "os" "path" "runtime" "testing" @@ -44,10 +43,10 @@ func TestTerraformACIInvalidVarFile(t *testing.T) { mpfArgs.MPFMode = "terraform" var tfpath string - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath = mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath = os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) @@ -92,10 +91,10 @@ func TestTerraformACIInvalidTfFile(t *testing.T) { mpfArgs.MPFMode = "terraform" var tfpath string - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath = mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath = os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) diff --git a/e2eTests/e2eTerraformWithImportAndTargeting_test.go b/e2eTests/e2eTerraformWithImportAndTargeting_test.go index 9e484fc..9ccde4b 100644 --- a/e2eTests/e2eTerraformWithImportAndTargeting_test.go +++ b/e2eTests/e2eTerraformWithImportAndTargeting_test.go @@ -23,7 +23,6 @@ package e2etests import ( - "os" "path" "runtime" "testing" @@ -46,10 +45,10 @@ func TestTerraformWithImport(t *testing.T) { mpfArgs.MPFMode = "terraform" var tfpath string - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath = mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath = os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) @@ -97,10 +96,10 @@ func TestTerraformWithTargetting(t *testing.T) { mpfArgs.MPFMode = "terraform" var tfpath string - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath = mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath = os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) diff --git a/e2eTests/e2eTerraform_test.go b/e2eTests/e2eTerraform_test.go index 89421a4..a2d7623 100644 --- a/e2eTests/e2eTerraform_test.go +++ b/e2eTests/e2eTerraform_test.go @@ -73,10 +73,10 @@ func TestTerraformACI(t *testing.T) { mpfArgs.MPFMode = "terraform" var tfpath string - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath = mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath = os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) @@ -126,10 +126,10 @@ func TestTerraformACINoTfvarsFile(t *testing.T) { mpfArgs.MPFMode = "terraform" var tfpath string - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath = mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath = os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) @@ -176,10 +176,10 @@ func TestTerraformModuleTest(t *testing.T) { mpfArgs.MPFMode = "terraform" var tfpath string - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath = mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath = os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) @@ -308,10 +308,10 @@ func TestTerraformACIWithInitialPermissions(t *testing.T) { mpfArgs.MPFMode = "terraform" var tfpath string - if os.Getenv("MPF_TFPATH") == "" { - t.Skip("Terraform Path MPF_TFPATH not set, skipping end to end test") + tfpath = mpfEnv("MPF_TFPATH", "MPF_TF_PATH") + if tfpath == "" { + t.Skip("Terraform Path MPF_TFPATH/MPF_TF_PATH not set, skipping end to end test") } - tfpath = os.Getenv("MPF_TFPATH") _, filename, _, _ := runtime.Caller(0) curDir := path.Dir(filename) diff --git a/e2eTests/env_helpers.go b/e2eTests/env_helpers.go new file mode 100644 index 0000000..5929e6a --- /dev/null +++ b/e2eTests/env_helpers.go @@ -0,0 +1,37 @@ +// 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 e2etests + +import "os" + +// mpfEnv returns the first non-empty value among the given environment variable +// names. Callers should list the legacy concatenated name first, then the +// snake_case form, matching azmpf CLI precedence. +func mpfEnv(keys ...string) string { + for _, key := range keys { + if val := os.Getenv(key); val != "" { + return val + } + } + return "" +}