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.

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

oc rsync fails if 'oc' command has space in it's path #15187

Closed
robstryker opened this issue Jul 13, 2017 · 10 comments
Closed

oc rsync fails if 'oc' command has space in it's path #15187

robstryker opened this issue Jul 13, 2017 · 10 comments
Assignees
Labels
component/cli kind/bug Categorizes issue or PR as related to a bug. priority/P2

Comments

@robstryker
Copy link

[provide a description of the issue]

Version

[provide output of the openshift version or oc version command]
oc 3.3.1.5 and oc 3.5.5.5

Steps To Reproduce
  1. [step 1] copy 'oc' command to a folder with spaces in it
  2. [step 2] use full path to call that oc command, and call oc rsync. ex:

home/rob/tmp/with\ spaces/oc rsync --token=xxx --server=https://192.168.99.100:8443 --insecure-skip-tls-verify=true --exclude='.git' eap-app-1-qpldm:/opt/eap/standalone/deployments/ -n myproj /home/rob/apps/eclipse/workspaces/runtime-New_configuration/.metadata/.plugins/org.jboss.ide.eclipse.as.core/myproj@eap-app/deploy/

Current Result

rsync: Failed to exec /home/rob/tmp/with: No such file or directory (2)
rsync error: error in IPC code (code 14) at pipe.c(85) [Receiver=3.1.2]
rsync: connection unexpectedly closed (0 bytes received so far) [Receiver]
rsync error: error in IPC code (code 14) at io.c(226) [Receiver=3.1.2]
error: exit status 14

Expected Result

The rsync occurs as expected

Additional Information

Hardy thinks the problem may be here:
https://github.com/openshift/origin/blob/master/pkg/cmd/cli/cmd/rsync/copyrsync.go#L37

The oc command calls rsync and passes in the "OpenShift CLI rsh command as the remote shell command for rsync". This is obviously failing hard because of the spaces. rsync thinks only the first part of the 'oc' command's path (pre space) is the command to call.

This code may need to ensure that the 'oc' command's spaces are properly escaped.

The issue may be in SiblingCommand:

func SiblingCommand(cmd *cobra.Command, name string) string {

Either way, when passing the 'oc' command into rsync, it is not accounting for spaces.

@robstryker
Copy link
Author

I cannot figure out how to add an cli/rsync label to this...

@robstryker
Copy link
Author

It's worth noting here that the following commands DO work as expected:

/home/rob/tmp/with\ spaces/oc status
/home/rob/tmp/with\ spaces/oc login
/home/rob/tmp/with\ spaces/oc get pods

etc etc etc. Everything functions as normal except the rsync.

@robstryker
Copy link
Author

related issue: https://issues.jboss.org/browse/JBIDE-23862

@csrwng csrwng added component/cli kind/bug Categorizes issue or PR as related to a bug. priority/P2 labels Jul 13, 2017
@bparees
Copy link
Contributor

bparees commented Jul 13, 2017

unless i'm missing something, you can work around this by running oc from another dir (and still pointing to the same source dir for the rsync), right?

@robstryker
Copy link
Author

Moving 'oc' to a folder that does not have a space in its path is a valid workaround. And I'll admit I don't know at all (plus or minus) which of our workflows may be installing oc in a path with spaces, or allow users to choose, or anything. I simply have no data on that at this time...

It might also be a workaround to just make a symlink (but I haven't tried it yet ;)) ... so there are workarounds, but, we need to know if our tools should TRY to allow paths with spaces, or if we should ban it outright for the time being.

@bparees
Copy link
Contributor

bparees commented Jul 13, 2017

it's a bug and we'll fix it, i'm just trying to assess the urgency. I can't promise it's going to make the 3.6 release and it doesn't sound like something we'd backport to 3.5, so i think you will want to consider trying to handle it in your tooling.

@robstryker
Copy link
Author

Thanks for the candid update. That gives me some good guidance to work with.

openshift-merge-robot added a commit that referenced this issue Aug 1, 2017
…ls_with_space_in_path

Automatic merge from submit-queue (batch tested with PRs 15162, 14901, 15195)

Fixing oc rsync command fails if oc command has space in path

oc sibling commands (example rsync) fail if there is a space
in the path to the oc command

escapes spaces in the command path

Fixes #15187
@adietish
Copy link
Member

@coreydaley thx for fixing this. Is there any plan to backport it to existing versions?

@coreydaley
Copy link
Member

@adietish not that I am aware of

@adietish
Copy link
Member

@coreydaley ok, can you then please point me to the version of oc this will be included in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/cli kind/bug Categorizes issue or PR as related to a bug. priority/P2
Projects
None yet
Development

No branches or pull requests

5 participants