Skip to content

fix: update base image to maven supported #7001

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

Merged
merged 3 commits into from
Aug 24, 2020
Merged

fix: update base image to maven supported #7001

merged 3 commits into from
Aug 24, 2020

Conversation

donbowman
Copy link
Contributor

@donbowman donbowman commented Jul 20, 2020

Switch base image from jimschubert/8-jdk-alpine-mvn:1.0 to
maven:3.6.3-jdk-11-openj9.

The jimschubert/8-jdk-alpine-mvn:1.0 is not managed by
any project, it has a very old (alpine 3.4) base image
with many CVE security flaw.

Swap to the image managed by the Maven apache project.

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
  • Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

Switch base image from jimschubert/8-jdk-alpine-mvn:1.0 to
maven:3.6.3-jdk-11-openj9.

The jimschubert/8-jdk-alpine-mvn:1.0 is not managed by
any project, it has a very old (alpine 3.4) base image
with many CVE security flaw.

Swap to the image managed by the Maven apache project.
@wing328
Copy link
Member

wing328 commented Jul 21, 2020

cc @jimschubert (the owner of jimschubert/8-jdk-alpine-mvn)

@jimschubert
Copy link
Member

I'm fine with changing it. I rebuild the 2.0 version of that image weekly, not the 1.0 version. I created the image because at the time the official Maven image pulled deps over insecure http. I'll need to bump the Maven version, but my 2.0 image picks up all updates from the underlying openjdk image.

I would suggest that we target openjdk, though, as that's what we test against and support.

@donbowman
Copy link
Contributor Author

ok so change the target to be
https://hub.docker.com/_/openjdk
and use 15-slim-buster tag?

@jimschubert
Copy link
Member

@donbowman maven:3-jdk-8 would be the one to use. This matches the run-in-docker.sh script. We don't have to worry too much about image size for this container because it's only used on developer machines. The Maven image is built off openjdk, maintained by the maven community, and is kept up to date regularly.

Notice as well that the Dockerfile in our project root is not intended for any production usage, and we've really intended for run-in-docker.sh to be the development entrypoint.

@donbowman
Copy link
Contributor Author

donbowman commented Aug 4, 2020

we use it in our CI so the image size is a bit of a concern.
i'll take a look at above

i don't understand how run-in-docker.sh could be the entrypoint, it calls docker itself, which would imply dind, which we don't allow.

@jimschubert
Copy link
Member

@donbowman I'm curious for what use case you'd be using the Dockerfile in the project root for CI? We don't publish any images from that Dockerfile, so you'd have to be cloning the repo and calling docker directly anyway (in which case run-in-docker.sh is probably a better bet).

If you're pulling the CLI image from Docker Hub, I'll have updated the builder images to my bash-2.0 image via #7129. This image is updated via cron every week. It does still target Maven 3.3.9, and we use these specifically because of the size: 120MB versus maven:3-jdk-8 at 522MB. Even openjdk:15-slim-buster which you'd suggested is 415MB and doesn't include maven. We use a builder context as the base image jimschubert/8-jdk-alpine-mvn:bash-2.0 is not included in the final image, allowing our released images to be 78MB compressed / 138MB uncompressed or smaller.

If you are using one of those released images I'd also be interested in understanding how or why your CI has flagged them with CVEs because as you can see by image history they have no details about the builder image:

docker history --no-trunc --format '{{.CreatedBy}}' openapitools/openapi-generator-cli
/bin/sh -c #(nop)  LABEL org.opencontainers.image.version=
/bin/sh -c #(nop)  LABEL org.opencontainers.image.title=openapi-generator-cli
/bin/sh -c #(nop)  LABEL org.opencontainers.image.revision=66e08d3b69b74ba66a4c5668530cb2653736d9d5
/bin/sh -c #(nop)  LABEL org.opencontainers.image.created=2020-08-04T17:06:47Z
/bin/sh -c #(nop)  CMD ["help"]
/bin/sh -c #(nop)  ENTRYPOINT ["docker-entrypoint.sh"]
/bin/sh -c #(nop) COPY file:727b262d7466faa5949730335b32c8d97c025492626761af1fe470f83df28aea in /usr/local/bin/
/bin/sh -c #(nop) ADD file:79598033ba2e6ca88388c64ce8eb666762e5d645238d3d7af3619b123d53b924 in /opt/openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar
/bin/sh -c apk add --no-cache bash
/bin/sh -c set -x  && apk add --no-cache   openjdk8-jre="$JAVA_ALPINE_VERSION"  && [ "$JAVA_HOME" = "$(docker-java-home)" ]
/bin/sh -c #(nop)  ENV JAVA_ALPINE_VERSION=8.111.14-r0
/bin/sh -c #(nop)  ENV JAVA_VERSION=8u111
/bin/sh -c #(nop)  ENV PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/lib/jvm/java-1.8-openjdk/jre/bin:/usr/lib/jvm/java-1.8-openjdk/bin
/bin/sh -c #(nop)  ENV JAVA_HOME=/usr/lib/jvm/java-1.8-openjdk/jre
/bin/sh -c {   echo '#!/bin/sh';   echo 'set -e';   echo;   echo 'dirname "$(dirname "$(readlink -f "$(which javac || which java)")")"';  } > /usr/local/bin/docker-java-home  && chmod +x /usr/local/bin/docker-java-home
/bin/sh -c #(nop)  ENV LANG=C.UTF-8
/bin/sh -c #(nop) ADD file:3df55c321c1c8d73f22bc69240c0764290d6cb293da46ba8f94ed25473fb5853 in /

@donbowman
Copy link
Contributor Author

donbowman commented Aug 5, 2020

alpine doesn't publish CVE, which is a huge red flag. Its one of the reasons we don't use it (the other is the poor performance and poor install time w/ python).
when i looked into it, the image on docker hub had a many year old alpine base in it. It is also based on jdk8 which runs poorly in Kubernetes due to lack of cgroup support.

$ docker run --rm -it --entrypoint=sh openapitools/openapi-generator-cli:v4.3.1
/ # cat /etc/alpine-release 
3.4.6

~ 2016 release. All the apk in it are very old and full of security flaws.

docker-in-docker is a terrible thing to use: the security is awful, the performance is awful. Since the CI (self-hosted gitlab CI) runs in Kubernetes, its easy to run an arbitrary image, but I don't allow that image to turn around and run another one locally (there is no 2nd docker running).

As a consequence in the CI I don't run a 2nd docker image (thus the comment about the run-in-docker script, that is a desktop developer tool).

I look for projects to have maintained by the project team, built by their CI, images. It seems that is not the case here (since there is an out-of-band build process? and, the base image is not part of the project but is single maintainer?), so i sense i should locally maintain my own image?

Maybe i misunderstood, I assumed the Dockerfile in the root of this repo would automatically build on Dockerhub, and was the base of the openapitools/openapi-generator-cli:v4.3.1?

@donbowman
Copy link
Contributor Author

Oh i see, you have a .hub.cli.dockerfile etc. I should change those too.
I get it.

@jimschubert jimschubert added this to the 5.0.0 milestone Aug 24, 2020
@jimschubert
Copy link
Member

@donbowman thanks for running through your use case. I think these are excellent points!

Sorry for the late response.

@jimschubert jimschubert merged commit 5e79aaa into OpenAPITools:master Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants