jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.84k stars 1.91k forks source link

Jetty 10.0.15: Env variable in console-capture.ini not expanded anymore #9663

Closed jnehlmeier closed 1 month ago

jnehlmeier commented 1 year ago

Jetty version(s)

10.0.15

Java version/vendor (use: java -version)

OpenJDK Runtime Environment Temurin-19.0.1+10 (build 19.0.1+10) OpenJDK 64-Bit Server VM Temurin-19.0.1+10 (build 19.0.1+10, mixed mode, sharing)

OS type/version

Docker Ubuntu image / Mac OS

Description

I used 10.0.13 and have customized jetty-base/start.d/console-capture.ini using jetty.console-capture.dir=logs/${INSTANCE_NAME} with INSTANCE_NAME being an environment variable.

Because Jetty is run in a docker container and because Jetty forks a new VM because of a module, we used a docker entrypoint script that does:

# Save real command line which still contains ENV variables that need to be further expanded
CMD=`java -jar $JETTY_HOME/start.jar --dry-run`

# Use eval to expand ENV variables inside $CMD
CMD=$(eval "echo $CMD")

With Jetty 10.0.15 the --dry-run command now emits properties wrapped with single tick quotes ('jetty.console-capture.dir'='logs/${INSTANCE_NAME}') which in turn disables further variable expansion using the above approach.

So Jetty now fails to start as it can not create directory logs/${INSTANCE_NAME}.

To mimic the above in shell you could do

INSTANCE_NAME=MY-INSTANCE

CMD="prefix 'jetty.console-capture.dir'='logs/\${INSTANCE_NAME}' suffix"
echo $CMD # prints "prefix 'jetty.console-capture.dir'='logs/${INSTANCE_NAME}' suffix"
eval "echo $CMD" # prints "prefix jetty.console-capture.dir=logs/${INSTANCE_NAME} suffix"

CMD_OLD="prefix jetty.console-capture.dir=logs/\${INSTANCE_NAME} suffix"
echo $CMD_OLD #prints "prefix jetty.console-capture.dir=logs/${INSTANCE_NAME} suffix"
eval "echo $CMD_OLD" # prints "prefix jetty.console-capture.dir=logs/MY-INSTANCE suffix"

While we use a custom docker image I think your official jetty image might also be affected.

joakime commented 1 year ago

This was a change due to Issue #9309

Our jetty.sh was changed to use xargs instead (see PR #9313), perhaps you can do the same.

Or, if you can solve issue #9309 without xargs, we'd like to hear how.

joakime commented 1 year ago

And for the record, environment variable expansion was never a supported feature of start.jar.

gregw commented 1 year ago

@lachlan-roberts can you give docker a check to ensure that all is good after #9309... including documentation.

jnehlmeier commented 1 year ago

And for the record, environment variable expansion was never a supported feature of start.jar.

@joakime Yes that is why we eval'd the final command. As far as I know Jetty only supports expanding jetty properties in *.mod files, right? At least there are some mod files that use ${jetty.base} in their [exec] block.

So what is the preferred way of configuring Jetty when using Docker/Docker Swarm/Kubernetes and usually using environment variables to configure the container?

For example I tried saving the output of --dry-run in a new shell script that execs the generated java command. That generated shell script is then execed from the docker entrypoint shell script. The hope was that the shell would expand all variables as long as they were exported and no explicit eval is needed. However that fails because --add-opens .... JVM params stored in a custom jetty module now have single tick quotes and the JVM does not recognize the '--add-opens ...' parameter and fails to start.

Next I tried using ENTRYPOINT and CMD together with CMD holding jetty properties that I want to set and use jetty properties in custom mod files. The entrypoint would then append the CMD contents to the generated jetty command. It works but I can not use env variables within CMD so everything needs to be hardcoded in CMD. This is quite annoying because while you can define the default CMD within the Dockerfile you need to copy everything if you want to temporarily adjust the properties in production.

Currently the easiest workaround for me is to take my existing entrypoint and let it eval the --dry-run command twice. The first eval will remove the single tick quotes and the second eval will expand env variables that have been used in jetty ini / mod files. Obviously I don't really like that.

Wouldn't it be easier in general if jetty would implement expanding env variables in mod/ini files as well? The code should already exist because of expanding jetty properties, just the source is a different one.

joakime commented 1 year ago

You could pass in shell environment variables as Java system properties, then they are expanded by the Jetty start mechanism.

As any property in a Jetty Start configuration can contain characters that are interpreted special on different shells, the --dry-run will preserve those characters even from the shell messing with them.

Some examples of problematic properties.

jetty.requestlog.formatString=%{client}a - %u %{dd/MMM/yyyy:HH:mm:ss ZZZ|GMT}t "%r" %s %O "%{Referer}i" "%{User-Agent}i"
contextHandlerClass=org.eclipse.jetty.server.handler.ResourceHandler$ResourceContext
lachlan-roberts commented 1 year ago

this was the workaround for the official Jetty docker image https://github.com/eclipse/jetty.docker/pull/140

jnehlmeier commented 1 year ago

@lachlan-roberts Thanks. I am currently experimenting with moving to the official jetty image to experience less surprises while upgrading jetty. But if migration doesn't work well then I try to replicate your workaround in my custom image.

jnehlmeier commented 1 year ago

@lachlan-roberts Please see https://github.com/eclipse/jetty.project/issues/8348#issuecomment-1665765701

It is now almost impossible to use --add-opens if you don't want to run jetty in full JPMS mode.

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.

joakime commented 1 month ago

Being handled by #11408