diff --git a/common/commands/config.go b/common/commands/config.go index c69adc403..a54814af1 100644 --- a/common/commands/config.go +++ b/common/commands/config.go @@ -2,26 +2,34 @@ package commands import ( "fmt" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" + "github.com/jfrog/jfrog-cli-core/v2/utils/ioutils" + "github.com/jfrog/jfrog-cli-core/v2/utils/lock" "io/ioutil" "reflect" "strconv" "strings" "sync" - "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" - "github.com/jfrog/jfrog-cli-core/v2/utils/ioutils" - "github.com/jfrog/jfrog-client-go/auth" "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" "github.com/jfrog/jfrog-cli-core/v2/utils/config" - "github.com/jfrog/jfrog-cli-core/v2/utils/lock" clientutils "github.com/jfrog/jfrog-client-go/utils" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "github.com/jfrog/jfrog-client-go/utils/log" ) +type ConfigAction string + +const ( + AddOrEdit ConfigAction = "AddOrEdit" + Delete ConfigAction = "Delete" + Use ConfigAction = "Use" + Clear ConfigAction = "Clear" +) + // Internal golang locking for the same process. var mutex sync.Mutex @@ -34,10 +42,11 @@ type ConfigCommand struct { serverId string // For unit tests disablePromptUrls bool + cmdType ConfigAction } -func NewConfigCommand() *ConfigCommand { - return &ConfigCommand{} +func NewConfigCommand(cmdType ConfigAction, serverId string) *ConfigCommand { + return &ConfigCommand{cmdType: cmdType, serverId: serverId} } func (cc *ConfigCommand) SetServerId(serverId string) *ConfigCommand { @@ -71,7 +80,34 @@ func (cc *ConfigCommand) SetDetails(details *config.ServerDetails) *ConfigComman } func (cc *ConfigCommand) Run() error { - return cc.Config() + mutex.Lock() + defer mutex.Unlock() + lockDirPath, err := coreutils.GetJfrogConfigLockDir() + if err != nil { + return err + } + lockFile, err := lock.CreateLock(lockDirPath) + defer func() { + e := lockFile.Unlock() + if err == nil { + err = e + } + }() + if err != nil { + return err + } + switch cc.cmdType { + case AddOrEdit: + return cc.config() + case Delete: + return cc.delete() + case Use: + return cc.use() + case Clear: + return cc.clear() + default: + return fmt.Errorf("Not supported config command type: " + string(cc.cmdType)) + } } func (cc *ConfigCommand) ServerDetails() (*config.ServerDetails, error) { @@ -90,20 +126,7 @@ func (cc *ConfigCommand) CommandName() string { return "config" } -func (cc *ConfigCommand) Config() error { - mutex.Lock() - defer mutex.Unlock() - lockDirPath, err := coreutils.GetJfrogConfigLockDir() - if err != nil { - return err - } - lockFile, err := lock.CreateLock(lockDirPath) - defer lockFile.Unlock() - - if err != nil { - return err - } - +func (cc *ConfigCommand) config() error { configurations, err := cc.prepareConfigurationData() if err != nil { return err @@ -134,7 +157,7 @@ func (cc *ConfigCommand) Config() error { // Artifactory expects the username to be lower-cased. In case it is not, // Artifactory will silently save it lower-cased, but the token creation - // REST API will fail with a non lower-cased username. + // REST API will fail with a non-lower-cased username. cc.details.User = strings.ToLower(cc.details.User) if len(configurations) == 1 { @@ -211,7 +234,7 @@ func (cc *ConfigCommand) prepareConfigurationData() ([]*config.ServerDetails, er return configurations, err } -/// Returning the first non empty value: +/// Returning the first non-empty value: // 1. The serverId argument sent. // 2. details.ServerId // 3. defaultDetails.ServerId @@ -388,7 +411,7 @@ func Import(serverToken string) error { details: serverDetails, serverId: serverDetails.ServerId, } - return configCommand.Config() + return configCommand.config() } func Export(serverName string) error { @@ -438,14 +461,14 @@ func logIfNotEmpty(value, prefix string, mask bool) { } } -func DeleteConfig(serverName string) error { +func (cc *ConfigCommand) delete() error { configurations, err := config.GetAllServersConfigs() if err != nil { return err } var isDefault, isFoundName bool for i, serverDetails := range configurations { - if serverDetails.ServerId == serverName { + if serverDetails.ServerId == cc.serverId { isDefault = serverDetails.IsDefault configurations = append(configurations[:i], configurations[i+1:]...) isFoundName = true @@ -459,12 +482,12 @@ func DeleteConfig(serverName string) error { if isFoundName { return config.SaveServersConf(configurations) } - log.Info("\"" + serverName + "\" configuration could not be found.\n") + log.Info("\"" + cc.serverId + "\" configuration could not be found.\n") return nil } // Set the default configuration -func Use(serverId string) error { +func (cc *ConfigCommand) use() error { configurations, err := config.GetAllServersConfigs() if err != nil { return err @@ -472,7 +495,7 @@ func Use(serverId string) error { var serverFound *config.ServerDetails newDefaultServer := true for _, serverDetails := range configurations { - if serverDetails.ServerId == serverId { + if serverDetails.ServerId == cc.serverId { serverFound = serverDetails if serverDetails.IsDefault { newDefaultServer = false @@ -494,11 +517,11 @@ func Use(serverId string) error { log.Info(fmt.Sprintf("Using server ID '%s' (%s).", serverFound.ServerId, serverFound.Url)) return nil } - return errorutils.CheckErrorf("Could not find a server with ID '%s'.", serverId) + return errorutils.CheckErrorf("Could not find a server with ID '%s'.", cc.serverId) } -func ClearConfig(interactive bool) error { - if interactive { +func (cc *ConfigCommand) clear() error { + if cc.interactive { confirmed := coreutils.AskYesNo("Are you sure you want to delete all the configurations?", false) if !confirmed { return nil diff --git a/common/commands/config_test.go b/common/commands/config_test.go index b197362eb..4a1c5982e 100644 --- a/common/commands/config_test.go +++ b/common/commands/config_test.go @@ -124,13 +124,13 @@ func TestBasicAuthOnlyOption(t *testing.T) { outputConfig, err := configAndGetTestServer(t, inputDetails, true, false) assert.NoError(t, err) assert.Equal(t, coreutils.TokenRefreshDisabled, outputConfig.TokenRefreshInterval, "expected refreshable token to be disabled") - assert.NoError(t, DeleteConfig("test")) + assert.NoError(t, NewConfigCommand(Delete, "test").Run()) // Verify setting the option enables refreshable tokens. outputConfig, err = configAndGetTestServer(t, inputDetails, false, false) assert.NoError(t, err) assert.Equal(t, coreutils.TokenRefreshDefaultInterval, outputConfig.TokenRefreshInterval, "expected refreshable token to be enabled") - assert.NoError(t, DeleteConfig("test")) + assert.NoError(t, NewConfigCommand(Delete, "test").Run()) } func TestExportEmptyConfig(t *testing.T) { @@ -161,14 +161,14 @@ func configAndTest(t *testing.T, inputDetails *config.ServerDetails, interactive outputConfig, err := configAndGetTestServer(t, inputDetails, true, interactive) assert.NoError(t, err) assert.Equal(t, configStructToString(inputDetails), configStructToString(outputConfig), "unexpected configuration was saved to file") - assert.NoError(t, DeleteConfig("test")) + assert.NoError(t, NewConfigCommand(Delete, "test").Run()) testExportImport(t, inputDetails) } func configAndGetTestServer(t *testing.T, inputDetails *config.ServerDetails, basicAuthOnly, interactive bool) (*config.ServerDetails, error) { - configCmd := NewConfigCommand().SetDetails(inputDetails).SetServerId("test").SetUseBasicAuthOnly(basicAuthOnly).SetInteractive(interactive) + configCmd := NewConfigCommand(AddOrEdit, "test").SetDetails(inputDetails).SetUseBasicAuthOnly(basicAuthOnly).SetInteractive(interactive) configCmd.disablePromptUrls = true - assert.NoError(t, configCmd.Config()) + assert.NoError(t, configCmd.Run()) return GetConfig("test", false) } diff --git a/general/envsetup/envsetup.go b/general/envsetup/envsetup.go index 485eb031e..c89c4b47c 100644 --- a/general/envsetup/envsetup.go +++ b/general/envsetup/envsetup.go @@ -218,11 +218,11 @@ func configServer(server *config.ServerDetails) error { } // Take the server name from host name: https://myjfrog.jfrog.com/ -> myjfrog serverId := strings.Split(u.Host, ".")[0] - configCmd := commands.NewConfigCommand().SetInteractive(false).SetServerId(serverId).SetDetails(server) - if err = configCmd.Config(); err != nil { + configCmd := commands.NewConfigCommand(commands.AddOrEdit, serverId).SetInteractive(false).SetDetails(server) + if err = configCmd.Run(); err != nil { return err } - return commands.Use(serverId) + return commands.NewConfigCommand(commands.Use, serverId).SetInteractive(false).SetDetails(server).Run() } type myJfrogGetStatusRequest struct { diff --git a/utils/config/config.go b/utils/config/config.go index 5a1ed3f6b..0fbeb52e1 100644 --- a/utils/config/config.go +++ b/utils/config/config.go @@ -4,26 +4,25 @@ import ( "bytes" "encoding/json" "errors" - accessAuth "github.com/jfrog/jfrog-client-go/access/auth" - pipelinesAuth "github.com/jfrog/jfrog-client-go/pipelines/auth" - "io/ioutil" - "os" - "path/filepath" - "strconv" - "strings" - "time" - "github.com/buger/jsonparser" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" cliLog "github.com/jfrog/jfrog-cli-core/v2/utils/log" + accessAuth "github.com/jfrog/jfrog-client-go/access/auth" artifactoryAuth "github.com/jfrog/jfrog-client-go/artifactory/auth" "github.com/jfrog/jfrog-client-go/auth" distributionAuth "github.com/jfrog/jfrog-client-go/distribution/auth" + pipelinesAuth "github.com/jfrog/jfrog-client-go/pipelines/auth" "github.com/jfrog/jfrog-client-go/utils" "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "github.com/jfrog/jfrog-client-go/utils/log" xrayAuth "github.com/jfrog/jfrog-client-go/xray/auth" + "io/ioutil" + "os" + "path/filepath" + "strconv" + "strings" + "time" ) func init() { diff --git a/utils/lock/lock.go b/utils/lock/lock.go index ab69788d4..96054be19 100644 --- a/utils/lock/lock.go +++ b/utils/lock/lock.go @@ -38,7 +38,6 @@ func (locks Locks) Less(i, j int) bool { // Creating a new lock object. func (lock *Lock) createNewLockFile(lockDirPath string) error { - lock.currentTime = time.Now().UnixNano() err := fileutils.CreateDirIfNotExist(lockDirPath) if err != nil { return err @@ -50,13 +49,17 @@ func (lock *Lock) createNewLockFile(lockDirPath string) error { func (lock *Lock) createFile(folderName string, pid int) error { // We are creating an empty file with the pid and current time part of the name - lock.fileName = filepath.Join(folderName, "jfrog-cli.conf.lck."+strconv.Itoa(pid)+"."+strconv.FormatInt(lock.currentTime, 10)) + lock.fileName = filepath.Join(folderName, "jfrog-cli.conf.lck."+strconv.Itoa(pid)+"."+strconv.FormatInt(time.Now().UnixNano(), 10)) log.Debug("Creating lock file: ", lock.fileName) file, err := os.OpenFile(lock.fileName, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666) if err != nil { return errorutils.CheckError(err) } - + fileInfo, err := file.Stat() + if err != nil { + return errorutils.CheckError(err) + } + lock.currentTime = fileInfo.ModTime().UnixNano() if err = file.Close(); err != nil { return errorutils.CheckError(err) } @@ -85,7 +88,7 @@ func (lock *Lock) lock() error { // If the first timestamp in the sorted locks slice is equal to this timestamp // means that the lock can be acquired if locks[0].currentTime == lock.currentTime { - // Edge case, if at the same time (by the nano seconds) two different process created two files. + // Edge case, if at the same time (by the nanoseconds) two different process created two files. // We are checking the PID to know which process can run. if locks[0].pid != lock.pid { err := lock.removeOtherLockOrWait(locks[0], &filesList) @@ -163,23 +166,24 @@ func (lock *Lock) getLocks(filesList []string) (Locks, error) { // Slice of all the timestamps that currently the lock directory has var files Locks for _, path := range filesList { - fileName := filepath.Base(path) - splitted := strings.Split(fileName, ".") - - if len(splitted) != 5 { - return nil, errorutils.CheckErrorf("Failed while parsing the file name: %s located at: %s. Expecting a different format.", fileName, path) - } - // Last element is the timestamp. - time, err := strconv.ParseInt(splitted[4], 10, 64) + fileInfo, err := os.Stat(path) if err != nil { + if os.IsNotExist(err) { + // If file doesn't exist, then lock already deleted + continue + } return nil, errorutils.CheckError(err) } + splitted := strings.Split(fileInfo.Name(), ".") + if len(splitted) != 5 { + return nil, errorutils.CheckErrorf("Failed while parsing the file name: %s located at: %s. Expecting a different format.", fileInfo.Name(), path) + } pid, err := strconv.Atoi(splitted[3]) if err != nil { return nil, errorutils.CheckError(err) } file := Lock{ - currentTime: time, + currentTime: fileInfo.ModTime().UnixNano(), pid: pid, fileName: path, } diff --git a/utils/lock/lock_test.go b/utils/lock/lock_test.go index 6a2598324..c03bf327c 100644 --- a/utils/lock/lock_test.go +++ b/utils/lock/lock_test.go @@ -10,7 +10,6 @@ import ( "os" "path/filepath" "testing" - "time" ) var testLockDirPath string @@ -180,11 +179,7 @@ func TestCreateFile(t *testing.T) { } func getLock(pid int, t *testing.T) (Lock, string) { - currentTime := time.Now().UnixNano() - lock := Lock{ - pid: pid, - currentTime: currentTime, - } + lock := Lock{pid: pid} assert.NotZero(t, testLockDirPath, "An error occurred while initializing testLockDirPath") assert.NoError(t, fileutils.CreateDirIfNotExist(testLockDirPath)) err := lock.createFile(testLockDirPath, pid)