From 4631ec9c030a41c81215fad6017685b4b77c177c Mon Sep 17 00:00:00 2001 From: Pavel Tisnovsky Date: Fri, 30 Oct 2020 11:56:41 +0100 Subject: [PATCH 1/2] Cluster list handling code --- http/router_utils.go | 40 ++++++++++++ http/router_utils_test.go | 125 ++++++++++++++++++++++++++++++++++++++ types/types.go | 5 ++ 3 files changed, 170 insertions(+) diff --git a/http/router_utils.go b/http/router_utils.go index c1bab334..9f2e3064 100644 --- a/http/router_utils.go +++ b/http/router_utils.go @@ -15,6 +15,8 @@ package httputils import ( + "encoding/json" + "errors" "fmt" "net/http" "regexp" @@ -247,3 +249,41 @@ func handleClusterNameError(writer http.ResponseWriter, err error) { func SplitRequestParamArray(arrayParam string) []string { return strings.Split(arrayParam, ",") } + +// ReadClusterListFromPath retrieves list of clusters from request's path +// if it's not possible, it writes http error to the writer and returns false +func ReadClusterListFromPath(writer http.ResponseWriter, request *http.Request) ([]string, bool) { + rawClusterList, err := GetRouterParam(request, "cluster_list") + if err != nil { + types.HandleServerError(writer, err) + return []string{}, false + } + + // basic check that should not happen in reality (because of Gorilla mux checks) + if rawClusterList == "" { + types.HandleServerError(writer, errors.New("cluster list is empty")) + return []string{}, false + } + + // split the list into items + clusterList := strings.Split(rawClusterList, ",") + + // everything seems ok -> return list of clusters + 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 types.ClusterListInRequest + + // try to read cluster list from request parameter + err := json.NewDecoder(request.Body).Decode(&clusterList) + if err != nil { + types.HandleServerError(writer, err) + return []string{}, false + } + + // everything seems ok -> return list of clusters + return clusterList.Clusters, true +} diff --git a/http/router_utils_test.go b/http/router_utils_test.go index fefff12d..4c5ca7e0 100644 --- a/http/router_utils_test.go +++ b/http/router_utils_test.go @@ -34,6 +34,11 @@ import ( "github.com/RedHatInsights/insights-operator-utils/types" ) +const ( + cluster1ID = "715e10eb-e6ac-49b3-bd72-61734c35b6fb" + cluster2ID = "931f1495-7b16-4637-a41e-963e117bfd02" +) + func TestGetRouterPositiveIntParam_NonIntError(t *testing.T) { request := mustGetRequestWithMuxVars(t, http.MethodGet, "", nil, map[string]string{ "id": "non int", @@ -351,3 +356,123 @@ func paramsToString(separator string, params ...interface{}) string { res := strings.Join(stringParams, separator) return res } + +// TestReadClusterListFromPathMissingClusterList function checks if missing +// cluster list in path is detected and processed correctly by function +// ReadClusterListFromPath. +func TestReadClusterListFromPathMissingClusterList(t *testing.T) { + request, err := http.NewRequest(http.MethodGet, "", nil) + helpers.FailOnError(t, err) + + // try to read list of clusters from path + _, successful := httputils.ReadClusterListFromPath(httptest.NewRecorder(), request) + + // missing list means that the read operation should fail + assert.False(t, successful) +} + +// TestReadClusterListFromPathEmptyClusterList function checks if empty cluster +// list in path is detected and processed correctly by function +// ReadClusterListFromPath. +func TestReadClusterListFromPathEmptyClusterList(t *testing.T) { + request := mustGetRequestWithMuxVars(t, http.MethodGet, "", nil, map[string]string{ + "cluster_list": "", + }) + + // try to read list of clusters from path + _, successful := httputils.ReadClusterListFromPath(httptest.NewRecorder(), request) + + // empty list means that the read operation should fail + assert.False(t, successful) +} + +// TestReadClusterListFromPathOneCluster function checks if list with one +// cluster ID is processed correctly by function ReadClusterListFromPath. +func TestReadClusterListFromPathOneCluster(t *testing.T) { + request := mustGetRequestWithMuxVars(t, http.MethodGet, "", nil, map[string]string{ + "cluster_list": fmt.Sprintf("%v", cluster1ID), + }) + + // try to read list of clusters from path + list, successful := httputils.ReadClusterListFromPath(httptest.NewRecorder(), request) + + // cluster list exists so the read operation should not fail + assert.True(t, successful) + + // we expect do get list with one cluster ID + assert.ElementsMatch(t, list, []string{cluster1ID}) +} + +// TestReadClusterListFromPathTwoClusters function checks if list with two +// cluster IDs is processed correctly by function ReadClusterListFromPath. +func TestReadClusterListFromPathTwoClusters(t *testing.T) { + request := mustGetRequestWithMuxVars(t, http.MethodGet, "", nil, map[string]string{ + "cluster_list": fmt.Sprintf("%v,%v", cluster1ID, cluster2ID), + }) + + // try to read list of clusters from path + list, successful := httputils.ReadClusterListFromPath(httptest.NewRecorder(), request) + + // cluster list exists so the read operation should not fail + assert.True(t, successful) + + // we expect do get list with two cluster IDs + assert.ElementsMatch(t, list, []string{cluster1ID, cluster2ID}) +} + +// TestReadClusterListFromBodyNoJSON function checks if reading list of +// clusters from empty request body is detected properly by function +// ReadClusterListFromBody. +func TestReadClusterListFromBodyNoJSON(t *testing.T) { + request, err := http.NewRequest( + http.MethodGet, + "", + strings.NewReader(""), + ) + helpers.FailOnError(t, err) + + // try to read list of clusters from path + _, successful := httputils.ReadClusterListFromBody(httptest.NewRecorder(), request) + + // the read should fail because of empty request body + assert.False(t, successful) +} + +// TestReadClusterListFromBodyCorrectJSON function checks if reading list of +// clusters from correct request body containing JSON data is done correctly by +// function ReadClusterListFromBody. +func TestReadClusterListFromBodyCorrectJSON(t *testing.T) { + request, err := http.NewRequest( + http.MethodGet, + "", + strings.NewReader(fmt.Sprintf(`{"clusters": ["%v","%v"]}`, cluster1ID, cluster2ID)), + ) + helpers.FailOnError(t, err) + + // try to read list of clusters from path + list, successful := httputils.ReadClusterListFromBody(httptest.NewRecorder(), request) + + // cluster list exists so the call should not fail + assert.True(t, successful) + + // we expect do get list with two cluster IDs + assert.ElementsMatch(t, list, []string{cluster1ID, cluster2ID}) +} + +// TestReadClusterListFromBodyWrongJSON function checks if reading list of +// clusters from request body with improper format is processed correctly by +// function ReadClusterListFromBody. +func TestReadClusterListFromBodyWrongJSON(t *testing.T) { + request, err := http.NewRequest( + http.MethodGet, + "", + strings.NewReader("this-is-not-json"), + ) + helpers.FailOnError(t, err) + + // try to read list of clusters from path + _, successful := httputils.ReadClusterListFromBody(httptest.NewRecorder(), request) + + // the read should fail because of broken JSON + assert.False(t, successful) +} diff --git a/types/types.go b/types/types.go index 155847ee..ead43659 100644 --- a/types/types.go +++ b/types/types.go @@ -196,3 +196,8 @@ func (e *ValidationError) Error() string { e.ParamName, e.ParamValue, e.ErrString, ) } + +// ClusterListInRequest represents request body containing list of clusters +type ClusterListInRequest struct { + Clusters []string `json:"clusters"` +} From bec407eb166a20ebd2ea16155a3b57605deed689 Mon Sep 17 00:00:00 2001 From: Pavel Tisnovsky Date: Fri, 30 Oct 2020 11:56:54 +0100 Subject: [PATCH 2/2] Updated generated documentation --- docs/packages/http/router_utils.html | 75 +++++++ docs/packages/http/router_utils_test.html | 245 ++++++++++++++++++++++ docs/packages/types/content.html | 2 +- docs/packages/types/types.html | 10 + 4 files changed, 331 insertions(+), 1 deletion(-) diff --git a/docs/packages/http/router_utils.html b/docs/packages/http/router_utils.html index 965c8402..83714e60 100644 --- a/docs/packages/http/router_utils.html +++ b/docs/packages/http/router_utils.html @@ -134,6 +134,8 @@ 
package
 
httputils

 
 
import
 
(

+	
"encoding/json"

+	
"errors"

 	
"fmt"

 	
"net/http"

 	
"regexp"

@@ -432,6 +434,79 @@
 	
return
 
strings
.
Split
(
arrayParam
,
 
","
)

 
}

 
+
+ + + +

ReadClusterListFromPath retrieves list of clusters from request's path +if it's not possible, it writes http error to the writer and returns false

+ + 
func
 
ReadClusterListFromPath
(
writer
 
http
.
ResponseWriter
,
 
request
 
*
http
.
Request
)
 
(
[
]
string
,
 
bool
)
 
{

+	
rawClusterList
,
 
err
 
:=
 
GetRouterParam
(
request
,
 
"cluster_list"
)

+	
if
 
err
 
!=
 
nil
 
{

+		
types
.
HandleServerError
(
writer
,
 
err
)

+		
return
 
[
]
string
{
}
,
 
false

+	
}

+
+
+ + + +

basic check that should not happen in reality (because of Gorilla mux checks)

+ + 
	
if
 
rawClusterList
 
==
 
""
 
{

+		
types
.
HandleServerError
(
writer
,
 
errors
.
New
(
"cluster list is empty"
)
)

+		
return
 
[
]
string
{
}
,
 
false

+	
}

+
+
+ + + +

split the list into items

+ + 
	
clusterList
 
:=
 
strings
.
Split
(
rawClusterList
,
 
","
)

+
+
+ + + +

everything seems ok -> return list of clusters

+ + 
	
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
 
types
.
ClusterListInRequest

+
+
+ + + +

try to read cluster list from request parameter

+ + 
	
err
 
:=
 
json
.
NewDecoder
(
request
.
Body
)
.
Decode
(
&
clusterList
)

+	
if
 
err
 
!=
 
nil
 
{

+		
types
.
HandleServerError
(
writer
,
 
err
)

+		
return
 
[
]
string
{
}
,
 
false

+	
}

+
+
+ + + +

everything seems ok -> return list of clusters

+ + 
	
return
 
clusterList
.
Clusters
,
 
true

+
}

+
diff --git a/docs/packages/http/router_utils_test.html b/docs/packages/http/router_utils_test.html index 41c58759..e968e67f 100644 --- a/docs/packages/http/router_utils_test.html +++ b/docs/packages/http/router_utils_test.html @@ -153,6 +153,11 @@
"github.com/RedHatInsights/insights-operator-utils/types"
)
+
const
(
+
cluster1ID
=
"715e10eb-e6ac-49b3-bd72-61734c35b6fb"
+
cluster2ID
=
"931f1495-7b16-4637-a41e-963e117bfd02"
+
)
+
func
TestGetRouterPositiveIntParam_NonIntError
(
t
*
testing
.
T
)
{
request
:=
mustGetRequestWithMuxVars
(
t
,
http
.
MethodGet
,
""
,
nil
,
map
[
string
]
string
{
"id"
:
"non int"
,
@@ -471,6 +476,246 @@
return
res
}
+ + + + +

TestReadClusterListFromPathMissingClusterList function checks if missing +cluster list in path is detected and processed correctly by function +ReadClusterListFromPath.

+ + 
func
 
TestReadClusterListFromPathMissingClusterList
(
t
 
*
testing
.
T
)
 
{

+	
request
,
 
err
 
:=
 
http
.
NewRequest
(
http
.
MethodGet
,
 
""
,
 
nil
)

+	
helpers
.
FailOnError
(
t
,
 
err
)

+
+
+ + + +

try to read list of clusters from path

+ + 
	
_
,
 
successful
 
:=
 
httputils
.
ReadClusterListFromPath
(
httptest
.
NewRecorder
(
)
,
 
request
)

+
+
+ + + +

missing list means that the read operation should fail

+ + 
	
assert
.
False
(
t
,
 
successful
)

+
}

+
+
+ + + +

TestReadClusterListFromPathEmptyClusterList function checks if empty cluster +list in path is detected and processed correctly by function +ReadClusterListFromPath.

+ + 
func
 
TestReadClusterListFromPathEmptyClusterList
(
t
 
*
testing
.
T
)
 
{

+	
request
 
:=
 
mustGetRequestWithMuxVars
(
t
,
 
http
.
MethodGet
,
 
""
,
 
nil
,
 
map
[
string
]
string
{

+		
"cluster_list"
:
 
""
,

+	
}
)

+
+
+ + + +

try to read list of clusters from path

+ + 
	
_
,
 
successful
 
:=
 
httputils
.
ReadClusterListFromPath
(
httptest
.
NewRecorder
(
)
,
 
request
)

+
+
+ + + +

empty list means that the read operation should fail

+ + 
	
assert
.
False
(
t
,
 
successful
)

+
}

+
+
+ + + +

TestReadClusterListFromPathOneCluster function checks if list with one +cluster ID is processed correctly by function ReadClusterListFromPath.

+ + 
func
 
TestReadClusterListFromPathOneCluster
(
t
 
*
testing
.
T
)
 
{

+	
request
 
:=
 
mustGetRequestWithMuxVars
(
t
,
 
http
.
MethodGet
,
 
""
,
 
nil
,
 
map
[
string
]
string
{

+		
"cluster_list"
:
 
fmt
.
Sprintf
(
"%v"
,
 
cluster1ID
)
,

+	
}
)

+
+
+ + + +

try to read list of clusters from path

+ + 
	
list
,
 
successful
 
:=
 
httputils
.
ReadClusterListFromPath
(
httptest
.
NewRecorder
(
)
,
 
request
)

+
+
+ + + +

cluster list exists so the read operation should not fail

+ + 
	
assert
.
True
(
t
,
 
successful
)

+
+
+ + + +

we expect do get list with one cluster ID

+ + 
	
assert
.
ElementsMatch
(
t
,
 
list
,
 
[
]
string
{
cluster1ID
}
)

+
}

+
+
+ + + +

TestReadClusterListFromPathTwoClusters function checks if list with two +cluster IDs is processed correctly by function ReadClusterListFromPath.

+ + 
func
 
TestReadClusterListFromPathTwoClusters
(
t
 
*
testing
.
T
)
 
{

+	
request
 
:=
 
mustGetRequestWithMuxVars
(
t
,
 
http
.
MethodGet
,
 
""
,
 
nil
,
 
map
[
string
]
string
{

+		
"cluster_list"
:
 
fmt
.
Sprintf
(
"%v,%v"
,
 
cluster1ID
,
 
cluster2ID
)
,

+	
}
)

+
+
+ + + +

try to read list of clusters from path

+ + 
	
list
,
 
successful
 
:=
 
httputils
.
ReadClusterListFromPath
(
httptest
.
NewRecorder
(
)
,
 
request
)

+
+
+ + + +

cluster list exists so the read operation should not fail

+ + 
	
assert
.
True
(
t
,
 
successful
)

+
+
+ + + +

we expect do get list with two cluster IDs

+ + 
	
assert
.
ElementsMatch
(
t
,
 
list
,
 
[
]
string
{
cluster1ID
,
 
cluster2ID
}
)

+
}

+
+
+ + + +

TestReadClusterListFromBodyNoJSON function checks if reading list of +clusters from empty request body is detected properly by function +ReadClusterListFromBody.

+ + 
func
 
TestReadClusterListFromBodyNoJSON
(
t
 
*
testing
.
T
)
 
{

+	
request
,
 
err
 
:=
 
http
.
NewRequest
(

+		
http
.
MethodGet
,

+		
""
,

+		
strings
.
NewReader
(
""
)
,

+	
)

+	
helpers
.
FailOnError
(
t
,
 
err
)

+
+
+ + + +

try to read list of clusters from path

+ + 
	
_
,
 
successful
 
:=
 
httputils
.
ReadClusterListFromBody
(
httptest
.
NewRecorder
(
)
,
 
request
)

+
+
+ + + +

the read should fail because of empty request body

+ + 
	
assert
.
False
(
t
,
 
successful
)

+
}

+
+
+ + + +

TestReadClusterListFromBodyCorrectJSON function checks if reading list of +clusters from correct request body containing JSON data is done correctly by +function ReadClusterListFromBody.

+ + 
func
 
TestReadClusterListFromBodyCorrectJSON
(
t
 
*
testing
.
T
)
 
{

+	
request
,
 
err
 
:=
 
http
.
NewRequest
(

+		
http
.
MethodGet
,

+		
""
,

+		
strings
.
NewReader
(
fmt
.
Sprintf
(
`{"clusters": ["%v","%v"]}`
,
 
cluster1ID
,
 
cluster2ID
)
)
,

+	
)

+	
helpers
.
FailOnError
(
t
,
 
err
)

+
+
+ + + +

try to read list of clusters from path

+ + 
	
list
,
 
successful
 
:=
 
httputils
.
ReadClusterListFromBody
(
httptest
.
NewRecorder
(
)
,
 
request
)

+
+
+ + + +

cluster list exists so the call should not fail

+ + 
	
assert
.
True
(
t
,
 
successful
)

+
+
+ + + +

we expect do get list with two cluster IDs

+ + 
	
assert
.
ElementsMatch
(
t
,
 
list
,
 
[
]
string
{
cluster1ID
,
 
cluster2ID
}
)

+
}

+
+
+ + + +

TestReadClusterListFromBodyWrongJSON function checks if reading list of +clusters from request body with improper format is processed correctly by +function ReadClusterListFromBody.

+ + 
func
 
TestReadClusterListFromBodyWrongJSON
(
t
 
*
testing
.
T
)
 
{

+	
request
,
 
err
 
:=
 
http
.
NewRequest
(

+		
http
.
MethodGet
,

+		
""
,

+		
strings
.
NewReader
(
"this-is-not-json"
)
,

+	
)

+	
helpers
.
FailOnError
(
t
,
 
err
)

+
+
+ + + +

try to read list of clusters from path

+ + 
	
_
,
 
successful
 
:=
 
httputils
.
ReadClusterListFromBody
(
httptest
.
NewRecorder
(
)
,
 
request
)

+
+
+ + + +

the read should fail because of broken JSON

+ + 
	
assert
.
False
(
t
,
 
successful
)

+
}

+
diff --git a/docs/packages/types/content.html b/docs/packages/types/content.html index d6b86bd7..7cbf66f2 100644 --- a/docs/packages/types/content.html +++ b/docs/packages/types/content.html @@ -134,7 +134,7 @@
MoreInfo
json
.
RawMessage
`json:"more_info"`
Plugin
RulePluginInfo
`json:"plugin"`
ErrorKeys
map
[
string
]
RuleErrorKeyContent
`json:"error_keys"`
-
hasReason
bool
+
HasReason
bool
}
diff --git a/docs/packages/types/types.html b/docs/packages/types/types.html index 8d5cbf05..04441788 100644 --- a/docs/packages/types/types.html +++ b/docs/packages/types/types.html @@ -461,6 +461,16 @@
)
}
+ + + + +

ClusterListInRequest represents request body containing list of clusters

+ + 
type
 
ClusterListInRequest
 
struct
 
{

+	
Clusters
 
[
]
string
 
`json:"clusters"`

+
}

+