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

GH-4304 improve docker image security #4305

Merged
merged 4 commits into from
Dec 9, 2022

Conversation

hmottestad
Copy link
Contributor

GitHub issue resolved: #4304

Briefly describe the changes proposed in this PR:


PR Author Checklist (see the contributor guidelines for more details):

  • my pull request is self-contained
  • I've added tests for the changes I made
  • I've applied code formatting (you can use mvn process-resources to format from the command line)
  • I've squashed my commits where necessary
  • every commit message starts with the issue number (GH-xxxx) followed by a meaningful description of the change

@hmottestad
Copy link
Contributor Author

hmottestad commented Dec 8, 2022

@hmottestad
Copy link
Contributor Author

Before:
image

After:
image

@barthanssens
Copy link
Contributor

Hmz, looks like you've accidentally committed fixes for (white space) formatting issues in a series of unrelated files

…compiling in parallel and upgrading some plugins
@hmottestad hmottestad force-pushed the GH-4304-improve-docker-image-security branch from 2973761 to ea341f7 Compare December 8, 2022 17:11
Comment on lines -653 to +655
<version>3.8.1</version>
<version>3.10.1</version>
<configuration>
<fork>true</fork>
<fork>false</fork>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This improves the build performance. It was taking 7 minutes to do a clean build, and that was very annoying when I was testing my actual changes, now it takes 2 1/2 minutes.

@@ -22,6 +22,7 @@ MVN_VERSION=$(xmllint --xpath "//*[local-name()='project']/*[local-name()='versi
echo "Building with Maven"
mvn clean
mvn -T 2C formatter:format impsort:sort && mvn xml-format:xml-format
mvn -T 2C compile -P-use-sonatype-snapshots package -DskipTests -Dmaven.javadoc.skip=true -Dformatter.skip=true -Dimpsort.skip=true -Dxml-format.skip=true -Djapicmp.skip -Denforcer.skip=true -Dbuildnumber.plugin.phase=none -Ddefault-jar.phase=none -Danimal.sniffer.skip=true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This improves the build performance. It was taking 7 minutes to do a clean build, and that was very annoying when I was testing my actual changes, now it takes 2 1/2 minutes.

Comment on lines -52 to -57
<version>3.8.1</version>
<configuration>
<fork>true</fork>
<release>11</release>
<encoding>utf8</encoding>
</configuration>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a duplicate of the root pom.

@hmottestad
Copy link
Contributor Author

Hmz, looks like you've accidentally committed fixes for (white space) formatting issues in a series of unrelated files

I got a bit ahead of myself by upgrading a bunch of build time dependencies while I was at it, but then I realise I would be spending the entire weekend filing CQs for them...so I reverted those changes. The only "unrelated" thing I'm keeping is some performance improvements to the maven build.

docker/Dockerfile Outdated Show resolved Hide resolved
Comment on lines -38 to +39
docker-compose build
docker-compose build --pull --no-cache
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pulls the base image and also disables caching (which would cache the apt-get upgrade command).

@barthanssens
Copy link
Contributor

barthanssens commented Dec 8, 2022

I got a bit ahead of myself by upgrading a bunch of build time dependencies while I was at it, but then I realise I would be spending the entire weekend filing CQs for them...so I reverted those changes.

Have you tested the Eclipse dash-license tool ? Filing CQs is now a (mostly) automated process... a much nicer experience than manually adding CQs and attaching jars via the old website ...

@hmottestad
Copy link
Contributor Author

hmottestad commented Dec 8, 2022

I got a bit ahead of myself by upgrading a bunch of build time dependencies while I was at it, but then I realise I would be spending the entire weekend filing CQs for them...so I reverted those changes.

Have you tested the Eclipse dash-license tool ? Filing CQs is now a (mostly) automated process... a much nicer experience than manually adding CQs and attaching jars via the old website ...

It didn't pick up any of the maven plugins :(

@abrokenjester
Copy link
Contributor

Fwiw I think we don't really have to file CQs for maven plugins anymore. We did so in the past, but it later turned out that we have a lot of discretion there. Especially as we don't actually distribute these, they're just part of our build env.

@hmottestad hmottestad merged commit af6e7cb into main Dec 9, 2022
@hmottestad hmottestad deleted the GH-4304-improve-docker-image-security branch December 9, 2022 09:12
@hmottestad hmottestad changed the title Gh 4304 improve docker image security GH-4304 improve docker image security Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve docker image security
3 participants