github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.73k stars 1.55k forks source link

Version 2.13.4 has incorrect Java version discovery. #13541

Open brendandburns opened 1 year ago

brendandburns commented 1 year ago

I'm using the setup-java action in my config to set the Java version to 17.

Previous to the 2.13.4 release, this was detected correctly, but starting with the 2.13.4 releases, it is selecting the built-in Java 8 version that is in the github action image.

Output from 2.13.3:

/opt/hostedtoolcache/CodeQL/2.13.3-20230524/x64/codeql/codeql version --format=terse
  2.13.3
  /opt/hostedtoolcache/CodeQL/2.13.3-20230524/x64/codeql/java/tools/autobuild.sh
  Picked up JAVA_TOOL_OPTIONS:  -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false
  [2023-06-22 16:06:11] Build directory is .
  [2023-06-22 16:06:12] Java source version 8 detected
  [2023-06-22 16:06:12] [WARN] Could not find matching toolchain for source version 8
  [2023-06-22 16:06:12] [autobuild] > /home/runner/work/java/java/./mvnw clean package -f "pom.xml" -B -V -e -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip -DskipTests -Dmaven.test.skip.exec -Dlicense.skip=true -Drat.skip=true -Dspotless.check.skip=true  -t /home/runner/.m2/toolchains.xml
  [2023-06-22 16:06:12] [autobuild] Picked up JAVA_TOOL_OPTIONS:  -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false
  [2023-06-22 16:06:13] [autobuild] Apache Maven 3.8.5 (3599d3414f0[46](https://github.com/kubernetes-client/java/actions/runs/5347790168/jobs/9696787377#step:5:47)de2324203b78ddcf9b5e4388aa0)
  [2023-06-22 16:06:13] [autobuild] Maven home: /home/runner/.m2/wrapper/dists/apache-maven-3.8.5-bin/5i5jha092a3i37g0paqnfr15e0/apache-maven-3.8.5
  [2023-06-22 16:06:13] [autobuild] Java version: 17.0.7, vendor: Eclipse Adoptium, runtime: /usr/lib/jvm/temurin-17-jdk-amd64
  [2023-06-22 16:06:13] [autobuild] Default locale: en, platform encoding: UTF-8
  [2023-06-22 16:06:13] [autobuild] OS name: "linux", version: "5.15.0-1040-azure", arch: "amd64", family: "unix"

Output from 2.3.14:

/opt/hostedtoolcache/CodeQL/2.13.4-v2.13.4/x64/codeql/codeql version --format=terse
  2.13.4
  /opt/hostedtoolcache/CodeQL/2.13.4-v2.13.4/x64/codeql/java/tools/autobuild.sh
  Picked up JAVA_TOOL_OPTIONS:  -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false
  [2023-06-22 02:57:[51](https://github.com/kubernetes-client/java/actions/runs/5347790168/jobs/9696787377#step:5:52)] Build directory is .
  [2023-06-22 02:57:[52](https://github.com/kubernetes-client/java/actions/runs/5347790168/jobs/9696787377#step:5:53)] Discovered Java toolchain for version 17.0.0 at /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.7-7/x64 in JAVA_HOME_17_X64
  [2023-06-22 02:57:52] Discovered Java toolchain for version 8.0.0 at /usr/lib/jvm/temurin-8-jdk-amd64 in JAVA_HOME_8_X64
  [2023-06-22 02:57:52] Discovered Java toolchain for version 11.0.0 at /usr/lib/jvm/temurin-11-jdk-amd64 in JAVA_HOME_11_X64
  [2023-06-22 02:57:52] Setting Java version to 8.0.0 (GHA)
  [2023-06-22 02:57:52] Setting JAVA_HOME to /usr/lib/jvm/temurin-8-jdk-amd64
  [2023-06-22 02:57:52] Adding /usr/lib/jvm/temurin-8-jdk-amd64/bin to PATH
brendandburns commented 1 year ago

I fixed this by locking to 2.13.3 but that's not a long term solution.

https://github.com/kubernetes-client/java/pull/2716

mbg commented 1 year ago

Hi @brendandburns! Thank you for raising this. We have recently made some changes to CodeQL's ability to discover which versions of Java are installed in the build environment.

As you can see in the logs, both versions of CodeQL infer that the project wants to be built with Java 8 (more on that in a moment). The difference is that the old version of CodeQL couldn't find Java 8 installed, and therefore just stuck to the default. The new version of CodeQL can find Java 8 and therefore acts on its conclusion that Java 8 is best for the project.

For Maven-based projects, CodeQL looks at the pom.xml in the root directory of the project and searches a few elements for hints about what Java version should be used to build the project. One of the first locations it looks at is project/properties/java.version. In the case of this repository, the Java version specified there is 1.8 and so CodeQL concludes that this is the best Java version for the project.

So a simple fix here might be to just change that value in the root-level pom.xml to 17 instead of 1.8. Would that work for you?

brendandburns commented 1 year ago

Unfortunately, that doesn't work. We mostly build for java 8, but we have one sub-module pom that conditionally builds exclusively for Java 17, because it uses Spring 6 which only supports Java 17:

https://github.com/kubernetes-client/java/blob/master/spring-aot/pom.xml#L71

So in this case, infering the language from the base pom.xml doesn't work correctly for this project.

Additionally, I think this should be considered a breaking change, since we have something that worked in 2.13.3 and no longer works in 2.13.4

mbg commented 1 year ago

Thanks for the update that switching to Java 17 for the entire repository is not an option for you.

I would recommend that you change the CodeQL workflow to use a manual build, rather than rely on CodeQL's automatic build functionality. In essence, all the automatic build process currently does for you here is:

You can invoke Maven yourself in the CodeQL workflow in place of the automatic build step:

    - name: Setup Java 17
      uses: actions/setup-java@v3
      with:
        distribution: 'temurin'
        java-version: '17.0.x'

    # Initializes the CodeQL tools for scanning.
    - name: Initialize CodeQL
      uses: github/codeql-action/init@v2
      with:
        languages: ${{ matrix.language }}

    # This is what the CodeQL automatic build currently invokes
    - name: Build project
      run: ./mvnw clean package -f "pom.xml" -B -V -e -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip -DskipTests -Dmaven.test.skip.exec -Dlicense.skip=true -Drat.skip=true -Dspotless.check.skip=true

    - name: Perform CodeQL Analysis
      uses: github/codeql-action/analyze@v2

This will skip the Java version inference and generally give you more control over the build. If you wanted to, you could e.g. build using an older version of Java and exclude the spring-aot submodule instead, but the above will do what happened with older versions of CodeQL.

Another approach that I explored would be to teach Maven to actually use the configured Java versions for each project. You can do this by having a workflow which installs both Java 8 and 17:

    - name: Setup Java 8
      uses: actions/setup-java@v3
      with:
        distribution: 'temurin'
        java-version: 8

    - name: Setup Java 17
      uses: actions/setup-java@v3
      with:
        distribution: 'temurin'
        java-version: 17

    # Initializes the CodeQL tools for scanning.
    - name: Initialize CodeQL
      uses: github/codeql-action/init@v2
      with:
        languages: ${{ matrix.language }}

    - name: Autobuild
      uses: github/codeql-action/autobuild@v2

This results in a toolchains.xml file containing both Java versions. Maven can then be configured to detect these using the maven-toolchains-plugin:

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-toolchains-plugin</artifactId>
        <version>3.1.0</version>
        <configuration>
          <toolchains>
            <jdk>
              <version>8</version>
            </jdk>
            <jdk>
              <version>17</version>
            </jdk>
          </toolchains>
        </configuration>
        <executions>
          <execution>
            <goals>
              <goal>toolchain</goal>
            </goals>
          </execution>
        </executions>
      </plugin>

During the build, Maven will then automatically use the respective Java versions based on what is configured in the submodules' pom.xml files. However, I don't know whether this approach is feasible for you since Maven will now require both Java 8 and Java 17 to be present for each build and I can see that in your regular build process you are targeting Java 8, 11, or 17 exclusively for the entire repo, but exclude the spring-aot submodule from the build for Java 8 and 11.