diff --git a/.github/workflows/smoketest.yml b/.github/workflows/smoketest.yml index d5493e7..3f8eb6c 100644 --- a/.github/workflows/smoketest.yml +++ b/.github/workflows/smoketest.yml @@ -69,6 +69,15 @@ jobs: CI_USER: ${{ vars.JETKVM_CI_USER }} CI_HOST: ${{ vars.JETKVM_CI_HOST }} CI_SSH_PRIVATE: ${{ secrets.JETKVM_CI_SSH_PRIVATE }} + - name: Run tests + run: | + set -e + make build_dev_test + + echo "+ Copying device-tests.tar.gz to remote host" + ssh jkci "cat > /userdata/jetkvm/device-tests.tar.gz" < device-tests.tar.gz + echo "+ Running go tests" + ssh jkci "cd /userdata/jetkvm && tar zxvf device-tests.tar.gz && ./run_all_tests -json" - name: Deploy application run: | set -e diff --git a/dev_deploy.sh b/dev_deploy.sh index 267e2c7..7bd649b 100755 --- a/dev_deploy.sh +++ b/dev_deploy.sh @@ -26,6 +26,8 @@ show_help() { echo "Optional:" echo " -u, --user Remote username (default: root)" echo " --run-go-tests Run go tests" + echo " --run-go-tests-only Run go tests and exit" + echo " --run-go-tests-json Run go tests and output JSON" echo " --skip-ui-build Skip frontend/UI build" echo " --help Display this help message" echo @@ -42,6 +44,8 @@ RESET_USB_HID_DEVICE=false LOG_TRACE_SCOPES="${LOG_TRACE_SCOPES:-jetkvm,cloud,websocket,native,jsonrpc}" RUN_GO_TESTS=false RUN_GO_TESTS_JSON=false +RUN_GO_TESTS_ONLY=false + # Parse command line arguments while [[ $# -gt 0 ]]; do case $1 in @@ -67,6 +71,12 @@ while [[ $# -gt 0 ]]; do ;; --run-go-tests-json) RUN_GO_TESTS_JSON=true + RUN_GO_TESTS=true + shift + ;; + --run-go-tests-only) + RUN_GO_TESTS_ONLY=true + RUN_GO_TESTS=true shift ;; --help) @@ -81,10 +91,6 @@ while [[ $# -gt 0 ]]; do esac done -if [ "$RUN_GO_TESTS_JSON" = true ]; then - RUN_GO_TESTS=true -fi - # Verify required parameters if [ -z "$REMOTE_HOST" ]; then msg_err "Error: Remote IP is a required parameter" @@ -114,8 +120,13 @@ if [ "$RUN_GO_TESTS" = true ]; then set -e cd ${REMOTE_PATH} tar zxvf device-tests.tar.gz -./run_all_tests $TEST_ARGS +PION_LOG_TRACE=all ./run_all_tests $TEST_ARGS EOF + + if [ "$RUN_GO_TESTS_ONLY" = true ]; then + msg_info "▶ Go tests completed" + exit 0 + fi fi msg_info "▶ Building go binary" diff --git a/internal/usbgadget/changeset.go b/internal/usbgadget/changeset.go index 4465160..3a5ceaa 100644 --- a/internal/usbgadget/changeset.go +++ b/internal/usbgadget/changeset.go @@ -35,20 +35,23 @@ const ( FileStateMounted FileStateMountedConfigFS FileStateSymlink + FileStateSymlinkInOrderConfigFS // configfs is a shithole, so we need to check if the symlinks are created in the correct order + FileStateSymlinkNotInOrderConfigFS FileStateTouch ) var FileStateString = map[FileState]string{ - FileStateUnknown: "UNKNOWN", - FileStateAbsent: "ABSENT", - FileStateDirectory: "DIRECTORY", - FileStateFile: "FILE", - FileStateFileContentMatch: "FILE_CONTENT_MATCH", - FileStateFileWrite: "FILE_WRITE", - FileStateMounted: "MOUNTED", - FileStateMountedConfigFS: "CONFIGFS_MOUNT", - FileStateSymlink: "SYMLINK", - FileStateTouch: "TOUCH", + FileStateUnknown: "UNKNOWN", + FileStateAbsent: "ABSENT", + FileStateDirectory: "DIRECTORY", + FileStateFile: "FILE", + FileStateFileContentMatch: "FILE_CONTENT_MATCH", + FileStateFileWrite: "FILE_WRITE", + FileStateMounted: "MOUNTED", + FileStateMountedConfigFS: "CONFIGFS_MOUNT", + FileStateSymlink: "SYMLINK", + FileStateSymlinkInOrderConfigFS: "SYMLINK_IN_ORDER_CONFIGFS", + FileStateTouch: "TOUCH", } const ( @@ -69,6 +72,8 @@ const ( FileChangeResolvedActionAppendFile FileChangeResolvedActionCreateSymlink FileChangeResolvedActionRecreateSymlink + FileChangeResolvedActionCreateDirectoryAndSymlinks + FileChangeResolvedActionReorderSymlinks FileChangeResolvedActionCreateDirectory FileChangeResolvedActionRemoveDirectory FileChangeResolvedActionTouch @@ -76,19 +81,21 @@ const ( ) var FileChangeResolvedActionString = map[FileChangeResolvedAction]string{ - FileChangeResolvedActionUnknown: "UNKNOWN", - FileChangeResolvedActionDoNothing: "DO_NOTHING", - FileChangeResolvedActionRemove: "REMOVE", - FileChangeResolvedActionCreateFile: "FILE_CREATE", - FileChangeResolvedActionWriteFile: "FILE_WRITE", - FileChangeResolvedActionUpdateFile: "FILE_UPDATE", - FileChangeResolvedActionAppendFile: "FILE_APPEND", - FileChangeResolvedActionCreateSymlink: "SYMLINK_CREATE", - FileChangeResolvedActionRecreateSymlink: "SYMLINK_RECREATE", - FileChangeResolvedActionCreateDirectory: "DIR_CREATE", - FileChangeResolvedActionRemoveDirectory: "DIR_REMOVE", - FileChangeResolvedActionTouch: "TOUCH", - FileChangeResolvedActionMountConfigFS: "CONFIGFS_MOUNT", + FileChangeResolvedActionUnknown: "UNKNOWN", + FileChangeResolvedActionDoNothing: "DO_NOTHING", + FileChangeResolvedActionRemove: "REMOVE", + FileChangeResolvedActionCreateFile: "FILE_CREATE", + FileChangeResolvedActionWriteFile: "FILE_WRITE", + FileChangeResolvedActionUpdateFile: "FILE_UPDATE", + FileChangeResolvedActionAppendFile: "FILE_APPEND", + FileChangeResolvedActionCreateSymlink: "SYMLINK_CREATE", + FileChangeResolvedActionRecreateSymlink: "SYMLINK_RECREATE", + FileChangeResolvedActionCreateDirectoryAndSymlinks: "DIR_CREATE_AND_SYMLINKS", + FileChangeResolvedActionReorderSymlinks: "SYMLINK_REORDER", + FileChangeResolvedActionCreateDirectory: "DIR_CREATE", + FileChangeResolvedActionRemoveDirectory: "DIR_REMOVE", + FileChangeResolvedActionTouch: "TOUCH", + FileChangeResolvedActionMountConfigFS: "CONFIGFS_MOUNT", } type ChangeSet struct { @@ -99,6 +106,7 @@ type RequestedFileChange struct { Component string Key string Path string // will be used as Key if Key is empty + ParamSymlinks []symlink ExpectedState FileState ExpectedContent []byte DependsOn []string @@ -127,6 +135,10 @@ func (f *RequestedFileChange) String() string { s = fmt.Sprintf("file: %s", f.Path) case FileStateSymlink: s = fmt.Sprintf("symlink: %s -> %s", f.Path, f.ExpectedContent) + case FileStateSymlinkInOrderConfigFS: + s = fmt.Sprintf("symlink_in_order_configfs: %s -> %s", f.Path, f.ExpectedContent) + case FileStateSymlinkNotInOrderConfigFS: + s = fmt.Sprintf("symlink_not_in_order_configfs: %s -> %s", f.Path, f.ExpectedContent) case FileStateAbsent: s = fmt.Sprintf("absent: %s", f.Path) case FileStateFileContentMatch: @@ -217,12 +229,20 @@ func (fc *FileChange) getActualState() error { if fi.IsDir() { fc.ActualState = FileStateDirectory - if fc.ExpectedState == FileStateMountedConfigFS { + switch fc.ExpectedState { + case FileStateMountedConfigFS: err := fc.checkIfDirIsMountPoint() if err != nil { l.Warn().Err(err).Msg("failed to check if dir is mount point") return err } + case FileStateSymlinkInOrderConfigFS: + state, err := checkIfSymlinksInOrder(fc, &l) + if err != nil { + l.Warn().Err(err).Msg("failed to check if symlinks are in order") + return err + } + fc.ActualState = state } return nil } @@ -323,6 +343,12 @@ func (fc *FileChange) getFileChangeResolvedAction() FileChangeResolvedAction { return FileChangeResolvedActionRecreateSymlink } return FileChangeResolvedActionCreateSymlink + case FileStateSymlinkInOrderConfigFS: + // if the file is already a symlink, check if the target is the same + if fc.ActualState == FileStateSymlinkInOrderConfigFS { + return FileChangeResolvedActionDoNothing + } + return FileChangeResolvedActionReorderSymlinks case FileStateAbsent: if fc.ActualState == FileStateAbsent { return FileChangeResolvedActionDoNothing @@ -361,6 +387,7 @@ func (c *ChangeSet) ApplyChanges() error { r := ChangeSetResolver{ changeset: c, g: &dag.AcyclicGraph{}, + l: defaultLogger, } return r.Apply() @@ -381,6 +408,8 @@ func (c *ChangeSet) applyChange(change *FileChange) error { return fmt.Errorf("failed to remove symlink: %w", err) } return os.Symlink(string(change.ExpectedContent), change.Path) + case FileChangeResolvedActionReorderSymlinks: + return recreateSymlinks(change, nil) case FileChangeResolvedActionCreateDirectory: return os.MkdirAll(change.Path, 0755) case FileChangeResolvedActionRemove: diff --git a/internal/usbgadget/changeset_arm_test.go b/internal/usbgadget/changeset_arm_test.go index c71c9f6..8c0abd5 100644 --- a/internal/usbgadget/changeset_arm_test.go +++ b/internal/usbgadget/changeset_arm_test.go @@ -3,6 +3,8 @@ package usbgadget import ( + "os" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -27,6 +29,50 @@ var ( usbGadget *UsbGadget ) +var oldAbsoluteMouseCombinedReportDesc = []byte{ + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x02, // Usage (Mouse) + 0xA1, 0x01, // Collection (Application) + + // Report ID 1: Absolute Mouse Movement + 0x85, 0x01, // Report ID (1) + 0x09, 0x01, // Usage (Pointer) + 0xA1, 0x00, // Collection (Physical) + 0x05, 0x09, // Usage Page (Button) + 0x19, 0x01, // Usage Minimum (0x01) + 0x29, 0x03, // Usage Maximum (0x03) + 0x15, 0x00, // Logical Minimum (0) + 0x25, 0x01, // Logical Maximum (1) + 0x75, 0x01, // Report Size (1) + 0x95, 0x03, // Report Count (3) + 0x81, 0x02, // Input (Data, Var, Abs) + 0x95, 0x01, // Report Count (1) + 0x75, 0x05, // Report Size (5) + 0x81, 0x03, // Input (Cnst, Var, Abs) + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x30, // Usage (X) + 0x09, 0x31, // Usage (Y) + 0x16, 0x00, 0x00, // Logical Minimum (0) + 0x26, 0xFF, 0x7F, // Logical Maximum (32767) + 0x36, 0x00, 0x00, // Physical Minimum (0) + 0x46, 0xFF, 0x7F, // Physical Maximum (32767) + 0x75, 0x10, // Report Size (16) + 0x95, 0x02, // Report Count (2) + 0x81, 0x02, // Input (Data, Var, Abs) + 0xC0, // End Collection + + // Report ID 2: Relative Wheel Movement + 0x85, 0x02, // Report ID (2) + 0x09, 0x38, // Usage (Wheel) + 0x15, 0x81, // Logical Minimum (-127) + 0x25, 0x7F, // Logical Maximum (127) + 0x75, 0x08, // Report Size (8) + 0x95, 0x01, // Report Count (1) + 0x81, 0x06, // Input (Data, Var, Rel) + + 0xC0, // End Collection +} + func TestUsbGadgetInit(t *testing.T) { assert := assert.New(t) usbGadget = NewUsbGadget(usbGadgetName, usbDevices, usbConfig, nil) @@ -39,3 +85,31 @@ func TestUsbGadgetStrictModeInitFail(t *testing.T) { u := NewUsbGadget("test", usbDevices, usbConfig, nil) assert.Nil(t, u, "should be nil") } + +func TestUsbGadgetUDCNotBoundAfterReportDescrChanged(t *testing.T) { + assert := assert.New(t) + usbGadget = NewUsbGadget(usbGadgetName, usbDevices, usbConfig, nil) + assert.NotNil(usbGadget) + + // release the usb gadget and create a new one + usbGadget = nil + + altGadgetConfig := defaultGadgetConfig + + oldAbsoluteMouseConfig := altGadgetConfig["absolute_mouse"] + oldAbsoluteMouseConfig.reportDesc = oldAbsoluteMouseCombinedReportDesc + altGadgetConfig["absolute_mouse"] = oldAbsoluteMouseConfig + + usbGadget = newUsbGadget(usbGadgetName, altGadgetConfig, usbDevices, usbConfig, nil) + assert.NotNil(usbGadget) + + udcs := getUdcs() + assert.Equal(1, len(udcs), "should be only one UDC") + // check if the UDC is bound + udc := udcs[0] + assert.NotNil(udc, "UDC should exist") + + udcStr, err := os.ReadFile("/sys/kernel/config/usb_gadget/jetkvm/UDC") + assert.Nil(err, "usb_gadget/UDC should exist") + assert.Equal(strings.TrimSpace(udc), strings.TrimSpace(string(udcStr)), "UDC should be the same") +} diff --git a/internal/usbgadget/changeset_resolver.go b/internal/usbgadget/changeset_resolver.go index a4bc546..9369daf 100644 --- a/internal/usbgadget/changeset_resolver.go +++ b/internal/usbgadget/changeset_resolver.go @@ -3,12 +3,14 @@ package usbgadget import ( "fmt" + "github.com/rs/zerolog" "github.com/sourcegraph/tf-dag/dag" ) type ChangeSetResolver struct { changeset *ChangeSet + l *zerolog.Logger g *dag.AcyclicGraph changesMap map[string]*FileChange @@ -95,6 +97,10 @@ func (c *ChangeSetResolver) resolveChanges(initial bool) error { return err } + for _, change := range c.resolvedChanges { + c.l.Trace().Str("change", change.String()).Msg("resolved change") + } + if !c.additionalResolveRequired || !initial { return nil } @@ -108,9 +114,9 @@ func (c *ChangeSetResolver) applyChanges() error { action := change.Action() actionStr := FileChangeResolvedActionString[action] - l := defaultLogger.Info() + l := c.l.Info() if action == FileChangeResolvedActionDoNothing { - l = defaultLogger.Trace() + l = c.l.Trace() } l.Str("action", actionStr).Str("change", change.String()).Msg("applying change") diff --git a/internal/usbgadget/changeset_symlink.go b/internal/usbgadget/changeset_symlink.go new file mode 100644 index 0000000..16ffb77 --- /dev/null +++ b/internal/usbgadget/changeset_symlink.go @@ -0,0 +1,136 @@ +package usbgadget + +import ( + "fmt" + "os" + "path" + "path/filepath" + "reflect" + + "github.com/rs/zerolog" +) + +type symlink struct { + Path string + Target string +} + +func compareSymlinks(expected []symlink, actual []symlink) bool { + if len(expected) != len(actual) { + return false + } + + return reflect.DeepEqual(expected, actual) +} + +func checkIfSymlinksInOrder(fc *FileChange, logger *zerolog.Logger) (FileState, error) { + if logger == nil { + logger = defaultLogger + } + l := logger.With().Str("path", fc.Path).Logger() + + if fc.ParamSymlinks == nil || len(fc.ParamSymlinks) == 0 { + return FileStateUnknown, fmt.Errorf("no symlinks to check") + } + + fi, err := os.Lstat(fc.Path) + + if err != nil { + if os.IsNotExist(err) { + return FileStateAbsent, nil + } else { + l.Warn().Err(err).Msg("failed to stat file") + return FileStateUnknown, fmt.Errorf("failed to stat file") + } + } + + if !fi.IsDir() { + return FileStateUnknown, fmt.Errorf("file is not a directory") + } + + files, err := os.ReadDir(fc.Path) + symlinks := make([]symlink, 0) + if err != nil { + return FileStateUnknown, fmt.Errorf("failed to read directory") + } + + for _, file := range files { + if file.Type()&os.ModeSymlink != os.ModeSymlink { + continue + } + + path := filepath.Join(fc.Path, file.Name()) + target, err := os.Readlink(path) + if err != nil { + return FileStateUnknown, fmt.Errorf("failed to read symlink") + } + + if !filepath.IsAbs(target) { + target = filepath.Join(fc.Path, target) + newTarget, err := filepath.Abs(target) + if err != nil { + return FileStateUnknown, fmt.Errorf("failed to get absolute path") + } + target = newTarget + } + + symlinks = append(symlinks, symlink{ + Path: path, + Target: target, + }) + } + + // compare the symlinks with the expected symlinks + if compareSymlinks(fc.ParamSymlinks, symlinks) { + return FileStateSymlinkInOrderConfigFS, nil + } + + l.Trace().Interface("expected", fc.ParamSymlinks).Interface("actual", symlinks).Msg("symlinks are not in order") + + return FileStateSymlinkNotInOrderConfigFS, nil +} + +func recreateSymlinks(fc *FileChange, logger *zerolog.Logger) error { + if logger == nil { + logger = defaultLogger + } + // remove all symlinks + files, err := os.ReadDir(fc.Path) + if err != nil { + return fmt.Errorf("failed to read directory") + } + + l := logger.With().Str("path", fc.Path).Logger() + l.Info().Msg("recreate symlinks") + + for _, file := range files { + if file.Type()&os.ModeSymlink != os.ModeSymlink { + continue + } + l.Info().Str("name", file.Name()).Msg("remove symlink") + err := os.Remove(path.Join(fc.Path, file.Name())) + if err != nil { + return fmt.Errorf("failed to remove symlink") + } + } + + l.Info().Interface("param-symlinks", fc.ParamSymlinks).Msg("create symlinks") + + // create the symlinks + for _, symlink := range fc.ParamSymlinks { + l.Info().Str("name", symlink.Path).Str("target", symlink.Target).Msg("create symlink") + + path := symlink.Path + if !filepath.IsAbs(path) { + path = filepath.Join(fc.Path, path) + } + + err := os.Symlink(symlink.Target, path) + if err != nil { + l.Warn().Err(err).Msg("failed to create symlink") + return fmt.Errorf("failed to create symlink") + } + } + + return nil +} diff --git a/internal/usbgadget/config_tx.go b/internal/usbgadget/config_tx.go index b4f1be0..be72487 100644 --- a/internal/usbgadget/config_tx.go +++ b/internal/usbgadget/config_tx.go @@ -22,6 +22,8 @@ type UsbGadgetTransaction struct { configC1Path string orderedConfigItems orderedGadgetConfigItems isGadgetConfigItemEnabled func(key string) bool + + reorderSymlinkChanges *RequestedFileChange } func (u *UsbGadget) newUsbGadgetTransaction(lock bool) error { @@ -96,6 +98,8 @@ func (tx *UsbGadgetTransaction) removeFile(component string, path string, descri } func (tx *UsbGadgetTransaction) Commit() error { + tx.addFileChange("gadget-finalize", *tx.reorderSymlinkChanges) + err := tx.c.Apply() if err != nil { tx.log.Error().Err(err).Msg("failed to update usbgadget configuration") @@ -151,6 +155,21 @@ func (tx *UsbGadgetTransaction) WriteGadgetConfig() { tx.WriteUDC() } +func (tx *UsbGadgetTransaction) getDisableKeys() []string { + disableKeys := make([]string, 0) + for _, item := range tx.orderedConfigItems { + if !tx.isGadgetConfigItemEnabled(item.key) { + continue + } + if item.item.configPath == nil || item.item.configAttrs != nil { + continue + } + + disableKeys = append(disableKeys, fmt.Sprintf("disable-%s", item.item.device)) + } + return disableKeys +} + func (tx *UsbGadgetTransaction) DisableGadgetItemConfig(item gadgetConfigItem) { // remove symlink if exists if item.configPath == nil { @@ -174,7 +193,7 @@ func (tx *UsbGadgetTransaction) writeGadgetItemConfig(item gadgetConfigItem, dep beforeChange := make([]string, 0) disableGadgetItemKey := fmt.Sprintf("disable-%s", item.device) if item.configPath != nil && item.configAttrs == nil { - beforeChange = append(beforeChange, disableGadgetItemKey) + beforeChange = append(beforeChange, tx.getDisableKeys()...) } if len(item.attrs) > 0 { @@ -234,13 +253,7 @@ func (tx *UsbGadgetTransaction) writeGadgetItemConfig(item gadgetConfigItem, dep Description: "remove symlink", }) - tx.addFileChange(component, RequestedFileChange{ - Path: configPath, - ExpectedState: FileStateSymlink, - ExpectedContent: []byte(gadgetItemPath), - Description: "create symlink", - DependsOn: files, - }) + tx.addReorderSymlinkChange(configPath, gadgetItemPath, files) } return files @@ -263,6 +276,27 @@ func (tx *UsbGadgetTransaction) writeGadgetAttrs(basePath string, attrs gadgetAt return files } +func (tx *UsbGadgetTransaction) addReorderSymlinkChange(path string, target string, deps []string) { + tx.log.Trace().Str("path", path).Str("target", target).Msg("add reorder symlink change") + + if tx.reorderSymlinkChanges == nil { + tx.reorderSymlinkChanges = &RequestedFileChange{ + Component: "gadget-finalize", + Key: "reorder-symlinks", + Path: tx.configC1Path, + ExpectedState: FileStateSymlinkInOrderConfigFS, + Description: "order symlinks", + ParamSymlinks: []symlink{}, + } + } + + tx.reorderSymlinkChanges.DependsOn = append(tx.reorderSymlinkChanges.DependsOn, deps...) + tx.reorderSymlinkChanges.ParamSymlinks = append(tx.reorderSymlinkChanges.ParamSymlinks, symlink{ + Path: path, + Target: target, + }) +} + func (tx *UsbGadgetTransaction) WriteUDC() { // bound the gadget to a UDC (USB Device Controller) path := path.Join(tx.kvmGadgetPath, "UDC") @@ -270,6 +304,7 @@ func (tx *UsbGadgetTransaction) WriteUDC() { Path: path, ExpectedState: FileStateFileContentMatch, ExpectedContent: []byte(tx.udc), + DependsOn: []string{"reorder-symlinks"}, Description: "write UDC", }) } diff --git a/internal/usbgadget/usbgadget.go b/internal/usbgadget/usbgadget.go index 663aa22..f8b2b3e 100644 --- a/internal/usbgadget/usbgadget.go +++ b/internal/usbgadget/usbgadget.go @@ -80,6 +80,10 @@ var defaultLogger = logging.GetSubsystemLogger("usbgadget") // NewUsbGadget creates a new UsbGadget. func NewUsbGadget(name string, enabledDevices *Devices, config *Config, logger *zerolog.Logger) *UsbGadget { + return newUsbGadget(name, defaultGadgetConfig, enabledDevices, config, logger) +} + +func newUsbGadget(name string, configMap map[string]gadgetConfigItem, enabledDevices *Devices, config *Config, logger *zerolog.Logger) *UsbGadget { if logger == nil { logger = defaultLogger } @@ -96,7 +100,7 @@ func NewUsbGadget(name string, enabledDevices *Devices, config *Config, logger * name: name, kvmGadgetPath: path.Join(gadgetPath, name), configC1Path: path.Join(gadgetPath, name, "configs/c.1"), - configMap: defaultGadgetConfig, + configMap: configMap, customConfig: *config, configLock: sync.Mutex{}, keyboardLock: sync.Mutex{},