diff --git a/cmd/finch/nerdctl.go b/cmd/finch/nerdctl.go index bcf5ccde1..468af0bed 100644 --- a/cmd/finch/nerdctl.go +++ b/cmd/finch/nerdctl.go @@ -173,7 +173,15 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { var additionalEnv []string switch cmdName { - case "build", "pull", "push": + case "image": + if slices.Contains(args, "build") || slices.Contains(args, "pull") || slices.Contains(args, "push") { + ensureRemoteCredentials(nc.fc, nc.ecc, &additionalEnv, nc.logger) + } + case "container": + if slices.Contains(args, "run") { + ensureRemoteCredentials(nc.fc, nc.ecc, &additionalEnv, nc.logger) + } + case "build", "pull", "push", "run": ensureRemoteCredentials(nc.fc, nc.ecc, &additionalEnv, nc.logger) } diff --git a/pkg/dependency/credhelper/cred_helper_binary.go b/pkg/dependency/credhelper/cred_helper_binary.go index 8a3ed2b50..f3f2f241e 100644 --- a/pkg/dependency/credhelper/cred_helper_binary.go +++ b/pkg/dependency/credhelper/cred_helper_binary.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" "strings" "github.com/opencontainers/go-digest" @@ -49,7 +50,7 @@ func newCredHelperBinary(fp path.Finch, fs afero.Fs, cmdCreator command.Creator, // updateConfigFile updates the config.json file to configure the credential helper. func updateConfigFile(bin *credhelperbin) error { - cfgPath := fmt.Sprintf("%s%s", bin.hcfg.finchPath, "config.json") + cfgPath := filepath.Join(bin.hcfg.finchPath, "config.json") binCfgName := bin.credHelperConfigName() fileExists, err := afero.Exists(bin.fs, cfgPath) if err != nil { @@ -71,6 +72,7 @@ func updateConfigFile(bin *credhelperbin) error { if err != nil { return err } + defer fileRead.Close() //nolint:errcheck // closing the file bytes, err := afero.ReadAll(fileRead) if err != nil { return err @@ -81,8 +83,7 @@ func updateConfigFile(bin *credhelperbin) error { return err } credsStore := cfg.CredentialsStore - defer fileRead.Close() //nolint:errcheck // closing the file - if strings.Compare(credsStore, binCfgName) != 0 { + if credsStore != binCfgName { file, err := bin.fs.OpenFile(cfgPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o755) if err != nil { return err @@ -103,6 +104,38 @@ func updateConfigFile(bin *credhelperbin) error { return nil } +func (bin *credhelperbin) binaryInstalled() (bool, error) { + return binaryInstalled(bin) +} + +func (bin *credhelperbin) configFileInstalled() (bool, error) { + cfgPath := filepath.Join(bin.hcfg.finchPath, "config.json") + binCfgName := bin.credHelperConfigName() + + if fileExists, err := afero.Exists(bin.fs, cfgPath); err != nil { + return false, err + } else if !fileExists { + return false, nil + } + + fileRead, err := bin.fs.Open(cfgPath) + if err != nil { + return false, err + } + bytes, err := afero.ReadAll(fileRead) + if err != nil { + return false, err + } + var cfg configfile.ConfigFile + err = json.Unmarshal(bytes, &cfg) + if err != nil { + return false, err + } + credsStore := cfg.CredentialsStore + defer fileRead.Close() //nolint:errcheck // closing the file + return credsStore == binCfgName, nil +} + // credHelperConfigName returns the name of the credential helper binary that will be used // inside the config.json. func (bin *credhelperbin) credHelperConfigName() string { @@ -111,81 +144,80 @@ func (bin *credhelperbin) credHelperConfigName() string { // fullInstallPath returns the full installation path of the credential helper binary. func (bin *credhelperbin) fullInstallPath() string { - return fmt.Sprintf("%s%s", bin.hcfg.installFolder, bin.hcfg.binaryName) + return filepath.Join(bin.hcfg.installFolder, bin.hcfg.binaryName) } // Installed checks if the credential helper already exists in the specified // folder and checks if the hash of the installed binary is correct. func (bin *credhelperbin) Installed() bool { - dirExists, err := afero.DirExists(bin.fs, bin.hcfg.installFolder) - if err != nil { - bin.l.Errorf("failed to get status of credential helper directory: %v", err) - return false - } - if !dirExists { - return false - } - fileExists, err := afero.Exists(bin.fs, bin.fullInstallPath()) - if err != nil { - bin.l.Errorf("failed to get status of credential helper binary: %v", err) - return false - } - if !fileExists { - return false - } - file, err := bin.fs.Open(bin.fullInstallPath()) - if err != nil { + if binInstalled, err := bin.binaryInstalled(); err != nil { bin.l.Error(err) return false + } else if !binInstalled { + return false } - defer file.Close() //nolint:errcheck // closing the file - hash, err := digest.FromReader(file) - if err != nil { + + if cfgInstalled, err := bin.configFileInstalled(); err != nil { bin.l.Error(err) return false - } - if strings.Compare(hash.String(), bin.hcfg.hash) != 0 { - bin.l.Info("Hash of the installed credential helper binary does not match") - err := bin.fs.Remove(bin.fullInstallPath()) - if err != nil { - bin.l.Error(err) - } + } else if !cfgInstalled { return false } + return true } // Install installs and configures the specified credential helper. func (bin *credhelperbin) Install() error { - if bin.helper == "" { - return nil - } - if strings.Compare(bin.helper, bin.credHelperConfigName()) != 0 { - return nil - } - credHelperName := strings.ReplaceAll(bin.credHelperConfigName(), "-login", "") - bin.l.Infof("Installing %s credential helper", credHelperName) - mkdirCmd := bin.cmdCreator.Create("mkdir", "-p", bin.hcfg.installFolder) - _, err := mkdirCmd.Output() + binInstalled, err := bin.binaryInstalled() if err != nil { - return fmt.Errorf("error creating installation directory %s, err: %w", bin.hcfg.installFolder, err) + return err } - curlCmd := bin.cmdCreator.Create("curl", "--retry", "5", "--retry-max-time", "30", "--url", - bin.hcfg.credHelperURL, "--output", bin.fullInstallPath()) + if !binInstalled { + if bin.helper == "" { + return nil + } + if bin.helper != bin.credHelperConfigName() { + return nil + } + credHelperName := strings.ReplaceAll(bin.credHelperConfigName(), "-login", "") + bin.l.Infof("Installing %s credential helper", credHelperName) + if err := bin.fs.MkdirAll(bin.hcfg.installFolder, 0o700); err != nil { + return fmt.Errorf("error creating installation directory %s, err: %w", bin.hcfg.installFolder, err) + } - _, err = curlCmd.Output() - if err != nil { - return fmt.Errorf("error installing binary %s, err: %w", bin.hcfg.binaryName, err) + curlCmd := bin.cmdCreator.Create( + "curl", + "--retry", + "5", + "--retry-max-time", + "30", + "--url", + bin.hcfg.credHelperURL, + "--output", + bin.fullInstallPath(), + ) + + if _, err = curlCmd.Output(); err != nil { + return fmt.Errorf("error installing binary %s, err: %w", bin.hcfg.binaryName, err) + } + if err := bin.fs.Chmod(bin.fullInstallPath(), 0o700); err != nil { + return err + } } - err = bin.fs.Chmod(bin.fullInstallPath(), 0o755) + + cfgInstalled, err := bin.configFileInstalled() if err != nil { return err } - err = updateConfigFile(bin) - if err != nil { - return err + + if !cfgInstalled { + if err := updateConfigFile(bin); err != nil { + return err + } } + return nil } @@ -193,3 +225,45 @@ func (bin *credhelperbin) Install() error { func (bin *credhelperbin) RequiresRoot() bool { return false } + +// Using a var function allows overriding during testing. +// This is needed because the curl command directly outputs to a file, but binaryInstalled deletes +// any incorrect file that might exist at the fullInstallPath. +// This means that the mocking method that is typically used to mock filesystem will get the file it +// creates deleted, and then, since cmd.Output() is what is writing the new/correct file, and there's +// no opportunity to mock it or insert the file after it runs, the code that expects the file to exist +// then errors out because it was deleted by binaryInstalled. +var binaryInstalled = func(bin *credhelperbin) (bool, error) { + dirExists, err := afero.DirExists(bin.fs, bin.hcfg.installFolder) + if err != nil { + return false, fmt.Errorf("failed to get status of credential helper directory: %w", err) + } + if !dirExists { + return false, nil + } + fileExists, err := afero.Exists(bin.fs, bin.fullInstallPath()) + if err != nil { + return false, fmt.Errorf("failed to get status of credential helper binary: %w", err) + } + if !fileExists { + return false, nil + } + file, err := bin.fs.Open(bin.fullInstallPath()) + if err != nil { + return false, fmt.Errorf("failed to open cred helper binary: %w", err) + } + defer file.Close() //nolint:errcheck // closing the file + hash, err := digest.FromReader(file) + if err != nil { + return false, fmt.Errorf("failed to get cred helper binary hash: %w", err) + } + if hash.String() != bin.hcfg.hash { + bin.l.Info("Hash of the installed credential helper binary does not match") + if err := bin.fs.Remove(bin.fullInstallPath()); err != nil { + return false, fmt.Errorf("failed to remove mismatched cred helper binary: %w", err) + } + return false, nil + } + + return true, nil +} diff --git a/pkg/dependency/credhelper/cred_helper_binary_test.go b/pkg/dependency/credhelper/cred_helper_binary_test.go index 000cdea42..c132a44a7 100644 --- a/pkg/dependency/credhelper/cred_helper_binary_test.go +++ b/pkg/dependency/credhelper/cred_helper_binary_test.go @@ -149,9 +149,23 @@ func TestBinaries_Installed(t *testing.T) { fileData := []byte("") _, err := mFs.Create("mock_prefix/cred-helpers/docker-credential-binary") require.NoError(t, err) - err = afero.WriteFile(mFs, "mock_prefix/cred-helpers/docker-credential-binary", - fileData, 0o666) + err = afero.WriteFile( + mFs, + "mock_prefix/cred-helpers/docker-credential-binary", + fileData, + 0o666, + ) + require.NoError(t, err) + cfgFileData := []byte(`{"credsStore":"binary"}`) + _, err = mFs.Create("mock_prefix/.finch/config.json") + require.NoError(t, err) + err = afero.WriteFile( + mFs, + "mock_prefix/.finch/config.json", + cfgFileData, + 0o666, + ) require.NoError(t, err) }, want: true, @@ -197,7 +211,8 @@ func TestBinaries_Installed(t *testing.T) { tc.mockSvc(t, mFs, l) hc := helperConfig{ "docker-credential-binary", "", - "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "mock_prefix/cred-helpers/", + "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "mock_prefix/cred-helpers/", "mock_prefix/.finch/", } // hash of an empty file @@ -208,45 +223,66 @@ func TestBinaries_Installed(t *testing.T) { } } +//nolint:paralleltest // This function manipulates a global variable to facilitate mocking. func TestBinaries_Install(t *testing.T) { - t.Parallel() - testCases := []struct { name string mockSvc func( l *mocks.Logger, cmd *mocks.Command, creator *mocks.CommandCreator, - mFs afero.Fs) - want error + mFs afero.Fs, + ) + want error + postRunCheck func(t *testing.T, mFs afero.Fs) }{ { name: "happy path", mockSvc: func(l *mocks.Logger, cmd *mocks.Command, creator *mocks.CommandCreator, mFs afero.Fs) { + binaryInstalled = func(*credhelperbin) (bool, error) { + return false, nil + } _, err := mFs.Create("mock_prefix/cred-helpers/docker-credential-ecr-login") require.NoError(t, err) - cmd.EXPECT().Output().Times(2) + cmd.EXPECT().Output() l.EXPECT().Infof("Installing %s credential helper", "ecr") - creator.EXPECT().Create("mkdir", "-p", "mock_prefix/cred-helpers/").Return(cmd) creator.EXPECT().Create("curl", "--retry", "5", "--retry-max-time", "30", "--url", "https://amazon-ecr-credential-helper-releases.s3.us-east-2.amazonaws.com"+ "/0.7.0/linux-arm64/docker-credential-ecr-login", "--output", "mock_prefix/cred-helpers/docker-credential-ecr-login").Return(cmd) }, want: nil, + postRunCheck: func(t *testing.T, mFs afero.Fs) { + f, err := afero.ReadFile(mFs, "mock_prefix/.finch/config.json") + require.NoError(t, err) + require.Equal(t, string(f), `{"credsStore":"ecr-login"}`) + }, + }, + { + name: "credential helper already installed, but config file not configured", + mockSvc: func(l *mocks.Logger, cmd *mocks.Command, creator *mocks.CommandCreator, mFs afero.Fs) { + binaryInstalled = func(*credhelperbin) (bool, error) { + return true, nil + } + }, + want: nil, + postRunCheck: func(t *testing.T, mFs afero.Fs) { + f, err := afero.ReadFile(mFs, "mock_prefix/.finch/config.json") + require.NoError(t, err) + require.Equal(t, string(f), `{"credsStore":"ecr-login"}`) + }, }, } for _, tc := range testCases { tc := tc t.Run(tc.name, func(t *testing.T) { - t.Parallel() - ctrl := gomock.NewController(t) cmd := mocks.NewCommand(ctrl) mFs := afero.NewMemMapFs() l := mocks.NewLogger(ctrl) creator := mocks.NewCommandCreator(ctrl) + origBinaryInstalled := binaryInstalled tc.mockSvc(l, cmd, creator, mFs) credHelperURL := "https://amazon-ecr-credential-helper-releases.s3.us-east-2.amazonaws.com" + @@ -254,13 +290,15 @@ func TestBinaries_Install(t *testing.T) { hc := helperConfig{ "docker-credential-ecr-login", credHelperURL, - "sha256:ff14a4da40d28a2d2d81a12a7c9c36294ddf8e6439780c4ccbc96622991f3714", + "sha256:ec5c04babea79b08dffb0c8acb67b9e28dc05be0fe9bd4df2e234d75516061d7", "mock_prefix/cred-helpers/", "mock_prefix/.finch/", } fc := "ecr-login" got := newCredHelperBinary(mockFinchPath, mFs, creator, l, fc, "", hc).Install() + binaryInstalled = origBinaryInstalled assert.Equal(t, tc.want, got) + tc.postRunCheck(t, mFs) }) } }