-
Notifications
You must be signed in to change notification settings - Fork 259
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
[Fix #4280] Introduce new endpoint _plugins/_security/api/certificates #4299
[Fix #4280] Introduce new endpoint _plugins/_security/api/certificates #4299
Conversation
bb574e0
to
6808230
Compare
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.
Took an initial pass. This approach looks great. Left some comments. I do see changes from #4252, was that accidental?
src/integrationTest/java/org/opensearch/security/api/AbstractApiIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificatesNodesResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificatesNodesResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/ssl/CertificatesActionType.java
Show resolved
Hide resolved
|
6b6502c
to
67645ed
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4299 +/- ##
==========================================
- Coverage 66.02% 65.45% -0.57%
==========================================
Files 302 310 +8
Lines 21762 21986 +224
Branches 3523 3552 +29
==========================================
+ Hits 14368 14391 +23
- Misses 5626 5825 +199
- Partials 1768 1770 +2
|
67645ed
to
1a01482
Compare
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.
Thank you for addressing all the comments @willyborankin . This looks close to me, left a few final comments from my end.
src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsApiAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java
Show resolved
Hide resolved
src/integrationTest/java/org/opensearch/security/api/CertificatesRestApiIntegrationTest.java
Show resolved
Hide resolved
09917ea
to
2c07bfb
Compare
src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/CertificatesApiAction.java
Show resolved
Hide resolved
@willyborankin Great work! A couple of questions- what would the behavior be when-
|
An example of failure response: {
"_nodes": {
"total": 5,
"successful": 1,
"failed": 4,
"failures": [
{
"type": "failed_node_exception",
"reason": "Failed node [meWIrv__T_-3_hR0AAAAAA]",
"node_id": "meWIrv__T_-3_hR0AAAAAA",
"caused_by": {
"type": "illegal_argument_exception",
"reason": "some reason"
}
},
{
"type": "failed_node_exception",
"reason": "Failed node [ierwfAAAQAC_luT5_____w]",
"node_id": "ierwfAAAQAC_luT5_____w",
"caused_by": {
"type": "illegal_argument_exception",
"reason": "some reason"
}
},
{
"type": "failed_node_exception",
"reason": "Failed node [tRcffAAAQACcRK-n_____w]",
"node_id": "tRcffAAAQACcRK-n_____w",
"caused_by": {
"type": "illegal_argument_exception",
"reason": "some reason"
}
},
{
"type": "failed_node_exception",
"reason": "Failed node [sWODKwAAQACeoJxjAAAAAA]",
"node_id": "sWODKwAAQACeoJxjAAAAAA",
"caused_by": {
"type": "illegal_argument_exception",
"reason": "some reason"
}
}
]
},
"cluster_name": "local_cluster_1",
"nodes": {
"YOYidQAAQACP5uIKAAAAAA" : {
"name" : "data_0",
"certificates" : {
"http" : [ {
"subject_dn" : "DC=de,L=test,O=node,OU=node,CN=node-3.example.com",
"san" : "[[8, 1.2.3.4.5.5], [2, node-3.example.com], [2, localhost], [7, 127.0.0.1]]",
"issuer_dn" : "DC=com,DC=example,O=Example Com Inc.,OU=Example Com Inc. Root CA,CN=Example Com Inc. Root CA",
"not_after" : "2025-04-28T09:23:47Z",
"not_before" : "2024-04-28T09:23:47Z"
} ],
"transport" : [ {
"subject_dn" : "DC=de,L=test,O=node,OU=node,CN=node-3.example.com",
"san" : "[[8, 1.2.3.4.5.5], [2, node-3.example.com], [2, localhost], [7, 127.0.0.1]]",
"issuer_dn" : "DC=com,DC=example,O=Example Com Inc.,OU=Example Com Inc. Root CA,CN=Example Com Inc. Root CA",
"not_after" : "2025-04-28T09:23:47Z",
"not_before" : "2024-04-28T09:23:47Z"
} ]
}
}
}
}
Same as above. |
2c07bfb
to
a0a89ba
Compare
@shikharj05 Does @willyborankin's comment address your questions? Since we have 2 approvals we will be merging soon if there are no more questions. |
Thanks for the response @willyborankin. @DarshitChanpura - we should be good to merge, as we can see from the results above, the API doesn't fail in a mixed cluster. |
@willyborankin Can you check the windows integration test CI failures once. They seem to be failing even after 3 retries. |
a0a89ba
to
00a0356
Compare
|
Introduce 2 new endpoint: `_plugins/_security/api/certificates` `_plugins/_security/api/certificates/{nodeId}` Query parameters: - cert_type - timeout which provides information about SSL certificates for each node in the cluster Signed-off-by: Andrey Pleskach <ples@aiven.io>
9ab5e0c
00a0356
to
9ab5e0c
Compare
Description
Introduce new endpoint:
_plugins/_security/api/certificates
_plugins/_security/api/certificates/{nodeId}
which provides information about SSL certificates for each node in the cluster. The endpoint works only with Default key store but not with external key store.
Path parameters:
nodeId
- (Optional, string) The names of particular nodes in the cluster to target. For example,nodeId1,nodeId2
Query string parameters:
cert_type
- (Optional, string) The SSL certificate type. Expected values are:http
,transport
andall
. The default value isall
timeout
- node request timeoutHTTP Response:
Example HTTP response:
Issues Resolved
#4280
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List
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.