-
Notifications
You must be signed in to change notification settings - Fork 281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade json-smart from 2.4.2 to 2.4.7 #1299
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1299 +/- ##
============================================
- Coverage 64.73% 64.70% -0.03%
Complexity 3194 3194
============================================
Files 247 247
Lines 17230 17230
Branches 3045 3045
============================================
- Hits 11153 11149 -4
- Misses 4528 4532 +4
Partials 1549 1549
Continue to review full report at Codecov.
|
pom.xml
Outdated
</dependency> | ||
<dependency> | ||
<groupId>net.minidev</groupId> | ||
<artifactId>accessors-smart</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
json-smart-2.4.5
introduces accessors-smart-2.4.3
that is not available in Maven Central. Had to have accessors-smart
remain 2.4.2
.
Failed to execute goal on project opensearch-security: Could not resolve dependencies for project org.opensearch:opensearch-security:jar:1.0.0.0-rc1: Could not find artifact net.minidev:accessors-smart:jar:2.4.3 in central (https://repo.maven.apache.org/maven2) -> [Help 1]
Failing CI: https://github.com/opensearch-project/security/runs/2906736355?check_suite_focus=true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created an issue: netplex/json-smart-v2#76
pom.xml
Outdated
@@ -131,6 +131,11 @@ | |||
<dependency> | |||
<groupId>net.minidev</groupId> | |||
<artifactId>json-smart</artifactId> | |||
<version>2.4.5</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not to upgrade json-smart
to 2.4.7 that is more widely used compared to 2.4.5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, it will be better to remove the dependency on json-smart completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, it will be better to remove the dependency on json-smart completely.
Tried replacingjson-smart
with Jackson, but Jackson automatically wraps the values in Json with quotes, which fails the tests currently relying onjson-smar
. The following is the failure:
org.junit.ComparisonFailure:
Expected :{"http_certificates_list":"[{issuer_dn=CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com, subject_dn=CN=node-1.example.com,OU=SSL,O=Test,L=Test,C=DE, san=[[2, node-1.example.com], [2, localhost], [7, 127.0.0.1], [8, 1.2.3.4.5.5]], not_before=2020-02-17T16:19:25Z, not_after=2022-02-16T16:19:25Z}]","transport_certificates_list":"[{issuer_dn=CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com, subject_dn=CN=node-1.example.com,OU=SSL,O=Test,L=Test,C=DE, san=[[2, node-1.example.com], [2, localhost], [7, 127.0.0.1], [8, 1.2.3.4.5.5]], not_before=2020-02-17T16:19:25Z, not_after=2022-02-16T16:19:25Z}]"}
Actual :{"http_certificates_list":[{"issuer_dn":"CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com","subject_dn":"CN=node-1.example.com,OU=SSL,O=Test,L=Test,C=DE","san":"[[2, node-1.example.com], [2, localhost], [7, 127.0.0.1], [8, 1.2.3.4.5.5]]","not_before":"2020-02-17T16:19:25Z","not_after":"2022-02-16T16:19:25Z"}],"transport_certificates_list":[{"issuer_dn":"CN=Example Com Inc. Signing CA,OU=Example Com Inc. Signing CA,O=Example Com Inc.,DC=example,DC=com","subject_dn":"CN=node-1.example.com,OU=SSL,O=Test,L=Test,C=DE","san":"[[2, node-1.example.com], [2, localhost], [7, 127.0.0.1], [8, 1.2.3.4.5.5]]","not_before":"2020-02-17T16:19:25Z","not_after":"2022-02-16T16:19:25Z"}]}
Probably Jackson is not the answer. Will need explore more options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cliu123 I don't see why jackson is not an option. The expected is not properly constructed.
(cherry picked from commit e341bd5)
(cherry picked from commit e341bd5)
(cherry picked from commit e341bd5)
opensearch-security pull request intake form
Please provide as much details as possible to get feedback/acceptance on your PR quickly
Category: (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)
Maintenance
Github Issue # or road-map entry, if available:
Description of changes:
Why these changes are required?
What is the old behavior before changes and new behavior after changes? (Please add any example/logs/screen-shot if available)
Testing done: (Please provide details of testing done: Unit testing, integration testing and manual testing)
TO-DOs, if any: (Please describe pending items and provide Github issues# for each of them)
Is it backport from main branch? (If yes, please add backport PR # and commits #)
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
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.