From 483bf624eece809e39eb5fd9cd59d493e3b10d56 Mon Sep 17 00:00:00 2001 From: Jociele Padilha <45459238+jopadi@users.noreply.github.com> Date: Thu, 14 Apr 2022 10:02:54 +0200 Subject: [PATCH] add test team member (#1842) * return err if teams API fails with StatusCode other than 404 * add unit test for 404 at team members Co-authored-by: Jociele Padilha Co-authored-by: Felix Kunde --- pkg/cluster/cluster_test.go | 10 +- pkg/cluster/util.go | 10 +- pkg/util/teams/teams.go | 21 +- pkg/util/teams/teams_test.go | 468 ++++++++++++++++++----------------- 4 files changed, 267 insertions(+), 242 deletions(-) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index acfecc166..f531962d1 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -194,8 +194,8 @@ type mockTeamsAPIClient struct { members []string } -func (m *mockTeamsAPIClient) TeamInfo(teamID, token string) (tm *teams.Team, err error) { - return &teams.Team{Members: m.members}, nil +func (m *mockTeamsAPIClient) TeamInfo(teamID, token string) (tm *teams.Team, statusCode int, err error) { + return &teams.Team{Members: m.members}, statusCode, nil } func (m *mockTeamsAPIClient) setMembers(members []string) { @@ -260,15 +260,15 @@ type mockTeamsAPIClientMultipleTeams struct { teams []mockTeam } -func (m *mockTeamsAPIClientMultipleTeams) TeamInfo(teamID, token string) (tm *teams.Team, err error) { +func (m *mockTeamsAPIClientMultipleTeams) TeamInfo(teamID, token string) (tm *teams.Team, statusCode int, err error) { for _, team := range m.teams { if team.teamID == teamID { - return &teams.Team{Members: team.members}, nil + return &teams.Team{Members: team.members}, statusCode, nil } } // should not be reached if a slice with teams is populated correctly - return nil, nil + return nil, statusCode, nil } // Test adding members of maintenance teams that get superuser rights for all PG databases diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index cf320e057..0f71d2d64 100644 --- a/pkg/cluster/util.go +++ b/pkg/cluster/util.go @@ -6,6 +6,7 @@ import ( "encoding/gob" "encoding/json" "fmt" + "net/http" "reflect" "sort" "strings" @@ -272,9 +273,14 @@ func (c *Cluster) getTeamMembers(teamID string) ([]string, error) { return nil, fmt.Errorf("could not get oauth token to authenticate to team service API: %v", err) } - teamInfo, err := c.teamsAPIClient.TeamInfo(teamID, token) + teamInfo, statusCode, err := c.teamsAPIClient.TeamInfo(teamID, token) + if err != nil { - c.logger.Warningf("could not get team info for team %q: %v", teamID, err) + if statusCode == http.StatusNotFound { + c.logger.Warningf("could not get team info for team %q: %v", teamID, err) + } else { + return nil, fmt.Errorf("could not get team info for team %q: %v", teamID, err) + } } else { for _, member := range teamInfo.Members { if !(util.SliceContains(members, member)) { diff --git a/pkg/util/teams/teams.go b/pkg/util/teams/teams.go index d7413ab9c..bf1260d6e 100644 --- a/pkg/util/teams/teams.go +++ b/pkg/util/teams/teams.go @@ -45,7 +45,7 @@ type httpClient interface { // Interface to the TeamsAPIClient type Interface interface { - TeamInfo(teamID, token string) (tm *Team, err error) + TeamInfo(teamID, token string) (tm *Team, statusCode int, err error) } // API describes teams API @@ -67,7 +67,7 @@ func NewTeamsAPI(url string, log *logrus.Entry) *API { } // TeamInfo returns information about a given team using its ID and a token to authenticate to the API service. -func (t *API) TeamInfo(teamID, token string) (tm *Team, err error) { +func (t *API) TeamInfo(teamID, token string) (tm *Team, statusCode int, err error) { var ( req *http.Request resp *http.Response @@ -77,37 +77,38 @@ func (t *API) TeamInfo(teamID, token string) (tm *Team, err error) { t.logger.Debugf("request url: %s", url) req, err = http.NewRequest("GET", url, nil) if err != nil { - return nil, err + return nil, http.StatusBadRequest, err } req.Header.Add("Authorization", "Bearer "+token) if resp, err = t.httpClient.Do(req); err != nil { - return nil, err + return nil, http.StatusUnauthorized, err } defer func() { if closeErr := resp.Body.Close(); closeErr != nil { err = fmt.Errorf("error when closing response: %v", closeErr) } }() - if resp.StatusCode != 200 { + statusCode = resp.StatusCode + if statusCode != http.StatusOK { var raw map[string]json.RawMessage d := json.NewDecoder(resp.Body) err = d.Decode(&raw) if err != nil { - return nil, fmt.Errorf("team API query failed with status code %d and malformed response: %v", resp.StatusCode, err) + return nil, statusCode, fmt.Errorf("team API query failed with status code %d and malformed response: %v", statusCode, err) } if errMessage, ok := raw["error"]; ok { - return nil, fmt.Errorf("team API query failed with status code %d and message: '%v'", resp.StatusCode, string(errMessage)) + return nil, statusCode, fmt.Errorf("team API query failed with status code %d and message: '%v'", statusCode, string(errMessage)) } - return nil, fmt.Errorf("team API query failed with status code %d", resp.StatusCode) + return nil, statusCode, fmt.Errorf("team API query failed with status code %d", statusCode) } tm = &Team{} d := json.NewDecoder(resp.Body) if err = d.Decode(tm); err != nil { - return nil, fmt.Errorf("could not parse team API response: %v", err) + return nil, statusCode, fmt.Errorf("could not parse team API response: %v", err) } - return tm, nil + return tm, statusCode, nil } diff --git a/pkg/util/teams/teams_test.go b/pkg/util/teams/teams_test.go index 33d01b75b..da9f497c1 100644 --- a/pkg/util/teams/teams_test.go +++ b/pkg/util/teams/teams_test.go @@ -1,225 +1,243 @@ -package teams - -import ( - "fmt" - "net/http" - "net/http/httptest" - "reflect" - "testing" - - "github.com/sirupsen/logrus" -) - -var ( - logger = logrus.New().WithField("pkg", "teamsapi") - token = "ec45b1cfbe7100c6315d183a3eb6cec0M2U1LWJkMzEtZDgzNzNmZGQyNGM3IiwiYXV0aF90aW1lIjoxNDkzNzMwNzQ1LCJpc3MiOiJodHRwcz" -) - -var teamsAPItc = []struct { - in string - inCode int - out *Team - err error -}{ - {`{ -"dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", -"id": "acid", -"id_name": "acid", -"team_id": "111222", -"type": "official", -"name": "Acid team name", -"mail": [ -"email1@example.com", -"email2@example.com" -], -"alias": [ -"acid" -], -"member": [ - "member1", - "member2", - "member3" -], -"infrastructure-accounts": [ -{ - "id": "1234512345", - "name": "acid", - "provider": "aws", - "type": "aws", - "description": "", - "owner": "acid", - "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", - "disabled": false -}, -{ - "id": "5432154321", - "name": "db", - "provider": "aws", - "type": "aws", - "description": "", - "owner": "acid", - "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", - "disabled": false -} -], -"cost_center": "00099999", -"delivery_lead": "member4", -"parent_team_id": "111221" -}`, - 200, - &Team{ - Dn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", - ID: "acid", - TeamName: "acid", - TeamID: "111222", - Type: "official", - FullName: "Acid team name", - Aliases: []string{"acid"}, - Mails: []string{"email1@example.com", "email2@example.com"}, - Members: []string{"member1", "member2", "member3"}, - CostCenter: "00099999", - DeliveryLead: "member4", - ParentTeamID: "111221", - InfrastructureAccounts: []infrastructureAccount{ - { - ID: "1234512345", - Name: "acid", - Provider: "aws", - Type: "aws", - Description: "", - Owner: "acid", - OwnerDn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", - Disabled: false}, - { - ID: "5432154321", - Name: "db", - Provider: "aws", - Type: "aws", - Description: "", - Owner: "acid", - OwnerDn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", - Disabled: false}, - }, - }, - nil}, { - `{"error": "Access Token not valid"}`, - 401, - nil, - fmt.Errorf(`team API query failed with status code 401 and message: '"Access Token not valid"'`), - }, - { - `{"status": "I'm a teapot'"}`, - 418, - nil, - fmt.Errorf(`team API query failed with status code 418`), - }, - { - `{"status": "I'm a teapot`, - 418, - nil, - fmt.Errorf(`team API query failed with status code 418 and malformed response: unexpected EOF`), - }, - { - `{"status": "I'm a teapot`, - 200, - nil, - fmt.Errorf(`could not parse team API response: unexpected EOF`), - }, -} - -var requestsURLtc = []struct { - url string - err error -}{ - { - "coffee://localhost/", - fmt.Errorf(`Get "coffee://localhost/teams/acid": unsupported protocol scheme "coffee"`), - }, - { - "http://192.168.0.%31/", - fmt.Errorf(`parse "http://192.168.0.%%31/teams/acid": invalid URL escape "%%31"`), - }, -} - -func TestInfo(t *testing.T) { - for _, tc := range teamsAPItc { - func() { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Header.Get("Authorization") != "Bearer "+token { - t.Errorf("authorization token is wrong or not provided") - } - w.WriteHeader(tc.inCode) - if _, err := fmt.Fprint(w, tc.in); err != nil { - t.Errorf("error writing teams api response %v", err) - } - })) - defer ts.Close() - api := NewTeamsAPI(ts.URL, logger) - - actual, err := api.TeamInfo("acid", token) - if err != nil && err.Error() != tc.err.Error() { - t.Errorf("expected error: %v, got: %v", tc.err, err) - return - } - - if !reflect.DeepEqual(actual, tc.out) { - t.Errorf("expected %#v, got: %#v", tc.out, actual) - } - }() - } -} - -type mockHTTPClient struct { -} - -type mockBody struct { -} - -func (b *mockBody) Read(p []byte) (n int, err error) { - return 2, nil -} - -func (b *mockBody) Close() error { - return fmt.Errorf("close error") -} - -func (c *mockHTTPClient) Do(req *http.Request) (*http.Response, error) { - resp := http.Response{ - Status: "200 OK", - StatusCode: 200, - ContentLength: 2, - Close: false, - Request: req, - } - resp.Body = &mockBody{} - - return &resp, nil -} - -func TestHttpClientClose(t *testing.T) { - ts := httptest.NewServer(nil) - - api := NewTeamsAPI(ts.URL, logger) - api.httpClient = &mockHTTPClient{} - - _, err := api.TeamInfo("acid", token) - expError := fmt.Errorf("error when closing response: close error") - if err.Error() != expError.Error() { - t.Errorf("expected error: %v, got: %v", expError, err) - } -} - -func TestRequest(t *testing.T) { - for _, tc := range requestsURLtc { - api := NewTeamsAPI(tc.url, logger) - resp, err := api.TeamInfo("acid", token) - if resp != nil { - t.Errorf("response expected to be nil") - continue - } - - if err.Error() != tc.err.Error() { - t.Errorf("expected error: %v, got: %v", tc.err, err) - } - } -} +package teams + +import ( + "fmt" + "net/http" + "net/http/httptest" + "reflect" + "testing" + + "github.com/sirupsen/logrus" +) + +var ( + logger = logrus.New().WithField("pkg", "teamsapi") + token = "ec45b1cfbe7100c6315d183a3eb6cec0M2U1LWJkMzEtZDgzNzNmZGQyNGM3IiwiYXV0aF90aW1lIjoxNDkzNzMwNzQ1LCJpc3MiOiJodHRwcz" + input = `{ + "dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + "id": "acid", + "id_name": "acid", + "team_id": "111222", + "type": "official", + "name": "Acid team name", + "mail": [ + "email1@example.com", + "email2@example.com" + ], + "alias": [ + "acid" + ], + "member": [ + "member1", + "member2", + "member3" + ], + "infrastructure-accounts": [ + { + "id": "1234512345", + "name": "acid", + "provider": "aws", + "type": "aws", + "description": "", + "owner": "acid", + "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + "disabled": false + }, + { + "id": "5432154321", + "name": "db", + "provider": "aws", + "type": "aws", + "description": "", + "owner": "acid", + "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + "disabled": false + } + ], + "cost_center": "00099999", + "delivery_lead": "member4", + "parent_team_id": "111221" + }` +) +var teamsAPItc = []struct { + in string + inCode int + inTeam string + out *Team + err error +}{ + { + input, + 200, + "acid", + &Team{ + Dn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + ID: "acid", + TeamName: "acid", + TeamID: "111222", + Type: "official", + FullName: "Acid team name", + Aliases: []string{"acid"}, + Mails: []string{"email1@example.com", "email2@example.com"}, + Members: []string{"member1", "member2", "member3"}, + CostCenter: "00099999", + DeliveryLead: "member4", + ParentTeamID: "111221", + InfrastructureAccounts: []infrastructureAccount{ + { + ID: "1234512345", + Name: "acid", + Provider: "aws", + Type: "aws", + Description: "", + Owner: "acid", + OwnerDn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + Disabled: false}, + { + ID: "5432154321", + Name: "db", + Provider: "aws", + Type: "aws", + Description: "", + Owner: "acid", + OwnerDn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + Disabled: false}, + }, + }, + nil}, { + `{"error": "Access Token not valid"}`, + 401, + "acid", + nil, + fmt.Errorf(`team API query failed with status code 401 and message: '"Access Token not valid"'`), + }, + { + `{"status": "I'm a teapot'"}`, + 418, + "acid", + nil, + fmt.Errorf(`team API query failed with status code 418`), + }, + { + `{"status": "I'm a teapot`, + 418, + "acid", + nil, + fmt.Errorf(`team API query failed with status code 418 and malformed response: unexpected EOF`), + }, + { + `{"status": "I'm a teapot`, + 200, + "acid", + nil, + fmt.Errorf(`could not parse team API response: unexpected EOF`), + }, + { + input, + 404, + "banana", + nil, + fmt.Errorf(`team API query failed with status code 404`), + }, +} + +var requestsURLtc = []struct { + url string + err error +}{ + { + "coffee://localhost/", + fmt.Errorf(`Get "coffee://localhost/teams/acid": unsupported protocol scheme "coffee"`), + }, + { + "http://192.168.0.%31/", + fmt.Errorf(`parse "http://192.168.0.%%31/teams/acid": invalid URL escape "%%31"`), + }, +} + +func TestInfo(t *testing.T) { + for _, tc := range teamsAPItc { + func() { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer "+token { + t.Errorf("authorization token is wrong or not provided") + } + w.WriteHeader(tc.inCode) + if _, err := fmt.Fprint(w, tc.in); err != nil { + t.Errorf("error writing teams api response %v", err) + } + })) + defer ts.Close() + api := NewTeamsAPI(ts.URL, logger) + + actual, statusCode, err := api.TeamInfo(tc.inTeam, token) + if err != nil && err.Error() != tc.err.Error() { + t.Errorf("expected error: %v, got: %v", tc.err, err) + return + } + + if !reflect.DeepEqual(actual, tc.out) { + t.Errorf("expected %#v, got: %#v", tc.out, actual) + } + + if statusCode != tc.inCode { + t.Errorf("expected %d, got: %d", tc.inCode, statusCode) + } + }() + } +} + +type mockHTTPClient struct { +} + +type mockBody struct { +} + +func (b *mockBody) Read(p []byte) (n int, err error) { + return 2, nil +} + +func (b *mockBody) Close() error { + return fmt.Errorf("close error") +} + +func (c *mockHTTPClient) Do(req *http.Request) (*http.Response, error) { + resp := http.Response{ + Status: "200 OK", + StatusCode: 200, + ContentLength: 2, + Close: false, + Request: req, + } + resp.Body = &mockBody{} + + return &resp, nil +} + +func TestHttpClientClose(t *testing.T) { + ts := httptest.NewServer(nil) + + api := NewTeamsAPI(ts.URL, logger) + api.httpClient = &mockHTTPClient{} + + _, _, err := api.TeamInfo("acid", token) + expError := fmt.Errorf("error when closing response: close error") + if err.Error() != expError.Error() { + t.Errorf("expected error: %v, got: %v", expError, err) + } +} + +func TestRequest(t *testing.T) { + for _, tc := range requestsURLtc { + api := NewTeamsAPI(tc.url, logger) + resp, _, err := api.TeamInfo("acid", token) + if resp != nil { + t.Errorf("response expected to be nil") + continue + } + + if err.Error() != tc.err.Error() { + t.Errorf("expected error: %v, got: %v", tc.err, err) + } + } +}