-
-
Notifications
You must be signed in to change notification settings - Fork 7
Various update: log4shell removal, testing-tools uid/gid #1192
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
base: main
Are you sure you want to change the base?
Conversation
@@ -53,8 +53,6 @@ RUN microdnf update && \ | |||
which \ | |||
xz \ | |||
zlib-devel \ | |||
# Required for log4shell.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff looks like it is now missing a &&
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I'll take a look!
tar \ | ||
zip \ | ||
unzip \ | ||
# Java 11 seems like the best middle-ground for all tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I still dislike us commenting like 20 lines above and prefer keeping the comments at the line where they apply.
Not blocking this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not possible with the EOF/heredoc style unfortunately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use a trick in other places, such as
docker-images/trino/trino/Dockerfile
Lines 46 to 55 in e93d8f3
./mvnw \ | |
--batch-mode \ | |
--no-transfer-progress \ | |
install \ | |
`# -Dmaven.test.skip # Unable to skip test compilation without an unused dependency error for software.amazon.awssdk:identity-spi` \ | |
-DskipTests `# Skip test execution` \ | |
-Dcheckstyle.skip `# Skip checkstyle checks. We dont care if the code is properly formatted, it just wastes time` \ | |
-Dmaven.javadoc.skip=true `# Dont generate javadoc` \ | |
-Ddep.presto-jdbc-under-test=${NEW_VERSION} \ | |
--projects="$SKIP_PROJECTS" |
The following diff works:
diff --git a/testing-tools/Dockerfile b/testing-tools/Dockerfile
index 06e10f0..898404f 100644
--- a/testing-tools/Dockerfile
+++ b/testing-tools/Dockerfile
@@ -33,7 +33,6 @@ COPY testing-tools/python /stackable/python
COPY testing-tools/licenses /licenses
# krb5-user/libkrb5-dev are needed for Kerberos support.
-# Java 11 seems like the best middle-ground for all tools
RUN <<EOF
apt-get update
apt-get install -y --no-install-recommends \
@@ -42,8 +41,10 @@ apt-get install -y --no-install-recommends \
curl \
gzip \
jq \
+ `# Needed for Kerberos support` \
krb5-user \
kubernetes-client \
+ `# Needed for Kerberos support` \
libkrb5-dev \
libssl-dev \
libxml2-dev \
@@ -58,6 +59,7 @@ apt-get install -y --no-install-recommends \
tar \
zip \
unzip \
+ `# Java 11 seems like the best middle-ground for all tools` \
openjdk-11-jdk-headless
apt-get clean
Description
These two are related because the log4shell scanner had uid 1000 hardcoded as well.
It was only used in Omid...
Definition of Done Checklist
Note
Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant.
Please make sure all these things are done and tick the boxes
TIP: Running integration tests with a new product image
The image can be built and uploaded to the kind cluster with the following commands:
See the output of
bake
to retrieve the image tag for<image-tagged-with-the-major-version>
.