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

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

Closed
jnehlmeier opened this issue Apr 19, 2023 · 10 comments
Closed
Labels
Bug For general bugs on Jetty side Stale For auto-closed stale issues and pull requests

Comments

@jnehlmeier
Copy link

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.

@jnehlmeier jnehlmeier added the Bug For general bugs on Jetty side label Apr 19, 2023
@joakime
Copy link
Contributor

joakime commented Apr 19, 2023

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
Copy link
Contributor

joakime commented Apr 19, 2023

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

@gregw
Copy link
Contributor

gregw commented Apr 19, 2023

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

@jnehlmeier
Copy link
Author

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
Copy link
Contributor

joakime commented Jul 12, 2023

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
Copy link
Contributor

this was the workaround for the official Jetty docker image
jetty/jetty.docker#140

@jnehlmeier
Copy link
Author

@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
Copy link
Author

@lachlan-roberts Please see #8348 (comment)

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

Copy link

github-actions bot commented Aug 4, 2024

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.

@github-actions github-actions bot added the Stale For auto-closed stale issues and pull requests label Aug 4, 2024
@joakime
Copy link
Contributor

joakime commented Aug 5, 2024

Being handled by #11408

@joakime joakime closed this as completed Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side Stale For auto-closed stale issues and pull requests
Projects
None yet
Development

No branches or pull requests

4 participants