Run precommit checks before checkstyle task #3727

cwperks · 2023-11-16T18:48:22Z

Description

This PR runs the License Header check with the checkstyle CI check to ensure that PRs contain licenses on top of new files. This PR also adds the license on 2 files where its missing today.

Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Maintenance

Issues Resolved

Resolves [BUG] Add precommit hook to the pipeline #3471
Resolves Execution failed for task ':licenseHeaders' while executing the build on latest release(2.11.0.0) as well on main branch for x86_64 architecture. #3691

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Craig Perkins <cwperx@amazon.com>

src/main/java/org/opensearch/security/filter/SecurityResponse.java

peternied

There are a number of other checks like similar to the license check such as jarHell are those not running either? Do you know what is missing?

build.gradle

cwperks · 2023-11-16T19:52:07Z

@peternied I'm looking into it further. There's quite a few differences in ./gradlew assemble --dry-run between this repo and other plugins like job-scheduler. I'm not sure why there's a difference.

Job Scheduler

➜  job-scheduler git:(main) ✗ ./gradlew assemble --dry-run --no-build-cache
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 8.1.1
  OS Info               : Mac OS X 13.6 (aarch64)
  JDK Version           : 17 (Amazon Corretto JDK)
  JAVA_HOME             : /Users/cwperx/.sdkman/candidates/java/17.0.9-amzn
  Random Testing Seed   : F206F608D0543AFE
  In FIPS 140 mode      : false
=======================================
:opensearch-job-scheduler-spi:compileJava SKIPPED
:opensearch-job-scheduler-spi:processResources SKIPPED
:opensearch-job-scheduler-spi:classes SKIPPED
:opensearch-job-scheduler-spi:shadowJar SKIPPED
:compileJava SKIPPED
:processResources SKIPPED
:classes SKIPPED
:jar SKIPPED
:copyPluginPropertiesTemplate SKIPPED
:pluginProperties SKIPPED
:bundlePlugin SKIPPED
:generatePomFileForNebulaPublication SKIPPED
:generatePomFileForPluginZipPublication SKIPPED
:generatePom SKIPPED
:javadoc SKIPPED
:javadocJar SKIPPED
:sourcesJar SKIPPED
:assemble SKIPPED
:compileTestJava SKIPPED
:processTestResources SKIPPED
:testClasses SKIPPED
:compileJavaRestTestJava SKIPPED
:forbiddenApisResources SKIPPED
:processJavaRestTestResources SKIPPED
:javaRestTestClasses SKIPPED
:forbiddenApisJavaRestTest SKIPPED
:forbiddenApisMain SKIPPED
:forbiddenApisTest SKIPPED
:forbiddenApis SKIPPED
:dependencyLicenses SKIPPED
:filepermissions SKIPPED
:forbiddenPatterns SKIPPED
:jarHell SKIPPED
:licenseHeaders SKIPPED
:loggerUsageCheck SKIPPED
:testingConventions SKIPPED
:thirdPartyAuditResources SKIPPED
:thirdPartyAudit SKIPPED
:validateNebulaPom SKIPPED
:validatePluginZipPom SKIPPED
:validatePom SKIPPED
:precommit SKIPPED
:integTest SKIPPED
:test SKIPPED
:jacocoTestReport SKIPPED
:javaRestTest SKIPPED
:spotlessInternalRegisterDependencies SKIPPED
:spotlessJava SKIPPED
:spotlessJavaCheck SKIPPED
:spotlessLicense SKIPPED
:spotlessLicenseCheck SKIPPED
:spotlessMisc SKIPPED
:spotlessMiscCheck SKIPPED
:spotlessCheck SKIPPED
:check SKIPPED
:build SKIPPED
:opensearch-job-scheduler-sample-extension:compileJava SKIPPED
:opensearch-job-scheduler-sample-extension:processResources SKIPPED
:opensearch-job-scheduler-sample-extension:classes SKIPPED
:opensearch-job-scheduler-sample-extension:jar SKIPPED
:opensearch-job-scheduler-sample-extension:copyPluginPropertiesTemplate SKIPPED
:opensearch-job-scheduler-sample-extension:pluginProperties SKIPPED
:opensearch-job-scheduler-sample-extension:bundlePlugin SKIPPED
:opensearch-job-scheduler-sample-extension:generatePomFileForNebulaPublication SKIPPED
:opensearch-job-scheduler-sample-extension:generatePom SKIPPED
:opensearch-job-scheduler-sample-extension:javadoc SKIPPED
:opensearch-job-scheduler-sample-extension:javadocJar SKIPPED
:opensearch-job-scheduler-sample-extension:sourcesJar SKIPPED
:opensearch-job-scheduler-sample-extension:assemble SKIPPED
:opensearch-job-scheduler-spi:jar SKIPPED
:opensearch-job-scheduler-spi:assemble SKIPPED

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/8.1.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 387ms

Security

➜  security git:(license-header-check) ./gradlew assemble --dry-run
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 8.4
  OS Info               : Mac OS X 13.6 (aarch64)
  JDK Version           : 17 (Amazon Corretto JDK)
  JAVA_HOME             : /Users/cwperx/.sdkman/candidates/java/17.0.9-amzn
  Random Testing Seed   : 9049D5E0A2BFEC2B
  In FIPS 140 mode      : false
=======================================
:compileJava SKIPPED
:processResources SKIPPED
:classes SKIPPED
:jar SKIPPED
:copyPluginPropertiesTemplate SKIPPED
:pluginProperties SKIPPED
:bundlePlugin SKIPPED
:generatePomFileForMavenPublication SKIPPED
:generatePomFileForNebulaPublication SKIPPED
:generatePomFileForPluginZipPublication SKIPPED
:generatePom SKIPPED
:javadoc SKIPPED
:javadocJar SKIPPED
:sourcesJar SKIPPED
:assemble SKIPPED

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.4/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 1s

peternied · 2023-11-16T20:06:28Z

Here we go, used https://stackoverflow.com/a/34880233/533057 - looks like precommit is another kind of check we should add to the hygiene GHA - what do you think?

% ./gradlew build taskTree                                                                                                                                                                   /local/home/petern/git/security
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 8.4
  OS Info               : Linux 5.4.259-180.361.amzn2int.x86_64 (amd64)
  JDK Version           : 17 (OpenJDK)
  JAVA_HOME             : /local/home/petern/jdk-17.0.1
  Random Testing Seed   : FE779BBF87E47EC5
  In FIPS 140 mode      : false
=======================================

> Task :taskTree

------------------------------------------------------------
Root project 'opensearch-security' - Provide access control related features for OpenSearch
------------------------------------------------------------

:build
+--- :assemble
|    +--- :bundlePlugin
|    |    +--- :jar
|    |    |    +--- :classes
|    |    |    |    +--- :compileJava
|    |    |    |    \--- :processResources
|    |    |    \--- :compileJava *
|    |    \--- :pluginProperties
|    |         \--- :copyPluginPropertiesTemplate
|    +--- :generatePom
|    |    +--- :generatePomFileForMavenPublication
|    |    +--- :generatePomFileForNebulaPublication
|    |    \--- :generatePomFileForPluginZipPublication
|    +--- :jar *
|    +--- :javadocJar
|    |    \--- :javadoc
|    \--- :sourcesJar
\--- :check
     +--- :checkstyleIntegrationTest
     |    +--- :compileIntegrationTestJava
     |    |    +--- :classes *
     |    |    \--- :compileJava *
     |    \--- :integrationTestClasses
     |         +--- :compileIntegrationTestJava *
     |         \--- :processIntegrationTestResources
     +--- :checkstyleMain
     |    +--- :classes *
     |    \--- :compileJava *
     +--- :checkstyleTest
     |    +--- :classes *
     |    +--- :compileJava *
     |    +--- :compileTestJava
     |    |    +--- :classes *
     |    |    \--- :compileJava *
     |    +--- :pluginProperties *
     |    \--- :testClasses
     |         +--- :compileTestJava *
     |         +--- :pluginProperties *
     |         \--- :processTestResources
     +--- :forbiddenApis
     |    +--- :forbiddenApisIntegrationTest
     |    |    +--- :compileIntegrationTestJava *
     |    |    +--- :forbiddenApisResources
     |    |    \--- :integrationTestClasses *
     |    +--- :forbiddenApisMain
     |    |    +--- :classes *
     |    |    +--- :compileJava *
     |    |    \--- :forbiddenApisResources *
     |    \--- :forbiddenApisTest
     |         +--- :classes *
     |         +--- :compileJava *
     |         +--- :compileTestJava *
     |         +--- :forbiddenApisResources *
     |         +--- :pluginProperties *
     |         \--- :testClasses *
     +--- :integTest
     |    +--- :classes *
     |    +--- :compileJava *
     |    +--- :compileTestJava *
     |    +--- :pluginProperties *
     |    \--- :testClasses *
     +--- :integrationTest
     |    +--- :classes *
     |    +--- :compileIntegrationTestJava *
     |    +--- :compileJava *
     |    \--- :integrationTestClasses *
     +--- :javadoc *
     +--- :precommit
     |    +--- :classes *
     |    +--- :dependencyLicenses
     |    +--- :filepermissions
     |    +--- :forbiddenApis *
     |    +--- :forbiddenPatterns
     |    +--- :jarHell
     |    |    +--- :classes *
     |    |    +--- :compileJava *
     |    |    +--- :compileTestJava *
     |    |    +--- :pluginProperties *
     |    |    \--- :testClasses *
     |    +--- :licenseHeaders
     |    +--- :loggerUsageCheck
     |    |    +--- :compileJava *
     |    |    \--- :compileTestJava *
     |    +--- :testClasses *
     |    +--- :testingConventions
     |    |    +--- :classes *
     |    |    +--- :compileIntegrationTestJava *
     |    |    +--- :compileJava *
     |    |    +--- :compileTestJava *
     |    |    +--- :pluginProperties *
     |    |    \--- :testClasses *
     |    +--- :thirdPartyAudit
     |    |    \--- :thirdPartyAuditResources
     |    \--- :validatePom
     |         +--- :validateMavenPom
     |         |    +--- :generatePomFileForMavenPublication *
     |         |    +--- :generatePomFileForNebulaPublication *
     |         |    \--- :generatePomFileForPluginZipPublication *
     |         +--- :validateNebulaPom
     |         |    +--- :generatePomFileForMavenPublication *
     |         |    +--- :generatePomFileForNebulaPublication *
     |         |    \--- :generatePomFileForPluginZipPublication *
     |         \--- :validatePluginZipPom
     |              +--- :generatePomFileForMavenPublication *
     |              +--- :generatePomFileForNebulaPublication *
     |              \--- :generatePomFileForPluginZipPublication *
     +--- :spotbugsIntegrationTest
     |    +--- :compileIntegrationTestJava *
     |    \--- :integrationTestClasses *
     +--- :spotbugsMain
     |    +--- :classes *
     |    \--- :compileJava *
     +--- :spotbugsTest
     |    +--- :classes *
     |    +--- :compileJava *
     |    +--- :compileTestJava *
     |    +--- :pluginProperties *
     |    \--- :testClasses *
     +--- :spotlessCheck
     |    +--- :spotlessJavaCheck
     |    |    \--- :spotlessJava
     |    |         \--- :spotlessInternalRegisterDependencies
     |    \--- :spotlessMiscCheck
     |         \--- :spotlessMisc
     |              \--- :spotlessInternalRegisterDependencies *
     \--- :test
          +--- :classes *
          +--- :compileJava *
          +--- :compileTestJava *
          +--- :copyExtraTestResources
          |    \--- :testClasses *
          +--- :pluginProperties *
          \--- :testClasses *


(*) - subtree omitted (printed previously)
Add --repeat to allow printing a subtree of the same task more than once.

To see task dependency tree for a specific task, run gradlew <project-path>:<task> <project-path>:taskTree [--depth <depth>] [--with-inputs] [--with-outputs] [--repeat]
Executions of all tasks except for taskTree are skipped. They are used for building the task graph only.
For example, try running gradlew :build :taskTree

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.4/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 4s
1 actionable task: 1 executed

Signed-off-by: Craig Perkins <cwperx@amazon.com>

cwperks · 2023-11-16T20:20:41Z

@peternied I just modified this PR so that the entire precommit runs with the checkstyle CI check.

peternied

Could you create tracking issues for those flaky tests?

cwperks · 2023-11-16T21:44:01Z

@peternied Sure I just created issues to track the flaky tests.

codecov · 2023-11-17T01:51:08Z

Codecov Report

Merging #3727 (bea9645) into main (ee66199) will increase coverage by 1.36%.
Report is 3 commits behind head on main.
The diff coverage is 81.81%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3727      +/-   ##
==========================================
+ Coverage   64.88%   66.24%   +1.36%     
==========================================
  Files         292      292              
  Lines       20776    20805      +29     
  Branches     3409     3411       +2     
==========================================
+ Hits        13481    13783     +302     
+ Misses       5606     5323     -283     
- Partials     1689     1699      +10

Files	Coverage Δ
.../org/opensearch/security/auth/BackendRegistry.java	`77.92% <100.00%> (+15.38%)`	⬆️
...org/opensearch/security/filter/NettyAttribute.java	`81.81% <ø> (ø)`
...g/opensearch/security/filter/SecurityResponse.java	`96.22% <100.00%> (+3.36%)`	⬆️
...org/opensearch/security/httpclient/HttpClient.java	`86.13% <ø> (ø)`
...curity/securityconf/impl/AllowlistingSettings.java	`75.00% <100.00%> (ø)`
...curity/securityconf/impl/WhitelistingSettings.java	`75.00% <100.00%> (ø)`
...dlic/auth/http/saml/AuthTokenProcessorHandler.java	`50.28% <50.00%> (ø)`
...opensearch/security/filter/SecurityRestFilter.java	`83.84% <50.00%> (+14.61%)`	⬆️
...ic/auth/http/kerberos/HTTPSpnegoAuthenticator.java	`0.00% <0.00%> (ø)`

... and 34 files with indirect coverage changes

Run license header check before checkstyle

cc42ac0

Signed-off-by: Craig Perkins <cwperx@amazon.com>

cwperks requested review from cliu123, DarshitChanpura, davidlago, peternied, RyanL1997, stephen-crawford, reta and willyborankin as code owners November 16, 2023 18:48

cwperks commented Nov 16, 2023

View reviewed changes

src/main/java/org/opensearch/security/filter/SecurityResponse.java Show resolved Hide resolved

peternied reviewed Nov 16, 2023

View reviewed changes

build.gradle Outdated Show resolved Hide resolved

Run entire precommit with checkstyle checks

bea9645

Signed-off-by: Craig Perkins <cwperx@amazon.com>

peternied changed the title ~~Run license header check before checkstyle~~ Run precommit checks before checkstyle task Nov 16, 2023

peternied approved these changes Nov 16, 2023

View reviewed changes

peternied mentioned this pull request Nov 17, 2023

Execution failed for task ':licenseHeaders' while executing the build on latest release(2.11.0.0) as well on main branch for x86_64 architecture. #3691

Closed

willyborankin approved these changes Nov 17, 2023

View reviewed changes

willyborankin merged commit 9be1b27 into opensearch-project:main Nov 17, 2023
79 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run precommit checks before checkstyle task #3727

Run precommit checks before checkstyle task #3727

cwperks commented Nov 16, 2023 •

edited by peternied

Loading

peternied left a comment

cwperks commented Nov 16, 2023 •

edited

Loading

peternied commented Nov 16, 2023

cwperks commented Nov 16, 2023

peternied left a comment

cwperks commented Nov 16, 2023

codecov bot commented Nov 17, 2023

Run precommit checks before checkstyle task #3727

Run precommit checks before checkstyle task #3727

Conversation

cwperks commented Nov 16, 2023 • edited by peternied Loading

Description

Issues Resolved

Check List

peternied left a comment

Choose a reason for hiding this comment

cwperks commented Nov 16, 2023 • edited Loading

peternied commented Nov 16, 2023

cwperks commented Nov 16, 2023

peternied left a comment

Choose a reason for hiding this comment

cwperks commented Nov 16, 2023

codecov bot commented Nov 17, 2023

Codecov Report

cwperks commented Nov 16, 2023 •

edited by peternied

Loading

cwperks commented Nov 16, 2023 •

edited

Loading