Skip to content
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.

Sign up for GitHub

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

Jump to bottom

removing api keys from the integ test log #3112

Merged
merged 4 commits into from
Oct 17, 2024
Merged

removing api keys from the integ test log #3112

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
876f74d
removing api keys from the integ test log
dhrubo-os Oct 15, 2024
603ed39
apply spotless
dhrubo-os Oct 15, 2024
7723c35
addressed comment
dhrubo-os Oct 15, 2024
15c19fa
adding unit test to address comments
dhrubo-os Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 57 additions & 26 deletions plugin/src/test/java/org/opensearch/ml/rest/RestMLRemoteInferenceIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.opensearch.client.Response;
import org.opensearch.client.ResponseException;
import org.opensearch.ml.common.MLTaskState;
import org.opensearch.ml.utils.TestHelper;

Expand Down Expand Up @@ -76,39 +78,63 @@ public void setup() throws IOException, InterruptedException {
Thread.sleep(20000);
}

public void testCreateConnector() throws IOException {
Response response = createConnector(completionModelConnectorEntity);
Map responseMap = parseResponseToMap(response);
assertNotNull((String) responseMap.get("connector_id"));
}

public void testGetConnector() throws IOException {
public void testCreate_Get_DeleteConnector() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we have combined the 3 api calls, does the error message give enough information to validate at which step (create, get, delete) the test failed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If any assertion fails, it will indicate which assertion is failing within the method, correct? I’ve noticed a lot of redundant resource creation in this integration test class, which I plan to remove step by step. In this case, we are now creating the connector resource once and reusing it for three tests, rather than creating the resource three separate times.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, i misunderstood the change - assumed it was a connection test to the connector itself during creation phase via API and not integ tests. Thanks for clearing this up!

Response response = createConnector(completionModelConnectorEntity);
Map responseMap = parseResponseToMap(response);
String connectorId = (String) responseMap.get("connector_id");
assertNotNull(connectorId); // Testing create connector

// Testing Get connector
response = TestHelper.makeRequest(client(), "GET", "/_plugins/_ml/connectors/" + connectorId, null, "", null);
responseMap = parseResponseToMap(response);
assertEquals("OpenAI Connector", (String) responseMap.get("name"));
assertEquals("1", (String) responseMap.get("version"));
assertEquals("The connector to public OpenAI model service for GPT 3.5", (String) responseMap.get("description"));
assertEquals("http", (String) responseMap.get("protocol"));
}
assertEquals("OpenAI Connector", responseMap.get("name"));
assertEquals("1", responseMap.get("version"));
assertEquals("The connector to public OpenAI model service for GPT 3.5", responseMap.get("description"));
assertEquals("http", responseMap.get("protocol"));

public void testDeleteConnector() throws IOException {
Response response = createConnector(completionModelConnectorEntity);
Map responseMap = parseResponseToMap(response);
String connectorId = (String) responseMap.get("connector_id");
// Testing delete connector
response = TestHelper.makeRequest(client(), "DELETE", "/_plugins/_ml/connectors/" + connectorId, null, "", null);
responseMap = parseResponseToMap(response);
assertEquals("deleted", (String) responseMap.get("result"));
assertEquals("deleted", responseMap.get("result"));

}

private static String maskSensitiveInfo(String input) {
Copy link
Contributor

@pyek-bot pyek-bot Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: can add a simple test case for this

// Regex to remove the whole credential object and replace it with "***"
String regex = "\"credential\":\\{.*?}";
return input.replaceAll(regex, "\"credential\": \"***\"");
}

@Test
public void testMaskSensitiveInfo_withCredential() {
String input = "{\"credential\":{\"username\":\"admin\",\"password\":\"secret\"}}";
String expectedOutput = "{\"credential\": \"***\"}";
String actualOutput = maskSensitiveInfo(input);
assertEquals(expectedOutput, actualOutput);
}

@Test
public void testMaskSensitiveInfo_noCredential() {
String input = "{\"otherInfo\":\"someValue\"}";
String expectedOutput = "{\"otherInfo\":\"someValue\"}";
String actualOutput = maskSensitiveInfo(input);
assertEquals(expectedOutput, actualOutput);
}

@Test
public void testMaskSensitiveInfo_emptyInput() {
String input = "";
String expectedOutput = "";
String actualOutput = maskSensitiveInfo(input);
assertEquals(expectedOutput, actualOutput);
}

public void testSearchConnectors_beforeCreation() throws IOException {
String searchEntity = "{\n" + " \"query\": {\n" + " \"match_all\": {}\n" + " },\n" + " \"size\": 1000\n" + "}";
Response response = TestHelper
.makeRequest(client(), "GET", "/_plugins/_ml/connectors/_search", null, TestHelper.toHttpEntity(searchEntity), null);
Map responseMap = parseResponseToMap(response);
assertEquals((Double) 0.0, (Double) ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
assertEquals(0.0, ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
}

public void testSearchConnectors_afterCreation() throws IOException {
Expand All @@ -125,7 +151,7 @@ public void testSearchRemoteModels_beforeCreation() throws IOException {
Response response = TestHelper
.makeRequest(client(), "GET", "/_plugins/_ml/models/_search", null, TestHelper.toHttpEntity(searchEntity), null);
Map responseMap = parseResponseToMap(response);
assertEquals((Double) 0.0, (Double) ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
assertEquals(0.0, ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
}

public void testSearchRemoteModels_afterCreation() throws IOException {
Expand All @@ -134,15 +160,15 @@ public void testSearchRemoteModels_afterCreation() throws IOException {
Response response = TestHelper
.makeRequest(client(), "GET", "/_plugins/_ml/models/_search", null, TestHelper.toHttpEntity(searchEntity), null);
Map responseMap = parseResponseToMap(response);
assertEquals((Double) 1.0, (Double) ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
assertEquals(1.0, ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
}

public void testSearchModelGroups_beforeCreation() throws IOException {
String searchEntity = "{\n" + " \"query\": {\n" + " \"match_all\": {}\n" + " },\n" + " \"size\": 1000\n" + "}";
Response response = TestHelper
.makeRequest(client(), "GET", "/_plugins/_ml/model_groups/_search", null, TestHelper.toHttpEntity(searchEntity), null);
Map responseMap = parseResponseToMap(response);
assertEquals((Double) 0.0, (Double) ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
assertEquals(0.0, ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
}

public void testSearchModelGroups_afterCreation() throws IOException {
Expand All @@ -151,15 +177,15 @@ public void testSearchModelGroups_afterCreation() throws IOException {
Response response = TestHelper
.makeRequest(client(), "GET", "/_plugins/_ml/model_groups/_search", null, TestHelper.toHttpEntity(searchEntity), null);
Map responseMap = parseResponseToMap(response);
assertEquals((Double) 1.0, (Double) ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
assertEquals(1.0, ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
}

public void testSearchMLTasks_beforeCreation() throws IOException {
String searchEntity = "{\n" + " \"query\": {\n" + " \"match_all\": {}\n" + " },\n" + " \"size\": 1000\n" + "}";
Response response = TestHelper
.makeRequest(client(), "GET", "/_plugins/_ml/tasks/_search", null, TestHelper.toHttpEntity(searchEntity), null);
Map responseMap = parseResponseToMap(response);
assertEquals((Double) 0.0, (Double) ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
assertEquals(0.0, ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
}

public void testSearchMLTasks_afterCreation() throws IOException {
Expand All @@ -169,7 +195,7 @@ public void testSearchMLTasks_afterCreation() throws IOException {
Response response = TestHelper
.makeRequest(client(), "GET", "/_plugins/_ml/tasks/_search", null, TestHelper.toHttpEntity(searchEntity), null);
Map responseMap = parseResponseToMap(response);
assertEquals((Double) 1.0, (Double) ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
assertEquals(1.0, ((Map) ((Map) responseMap.get("hits")).get("total")).get("value"));
}

public void testDeployRemoteModel() throws IOException, InterruptedException {
Expand All @@ -185,7 +211,7 @@ public void testDeployRemoteModel() throws IOException, InterruptedException {
String modelId = (String) responseMap.get("model_id");
response = deployRemoteModel(modelId);
responseMap = parseResponseToMap(response);
assertEquals("COMPLETED", (String) responseMap.get("status"));
assertEquals("COMPLETED", responseMap.get("status"));
taskId = (String) responseMap.get("task_id");
waitForTask(taskId, MLTaskState.COMPLETED);
}
Expand Down Expand Up @@ -838,7 +864,12 @@ public void testCohereClassifyModel() throws IOException, InterruptedException {
}

public static Response createConnector(String input) throws IOException {
return TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/connectors/_create", null, TestHelper.toHttpEntity(input), null);
try {
return TestHelper.makeRequest(client(), "POST", "/_plugins/_ml/connectors/_create", null, TestHelper.toHttpEntity(input), null);
} catch (ResponseException e) {
String sanitizedMessage = maskSensitiveInfo(e.getMessage());// Log sanitized message
throw new RuntimeException("Request failed: " + sanitizedMessage); // Re-throw sanitized exception
}
}

public static Response registerRemoteModel(String name, String connectorId) throws IOException {
Expand Down
Loading