From 66693e5bea4421dc60a93b7ef22e481d93725923 Mon Sep 17 00:00:00 2001 From: Imran Ismail Date: Wed, 7 Oct 2020 10:50:49 +0800 Subject: [PATCH 1/2] Capture parent team information in github connector Signed-off-by: Imran Ismail --- connector/github/github.go | 30 ++++++++++++++++++++++++------ connector/github/github_test.go | 24 +++++++++++++----------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/connector/github/github.go b/connector/github/github.go index e38eb26a13..29986baa99 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -449,7 +449,7 @@ func (c *githubConnector) userOrgTeams(ctx context.Context, client *http.Client) for { // https://developer.github.com/v3/orgs/teams/#list-user-teams var ( - teams []team + teams []teamWithParent err error ) if apiURL, err = get(ctx, client, apiURL, &teams); err != nil { @@ -656,6 +656,11 @@ type team struct { Slug string `json:"slug"` } +type teamWithParent struct { + team + Parent *team `json:"parent"` +} + type org struct { Login string `json:"login"` } @@ -669,7 +674,7 @@ func (c *githubConnector) teamsForOrg(ctx context.Context, client *http.Client, for { // https://developer.github.com/v3/orgs/teams/#list-user-teams var ( - teams []team + teams []teamWithParent err error ) if apiURL, err = get(ctx, client, apiURL, &teams); err != nil { @@ -693,13 +698,26 @@ func (c *githubConnector) teamsForOrg(ctx context.Context, client *http.Client, // teamGroupClaims returns team slug if 'teamNameField' option is set to // 'slug', returns the slug *and* name if set to 'both', otherwise returns team // name. -func (c *githubConnector) teamGroupClaims(t team) []string { +func (c *githubConnector) teamGroupClaims(t teamWithParent) []string { + var groups []string + switch c.teamNameField { case "both": - return []string{t.Name, t.Slug} + if t.Parent != nil { + groups = append(groups, t.Parent.Name, t.Parent.Slug) + } + groups = append(groups, t.Name, t.Slug) case "slug": - return []string{t.Slug} + if t.Parent != nil { + groups = append(groups, t.Parent.Slug) + } + groups = append(groups, t.Slug) default: - return []string{t.Name} + if t.Parent != nil { + groups = append(groups, t.Parent.Name) + } + groups = append(groups, t.Name) } + + return groups } diff --git a/connector/github/github_test.go b/connector/github/github_test.go index 76d7463cf6..d9b89a98f0 100644 --- a/connector/github/github_test.go +++ b/connector/github/github_test.go @@ -30,17 +30,17 @@ func TestUserGroups(t *testing.T) { }, "/user/orgs?since=2": {data: []org{{Login: "org-3"}}}, "/user/teams": { - data: []team{ - {Name: "team-1", Org: org{Login: "org-1"}}, - {Name: "team-2", Org: org{Login: "org-1"}}, + data: []teamWithParent{ + {team: team{Name: "team-1", Org: org{Login: "org-1"}}, Parent: &team{Name: "team-0", Org: org{Login: "org-1"}}}, + {team: team{Name: "team-2", Org: org{Login: "org-1"}}, Parent: &team{Name: "team-0", Org: org{Login: "org-1"}}}, }, nextLink: "/user/teams?since=2", lastLink: "/user/teams?since=2", }, "/user/teams?since=2": { - data: []team{ - {Name: "team-3", Org: org{Login: "org-1"}}, - {Name: "team-4", Org: org{Login: "org-2"}}, + data: []teamWithParent{ + {team: team{Name: "team-3", Org: org{Login: "org-1"}}, Parent: nil}, + {team: team{Name: "team-4", Org: org{Login: "org-2"}}, Parent: nil}, }, nextLink: "/user/teams?since=2", lastLink: "/user/teams?since=2", @@ -54,7 +54,9 @@ func TestUserGroups(t *testing.T) { expectNil(t, err) expectEquals(t, groups, []string{ "org-1", + "org-1:team-0", "org-1:team-1", + "org-1:team-0", "org-1:team-2", "org-1:team-3", "org-2", @@ -66,7 +68,7 @@ func TestUserGroups(t *testing.T) { func TestUserGroupsWithoutOrgs(t *testing.T) { s := newTestServer(map[string]testResponse{ "/user/orgs": {data: []org{}}, - "/user/teams": {data: []team{}}, + "/user/teams": {data: []teamWithParent{}}, }) defer s.Close() @@ -83,8 +85,8 @@ func TestUserGroupsWithTeamNameFieldConfig(t *testing.T) { data: []org{{Login: "org-1"}}, }, "/user/teams": { - data: []team{ - {Name: "Team 1", Slug: "team-1", Org: org{Login: "org-1"}}, + data: []teamWithParent{ + {team: team{Name: "Team 1", Slug: "team-1", Org: org{Login: "org-1"}}, Parent: nil}, }, }, }) @@ -106,8 +108,8 @@ func TestUserGroupsWithTeamNameAndSlugFieldConfig(t *testing.T) { data: []org{{Login: "org-1"}}, }, "/user/teams": { - data: []team{ - {Name: "Team 1", Slug: "team-1", Org: org{Login: "org-1"}}, + data: []teamWithParent{ + {team: team{Name: "Team 1", Slug: "team-1", Org: org{Login: "org-1"}}, Parent: nil}, }, }, }) From c123abf1775beb46cbb4f83cb491cd5e4839dda0 Mon Sep 17 00:00:00 2001 From: Imran Ismail Date: Wed, 7 Oct 2020 11:54:09 +0800 Subject: [PATCH 2/2] Unique groups Signed-off-by: Imran Ismail --- connector/github/github.go | 26 ++++++++++++++++++++++++++ connector/github/github_test.go | 1 - 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/connector/github/github.go b/connector/github/github.go index 29986baa99..9062e1ae75 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -460,6 +460,20 @@ func (c *githubConnector) userOrgTeams(ctx context.Context, client *http.Client) groups[t.Org.Login] = append(groups[t.Org.Login], c.teamGroupClaims(t)...) } + for _, t := range teams { + keys := make(map[string]bool) + uniqueGroups := []string{} + + for _, group := range groups[t.Org.Login] { + if _, exists := keys[group]; !exists { + keys[group] = true + uniqueGroups = append(uniqueGroups, group) + } + + groups[t.Org.Login] = uniqueGroups + } + } + if apiURL == "" { break } @@ -687,6 +701,18 @@ func (c *githubConnector) teamsForOrg(ctx context.Context, client *http.Client, } } + keys := make(map[string]bool) + uniqueGroups := []string{} + + for _, group := range groups { + if _, exists := keys[group]; !exists { + keys[group] = true + uniqueGroups = append(uniqueGroups, group) + } + } + + groups = uniqueGroups + if apiURL == "" { break } diff --git a/connector/github/github_test.go b/connector/github/github_test.go index d9b89a98f0..721c069558 100644 --- a/connector/github/github_test.go +++ b/connector/github/github_test.go @@ -56,7 +56,6 @@ func TestUserGroups(t *testing.T) { "org-1", "org-1:team-0", "org-1:team-1", - "org-1:team-0", "org-1:team-2", "org-1:team-3", "org-2",