Skip to content

Commit

Permalink
Switch remaining LLREST usage to new style Requests (#33171)
Browse files Browse the repository at this point in the history 
In #29623 we added `Request` object flavored requests to the low level
REST client and in #30315 we deprecated the old `performRequest`s. In a
long series of PRs I've changed all of the old style requests that I
could find with `grep`. In this PR I change all requests that I could
find by *removing* the deprecated methods. Since this is a non-trivial
change I do not include actually removing the deprecated requests. I'll
do that in a follow up. But this should be the last set of usage
removals before the actual deprecated method removal. Yay!
  • Loading branch information
@nik9000
nik9000 authored Aug 28, 2018
1 parent 7f5e29d commit 6c8f568
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 75 deletions.
5 changes: 3 additions & 2 deletions ...tache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateWithoutContentIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.script.mustache;

import org.elasticsearch.client.Request;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.test.rest.ESRestTestCase;

Expand All @@ -30,14 +31,14 @@ public class SearchTemplateWithoutContentIT extends ESRestTestCase {

public void testSearchTemplateMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "GET", "/_search/template"));
new Request(randomBoolean() ? "POST" : "GET", "/_search/template")));
assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode());
assertThat(responseException.getMessage(), containsString("request body or source parameter is required"));
}

public void testMultiSearchTemplateMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
randomBoolean() ? "POST" : "GET", "/_msearch/template"));
new Request(randomBoolean() ? "POST" : "GET", "/_msearch/template")));
assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode());
assertThat(responseException.getMessage(), containsString("request body or source parameter is required"));
}
Expand Down
3 changes: 2 additions & 1 deletion modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexWithoutContentIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.index.reindex;

import org.elasticsearch.client.Request;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.test.rest.ESRestTestCase;

Expand All @@ -30,7 +31,7 @@ public class ReindexWithoutContentIT extends ESRestTestCase {

public void testReindexMissingBody() throws IOException {
ResponseException responseException = expectThrows(ResponseException.class, () -> client().performRequest(
"POST", "/_reindex"));
new Request("POST", "/_reindex")));
assertEquals(400, responseException.getResponse().getStatusLine().getStatusCode());
assertThat(responseException.getMessage(), containsString("request body is required"));
}
Expand Down
3 changes: 1 addition & 2 deletions modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4BadRequestIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@

import java.io.IOException;
import java.nio.charset.Charset;
import java.util.Collections;
import java.util.Map;

import static org.elasticsearch.rest.RestStatus.BAD_REQUEST;
Expand Down Expand Up @@ -71,7 +70,7 @@ public void testBadRequest() throws IOException {
final ResponseException e =
expectThrows(
ResponseException.class,
() -> client().performRequest(randomFrom("GET", "POST", "PUT"), path, Collections.emptyMap()));
() -> client().performRequest(new Request(randomFrom("GET", "POST", "PUT"), path)));
assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(BAD_REQUEST.getStatus()));
assertThat(e, hasToString(containsString("too_long_frame_exception")));
assertThat(e, hasToString(matches("An HTTP line is larger than \\d+ bytes")));
Expand Down
12 changes: 8 additions & 4 deletions ...toring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExportBulk.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
*/
package org.elasticsearch.xpack.monitoring.exporter.http;

import org.apache.http.HttpEntity;
import org.apache.http.entity.ByteArrayEntity;
import org.apache.http.entity.ContentType;
import org.apache.http.nio.entity.NByteArrayEntity;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseListener;
import org.elasticsearch.client.RestClient;
Expand Down Expand Up @@ -94,9 +94,13 @@ public void doFlush(ActionListener<Void> listener) throws ExportException {
if (payload == null) {
listener.onFailure(new ExportException("unable to send documents because none were loaded for export bulk [{}]", name));
} else if (payload.length != 0) {
final HttpEntity body = new ByteArrayEntity(payload, ContentType.APPLICATION_JSON);
final Request request = new Request("POST", "/_bulk");
for (Map.Entry<String, String> param : params.entrySet()) {
request.addParameter(param.getKey(), param.getValue());
}
request.setEntity(new NByteArrayEntity(payload, ContentType.APPLICATION_JSON));

client.performRequestAsync("POST", "/_bulk", params, body, new ResponseListener() {
client.performRequestAsync(request, new ResponseListener() {
@Override
public void onSuccess(Response response) {
try {
Expand Down
82 changes: 39 additions & 43 deletions ...lugin/security/src/test/java/org/elasticsearch/integration/AbstractPrivilegeTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,97 +7,93 @@

import org.apache.http.HttpEntity;
import org.apache.http.StatusLine;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.message.BasicHeader;
import org.apache.http.util.EntityUtils;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.test.SecuritySingleNodeTestCase;
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;

import java.io.IOException;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;

/**
* a helper class that contains a couple of HTTP helper methods
* A helper class that contains a couple of HTTP helper methods.
*/
public abstract class AbstractPrivilegeTestCase extends SecuritySingleNodeTestCase {

protected void assertAccessIsAllowed(String user, String method, String uri, String body,
Map<String, String> params) throws IOException {
Response response = getRestClient().performRequest(method, uri, params, entityOrNull(body),
new BasicHeader(UsernamePasswordToken.BASIC_AUTH_HEADER,
UsernamePasswordToken.basicAuthHeaderValue(user, new SecureString("passwd".toCharArray()))));
protected void assertAccessIsAllowed(String user, Request request) throws IOException {
setUser(request, user);
Response response = getRestClient().performRequest(request);
StatusLine statusLine = response.getStatusLine();
String message = String.format(Locale.ROOT, "%s %s: Expected no error got %s %s with body %s", method, uri,
statusLine.getStatusCode(), statusLine.getReasonPhrase(), EntityUtils.toString(response.getEntity()));
String message = String.format(Locale.ROOT, "%s %s: Expected no error got %s %s with body %s",
request.getMethod(), request.getEndpoint(), statusLine.getStatusCode(),
statusLine.getReasonPhrase(), EntityUtils.toString(response.getEntity()));
assertThat(message, statusLine.getStatusCode(), is(not(greaterThanOrEqualTo(400))));
}

protected void assertAccessIsAllowed(String user, String method, String uri, String body) throws IOException {
assertAccessIsAllowed(user, method, uri, body, new HashMap<>());
Request request = new Request(method, uri);
request.setJsonEntity(body);
assertAccessIsAllowed(user, request);
}

protected void assertAccessIsAllowed(String user, String method, String uri) throws IOException {
assertAccessIsAllowed(user, method, uri, null, new HashMap<>());
assertAccessIsAllowed(user, new Request(method, uri));
}

protected void assertAccessIsDenied(String user, String method, String uri, String body) throws IOException {
assertAccessIsDenied(user, method, uri, body, new HashMap<>());
}

protected void assertAccessIsDenied(String user, String method, String uri) throws IOException {
assertAccessIsDenied(user, method, uri, null, new HashMap<>());
}

protected void assertAccessIsDenied(String user, String method, String uri, String body,
Map<String, String> params) throws IOException {
ResponseException responseException = expectThrows(ResponseException.class,
() -> getRestClient().performRequest(method, uri, params, entityOrNull(body),
new BasicHeader(UsernamePasswordToken.BASIC_AUTH_HEADER,
UsernamePasswordToken.basicAuthHeaderValue(user, new SecureString("passwd".toCharArray())))));
protected void assertAccessIsDenied(String user, Request request) throws IOException {
setUser(request, user);
ResponseException responseException = expectThrows(ResponseException.class, () -> getRestClient().performRequest(request));
StatusLine statusLine = responseException.getResponse().getStatusLine();
String message = String.format(Locale.ROOT, "%s %s body %s: Expected 403, got %s %s with body %s", method, uri, body,
String requestBody = request.getEntity() == null ? "" : "with body " + EntityUtils.toString(request.getEntity());
String message = String.format(Locale.ROOT, "%s %s body %s: Expected 403, got %s %s with body %s",
request.getMethod(), request.getEndpoint(), requestBody,
statusLine.getStatusCode(), statusLine.getReasonPhrase(),
EntityUtils.toString(responseException.getResponse().getEntity()));
assertThat(message, statusLine.getStatusCode(), is(403));
}

protected void assertAccessIsDenied(String user, String method, String uri, String body) throws IOException {
Request request = new Request(method, uri);
request.setJsonEntity(body);
assertAccessIsDenied(user, request);
}

protected void assertBodyHasAccessIsDenied(String user, String method, String uri, String body) throws IOException {
assertBodyHasAccessIsDenied(user, method, uri, body, new HashMap<>());
protected void assertAccessIsDenied(String user, String method, String uri) throws IOException {
assertAccessIsDenied(user, new Request(method, uri));
}

/**
* Like {@code assertAcessIsDenied}, but for _bulk requests since the entire
* request will not be failed, just the individual ones
*/
protected void assertBodyHasAccessIsDenied(String user, String method, String uri, String body,
Map<String, String> params) throws IOException {
Response resp = getRestClient().performRequest(method, uri, params, entityOrNull(body),
new BasicHeader(UsernamePasswordToken.BASIC_AUTH_HEADER,
UsernamePasswordToken.basicAuthHeaderValue(user, new SecureString("passwd".toCharArray()))));
protected void assertBodyHasAccessIsDenied(String user, Request request) throws IOException {
setUser(request, user);
Response resp = getRestClient().performRequest(request);
StatusLine statusLine = resp.getStatusLine();
assertThat(statusLine.getStatusCode(), is(200));
HttpEntity bodyEntity = resp.getEntity();
String bodyStr = EntityUtils.toString(bodyEntity);
assertThat(bodyStr, containsString("unauthorized for user [" + user + "]"));
}

private static HttpEntity entityOrNull(String body) {
HttpEntity entity = null;
if (body != null) {
entity = new StringEntity(body, ContentType.APPLICATION_JSON);
}
return entity;
protected void assertBodyHasAccessIsDenied(String user, String method, String uri, String body) throws IOException {
Request request = new Request(method, uri);
request.setJsonEntity(body);
assertBodyHasAccessIsDenied(user, request);
}

private void setUser(Request request, String user) {
RequestOptions.Builder options = RequestOptions.DEFAULT.toBuilder();
options.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue(user, new SecureString("passwd".toCharArray())));
request.setOptions(options);
}
}
22 changes: 12 additions & 10 deletions ...ck/plugin/security/src/test/java/org/elasticsearch/integration/ClusterPrivilegeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.elasticsearch.integration;

import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse;
import org.elasticsearch.client.Request;
import org.elasticsearch.cluster.SnapshotsInProgress;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.SecureString;
Expand All @@ -15,9 +16,7 @@
import org.junit.BeforeClass;

import java.nio.file.Path;
import java.util.Map;

import static java.util.Collections.singletonMap;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.is;

Expand Down Expand Up @@ -132,10 +131,12 @@ public void testThatSnapshotAndRestore() throws Exception {
assertAccessIsDenied("user_c", "PUT", "/_snapshot/my-repo", repoJson);
assertAccessIsAllowed("user_a", "PUT", "/_snapshot/my-repo", repoJson);

Map<String, String> params = singletonMap("refresh", "true");
assertAccessIsDenied("user_a", "PUT", "/someindex/bar/1", "{ \"name\" : \"elasticsearch\" }", params);
assertAccessIsDenied("user_b", "PUT", "/someindex/bar/1", "{ \"name\" : \"elasticsearch\" }", params);
assertAccessIsAllowed("user_c", "PUT", "/someindex/bar/1", "{ \"name\" : \"elasticsearch\" }", params);
Request createBar = new Request("PUT", "/someindex/bar/1");
createBar.setJsonEntity("{ \"name\" : \"elasticsearch\" }");
createBar.addParameter("refresh", "true");
assertAccessIsDenied("user_a", createBar);
assertAccessIsDenied("user_b", createBar);
assertAccessIsAllowed("user_c", createBar);

assertAccessIsDenied("user_b", "PUT", "/_snapshot/my-repo/my-snapshot", "{ \"indices\": \"someindex\" }");
assertAccessIsDenied("user_c", "PUT", "/_snapshot/my-repo/my-snapshot", "{ \"indices\": \"someindex\" }");
Expand All @@ -152,10 +153,11 @@ public void testThatSnapshotAndRestore() throws Exception {
assertAccessIsDenied("user_b", "DELETE", "/someindex");
assertAccessIsAllowed("user_c", "DELETE", "/someindex");

params = singletonMap("wait_for_completion", "true");
assertAccessIsDenied("user_b", "POST", "/_snapshot/my-repo/my-snapshot/_restore", null, params);
assertAccessIsDenied("user_c", "POST", "/_snapshot/my-repo/my-snapshot/_restore", null, params);
assertAccessIsAllowed("user_a", "POST", "/_snapshot/my-repo/my-snapshot/_restore", null, params);
Request restoreSnapshotRequest = new Request("POST", "/_snapshot/my-repo/my-snapshot/_restore");
restoreSnapshotRequest.addParameter("wait_for_completion", "true");
assertAccessIsDenied("user_b", restoreSnapshotRequest);
assertAccessIsDenied("user_c", restoreSnapshotRequest);
assertAccessIsAllowed("user_a", restoreSnapshotRequest);

assertAccessIsDenied("user_a", "GET", "/someindex/bar/1");
assertAccessIsDenied("user_b", "GET", "/someindex/bar/1");
Expand Down
22 changes: 9 additions & 13 deletions x-pack/plugin/security/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,8 @@
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
import org.junit.Before;

import java.util.Collections;
import java.util.Locale;
import java.util.Map;

import static java.util.Collections.singletonMap;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout;
import static org.hamcrest.Matchers.is;

Expand Down Expand Up @@ -143,11 +140,12 @@ protected String configUsersRoles() {
@Before
public void insertBaseDocumentsAsAdmin() throws Exception {
// indices: a,b,c,abc
Map<String, String> params = singletonMap("refresh", "true");
assertAccessIsAllowed("admin", "PUT", "/a/foo/1", jsonDoc, params);
assertAccessIsAllowed("admin", "PUT", "/b/foo/1", jsonDoc, params);
assertAccessIsAllowed("admin", "PUT", "/c/foo/1", jsonDoc, params);
assertAccessIsAllowed("admin", "PUT", "/abc/foo/1", jsonDoc, params);
for (String index : new String[] {"a", "b", "c", "abc"}) {
Request request = new Request("PUT", "/" + index + "/foo/1");
request.setJsonEntity(jsonDoc);
request.addParameter("refresh", "true");
assertAccessIsAllowed("admin", request);
}
}

private static String randomIndex() {
Expand Down Expand Up @@ -402,8 +400,6 @@ public void testThatUnknownUserIsRejectedProperly() throws Exception {
}

private void assertUserExecutes(String user, String action, String index, boolean userIsAllowed) throws Exception {
Map<String, String> refreshParams = Collections.emptyMap();//singletonMap("refresh", "true");

switch (action) {
case "all" :
if (userIsAllowed) {
Expand Down Expand Up @@ -438,7 +434,7 @@ private void assertUserExecutes(String user, String action, String index, boolea
assertAccessIsAllowed(user, "POST", "/" + index + "/_open");
assertAccessIsAllowed(user, "POST", "/" + index + "/_cache/clear");
// indexing a document to have the mapping available, and wait for green state to make sure index is created
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/1", jsonDoc, refreshParams);
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/1", jsonDoc);
assertNoTimeout(client().admin().cluster().prepareHealth(index).setWaitForGreenStatus().get());
assertAccessIsAllowed(user, "GET", "/" + index + "/_mapping/foo/field/name");
assertAccessIsAllowed(user, "GET", "/" + index + "/_settings");
Expand Down Expand Up @@ -535,8 +531,8 @@ private void assertUserExecutes(String user, String action, String index, boolea

case "delete" :
String jsonDoc = "{ \"name\" : \"docToDelete\"}";
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/docToDelete", jsonDoc, refreshParams);
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/docToDelete2", jsonDoc, refreshParams);
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/docToDelete", jsonDoc);
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/docToDelete2", jsonDoc);
if (userIsAllowed) {
assertAccessIsAllowed(user, "DELETE", "/" + index + "/foo/docToDelete");
} else {
Expand Down

0 comments on commit 6c8f568

Please sign in to comment.