Merge remote-tracking branch 'upstream/main' into integ-geopoint

Signed-off-by: Yury-Fridlyand <[email protected]>

.github/workflows/backport.yml

@@ -7,6 +7,7 @@ on:
 
 jobs:
   backport:
+    if: github.event.pull_request.merged == true
     runs-on: ubuntu-latest
     permissions:
       contents: write
@@ -22,7 +23,8 @@ jobs:
           installation_id: 22958780
 
       - name: Backport
-        uses: VachaShah/backport@v1.1.4
+        uses: VachaShah/backport@v2.2.0
         with:
           github_token: ${{ steps.github_app_token.outputs.token }}
-          branch_name: backport/backport-${{ github.event.number }}
+          head_template: backport/backport-<%= number %>-to-<%= base %>
+          failure_labels: backport-failed

.github/workflows/integ-tests-with-security.yml

@@ -0,0 +1,43 @@
+name: Security Plugin IT
+
+on:
+  pull_request:
+  push:
+    branches-ignore:
+      - 'dependabot/**'
+    paths:
+      - 'integ-test/**'
+      - '.github/workflows/integ-tests-with-security.yml'
+
+jobs:
+  security-it:
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ ubuntu-latest, windows-latest, macos-latest ]
+        java: [ 11, 17 ]
+
+    runs-on: ${{ matrix.os }}
+
+    steps:
+    - uses: actions/checkout@v3
+
+    - name: Set up JDK ${{ matrix.java }}
+      uses: actions/setup-java@v3
+      with:
+        distribution: 'temurin'
+        java-version: ${{ matrix.java }}
+
+    - name: Build with Gradle
+      run: ./gradlew integTestWithSecurity
+
+    - name: Upload test reports
+      if: ${{ always() }}
+      uses: actions/upload-artifact@v2
+      continue-on-error: true
+      with:
+        name: test-reports-${{ matrix.os }}-${{ matrix.java }}
+        path: |
+          integ-test/build/reports/**
+          integ-test/build/testclusters/*/logs/*
+          integ-test/build/testclusters/*/config/*

.github/workflows/sql-test-and-build-workflow.yml

@@ -11,7 +11,6 @@ on:
       - '!sql-jdbc/**'
       - '**gradle*'
       - '**lombok*'
-      - '**checkstyle*'
       - 'integ-test/**'
       - '**/*.jar'
       - '**/*.pom'

.github/workflows/sql-test-workflow.yml

@@ -31,24 +31,16 @@ jobs:
     - name: Run tests
       id: tests
       run: |
-        # checkstyle
-        ./gradlew :opensearch:checkstyleMain              || echo "* Checkstyle failed for opensearch/src" > report.log
-        ./gradlew :opensearch:checkstyleTest              || echo "* Checkstyle failed for opensearch/test" >> report.log
-        ./gradlew :sql:checkstyleMain                     || echo "* Checkstyle failed for sql/src" >> report.log
-        ./gradlew :sql:checkstyleTest                     || echo "* Checkstyle failed for sql/test" >> report.log
-        ./gradlew :ppl:checkstyleMain                     || echo "* Checkstyle failed for ppl/src" >> report.log
-        ./gradlew :ppl:checkstyleTest                     || echo "* Checkstyle failed for ppl/test" >> report.log
-        ./gradlew :core:checkstyleMain                    || echo "* Checkstyle failed for core/src" >> report.log
-        ./gradlew :core:checkstyleTest                    || echo "* Checkstyle failed for core/test" >> report.log
-        ./gradlew :common:checkstyleMain                  || echo "* Checkstyle failed for common/src" >> report.log
-        ./gradlew :common:checkstyleTest                  || echo "* Checkstyle failed for common/test" >> report.log
-        ./gradlew :legacy:checkstyleMain                  || echo "* Checkstyle failed for legacy/src" >> report.log
-        ./gradlew :legacy:checkstyleTest                  || echo "* Checkstyle failed for legacy/test" >> report.log
-        ./gradlew :protocol:checkstyleMain                || echo "* Checkstyle failed for protocol/src" >> report.log
-        ./gradlew :protocol:checkstyleTest                || echo "* Checkstyle failed for protocol/test" >> report.log
-        ./gradlew :opensearch-sql-plugin:checkstyleMain   || echo "* Checkstyle failed for plugin/src" >> report.log
-        ./gradlew :opensearch-sql-plugin:checkstyleTest   || echo "* Checkstyle failed for plugin/test" >> report.log
-        # Add checkstyle for `integ-test` when fixed
+        # Spotless
+        ./gradlew :opensearch:spotlessCheck              || echo "* Spotless failed for opensearch" > report.log
+        ./gradlew :sql:spotlessCheck                     || echo "* Spotless failed for sql" >> report.log
+        ./gradlew :ppl:spotlessCheck                     || echo "* Spotless failed for ppl" >> report.log
+        ./gradlew :core:spotlessCheck                    || echo "* Spotless failed for core" >> report.log
+        ./gradlew :common:spotlessCheck                  || echo "* Spotless failed for common" >> report.log
+        ./gradlew :legacy:spotlessCheck                  || echo "* Spotless failed for legacy" >> report.log
+        ./gradlew :protocol:spotlessCheck                || echo "* Spotless failed for protocol" >> report.log
+        ./gradlew :opensearch-sql-plugin:spotlessCheck   || echo "* Spotless failed for plugin" >> report.log
+        ./gradlew :integ-test:spotlessCheck              || echo "* Spotless failed for integ-test" >> report.log
         # Unit tests
         ./gradlew :opensearch:test                        || echo "* Unit tests failed for opensearch" >> report.log
         ./gradlew :ppl:test                               || echo "* Unit tests failed for sql" >> report.log

DEVELOPER_GUIDE.rst

@@ -127,7 +127,6 @@ The plugin codebase is in standard layout of Gradle project::
    ├── THIRD-PARTY
    ├── build.gradle
    ├── config
-   │   └── checkstyle
    ├── docs
    │   ├── attributions.md
    │   ├── category.json
@@ -170,7 +169,6 @@ Here are sub-folders (Gradle modules) for plugin source code:
 Here are other files and sub-folders that you are likely to touch:
 
 - ``build.gradle``: Gradle build script.
-- ``config``: only Checkstyle configuration files for now.
 - ``docs``: documentation for developers and reference manual for users.
 - ``doc-test``: code that run .rst docs in ``docs`` folder by Python doctest library.
 
@@ -185,14 +183,31 @@ Note that other related project code has already merged into this single reposit
 Code Convention
 ---------------
 
-We’re integrated Checkstyle plugin into Gradle build: https://github.com/opensearch-project/sql/blob/main/config/checkstyle/google_checks.xml. So any violation will fail the build. You need to identify the offending code from Gradle error message and fix them and rerun the Gradle build. Here are the highlight of some Checkstyle rules:
+Java files in the OpenSearch codebase are formatted with the Eclipse JDT formatter, using the `Spotless Gradle <https://github.com/diffplug/spotless/tree/master/plugin-gradle>`_ plugin. This plugin is configured in the project  `./gradle.properties`.
 
-* 2 spaces indentation.
-* No line starts with tab character in source file.
-* Line width <= 100 characters.
-* Wildcard imports: You can enforce single import by configuring your IDE. Instructions for Intellij IDEA: https://www.jetbrains.com/help/idea/creating-and-optimizing-imports.html#disable-wildcard-imports.
-* Operator needs to wrap at next line.
+The formatting check can be run explicitly with::
 
+./gradlew spotlessCheck
+
+The code can be formatted with::
+
+./gradlew spotlessApply
+
+These tasks can also be run for specific modules, e.g.::
+
+./gradlew server:spotlessCheck
+
+For more information on the spotless for the OpenSearch project please see `https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md#java-language-formatting-guidelines <https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md#java-language-formatting-guidelines>`_.
+
+Java files are formatted using `Spotless <https://github.com/diffplug/spotless>`_ conforming to `Google Java Format <https://github.com/google/google-java-format>`_.
+   * - New line at end of file
+   * - No unused import statements
+   * - Fix import order to be alphabetical with static imports first (one block for static and one for non-static imports)
+   * - Max line length is 100 characters (does not apply to import statements)
+   * - Line spacing is 2 spaces
+   * - Javadocs should be properly formatted in accordance to `Javadoc guidelines <https://www.oracle.com/ca-en/technical-resources/articles/java/javadoc-tool.html>`_
+   * - Javadoc format can be maintained by wrapping javadoc with `<pre></pre>` HTML tags
+   * - Strings can be formatted on multiple lines with a `+` with the correct indentation for the string.
 
 Building and Running Tests
 ==========================
@@ -213,9 +228,7 @@ Most of the time you just need to run ./gradlew build which will make sure you p
    * - ./gradlew generateGrammarSource
      - (Re-)Generate ANTLR parser from grammar file.
    * - ./gradlew compileJava
-     - Compile all Java source files. 
-   * - ./gradlew checkstyle
-     - Run all checks according to Checkstyle configuration.
+     - Compile all Java source files.
    * - ./gradlew test
      - Run all unit tests.
    * - ./gradlew :integ-test:integTest

benchmarks/src/jmh/java/org/opensearch/sql/expression/operator/predicate/ComparisonOperatorBenchmark.java

@@ -38,7 +38,7 @@
 @Fork(value = 1)
 public class ComparisonOperatorBenchmark {
 
-  @Param(value = { "int", "string", "date" })
+  @Param(value = {"int", "string", "date"})
   private String testDataType;
 
   private final Map<String, ExprValue> params =
@@ -65,9 +65,7 @@ public void testGreaterOperator() {
 
   private void run(Function<Expression[], FunctionExpression> dsl) {
     ExprValue param = params.get(testDataType);
-    FunctionExpression func = dsl.apply(new Expression[] {
-        literal(param), literal(param)
-    });
+    FunctionExpression func = dsl.apply(new Expression[] {literal(param), literal(param)});
     func.valueOf();
   }
 }

build.gradle

@@ -62,7 +62,6 @@ buildscript {
 plugins {
     id 'nebula.ospackage' version "8.3.0"
     id 'java-library'
-    id 'checkstyle'
     id "io.freefair.lombok" version "6.4.0"
     id 'jacoco'
     id 'com.diffplug.spotless' version '6.19.0'
@@ -80,22 +79,21 @@ repositories {
     maven { url 'https://jitpack.io' }
 }
 
-// Spotless checks will be added as PRs are applied to resolve each style issue is approved.
 spotless {
     java {
-//        target fileTree('.') {
-//            include '**/*.java', 'src/*/java/**/*.java'
-//            exclude '**/build/**', '**/build-*/**'
-//        }
-//        importOrder()
+        target fileTree('.') {
+            include '**/*.java'
+            exclude '**/build/**', '**/build-*/**'
+        }
+        importOrder()
 //        licenseHeader("/*\n" +
 //                " * Copyright OpenSearch Contributors\n" +
 //                " * SPDX-License-Identifier: Apache-2.0\n" +
-//                " */\n\n\n")
-//        removeUnusedImports()
-//        trimTrailingWhitespace()
-//        endWithNewline()
-//        googleJavaFormat('1.17.0').reflowLongStrings().groupArtifact('com.google.googlejavaformat:google-java-format')
+//                " */\n\n")
+        removeUnusedImports()
+        trimTrailingWhitespace()
+        endWithNewline()
+        googleJavaFormat('1.17.0').reflowLongStrings().groupArtifact('com.google.googlejavaformat:google-java-format')
     }
 }
 
@@ -183,22 +181,6 @@ jacocoTestCoverageVerification {
 }
 check.dependsOn jacocoTestCoverageVerification
 
-// TODO: fix code style in main and test source code
-allprojects {
-    apply plugin: 'checkstyle'
-    checkstyle {
-        configFile rootProject.file("config/checkstyle/google_checks.xml")
-        toolVersion "10.3.2"
-        configProperties = [
-                "org.checkstyle.google.suppressionfilter.config": rootProject.file("config/checkstyle/suppressions.xml")]
-        ignoreFailures = false
-    }
-}
-checkstyle {
-    configFile file("config/checkstyle/checkstyle.xml")
-}
-checkstyleMain.ignoreFailures = false
-checkstyleTest.ignoreFailures = true
 
 configurations.all {
     resolutionStrategy.force 'junit:junit:4.13.2'

common/build.gradle

@@ -25,6 +25,7 @@
 plugins {
     id 'java-library'
     id "io.freefair.lombok"
+    id 'com.diffplug.spotless' version '6.19.0'
 }
 
 repositories {
@@ -62,4 +63,23 @@ configurations.all {
     resolutionStrategy.force "org.apache.httpcomponents:httpcore:4.4.13"
     resolutionStrategy.force "joda-time:joda-time:2.10.12"
     resolutionStrategy.force "org.slf4j:slf4j-api:1.7.36"
-}
+}
+
+spotless {
+    java {
+        target fileTree('.') {
+            include '**/*.java'
+            exclude '**/build/**', '**/build-*/**'
+        }
+        importOrder()
+// Needs https://github.com/opensearch-project/sql/issues/1893 to be addressed first
+//        licenseHeader("/*\n" +
+//                " * Copyright OpenSearch Contributors\n" +
+//                " * SPDX-License-Identifier: Apache-2.0\n" +
+//                " */\n\n")
+        removeUnusedImports()
+        trimTrailingWhitespace()
+        endWithNewline()
+        googleJavaFormat('1.17.0').reflowLongStrings().groupArtifact('com.google.googlejavaformat:google-java-format')
+    }
+}

common/src/main/java/org/opensearch/sql/common/antlr/CaseInsensitiveCharStream.java

@@ -3,7 +3,6 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-
 package org.opensearch.sql.common.antlr;
 
 import org.antlr.v4.runtime.CharStream;

common/src/main/java/org/opensearch/sql/common/antlr/SyntaxAnalysisErrorListener.java

@@ -3,7 +3,6 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-
 package org.opensearch.sql.common.antlr;
 
 import java.util.Locale;

common/src/main/java/org/opensearch/sql/common/antlr/SyntaxCheckException.java

@@ -3,7 +3,6 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-
 package org.opensearch.sql.common.antlr;
 
 public class SyntaxCheckException extends RuntimeException {

common/src/main/java/org/opensearch/sql/common/authinterceptors/AwsSigningInterceptor.java

@@ -31,15 +31,17 @@ public class AwsSigningInterceptor implements Interceptor {
   private static final Logger LOG = LogManager.getLogger();
 
   /**
-   * AwsSigningInterceptor which intercepts http requests
-   * and adds required headers for sigv4 authentication.
+   * AwsSigningInterceptor which intercepts http requests and adds required headers for sigv4
+   * authentication.
    *
    * @param awsCredentialsProvider awsCredentialsProvider.
    * @param region region.
    * @param serviceName serviceName.
    */
-  public AwsSigningInterceptor(@NonNull AWSCredentialsProvider awsCredentialsProvider,
-                               @NonNull String region, @NonNull String serviceName) {
+  public AwsSigningInterceptor(
+      @NonNull AWSCredentialsProvider awsCredentialsProvider,
+      @NonNull String region,
+      @NonNull String serviceName) {
     this.okHttpAwsV4Signer = new OkHttpAwsV4Signer(region, serviceName);
     this.awsCredentialsProvider = awsCredentialsProvider;
   }
@@ -48,25 +50,27 @@ public AwsSigningInterceptor(@NonNull AWSCredentialsProvider awsCredentialsProvi
   public Response intercept(Interceptor.Chain chain) throws IOException {
     Request request = chain.request();
 
-    DateTimeFormatter timestampFormat = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'")
-        .withZone(ZoneId.of("GMT"));
+    DateTimeFormatter timestampFormat =
+        DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'").withZone(ZoneId.of("GMT"));
 
-
-    Request.Builder newRequestBuilder = request.newBuilder()
-        .addHeader("x-amz-date", timestampFormat.format(ZonedDateTime.now()))
-        .addHeader("host", request.url().host());
+    Request.Builder newRequestBuilder =
+        request
+            .newBuilder()
+            .addHeader("x-amz-date", timestampFormat.format(ZonedDateTime.now()))
+            .addHeader("host", request.url().host());
 
     AWSCredentials awsCredentials = awsCredentialsProvider.getCredentials();
     if (awsCredentialsProvider instanceof STSAssumeRoleSessionCredentialsProvider) {
-      newRequestBuilder.addHeader("x-amz-security-token",
+      newRequestBuilder.addHeader(
+          "x-amz-security-token",
           ((STSAssumeRoleSessionCredentialsProvider) awsCredentialsProvider)
               .getCredentials()
               .getSessionToken());
     }
     Request newRequest = newRequestBuilder.build();
-    Request signed = okHttpAwsV4Signer.sign(newRequest,
-        awsCredentials.getAWSAccessKeyId(), awsCredentials.getAWSSecretKey());
+    Request signed =
+        okHttpAwsV4Signer.sign(
+            newRequest, awsCredentials.getAWSAccessKeyId(), awsCredentials.getAWSSecretKey());
     return chain.proceed(signed);
   }
-
 }

common/src/main/java/org/opensearch/sql/common/authinterceptors/BasicAuthenticationInterceptor.java

@@ -22,13 +22,11 @@ public BasicAuthenticationInterceptor(@NonNull String username, @NonNull String
     this.credentials = Credentials.basic(username, password);
   }
 
-
   @Override
   public Response intercept(Interceptor.Chain chain) throws IOException {
     Request request = chain.request();
-    Request authenticatedRequest = request.newBuilder()
-        .header("Authorization", credentials).build();
+    Request authenticatedRequest =
+        request.newBuilder().header("Authorization", credentials).build();
     return chain.proceed(authenticatedRequest);
   }
-
 }