From d0e5859a24f7ff941df63a4aaecaa2199e4452d7 Mon Sep 17 00:00:00 2001 From: Abhineshhh Date: Wed, 15 Jul 2026 01:09:19 +0530 Subject: [PATCH] feat: accept snake_case environment variable names (#248) Support preferred snake_case env vars (e.g. MPF_SUBSCRIPTION_ID) while keeping legacy concatenated names (e.g. MPF_SUBSCRIPTIONID) for backward compatibility. Bind both forms via Viper for all CLI flags, prefer legacy when both are set, and document the dual naming. E2E helpers accept either form so tests stay aligned with the CLI. Fixes #248 --- AGENTS.md | 27 +- README.md | 70 ++-- cmd/envbinding.go | 119 ++++++ cmd/envbinding_test.go | 389 ++++++++++++++++++ cmd/rootCmd.go | 25 +- docs/commandline-flags-and-env-variables.md | 110 ++--- e2eTests/e2eArm_test.go | 12 +- e2eTests/e2eBicepInvalid_test.go | 7 +- e2eTests/e2eBicep_test.go | 8 +- ...e2eTerraformAuthPermissionMismatch_test.go | 7 +- ...erraformAuthorizationRequestDenied_test.go | 7 +- e2eTests/e2eTerraformInvalid_test.go | 13 +- ...e2eTerraformWithImportAndTargeting_test.go | 13 +- e2eTests/e2eTerraform_test.go | 24 +- e2eTests/env_helpers.go | 37 ++ 15 files changed, 702 insertions(+), 166 deletions(-) create mode 100644 cmd/envbinding.go create mode 100644 cmd/envbinding_test.go create mode 100644 e2eTests/env_helpers.go 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 "" +}