diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 2d0373c35..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 @@ -302,12 +302,6 @@ func TestInitHumanUsersWithSuperuserTeams(t *testing.T) { isPostgresSuperuserTeam: true, } - teamC := mockTeam{ - teamID: "", - members: []string{""}, - isPostgresSuperuserTeam: true, - } - userB := spec.PgUser{ Name: "postgres_admin", Origin: spec.RoleOriginTeamsAPI, @@ -340,7 +334,7 @@ func TestInitHumanUsersWithSuperuserTeams(t *testing.T) { ownerTeam: "test", existingRoles: map[string]spec.PgUser{}, superuserTeams: []string{"postgres_superusers", "postgres_admins"}, - teams: []mockTeam{teamA, teamB, teamC, teamTest}, + teams: []mockTeam{teamA, teamB, teamTest}, result: map[string]spec.PgUser{ "postgres_superuser": userA, "postgres_admin": userB, diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index cf320e057..be5da27a4 100644 --- a/pkg/cluster/util.go +++ b/pkg/cluster/util.go @@ -272,9 +272,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 == 404 { + 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..1ec3ec7c9 100644 --- a/pkg/util/teams/teams_test.go +++ b/pkg/util/teams/teams_test.go @@ -13,59 +13,60 @@ import ( 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 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" -}`, + { + input, 200, &Team{ Dn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", @@ -125,6 +126,12 @@ var teamsAPItc = []struct { nil, fmt.Errorf(`could not parse team API response: unexpected EOF`), }, + { + input, + 404, + nil, + fmt.Errorf(`team API query failed with status code 404`), + }, } var requestsURLtc = []struct { @@ -156,7 +163,7 @@ func TestInfo(t *testing.T) { defer ts.Close() api := NewTeamsAPI(ts.URL, logger) - actual, err := api.TeamInfo("acid", token) + 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 @@ -165,6 +172,16 @@ func TestInfo(t *testing.T) { if !reflect.DeepEqual(actual, tc.out) { t.Errorf("expected %#v, got: %#v", tc.out, actual) } + + _, statusCode, err := api.TeamInfo("sqlserver", token) + if err != nil && err.Error() != tc.err.Error() { + t.Errorf("expected error: %v, got: %v", tc.err, err) + return + } + + if statusCode != tc.inCode { + t.Errorf("expected %d, got: %d", tc.inCode, statusCode) + } }() } } @@ -202,7 +219,7 @@ func TestHttpClientClose(t *testing.T) { api := NewTeamsAPI(ts.URL, logger) api.httpClient = &mockHTTPClient{} - _, err := api.TeamInfo("acid", token) + _, _, 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) @@ -212,7 +229,7 @@ func TestHttpClientClose(t *testing.T) { func TestRequest(t *testing.T) { for _, tc := range requestsURLtc { api := NewTeamsAPI(tc.url, logger) - resp, err := api.TeamInfo("acid", token) + resp, _, err := api.TeamInfo("acid", token) if resp != nil { t.Errorf("response expected to be nil") continue