From cd45e7838e24327fb437a356acd83325b88fe706 Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Fri, 1 Sep 2023 15:54:15 +0200 Subject: [PATCH] Fix checkstyle (#3283) ### Description Fix checkstyle. Now it works as expected. The problem is that chectyle works only with one config file. It does not support multiple files except per-defined files like `suppressions.xml`. So the previous config effectively replaced `sun_checks.xml` and validated only `System.out.println` lines in `tools`. Since `println_checks.xml` replaced the main file we did not notice that new version of checktyle removed `PrintlnModule` from the code base. Changes: - All code related to checking `tools` folder was moved in the main file - Renamed the `sun_...xml` file to the `checktyle.xml` which is default settings for checkstyle so we can track changes in it - Set the latest version for `checktyle` so it can validate new JDK features as well - Fixed problematic files which checkstyle highlighted ### Issues Resolved #3260 ### Testing `System.out.println` I tested manually all I can say it works :-) ### 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](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). Signed-off-by: Andrey Pleskach --- build.gradle | 4 ++-- checkstyle/{sun_checks.xml => checkstyle.xml} | 18 ++++++++++++++-- checkstyle/println_checks.xml | 21 ------------------- .../opensearch/security/util/KeyUtils.java | 7 ++++++- .../security/authtoken/jwt/JwtVendorTest.java | 12 ++++++----- .../PrivilegesEvaluatorUnitTest.java | 3 ++- 6 files changed, 33 insertions(+), 32 deletions(-) rename checkstyle/{sun_checks.xml => checkstyle.xml} (92%) delete mode 100644 checkstyle/println_checks.xml diff --git a/build.gradle b/build.gradle index a8b4eced0d..76124bcf02 100644 --- a/build.gradle +++ b/build.gradle @@ -322,8 +322,8 @@ jacocoTestReport { } checkstyle { - configFile file("checkstyle/sun_checks.xml") - configFile file("checkstyle/println_checks.xml") + toolVersion "10.3.3" + configDirectory.set(rootProject.file("checkstyle/")) } opensearchplugin { diff --git a/checkstyle/sun_checks.xml b/checkstyle/checkstyle.xml similarity index 92% rename from checkstyle/sun_checks.xml rename to checkstyle/checkstyle.xml index 524ded931e..4484ea4e04 100644 --- a/checkstyle/sun_checks.xml +++ b/checkstyle/checkstyle.xml @@ -19,7 +19,7 @@ To suppress certain violations please review suppression filters. Finally, it is worth reading the documentation. --> - + + + + + + + @@ -188,6 +195,14 @@ + + + + + + + + @@ -222,5 +237,4 @@ - diff --git a/checkstyle/println_checks.xml b/checkstyle/println_checks.xml deleted file mode 100644 index 6fd2f7aff4..0000000000 --- a/checkstyle/println_checks.xml +++ /dev/null @@ -1,21 +0,0 @@ - - - - - - - - - - - - - - - - - - - diff --git a/src/main/java/org/opensearch/security/util/KeyUtils.java b/src/main/java/org/opensearch/security/util/KeyUtils.java index 4aebf0cb12..4b3aafbbc6 100644 --- a/src/main/java/org/opensearch/security/util/KeyUtils.java +++ b/src/main/java/org/opensearch/security/util/KeyUtils.java @@ -18,7 +18,12 @@ import org.opensearch.SpecialPermission; import org.opensearch.core.common.Strings; -import java.security.*; +import java.security.AccessController; +import java.security.Key; +import java.security.KeyFactory; +import java.security.NoSuchAlgorithmException; +import java.security.PrivilegedAction; +import java.security.PublicKey; import java.security.spec.InvalidKeySpecException; import java.security.spec.X509EncodedKeySpec; import java.util.Base64; diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java index 1322777cac..aa8faa284d 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -11,10 +11,6 @@ package org.opensearch.security.authtoken.jwt; -import java.util.List; -import java.util.Optional; -import java.util.function.LongSupplier; - import org.apache.commons.lang3.RandomStringUtils; import org.apache.cxf.rs.security.jose.jwk.JsonWebKey; import org.apache.cxf.rs.security.jose.jws.JwsJwtCompactConsumer; @@ -24,6 +20,10 @@ import org.opensearch.common.settings.Settings; import org.opensearch.security.support.ConfigConstants; +import java.util.List; +import java.util.Optional; +import java.util.function.LongSupplier; + public class JwtVendorTest { @Test @@ -99,7 +99,9 @@ public void testCreateJwtWithRoleSecurityMode() throws Exception { Settings settings = Settings.builder() .put("signing_key", "abc123") .put("encryption_key", claimsEncryptionKey) - .put(ConfigConstants.EXTENSIONS_BWC_PLUGIN_MODE, true) + // CS-SUPPRESS-SINGLE: RegexpSingleline get Extensions Settings + .put(ConfigConstants.EXTENSIONS_BWC_PLUGIN_MODE, "true") + // CS-ENFORCE-SINGLE .build(); Long expectedExp = currentTime.getAsLong() + expirySeconds; diff --git a/src/test/java/org/opensearch/security/privileges/PrivilegesEvaluatorUnitTest.java b/src/test/java/org/opensearch/security/privileges/PrivilegesEvaluatorUnitTest.java index 1b1874a106..811c817b65 100644 --- a/src/test/java/org/opensearch/security/privileges/PrivilegesEvaluatorUnitTest.java +++ b/src/test/java/org/opensearch/security/privileges/PrivilegesEvaluatorUnitTest.java @@ -17,7 +17,8 @@ import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.opensearch.security.privileges.PrivilegesEvaluator.*; +import static org.opensearch.security.privileges.PrivilegesEvaluator.DNFOF_MATCHER; +import static org.opensearch.security.privileges.PrivilegesEvaluator.isClusterPerm; public class PrivilegesEvaluatorUnitTest {