Add fips config to jvm.options for observabilitySRE #17958
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
This pull request does not have a backport label. Could you fix it @donoghuc? 🙏
|
In addition to setting `LS_JAVA_OPTS` we now include the fips config java options in the `/usr/share/logstash/config/jvm.options` file. This ensures that if consumers of the image overwrite `LS_JAVA_OPTS` the fips config is still respected from `jvm.options`.
donoghuc
force-pushed
the
fips-config-jvm-config
branch
from
7e41bfa to
51bdea0
Compare
|
Exhaustive test run https://buildkite.com/elastic/logstash-exhaustive-tests-pipeline/builds/2286 (note this will actually pull the already published observabilitySRE container). |
docker/templates/Dockerfile.erb
Outdated
| # echos are for ensuring that the file ends with a newline | ||
| RUN echo "" >> /usr/share/logstash/config/jvm.options && \ | ||
| echo "" >> /usr/share/logstash/config/jvm.options && \ | ||
| cat /tmp/fips-jvm.options >> /usr/share/logstash/config/jvm.options && \ |
There was a problem hiding this comment.
🤔 is there a reason we still set LS_JAVA_OPTS, if we are injecting the same settings into the config file? This means that by default the LS jvm will be invoked with doubled-up settings that are currently identical but from two different sources.
We already have the safety of LS plugin that prevents instantiation if these settings are not properly applied.
There was a problem hiding this comment.
ah, yeah i did not think that through. I think file only is probably best. I'll update. Good catch.
docker/templates/Dockerfile.erb
Outdated
| RUN echo "" >> /usr/share/logstash/config/jvm.options && \ | ||
| echo "" >> /usr/share/logstash/config/jvm.options && \ | ||
| cat /tmp/fips-jvm.options >> /usr/share/logstash/config/jvm.options && \ |
There was a problem hiding this comment.
nit: dry it up with a subshell?
| RUN echo "" >> /usr/share/logstash/config/jvm.options && \ | |
| echo "" >> /usr/share/logstash/config/jvm.options && \ | |
| cat /tmp/fips-jvm.options >> /usr/share/logstash/config/jvm.options && \ | |
| RUN ( \ | |
| echo ""; echo ""; \ | |
| cat /tmp/fips-jvm.options \ | |
| ) >> /usr/share/logstash/config/jvm.options && \ |
Stop setting LS_JAVA_OPTS in favor of jvm.options.
Codereview suggestion
|
💚 Build Succeeded
History
|
* Add fips config to jvm.options for observabilitySRE In addition to setting `LS_JAVA_OPTS` we now include the fips config java options in the `/usr/share/logstash/config/jvm.options` file. This ensures that if consumers of the image overwrite `LS_JAVA_OPTS` the fips config is still respected from `jvm.options`. * *only* set jvm opts via jvm.options Stop setting LS_JAVA_OPTS in favor of jvm.options. * Use subshell to clean up file concat Codereview suggestion (cherry picked from commit 64e6462)
* Add fips config to jvm.options for observabilitySRE In addition to setting `LS_JAVA_OPTS` we now include the fips config java options in the `/usr/share/logstash/config/jvm.options` file. This ensures that if consumers of the image overwrite `LS_JAVA_OPTS` the fips config is still respected from `jvm.options`. * *only* set jvm opts via jvm.options Stop setting LS_JAVA_OPTS in favor of jvm.options. * Use subshell to clean up file concat Codereview suggestion (cherry picked from commit 64e6462) Co-authored-by: Cas Donoghue <[email protected]>
3 participants
Release notes
[rn:skip]
What does this PR do?
In addition to setting
LS_JAVA_OPTSwe now include the fips config java options in the/usr/share/logstash/config/jvm.optionsfile. This ensures that if consumers of the image overwriteLS_JAVA_OPTSthe fips config is still respected fromjvm.options.Related Issues