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 <jociele.padilha@zalando.de>
Co-authored-by: Felix Kunde <felix-kunde@gmx.de>

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) {
+func (m *mockTeamsAPIClient) TeamInfo(teamID, token string) (tm *teams.Team, statusCode int, err error) {
-	return &teams.Team{Members: m.members}, nil
+	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

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)) {

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
 }

pkg/util/teams/teams_test.go

@@ -1,225 +1,243 @@
-package teams
+package teams
-
+
-import (
+import (
-	"fmt"
+	"fmt"
-	"net/http"
+	"net/http"
-	"net/http/httptest"
+	"net/http/httptest"
-	"reflect"
+	"reflect"
-	"testing"
+	"testing"
-
+
-	"github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus"
-)
+)
-
+
-var (
+var (
-	logger = logrus.New().WithField("pkg", "teamsapi")
+	logger = logrus.New().WithField("pkg", "teamsapi")
-	token  = "ec45b1cfbe7100c6315d183a3eb6cec0M2U1LWJkMzEtZDgzNzNmZGQyNGM3IiwiYXV0aF90aW1lIjoxNDkzNzMwNzQ1LCJpc3MiOiJodHRwcz"
+	token  = "ec45b1cfbe7100c6315d183a3eb6cec0M2U1LWJkMzEtZDgzNzNmZGQyNGM3IiwiYXV0aF90aW1lIjoxNDkzNzMwNzQ1LCJpc3MiOiJodHRwcz"
-)
+	input  = `{
-
+	"dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
-var teamsAPItc = []struct {
+	"id": "acid",
-	in     string
+	"id_name": "acid",
-	inCode int
+	"team_id": "111222",
-	out    *Team
+	"type": "official",
-	err    error
+	"name": "Acid team name",
-}{
+	"mail": [
-	{`{
+	"email1@example.com",
-"dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
+	"email2@example.com"
-"id": "acid",
+	],
-"id_name": "acid",
+	"alias": [
-"team_id": "111222",
+	"acid"
-"type": "official",
+	],
-"name": "Acid team name",
+	"member": [
-"mail": [
+	  "member1",
-"email1@example.com",
+	  "member2",
-"email2@example.com"
+	  "member3"
-],
+	],
-"alias": [
+	"infrastructure-accounts": [
-"acid"
+	{
-],
+	  "id": "1234512345",
-"member": [
+	  "name": "acid",
-  "member1",
+	  "provider": "aws",
-  "member2",
+	  "type": "aws",
-  "member3"
+	  "description": "",
-],
+	  "owner": "acid",
-"infrastructure-accounts": [
+	  "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
-{
+	  "disabled": false
-  "id": "1234512345",
+	},
-  "name": "acid",
+	{
-  "provider": "aws",
+	  "id": "5432154321",
-  "type": "aws",
+	  "name": "db",
-  "description": "",
+	  "provider": "aws",
-  "owner": "acid",
+	  "type": "aws",
-  "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
+	  "description": "",
-  "disabled": false
+	  "owner": "acid",
-},
+	  "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
-{
+	  "disabled": false
-  "id": "5432154321",
+	}
-  "name": "db",
+	],
-  "provider": "aws",
+	"cost_center": "00099999",
-  "type": "aws",
+	"delivery_lead": "member4",
-  "description": "",
+	"parent_team_id": "111221"
-  "owner": "acid",
+	}`
-  "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
+)
-  "disabled": false
+var teamsAPItc = []struct {
-}
+	in     string
-],
+	inCode int
-"cost_center": "00099999",
+	inTeam string
-"delivery_lead": "member4",
+	out    *Team
-"parent_team_id": "111221"
+	err    error
-}`,
+}{
-		200,
+	{
-		&Team{
+		input,
-			Dn:           "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
+		200,
-			ID:           "acid",
+		"acid",
-			TeamName:     "acid",
+		&Team{
-			TeamID:       "111222",
+			Dn:           "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
-			Type:         "official",
+			ID:           "acid",
-			FullName:     "Acid team name",
+			TeamName:     "acid",
-			Aliases:      []string{"acid"},
+			TeamID:       "111222",
-			Mails:        []string{"email1@example.com", "email2@example.com"},
+			Type:         "official",
-			Members:      []string{"member1", "member2", "member3"},
+			FullName:     "Acid team name",
-			CostCenter:   "00099999",
+			Aliases:      []string{"acid"},
-			DeliveryLead: "member4",
+			Mails:        []string{"email1@example.com", "email2@example.com"},
-			ParentTeamID: "111221",
+			Members:      []string{"member1", "member2", "member3"},
-			InfrastructureAccounts: []infrastructureAccount{
+			CostCenter:   "00099999",
-				{
+			DeliveryLead: "member4",
-					ID:          "1234512345",
+			ParentTeamID: "111221",
-					Name:        "acid",
+			InfrastructureAccounts: []infrastructureAccount{
-					Provider:    "aws",
+				{
-					Type:        "aws",
+					ID:          "1234512345",
-					Description: "",
+					Name:        "acid",
-					Owner:       "acid",
+					Provider:    "aws",
-					OwnerDn:     "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
+					Type:        "aws",
-					Disabled:    false},
+					Description: "",
-				{
+					Owner:       "acid",
-					ID:          "5432154321",
+					OwnerDn:     "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
-					Name:        "db",
+					Disabled:    false},
-					Provider:    "aws",
+				{
-					Type:        "aws",
+					ID:          "5432154321",
-					Description: "",
+					Name:        "db",
-					Owner:       "acid",
+					Provider:    "aws",
-					OwnerDn:     "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
+					Type:        "aws",
-					Disabled:    false},
+					Description: "",
-			},
+					Owner:       "acid",
-		},
+					OwnerDn:     "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net",
-		nil}, {
+					Disabled:    false},
-		`{"error": "Access Token not valid"}`,
+			},
-		401,
+		},
-		nil,
+		nil}, {
-		fmt.Errorf(`team API query failed with status code 401 and message: '"Access Token not valid"'`),
+		`{"error": "Access Token not valid"}`,
-	},
+		401,
-	{
+		"acid",
-		`{"status": "I'm a teapot'"}`,
+		nil,
-		418,
+		fmt.Errorf(`team API query failed with status code 401 and message: '"Access Token not valid"'`),
-		nil,
+	},
-		fmt.Errorf(`team API query failed with status code 418`),
+	{
-	},
+		`{"status": "I'm a teapot'"}`,
-	{
+		418,
-		`{"status": "I'm a teapot`,
+		"acid",
-		418,
+		nil,
-		nil,
+		fmt.Errorf(`team API query failed with status code 418`),
-		fmt.Errorf(`team API query failed with status code 418 and malformed response: unexpected EOF`),
+	},
-	},
+	{
-	{
+		`{"status": "I'm a teapot`,
-		`{"status": "I'm a teapot`,
+		418,
-		200,
+		"acid",
-		nil,
+		nil,
-		fmt.Errorf(`could not parse team API response: unexpected EOF`),
+		fmt.Errorf(`team API query failed with status code 418 and malformed response: unexpected EOF`),
-	},
+	},
-}
+	{
-
+		`{"status": "I'm a teapot`,
-var requestsURLtc = []struct {
+		200,
-	url string
+		"acid",
-	err error
+		nil,
-}{
+		fmt.Errorf(`could not parse team API response: unexpected EOF`),
-	{
+	},
-		"coffee://localhost/",
+	{
-		fmt.Errorf(`Get "coffee://localhost/teams/acid": unsupported protocol scheme "coffee"`),
+		input,
-	},
+		404,
-	{
+		"banana",
-		"http://192.168.0.%31/",
+		nil,
-		fmt.Errorf(`parse "http://192.168.0.%%31/teams/acid": invalid URL escape "%%31"`),
+		fmt.Errorf(`team API query failed with status code 404`),
-	},
+	},
-}
+}
-
+
-func TestInfo(t *testing.T) {
+var requestsURLtc = []struct {
-	for _, tc := range teamsAPItc {
+	url string
-		func() {
+	err error
-			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")
+		"coffee://localhost/",
-				}
+		fmt.Errorf(`Get "coffee://localhost/teams/acid": unsupported protocol scheme "coffee"`),
-				w.WriteHeader(tc.inCode)
+	},
-				if _, err := fmt.Fprint(w, tc.in); err != nil {
+	{
-					t.Errorf("error writing teams api response %v", err)
+		"http://192.168.0.%31/",
-				}
+		fmt.Errorf(`parse "http://192.168.0.%%31/teams/acid": invalid URL escape "%%31"`),
-			}))
+	},
-			defer ts.Close()
+}
-			api := NewTeamsAPI(ts.URL, logger)
+
-
+func TestInfo(t *testing.T) {
-			actual, err := api.TeamInfo("acid", token)
+	for _, tc := range teamsAPItc {
-			if err != nil && err.Error() != tc.err.Error() {
+		func() {
-				t.Errorf("expected error: %v, got: %v", tc.err, err)
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				return
+				if r.Header.Get("Authorization") != "Bearer "+token {
-			}
+					t.Errorf("authorization token is wrong or not provided")
-
+				}
-			if !reflect.DeepEqual(actual, tc.out) {
+				w.WriteHeader(tc.inCode)
-				t.Errorf("expected %#v, got: %#v", tc.out, actual)
+				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)
-type mockHTTPClient struct {
+
-}
+			actual, statusCode, err := api.TeamInfo(tc.inTeam, token)
-
+			if err != nil && err.Error() != tc.err.Error() {
-type mockBody struct {
+				t.Errorf("expected error: %v, got: %v", tc.err, err)
-}
+				return
-
+			}
-func (b *mockBody) Read(p []byte) (n int, err error) {
+
-	return 2, nil
+			if !reflect.DeepEqual(actual, tc.out) {
-}
+				t.Errorf("expected %#v, got: %#v", tc.out, actual)
-
+			}
-func (b *mockBody) Close() error {
+
-	return fmt.Errorf("close error")
+			if statusCode != tc.inCode {
-}
+				t.Errorf("expected %d, got: %d", tc.inCode, statusCode)
-
+			}
-func (c *mockHTTPClient) Do(req *http.Request) (*http.Response, error) {
+		}()
-	resp := http.Response{
+	}
-		Status:        "200 OK",
+}
-		StatusCode:    200,
+
-		ContentLength: 2,
+type mockHTTPClient struct {
-		Close:         false,
+}
-		Request:       req,
+
-	}
+type mockBody struct {
-	resp.Body = &mockBody{}
+}
-
+
-	return &resp, nil
+func (b *mockBody) Read(p []byte) (n int, err error) {
-}
+	return 2, nil
-
+}
-func TestHttpClientClose(t *testing.T) {
+
-	ts := httptest.NewServer(nil)
+func (b *mockBody) Close() error {
-
+	return fmt.Errorf("close error")
-	api := NewTeamsAPI(ts.URL, logger)
+}
-	api.httpClient = &mockHTTPClient{}
+
-
+func (c *mockHTTPClient) Do(req *http.Request) (*http.Response, error) {
-	_, err := api.TeamInfo("acid", token)
+	resp := http.Response{
-	expError := fmt.Errorf("error when closing response: close error")
+		Status:        "200 OK",
-	if err.Error() != expError.Error() {
+		StatusCode:    200,
-		t.Errorf("expected error: %v, got: %v", expError, err)
+		ContentLength: 2,
-	}
+		Close:         false,
-}
+		Request:       req,
-
+	}
-func TestRequest(t *testing.T) {
+	resp.Body = &mockBody{}
-	for _, tc := range requestsURLtc {
+
-		api := NewTeamsAPI(tc.url, logger)
+	return &resp, nil
-		resp, err := api.TeamInfo("acid", token)
+}
-		if resp != nil {
+
-			t.Errorf("response expected to be nil")
+func TestHttpClientClose(t *testing.T) {
-			continue
+	ts := httptest.NewServer(nil)
-		}
+
-
+	api := NewTeamsAPI(ts.URL, logger)
-		if err.Error() != tc.err.Error() {
+	api.httpClient = &mockHTTPClient{}
-			t.Errorf("expected error: %v, got: %v", tc.err, err)
+
-		}
+	_, _, 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)
+		}
+	}
+}