Windows build and test support for 1.3 (#2291)

Windows build and test support for 1.3 - Use most recent format of CI workflows from main - Backport #2206 - Supply workarounds for JDK8 incompatible APIs for Encoding / Pattern matching (Thanks @cwperks!) - Backport only freeport logic from #1638 - Backport #1758 - All updates to TestAuditlogImpl.java from main - #2180 - #1935 - #1920 - #1914 - #1829 - And Targeted test updates for ComplianceAuditlogTest and BasicAuditlogTest to keep up with TestAuditlogImpl.java updates Signed-off-by: Peter Nied <petern@amazon.com> Signed-off-by: Stephen Crawford <steecraw@amazon.com> Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Co-authored-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
opensearch-project · Dec 6, 2022 · 8e1adfa · 8e1adfa
1 parent 7eff736
commit 8e1adfa
Show file tree

Hide file tree

Showing 24 changed files with 741 additions and 491 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -8,55 +8,69 @@ env:
 jobs:
   build:
     name: build
-    runs-on: ubuntu-latest
     strategy:
       fail-fast: false
       matrix:
         jdk: [8, 11, 14]
+        platform: ["ubuntu-latest", "windows-latest"]
+    runs-on: ${{ matrix.platform }}
 
     steps:
 
-    - name: Set up JDK
+    - name: Set up JDK for build and test on 8 and 11
+      if: matrix.jdk != '14'
+      uses: actions/setup-java@v2
+      with:
+        distribution: temurin # Temurin is a distribution of adoptium
+        java-version: ${{ matrix.jdk }}
+
+    - name: Set up JDK for build and test on 14
+      if: matrix.jdk == '14'
       uses: actions/setup-java@v1
       with:
+        distribution: temurin
         java-version: ${{ matrix.jdk }}
 
     - name: Checkout security
       uses: actions/checkout@v2
 
-    - name: Initialize CodeQL
-      uses: github/codeql-action/init@v1
+    - name: Build and Test
+      uses: gradle/gradle-build-action@v2
       with:
-        languages: java
+        arguments: |
+          build test -Dbuild.snapshot=false
+          -x checkstyleMain
+          -x checkstyleTest
+
+    - name: Coverage
+      uses: codecov/codecov-action@v1
+      with:
+        token: ${{ secrets.CODECOV_TOKEN }}
+        files: ./build/reports/jacoco/test/jacocoTestReport.xml
 
-    - name: Cache Gradle packages
-      uses: actions/cache@v2
+    - uses: actions/upload-artifact@v3
+      if: always()
       with:
+        name: ${{ matrix.platform }}-JDK${{ matrix.jdk }}-reports
         path: |
-          ~/.gradle/caches
-          ~/.gradle/wrapper
-        key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }}
-        restore-keys: |
-          ${{ runner.os }}-gradle-
-
-
-    - name: Checkstyle
-      run: ./gradlew clean checkstyleMain checkstyleTest
-
-    - name: Package
-      run: ./gradlew clean build -Dbuild.snapshot=false -x test
-
-    - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@v1
+          ./build/reports/
 
-    - name: Test
-      run: OPENDISTRO_SECURITY_TEST_OPENSSL_OPT=true ./gradlew test -i
+    - name: check archive for debugging
+      if: always()
+      run: echo "Check the artifact ${{ matrix.platform }}-JDK${{ matrix.jdk }}-reports for detailed test results"
 
-    - name: Coverage
-      uses: codecov/codecov-action@v1
+  code-ql:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-java@v1
       with:
-        token: ${{ secrets.CODECOV_TOKEN }}
-        files: ./build/jacoco/test/jacocoTestReport.xml
+        java-version: 11
+    - uses: github/codeql-action/init@v1
+      with:
+        languages: java
+    - run: ./gradlew clean build -Dbuild.snapshot=false -x test
+    - uses: github/codeql-action/analyze@v1
 
   build-artifact-names:
     runs-on: ubuntu-latest
@@ -72,18 +86,15 @@ jobs:
         security_plugin_version_no_snapshot=$(echo $security_plugin_version | sed 's/-SNAPSHOT//g')
         security_plugin_version_only_number=$(echo $security_plugin_version_no_snapshot | cut -d- -f1)
         test_qualifier=alpha2
-
         echo "SECURITY_PLUGIN_VERSION=$security_plugin_version" >> $GITHUB_ENV
         echo "SECURITY_PLUGIN_VERSION_NO_SNAPSHOT=$security_plugin_version_no_snapshot" >> $GITHUB_ENV
         echo "SECURITY_PLUGIN_VERSION_ONLY_NUMBER=$security_plugin_version_only_number" >> $GITHUB_ENV
         echo "TEST_QUALIFIER=$test_qualifier" >> $GITHUB_ENV
-
     - run: |
         echo ${{ env.SECURITY_PLUGIN_VERSION }}
         echo ${{ env.SECURITY_PLUGIN_VERSION_NO_SNAPSHOT }}
         echo ${{ env.SECURITY_PLUGIN_VERSION_ONLY_NUMBER }}
         echo ${{ env.TEST_QUALIFIER }}
-
     - run: ./gradlew clean assemble && test -s ./build/opensearch-security-${{ env.SECURITY_PLUGIN_VERSION }}.jar
 
     - run: ./gradlew clean assemble -Dbuild.snapshot=false && test -s ./build/opensearch-security-${{ env.SECURITY_PLUGIN_VERSION_NO_SNAPSHOT }}.jar
@@ -96,10 +107,9 @@ jobs:
         ## EXISTING_OS_VERSION outputs the major version, example as 2
         EXISTING_OS_VERSION=$(./gradlew properties | grep opensearch.version | cut -d':' -f2- | awk '{$1=$1};1' | cut -d '-' -f1 | cut -d '.' -f1)
         ## INCREMENT_OS_VERSION in an increment of 1, example if EXISTING_OS_VERSION is 2, INCREMENT_OS_VERSION is 3
-        INCREMENT_OS_VERSION=$((++EXISTING_OS_VERSION)) 
+        INCREMENT_OS_VERSION=$((++EXISTING_OS_VERSION))
         ./gradlew clean updateVersion -DnewVersion=$INCREMENT_OS_VERSION.0.0-SNAPSHOT
         test `./gradlew properties | grep opensearch.version | cut -d':' -f2- | awk '{$1=$1};1'` = $INCREMENT_OS_VERSION.0.0-SNAPSHOT
-
     - name: List files in the build directory if there was an error
       run: ls -al ./build/
       if: failure()
diff --git a/build.gradle b/build.gradle
@@ -42,6 +42,7 @@ plugins {
     id "nebula.ospackage" version "9.0.0"
     id "com.google.osdetector" version "1.7.0"
     id "org.gradle.test-retry" version "1.3.1"
+    id "com.github.spotbugs" version "5.0.13"
 }
 import org.gradle.crypto.checksum.Checksum
 
@@ -164,6 +165,7 @@ publishing {
 
 tasks.withType(JavaCompile) {
     options.encoding = 'UTF-8'
+    options.warnings = false
 }
 
 static def getTimestamp() {
@@ -223,6 +225,13 @@ testsJar {
     libsDirName = '.'
 }
 
+spotbugs {
+    includeFilter = file('spotbugs-include.xml')
+}
+
+spotbugsTest {
+    enabled = false
+}
 
 test {
     maxParallelForks = 3

diff --git a/spotbugs-include.xml b/spotbugs-include.xml
@@ -0,0 +1,5 @@
+<FindBugsFilter>
+    <Match>
+        <Bug category="I18N" />
+    </Match>
+</FindBugsFilter>
diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java
@@ -18,6 +18,7 @@
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
@@ -155,7 +156,7 @@ private AuthTokenProcessorAction.Response handleImpl(RestRequest restRequest, Re
             SettingsException {
         if (token_log.isDebugEnabled()) {
             try {
-                token_log.debug("SAMLResponse for {}\n{}", samlRequestId, new String(Util.base64decoder(samlResponseBase64), "UTF-8"));
+                token_log.debug("SAMLResponse for {}\n{}", samlRequestId, new String(Util.base64decoder(samlResponseBase64), StandardCharsets.UTF_8));
             } catch (Exception e) {
                 token_log.warn(
                         "SAMLResponse for {} cannot be decoded from base64\n{}",

diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java
@@ -454,7 +454,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
                     try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, originalResult.internalSourceRef(), XContentType.JSON)) {
                         Object base64 = parser.map().values().iterator().next();
                         if(base64 instanceof String) {
-                            originalSource = (new String(BaseEncoding.base64().decode((String) base64)));
+                            originalSource = (new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8));
                         } else {
                             originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
                         }
@@ -465,7 +465,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
                     try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, currentIndex.source(), XContentType.JSON)) {
                         Object base64 = parser.map().values().iterator().next();
                         if(base64 instanceof String) {
-                            currentSource = (new String(BaseEncoding.base64().decode((String) base64)));
+                            currentSource = new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8);
                         } else {
                             currentSource = XContentHelper.convertToJson(currentIndex.source(), false, XContentType.JSON);
                         }
@@ -492,7 +492,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
                 try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, currentIndex.source(), XContentType.JSON)) {
                    Object base64 = parser.map().values().iterator().next();
                    if(base64 instanceof String) {
-                       msg.addSecurityConfigContentToRequestBody(new String(BaseEncoding.base64().decode((String) base64)), id);
+                       msg.addSecurityConfigContentToRequestBody(new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8), id);
                    } else {
                        msg.addSecurityConfigTupleToRequestBody(new Tuple<XContentType, BytesReference>(XContentType.JSON, currentIndex.source()), id);
                    }

diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java b/src/main/java/org/opensearch/security/auditlog/impl/AuditMessage.java
@@ -1,16 +1,12 @@
 /*
- * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
  *
- *  Licensed under the Apache License, Version 2.0 (the "License").
- *  You may not use this file except in compliance with the License.
- *  A copy of the License is located at
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
  *
- *  http://www.apache.org/licenses/LICENSE-2.0
- *
- *  or in the "license" file accompanying this file. This file is distributed
- *  on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
- *  express or implied. See the License for the specific language governing
- *  permissions and limitations under the License.
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
  */
 
 package org.opensearch.security.auditlog.impl;
@@ -433,10 +429,38 @@ public String getRequestType() {
         return (String) this.auditInfo.get(TRANSPORT_REQUEST_TYPE);
     }
 
+    public RestRequest.Method getRequestMethod() {
+        return (RestRequest.Method) this.auditInfo.get(REST_REQUEST_METHOD);
+    }
+
 	public AuditCategory getCategory() {
 		return msgCategory;
 	}
 
+    public Origin getOrigin() {
+        return (Origin) this.auditInfo.get(ORIGIN);
+    }
+
+    public String getPrivilege() {
+        return (String) this.auditInfo.get(PRIVILEGE);
+    }
+
+    public String getExceptionStackTrace() {
+        return (String) this.auditInfo.get(EXCEPTION);
+    }
+
+    public String getRequestBody() {
+        return (String) this.auditInfo.get(REQUEST_BODY);
+    }
+
+    public String getNodeId() {
+        return (String) this.auditInfo.get(NODE_ID);
+    }
+
+    public String getDocId() {
+        return (String) this.auditInfo.get(ID);
+    }
+
 	@Override
 	public String toString() {
 		try {

diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java b/src/main/java/org/opensearch/security/configuration/ConfigurationLoaderSecurity7.java
@@ -31,6 +31,7 @@
 package org.opensearch.security.configuration;
 
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
@@ -275,7 +276,7 @@ private SecurityDynamicConfiguration<?> toConfig(GetResponse singleGetResponse,
 
             parser.nextToken();
 
-            final String jsonAsString = SecurityUtils.replaceEnvVars(new String(parser.binaryValue()), settings);
+            final String jsonAsString = SecurityUtils.replaceEnvVars(new String(parser.binaryValue(), StandardCharsets.UTF_8), settings);
             final JsonNode jsonNode = DefaultObjectMapper.readTree(jsonAsString);
             int configVersion = 1;
 

diff --git a/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java b/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java
@@ -28,6 +28,7 @@
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.File;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.Path;

diff --git a/src/main/java/org/opensearch/security/support/ConfigHelper.java b/src/main/java/org/opensearch/security/support/ConfigHelper.java
@@ -31,10 +31,13 @@
 package org.opensearch.security.support;
 
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.FileReader;
 import java.io.IOException;
+import java.io.InputStreamReader;
 import java.io.Reader;
 import java.io.StringReader;
+import java.nio.charset.StandardCharsets;
 
 import org.opensearch.security.securityconf.impl.Meta;
 import org.apache.logging.log4j.Logger;
@@ -96,7 +99,7 @@ public static void uploadFile(Client tc, String filepath, String index, CType cT
     public static Reader createFileOrStringReader(CType cType, int configVersion, String filepath, boolean populateEmptyIfFileMissing) throws Exception {
         Reader reader;
         if (!populateEmptyIfFileMissing || new File(filepath).exists()) {
-            reader = new FileReader(filepath);
+            reader = new InputStreamReader(new FileInputStream(filepath), StandardCharsets.UTF_8);
         } else {
             reader = new StringReader(createEmptySdcYaml(cType, configVersion));
         }
@@ -148,7 +151,7 @@ public static <T> SecurityDynamicConfiguration<T> fromYamlReader(Reader yamlRead
     }
 
     public static <T> SecurityDynamicConfiguration<T> fromYamlFile(String filepath, CType ctype, int version, long seqNo, long primaryTerm) throws IOException {
-        return fromYamlReader(new FileReader(filepath), ctype, version, seqNo, primaryTerm);
+        return fromYamlReader(new InputStreamReader(new FileInputStream(filepath), StandardCharsets.UTF_8), ctype, version, seqNo, primaryTerm);
     }
 
     public static <T> SecurityDynamicConfiguration<T> fromYamlString(String yamlString, CType ctype, int version, long seqNo, long primaryTerm) throws IOException {

diff --git a/src/main/java/org/opensearch/security/support/SecurityUtils.java b/src/main/java/org/opensearch/security/support/SecurityUtils.java
@@ -50,9 +50,10 @@
 public final class SecurityUtils {
 
     protected final static Logger log = LogManager.getLogger(SecurityUtils.class);
-    private static final Pattern ENV_PATTERN = Pattern.compile("\\$\\{env\\.([\\w]+)((\\:\\-)?[\\w]*)\\}");
-    private static final Pattern ENVBC_PATTERN = Pattern.compile("\\$\\{envbc\\.([\\w]+)((\\:\\-)?[\\w]*)\\}");
-    private static final Pattern ENVBASE64_PATTERN = Pattern.compile("\\$\\{envbase64\\.([\\w]+)((\\:\\-)?[\\w]*)\\}");
+    private static final String ENV_PATTERN_SUFFIX = "\\.([\\w=():\\-_.]+?)(\\:\\-[\\w=():\\-_.]*)?\\}";
+    static final Pattern ENV_PATTERN = Pattern.compile("\\$\\{env" + ENV_PATTERN_SUFFIX);
+    static final Pattern ENVBC_PATTERN = Pattern.compile("\\$\\{envbc" + ENV_PATTERN_SUFFIX);
+    static final Pattern ENVBASE64_PATTERN = Pattern.compile("\\$\\{envbase64" + ENV_PATTERN_SUFFIX);
     public static Locale EN_Locale = forEN();
 
 

diff --git a/src/main/java/org/opensearch/security/tools/SecurityAdmin.java b/src/main/java/org/opensearch/security/tools/SecurityAdmin.java
@@ -33,8 +33,10 @@
 import java.io.ByteArrayInputStream;
 import java.io.Console;
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.io.OutputStreamWriter;
 import java.io.Reader;
 import java.io.Writer;
 import java.net.InetSocketAddress;
@@ -911,8 +913,8 @@ private static boolean retrieveFile(final Client tc, final String filepath, fina
 
         }
 
-        System.out.println("Will retrieve '"+type+"/" +id+"' into "+filepath+" "+(legacy?"(legacy mode)":""));
-        try (Writer writer = new FileWriter(filepath)) {
+        System.out.println("Will retrieve '"+"/" +id+"' into "+filepath+" "+(legacy?"(legacy mode)":""));
+        try (Writer writer = new OutputStreamWriter(new FileOutputStream(filepath), StandardCharsets.UTF_8)) {
 
             final GetResponse response = tc.get(new GetRequest(index).type(type).id(id).refresh(true).realtime(false)).actionGet();