search

opensearch-project / OpenSearch

🔎 Open source distributed and RESTful search engine.
https://opensearch.org/docs/latest/opensearch/index/
Apache License 2.0
9.47k stars 1.74k forks source link

[BUG] OpenSearch 2.1.0 SNAPSHOT recent changes fail security plugin integration tests #3554

Closed cliu123 closed 2 years ago

cliu123 commented 2 years ago

Describe the bug JSON is not parsed properly by Jackson.

To Reproduce

Plugins Security plugin

Additional context Stacktrace:

[2022-06-08T23:32:20,280][ERROR][org.opensearch.security.dlic.rest.validation.RolesMappingValidator] BODY_NOT_PARSEABLE
2022-06-08T23:32:20.3699823Z     com.fasterxml.jackson.core.JsonParseException: Unexpected character ('"' (code 34)): was expecting comma to separate Object entries
2022-06-08T23:32:20.3700239Z      at [Source: (String)"{
2022-06-08T23:32:20.3700469Z       "users": [
2022-06-08T23:32:20.3700677Z         "sisko",
2022-06-08T23:32:20.3700876Z         "janeway",
2022-06-08T23:32:20.3701078Z         "kirk"
2022-06-08T23:32:20.3701253Z       ]
2022-06-08T23:32:20.3701455Z       "backend_roles": [
2022-06-08T23:32:20.3701672Z         "captains",
2022-06-08T23:32:20.3701873Z         "role2",
2022-06-08T23:32:20.3702072Z         "role3"
2022-06-08T23:32:20.3702263Z       ]
2022-06-08T23:32:20.3702549Z       "hosts": [
2022-06-08T23:32:20.3702736Z         "8.8.8.8",
2022-06-08T23:32:20.3702930Z         "8.8.4.4"
2022-06-08T23:32:20.3703121Z       ]
2022-06-08T23:32:20.3703298Z     }
2022-06-08T23:32:20.3703493Z     "; line: 7, column: 4]
2022-06-08T23:32:20.3704031Z        at com.fasterxml.jackson.core.JsonParser._constructError(JsonParser.java:2391) ~[jackson-core-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3705158Z        at com.fasterxml.jackson.core.base.ParserMinimalBase._reportError(ParserMinimalBase.java:735) ~[jackson-core-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3705970Z        at com.fasterxml.jackson.core.base.ParserMinimalBase._reportUnexpectedChar(ParserMinimalBase.java:659) ~[jackson-core-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3706803Z        at com.fasterxml.jackson.core.json.ReaderBasedJsonParser._skipComma(ReaderBasedJsonParser.java:2382) ~[jackson-core-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3707781Z        at com.fasterxml.jackson.core.json.ReaderBasedJsonParser.nextFieldName(ReaderBasedJsonParser.java:947) ~[jackson-core-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3708774Z        at com.fasterxml.jackson.databind.deser.std.BaseNodeDeserializer._deserializeContainerNoRecursion(JsonNodeDeserializer.java:437) ~[jackson-databind-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3709749Z        at com.fasterxml.jackson.databind.deser.std.JsonNodeDeserializer.deserialize(JsonNodeDeserializer.java:84) ~[jackson-databind-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3710712Z        at com.fasterxml.jackson.databind.deser.std.JsonNodeDeserializer.deserialize(JsonNodeDeserializer.java:20) ~[jackson-databind-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3711746Z        at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:322) ~[jackson-databind-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3712684Z        at com.fasterxml.jackson.databind.ObjectMapper._readTreeAndClose(ObjectMapper.java:4716) ~[jackson-databind-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3713443Z        at com.fasterxml.jackson.databind.ObjectMapper.readTree(ObjectMapper.java:3076) ~[jackson-databind-2.13.2.jar:2.13.2]
2022-06-08T23:32:20.3714023Z        at org.opensearch.security.DefaultObjectMapper$3.run(DefaultObjectMapper.java:154) ~[main/:?]
2022-06-08T23:32:20.3714552Z        at org.opensearch.security.DefaultObjectMapper$3.run(DefaultObjectMapper.java:151) ~[main/:?]
2022-06-08T23:32:20.3715066Z        at java.security.AccessController.doPrivileged(AccessController.java:569) ~[?:?]
2022-06-08T23:32:20.3715604Z        at org.opensearch.security.DefaultObjectMapper.readTree(DefaultObjectMapper.java:151) ~[main/:?]
2022-06-08T23:32:20.3716339Z        at org.opensearch.security.dlic.rest.validation.AbstractConfigurationValidator.validate(AbstractConfigurationValidator.java:128) ~[main/:?]
2022-06-08T23:32:20.3717135Z        at org.opensearch.security.dlic.rest.api.AbstractApiAction.handleApiRequest(AbstractApiAction.java:111) ~[main/:?]
2022-06-08T23:32:20.3717923Z        at org.opensearch.security.dlic.rest.api.PatchableResourceApiAction.handleApiRequest(PatchableResourceApiAction.java:268) ~[main/:?]
2022-06-08T23:32:20.3718687Z        at org.opensearch.security.dlic.rest.api.AbstractApiAction.lambda$prepareRequest$2(AbstractApiAction.java:401) ~[main/:?]
2022-06-08T23:32:20.3719245Z        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) [?:?]
2022-06-08T23:32:20.3719673Z        at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
2022-06-08T23:32:20.3720431Z        at org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:739) [opensearch-2.1.0-SNAPSHOT.jar:2.1.0-SNAPSHOT]
2022-06-08T23:32:20.3721069Z        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
2022-06-08T23:32:20.3721571Z        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
2022-06-08T23:32:20.3721971Z        at java.lang.Thread.run(Thread.java:833) [?:?]
cliu123 commented 2 years ago

Security plugin has to downgrade to OpenSearch 2.0.0 because of this issue: https://github.com/opensearch-project/security/pull/1882

reta commented 2 years ago

@tlfeng I think you know what it is :), could be related to https://github.com/opensearch-project/OpenSearch/commit/ab478ba5f33d45b98a3dd2b12038680d443cc1e1 ?

tlfeng commented 2 years ago

could be related to https://github.com/opensearch-project/OpenSearch/commit/ab478ba5f33d45b98a3dd2b12038680d443cc1e1

@reta 😄 It's not likely related to the parsing change in Node Sniffer, because the change is limited to the Sniffer, and Sniffer is a separate library based on Rest Client, which is not used by security plugin. All json format data is parsed by fasterxml.jackson in the code.

cliu123 commented 2 years ago

@reta @tlfeng Thanks for checking the issue! Security plugin has downgraded to OpenSearch 2.1.0 where this issue doesn't exist. Please let us know when the root cause gets identified and the issue gets fixed. We're happy to verify the fix.

xuezhou25 commented 2 years ago

👀

xuezhou25 commented 2 years ago

In my point of view, the error JsonParseException is not the fail reason because there is the same error in 2.0.0. The actual cause for the test failure is PR#3524, commit https://github.com/opensearch-project/OpenSearch/commit/fc8803fe5f34b948a888365851cc46e320b40f81. In create document API: In version 2.0.1 and above, "type" parameter is accepted, but will be ignored. In version 2.0.0, "type" parmater is not accepted. The failure occurs in this test case, and maybe it should be adjusted for OpenSearch 2.0.1 and 2.1.0 . https://github.com/opensearch-project/security/blob/2.0.0.0/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java#L344-L345

The error message for the actual test failure is:

java.lang.AssertionError: expected:<400> but was:<200>
    at org.junit.Assert.fail(Assert.java:89)
    at org.junit.Assert.failNotEquals(Assert.java:835)
    at org.junit.Assert.assertEquals(Assert.java:647)
    at org.junit.Assert.assertEquals(Assert.java:633)
    at org.opensearch.security.dlic.rest.api.AbstractRestApiUnitTest.checkWriteAccess(AbstractRestApiUnitTest.java:226)
    at org.opensearch.security.dlic.rest.api.RolesMappingApiTest.checkAllSfAllowed(RolesMappingApiTest.java:345)
    at org.opensearch.security.dlic.rest.api.RolesMappingApiTest.testRolesMappingApi(RolesMappingApiTest.java:308)
...
cliu123 commented 2 years ago

Thanks @xuezhou25 @dreamer-89 @tlfeng for helping with this issue!

tlfeng commented 2 years ago

Draw a conclusion here, looks like security plugin have to do nothing with the test, since the behavior of "type" parameter in the REST API is changed back to the same as 2.0.0 in the commit https://github.com/opensearch-project/OpenSearch/commit/6462a546240f6d7a158519499729bce12dc1058b, "type" is not allowed as a parameter.

cliu123 commented 2 years ago

Draw a conclusion here, looks like security plugin have to do nothing with the test, since the behavior of "type" parameter in the REST API is changed back to the same as 2.0.0 in the commit 6462a54, "type" is not allowed as a parameter.

A quick clarification on the conclusion: The fix in security plugin tests is a part of the version bump PR.