Support creating/updating prometheus rules

[lezzago]

Description

Add CRUD support for the Prometheus/Cortex/AMP Ruler API through the Direct Query resources framework. This enables creating, reading, updating, and deleting recording and alerting rule groups via the OpenSearch
SQL plugin.

New capabilities:

• GET /api/v1/rules/{namespace} — Retrieve rules for a specific namespace
• POST /api/v1/rules/{namespace} — Create or update a rule group (YAML body)
• DELETE /api/v1/rules/{namespace} — Delete all rules in a namespace
• DELETE /api/v1/rules/{namespace}/{groupName} — Delete a specific rule group

Response normalization: The Ruler API returns different formats depending on the backend (Prometheus JSON, Cortex/Thanos YAML map, AMP JSON). A new normalizeRulesResponse() layer handles all three and returns a
consistent {"groups":[...]} structure.

Configurable Ruler URI: A new prometheus.ruler.uri datasource property allows configuring a separate base URL for Ruler operations. This is needed for Cortex where the Ruler API lives at a different path than the
query API (e.g., http://host:9009 vs http://host:9009/api/prom). Defaults to prometheus.uri when not set.

Behavioral change to getRules(): The existing getRules() method was updated to use the same response normalization. Previously it only handled Prometheus JSON ({"status":"success","data":{...}}); now it also
handles Cortex/Thanos YAML responses. The return shape is unchanged ({"groups":[...]}), and a resource leak (unclosed HTTP response) was fixed with try-with-resources. Error messages now use a different format
("Ruler request failed with code: ..." instead of the generic readResponse() error).

Related Issues

N/A

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit b5480e9)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

📝 TODO sections direct-query/src/main/java/org/opensearch/sql/directquery/rest/RestDirectQueryResourcesManagementAction.java [65]: Update routes to be more generic and move away from prometheus direct-query/src/main/java/org/opensearch/sql/directquery/transport/format/DirectQueryResourcesRequestConverter.java [32]: Move prometheus code into prometheus module/classes

🔀 Multiple PR themes Sub-PR theme: Add Ruler API client methods to PrometheusClient Relevant files: direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClient.java direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java direct-query-core/src/main/java/org/opensearch/sql/prometheus/utils/PrometheusClientUtils.java direct-query-core/src/test/java/org/opensearch/sql/prometheus/client/PrometheusClientImplTest.java direct-query-core/src/test/java/org/opensearch/sql/prometheus/utils/PrometheusClientUtilsTest.java Sub-PR theme: Wire Ruler API into REST layer and query handler Relevant files: direct-query-core/src/main/java/org/opensearch/sql/directquery/rest/model/WriteDirectQueryResourcesRequest.java direct-query-core/src/main/java/org/opensearch/sql/prometheus/query/PrometheusQueryHandler.java direct-query/src/main/java/org/opensearch/sql/directquery/rest/RestDirectQueryResourcesManagementAction.java direct-query/src/main/java/org/opensearch/sql/directquery/transport/format/DirectQueryResourcesRequestConverter.java direct-query/src/main/java/org/opensearch/sql/directquery/transport/model/WriteDirectQueryResourcesActionRequest.java direct-query/src/test/java/org/opensearch/sql/directquery/rest/RestDirectQueryResourcesManagementActionTest.java direct-query/src/test/java/org/opensearch/sql/directquery/transport/format/DirectQueryResourcesRequestConverterTest.java direct-query/src/test/java/org/opensearch/sql/directquery/transport/model/WriteDirectQueryResourcesActionRequestTest.java direct-query-core/src/test/java/org/opensearch/sql/prometheus/query/PrometheusQueryHandlerTest.java

⚡ Recommended focus areas for review Wrong HTTP Client In getRulesByNamespace, createOrUpdateRuleGroup, deleteRuleNamespace, and deleteRuleGroup, the code uses this.prometheusHttpClient to make requests to the rulerUri. If the ruler endpoint requires different authentication or TLS settings than the Prometheus endpoint, this will silently use the wrong client. A dedicated rulerHttpClient should be considered, similar to how alertmanagerHttpClient is handled separately. try (Response response = AccessController.doPrivilegedChecked( () -> this.prometheusHttpClient.newCall(request).execute())) { String body = readRulerResponse(response, "GET namespace " + namespace); return normalizeRulesResponse(body, namespace); } } @Override public String createOrUpdateRuleGroup(String namespace, String yamlBody) throws IOException { String queryUrl = String.format( "%s/api/v1/rules/%s", rulerUri.toString().replaceAll("/$", ""), URLEncoder.encode(namespace, StandardCharsets.UTF_8)); logger.debug("Making Ruler POST request to create/update rule group"); Request request = new Request.Builder() .url(queryUrl) .header("Content-Type", "application/yaml") .post(RequestBody.create(yamlBody.getBytes(StandardCharsets.UTF_8))) .build(); try (Response response = AccessController.doPrivilegedChecked( () -> this.prometheusHttpClient.newCall(request).execute())) { String body = readRulerResponse(response, "POST create/update rule group"); return body.isEmpty() ? "{\"status\":\"success\"}" : body; } } @Override public String deleteRuleNamespace(String namespace) throws IOException { String queryUrl = String.format( "%s/api/v1/rules/%s", rulerUri.toString().replaceAll("/$", ""), URLEncoder.encode(namespace, StandardCharsets.UTF_8)); logger.debug("Making Ruler DELETE request for namespace"); Request request = new Request.Builder().url(queryUrl).delete().build(); try (Response response = AccessController.doPrivilegedChecked( () -> this.prometheusHttpClient.newCall(request).execute())) { String body = readRulerResponse(response, "DELETE namespace " + namespace); return body.isEmpty() ? "{\"status\":\"success\"}" : body; } } @Override public String deleteRuleGroup(String namespace, String groupName) throws IOException { String queryUrl = String.format( "%s/api/v1/rules/%s/%s", rulerUri.toString().replaceAll("/$", ""), URLEncoder.encode(namespace, StandardCharsets.UTF_8), URLEncoder.encode(groupName, StandardCharsets.UTF_8)); logger.debug("Making Ruler DELETE request for group"); Request request = new Request.Builder().url(queryUrl).delete().build(); try (Response response = AccessController.doPrivilegedChecked( () -> this.prometheusHttpClient.newCall(request).execute())) { String body = readRulerResponse(response, "DELETE group " + groupName); return body.isEmpty() ? "{\"status\":\"success\"}" : body; } } Null Body Handling In readRulerResponse, when the response is successful, Objects.requireNonNull is used which throws a NullPointerException (not a PrometheusClientException) if the body is null. This is inconsistent with the error path which returns "No response body". A null body on a successful response should be handled gracefully (e.g., return empty string) rather than throwing NPE. if (response.isSuccessful()) { return Objects.requireNonNull(response.body(), "Ruler response body is null").string(); Missing GET Route There is no GET route defined for /api/v1/rules/{namespace}/{groupName} in RestDirectQueryResourcesManagementAction, but the converter handles namespace-based GET requests. Additionally, there is no route for fetching all rules (GET /api/v1/rules) without a namespace via the new ruler path — the existing generic route handles it, but the path-based routing logic in the converter may conflict or be ambiguous. } else if (path.contains("/api/v1/rules/") && restRequest.param("namespace") != null) { // Handle Ruler API - GET /api/v1/rules/{namespace} String namespace = restRequest.param("namespace").trim(); if (namespace.isEmpty()) { throw new IllegalArgumentException("Namespace cannot be empty"); } directQueryRequest.setResourceType(DirectQueryResourceType.RULES); directQueryRequest.setResourceName(namespace); } else { Naming Convention The field RequestOptions (line 23) uses PascalCase instead of camelCase (requestOptions), which violates Java naming conventions. This appears to be a pre-existing issue not introduced in this PR, but the new fields groupName and delete follow correct conventions. The inconsistency should be noted. private String groupName; private boolean delete; Single-Constructor Default The single-argument constructor PrometheusClientImpl(OkHttpClient, URI) now sets rulerUri to the same value as prometheusUri (via the chained constructor). This means by default the ruler API calls go to the same base URI as Prometheus (e.g., http://prometheus:9090/api/v1/rules/...). For Cortex/Thanos, the ruler typically runs on a different port/path. This default may silently produce incorrect behavior for users who don't configure prometheus.ruler.uri. public PrometheusClientImpl(OkHttpClient prometheusHttpClient, URI prometheusUri) { this( prometheusHttpClient, prometheusUri, prometheusHttpClient, URI.create(prometheusUri.toString().replaceAll("/$", "") + "/alertmanager"), prometheusUri);

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to b5480e9

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Handle null body on successful response gracefully When response.isSuccessful() is true but response.body() is null, Objects.requireNonNull throws a NullPointerException instead of a PrometheusClientException, breaking the consistent error-handling contract. Consider returning an empty string (or throwing a PrometheusClientException) when the body is null on a successful response, similar to how the error path handles it. direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java [426-429] if (response.isSuccessful()) { - return Objects.requireNonNull(response.body(), "Ruler response body is null").string(); + return response.body() != null ? response.body().string() : ""; } else { String errorBody = response.body() != null ? response.body().string() : "No response body"; Suggestion importance[1-10]: 6 __ Why: When response.isSuccessful() is true but response.body() is null, Objects.requireNonNull throws a NullPointerException rather than a PrometheusClientException, breaking the consistent error-handling contract. The fix is valid and improves robustness, though this is an edge case.	Low
General	Set MediaType directly on RequestBody for YAML POST RequestBody.create(byte[]) is deprecated in newer OkHttp versions and does not set a MediaType, which may cause the server to reject the request or misinterpret the content type even though the Content-Type header is set separately. Pass the MediaType directly to RequestBody.create to ensure it is correctly associated with the body. direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java [366-371] Request request = new Request.Builder() .url(queryUrl) - .header("Content-Type", "application/yaml") - .post(RequestBody.create(yamlBody.getBytes(StandardCharsets.UTF_8))) + .post(RequestBody.create( + yamlBody.getBytes(StandardCharsets.UTF_8), + okhttp3.MediaType.parse("application/yaml"))) .build(); Suggestion importance[1-10]: 5 __ Why: Using RequestBody.create(byte[]) without a MediaType is deprecated and may cause issues with some servers. Passing the MediaType directly to RequestBody.create is the correct approach, though the separate Content-Type header partially mitigates the issue.	Low
	Guard against non-Map elements in YAML list parsing The unchecked cast (List<Map<String, Object>>) parsed will succeed even if the list contains non-Map elements (e.g., plain strings), causing a ClassCastException inside the loop when new JSONObject(group) is called. The outer catch (Exception e) in normalizeRulesResponse will swallow this silently, but it is better to guard against it explicitly with an instanceof check per element. direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java [499-503] } else if (parsed instanceof List) { // Single-namespace format: List<RuleGroup> - List<Map<String, Object>> groups = (List<Map<String, Object>>) parsed; - for (Map<String, Object> group : groups) { - JSONObject groupObj = new JSONObject(group); + List<?> rawGroups = (List<?>) parsed; + for (Object item : rawGroups) { + if (!(item instanceof Map)) { + throw new IllegalArgumentException("Unexpected list element type: " + item.getClass()); + } + JSONObject groupObj = new JSONObject((Map<?, ?>) item); Suggestion importance[1-10]: 4 __ Why: The unchecked cast can cause a ClassCastException if the list contains non-Map elements, which is silently swallowed by the outer catch. The suggestion improves robustness, though the outer catch already handles this gracefully by returning empty groups.	Low
	Simplify nullable Boolean unboxing to avoid potential NPE The new fields groupName and delete are read after requestOptions in the deserialization constructor, but they must also be written in the exact same order in writeTo. Verify that writeTo writes groupName and delete after requestOptions; if the order ever diverges between the two methods, deserialization will silently corrupt all fields that follow. Additionally, deleteValue != null ? deleteValue : false can be simplified to Boolean.TRUE.equals(deleteValue). direct-query/src/main/java/org/opensearch/sql/directquery/transport/model/WriteDirectQueryResourcesActionRequest.java [41-43] request.setGroupName(in.readOptionalString()); -Boolean deleteValue = in.readOptionalBoolean(); -request.setDelete(deleteValue != null ? deleteValue : false); +request.setDelete(Boolean.TRUE.equals(in.readOptionalBoolean())); Suggestion importance[1-10]: 3 __ Why: The suggestion to use Boolean.TRUE.equals(in.readOptionalBoolean()) is a minor simplification of the existing null-safe unboxing pattern. The existing code is already correct and safe, so this is a minor style improvement.	Low

---

Previous suggestions

Suggestions up to commit c047583

Category	Suggestion	Impact
Possible issue	Avoid NPE on null successful response body Objects.requireNonNull(response.body()) will throw a NullPointerException if the response body is null on a successful response, rather than a meaningful error. The error-path already handles null body gracefully with a null check, so the success path should be consistent. Use a null-safe check here as well. direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java [243-244] if (response.isSuccessful()) { - String bodyString = Objects.requireNonNull(response.body()).string(); + if (response.body() == null) { + throw new PrometheusClientException("Ruler request returned a successful response but with no body"); + } + String bodyString = response.body().string(); Suggestion importance[1-10]: 6 __ Why: Using Objects.requireNonNull on a successful response body throws a generic NullPointerException rather than a meaningful PrometheusClientException. The error path already handles null body gracefully, so the success path should be consistent. This is a valid improvement but not a critical bug since successful responses typically have a body.	Low
General	Handle backward compatibility in stream deserialization The read order in the constructor must exactly match the write order in writeTo. Currently writeTo writes groupName then delete, and the constructor reads them in the same order — this is correct. However, readBoolean will throw if the stream ends unexpectedly (e.g., when deserializing older serialized data that doesn't have these fields). Consider using in.readOptionalBoolean() or adding a version check for backward compatibility. direct-query/src/main/java/org/opensearch/sql/directquery/transport/model/WriteDirectQueryResourcesActionRequest.java [41-42] request.setGroupName(in.readOptionalString()); -request.setDelete(in.readBoolean()); +Boolean deleteValue = in.readOptionalBoolean(); +request.setDelete(deleteValue != null && deleteValue); Suggestion importance[1-10]: 5 __ Why: Using readBoolean() instead of readOptionalBoolean() could cause deserialization failures when reading older serialized data that doesn't include the new delete field. This is a valid backward compatibility concern for distributed systems where nodes may be running different versions.	Low
	Associate MediaType directly with request body RequestBody.create(byte[]) is deprecated in newer OkHttp versions and does not associate a MediaType with the body. The Content-Type header is set separately, but it's better practice to pass the MediaType directly to RequestBody.create to ensure consistency and avoid potential issues with some HTTP clients or proxies. direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java [380-385] +MediaType yamlMediaType = MediaType.parse("application/yaml"); Request request = new Request.Builder() .url(queryUrl) - .header("Content-Type", "application/yaml") - .post(RequestBody.create(yamlBody.getBytes(StandardCharsets.UTF_8))) + .post(RequestBody.create(yamlBody, yamlMediaType)) .build(); Suggestion importance[1-10]: 4 __ Why: Using RequestBody.create(byte[]) without a MediaType is deprecated in newer OkHttp versions and sets the Content-Type header separately. While functional, passing MediaType directly to RequestBody.create is better practice and more consistent, but this is a minor style/API usage improvement.	Low
	Fix field naming convention violation The field RequestOptions violates Java naming conventions — field names should start with a lowercase letter. While this is pre-existing, the new fields groupName and delete are correctly named. If RequestOptions is serialized/deserialized (e.g., via Jackson), the inconsistent naming may cause issues when the new fields are also serialized. Ensure the class-level serialization annotations are consistent for all fields. direct-query-core/src/main/java/org/opensearch/sql/directquery/rest/model/WriteDirectQueryResourcesRequest.java [23-25] -private Map<String, String> RequestOptions; +private Map<String, String> requestOptions; private String groupName; private boolean delete; Suggestion importance[1-10]: 2 __ Why: The RequestOptions field naming violation is pre-existing and not introduced by this PR. The suggestion to rename it could break existing serialization/deserialization behavior and is out of scope for this PR's changes.	Low

Suggestions up to commit 3a17d23

Category	Suggestion	Impact
Possible issue	Add null check before accessing JSON field The getRulesByNamespace method calls readResponse which may throw an exception if the response is not successful, but if the response body doesn't contain a "data" key, a JSONException will be thrown without a meaningful error message. Add a null/existence check before accessing the "data" key to provide a clearer error. direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java [335-336] JSONObject jsonObject = readResponse(response); +if (!jsonObject.has("data")) { + throw new PrometheusClientException("Unexpected response format: missing 'data' field"); +} return jsonObject.getJSONObject("data"); Suggestion importance[1-10]: 5 __ Why: The suggestion is valid - accessing jsonObject.getJSONObject("data") without checking if the key exists could throw an unhelpful JSONException. However, this is a minor defensive coding improvement rather than a critical bug fix.	Low
	Ensure serialization order matches deserialization order The serialization order in writeTo must exactly match the deserialization order in the constructor. The new fields groupName and delete are written after requestOptions in writeTo, but they must also be read in the same order. Verify that the writeTo method writes groupName before delete to match this read order, otherwise deserialization will be corrupted. direct-query/src/main/java/org/opensearch/sql/directquery/transport/model/WriteDirectQueryResourcesActionRequest.java [41-42] +request.setGroupName(in.readOptionalString()); +request.setDelete(in.readBoolean()); - Suggestion importance[1-10]: 2 __ Why: The existing_code and improved_code are identical, making this a verification suggestion rather than an actual code change. The PR already shows writeTo writes groupName before delete (lines 62-63), matching the read order, so the concern is already addressed.	Low
General	Use MediaType in RequestBody creation The RequestBody.create(byte[]) method without a MediaType parameter is deprecated in newer OkHttp versions and may not set the content type correctly. The Content-Type header is set separately, but it's better to pass the MediaType directly to RequestBody.create to ensure consistency. direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java [347-352] +MediaType yamlMediaType = MediaType.parse("application/yaml"); Request request = new Request.Builder() .url(queryUrl) - .header("Content-Type", "application/yaml") - .post(RequestBody.create(yamlBody.getBytes(StandardCharsets.UTF_8))) + .post(RequestBody.create(yamlBody.getBytes(StandardCharsets.UTF_8), yamlMediaType)) .build(); Suggestion importance[1-10]: 4 __ Why: The suggestion to pass MediaType directly to RequestBody.create is a valid improvement for consistency and to avoid potential deprecation issues, but it's a minor style/API usage improvement rather than a critical fix.	Low
	Fix field naming convention inconsistency The field RequestOptions violates Java naming conventions (should be requestOptions with a lowercase first letter). While this may be pre-existing, the new fields groupName and delete follow correct conventions. This inconsistency could cause issues with JSON deserialization frameworks that rely on field naming conventions. direct-query-core/src/main/java/org/opensearch/sql/directquery/rest/model/WriteDirectQueryResourcesRequest.java [23-25] -private Map<String, String> RequestOptions; +private Map<String, String> requestOptions; private String groupName; private boolean delete; Suggestion importance[1-10]: 3 __ Why: The RequestOptions field naming violation is pre-existing and not introduced by this PR. The suggestion is valid for code quality but is out of scope for this PR and could break existing code that references this field.	Low

Suggestions up to commit 98d6a33

Category	Suggestion	Impact
Possible issue	Guard against missing 'data' key in response The readResponse method may throw an exception if the response is not successful, but if it returns a JSONObject that doesn't contain a "data" key, calling getJSONObject("data") will throw a JSONException. Add a null/key check before accessing the "data" field to avoid unexpected runtime exceptions. direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java [335-337] Response response = AccessController.doPrivilegedChecked( () -> this.prometheusHttpClient.newCall(request).execute()); JSONObject jsonObject = readResponse(response); +if (!jsonObject.has("data")) { + throw new PrometheusClientException("Unexpected response format: missing 'data' field"); +} return jsonObject.getJSONObject("data"); Suggestion importance[1-10]: 6 __ Why: If the response JSON doesn't contain a "data" key, getJSONObject("data") will throw a JSONException at runtime. Adding a key check before accessing it would make the error handling more explicit and informative.	Low
General	Set media type on request body correctly RequestBody.create(byte[]) is deprecated in newer OkHttp versions and the overload without a MediaType may not set the content type correctly on the wire. The Content-Type header is set separately, but the RequestBody itself should also carry the media type to ensure correct behavior. Pass the media type directly to RequestBody.create. direct-query-core/src/main/java/org/opensearch/sql/prometheus/client/PrometheusClientImpl.java [347-352] Request request = new Request.Builder() .url(queryUrl) - .header("Content-Type", "application/yaml") - .post(RequestBody.create(yamlBody.getBytes(StandardCharsets.UTF_8))) + .post(RequestBody.create(yamlBody.getBytes(StandardCharsets.UTF_8), + okhttp3.MediaType.parse("application/yaml"))) .build(); Suggestion importance[1-10]: 4 __ Why: The Content-Type header is set separately via .header(), but the RequestBody itself doesn't carry the media type. While this may work in practice, passing the MediaType directly to RequestBody.create is the more correct and idiomatic OkHttp approach.	Low
	Derive resource type from request path dynamically The toDeleteDirectRestRequest method hardcodes DirectQueryResourceType.RULES regardless of the request path or parameters. If additional resource types are supported for DELETE in the future, this will silently misclassify requests. Consider deriving the resource type from the request path or a parameter, consistent with how toGetDirectRestRequest and toWriteDirectRestRequest handle routing. direct-query/src/main/java/org/opensearch/sql/directquery/transport/format/DirectQueryResourcesRequestConverter.java [117-122] -deleteRequest.setResourceType(DirectQueryResourceType.RULES); +String path = restRequest.path(); +if (path.contains("/api/v1/rules/")) { + deleteRequest.setResourceType(DirectQueryResourceType.RULES); +} else { + deleteRequest.setResourceTypeFromString(restRequest.param("resourceType")); +} String namespace = restRequest.param("namespace"); if (namespace == null || namespace.isEmpty()) { throw new IllegalArgumentException("Namespace is required for delete rule operations"); } Suggestion importance[1-10]: 3 __ Why: The hardcoded RULES resource type is consistent with the current DELETE routes which only support rules, so this is a minor extensibility concern rather than a current bug. The suggestion is valid for future-proofing but has low immediate impact.	Low

[github-actions]

Persistent review updated to latest commit 3a17d23

[github-actions]

Persistent review updated to latest commit c047583

[github-actions]

Persistent review updated to latest commit b5480e9