fix: simplify RunRemote to not re-prefix source or dest
RunRemote was adding user@host: prefixes on top of the paths already pre-prefixed by engine.go in the remote-to-remote branch, causing "both remote" rsync errors. The source path was also incorrectly prefixed with srcUserHost:, making rsync reject the command entirely. The fix: buildArgs already orders args correctly for rsync (source then dest), so just use args[-2] and args[-1] as-is without any additional prefixing.
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
BINARY=syncserver
|
BINARY=syncserver
|
||||||
VERSION?=1.0.38
|
VERSION?=1.0.39
|
||||||
GO?=go
|
GO?=go
|
||||||
LDFLAGS=-s -w -X main.version=$(VERSION) -X main.commit=$(shell git rev-parse --short HEAD 2>/dev/null || echo unknown)
|
LDFLAGS=-s -w -X main.version=$(VERSION) -X main.commit=$(shell git rev-parse --short HEAD 2>/dev/null || echo unknown)
|
||||||
BUILD_FLAGS=CGO_ENABLED=0
|
BUILD_FLAGS=CGO_ENABLED=0
|
||||||
|
|||||||
@@ -96,39 +96,16 @@ func (r *RsyncRunner) RunRemote(ctx context.Context, pair *SyncPairConfig, src *
|
|||||||
destKey = filepath.Join(r.sshDir, "id_ed25519")
|
destKey = filepath.Join(r.sshDir, "id_ed25519")
|
||||||
}
|
}
|
||||||
|
|
||||||
srcUserHost := fmt.Sprintf("%s@%s", src.SSHUser, src.Host)
|
|
||||||
args := r.buildArgs(pair)
|
args := r.buildArgs(pair)
|
||||||
|
|
||||||
innerSSH := fmt.Sprintf("ssh -i %s -o StrictHostKeyChecking=accept-new -o UserKnownHostsFile=%s",
|
innerSSH := fmt.Sprintf("ssh -i %s -o StrictHostKeyChecking=accept-new -o UserKnownHostsFile=%s",
|
||||||
destKey, filepath.Join(r.sshDir, "known_hosts"))
|
destKey, filepath.Join(r.sshDir, "known_hosts"))
|
||||||
rsyncFlags := strings.Join(args[:len(args)-2], " ")
|
rsyncFlags := strings.Join(args[:len(args)-2], " ")
|
||||||
|
sourcePath := args[len(args)-2]
|
||||||
var sourcePath, destPath string
|
destPath := args[len(args)-1]
|
||||||
if pair.Direction == "pull" {
|
|
||||||
sourcePath = args[len(args)-1]
|
|
||||||
destPath = args[len(args)-2]
|
|
||||||
} else {
|
|
||||||
sourcePath = args[len(args)-2]
|
|
||||||
destPath = args[len(args)-1]
|
|
||||||
}
|
|
||||||
|
|
||||||
var remoteSrc, remoteDst string
|
|
||||||
if pair.Direction == "pull" {
|
|
||||||
remoteSrc = sourcePath
|
|
||||||
remoteDst = destPath
|
|
||||||
if !strings.Contains(destPath, "@") {
|
|
||||||
remoteDst = fmt.Sprintf("%s@%s:%s", dst.SSHUser, dst.Host, destPath)
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
remoteDst = destPath
|
|
||||||
if !strings.Contains(destPath, "@") {
|
|
||||||
remoteDst = fmt.Sprintf("%s@%s:%s", dst.SSHUser, dst.Host, destPath)
|
|
||||||
}
|
|
||||||
remoteSrc = fmt.Sprintf("%s:%s", srcUserHost, sourcePath)
|
|
||||||
}
|
|
||||||
|
|
||||||
remoteCmd := fmt.Sprintf("rsync %s -e %q %s %s",
|
remoteCmd := fmt.Sprintf("rsync %s -e %q %s %s",
|
||||||
rsyncFlags, innerSSH, remoteSrc, remoteDst)
|
rsyncFlags, innerSSH, sourcePath, destPath)
|
||||||
|
|
||||||
sshArgs := []string{
|
sshArgs := []string{
|
||||||
"-i", src.PrivKey,
|
"-i", src.PrivKey,
|
||||||
|
|||||||
@@ -16,40 +16,15 @@ type remoteCmdTest struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (tc remoteCmdTest) build() (srcArg, dstArg string) {
|
func (tc remoteCmdTest) build() (srcArg, dstArg string) {
|
||||||
srcUserHost := tc.srcUser + "@" + tc.srcHost
|
|
||||||
dstUserHost := tc.dstUser + "@" + tc.dstHost
|
|
||||||
|
|
||||||
runner := NewRsyncRunner("/tmp/ssh", "")
|
runner := NewRsyncRunner("/tmp/ssh", "")
|
||||||
args := runner.buildArgs(&SyncPairConfig{
|
args := runner.buildArgs(&SyncPairConfig{
|
||||||
Source: tc.srcPath,
|
Source: tc.srcPath,
|
||||||
Dest: tc.dstPath,
|
Dest: tc.dstPath,
|
||||||
Direction: tc.direction,
|
Direction: tc.direction,
|
||||||
})
|
})
|
||||||
|
sourcePath := args[len(args)-2]
|
||||||
var sourcePath, destPath string
|
destPath := args[len(args)-1]
|
||||||
if tc.direction == "pull" {
|
return sourcePath, destPath
|
||||||
sourcePath = args[len(args)-1]
|
|
||||||
destPath = args[len(args)-2]
|
|
||||||
} else {
|
|
||||||
sourcePath = args[len(args)-2]
|
|
||||||
destPath = args[len(args)-1]
|
|
||||||
}
|
|
||||||
|
|
||||||
var remoteSrc, remoteDst string
|
|
||||||
if tc.direction == "pull" {
|
|
||||||
remoteSrc = sourcePath
|
|
||||||
remoteDst = destPath
|
|
||||||
if !strings.Contains(destPath, "@") {
|
|
||||||
remoteDst = dstUserHost + ":" + destPath
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
remoteDst = destPath
|
|
||||||
if !strings.Contains(destPath, "@") {
|
|
||||||
remoteDst = dstUserHost + ":" + destPath
|
|
||||||
}
|
|
||||||
remoteSrc = srcUserHost + ":" + sourcePath
|
|
||||||
}
|
|
||||||
return remoteSrc, remoteDst
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestBuildArgs_Push(t *testing.T) {
|
func TestBuildArgs_Push(t *testing.T) {
|
||||||
@@ -175,7 +150,7 @@ func TestRunRemote_PushDestNoDoublePrefix(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRunRemote_PushSrcCorrectlyPrefixed(t *testing.T) {
|
func TestRunRemote_PushSrcAndDestStayAsIs(t *testing.T) {
|
||||||
tc := remoteCmdTest{
|
tc := remoteCmdTest{
|
||||||
srcHost: "10.5.1.10",
|
srcHost: "10.5.1.10",
|
||||||
srcUser: "root",
|
srcUser: "root",
|
||||||
@@ -187,15 +162,15 @@ func TestRunRemote_PushSrcCorrectlyPrefixed(t *testing.T) {
|
|||||||
}
|
}
|
||||||
srcArg, dstArg := tc.build()
|
srcArg, dstArg := tc.build()
|
||||||
|
|
||||||
if !strings.HasPrefix(srcArg, "root@10.5.1.10:/") {
|
if srcArg != "/share/homes/admin/media" {
|
||||||
t.Errorf("push src = %q, want prefix 'root@10.5.1.10:/'", srcArg)
|
t.Errorf("push src = %q, want raw path '/share/homes/admin/media' (no user@host: prefix added)", srcArg)
|
||||||
}
|
}
|
||||||
if !strings.HasPrefix(dstArg, "admin@10.5.0.144:/") {
|
if dstArg != "/share/media/peliculas" {
|
||||||
t.Errorf("push dst = %q, want prefix 'admin@10.5.0.144:/'", dstArg)
|
t.Errorf("push dst = %q, want raw path '/share/media/peliculas'", dstArg)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRunRemote_PullSrcNotPrefixed(t *testing.T) {
|
func TestRunRemote_PullSrcAndDestStayAsIs(t *testing.T) {
|
||||||
tc := remoteCmdTest{
|
tc := remoteCmdTest{
|
||||||
srcHost: "10.5.0.144",
|
srcHost: "10.5.0.144",
|
||||||
srcUser: "admin",
|
srcUser: "admin",
|
||||||
@@ -205,13 +180,16 @@ func TestRunRemote_PullSrcNotPrefixed(t *testing.T) {
|
|||||||
dstPath: "/share/data",
|
dstPath: "/share/data",
|
||||||
direction: "pull",
|
direction: "pull",
|
||||||
}
|
}
|
||||||
srcArg, _ := tc.build()
|
srcArg, dstArg := tc.build()
|
||||||
|
|
||||||
if countOccurrences(srcArg, "@") > 1 {
|
if countOccurrences(srcArg, "@") > 1 {
|
||||||
t.Errorf("pull src %q has double SSH spec", srcArg)
|
t.Errorf("pull src %q has double SSH spec", srcArg)
|
||||||
}
|
}
|
||||||
if srcArg != "admin@10.5.0.144:/share/media/peliculas" {
|
if srcArg != "/share/data" {
|
||||||
t.Errorf("pull src = %q, want 'admin@10.5.0.144:/share/media/peliculas'", srcArg)
|
t.Errorf("pull src (rsync dest) = %q, want raw '/share/data'", srcArg)
|
||||||
|
}
|
||||||
|
if !strings.HasPrefix(dstArg, "admin@10.5.0.144:/") {
|
||||||
|
t.Errorf("pull dst (rsync src) = %q, want 'admin@10.5.0.144:/...' prefix", dstArg)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user