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

problem with GRADLE_OPTS containing "|" (pipeline character) #261

Open
62mkv opened this issue May 5, 2021 · 19 comments
Open

problem with GRADLE_OPTS containing "|" (pipeline character) #261

62mkv opened this issue May 5, 2021 · 19 comments

Comments

@62mkv
Copy link

62mkv commented May 5, 2021

Hi all. I assume this is the source repository for images, found at https://hub.docker.com/r/fabric8/s2i-java/

I've just spent some time trying to understand the problem I have, when I run builder images based on "*-java11" versions of images. (NB: I did not try with latest -java8, they might be also susceptible)

Anyway, when our private OpenShift 3.11 installation starts running an "s2i-java" build, it's being given GRADLE_OPTS variable (see below)

==================================================================
Starting S2I Java Build .....
S2I source build for Gradle detected, due to presence of a *.gradle* in /tmp/src
Using GRADLE_OPTS '-Dhttps.proxyHost=proxy.my.corp -Dhttps.proxyPort=80 -Dhttp.proxyHost=proxy.my.corp -Dhttp.proxyPort=80 -Dhttp.nonProxyHosts="*.cluster.local|*.my.corp|*.svc|127.0.0.1|169.254.169.254|172.28.0.1|cp-ham-internal.my.corp|localhost" -XX:+UseParallelGC -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -XX:MinHeapFreeRatio=20 -XX:MaxHeapFreeRatio=40 -XX:+ExitOnOutOfMemoryError'
Running './gradlew --no-daemon build -x test '

Builder image, that used to be "s2i-java:latest" (digest a1688dbedce5f19bfe99be506a555676a0280f5de8ae5ee9ee1a0043d306b545) works fine with such variable, just as the one tagged 3.0-java11. But both 3.1-java11 and latest-java11 fail utterly, like this:

Running './gradlew --no-daemon build -x test '
./gradlew: line 181: *.my.corp: command not found
./gradlew: line 181: *.svc: command not found
./gradlew: line 181: 127.0.0.1: command not found
./gradlew: line 181: 169.254.169.254: command not found
./gradlew: line 181: 172.28.0.1: command not found
... (to be continued)

the only difference between working and failing configuration is the builder image tag (3.0-java11 works, some ancient *-java8 works, 3.1-java11 and latest-java11 fail)

I hope this can be fixed... or if there're any suggestions how to override that (I guess I could implement some fixes in the "run" script, by putting one into my source code; but so far has no idea how to go about)

thanks!

@rhuss
Copy link
Contributor

rhuss commented Jun 8, 2021

The issue is that the pipe symbol is not escaped. This is probably fixed in one version of the image but not in the other one. You can try to unset the nonProxyHost variable if this is possible for you. I guess that 3.0 just did not pick up nonProxyHost env var, so that it does not fail.

@rhuss
Copy link
Contributor

rhuss commented Jun 8, 2021

Can you try to use \| instead of | in the GRADLE_OPTS variable ?

Sorry, I can't help much more here, as I'm not working on this project anymore (and don't really know much about the Gradle support)

@62mkv
Copy link
Author

62mkv commented Jun 8, 2021

thanks @rhuss I have no idea yet, how those environment variables are composed, tbh (I just see that GRADLE_OPTS value in the build log when I trigger the Build Configuration on our OpenShift cluster). Probably it's the way this value (NO_PROXY) is being set in the VM that is running the pod. It's not set explicitly in the Build Configuration environment. Will try to sort it out with our OpenShift team. Thanks!

@62mkv
Copy link
Author

62mkv commented Jun 8, 2021

had a closer look at the code, and this seems to be somehow related: fabric8io-images/run-java-sh@1bfb364

probably, after that change, pipe character began breaking stuff. Also, looking at the test provided, it seems like it should accept unescaped pipe character in a NO_PROXY env variable? maybe @valdar could chime in? Should I raise an issue in https://github.com/fabric8io-images/run-java-sh/ ?

@rhuss
Copy link
Contributor

rhuss commented Jun 8, 2021

ah, yes:

# If there is no user provided GRADLE_OPTS, use options from run-java.sh
if [ -f "${RUN_JAVA_DIR}/run-java.sh" ] && [ -z "${GRADLE_OPTS}" ]; then
export GRADLE_OPTS=$(${RUN_JAVA_DIR}/run-java.sh options)
fi
if [ ! -z "${GRADLE_OPTS}" ]; then
echo "Using GRADLE_OPTS '${GRADLE_OPTS}'"
fi
(run-java.sh is evaluated by s2i's assemble.sh)

So I guess its a matter to make a release that leverages the fixed version of run-java.sh ...

@62mkv
Copy link
Author

62mkv commented Jun 8, 2021

Actually this particular change seems to be included in s2i:

ret="$ret -Dhttp.nonProxyHosts=$(echo "|$noProxy" | sed -e 's/,[[:space:]]*/|/g' | sed -e 's/[[:space:]]//g' | sed -e 's/|\./|\*\./g' | cut -c 2-)"

so I suspect it is actually the reason for observed problems with the "latest" images

@62mkv
Copy link
Author

62mkv commented Jun 8, 2021

this comment from you @rhuss looks ... encouraging :) fabric8io-images/run-java-sh#85 (review)

@valdar
Copy link
Contributor

valdar commented Jun 8, 2021

@62mkv I am not quite sure how the changes in handling NO_PROXY and http.nonProxyHosts env vars can interact with GRADL_OPTS env var...

What I remember from the issue is that the intended way (as the only scenario tested in tests) is to have quotes (") removed around the value of http.nonProxyHosts= jvm var. It seems from your initial log that you are missing this fix fabric8io-images/run-java-sh@52b3501 in particular the part removing the "..." around ret=?

@62mkv
Copy link
Author

62mkv commented Jun 8, 2021 via email

@valdar
Copy link
Contributor

valdar commented Jun 8, 2021

Yep @62mkv see my prev comment #261 (comment)

@62mkv
Copy link
Author

62mkv commented Jun 8, 2021 via email

@valdar
Copy link
Contributor

valdar commented Jun 8, 2021

@62mkv can you do this test for me? Instead of removing pipes character try to se GRADEL_OPTS to -Dhttps.proxyHost=proxy.my.corp -Dhttps.proxyPort=80 -Dhttp.proxyHost=proxy.my.corp -Dhttp.proxyPort=80 -Dhttp.nonProxyHosts=*.cluster.local|*.my.corp|*.svc|127.0.0.1|169.254.169.254|172.28.0.1|cp-ham-internal.my.corp|localhost -XX:+UseParallelGC -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -XX:MinHeapFreeRatio=20 -XX:MaxHeapFreeRatio=40 -XX:+ExitOnOutOfMemoryError i.e. same stuff but with the removed "..." around http.nonProxyHosts value.

@62mkv
Copy link
Author

62mkv commented Jun 9, 2021

@valdar not sure I follow, it was without any quotes since the very beginning? may be you want me to run with them? I can confirm, that with the quotes around -Dhttp.nonProxyHosts value it works ok

@valdar
Copy link
Contributor

valdar commented Jun 10, 2021

Ah @62mkv in that case can you try escaping pipes? like setting GRADEL_OPTS to -Dhttps.proxyHost=proxy.my.corp -Dhttps.proxyPort=80 -Dhttp.proxyHost=proxy.my.corp -Dhttp.proxyPort=80 -Dhttp.nonProxyHosts=*.cluster.local\|*.my.corp\|*.svc\|127.0.0.1\|169.254.169.254\|172.28.0.1\|cp-ham-internal.my.corp\|localhost -XX:+UseParallelGC -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -XX:MinHeapFreeRatio=20 -XX:MaxHeapFreeRatio=40 -XX:+ExitOnOutOfMemoryError

@62mkv
Copy link
Author

62mkv commented Jun 10, 2021

hi @valdar I confirm that with escaped pipes it works too (escaped pipe character, no quotes around "value"). in other words - exactly as per your suggestion

@valdar
Copy link
Contributor

valdar commented Jun 11, 2021

Is that a viable approach? can you change the value of NO_PROXY env var to take in account the escaped pipes? "\|".

The problem with quotes (and the original issue my Pr solved) is that it does not work if the whole constructed command is used with exec. And that is done on a number of images based on this one...

@62mkv
Copy link
Author

62mkv commented Jun 11, 2021 via email

@62mkv
Copy link
Author

62mkv commented Jun 11, 2021

now things get even funnier: cluster admins say "we don't use pipe characters at all, we separate values with comma". so if they are to be believed, the NO_PROXY env variable looks along the lines of

.cluster.local,.corp.org,.svc,10.61.105.10,localhost

so. probably those pipes are created by run-java.sh itself when GRADLE_OPTS is produced, so maybe there's a chance they could be escaped?

@valdar
Copy link
Contributor

valdar commented Jun 11, 2021

Mmmmm probably there is: https://github.com/fabric8io-images/run-java-sh/blob/v1.3.8/fish-pepper/run-java-sh/fp-files/run-java.sh#L508 basically there is a substitution "," -> "|" that could be changed to "," -> "\|" and checked the tests....

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

No branches or pull requests

3 participants