From e55f379cb07dd2fef453e8fa61efd5488c18602b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 28 May 2024 12:05:42 +0100 Subject: [PATCH 1/8] fix issues in acceptance --- internal/cmd/root.go | 9 +++--- internal/pkg/auth/storage.go | 14 ++------ internal/pkg/config/config.go | 17 +++++----- internal/pkg/config/profiles.go | 57 ++++++++++++++++++++------------- 4 files changed, 50 insertions(+), 47 deletions(-) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 62a5b0b6e..8a5fd1227 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -61,12 +61,13 @@ func NewRootCmd(version, date string, p *print.Printer) *cobra.Command { return fmt.Errorf("get profile: %w", err) } - profileExists, err := config.ProfileExists(activeProfile) + profileSet, err := config.GetConfiguredProfile() if err != nil { - return fmt.Errorf("check if profile exists: %w", err) + return fmt.Errorf("get profile raw: %w", err) } - if !profileExists { - p.Warn("active profile does not exist, the default profile configuration will be used\n") + + if activeProfile != profileSet { + p.Warn("active profile %q does not exist, the %q profile configuration will be used\n", profileSet, activeProfile) } p.Debug(print.DebugLevel, "active configuration profile: %s", activeProfile) diff --git a/internal/pkg/auth/storage.go b/internal/pkg/auth/storage.go index 73bef1189..c23457d40 100644 --- a/internal/pkg/auth/storage.go +++ b/internal/pkg/auth/storage.go @@ -215,20 +215,10 @@ func getAuthFieldFromEncodedTextFile(activeProfile string, key authFieldKey) (st // If it doesn't, creates it with the content "{}" encoded. // If it does, does nothing (and returns nil). func createEncodedTextFile(activeProfile string) error { - configDir, err := os.UserConfigDir() - if err != nil { - return fmt.Errorf("get config dir: %w", err) - } - - profileTextFileFolderName := textFileFolderName - if activeProfile != "" { - profileTextFileFolderName = filepath.Join(textFileFolderName, activeProfile) - } - - textFileDir := filepath.Join(configDir, profileTextFileFolderName) + textFileDir := config.GetProfileFolderPath(activeProfile) textFilePath := filepath.Join(textFileDir, textFileName) - err = os.MkdirAll(textFileDir, os.ModePerm) + err := os.MkdirAll(textFileDir, os.ModePerm) if err != nil { return fmt.Errorf("create file dir: %w", err) } diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index b6ab86ea3..e8b219117 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -34,15 +34,15 @@ const ( ServiceAccountCustomEndpointKey = "service_account_custom_endpoint" SKECustomEndpointKey = "ske_custom_endpoint" - ProjectNameKey = "project_name" + ProjectNameKey = "project_name" + DefaultProfileName = "default" AsyncDefault = false SessionTimeLimitDefault = "2h" ) const ( - configFolder = "stackit" - defaultProfileName = "default" + configFolder = "stackit" configFileName = "cli-config" configFileExtension = "json" @@ -92,12 +92,9 @@ func InitConfig() { configProfile, err := GetProfile() cobra.CheckErr(err) - configFolderPath = defaultConfigFolderPath - if configProfile != defaultProfileName { - configFolderPath = filepath.Join(configFolderPath, profileRootFolder, configProfile) // If a profile is set, use the profile config folder - } + configFolderPath = GetProfileFolderPath(configProfile) - configFilePath := filepath.Join(configFolderPath, fmt.Sprintf("%s.%s", configFileName, configFileExtension)) + configFilePath := getConfigFilePath(configFolderPath) // This hack is required to allow creating the config file with `viper.WriteConfig` // see https://github.com/spf13/viper/issues/851#issuecomment-789393451 @@ -151,3 +148,7 @@ func setConfigDefaults() { viper.SetDefault(ServiceAccountCustomEndpointKey, "") viper.SetDefault(SKECustomEndpointKey, "") } + +func getConfigFilePath(configFolder string) string { + return filepath.Join(configFolder, fmt.Sprintf("%s.%s", configFileName, configFileExtension)) +} diff --git a/internal/pkg/config/profiles.go b/internal/pkg/config/profiles.go index 9723c3862..00d68785b 100644 --- a/internal/pkg/config/profiles.go +++ b/internal/pkg/config/profiles.go @@ -22,6 +22,24 @@ const ProfileEnvVar = "STACKIT_CLI_PROFILE" // // If the profile is not valid, it returns an error. func GetProfile() (string, error) { + profile, err := GetConfiguredProfile() + if err != nil { + return "", err + } + + // Make sure the profile exists + profileExists, err := ProfileExists(profile) + if err != nil { + return "", fmt.Errorf("check if profile exists: %w", err) + } + if !profileExists { + return DefaultProfileName, nil + } + + return profile, nil +} + +func GetConfiguredProfile() (string, error) { profile, profileSetInEnv := GetProfileFromEnv() if !profileSetInEnv { contents, exists, err := fileutils.ReadFileIfExists(profileFilePath) @@ -29,21 +47,12 @@ func GetProfile() (string, error) { return "", fmt.Errorf("read profile from file: %w", err) } if !exists { - return defaultProfileName, nil + return DefaultProfileName, nil } profile = contents } - // Make sure the profile exists - profileExists, err := ProfileExists(profile) - if err != nil { - return "", fmt.Errorf("check if profile exists: %w", err) - } - if !profileExists { - return defaultProfileName, nil - } - - err = ValidateProfile(profile) + err := ValidateProfile(profile) if err != nil { return "", fmt.Errorf("validate profile: %w", err) } @@ -68,13 +77,13 @@ func CreateProfile(p *print.Printer, profile string, setProfile, emptyProfile bo } // Cannot create a profile with the default name - if profile == defaultProfileName { + if profile == DefaultProfileName { return &errors.InvalidProfileNameError{ Profile: profile, } } - configFolderPath = filepath.Join(defaultConfigFolderPath, profileRootFolder, profile) + configFolderPath = GetProfileFolderPath(profile) // Error if the profile already exists _, err = os.Stat(configFolderPath) @@ -127,15 +136,10 @@ func CreateProfile(p *print.Printer, profile string, setProfile, emptyProfile bo // If the current profile does not exist, it returns an error. // If the new profile already exists, it will be overwritten. func DuplicateProfileConfiguration(p *print.Printer, currentProfile, newProfile string) error { - var currentConfigFilePath string - - if currentProfile == defaultProfileName { - currentConfigFilePath = filepath.Join(defaultConfigFolderPath, fmt.Sprintf("%s.%s", configFileName, configFileExtension)) - } else { - currentConfigFilePath = filepath.Join(defaultConfigFolderPath, profileRootFolder, currentProfile, fmt.Sprintf("%s.%s", configFileName, configFileExtension)) - } + currentProfileFolder := GetProfileFolderPath(currentProfile) + currentConfigFilePath := getConfigFilePath(currentProfileFolder) - newConfigFilePath := filepath.Join(configFolderPath, fmt.Sprintf("%s.%s", configFileName, configFileExtension)) + newConfigFilePath := getConfigFilePath(configFolderPath) err := fileutils.CopyFile(currentConfigFilePath, newConfigFilePath) if err != nil { @@ -169,7 +173,7 @@ func SetProfile(p *print.Printer, profile string) error { } p.Debug(print.DebugLevel, "persisted new active profile in: %s", profileFilePath) - configFolderPath = filepath.Join(defaultConfigFolderPath, profile) + configFolderPath = GetProfileFolderPath(profile) p.Debug(print.DebugLevel, "profile %q is now active", profile) return nil @@ -203,7 +207,7 @@ func ValidateProfile(profile string) error { } func ProfileExists(profile string) (bool, error) { - _, err := os.Stat(filepath.Join(defaultConfigFolderPath, profileRootFolder, profile)) + _, err := os.Stat(GetProfileFolderPath(profile)) if err != nil { if os.IsNotExist(err) { return false, nil @@ -212,3 +216,10 @@ func ProfileExists(profile string) (bool, error) { } return true, nil } + +func GetProfileFolderPath(profile string) string { + if profile == DefaultProfileName { + return defaultConfigFolderPath + } + return filepath.Join(defaultConfigFolderPath, profileRootFolder, profile) +} From 7ff1d1bb7858147e447def10e651093f36bb9db2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 28 May 2024 13:31:25 +0100 Subject: [PATCH 2/8] fix storage auth file --- internal/pkg/auth/storage.go | 25 ++------------ internal/pkg/auth/storage_test.go | 12 +------ internal/pkg/config/config.go | 12 ++++--- internal/pkg/config/config_test.go | 26 +++++++++++++++ internal/pkg/config/profiles.go | 4 +++ internal/pkg/config/profiles_test.go | 50 +++++++++++++++++++++++++++- 6 files changed, 90 insertions(+), 39 deletions(-) diff --git a/internal/pkg/auth/storage.go b/internal/pkg/auth/storage.go index c23457d40..fb511dbdb 100644 --- a/internal/pkg/auth/storage.go +++ b/internal/pkg/auth/storage.go @@ -88,18 +88,7 @@ func setAuthFieldInEncodedTextFile(activeProfile string, key authFieldKey, value if err != nil { return err } - - configDir, err := os.UserConfigDir() - if err != nil { - return fmt.Errorf("get config dir: %w", err) - } - - profileTextFileFolderName := textFileFolderName - if activeProfile != "" { - profileTextFileFolderName = filepath.Join(textFileFolderName, activeProfile) - } - - textFileDir := filepath.Join(configDir, profileTextFileFolderName) + textFileDir := config.GetProfileFolderPath(activeProfile) textFilePath := filepath.Join(textFileDir, textFileName) contentEncoded, err := os.ReadFile(textFilePath) @@ -178,17 +167,7 @@ func getAuthFieldFromEncodedTextFile(activeProfile string, key authFieldKey) (st return "", err } - configDir, err := os.UserConfigDir() - if err != nil { - return "", fmt.Errorf("get config dir: %w", err) - } - - profileTextFileFolderName := textFileFolderName - if activeProfile != "" { - profileTextFileFolderName = filepath.Join(textFileFolderName, activeProfile) - } - - textFileDir := filepath.Join(configDir, profileTextFileFolderName) + textFileDir := config.GetProfileFolderPath(activeProfile) textFilePath := filepath.Join(textFileDir, textFileName) contentEncoded, err := os.ReadFile(textFilePath) diff --git a/internal/pkg/auth/storage_test.go b/internal/pkg/auth/storage_test.go index 4d1593020..dde97d1e6 100644 --- a/internal/pkg/auth/storage_test.go +++ b/internal/pkg/auth/storage_test.go @@ -443,17 +443,7 @@ func deleteAuthFieldInEncodedTextFile(activeProfile string, key authFieldKey) er return err } - configDir, err := os.UserConfigDir() - if err != nil { - return fmt.Errorf("get config dir: %w", err) - } - - profileTextFileFolderName := textFileFolderName - if activeProfile != "" { - profileTextFileFolderName = filepath.Join(textFileFolderName, activeProfile) - } - - textFileDir := filepath.Join(configDir, profileTextFileFolderName) + textFileDir := config.GetProfileFolderPath(activeProfile) textFilePath := filepath.Join(textFileDir, textFileName) contentEncoded, err := os.ReadFile(textFilePath) diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index e8b219117..a1a7947fd 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -83,10 +83,7 @@ var configFolderPath string var profileFilePath string func InitConfig() { - configDir, err := os.UserConfigDir() - cobra.CheckErr(err) - - defaultConfigFolderPath = filepath.Join(configDir, configFolder) + defaultConfigFolderPath = getInitialConfigDir() profileFilePath = filepath.Join(defaultConfigFolderPath, fmt.Sprintf("%s.%s", profileFileName, profileFileExtension)) // Profile file path is in the default config folder configProfile, err := GetProfile() @@ -152,3 +149,10 @@ func setConfigDefaults() { func getConfigFilePath(configFolder string) string { return filepath.Join(configFolder, fmt.Sprintf("%s.%s", configFileName, configFileExtension)) } + +func getInitialConfigDir() string { + configDir, err := os.UserConfigDir() + cobra.CheckErr(err) + + return filepath.Join(configDir, configFolder) +} diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index b3483a635..159a8a086 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -69,3 +69,29 @@ func TestWrite(t *testing.T) { }) } } + +func TestGetInitialConfigDir(t *testing.T) { + tests := []struct { + description string + }{ + { + description: "base", + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + actual := getInitialConfigDir() + + userConfig, err := os.UserConfigDir() + if err != nil { + t.Fatalf("expected error to be nil, got %v", err) + } + + expected := filepath.Join(userConfig, "stackit") + if actual != expected { + t.Fatalf("expected %s, got %s", expected, actual) + } + }) + } +} diff --git a/internal/pkg/config/profiles.go b/internal/pkg/config/profiles.go index 00d68785b..ff11e2b47 100644 --- a/internal/pkg/config/profiles.go +++ b/internal/pkg/config/profiles.go @@ -218,6 +218,10 @@ func ProfileExists(profile string) (bool, error) { } func GetProfileFolderPath(profile string) string { + if defaultConfigFolderPath == "" { + defaultConfigFolderPath = getInitialConfigDir() + } + if profile == DefaultProfileName { return defaultConfigFolderPath } diff --git a/internal/pkg/config/profiles_test.go b/internal/pkg/config/profiles_test.go index e97451aeb..1250a16aa 100644 --- a/internal/pkg/config/profiles_test.go +++ b/internal/pkg/config/profiles_test.go @@ -1,6 +1,9 @@ package config -import "testing" +import ( + "path/filepath" + "testing" +) func TestValidateProfile(t *testing.T) { tests := []struct { @@ -57,3 +60,48 @@ func TestValidateProfile(t *testing.T) { }) } } + +func TestGetProfileFolderPath(t *testing.T) { + tests := []struct { + description string + defaultConfigFolderNotSet bool + profile string + expected string + }{ + { + description: "default profile", + profile: DefaultProfileName, + expected: getInitialConfigDir(), + }, + { + description: "default profile, default config folder not set", + defaultConfigFolderNotSet: true, + profile: DefaultProfileName, + expected: getInitialConfigDir(), + }, + { + description: "custom profile", + profile: "my-profile", + expected: filepath.Join(getInitialConfigDir(), profileRootFolder, "my-profile"), + }, + { + description: "custom profile, default config folder not set", + defaultConfigFolderNotSet: true, + profile: "my-profile", + expected: filepath.Join(getInitialConfigDir(), profileRootFolder, "my-profile"), + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + defaultConfigFolderPath = getInitialConfigDir() + if tt.defaultConfigFolderNotSet { + defaultConfigFolderPath = "" + } + actual := GetProfileFolderPath(tt.profile) + if actual != tt.expected { + t.Errorf("expected profile folder path to be %q but got %q", tt.expected, actual) + } + }) + } +} From faa14cfddb3c08b711fa8786ef1c9af27a8365ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 28 May 2024 13:53:39 +0100 Subject: [PATCH 3/8] add debug logs --- internal/cmd/root.go | 1 + internal/pkg/config/profiles.go | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 8a5fd1227..b1cac4861 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -67,6 +67,7 @@ func NewRootCmd(version, date string, p *print.Printer) *cobra.Command { } if activeProfile != profileSet { + p.Debug(print.DebugLevel, "the active profile %q does not exist, following folder is missing: %q", profileSet, config.GetProfileFolderPath(profileSet)) p.Warn("active profile %q does not exist, the %q profile configuration will be used\n", profileSet, activeProfile) } p.Debug(print.DebugLevel, "active configuration profile: %s", activeProfile) diff --git a/internal/pkg/config/profiles.go b/internal/pkg/config/profiles.go index ff11e2b47..c300a62ce 100644 --- a/internal/pkg/config/profiles.go +++ b/internal/pkg/config/profiles.go @@ -39,6 +39,11 @@ func GetProfile() (string, error) { return profile, nil } +// GetConfiguredProfile returns the profile that is configured by the user, which may not exist. +// The profile is determined by the value of the STACKIT_CLI_PROFILE environment variable, or, if not set, +// by the contents of the profile file in the CLI config folder. +// +// If the profile is not valid, it returns an error. func GetConfiguredProfile() (string, error) { profile, profileSetInEnv := GetProfileFromEnv() if !profileSetInEnv { From 744197cebac1721ab18a99565beeb88f26e2893c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 28 May 2024 14:02:06 +0100 Subject: [PATCH 4/8] support duplication of empty profile --- internal/pkg/config/profiles.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/pkg/config/profiles.go b/internal/pkg/config/profiles.go index c300a62ce..b04f43c81 100644 --- a/internal/pkg/config/profiles.go +++ b/internal/pkg/config/profiles.go @@ -146,7 +146,16 @@ func DuplicateProfileConfiguration(p *print.Printer, currentProfile, newProfile newConfigFilePath := getConfigFilePath(configFolderPath) - err := fileutils.CopyFile(currentConfigFilePath, newConfigFilePath) + // If the source profile configuration does not exist, do nothing + _, err := os.Stat(currentConfigFilePath) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("get current profile configuration: %w", err) + } + + err = fileutils.CopyFile(currentConfigFilePath, newConfigFilePath) if err != nil { return fmt.Errorf("copy config file: %w", err) } From f60431f58fe7ff42e13f91f0555f6636e353eb7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 28 May 2024 15:12:35 +0100 Subject: [PATCH 5/8] fix auth storage tests --- internal/pkg/auth/storage_test.go | 52 +++++++++++++++++++++++++++--- internal/pkg/config/config.go | 10 +++++- internal/pkg/config/config_test.go | 30 +++++++++++++++++ internal/pkg/config/profiles.go | 4 +++ 4 files changed, 91 insertions(+), 5 deletions(-) diff --git a/internal/pkg/auth/storage_test.go b/internal/pkg/auth/storage_test.go index dde97d1e6..f8e6b4dd6 100644 --- a/internal/pkg/auth/storage_test.go +++ b/internal/pkg/auth/storage_test.go @@ -12,6 +12,7 @@ import ( "github.com/zalando/go-keyring" "github.com/stackitcloud/stackit-cli/internal/pkg/config" + "github.com/stackitcloud/stackit-cli/internal/pkg/print" ) func TestSetGetAuthField(t *testing.T) { @@ -183,7 +184,7 @@ func TestSetGetAuthFieldKeyring(t *testing.T) { }{ { description: "simple assignments with default profile", - activeProfile: "", + activeProfile: config.DefaultProfileName, valueAssignments: []valueAssignment{ { key: testField1, @@ -201,7 +202,7 @@ func TestSetGetAuthFieldKeyring(t *testing.T) { }, { description: "overlapping assignments with default profile", - activeProfile: "", + activeProfile: config.DefaultProfileName, valueAssignments: []valueAssignment{ { key: testField1, @@ -267,6 +268,12 @@ func TestSetGetAuthFieldKeyring(t *testing.T) { t.Run(tt.description, func(t *testing.T) { keyring.MockInit() + // Make sure profile name is valid + err := config.ValidateProfile(tt.activeProfile) + if err != nil { + t.Fatalf("Profile name \"%s\" is invalid: %v", tt.activeProfile, err) + } + for _, assignment := range tt.valueAssignments { err := setAuthFieldInKeyring(tt.activeProfile, assignment.key, assignment.value) if err != nil { @@ -317,7 +324,7 @@ func TestSetGetAuthFieldEncodedTextFile(t *testing.T) { }{ { description: "simple assignments with default profile", - activeProfile: "", + activeProfile: config.DefaultProfileName, valueAssignments: []valueAssignment{ { key: testField1, @@ -335,7 +342,7 @@ func TestSetGetAuthFieldEncodedTextFile(t *testing.T) { }, { description: "overlapping assignments with default profile", - activeProfile: "", + activeProfile: config.DefaultProfileName, valueAssignments: []valueAssignment{ { key: testField1, @@ -399,6 +406,26 @@ func TestSetGetAuthFieldEncodedTextFile(t *testing.T) { for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { + // Make sure profile name is valid + err := config.ValidateProfile(tt.activeProfile) + if err != nil { + t.Fatalf("Profile name \"%s\" is invalid: %v", tt.activeProfile, err) + } + + // Create profile if it does not exist + // Will be deleted at the end of the test + profileExists, err := config.ProfileExists(tt.activeProfile) + if err != nil { + t.Fatalf("Failed to check if profile exists: %v", err) + } + if !profileExists { + p := print.NewPrinter() + err := config.CreateProfile(p, tt.activeProfile, true, true) + if err != nil { + t.Fatalf("Failed to create profile: %v", err) + } + } + for _, assignment := range tt.valueAssignments { err := setAuthFieldInEncodedTextFile(tt.activeProfile, assignment.key, assignment.value) if err != nil { @@ -424,6 +451,11 @@ func TestSetGetAuthFieldEncodedTextFile(t *testing.T) { t.Errorf("Post-test cleanup failed: remove field \"%s\" from text file: %v. Please remove it manually", key, err) } } + + err = deleteAuthFieldProfile(tt.activeProfile, profileExists) + if err != nil { + t.Errorf("Post-test cleanup failed: remove profile \"%s\": %v. Please remove it manually", tt.activeProfile, err) + } }) } } @@ -473,3 +505,15 @@ func deleteAuthFieldInEncodedTextFile(activeProfile string, key authFieldKey) er } return nil } + +func deleteAuthFieldProfile(activeProfile string, profileExisted bool) error { + textFileDir := config.GetProfileFolderPath(activeProfile) + if !profileExisted { + // Remove the entire directory if the profile does not exist + err := os.RemoveAll(textFileDir) + if err != nil { + return fmt.Errorf("remove directory: %w", err) + } + } + return nil +} diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index a1a7947fd..eb1ed73a2 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -84,7 +84,7 @@ var profileFilePath string func InitConfig() { defaultConfigFolderPath = getInitialConfigDir() - profileFilePath = filepath.Join(defaultConfigFolderPath, fmt.Sprintf("%s.%s", profileFileName, profileFileExtension)) // Profile file path is in the default config folder + profileFilePath = getInitialProfileFilePath() // Profile file path is in the default config folder configProfile, err := GetProfile() cobra.CheckErr(err) @@ -156,3 +156,11 @@ func getInitialConfigDir() string { return filepath.Join(configDir, configFolder) } + +func getInitialProfileFilePath() string { + configFolderPath := defaultConfigFolderPath + if configFolderPath == "" { + configFolderPath = getInitialConfigDir() + } + return filepath.Join(configFolderPath, fmt.Sprintf("%s.%s", profileFileName, profileFileExtension)) +} diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index 159a8a086..7c9954117 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "os" "path/filepath" "testing" @@ -95,3 +96,32 @@ func TestGetInitialConfigDir(t *testing.T) { }) } } + +func TestGetInitialProfileFilePath(t *testing.T) { + tests := []struct { + description string + configFolderPath string + }{ + { + description: "base", + configFolderPath: getInitialConfigDir(), + }, + { + description: "empty config folder path", + configFolderPath: "", + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + configFolderPath = getInitialConfigDir() + + actual := getInitialProfileFilePath() + + expected := filepath.Join(configFolderPath, fmt.Sprintf("%s.%s", profileFileName, profileFileExtension)) + if actual != expected { + t.Fatalf("expected %s, got %s", expected, actual) + } + }) + } +} diff --git a/internal/pkg/config/profiles.go b/internal/pkg/config/profiles.go index b04f43c81..d2a10b785 100644 --- a/internal/pkg/config/profiles.go +++ b/internal/pkg/config/profiles.go @@ -181,6 +181,10 @@ func SetProfile(p *print.Printer, profile string) error { return &errors.SetInexistentProfile{Profile: profile} } + if profileFilePath == "" { + profileFilePath = getInitialProfileFilePath() + } + err = os.WriteFile(profileFilePath, []byte(profile), os.ModePerm) if err != nil { return fmt.Errorf("write profile to file: %w", err) From 22aba48f6d52a44dbdbef7ddb91eb70fbddf691d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 28 May 2024 15:50:07 +0100 Subject: [PATCH 6/8] add debug logs, improve documentation --- internal/cmd/root.go | 18 ++++++------ internal/pkg/config/profiles.go | 51 +++++++++++++++++++-------------- 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index b1cac4861..0ff4839c2 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -56,20 +56,22 @@ func NewRootCmd(version, date string, p *print.Printer) *cobra.Command { configFilePath := viper.ConfigFileUsed() p.Debug(print.DebugLevel, "configuration is persisted and read from: %s", configFilePath) - activeProfile, err := config.GetProfile() + profileSet, activeProfile, configMethod, err := config.GetConfiguredProfile() if err != nil { - return fmt.Errorf("get profile: %w", err) + return fmt.Errorf("get configured profile: %w", err) } - profileSet, err := config.GetConfiguredProfile() - if err != nil { - return fmt.Errorf("get profile raw: %w", err) - } + p.Debug(print.DebugLevel, "read configuration profile %q via %s", profileSet, configMethod) if activeProfile != profileSet { - p.Debug(print.DebugLevel, "the active profile %q does not exist, following folder is missing: %q", profileSet, config.GetProfileFolderPath(profileSet)) - p.Warn("active profile %q does not exist, the %q profile configuration will be used\n", profileSet, activeProfile) + if configMethod == "" { + p.Debug(print.DebugLevel, "no profile is configured in env var or profile file") + } else { + p.Debug(print.DebugLevel, "the configured profile %q does not exist and the %q profile configuration will be used: folder %q is missing", profileSet, activeProfile, config.GetProfileFolderPath(profileSet)) + } + p.Warn("configured profile %q does not exist, the %q profile configuration will be used\n", profileSet, activeProfile) } + p.Debug(print.DebugLevel, "active configuration profile: %s", activeProfile) configKeys := viper.AllSettings() diff --git a/internal/pkg/config/profiles.go b/internal/pkg/config/profiles.go index d2a10b785..463817962 100644 --- a/internal/pkg/config/profiles.go +++ b/internal/pkg/config/profiles.go @@ -14,54 +14,58 @@ import ( const ProfileEnvVar = "STACKIT_CLI_PROFILE" // GetProfile returns the current profile to be used by the CLI. -// // The profile is determined by the value of the STACKIT_CLI_PROFILE environment variable, or, if not set, // by the contents of the profile file in the CLI config folder. -// // If the profile is not set (env var or profile file) or is set but does not exist, it falls back to the default profile. -// // If the profile is not valid, it returns an error. func GetProfile() (string, error) { - profile, err := GetConfiguredProfile() + _, profile, _, err := GetConfiguredProfile() if err != nil { return "", err } - // Make sure the profile exists - profileExists, err := ProfileExists(profile) - if err != nil { - return "", fmt.Errorf("check if profile exists: %w", err) - } - if !profileExists { - return DefaultProfileName, nil - } - return profile, nil } -// GetConfiguredProfile returns the profile that is configured by the user, which may not exist. +// GetConfiguredProfile returns the profile configured by the user, the profile to be used by the CLI and the method used to configure the profile. // The profile is determined by the value of the STACKIT_CLI_PROFILE environment variable, or, if not set, // by the contents of the profile file in the CLI config folder. -// +// If the configured profile is not set (env var or profile file) or is set but does not exist, it falls back to the default profile. +// The configuration method can be environment variable, profile file or empty if profile is not configured. // If the profile is not valid, it returns an error. -func GetConfiguredProfile() (string, error) { +func GetConfiguredProfile() (configuredProfile, activeProfile, configurationMethod string, err error) { + var configMethod string profile, profileSetInEnv := GetProfileFromEnv() if !profileSetInEnv { contents, exists, err := fileutils.ReadFileIfExists(profileFilePath) if err != nil { - return "", fmt.Errorf("read profile from file: %w", err) + return "", "", "", fmt.Errorf("read profile from file: %w", err) } if !exists { - return DefaultProfileName, nil + // No profile set in env or file + return "", DefaultProfileName, "", nil } profile = contents + configMethod = "profile file" + } else { + configMethod = "environment variable" } - err := ValidateProfile(profile) + // Make sure the profile exists + profileExists, err := ProfileExists(profile) if err != nil { - return "", fmt.Errorf("validate profile: %w", err) + return "", "", "", fmt.Errorf("check if profile exists: %w", err) } - return profile, nil + if !profileExists { + // Profile is configured but does not exist + return profile, DefaultProfileName, configMethod, nil + } + + err = ValidateProfile(profile) + if err != nil { + return "", "", "", fmt.Errorf("validate profile: %w", err) + } + return profile, DefaultProfileName, configMethod, nil } // GetProfileFromEnv returns the profile from the environment variable. @@ -138,7 +142,7 @@ func CreateProfile(p *print.Printer, profile string, setProfile, emptyProfile bo // DuplicateProfileConfiguration duplicates the current profile configuration to a new profile. // It copies the config file from the current profile to the new profile. -// If the current profile does not exist, it returns an error. +// If the current profile does not exist, it does nothing. // If the new profile already exists, it will be overwritten. func DuplicateProfileConfiguration(p *print.Printer, currentProfile, newProfile string) error { currentProfileFolder := GetProfileFolderPath(currentProfile) @@ -150,6 +154,7 @@ func DuplicateProfileConfiguration(p *print.Printer, currentProfile, newProfile _, err := os.Stat(currentConfigFilePath) if err != nil { if os.IsNotExist(err) { + p.Debug(print.DebugLevel, "current profile %q has no configuration, nothing to duplicate", currentProfile) return nil } return fmt.Errorf("get current profile configuration: %w", err) @@ -235,6 +240,8 @@ func ProfileExists(profile string) (bool, error) { return true, nil } +// GetProfileFolderPath returns the path to the folder where the profile configuration is stored. +// If the profile is the default profile, it returns the default config folder path. func GetProfileFolderPath(profile string) string { if defaultConfigFolderPath == "" { defaultConfigFolderPath = getInitialConfigDir() From fd03e32138267dd2dc56ccbb8d0f2c1b4308397a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Wed, 29 May 2024 10:12:16 +0100 Subject: [PATCH 7/8] fix GetConfiguredProfile --- internal/pkg/config/profiles.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pkg/config/profiles.go b/internal/pkg/config/profiles.go index 463817962..c96a42dc8 100644 --- a/internal/pkg/config/profiles.go +++ b/internal/pkg/config/profiles.go @@ -43,7 +43,7 @@ func GetConfiguredProfile() (configuredProfile, activeProfile, configurationMeth } if !exists { // No profile set in env or file - return "", DefaultProfileName, "", nil + return DefaultProfileName, DefaultProfileName, "", nil } profile = contents configMethod = "profile file" @@ -65,7 +65,7 @@ func GetConfiguredProfile() (configuredProfile, activeProfile, configurationMeth if err != nil { return "", "", "", fmt.Errorf("validate profile: %w", err) } - return profile, DefaultProfileName, configMethod, nil + return profile, profile, configMethod, nil } // GetProfileFromEnv returns the profile from the environment variable. From 517e47fee3166f3e4fd06296cdf7f53268a795ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Wed, 29 May 2024 10:28:24 +0100 Subject: [PATCH 8/8] improve debug logs --- internal/cmd/root.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 0ff4839c2..b35dd546e 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -67,8 +67,10 @@ func NewRootCmd(version, date string, p *print.Printer) *cobra.Command { if configMethod == "" { p.Debug(print.DebugLevel, "no profile is configured in env var or profile file") } else { - p.Debug(print.DebugLevel, "the configured profile %q does not exist and the %q profile configuration will be used: folder %q is missing", profileSet, activeProfile, config.GetProfileFolderPath(profileSet)) + p.Debug(print.DebugLevel, "the configured profile %q does not exist: folder %q is missing", profileSet, config.GetProfileFolderPath(profileSet)) } + p.Debug(print.DebugLevel, "the %q profile will be used", activeProfile) + p.Warn("configured profile %q does not exist, the %q profile configuration will be used\n", profileSet, activeProfile) }