Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Use LimaUser method instead of host user name #712

Merged
merged 4 commits into from
Dec 2, 2023
Merged

fix: Use LimaUser method instead of host user name #712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
05cdd11
fix: use LimaUser method instead of host user name
w-haibara Nov 30, 2023
3e1451f
fix: use host username and lima username
w-haibara Dec 1, 2023
92225bd
refactor: undo variable name
w-haibara Dec 1, 2023
f286bae
test: add a test in nerdctl_config_applier_test.go when local user is…
w-haibara Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmd/finch/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,22 +90,23 @@ var newApp = func(logger flog.Logger, fp path.Finch, fs afero.Fs, fc *config.Fin
fp.QEMUBinDir(),
system.NewStdLib(),
)
lima := wrapper.NewLimaWrapper()
supportBundleBuilder := support.NewBundleBuilder(
logger,
fs,
support.NewBundleConfig(fp, system.NewStdLib().Env("HOME")),
fp,
ecc,
lcc,
wrapper.NewLimaWrapper(),
lima,
)

// append nerdctl commands
allCommands := initializeNerdctlCommands(lcc, ecc, logger, fs, fc)
// append finch specific commands
allCommands = append(allCommands,
newVersionCommand(lcc, logger, stdOut),
virtualMachineCommands(logger, fp, lcc, ecc, fs, fc),
virtualMachineCommands(logger, fp, lcc, ecc, fs, fc, lima),
newSupportBundleCommand(logger, supportBundleBuilder, lcc),
newGenDocsCommand(rootCmd, logger, fs, system.NewStdLib()),
)
Expand All @@ -122,6 +123,7 @@ func virtualMachineCommands(
ecc *command.ExecCmdCreator,
fs afero.Fs,
fc *config.Finch,
lima wrapper.LimaWrapper,
) *cobra.Command {
optionalDepGroups := []*dependency.Group{
vmnet.NewDependencyGroup(ecc, lcc, fs, fp, logger),
Expand All @@ -133,7 +135,7 @@ func virtualMachineCommands(
logger,
optionalDepGroups,
config.NewLimaApplier(fc, ecc, fs, fp.LimaOverrideConfigPath(), system.NewStdLib()),
config.NewNerdctlApplier(fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath(), system.NewStdLib().Env("USER")),
config.NewNerdctlApplier(fssh.NewDialer(), fs, fp.LimaSSHPrivateKeyPath(), system.NewStdLib().Env("USER"), lima),
fp,
fs,
disk.NewUserDataDiskManager(lcc, ecc, &afero.OsFs{}, fp, system.NewStdLib().Env("HOME"), fc),
Expand Down
89 changes: 48 additions & 41 deletions pkg/config/nerdctl_config_applier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/spf13/afero/sftpfs"

"github.com/runfinch/finch/pkg/fssh"
"github.com/runfinch/finch/pkg/lima/wrapper"
)

const (
Expand All @@ -27,18 +28,20 @@ type nerdctlConfigApplier struct {
fs afero.Fs
privateKeyPath string
hostUser string
lima wrapper.LimaWrapper
}

var _ NerdctlConfigApplier = (*nerdctlConfigApplier)(nil)

// NewNerdctlApplier creates a new NerdctlConfigApplier that
// applies nerdctl configuration changes by SSHing to the lima VM to update the nerdctl configuration file in it.
func NewNerdctlApplier(dialer fssh.Dialer, fs afero.Fs, privateKeyPath, hostUser string) NerdctlConfigApplier {
func NewNerdctlApplier(dialer fssh.Dialer, fs afero.Fs, privateKeyPath string, hostUser string, lima wrapper.LimaWrapper) NerdctlConfigApplier {
return &nerdctlConfigApplier{
dialer: dialer,
fs: fs,
privateKeyPath: privateKeyPath,
hostUser: hostUser,
lima: lima,
}
}

Expand All @@ -53,45 +56,6 @@ func addLineToBashrc(fs afero.Fs, profileFilePath string, profStr string, cmd st
return profStr, nil
}

// updateEnvironment adds variables to the user's shell's environment. Currently it uses ~/.bashrc because
// Bash is the default shell and Bash will not load ~/.profile if ~/.bash_profile exists (which it does).
// ~/.bash_profile sources ~/.bashrc, so ~/.bashrc is currently the best place to define additional variables.
// The [GNU docs for Bash] explain how these files work together in more details.
// The default location of DOCKER_CONFIG is ~/.docker/config.json. This config change sets the location to
// ~/.finch/config.json, but from the perspective of macOS (/Users/<user>/.finch/config.json).
// The reason that we don't set environment variables inside /root/.bashrc is that the vars inside it are
// not able to be picked up even if we specify `sudo -E`. We have to switch to root user in order to access them, while
// normally we would access the VM as the regular user.
// For more information on the variable, see the registry nerdctl docs.
//
// [GNU docs for Bash]: https://www.gnu.org/software/bash/manual/html_node/Bash-Startup-Files.html
//
// [registry nerdctl docs]: https://github.com/containerd/nerdctl/blob/master/docs/registry.md

func updateEnvironment(fs afero.Fs, user string) error {
cmdArr := [4]string{
fmt.Sprintf("export DOCKER_CONFIG=\"/Users/%s/.finch\"", user),
fmt.Sprintf("[ -L /usr/local/bin/docker-credential-ecr-login ] "+
"|| sudo ln -s /Users/%s/.finch/cred-helpers/docker-credential-ecr-login /usr/local/bin/", user),
fmt.Sprintf("[ -L /root/.aws ] || sudo ln -fs /Users/%s/.aws /root/.aws", user),
}

profileFilePath := fmt.Sprintf("/home/%s.linux/.bashrc", user)
profBuf, err := afero.ReadFile(fs, profileFilePath)
if err != nil {
return fmt.Errorf("failed to read config file: %w", err)
}
profStr := string(profBuf)
for _, element := range cmdArr {
profStr, err = addLineToBashrc(fs, profileFilePath, profStr, element)
if err != nil {
return err
}
}

return nil
}

// updateNerdctlConfig reads from the nerdctl config and updates values.
func updateNerdctlConfig(fs afero.Fs, user string, rootless bool) error {
nerdctlRootlessCfgPath := fmt.Sprintf("/home/%s.linux/.config/nerdctl/nerdctl.toml", user)
Expand Down Expand Up @@ -137,6 +101,49 @@ func updateNerdctlConfig(fs afero.Fs, user string, rootless bool) error {
return nil
}

// updateEnvironment adds variables to the user's shell's environment. Currently it uses ~/.bashrc because
// Bash is the default shell and Bash will not load ~/.profile if ~/.bash_profile exists (which it does).
// ~/.bash_profile sources ~/.bashrc, so ~/.bashrc is currently the best place to define additional variables.
// The [GNU docs for Bash] explain how these files work together in more details.
// The default location of DOCKER_CONFIG is ~/.docker/config.json. This config change sets the location to
// ~/.finch/config.json, but from the perspective of macOS (/Users/<user>/.finch/config.json).
// The reason that we don't set environment variables inside /root/.bashrc is that the vars inside it are
// not able to be picked up even if we specify `sudo -E`. We have to switch to root user in order to access them, while
// normally we would access the VM as the regular user.
// For more information on the variable, see the registry nerdctl docs.
//
// [GNU docs for Bash]: https://www.gnu.org/software/bash/manual/html_node/Bash-Startup-Files.html
//
// [registry nerdctl docs]: https://github.com/containerd/nerdctl/blob/master/docs/registry.md
func (nca *nerdctlConfigApplier) updateEnvironment(fs afero.Fs) error {
cmdArr := [4]string{
fmt.Sprintf("export DOCKER_CONFIG=\"/Users/%s/.finch\"", nca.hostUser),
fmt.Sprintf("[ -L /usr/local/bin/docker-credential-ecr-login ] "+
"|| sudo ln -s /Users/%s/.finch/cred-helpers/docker-credential-ecr-login /usr/local/bin/", nca.hostUser),
fmt.Sprintf("[ -L /root/.aws ] || sudo ln -fs /Users/%s/.aws /root/.aws", nca.hostUser),
}

limaUser, err := nca.lima.LimaUser(false)
if err != nil {
return fmt.Errorf("failed to get lima user: %w", err)
}

profileFilePath := fmt.Sprintf("/home/%s.linux/.bashrc", limaUser.Username)
profBuf, err := afero.ReadFile(fs, profileFilePath)
if err != nil {
return fmt.Errorf("failed to read config file: %w", err)
}
profStr := string(profBuf)
for _, element := range cmdArr {
profStr, err = addLineToBashrc(fs, profileFilePath, profStr, element)
if err != nil {
return err
}
}

return nil
}

// Apply gets SSH and SFTP clients and uses them to update the nerdctl config.
func (nca *nerdctlConfigApplier) Apply(remoteAddr string) error {
user := "root"
Expand All @@ -162,7 +169,7 @@ func (nca *nerdctlConfigApplier) Apply(remoteAddr string) error {
return fmt.Errorf("failed to update the nerdctl config file: %w", err)
}

if err := updateEnvironment(sftpFs, nca.hostUser); err != nil {
if err := nca.updateEnvironment(sftpFs); err != nil {
return fmt.Errorf("failed to update the user's .profile file: %w", err)
}
return nil
Expand Down
73 changes: 58 additions & 15 deletions pkg/config/nerdctl_config_applier_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"io/fs"
"os/user"
"testing"

"github.com/golang/mock/gomock"
Expand All @@ -28,21 +29,26 @@ TZ6coT6ILioXcs0kX17JAAAAI2FsdmFqdXNAODg2NjVhMGJmN2NhLmFudC5hbWF6b24uY2
9tAQI=
-----END OPENSSH PRIVATE KEY-----`

func Test_updateEnvironment(t *testing.T) {
func TestNerdctlConfigApplier_updateEnvironment(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
user string
mockSvc func(t *testing.T, fs afero.Fs)
hostUser string
mockSvc func(t *testing.T, fs afero.Fs, lima *mocks.MockLimaWrapper)
postRunCheck func(t *testing.T, fs afero.Fs)
want error
}{
{
name: "happy path",
user: "mock_user",
mockSvc: func(t *testing.T, fs afero.Fs) {
name: "happy path",
hostUser: "mock_user",
mockSvc: func(t *testing.T, fs afero.Fs, lima *mocks.MockLimaWrapper) {
require.NoError(t, afero.WriteFile(fs, "/home/mock_user.linux/.bashrc", []byte(""), 0o644))

mockUser := &user.User{
Username: "mock_user",
}
lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes()
},
postRunCheck: func(t *testing.T, fs afero.Fs) {
fileBytes, err := afero.ReadFile(fs, "/home/mock_user.linux/.bashrc")
Expand All @@ -56,9 +62,9 @@ func Test_updateEnvironment(t *testing.T) {
want: nil,
},
{
name: "happy path, file already exists and already contains expected variables",
user: "mock_user",
mockSvc: func(t *testing.T, fs afero.Fs) {
name: "happy path, file already exists and already contains expected variables",
hostUser: "mock_user",
mockSvc: func(t *testing.T, fs afero.Fs, lima *mocks.MockLimaWrapper) {
require.NoError(
t,
afero.WriteFile(
Expand All @@ -70,6 +76,11 @@ func Test_updateEnvironment(t *testing.T) {
0o644,
),
)

mockUser := &user.User{
Username: "mock_user",
}
lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes()
},
postRunCheck: func(t *testing.T, fs afero.Fs) {
fileBytes, err := afero.ReadFile(fs, "/home/mock_user.linux/.bashrc")
Expand All @@ -82,26 +93,57 @@ func Test_updateEnvironment(t *testing.T) {
want: nil,
},
{
name: ".bashrc file doesn't exist",
user: "mock_user",
mockSvc: func(t *testing.T, fs afero.Fs) {},
name: ".bashrc file doesn't exist",
hostUser: "mock_user",
mockSvc: func(t *testing.T, fs afero.Fs, lima *mocks.MockLimaWrapper) {
mockUser := &user.User{
Username: "mock_user",
}
lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes()
},
postRunCheck: func(t *testing.T, fs afero.Fs) {},
want: fmt.Errorf(
"failed to read config file: %w",
&fs.PathError{Op: "open", Path: "/home/mock_user.linux/.bashrc", Err: errors.New("file does not exist")},
),
},
{
name: "host user is not a valid linux username",
hostUser: "invalid.user",
mockSvc: func(t *testing.T, fs afero.Fs, lima *mocks.MockLimaWrapper) {
require.NoError(t, afero.WriteFile(fs, "/home/lima.linux/.bashrc", []byte(""), 0o644))

mockUser := &user.User{
Username: "lima",
}
lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes()
},
postRunCheck: func(t *testing.T, fs afero.Fs) {
fileBytes, err := afero.ReadFile(fs, "/home/lima.linux/.bashrc")
require.NoError(t, err)
assert.Equal(t,
[]byte("\nexport DOCKER_CONFIG=\"/Users/invalid.user/.finch\""+
"\n[ -L /usr/local/bin/docker-credential-ecr-login ] || sudo ln -s "+
"/Users/invalid.user/.finch/cred-helpers/docker-credential-ecr-login /usr/local/bin/"+
"\n"+"[ -L /root/.aws ] || sudo ln -fs /Users/invalid.user/.aws /root/.aws"), fileBytes)
},
want: nil,
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

ctrl := gomock.NewController(t)
fs := afero.NewMemMapFs()
d := mocks.NewDialer(ctrl)
lima := mocks.NewMockLimaWrapper(ctrl)

tc.mockSvc(t, fs)
got := updateEnvironment(fs, tc.user)
tc.mockSvc(t, fs, lima)
nca, _ := NewNerdctlApplier(d, fs, "/private-key", tc.hostUser, lima).(*nerdctlConfigApplier)
got := nca.updateEnvironment(fs)
require.Equal(t, tc.want, got)

tc.postRunCheck(t, fs)
Expand Down Expand Up @@ -241,9 +283,10 @@ func TestNerdctlConfigApplier_Apply(t *testing.T) {
ctrl := gomock.NewController(t)
fs := afero.NewMemMapFs()
d := mocks.NewDialer(ctrl)
lima := mocks.NewMockLimaWrapper(ctrl)

tc.mockSvc(t, fs, d)
got := NewNerdctlApplier(d, fs, tc.path, tc.hostUser).Apply(tc.remoteAddr)
got := NewNerdctlApplier(d, fs, tc.path, tc.hostUser, lima).Apply(tc.remoteAddr)

assert.Equal(t, tc.want, got)
})
Expand Down
Loading