Skip to content

Commit

Permalink
Clusters details per rule (#1296)
Browse files Browse the repository at this point in the history 
* Update insights-operator-utils version

* New endpoint for list of clusters currently affected by a given rule

* Update OpenAPI specification

* Add TestDBStorageListClustersForHittingRules (nominal use case)

* Additional UT coverage

* Fix and reformat OpenAPI specification

* Add UTs for new server endpoint and ensure 404 error when no rows for given query parameters

* Fixes for linters and other checks

* Fix OpenAPI specification (broken in previous commit)

* Fix UTs not updated after latest changes

* Update tests that rely on httputils.CheckPermissions

* Small reference to new endpoint in docs

* Add filtering by list of active clusters received from smart proxy (+UTs)

* New helper function 'inClauseFromSlice' to remove duplicated code
  • Loading branch information
@epapbak
epapbak authored Oct 19, 2021
1 parent 445c3a5 commit 6cc54c2
Show file tree
Hide file tree
Showing 18 changed files with 612 additions and 87 deletions.
14 changes: 14 additions & 0 deletions docs/rest_api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,17 @@ curl -k -v $ADDRESS/organizations/{orgId}/clusters/reports -d @cluster_list.json
```
/organizations/{orgId}/clusters/{clusterId}/users/{userId}/rules/{ruleId}
```

#### List of clusters for a given rule selector (plugin_name|error_key) within the specified organization ID

```
/rules/{rule_selector}/organizations/{org_id}/users/{user_id}/clusters_detail
```

##### Usage:

```
curl -k -v $ADDRESS/rules/{rule_selector}/organizations/{org_id}/users/{user_id}/clusters_detail
```

Plase note that user ID is expected, but is only used for improving logging.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/BurntSushi/toml v0.3.1
github.com/DATA-DOG/go-sqlmock v1.4.1
github.com/RedHatInsights/insights-content-service v0.0.0-20201009081018-083923779f00
github.com/RedHatInsights/insights-operator-utils v1.21.0
github.com/RedHatInsights/insights-operator-utils v1.21.1-0.20211013104539-199e7104dbb0
github.com/RedHatInsights/insights-results-aggregator-data v1.3.0
github.com/Shopify/sarama v1.27.1
github.com/deckarep/golang-set v1.7.1
Expand Down
29 changes: 28 additions & 1 deletion go.sum
View file

Large diffs are not rendered by default.

172 changes: 140 additions & 32 deletions openapi.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,9 @@
"properties": {
"cluster": {
"type": "string",
"minLength": 36,
"maxLength": 36,
"format": "uuid"
"minLength": 36,
"maxLength": 36,
"format": "uuid"
},
"meta": {
"type": "object",
Expand Down Expand Up @@ -411,12 +411,12 @@
}
],
"requestBody": {
"description": "List of cluster IDs. Each ID must conform to UUID format. An example: `34c3ecc5-624a-49a5-bab8-4fdc5e51a266.",
"required": true,
"content": {
"text/plain": {
"schema": {
}
"description": "List of cluster IDs. Each ID must conform to UUID format. An example: `34c3ecc5-624a-49a5-bab8-4fdc5e51a266.",
"required": true,
"content": {
"text/plain": {
"schema": {
}
}
}
},
Expand Down Expand Up @@ -456,9 +456,9 @@
"properties": {
"cluster": {
"type": "string",
"minLength": 36,
"maxLength": 36,
"format": "uuid"
"minLength": 36,
"maxLength": 36,
"format": "uuid"
},
"meta": {
"type": "object",
Expand Down Expand Up @@ -1209,12 +1209,12 @@
}
],
"requestBody": {
"description": "List of cluster IDs. An example: `34c3ecc5-624a-49a5-bab8-4fdc5e51a266.",
"required": true,
"content": {
"application/json": {
"schema": {
}
"description": "List of cluster IDs. An example: `34c3ecc5-624a-49a5-bab8-4fdc5e51a266.",
"required": true,
"content": {
"application/json": {
"schema": {
}
}
}
},
Expand Down Expand Up @@ -1483,12 +1483,12 @@
}
],
"requestBody": {
"description": "Justification why rule was disabled",
"required": true,
"content": {
"text/plain": {
"schema": {
}
"description": "Justification why rule was disabled",
"required": true,
"content": {
"text/plain": {
"schema": {
}
}
}
},
Expand Down Expand Up @@ -1547,12 +1547,12 @@
}
],
"requestBody": {
"description": "Justification why rule was disabled",
"required": true,
"content": {
"text/plain": {
"schema": {
}
"description": "Justification why rule was disabled",
"required": true,
"content": {
"text/plain": {
"schema": {
}
}
}
},
Expand Down Expand Up @@ -1659,7 +1659,9 @@
},
"/rules/organizations/{orgId}/users/{userId}/rating": {
"post": {
"tags": ["prod"],
"tags": [
"prod"
],
"parameters": [
{
"name": "orgId",
Expand Down Expand Up @@ -1698,7 +1700,7 @@
"content": {
"application/json": {
"schema": {
"$ref":"#/components/schemas/ratingSchema"
"$ref": "#/components/schemas/ratingSchema"
}
}
},
Expand All @@ -1719,6 +1721,112 @@
}
}
}
},
"/rules/{rule_selector}/organizations/{org_id}/users/{user_id}/clusters_detail": {
"get": {
"summary": "Returns a list of cluster where the given rule is active.",
"description": "Provided a correct rule selector and a valid organization ID, this method will query the DB for all the organization's clusters being currently affected by the rule, if any.",
"operationId": "getClustersForRecommendation",
"parameters": [
{
"name": "rule_selector",
"in": "path",
"required": true,
"description": "Recommendation identifier, in the plugin_name|error_key format",
"schema": {
"type": "string"
},
"example": "existing.plugin.name|ERROR_KEY"
},
{
"name": "org_id",
"in": "path",
"required": true,
"description": "Numeric ID of the organization",
"schema": {
"type": "string"
},
"example": "123422"
},
{
"name": "user_id",
"in": "path",
"required": true,
"description": "Numeric ID of the user",
"schema": {
"type": "string"
},
"example": "42"
}
],
"responses": {
"200": {
"description": "List of cluster data for provided rule",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"report": {
"type": "object",
"properties": {
"meta": {
"type": "object",
"properties": {
"count": {
"type": "integer",
"description": "Number of clusters affected by the received rule.",
"example": "1"
},
"component": {
"type": "string",
"description": "The rule identifier included in the received rule selector.",
"example": "some.python.module"
},
"error_key": {
"type": "string",
"description": "The erroy key included in the received rule selector.",
"example": "SOME_ERROR_KEY"
}
}
},
"data": {
"type": "array",
"items": {
"type": "object",
"properties": {
"cluster": {
"type": "string",
"minLength": 36,
"maxLength": 36,
"format": "uuid"
}
}
}
}
}
},
"status": {
"type": "string",
"example": "ok"
}
}
}
}
}
},
"404": {
"description": "Resource not found, usually caused when some rule selector, organization or user doesn't exist"
},
"500": {
"description": "Internal server error, should only be returned if unexpected error happened when querying from the DB"
}
},
"tags": [
"rule",
"prod"
]
}
}
},
"security": [],
Expand Down
8 changes: 6 additions & 2 deletions server/auth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package server_test

import (
"fmt"
"net/http"
"testing"

Expand Down Expand Up @@ -137,13 +138,16 @@ func TestInvalidJsonAuthToken(t *testing.T) {

// TestBadOrganizationID checks if organization ID is checked properly
func TestBadOrganizationID(t *testing.T) {
providedOrgID := 12345
orgIDInXRH := 1234
body := fmt.Sprintf(`{"status":"you have no permissions to get or change info about the organization with ID %v; you can access info about organization with ID %v"}`, providedOrgID, orgIDInXRH)
helpers.AssertAPIRequest(t, nil, &configAuth, &helpers.APIRequest{
Method: http.MethodGet,
Endpoint: server.ClustersForOrganizationEndpoint,
EndpointArgs: []interface{}{12345},
EndpointArgs: []interface{}{providedOrgID},
XRHIdentity: "eyJpZGVudGl0eSI6IHsiaW50ZXJuYWwiOiB7Im9yZ19pZCI6ICIxMjM0In19fQo=",
}, &helpers.APIResponse{
StatusCode: http.StatusForbidden,
Body: `{"status":"you have no permissions to get or change info about this organization"}`,
Body: body,
})
}
3 changes: 3 additions & 0 deletions server/endpoints.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ const (
ListOfDisabledRules = "rules/users/{user_id}/disabled"
// ListOfDisabledRulesFeedback returns a list of reasons why rule has been disabled
ListOfDisabledRulesFeedback = "rules/users/{user_id}/disabled/feedback"
// RuleClusterDetailEndpoint returns a list of clusters affected by a given rule for current account
RuleClusterDetailEndpoint = "rules/{rule_selector}/organizations/{org_id}/users/{user_id}/clusters_detail"

// Endpoints to handle rules to be enabled, disabled, updated, and queried system-wide

Expand Down Expand Up @@ -123,6 +125,7 @@ func (server *HTTPServer) addEndpointsToRouter(router *mux.Router) {
router.HandleFunc(apiPrefix+ListOfDisabledRules, server.listOfDisabledRules).Methods(http.MethodGet)
router.HandleFunc(apiPrefix+ListOfDisabledRulesFeedback, server.listOfReasons).Methods(http.MethodGet)
router.HandleFunc(apiPrefix+Rating, server.setRuleRating).Methods(http.MethodPost)
router.HandleFunc(apiPrefix+RuleClusterDetailEndpoint, server.RuleClusterDetailEndpoint).Methods(http.MethodGet)

// Rule Enable/Disable/etc endpoints
server.addRuleEnableDisableEndpointsToRouter(router, apiPrefix)
Expand Down
5 changes: 3 additions & 2 deletions server/reports.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ const (

// OkStatusPayload is the text returned as body payload when an OK Status request is sent
OkStatusPayload = "ok"

// orgIDStr used in log messages
orgIDStr = "orgID"
// userIDstr used in log messages
userIDstr = "userID"
)

// validateClusterID function checks if the cluster ID is a valid UUID.
Expand Down Expand Up @@ -248,7 +249,7 @@ func (server *HTTPServer) getRecommendations(writer http.ResponseWriter, request
// everything has been handled
return
}
log.Info().Str("userID", string(userID)).Msg("getRecommendations")
log.Info().Str(userIDstr, string(userID)).Msg("getRecommendations")

// extract org_id from URL
orgID, ok := readOrgID(writer, request)
Expand Down
30 changes: 2 additions & 28 deletions server/router_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,10 @@ import (
"github.com/RedHatInsights/insights-results-aggregator/types"
)

// ClusterList is a data structure that store list of cluster IDs (names).
type ClusterList struct {
Clusters []string `json:"clusters"`
}

var (
readRuleID = httputils.ReadRuleID
readErrorKey = httputils.ReadErrorKey
readRuleSelector = httputils.ReadRuleSelector
getRouterParam = httputils.GetRouterParam
getRouterPositiveIntParam = httputils.GetRouterPositiveIntParam
validateClusterName = httputils.ValidateClusterName
Expand All @@ -46,6 +42,7 @@ var (
checkPermissions = httputils.CheckPermissions
readClusterNames = httputils.ReadClusterNames
readOrganizationIDs = httputils.ReadOrganizationIDs
readClusterListFromBody = httputils.ReadClusterListFromBody
)

// readUserID retrieves user_id from request
Expand Down Expand Up @@ -100,29 +97,6 @@ func readClusterListFromPath(writer http.ResponseWriter, request *http.Request)
return clusterList, true
}

// readClusterListFromBody retrieves list of clusters from request's body
// if it's not possible, it writes http error to the writer and returns false
func readClusterListFromBody(writer http.ResponseWriter, request *http.Request) ([]string, bool) {
var clusterList ClusterList

// check if there's any body provided in the request sent by client
if request.ContentLength <= 0 {
err := &NoBodyError{}
handleServerError(writer, err)
return []string{}, false
}

// try to read cluster list from request parameter
err := json.NewDecoder(request.Body).Decode(&clusterList)
if err != nil {
handleServerError(writer, err)
return []string{}, false
}

// everything seems ok -> return list of clusters
return clusterList.Clusters, true
}

func readRuleIDWithErrorKey(writer http.ResponseWriter, request *http.Request) (types.RuleID, types.ErrorKey, bool) {
ruleIDWithErrorKey, err := getRouterParam(request, "rule_id")
if err != nil {
Expand Down
Loading

0 comments on commit 6cc54c2

Please sign in to comment.