From 25a179c60ae4f6c6d9c8932d63100c23e6cdd018 Mon Sep 17 00:00:00 2001 From: Sam Berning Date: Thu, 24 Aug 2023 18:19:15 -0700 Subject: [PATCH] feat: supports adding files inside the VM to support bundles Signed-off-by: Sam Berning --- cmd/finch/main.go | 6 +- pkg/command/command.go | 2 + pkg/mocks/command_command.go | 28 ++++++++ pkg/support/support.go | 125 +++++++++++++++++++++++++++-------- pkg/support/support_test.go | 95 ++++++++++++++++++++++++-- 5 files changed, 220 insertions(+), 36 deletions(-) diff --git a/cmd/finch/main.go b/cmd/finch/main.go index dfef4cbd9..388243c9f 100644 --- a/cmd/finch/main.go +++ b/cmd/finch/main.go @@ -9,9 +9,6 @@ import ( "io" "os" - "github.com/spf13/afero" - "github.com/spf13/cobra" - "github.com/runfinch/finch/pkg/command" "github.com/runfinch/finch/pkg/config" "github.com/runfinch/finch/pkg/dependency" @@ -26,6 +23,8 @@ import ( "github.com/runfinch/finch/pkg/support" "github.com/runfinch/finch/pkg/system" "github.com/runfinch/finch/pkg/version" + "github.com/spf13/afero" + "github.com/spf13/cobra" ) const finchRootCmd = "finch" @@ -96,6 +95,7 @@ var newApp = func(logger flog.Logger, fp path.Finch, fs afero.Fs, fc *config.Fin support.NewBundleConfig(fp, system.NewStdLib().Env("HOME")), fp, ecc, + lcc, wrapper.NewLimaWrapper(), ) diff --git a/pkg/command/command.go b/pkg/command/command.go index d726d585c..478aeedd4 100644 --- a/pkg/command/command.go +++ b/pkg/command/command.go @@ -26,6 +26,8 @@ type Command interface { SetStderr(io.Writer) Run() error + Start() error + Wait() error Output() ([]byte, error) CombinedOutput() ([]byte, error) } diff --git a/pkg/mocks/command_command.go b/pkg/mocks/command_command.go index e1d621e7b..c9006d599 100644 --- a/pkg/mocks/command_command.go +++ b/pkg/mocks/command_command.go @@ -128,3 +128,31 @@ func (mr *CommandMockRecorder) SetStdout(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetStdout", reflect.TypeOf((*Command)(nil).SetStdout), arg0) } + +// Start mocks base method. +func (m *Command) Start() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Start") + ret0, _ := ret[0].(error) + return ret0 +} + +// Start indicates an expected call of Start. +func (mr *CommandMockRecorder) Start() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Start", reflect.TypeOf((*Command)(nil).Start)) +} + +// Wait mocks base method. +func (m *Command) Wait() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Wait") + ret0, _ := ret[0].(error) + return ret0 +} + +// Wait indicates an expected call of Wait. +func (mr *CommandMockRecorder) Wait() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Wait", reflect.TypeOf((*Command)(nil).Wait)) +} diff --git a/pkg/support/support.go b/pkg/support/support.go index 4d5b54022..3b479e9bb 100644 --- a/pkg/support/support.go +++ b/pkg/support/support.go @@ -6,6 +6,7 @@ package support import ( "archive/zip" + "bufio" "bytes" "errors" "fmt" @@ -15,14 +16,13 @@ import ( "strings" "time" - "github.com/spf13/afero" - "gopkg.in/yaml.v3" - "github.com/runfinch/finch/pkg/command" "github.com/runfinch/finch/pkg/flog" "github.com/runfinch/finch/pkg/lima/wrapper" fpath "github.com/runfinch/finch/pkg/path" "github.com/runfinch/finch/pkg/version" + "github.com/spf13/afero" + "gopkg.in/yaml.v3" ) const ( @@ -51,6 +51,7 @@ type bundleBuilder struct { config BundleConfig finch fpath.Finch ecc command.Creator + lcc command.LimaCmdCreator lima wrapper.LimaWrapper } @@ -61,6 +62,7 @@ func NewBundleBuilder( config BundleConfig, finch fpath.Finch, ecc command.Creator, + lcc command.LimaCmdCreator, lima wrapper.LimaWrapper, ) BundleBuilder { return &bundleBuilder{ @@ -69,6 +71,7 @@ func NewBundleBuilder( config: config, finch: finch, ecc: ecc, + lcc: lcc, lima: lima, } } @@ -108,7 +111,13 @@ func (bb *bundleBuilder) GenerateSupportBundle(additionalFiles []string, exclude bb.logger.Infof("Excluding %s...", file) continue } - err := bb.copyInFile(writer, file, path.Join(zipPrefix, logPrefix)) + bb.logger.Debugf("Copying %s...", file) + var err error + if isFileFromVM(file) { + err = bb.streamFileFromVM(writer, file, path.Join(zipPrefix, logPrefix)) + } else { + err = bb.copyInFile(writer, file, path.Join(zipPrefix, logPrefix)) + } if err != nil { bb.logger.Warnf("Could not copy in %q. Error: %s", file, err) } @@ -120,7 +129,13 @@ func (bb *bundleBuilder) GenerateSupportBundle(additionalFiles []string, exclude bb.logger.Infof("Excluding %s...", file) continue } - err := bb.copyInFile(writer, file, path.Join(zipPrefix, configPrefix)) + bb.logger.Debugf("Copying %s...", file) + var err error + if isFileFromVM(file) { + err = bb.streamFileFromVM(writer, file, path.Join(zipPrefix, configPrefix)) + } else { + err = bb.copyInFile(writer, file, path.Join(zipPrefix, configPrefix)) + } if err != nil { bb.logger.Warnf("Could not copy in %q. Error: %s", file, err) } @@ -132,7 +147,13 @@ func (bb *bundleBuilder) GenerateSupportBundle(additionalFiles []string, exclude bb.logger.Infof("Excluding %s...", file) continue } - err := bb.copyInFile(writer, file, path.Join(zipPrefix, additionalPrefix)) + bb.logger.Debugf("Copying %s...", file) + var err error + if isFileFromVM(file) { + err = bb.streamFileFromVM(writer, file, path.Join(zipPrefix, additionalPrefix)) + } else { + err = bb.copyInFile(writer, file, path.Join(zipPrefix, additionalPrefix)) + } if err != nil { bb.logger.Warnf("Could not add additional file %s. Error: %s", file, err) } @@ -146,30 +167,21 @@ func (bb *bundleBuilder) GenerateSupportBundle(additionalFiles []string, exclude return zipFileName, nil } -func (bb *bundleBuilder) copyInFile(writer *zip.Writer, fileName string, prefix string) error { - f, err := bb.fs.Open(fileName) - if err != nil { - return err - } - - bb.logger.Debugf("Copying %s...", fileName) - - var buf bytes.Buffer - _, err = buf.ReadFrom(f) - if err != nil { - return err - } +type bufReader interface { + ReadBytes(delim byte) ([]byte, error) +} - var redacted []byte +func (bb *bundleBuilder) copyAndRedactFile(writer io.Writer, reader bufReader) error { var bufErr error for bufErr == nil { var line []byte - line, bufErr = buf.ReadBytes('\n') + line, bufErr = reader.ReadBytes('\n') if bufErr != nil && !errors.Is(bufErr, io.EOF) { + bb.logger.Error(bufErr.Error()) continue } - line, err = redactFinchInstall(line, bb.finch) + line, err := redactFinchInstall(line, bb.finch) if err != nil { return err } @@ -187,7 +199,20 @@ func (bb *bundleBuilder) copyInFile(writer *zip.Writer, fileName string, prefix line = redactPorts(line) line = redactSSHKeys(line) - redacted = append(redacted, line...) + _, err = writer.Write(line) + if err != nil { + return err + } + } + + return nil +} + +func (bb *bundleBuilder) copyInFile(writer *zip.Writer, fileName string, prefix string) error { + // check filename validity? + f, err := bb.fs.Open(fileName) + if err != nil { + return err } baseName := path.Base(fileName) @@ -196,12 +221,50 @@ func (bb *bundleBuilder) copyInFile(writer *zip.Writer, fileName string, prefix return err } - _, err = zipCopy.Write(redacted) + buf := new(bytes.Buffer) + _, err = buf.ReadFrom(f) if err != nil { return err } - return nil + return bb.copyAndRedactFile(zipCopy, buf) +} + +func (bb *bundleBuilder) streamFileFromVM(writer *zip.Writer, filename, prefix string) error { + pipeReader, pipeWriter := io.Pipe() + errBuf := new(bytes.Buffer) + + _, filePathInVM, _ := strings.Cut(filename, ":") + cmd := bb.lcc.CreateWithoutStdio("shell", "finch", "sudo", "cat", filePathInVM) + cmd.SetStdout(pipeWriter) + cmd.SetStderr(errBuf) + + err := cmd.Start() + if err != nil { + return err + } + + waitStatus := make(chan error) + go func() { + err := cmd.Wait() + pipeWriter.Close() + waitStatus <- err + }() + + baseName := path.Base(filename) + zipCopy, err := writer.Create(path.Join(prefix, baseName)) + if err != nil { + return err + } + + bufReader := bufio.NewReader(pipeReader) + + err = bb.copyAndRedactFile(zipCopy, bufReader) + if err != nil { + return err + } + + return <-waitStatus } func (bb *bundleBuilder) getPlatformData() (*PlatformData, error) { @@ -280,7 +343,11 @@ func bundleFileName() string { } func fileShouldBeExcluded(filename string, exclude []string) bool { - fileAbs, err := filepath.Abs(filename) + realFilename := filename + if isFileFromVM(filename) { + _, realFilename, _ = strings.Cut(filename, ":") + } + fileAbs, err := filepath.Abs(realFilename) if err != nil { return true } @@ -292,9 +359,13 @@ func fileShouldBeExcluded(filename string, exclude []string) bool { if fileAbs == excludeAbs { return true } - if path.Base(filename) == excludeFile { + if path.Base(realFilename) == excludeFile { return true } } return false } + +func isFileFromVM(filename string) bool { + return strings.HasPrefix(filename, "vm:") +} diff --git a/pkg/support/support_test.go b/pkg/support/support_test.go index 33c16c30e..c3d1035dc 100644 --- a/pkg/support/support_test.go +++ b/pkg/support/support_test.go @@ -5,17 +5,17 @@ package support import ( "archive/zip" + "io" "os/user" "testing" "time" "github.com/golang/mock/gomock" + "github.com/runfinch/finch/pkg/mocks" + fpath "github.com/runfinch/finch/pkg/path" "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - - "github.com/runfinch/finch/pkg/mocks" - fpath "github.com/runfinch/finch/pkg/path" ) func TestSupport_NewBundleBuilder(t *testing.T) { @@ -23,13 +23,14 @@ func TestSupport_NewBundleBuilder(t *testing.T) { ctrl := gomock.NewController(t) ecc := mocks.NewCommandCreator(ctrl) + lcc := mocks.NewLimaCmdCreator(ctrl) logger := mocks.NewLogger(ctrl) fs := afero.NewMemMapFs() finch := fpath.Finch("mockfinch") lima := mocks.NewMockLimaWrapper(ctrl) config := NewBundleConfig(finch, "mockhome") - NewBundleBuilder(logger, fs, config, finch, ecc, lima) + NewBundleBuilder(logger, fs, config, finch, ecc, lcc, lima) } func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { @@ -41,7 +42,7 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { testCases := []struct { name string - mockSvc func(*mocks.Logger, *mocks.BundleConfig, *mocks.CommandCreator, *mocks.Command, *mocks.MockLimaWrapper) + mockSvc func(*mocks.Logger, *mocks.BundleConfig, *mocks.CommandCreator, *mocks.LimaCmdCreator, *mocks.Command, *mocks.MockLimaWrapper, afero.Fs) include []string exclude []string }{ @@ -51,8 +52,10 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, + lcc *mocks.LimaCmdCreator, cmd *mocks.Command, lima *mocks.MockLimaWrapper, + fs afero.Fs, ) { logger.EXPECT().Debugf("Creating %s...", gomock.Any()) logger.EXPECT().Debugln("Gathering platform data...") @@ -91,8 +94,10 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, + lcc *mocks.LimaCmdCreator, cmd *mocks.Command, lima *mocks.MockLimaWrapper, + fs afero.Fs, ) { logger.EXPECT().Debugf("Creating %s...", gomock.Any()) logger.EXPECT().Debugln("Gathering platform data...") @@ -128,8 +133,10 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, + lcc *mocks.LimaCmdCreator, cmd *mocks.Command, lima *mocks.MockLimaWrapper, + fs afero.Fs, ) { logger.EXPECT().Debugf("Creating %s...", gomock.Any()) logger.EXPECT().Debugln("Gathering platform data...") @@ -164,8 +171,10 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, + lcc *mocks.LimaCmdCreator, cmd *mocks.Command, lima *mocks.MockLimaWrapper, + fs afero.Fs, ) { logger.EXPECT().Debugf("Creating %s...", gomock.Any()) logger.EXPECT().Debugln("Gathering platform data...") @@ -200,8 +209,10 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { logger *mocks.Logger, config *mocks.BundleConfig, ecc *mocks.CommandCreator, + lcc *mocks.LimaCmdCreator, cmd *mocks.Command, lima *mocks.MockLimaWrapper, + fs afero.Fs, ) { logger.EXPECT().Debugf("Creating %s...", gomock.Any()) logger.EXPECT().Debugln("Gathering platform data...") @@ -231,6 +242,64 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { include: []string{"extra1"}, exclude: []string{"extra1"}, }, + { + name: "Generate support bundle with a VM file included", + mockSvc: func( + logger *mocks.Logger, + config *mocks.BundleConfig, + ecc *mocks.CommandCreator, + lcc *mocks.LimaCmdCreator, + cmd *mocks.Command, + lima *mocks.MockLimaWrapper, + fs afero.Fs, + ) { + logger.EXPECT().Debugf("Creating %s...", gomock.Any()) + logger.EXPECT().Debugln("Gathering platform data...") + + ecc.EXPECT().Create("sw_vers", "-productVersion").Return(cmd) + cmd.EXPECT().Output().Return([]byte("1.2.3\n"), nil) + ecc.EXPECT().Create("uname", "-m").Return(cmd) + cmd.EXPECT().Output().Return([]byte("arch\n"), nil) + + config.EXPECT().LogFiles().Return([]string{ + "log1", + }) + + config.EXPECT().ConfigFiles().Return([]string{ + "config1", + }) + + logger.EXPECT().Debugln("Copying in log files...") + logger.EXPECT().Debugf("Copying %s...", "log1") + logger.EXPECT().Debugln("Copying in config files...") + logger.EXPECT().Debugf("Copying %s...", "config1") + logger.EXPECT().Debugln("Copying in additional files...") + logger.EXPECT().Debugf("Copying %s...", "vm:extra1") + + var catWriter io.Writer + waitChan := make(chan int) + lcc.EXPECT().CreateWithoutStdio("shell", "finch", "sudo", "cat", "extra1").Return(cmd) + cmd.EXPECT().SetStdout(gomock.Any()).Do(func(writer io.Writer) { + catWriter = writer + }) + cmd.EXPECT().SetStderr(gomock.Any()) + cmd.EXPECT().Start().DoAndReturn(func() error { + go func() { + _, err := catWriter.Write([]byte("file contents\n")) + require.NoError(t, err) + waitChan <- 1 + }() + return nil + }) + cmd.EXPECT().Wait().DoAndReturn(func() error { + <-waitChan + return nil + }) + + lima.EXPECT().LimaUser(false).Return(mockUser, nil).AnyTimes() + }, + include: []string{"vm:extra1"}, + }, } for _, tc := range testCases { @@ -244,6 +313,7 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { config := mocks.NewBundleConfig(ctrl) finch := fpath.Finch("mockfinch") ecc := mocks.NewCommandCreator(ctrl) + lcc := mocks.NewLimaCmdCreator(ctrl) lima := mocks.NewMockLimaWrapper(ctrl) cmd := mocks.NewCommand(ctrl) @@ -253,6 +323,7 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { config: config, finch: finch, ecc: ecc, + lcc: lcc, lima: lima, } @@ -273,7 +344,7 @@ func TestSupportBundleBuilder_GenerateSupportBundle(t *testing.T) { require.NoError(t, err) } - tc.mockSvc(logger, config, ecc, cmd, lima) + tc.mockSvc(logger, config, ecc, lcc, cmd, lima, fs) zipFile, err := builder.GenerateSupportBundle(tc.include, tc.exclude) assert.NoError(t, err) @@ -377,6 +448,18 @@ func TestSupport_fileShouldBeExcluded(t *testing.T) { exclude: []string{"other.file"}, result: false, }, + { + name: "vm file with its whole path excluded", + file: "vm:/path/to/file", + exclude: []string{"/path/to/file"}, + result: true, + }, + { + name: "vm file with its base path excluded", + file: "vm:/path/to/file", + exclude: []string{"file"}, + result: true, + }, } for _, tc := range testCases {