Skip to content

Commit

Permalink
fix: Refresh ~/.finch/confg.json based on finch.yaml
Browse files Browse the repository at this point in the history
Suppose we have configured the `creds_helpers` in `~/.finch/finch.yaml` as
follows, and subsequently initialized a VM (`finch vm init`).

```
cpus: 6
creds_helpers:
    - ecr-login
memory: 8GiB
vmType: vz
rosetta: true
```

As a result, `~/.finch/config.json` is created, and it contains the
following:

```
{"credsStore":"ecr-login"}
```

This allows us to utilize the Amazon ECR Docker Credential Helper within Finch.

Subsequently, suppose we stop and remove the VM
(`finch vm stop` && `finch vm remove`), and then remove the
`creds_helpers` configuration from `finch.yaml`.

We then configure the `finch.yaml` file as follows:

```
cpus: 6
memory: 8GiB
vmType: vz
rosetta: true
```

As a result, when we reinitialize the VM (`finch vm init`), the expected
behavior is that it will no longer use the Amazon ECR Docker Credential Helper.

However, when initializing the VM, despite the absence of `creds_helpers`
configuration in `finch.yaml`, the `"credsStore": "ecr-login"` remains in
`config.json`, allowing the continued use of the Amazon ECR Docker
Credential Helper.

This behavior has been reported in the following issue:

- runfinch#480

Furthermore, this issue occurs when we stop the VM (`finch vm stop`),
modify `finch.yaml`, and subsequently start the VM (`finch vm start`).

Consequently, we will modify the behavior to update `config.json` in
accordance with the `creds_helpers` configuration in `finch.yaml` when
initiating or starting the VM.

Note that in this pull request, the commits are divided as follows:

- Implement the logic to update `config.json` according to `finch.yaml`
  and change the function name (loadFinchConfig) in the config package
- Modify to call the logic to update `config.json` according to
  `finch.yaml` on the VM side
- Add unit tests for the credhelper package and modify unit tests for the
  config package
- Modify and add Behavior-Driven Development (BDD) Tests using Ginkgo on
  the VM side
- Add e2e tests

On the other hand, in this commit, the logic to update `config.json`
according to `finch.yaml` is implemented and the function name
(loadFinchConfig) is changed in the config package.

Signed-off-by: Hayato Kiwata <haytok@amazon.co.jp>
  • Loading branch information
haytok committed Jun 8, 2024
1 parent f441fc9 commit 9a5dcae
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 3 deletions.
6 changes: 3 additions & 3 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ func Load(
return defCfg, nil
}

// loadFinchConfig Load Finch's configuration from a YAML file.
func loadFinchConfig(fs afero.Fs, finchConfigPath string, logger flog.Logger, systemDeps LoadSystemDeps, mem fmemory.Memory) (*Finch, error) {
// LoadFinchConfig Load Finch's configuration from a YAML file.
func LoadFinchConfig(fs afero.Fs, finchConfigPath string, logger flog.Logger, systemDeps LoadSystemDeps, mem fmemory.Memory) (*Finch, error) {
b, err := afero.ReadFile(fs, finchConfigPath)
if err != nil {
return nil, fmt.Errorf("failed to read config file: %w", err)
Expand All @@ -202,7 +202,7 @@ func ModifyFinchConfig(fs afero.Fs, logger flog.Logger, finchConfigPath string,
systemDeps := system.NewStdLib()
mem := fmemory.NewMemory()

finchCfg, err := loadFinchConfig(fs, finchConfigPath, logger, systemDeps, mem)
finchCfg, err := LoadFinchConfig(fs, finchConfigPath, logger, systemDeps, mem)
if err != nil {
return isConfigUpdated, err
}
Expand Down
68 changes: 68 additions & 0 deletions pkg/dependency/credhelper/cred_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,23 @@
package credhelper

import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"slices"

"github.com/docker/cli/cli/config/configfile"
"github.com/docker/cli/cli/config/types"
"github.com/spf13/afero"

"github.com/runfinch/finch/pkg/command"
"github.com/runfinch/finch/pkg/config"
"github.com/runfinch/finch/pkg/dependency"
"github.com/runfinch/finch/pkg/flog"
"github.com/runfinch/finch/pkg/fmemory"
"github.com/runfinch/finch/pkg/path"
"github.com/runfinch/finch/pkg/system"
)

const (
Expand Down Expand Up @@ -96,3 +103,64 @@ func newDeps(

return deps
}

// RefreshConfigFile refreshes config.json according to finch.yaml.
func RefreshConfigFile(fs afero.Fs, logger flog.Logger, finchConfigPath, configJSONPath string) error {
systemDeps := system.NewStdLib()
mem := fmemory.NewMemory()

finchCfg, err := config.LoadFinchConfig(fs, finchConfigPath, logger, systemDeps, mem)
if err != nil {
return err
}
if slices.Contains(finchCfg.CredsHelpers, "ecr-login") {
return nil
}

fileExists, err := afero.Exists(fs, configJSONPath)
if err != nil {
return err
}
if !fileExists {
return nil
}

fileRead, err := fs.Open(configJSONPath)
if err != nil {
return err
}
defer fileRead.Close() //nolint:errcheck // closing the file

var cfg configfile.ConfigFile

bytes, _ := afero.ReadAll(fileRead)
err = json.Unmarshal(bytes, &cfg)
if err != nil {
return err
}
if cfg.CredentialsStore == "" {
return nil
}

cfg.CredentialsStore = ""
if cfg.AuthConfigs == nil {
cfg.AuthConfigs = map[string]types.AuthConfig{}
}

finalCfgBytes, err := json.Marshal(&cfg)
if err != nil {
return err
}

file, err := fs.OpenFile(configJSONPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o600)
if err != nil {
return err
}
_, err = file.Write(finalCfgBytes)
if err != nil {
return err
}
defer file.Close() //nolint:errcheck // closing the file

return nil
}

0 comments on commit 9a5dcae

Please sign in to comment.