Skip to content

Commit

Permalink
Lock config file for all write operations (#1156)
Browse files Browse the repository at this point in the history
* Lock config file for all write operations

* Fix linter errs

* tidy

* more tidy
  • Loading branch information
agouin authored Mar 31, 2023
1 parent 9c7e897 commit 8dcf4c1
Show file tree
Hide file tree
Showing 10 changed files with 606 additions and 619 deletions.
191 changes: 107 additions & 84 deletions cmd/appstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

"github.com/cosmos/relayer/v2/relayer"
"github.com/gofrs/flock"
"github.com/spf13/cobra"
"github.com/spf13/viper"
"go.uber.org/zap"
"gopkg.in/yaml.v3"
Expand All @@ -22,18 +21,61 @@ type appState struct {
// Log is the root logger of the application.
// Consumers are expected to store and use local copies of the logger
// after modifying with the .With method.
Log *zap.Logger
log *zap.Logger

Viper *viper.Viper
viper *viper.Viper

HomePath string
Debug bool
Config *Config
homePath string
debug bool
config *Config
}

// AddPathFromFile modifies a.config.Paths to include the content stored in the given file.
func (a *appState) configPath() string {
return path.Join(a.homePath, "config", "config.yaml")
}

// loadConfigFile reads config file into a.Config if file is present.
func (a *appState) loadConfigFile(ctx context.Context) error {
cfgPath := a.configPath()

if _, err := os.Stat(cfgPath); err != nil {
// don't return error if file doesn't exist
return nil
}

// read the config file bytes
file, err := os.ReadFile(cfgPath)
if err != nil {
return fmt.Errorf("error reading file: %w", err)
}

// unmarshall them into the wrapper struct
cfgWrapper := &ConfigInputWrapper{}
err = yaml.Unmarshal(file, cfgWrapper)
if err != nil {
return fmt.Errorf("error unmarshalling config: %w", err)
}

// retrieve the runtime configuration from the disk configuration.
newCfg, err := cfgWrapper.RuntimeConfig(ctx, a)
if err != nil {
return err
}

// validate runtime configuration
if err := newCfg.validateConfig(); err != nil {
return fmt.Errorf("error parsing chain config: %w", err)
}

// save runtime configuration in app state
a.config = newCfg

return nil
}

// addPathFromFile modifies a.config.Paths to include the content stored in the given file.
// If a non-nil error is returned, a.config.Paths is not modified.
func (a *appState) AddPathFromFile(ctx context.Context, stderr io.Writer, file, name string) error {
func (a *appState) addPathFromFile(ctx context.Context, stderr io.Writer, file, name string) error {
if _, err := os.Stat(file); err != nil {
return err
}
Expand All @@ -48,17 +90,22 @@ func (a *appState) AddPathFromFile(ctx context.Context, stderr io.Writer, file,
return err
}

if err = a.Config.ValidatePath(ctx, stderr, p); err != nil {
if err = a.config.ValidatePath(ctx, stderr, p); err != nil {
return err
}

return a.Config.Paths.Add(name, p)
return a.config.Paths.Add(name, p)
}

// AddPathFromUserInput manually prompts the user to specify all the path details.
// addPathFromUserInput manually prompts the user to specify all the path details.
// It returns any input or validation errors.
// If the path was successfully added, it returns nil.
func (a *appState) AddPathFromUserInput(ctx context.Context, stdin io.Reader, stderr io.Writer, src, dst, name string) error {
func (a *appState) addPathFromUserInput(
ctx context.Context,
stdin io.Reader,
stderr io.Writer,
src, dst, name string,
) error {
// TODO: confirm name is available before going through input.

var (
Expand Down Expand Up @@ -118,108 +165,51 @@ func (a *appState) AddPathFromUserInput(ctx context.Context, stdin io.Reader, st
return err
}

if err := a.Config.ValidatePath(ctx, stderr, path); err != nil {
return err
}

return a.Config.Paths.Add(name, path)
}

// OverwriteConfig overwrites the config files on disk with the serialization of cfg,
// and it replaces a.Config with cfg.
//
// It is possible to use a brand new Config argument,
// but typically the argument is a.Config.
func (a *appState) OverwriteConfig(cfg *Config) error {
cfgPath := path.Join(a.HomePath, "config", "config.yaml")
if _, err := os.Stat(cfgPath); err != nil {
return fmt.Errorf("failed to check existence of config file at %s: %w", cfgPath, err)
}

a.Viper.SetConfigFile(cfgPath)
if err := a.Viper.ReadInConfig(); err != nil {
// TODO: if we failed to read in the new config, should we restore the old config?
return fmt.Errorf("failed to read config file at %s: %w", cfgPath, err)
}

// ensure validateConfig runs properly
if err := validateConfig(cfg); err != nil {
return fmt.Errorf("failed to validate config at %s: %w", cfgPath, err)
}

// marshal the new config
out, err := yaml.Marshal(cfg.Wrapped())
if err != nil {
if err := a.config.ValidatePath(ctx, stderr, path); err != nil {
return err
}

// Overwrite the config file.
if err := os.WriteFile(a.Viper.ConfigFileUsed(), out, 0600); err != nil {
return fmt.Errorf("failed to write config file at %s: %w", cfgPath, err)
}

// Write the config back into the app state.
a.Config = cfg
return nil
return a.config.Paths.Add(name, path)
}

// OverwriteConfigOnTheFly overwrites the config file concurrently,
// locking to read, modify, then write the config.
func (a *appState) OverwriteConfigOnTheFly(
cmd *cobra.Command,
pathName string,
clientSrc, clientDst string,
connectionSrc, connectionDst string,
) error {
if pathName == "" {
return errors.New("empty path name not allowed")
}

// use lock file to guard concurrent access to config.yaml
lockFilePath := path.Join(a.HomePath, "config", "config.lock")
func (a *appState) performConfigLockingOperation(ctx context.Context, operation func() error) error {
lockFilePath := path.Join(a.homePath, "config", "config.lock")
fileLock := flock.New(lockFilePath)
_, err := fileLock.TryLock()
if err != nil {
return fmt.Errorf("failed to acquire config lock: %w", err)
}
defer func() {
if err := fileLock.Unlock(); err != nil {
a.Log.Error("error unlocking config file lock, please manually delete",
a.log.Error("error unlocking config file lock, please manually delete",
zap.String("filepath", lockFilePath),
)
}
}()

// load config from file and validate it. don't want to miss
// any changes that may have been made while unlocked.
if err := initConfig(cmd, a); err != nil {
if err := a.loadConfigFile(ctx); err != nil {
return fmt.Errorf("failed to initialize config from file: %w", err)
}

path, ok := a.Config.Paths[pathName]
if !ok {
return fmt.Errorf("config does not exist for that path: %s", pathName)
}
if clientSrc != "" {
path.Src.ClientID = clientSrc
}
if clientDst != "" {
path.Dst.ClientID = clientDst
}
if connectionSrc != "" {
path.Src.ConnectionID = connectionSrc
// perform the operation that requires config flock.
if err := operation(); err != nil {
return err
}
if connectionDst != "" {
path.Dst.ConnectionID = connectionDst

// validate config after changes have been made.
if err := a.config.validateConfig(); err != nil {
return fmt.Errorf("error parsing chain config: %w", err)
}

// marshal the new config
out, err := yaml.Marshal(a.Config.Wrapped())
out, err := yaml.Marshal(a.config.Wrapped())
if err != nil {
return err
}

cfgPath := a.Viper.ConfigFileUsed()
cfgPath := a.configPath()

// Overwrite the config file.
if err := os.WriteFile(cfgPath, out, 0600); err != nil {
Expand All @@ -228,3 +218,36 @@ func (a *appState) OverwriteConfigOnTheFly(

return nil
}

// updatePathConfig overwrites the config file concurrently,
// locking to read, modify, then write the config.
func (a *appState) updatePathConfig(
ctx context.Context,
pathName string,
clientSrc, clientDst string,
connectionSrc, connectionDst string,
) error {
if pathName == "" {
return errors.New("empty path name not allowed")
}

return a.performConfigLockingOperation(ctx, func() error {
path, ok := a.config.Paths[pathName]
if !ok {
return fmt.Errorf("config does not exist for that path: %s", pathName)
}
if clientSrc != "" {
path.Src.ClientID = clientSrc
}
if clientDst != "" {
path.Dst.ClientID = clientDst
}
if connectionSrc != "" {
path.Src.ConnectionID = connectionSrc
}
if connectionDst != "" {
path.Dst.ConnectionID = connectionDst
}
return nil
})
}
Loading

0 comments on commit 8dcf4c1

Please sign in to comment.