fix: various Viper-related API URL bugs

commands/cfg/login.go

@@ -107,7 +107,7 @@ func RunLoginUser(c *core.CommandConfig) error {
 	}
 
 	var msg strings.Builder
-	if validCredentials {
+	if validCredentials && !viper.GetBool(core.GetFlagName(c.NS, constants.FlagSkipVerify)) {
 		ls, _, errTokens := client.NewClientFromCfgData(data).AuthClient.TokensApi.TokensGet(context.Background()).Execute()
 		if errTokens != nil {
 			return fmt.Errorf("failed retrieving current tokens: %w", errTokens)
@@ -150,8 +150,8 @@ func buildConfigData(c *core.CommandConfig) (map[string]string, error) {
 	configData := map[string]string{} // This map is what we will write to the config file
 
 	// API URL
-	fmt.Fprintf(c.Command.Command.ErrOrStderr(), jsontabwriter.GenerateVerboseOutput("API Url: %s", config.GetServerUrl()))
-	if explicitUrl := config.GetServerUrl(); explicitUrl != "" {
+	fmt.Fprintf(c.Command.Command.ErrOrStderr(), jsontabwriter.GenerateVerboseOutput("API Url: %s", viper.GetString(constants.ArgServerUrl)))
+	if explicitUrl := viper.GetString(constants.ArgServerUrl); explicitUrl != "" && explicitUrl != constants.DefaultApiURL {
 		// Don't save the API url to the config if it's the default, since we don't want to revert to it if the user doesn't provide any value.
 		// This was changed from old behaviour because some APIs (e.g. DNS API) [can] use a different server URL
 		fmt.Fprintf(c.Command.Command.ErrOrStderr(), jsontabwriter.GenerateVerboseOutput("Saving API URL to config file: %s", explicitUrl))

commands/root.go

@@ -93,11 +93,6 @@ func init() {
 		"Configuration file used for authentication",
 	)
 	_ = viper.BindPFlag(constants.ArgConfig, rootPFlagSet.Lookup(constants.ArgConfig))
-	rootPFlagSet.StringP(
-		constants.ArgServerUrl, constants.ArgServerUrlShort, constants.DefaultApiURL,
-		"Override default host url",
-	)
-	_ = viper.BindPFlag(constants.ArgServerUrl, rootPFlagSet.Lookup(constants.ArgServerUrl))
 	rootPFlagSet.StringVarP(
 		&Output, constants.ArgOutput, constants.ArgOutputShort, constants.DefaultOutputFormat,
 		"Desired output format [text|json|api-json]",
@@ -111,6 +106,10 @@ func init() {
 			}, cobra.ShellCompDirectiveNoFileComp
 		},
 	)
+	rootCmd.GlobalFlags().StringP(
+		constants.ArgServerUrl, constants.ArgServerUrlShort, constants.DefaultApiURL,
+		"Override default host url",
+	)
 	rootPFlagSet.BoolVarP(&Quiet, constants.ArgQuiet, constants.ArgQuietShort, false, "Quiet output")
 	_ = viper.BindPFlag(constants.ArgQuiet, rootPFlagSet.Lookup(constants.ArgQuiet))
 	rootPFlagSet.BoolVarP(
@@ -129,6 +128,9 @@ func init() {
 	// Add SubCommands to RootCmd
 	addCommands()
 
+	// because of Viper Shenanigans, we have to bind it last, after any commands, to avoid overwriting the default...
+	_ = viper.BindPFlag(constants.ArgServerUrl, rootCmd.GlobalFlags().Lookup(constants.ArgServerUrl))
+
 	cobra.OnInitialize(initConfig)
 }
 

internal/config/config.go

@@ -22,13 +22,12 @@ var FieldsWithSensitiveDataInConfigFile = []string{
 // GetServerUrl returns the server URL the SDK should use, with support for layered fallbacks.
 func GetServerUrl() string {
 	viper.AutomaticEnv()
+
 	if val := viper.GetString(constants.ArgServerUrl); viper.IsSet(constants.ArgServerUrl) && val != "" {
-		fmt.Println("arg", val)
 		// 1. Above all, use global flag val
 		return val
 	}
 	if val := viper.GetString(constants.EnvServerUrl); viper.IsSet(constants.EnvServerUrl) && val != "" {
-		fmt.Println("env", val)
 		// 2. Fallback to non-empty env vars
 		return val
 	}
@@ -104,7 +103,6 @@ func checkFilePermissions(fileInfo os.FileInfo, path string) error {
 // Read reads the config file and returns its data as a map
 func Read() (map[string]string, error) {
 	path := GetConfigFilePath()
-	fmt.Println("path", path)
 
 	fileInfo, err := os.Stat(path)
 	if err != nil {
@@ -127,8 +125,6 @@ func Read() (map[string]string, error) {
 		return nil, fmt.Errorf("failed unmarshalling config file data: %w", err)
 	}
 
-	fmt.Println("result", result)
-
 	return result, nil
 }
 

internal/config/config_test.go

@@ -260,6 +260,8 @@ func TestGetServerUrl(t *testing.T) {
 		},
 	}
 
+	oldConfig, errConfig := config.Read()
+
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			// Mock viper values
@@ -270,7 +272,8 @@ func TestGetServerUrl(t *testing.T) {
 				viper.Set(constants.EnvServerUrl, tt.envVal)
 			}
 			if tt.cfgVal != "" {
-				viper.Set(constants.CfgServerUrl, tt.cfgVal)
+				err := config.Write(map[string]string{constants.CfgServerUrl: tt.cfgVal})
+				assert.NoError(t, err, "failed to write config")
 			}
 
 			got := config.GetServerUrl()
@@ -279,4 +282,14 @@ func TestGetServerUrl(t *testing.T) {
 			}
 		})
 	}
+
+	t.Cleanup(func() {
+		viper.Reset()
+
+		if errConfig != nil {
+			config.Write(oldConfig)
+		} else {
+			config.Write(map[string]string{})
+		}
+	})
 }

internal/core/command.go

@@ -3,6 +3,7 @@ package core
 import (
 	"fmt"
 	"net"
+	"os"
 	"strings"
 	"time"
 
@@ -94,7 +95,6 @@ func WithRegionalFlags(c *Command, baseURL string, allowedLocations []string) *C
 	c.Command.PersistentFlags().StringP(
 		constants.ArgServerUrl, constants.ArgServerUrlShort, defaultUrl, "Override default host URL",
 	)
-	viper.BindPFlag(constants.ArgServerUrl, c.Command.PersistentFlags().Lookup(constants.ArgServerUrl))
 
 	// Add the location flag
 	c.Command.PersistentFlags().StringP(
@@ -131,6 +131,17 @@ func WithRegionalFlags(c *Command, baseURL string, allowedLocations []string) *C
 			}
 		}
 
+		// Because Viper has issues with binding to the same flag multiple times, we need to manually set the value
+		if c.Command.PersistentFlags().Changed(constants.ArgServerUrl) {
+			customURL, _ := c.Command.PersistentFlags().GetString(constants.ArgServerUrl)
+			viper.Set(constants.ArgServerUrl, customURL)
+		}
+
+		// Because Viper has issues with binding to the same env var multiple times, we need to manually set the value
+		if envURL := os.Getenv(constants.EnvServerUrl); envURL != "" {
+			viper.Set(constants.EnvServerUrl, envURL)
+		}
+
 		return nil
 	}