Repository navigation
Minimize the RPM content of the Quarkus container - #16879
Conversation
38bed25 to
41f732c
Compare
|
Tests ran over on my fork, built ok, but a lot of the tests failed https://github.com/ASzc/keycloak/actions/runs/4109534547/jobs/7091532236 Will look tomorrow at it |
|
CC @keycloak/cloud-native |
|
@ASzc Looking forward to have it .. How much we are saving in the image size? |
|
Since only deleting stuff from the same layer will affect total image size, and most of the content is in the parent layers, there's only a small reduction in image size. Looks like 569 MB, down from 731 MB |
|
Is there an easy way to get the container log? Those failing tests seem to be due to the pod getting stuck. I'm trying to run it locally, but it's taking forever |
|
Ok, the problem is the liveness/readiness probes use a local curl execution inside the container, and it no longer is installed. Curl and its large set of dependencies generate a lot of minor CVEs, that we're trying to avoid here, so I will look and see if the probes can be implemented differently operator/src/main/java/org/keycloak/operator/controllers/KeycloakDeployment.java#405 |
|
Great stuff @ASzc. Could you please create a GH issue for that and link to your fix following the contributing guidelines https://github.com/keycloak/keycloak/blob/main/CONTRIBUTING.md ? |
|
Created a matching issue: #16902 |
|
@ASzc Have we considered using the |
|
@jonkoops Ah, I had heard of something more minimal than minimal, but google never turned that micro image up! I will look at it |
|
Ok. I think I understand the technique they're using with ubi8-micro. Basically a multi-stage build, but copying everything (i.e. See their dockerfile https://catalog.redhat.com/software/containers/ubi8-micro/601a84aadd19c7786c47c8ea?container-tabs=dockerfile I'm going to fix the health probes first, then I will try to use micro for this PR. If it works, unfortunately that means I wrote the script for nothing, but at least it got the ball rolling here. |
41f732c to
634cec6
Compare
634cec6 to
28d8edc
Compare
|
So my script still seems to be required, the JDK package is just super broad. I've updated the script and dockerfile to work with ubi8-micro. Image size is ~400MB now. |
There was a problem hiding this comment.
@ASzc Nice work, I like the idea of shrinking the image and reducing deps to the bare minimum.
So my script still seems to be required, the JDK package is just super broad. I've updated the script and dockerfile to work with ubi8-micro. Image size is ~400MB now.
TBH, I'm not 100% about removing JDK dependencies. Can we be sure it won't affect server? We're not running full integration tests with the images, so I see some risk here. On the other hand, AFAICT we're speaking here mainly about cups-libs and copy-jdk-configs which should be safe to remove. Is that correct?
If we need the script just to remove JDK dependencies, possibly we could fully rely just on ubi8-micro (which would reduce maintenance costs). But not sure about this (we definitely don't need a printing libs in our image).
|
@vmuzikar All direct dependencies of java-17-openjdk-headless: and those that are missing after ubi8-null.sh: It seems to me these can be safely removed. I would imagine the JDK would crash if some code tried to use system audio or printers, but it seems to run fine otherwise? chkconfig and copy-jdk-configs are only used administratively, so they're irrelevant in a container. They also bring in a lot of transitive dependencies, like lua, python, etc. So they're best removed. |
28d8edc to
9103a63
Compare
There was a problem hiding this comment.
Looks like Dockerfile is not building. See https://github.com/keycloak/keycloak/actions/runs/4127383561/jobs/7130640669#step:4:9285.
Strange. I will check. |
|
@ASzc If you take too much time on it I can give it a chance. When running dist tests using a container the image is created at runtime. I don't know from the top of my head what else is missing. |
|
I found the problem. DockerKeycloakDistribution#getKeycloakContainer uses the API to submit the container for building, and doesn't send anything except the Dockerfile and the .tar.gz, so the script is indeed missing. I will try to fix it |
Even though we use `ubi8-minimal` as the parent of our container, it still has many RPMs installed that aren't necessary to run the Keycloak server. Also, since the JDK RPM (that we install on top of `ubi8-minimal`) is designed for general use, it pulls in more dependency RPMs than it strictly needs to, like cups and avahi. Keycloak will never need to access a printer itself! Trimming down these excess RPMs will improve our CVE statistics with automated scanners, and therefore let us perform fewer CVE rebuilds. `ubi8-null.sh` uses the low-level `rpm` command to identify and forcibly remove dependencies and operating system files that are not required to boot our Quarkus-based server. This includes `microdnf` and `rpm` itself! I have preserved bash however, so it's still possible to debug the container from a shell. I've created an initial set of allow/disallow lists, that seems to pass a smoke test (server boots, admin console works). This leaves 37 packages installed, with 96 removed relative to `ubi8-minimal`. We could go more minimal than this, or less minimal if required. Trial and error is required. Closes keycloak#16902
9103a63 to
e07fd22
Compare
|
The tests seem happy now |
|
Comparing the Trivy RPM CVE stats on the nightly, yesterday's versus today's: So seems successful! |
|
Nice, great work @ASzc! I wonder if moving to |
|
ubi9 might have less, but there's no requirement to fix medium CVEs under the CHI system. It only factors in high/critical https://access.redhat.com/articles/2803031 |
|
I've filed #17057 for updating to UBI 9. PR to follow shortly |
|
Hello all. Noticing This could be a breaking change for some use cases in the community. Do you have any guidance for the folks that make use of the ex. Building patterns/practices for use with low-footprint images such as ubi8-micro. |
|
@daurrutia That's a good point, thanks. We'll make sure to reflect this in the release notes. I reopened #16902 for this. @ASzc Could you please look into it? |
Fully agree to this. Unfortunately, this was not included in the release notes yet. Guides like this https://www.keycloak.org/server/containers are now also broken, due to removed curl (and package manager). |
|
I will look into updating the docs. At the moment I don't see anything that's totally broken with ubi micro
|
Keycloak are now using a micro version of the Redhat base image that doesn't include a package manager. There is a comment against the PR for that change which recommends using a base keycloak image and copying the files over to an image that has microdnf installed. keycloak/keycloak#16879 (comment) There are two other options. ADD in a Dockerfile supports URLs so we could download the python source and build Python into the image. This will take a long time for our image builds though so isn't ideal. Also, I haven't tried it and there's good chance a lot of the libraries needed to compile Python would be missing so we'd be back to square one. The other option is to re-write the python files in bash. This seems like a lot of work. The other issue is that there is a bug keycloak/keycloak#17248 where it throws an exception if the admin theme is missing. I've tried the workarounds on the ticket around adding a new theme but I can't get it to work. We're not going to be updating the database directly. They have said that the 21.0.1 release will have a fix for this. I've tried the nightly build and the fix is in there so we now just have to wait for 21.0.1 to release. The PR is ready for review though.
|
Docs updates happening under issue #17273 |
|
After upgrading from v20.0.0 to v21.1.1 we noticed that emails containing non-ascii characters are not accepted anymore. Not 100% sure, but by looking at the git history my gut feeling is that this PR removed the fix done in #11547 We have a custom image based on keycloakthat I think we can fix on our end as a workaround Test with v20
Test with v21.1.1
|
|
@dglozano I'm not sure how that could be, the parts added to the Dockerfile in that PR are still present in the current Dockerfile here. |
Even though we use
ubi8-minimalas the parent of our container, it still has many RPMs installed that aren't necessary to run the Keycloak server. Also, since the JDK RPM (that we install on top ofubi8-minimal) is designed for general use, it pulls in more dependency RPMs than it strictly needs to, like cups and avahi. Keycloak will never need to access a printer itself!Trimming down these excess RPMs will improve our CVE statistics with automated scanners, and therefore let us perform fewer CVE rebuilds.
ubi8-null.shuses the low-levelrpmcommand to identify and forcibly remove dependencies and operating system files that are not required to boot our Quarkus-based server. This includesmicrodnfandrpmitself! I have preserved bash however, so it's still possible to debug the container from a shell.I've created an initial set of allow/disallow lists, that seems to pass a smoke test (server boots, admin console works). This leaves 37 packages installed, with 96 removed relative to
ubi8-minimal. We could go more minimal than this, or less minimal if required. Trial and error is required.Closes #16902