Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 52 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/


import com.diffplug.gradle.spotless.JavaExtension
import org.opensearch.gradle.test.RestIntegTestTask

buildscript {
Expand Down Expand Up @@ -51,7 +52,7 @@ plugins {
id 'idea'
id 'jacoco'
id 'maven-publish'
id 'com.diffplug.spotless' version '6.18.0'
id 'com.diffplug.spotless' version '6.19.0'
id 'checkstyle'
id 'com.netflix.nebula.ospackage' version "11.1.0"
id "org.gradle.test-retry" version "1.5.2"
Expand All @@ -69,7 +70,56 @@ apply plugin: 'opensearch.opensearchplugin'
apply plugin: 'opensearch.pluginzip'
apply plugin: 'opensearch.rest-test'
apply plugin: 'opensearch.testclusters'
apply from: 'gradle/formatting.gradle'
//apply from: 'gradle/formatting.gradle'
Copy link
Collaborator

@RyanL1997 RyanL1997 May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should we just remove it? Or just leave as comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are going to leave it as a comment for the time being per this conversation: #2821 (comment)


spotless {
java {
// Normally this isn't necessary, but we have Java sources in
// non-standard places
target '**/com/amazon/dlic/**/*.java'

removeUnusedImports()
eclipse().configFile rootProject.file('formatter/formatterConfig.xml')
trimTrailingWhitespace()
endWithNewline();

// note: you can use an empty string for all the imports you didn't specify explicitly, and '\\#` prefix for static imports
importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')

custom 'Refuse wildcard imports', {
// Wildcard imports can't be resolved; fail the build
if (it =~ /\s+import .*\*;/) {
throw new AssertionError("Do not use wildcard imports. 'spotlessApply' cannot resolve this issue.")
}
}

// See DEVELOPER_GUIDE.md for details of when to enable this.
if (System.getProperty('spotless.paddedcell') != null) {
paddedCell()
}
}
format 'misc', {
target '*.md', '*.gradle', '**/*.json', '**/*.yaml', '**/*.yml', '**/*.svg'

trimTrailingWhitespace()
endWithNewline()
}
format('javaFoo', JavaExtension) {

importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')
target '**/*.java'
targetExclude '**/com/amazon/dlic/**/*.java'
targetExclude('src/integrationTest/**')

trimTrailingWhitespace()
endWithNewline();
}
format("integrationTest", JavaExtension) {
target('src/integrationTest/java/**/*.java')
importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')
indentWithTabs(4)
}
}

licenseFile = rootProject.file('LICENSE.txt')
noticeFile = rootProject.file('NOTICE.txt')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@
import java.util.Set;
import java.util.stream.Collectors;

import org.junit.Assume;
import com.google.common.collect.ImmutableMap;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Before;

import org.opensearch.Version;
import org.opensearch.client.Response;
import org.opensearch.common.settings.Settings;
import org.opensearch.rest.RestStatus;
import org.opensearch.test.rest.OpenSearchRestTestCase;

import org.opensearch.Version;
import com.google.common.collect.ImmutableMap;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasItem;

Expand Down
129 changes: 94 additions & 35 deletions gradle/formatting.gradle
Original file line number Diff line number Diff line change
@@ -1,36 +1,95 @@
allprojects {
project.apply plugin: "com.diffplug.spotless"
spotless {
java {
// Normally this isn't necessary, but we have Java sources in
// non-standard places
target '*/com/amazon/dlic/auth/**/*.java'

removeUnusedImports()
eclipse().configFile rootProject.file('formatter/formatterConfig.xml')
trimTrailingWhitespace()
endWithNewline();

// note: you can use an empty string for all the imports you didn't specify explicitly, and '\\#` prefix for static imports
importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')

custom 'Refuse wildcard imports', {
// Wildcard imports can't be resolved; fail the build
if (it =~ /\s+import .*\*;/) {
throw new AssertionError("Do not use wildcard imports. 'spotlessApply' cannot resolve this issue.")
}
}

// See DEVELOPER_GUIDE.md for details of when to enable this.
if (System.getProperty('spotless.paddedcell') != null) {
paddedCell()
}
}
format 'misc', {
target '*.md', '*.gradle', '**/*.json', '**/*.yaml', '**/*.yml', '**/*.svg'

trimTrailingWhitespace()
endWithNewline()
}
}
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License 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.
*/

import org.opensearch.gradle.BuildPlugin

/*
* This script plugin configures formatting for Java source using Spotless
* for Gradle. Since the act of formatting existing source can interfere
* with developers' workflows, we don't automatically format all code
* (yet). Instead, we maintain a list of projects that are excluded from
* formatting, until we reach a point where we can comfortably format them
* in one go without too much disruption.
*
* Any new sub-projects must not be added to the exclusions list!
*
* To perform a reformat, run:
*
* ./gradlew spotlessApply
*
* To check the current format, run:
*
* ./gradlew spotlessJavaCheck
*
* This is also carried out by the `precommit` task.
*
* For more about Spotless, see:
*
* https://github.com/diffplug/spotless/tree/master/plugin-gradle
*/

org.opensearch.gradle.BuildPlugin {
plugins.withType(BuildPlugin).whenPluginAdded {
project.apply plugin: "com.diffplug.spotless"

spotless {
java {
// Normally this isn't necessary, but we have Java sources in
// non-standard places
target '**/*.java'

removeUnusedImports()
eclipse().configFile rootProject.file('buildSrc/formatterConfig.xml')
trimTrailingWhitespace()
endWithNewline()

custom 'Refuse wildcard imports', {
// Wildcard imports can't be resolved; fail the build
if (it =~ /\s+import .*\*;/) {
throw new AssertionError("Do not use wildcard imports. 'spotlessApply' cannot resolve this issue.")
}
}

// See DEVELOPER_GUIDE.md for details of when to enable this.
if (System.getProperty('spotless.paddedcell') != null) {
paddedCell()
}
}
format 'misc', {
target '*.md', '*.gradle', '**/*.yaml', '**/*.yml', '**/*.svg'

trimTrailingWhitespace()
endWithNewline()
}
}

precommit.dependsOn 'spotlessJavaCheck'
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public abstract class AbstractHTTPJwtAuthenticator implements HTTPAuthenticator
private final String requiredIssuer;

public static final int DEFAULT_CLOCK_SKEW_TOLERANCE_SECONDS = 30;
private final int clockSkewToleranceSeconds ;
private final int clockSkewToleranceSeconds;

public AbstractHTTPJwtAuthenticator(Settings settings, Path configPath) {
jwtUrlParameter = settings.get("jwt_url_parameter");
Expand All @@ -83,8 +83,7 @@ public AbstractHTTPJwtAuthenticator(Settings settings, Path configPath) {

@Override
@SuppressWarnings("removal")
public AuthCredentials extractCredentials(RestRequest request, ThreadContext context)
throws OpenSearchSecurityException {
public AuthCredentials extractCredentials(RestRequest request, ThreadContext context) throws OpenSearchSecurityException {
final SecurityManager sm = System.getSecurityManager();

if (sm != null) {
Expand Down Expand Up @@ -186,8 +185,11 @@ public String extractSubject(JwtClaims claims) {
// warning
if (!(subjectObject instanceof String)) {
log.warn(
"Expected type String for roles in the JWT for subject_key {}, but value was '{}' ({}). Will convert this value to String.",
subjectKey, subjectObject, subjectObject.getClass());
"Expected type String for roles in the JWT for subject_key {}, but value was '{}' ({}). Will convert this value to String.",
subjectKey,
subjectObject,
subjectObject.getClass()
);
subject = String.valueOf(subjectObject);
} else {
subject = (String) subjectObject;
Expand All @@ -207,8 +209,9 @@ public String[] extractRoles(JwtClaims claims) {

if (rolesObject == null) {
log.warn(
"Failed to get roles from JWT claims with roles_key '{}'. Check if this key is correct and available in the JWT payload.",
rolesKey);
"Failed to get roles from JWT claims with roles_key '{}'. Check if this key is correct and available in the JWT payload.",
rolesKey
);
return new String[0];
}

Expand All @@ -218,8 +221,11 @@ public String[] extractRoles(JwtClaims claims) {
// String but issue a warning
if (!(rolesObject instanceof String) && !(rolesObject instanceof Collection<?>)) {
log.warn(
"Expected type String or Collection for roles in the JWT for roles_key {}, but value was '{}' ({}). Will convert this value to String.",
rolesKey, rolesObject, rolesObject.getClass());
"Expected type String or Collection for roles in the JWT for roles_key {}, but value was '{}' ({}). Will convert this value to String.",
rolesKey,
rolesObject,
rolesObject.getClass()
);
} else if (rolesObject instanceof Collection<?>) {
roles = ((Collection<String>) rolesObject).toArray(new String[0]);
}
Expand Down
Loading