eventmesh icon indicating copy to clipboard operation
eventmesh copied to clipboard

[ISSUE #4720] Modernize CI license check and Enable Dependabot

Open Pil0tXia opened this issue 4 months ago • 9 comments

Fixes #4720

Motivation

The list of artifacts recorded in known-dependencies.txt does not help the maintainer manage dependencies effectively. This list lacks a reference hierarchy of artifacts, and it's more practical to print the dependency tree using Gradle.

The purpose of check-dependencies.sh is to inspect the licenses of third-party dependencies, preventing developers from casually introducing untracked new artifacts. However, it can't prevent developers adding Apache 2.0 incompatible licenses.

The presence of known-dependencies.txt blocks Dependabot because it cannot update this file through CI. If our project does not keep up with new versions of dependencies for a long time, it will gradually fall behind and be submerged.

Therefore, I believe it is necessary to ~cancel the version checking of artifacts of~ remove known-dependencies.txt ~in~ and check-dependencies.sh, and introduce actions/dependency-review-action to check unsupported dependencies.

Why actions/dependency-review-action

  • apache/skywalking-eyes/dependency action : doesn't support Gradle dependency check; License header check superseded by Checkstyle.
  • https://github.com/jk1/Gradle-License-Report : It only supports adding Allowed licenses. According to https://spdx.org/licenses/ and ASF 3RD PARTY LICENSE POLICY, maintaining thousands of license variants is difficult. This approach has been deprecated after https://github.com/apache/eventmesh/pull/462.
  • actions/dependency-review-action : Supports adding denied licenses, which reduces maintaining pressure, as Apache Superset and HugeGraph did.

About allow-dependencies-licenses attribute

Due to the issue reported in https://github.com/actions/dependency-review-action/issues/670, for dependencies with multiple licenses, this action treats the OR separator as AND, meaning that if any of the licenses are listed in the deny-licenses list, they will be rejected. Ideally, for the OR separator, any dependency should not be rejected as long as at least one license is not listed in the deny-licenses list.

Therefore, I have temporarily added all existing dependencies with multiple licenses in the Repo to the exemptions of this action. Although this action only scans the modified dependencies in the pull request, these exemptions may never be used.

This issue is expected to be fixed in the next version of the action, at which point all exemptions can be removed.

Current implementation of this PR

Use checkDeniedLicense gradle task to check license, instead of dependency-review-action. dependency-review-action will be applied when upstream problems mentioned in https://github.com/apache/eventmesh/pull/4827#issuecomment-2074261234 is resolved.

dependency-review-action workflow files has been backed up to https://github.com/Pil0tXia/eventmesh/tree/pil0txia/action_4720_with-dependency-review-action.

Modifications

~Only the artifact name is recorded in known-dependencies.txt, the version number is no longer recorded.~

  • ~~Introduce https://github.com/actions/dependency-review-action~~ Use checkDeniedLicense gradle task in CI
  • Remove dependency check shell scripts
  • You can run ./gradlew printAllDependencyTrees > allDeps.log to get dependency trees of all EventMesh submodules.
  • Configure Dependabot and auto-approve its PR to reduce reviewers' pressure.

Documentation

  • Does this pull request introduce a new feature? (yes)
  • If yes, how is the feature documented? (not applicable)
  • If a feature is not applicable for documentation, explain why? See CI logs

Pil0tXia avatar Apr 11 '24 12:04 Pil0tXia

@Pil0tXia,

I ran a modified version of dependency-review on my personal fork (cf. this GHA run) to compare the dependency changes between your initial commit c0b36f487681c6c5f05a09738913e7454e515238 and master.

Instead of a deny list I used a simple allow list and the check passed:

allow-licenses: Apache-2.0, BSD-3-Clause, ISC, MIT

Remarks: as you can see from the the GH Actions log, only the Go and Github Action dependencies were checked, while the Java dependencies were completely ignored.

Apparently, if you use Gradle, you need to use a special action to submit Java dependencies to this repo's Dependency Graph. Following the deprecation notice in the Gradle Dependency Submission action, you should upgrade your gradle/gradle-build-action to version 3.

ppkarwasz avatar Apr 23 '24 07:04 ppkarwasz

@ppkarwasz

Thank you for providing valuable information! We have some old Dependabot PRs that tried to upgrade Gradle dependencies, and the Dependabot tab also reflects the correct entries. I wonder how dependabot made it to submit PRs without submitting dependency graph manually.

Pil0tXia avatar Apr 23 '24 09:04 Pil0tXia

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

I've already applied the recommended dependency graph submission approach for Gradle in this PR. Now the Gradle dependencies can be seen in Dependency Graph. I didn't upgrade gradle-build-action to v3 beacuse of this bug: https://github.com/gradle/actions/issues/196.

However, I've been making attempts in https://github.com/Pil0tXia/eventmesh/pull/1, but changed dependencies still can't be detected by dependency-review-action because of this known bug: https://github.com/actions/dependency-review-action/issues/632#issuecomment-2074252746. Therefore, this PR is temporarily blocked here.

Pil0tXia avatar Apr 24 '24 07:04 Pil0tXia

I've made some progress in dependency-submission and dependency-review-action. The gradle-build-action@v2 has been upgraded to gradle/actions/setup-gradle@v3 and gradle/actions/[email protected]. And dependency-review-action is now able to detect Gradle dependency changes in GH Actions, however it takes 30min~60min to detect dependency graph's change and it is considered as a bug to be fixed in a future release. So I think this PR does not have technical difficulties for now and can be updated in the next PR when upstream actions' bugs are fixed.

Pil0tXia avatar Apr 24 '24 13:04 Pil0tXia

I can write another checkDeniedLicenses Gradle task to check incompatible licenses in another PR before https://github.com/actions/dependency-review-action/issues/632 is resolved and after https://github.com/apache/eventmesh/pull/4831 is merged. It depends on the output of generateDistLicense task, search for 'incompatible-license-name' in tools/dist-license/LICENSE, and output the line when incompatible license detected.

Although it requires more work and may soon be deprecated after https://github.com/actions/dependency-review-action/issues/632 is resolved, it can save about 30min CI running time before https://github.com/actions/dependency-review-action/issues/632 is resolved.

Pil0tXia avatar Apr 24 '24 16:04 Pil0tXia

@xwm1992 PTAL~

image

The warning of aopalliance is due to this artifact itselves mistake. 1.0 is its only version.

The warning of mysql-connector-j will disappear after this artifact is removed from runtimeClasspath.

The warning of Public Domain can be resolved by upgrading xpp3:xpp3:1.1.4c to org.ogce:xpp3:1.1.6. (bundled by com.aliyun:tea-xml within DingTalk connector)

Pil0tXia avatar Apr 25 '24 11:04 Pil0tXia

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 15.94%. Comparing base (70f9892) to head (b3063e4). Report is 4 commits behind head on master.

:exclamation: Current head b3063e4 differs from pull request most recent head 7a87a69. Consider uploading reports for the commit 7a87a69 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4827      +/-   ##
============================================
+ Coverage     15.91%   15.94%   +0.03%     
- Complexity     1734     1735       +1     
============================================
  Files           897      897              
  Lines         31982    31943      -39     
  Branches       2737     2734       -3     
============================================
+ Hits           5089     5094       +5     
+ Misses        26413    26370      -43     
+ Partials        480      479       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Apr 25 '24 11:04 codecov-commenter

License check publish guide at https://github.com/apache/eventmesh-site/pull/218.

Pil0tXia avatar Apr 27 '24 06:04 Pil0tXia

@xwm1992 Conflicts resolved. PTAL~

Pil0tXia avatar May 08 '24 15:05 Pil0tXia