Skip to content

Commit

Permalink
Switch more 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 from master.

Note: I'm not actually removing all calls to the deprecated methods in
ths commit. I'm just backporting the commit from master that removes all
of the deprecated calls so future backports from master will be clean.
There is *probably* still calls to the deprecated methods in the 6.x
branch which is just fine because we'll only be dropping this methods
from master.
  • Loading branch information
@nik9000
nik9000 committed Sep 1, 2018
1 parent ebbfddc commit dabea4a
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.network.NetworkModule;
Expand All @@ -16,9 +17,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 @@ -129,10 +128,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 @@ -149,10 +150,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 @@ -15,11 +15,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 @@ -147,11 +144,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 @@ -406,8 +404,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 @@ -442,7 +438,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 @@ -539,8 +535,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 dabea4a

Please sign in to comment.