-
Notifications
You must be signed in to change notification settings - Fork 25.2k
HLRC: Get SSL Certificates API #34135
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
Conversation
Pinging @elastic/es-core-infra |
Pinging @elastic/es-security |
|
||
import java.io.IOException; | ||
|
||
public class EmptyBodyRequest implements Validatable, ToXContentObject { |
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.
Gracefully borrowed from #33552
return Objects.hash(certificates); | ||
} | ||
|
||
public static GetSslCertificatesResponse fromXContent(XContentParser parser) throws IOException { |
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.
Can't use ConstructingObjectParser
as this API returns an array of Objects
[[java-rest-high-security-get-certificates-execute-async]] | ||
==== Asynchronous Execution | ||
|
||
This request can be executed asynchronously: |
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.
Suggestion, ... using the security().getSslCertificatesAsync()
method:
this.certificates = certificates; | ||
} | ||
|
||
private static final DeprecationHandler DEPRECATION_HANDLER = new DeprecationHandler() { |
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.
This could be added to DeprecationHandler
as NOOP_DEPRECATION_HANDLER
just like we have THROW_UNSUPPORTED_OPERATION
so if need be others can use it, WDYT?
} | ||
List<CertificateInfo> certificates = new ArrayList<>(); | ||
for (Object cert : unparsedCerts) { | ||
try (XContentBuilder builder = XContentFactory.jsonBuilder()) { |
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.
I am confused, why can't we just parse using the passed in parser?
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.
Because ConstructingObjectParser
expects a parser with Objects in its parse()
method but we get an list of objects in the parser. So I had to get the list from the passed in parser and parse each one.
Does this make sense?
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.
But ConstructingObjectParser
can just parse a single node within the parser stream, so you can do something like this (untested, minimal error checking):
XContentParserUtils.ensureExpectedToken( Token.START_ARRAY, parser.currentToken(), parser::getTokenLocation);
while (parser.nextToken() != Token.END_ARRAY) {
certificates.add( CertificateInfo.PARSER.parse(parser, null) );
}
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.
What Tim said is what I had in mind
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.
Gotcha, thanks! I didn't know I could do that. Now that you mentioned it, I saw this being used elsewhere and it makes total sense too.
...rest-high-level/src/main/java/org/elasticsearch/client/security/support/CertificateInfo.java
Show resolved
Hide resolved
...rest-high-level/src/main/java/org/elasticsearch/client/security/support/CertificateInfo.java
Show resolved
Hide resolved
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.
LGTM
* master: Do not update number of replicas on no indices (elastic#34481) Core: Remove two methods from AbstractComponent (elastic#34336) Use RoleRetrievalResult for better caching (elastic#34197) Revert "Search: Fix spelling mistake in Javadoc (elastic#34480)" Search: Fix spelling mistake in Javadoc (elastic#34480) Docs: improve formatting of Query String Query doc page (elastic#34432) Tests: Handle epoch date formatters edge cases (elastic#34437) Test: Fix running with external cluster Fix handling of empty keyword in terms aggregation (elastic#34457) [DOCS] Fix tag in SecurityDocumentationIT [Monitoring] Add additional necessary mappings for apm-server (elastic#34392) SCRIPTING: Move Aggregation Script Context to its own class (elastic#33820) MINOR: Remove Deadcode in ExpressionTermSetQuery (elastic#34442) HLRC: Get SSL Certificates API (elastic#34135)
This change adds support for the get SSL certificate API to the high level rest client.
This change adds support for the get SSL certificate API to the high level rest client.
This change adds support for the get SSL certificate API to the high
level rest client.