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

Allow for tildes on exec path if they aren't the start of the path #39

Open
munkyshi opened this issue Nov 17, 2017 · 7 comments
Open

Comments

@munkyshi
Copy link

munkyshi commented Nov 17, 2017

We use tildes in install paths (/foo/bar/baz/group~value~id). This doesn't jive well with the current regex: https://github.com/palantir/go-java-launcher/blob/develop/launchlib/launcher.go#L110 when the provided JAVA_HOME is a subdirectory of that install path.

Request is to allow tildes if they aren't at the beginning of the path (AKA won't resolve to the home directory).

@munkyshi munkyshi changed the title Allow for tildes on exec path if they aren't a standalone segment Allow for tildes on exec path if they aren't the start of the path Nov 17, 2017
@markelliot
Copy link
Contributor

This hasn’t been an issue to date. Why is it an issue now?

@munkyshi
Copy link
Author

munkyshi commented May 15, 2018

I don't remember the full context at this point, but I think it was because I was looking at Skylab managing deployd, which would then change the deployd java binary's (symlinked) exec path to have tildes in it.

Evidently we were able to get things off the ground without this PR, so we must have found a workaround -- probably by using an absolute path with symlink resolution? -- so probably good to close out here.

@jlong8 to sanity check my statement, since she's evidently hitting something related?

@munkyshi
Copy link
Author

Caught up with some context, and seems like there was some sort of regression with the symlink resolution -- reopening to continue discussion, since it seems like it'd be nice to have this regardless of the situation?

@munkyshi munkyshi reopened this May 15, 2018
@markelliot
Copy link
Contributor

sorry, could you sketch out a dummy example of the directory tree?

@jlong8
Copy link

jlong8 commented May 15, 2018

The new service home path in Skylab-managed deployd is something like /foo/bar/services/daemons~deployd~id, which is a symlink to /foo/bar/services/.<id>. Before Skylab-managed deployd, deployd home was /foo/bar/skylab/deployd.

Our workaround was that in a script to start deployd when it was stopped, we used pwd -P when getting the service home to avoid symlinks. Looks like this change was never made to our init.sh, so if deployd was most recently started with the init script, then the java home directory was invalid according to this blacklist and java services couldn't start. We can workaround this by editing the init.sh script, so this change isn't urgent.

@munkyshi
Copy link
Author

munkyshi commented May 15, 2018

EDIT: whoops, it looks like @jlong8 already posted a more concise writeup of what I had here.

Sure -- I'm not sure if it's safe to post the exact details / paths on external github, so changing some details around.

We run go-java-launcher from a dummy-service from the path ${INSTALL_HOME}/service/lib/jdk8/bin/java.

Previously, $INSTALL_HOME did not contain tildes, and we'd run it from /foo/bar/dummy-service/service/lib/jdk8/bin/java.

We're now using a platform management tool that installs dummy-service on a path with tildes, and the bash launcher that wraps environment variables evaluates $INSTALL_HOME (using $(dirname "$0")) as: /foo/bar/baz/management~dummy-service~123/, so the java binary path is now: /foo/bar/baz/management~dummy-service~123/service/lib/jdk8/bin/java.

There is a workaround -- /foo/bar/baz/management~dummy-service~123 is actually a symlink to /foo/bar/baz/.123, and we can use the absolute path /foo/bar/baz/.123/service/lib/jdk8/bin/java, which satisfies the execution path safety check.

We've been using this workaround, but it recently failed us. There's a PR to fix that, but it'd be nice if the go-java-launcher worked with the tildes in the first place.

Summary of relevant paths:

$ cd /foo/bar/baz
$ tree

management~dummy-service~123 -> .123
.123
.123/service/lib/jdk8/bin/java

@markelliot
Copy link
Contributor

markelliot commented May 15, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants