Merge branch 'master' into dependabot/go_modules/github.com/openshift-online/ocm-sdk-go-0.1.497

Author: lenasolarova

.github/workflows/bdd.yaml

@@ -0,0 +1,27 @@
+# Copyright 2023-2026 Red Hat, Inc
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: BDD tests
+
+on:
+  push:
+    branches: ["master", "main"]
+  pull_request:
+
+jobs:
+  bdd:
+    name: BDD tests
+    uses: RedHatInsights/processing-tools/.github/workflows/bdd.yaml@master
+    with:
+      service: smart-proxy

.github/workflows/bdd.yml

@@ -488,9 +488,9 @@
     },
     "/cluster/{clusterId}/request/{requestId}/status": {
       "get": {
-        "summary": "Check status of a given cluster's request",
+        "summary": "Check the status of a given request_id, representing a single archive uploaded by the Insights Operator to Ingress. Doesn't distinguish between regular archives and those collected on-demand",
         "operationId": "getRequestStatusForCluster",
-        "description": "The status of the request with given ID if found. The cluster ID is required so only requests for the given cluster are returned, if any. Response should have the following format: ```\n{\n\"cluster\": \"{clusterID}\",\n\"requestID\":\"{requestID}\",\n\"status\": \"{string}\"\n}\n```",
+        "description": "This endpoint is to be used as a simple check whether a given cluster archive has been processed successfully. It's primarily used by the Insights Operator's on-demand data gathering feature to check whether an archive has been processed and stored successfully. The response should have the following format: ```\n{\n\"cluster\": \"{clusterID}\",\n\"requestID\":\"{requestID}\",\n\"status\": \"{string}\"\n}\n```",
         "parameters": [
           {
             "name": "clusterId",
@@ -512,7 +512,7 @@
         ],
         "responses": {
           "200": {
-            "description": "Stored status for the given cluster and request IDs",
+            "description": "A successful response means that the given request_id (a single Insights Operator archive uploaded to Ingress) has been successfully processed, stored, and the Insights results are available for retrieval",
             "content": {
               "application/json": {
                 "schema": {
@@ -538,13 +538,13 @@
             }
           },
           "400": {
-            "description": "Invalid request or invalid cluster ID"
+            "description": "Invalid request, invalid cluster_id param or invalid request_id param"
           },
           "403": {
             "$ref": "#/components/responses/unauthorized"
           },
           "404": {
-            "description": "Request ID or cluster ID not found"
+            "description": "Request ID not found for given org_id and cluster_id. Doesn't distinguish between different processing states. The 404 response simply indicates that we don't have any information about the given request_id"
           }
         }
       }

pr_check.sh

@@ -166,7 +166,8 @@ func (server *HTTPServer) addV2EndpointsToRouter(router *mux.Router) {
 }
 
 // addV2RedisEndpointsToRouter method registers handlers for endpoints that depend on our Redis storage
-// to provide responses.
+// to provide responses. These endpoints are used by the insights-operator, which is a versioned product,
+// meaning these endpoints have to be 100% backwards compatible.
 func (server *HTTPServer) addV2RedisEndpointsToRouter(router *mux.Router, apiPrefix string) {
 	router.HandleFunc(apiPrefix+ListAllRequestIDs, server.getRequestsForCluster).Methods(http.MethodGet)
 	router.HandleFunc(apiPrefix+ListAllRequestIDs, server.getRequestsForClusterPostVariant).Methods(http.MethodPost)

server/api/v2/openapi.json

@@ -1281,8 +1281,10 @@ func (server *HTTPServer) processClustersDetailResponse(
 	return responses.Send(http.StatusOK, writer, response)
 }
 
-// getRequestStatusForCluster method implements endpoint that should return a status
-// for given request ID.
+// getRequestStatusForCluster implements an endpoint returning the status of a
+// given request ID.
+// This endpoint was previously causing a performance issue affecting the insights-operator. We need to ensure
+// that this endpoint will always be 100% backwards compatible.
 func (server *HTTPServer) getRequestStatusForCluster(writer http.ResponseWriter, request *http.Request) {
 	orgID, err := server.GetCurrentOrgID(request)
 	if err != nil {
@@ -1305,42 +1307,28 @@ func (server *HTTPServer) getRequestStatusForCluster(writer http.ResponseWriter,
 
 	// make sure we don't access server.redis when it's nil
 	if !server.checkRedisClientReadiness(writer) {
-		// error has been handled already
+		// error handled by function
 		return
 	}
 
-	// get request ID list from Redis using SCAN command
-	requestIDsForCluster, err := server.redis.GetRequestIDsForClusterID(orgID, clusterID)
+	// perform EXISTS query to check whether an archive (request_id) was processed
+	err = server.redis.GetRequestStatus(orgID, clusterID, requestID)
 	if err != nil {
-		handleServerError(writer, err)
-		return
-	}
-	if len(requestIDsForCluster) == 0 {
-		err := responses.SendNotFound(writer, RequestsForClusterNotFound)
-		if err != nil {
-			log.Error().Err(err).Msg(responseDataError)
-		}
-		return
-	}
-
-	// try to find the required request ID in list of requests IDs from Redis
-	var found bool
-	for _, storedRequestID := range requestIDsForCluster {
-		if storedRequestID == requestID {
-			found = true
-			break
-		}
-	}
-
-	if !found {
-		err := responses.SendNotFound(writer, RequestIDNotFound)
-		if err != nil {
-			log.Error().Err(err).Msg(responseDataError)
+		switch err.(type) {
+		case *utypes.ItemNotFoundError:
+			// keep same message as in the original endpoint
+			err := responses.SendNotFound(writer, RequestIDNotFound)
+			if err != nil {
+				log.Error().Err(err).Msg(responseDataError)
+			}
+			return
+		default:
+			handleServerError(writer, err)
+			return
 		}
-		return
 	}
 
-	// prepare data structure
+	// prepare response
 	responseData := map[string]interface{}{}
 	responseData["cluster"] = string(clusterID)
 	responseData["requestID"] = requestID