diff --git a/prow/cmd/peribolos/BUILD.bazel b/prow/cmd/peribolos/BUILD.bazel index e1628f2c7ebb..b935f535cb4c 100644 --- a/prow/cmd/peribolos/BUILD.bazel +++ b/prow/cmd/peribolos/BUILD.bazel @@ -13,6 +13,7 @@ go_library( "//prow/config:go_default_library", "//prow/config/org:go_default_library", "//prow/config/secret:go_default_library", + "//prow/errorutil:go_default_library", "//prow/flagutil:go_default_library", "//prow/github:go_default_library", "//prow/logrusutil:go_default_library", diff --git a/prow/cmd/peribolos/README.md b/prow/cmd/peribolos/README.md index 3d3f582d2be0..cc2643d11743 100644 --- a/prow/cmd/peribolos/README.md +++ b/prow/cmd/peribolos/README.md @@ -46,6 +46,9 @@ orgs: - anne maintainers: - jane + repos: # Ensure the team has the following permissions levels on repos in the org + some-repo: admin + other-repo: read another-team: ... ... @@ -66,6 +69,7 @@ This config will: - Rename the backend team to node - Add anne as a member and jane as a maintainer to node - Similar things for another-team (details elided) +* Ensure that the team has admin rights to `some-repo`, read access to `other-repo` and no other privileges Note that any fields missing from the config will not be managed by peribolos. So if description is missing from the org setting, the current value will remain. diff --git a/prow/cmd/peribolos/main.go b/prow/cmd/peribolos/main.go index d8b64a41010f..db02131cc92f 100644 --- a/prow/cmd/peribolos/main.go +++ b/prow/cmd/peribolos/main.go @@ -30,6 +30,7 @@ import ( "k8s.io/test-infra/prow/config" "k8s.io/test-infra/prow/config/org" "k8s.io/test-infra/prow/config/secret" + "k8s.io/test-infra/prow/errorutil" "k8s.io/test-infra/prow/flagutil" "k8s.io/test-infra/prow/github" "k8s.io/test-infra/prow/logrusutil" @@ -55,10 +56,12 @@ type options struct { fixOrgMembers bool fixTeamMembers bool fixTeams bool + fixTeamRepos bool ignoreSecretTeams bool github flagutil.GitHubOptions tokenBurst int tokensPerHour int + logLevel string } func parseOptions() options { @@ -86,6 +89,8 @@ func (o *options) parseArgs(flags *flag.FlagSet, args []string) error { flags.BoolVar(&o.fixOrgMembers, "fix-org-members", false, "Add/remove org members if set") flags.BoolVar(&o.fixTeams, "fix-teams", false, "Create/delete/update teams if set") flags.BoolVar(&o.fixTeamMembers, "fix-team-members", false, "Add/remove team members if set") + flags.BoolVar(&o.fixTeamRepos, "fix-team-repos", false, "Add/remove team permissions on repos if set") + flags.StringVar(&o.logLevel, "log-level", logrus.InfoLevel.String(), fmt.Sprintf("Logging level, one of %v", logrus.AllLevels)) o.github.AddFlags(flags) if err := flags.Parse(args); err != nil { return err @@ -118,6 +123,16 @@ func (o *options) parseArgs(flags *flag.FlagSet, args []string) error { return fmt.Errorf("--fix-team-members requires --fix-teams") } + if o.fixTeamRepos && !o.fixTeams { + return fmt.Errorf("--fix-team-repos requires --fix-teams") + } + + level, err := logrus.ParseLevel(o.logLevel) + if err != nil { + return fmt.Errorf("--log-level invalid: %v", err) + } + logrus.SetLevel(level) + return nil } @@ -171,6 +186,7 @@ type dumpClient interface { ListOrgMembers(org, role string) ([]github.TeamMember, error) ListTeams(org string) ([]github.Team, error) ListTeamMembers(id int, role string) ([]github.TeamMember, error) + ListTeamRepos(id int) ([]github.Repo, error) } func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool) (*org.Config, error) { @@ -187,7 +203,7 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool) (* out.Metadata.Location = &meta.Location out.Metadata.HasOrganizationProjects = &meta.HasOrganizationProjects out.Metadata.HasRepositoryProjects = &meta.HasRepositoryProjects - drp := org.RepoPermissionLevel(meta.DefaultRepositoryPermission) + drp := github.RepoPermissionLevel(meta.DefaultRepositoryPermission) out.Metadata.DefaultRepositoryPermission = &drp out.Metadata.MembersCanCreateRepositories = &meta.MembersCanCreateRepositories @@ -195,7 +211,9 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool) (* if err != nil { return nil, fmt.Errorf("failed to list org admins: %v", err) } + logrus.Debugf("Found %d admins", len(admins)) for _, m := range admins { + logrus.WithField("login", m.Login).Debug("Recording admin.") out.Admins = append(out.Admins, m.Login) } @@ -203,7 +221,9 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool) (* if err != nil { return nil, fmt.Errorf("failed to list org members: %v", err) } + logrus.Debugf("Found %d members", len(orgMembers)) for _, m := range orgMembers { + logrus.WithField("login", m.Login).Debug("Recording member.") out.Members = append(out.Members, m.Login) } @@ -211,6 +231,7 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool) (* if err != nil { return nil, fmt.Errorf("failed to list teams: %v", err) } + logrus.Debugf("Found %d teams", len(teams)) names := map[int]string{} // what's the name of a team? idMap := map[int]org.Team{} // metadata for a team @@ -218,8 +239,10 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool) (* var tops []int // what are the top-level teams for _, t := range teams { + logger := logrus.WithFields(logrus.Fields{"id": t.ID, "name": t.Name}) p := org.Privacy(t.Privacy) if ignoreSecretTeams && p == org.Secret { + logger.Debug("Ignoring secret team.") continue } d := t.Description @@ -231,19 +254,24 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool) (* Maintainers: []string{}, Members: []string{}, Children: map[string]org.Team{}, + Repos: map[string]github.RepoPermissionLevel{}, } maintainers, err := client.ListTeamMembers(t.ID, github.RoleMaintainer) if err != nil { return nil, fmt.Errorf("failed to list team %d(%s) maintainers: %v", t.ID, t.Name, err) } + logger.Debugf("Found %d maintainers.", len(maintainers)) for _, m := range maintainers { + logger.WithField("login", m.Login).Debug("Recording maintainer.") nt.Maintainers = append(nt.Maintainers, m.Login) } teamMembers, err := client.ListTeamMembers(t.ID, github.RoleMember) if err != nil { return nil, fmt.Errorf("failed to list team %d(%s) members: %v", t.ID, t.Name, err) } + logger.Debugf("Found %d members.", len(teamMembers)) for _, m := range teamMembers { + logger.WithField("login", m.Login).Debug("Recording member.") nt.Members = append(nt.Members, m.Login) } @@ -251,10 +279,23 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool) (* idMap[t.ID] = nt if t.Parent == nil { // top level team + logger.Debug("Marking as top-level team.") tops = append(tops, t.ID) } else { // add this id to the list of the parent's children + logger.Debugf("Marking as child team of %d.", t.Parent.ID) children[t.Parent.ID] = append(children[t.Parent.ID], t.ID) } + + repos, err := client.ListTeamRepos(t.ID) + if err != nil { + return nil, fmt.Errorf("failed to list team %d(%s) repos: %v", t.ID, t.Name, err) + } + logger.Debugf("Found %d repo permissions.", len(repos)) + for _, repo := range repos { + level := github.LevelFromPermissions(repo.Permissions) + logger.WithFields(logrus.Fields{"repo": repo, "permission": level}).Debug("Recording repo permission.") + nt.Repos[repo.Name] = level + } } var makeChild func(id int) org.Team @@ -726,6 +767,14 @@ func configureOrg(opt options, client *github.Client, orgName string, orgConfig if err != nil { return fmt.Errorf("failed to configure %s teams: %v", orgName, err) } + + if !opt.fixTeamRepos { + logrus.Infof("Skipping team repo permissions configuration") + continue + } + if err := configureTeamRepos(client, githubTeams, name, orgName, team); err != nil { + return fmt.Errorf("failed to configure %s team %s repos: %v", orgName, name, err) + } } return nil } @@ -811,6 +860,62 @@ func configureTeam(client editTeamClient, orgName, teamName string, team org.Tea return nil } +type teamRepoClient interface { + ListTeamRepos(id int) ([]github.Repo, error) + UpdateTeamRepo(id int, org, repo string, permission github.RepoPermissionLevel) error + RemoveTeamRepo(id int, org, repo string) error +} + +// configureTeamRepos updates the list of repos that the team has permissions for when necessary +func configureTeamRepos(client teamRepoClient, githubTeams map[string]github.Team, name, orgName string, team org.Team) error { + gt, ok := githubTeams[name] + if !ok { // configureTeams is buggy if this is the case + return fmt.Errorf("%s not found in id list", name) + } + + want := team.Repos + have := map[string]github.RepoPermissionLevel{} + repos, err := client.ListTeamRepos(gt.ID) + if err != nil { + return fmt.Errorf("failed to list team %d(%s) repos: %v", gt.ID, name, err) + } + for _, repo := range repos { + have[repo.Name] = github.LevelFromPermissions(repo.Permissions) + } + + actions := map[string]github.RepoPermissionLevel{} + for wantRepo, wantPermission := range want { + if havePermission, haveRepo := have[wantRepo]; haveRepo && havePermission == wantPermission { + // nothing to do + continue + } + // create or update this permission + actions[wantRepo] = wantPermission + } + + for haveRepo := range have { + if _, wantRepo := want[haveRepo]; !wantRepo { + // should remove these permissions + actions[haveRepo] = github.None + } + } + + var updateErrors []error + for repo, permission := range actions { + var err error + if permission == github.None { + err = client.RemoveTeamRepo(gt.ID, orgName, repo) + } else { + err = client.UpdateTeamRepo(gt.ID, orgName, repo, permission) + } + if err != nil { + updateErrors = append(updateErrors, fmt.Errorf("failed to update team %d(%s) permissions on repo %s to %s: %v", gt.ID, name, repo, permission, err)) + } + } + + return errorutil.NewAggregate(updateErrors...) +} + // teamMembersClient can list/remove/update people to a team. type teamMembersClient interface { ListTeamMembers(id int, role string) ([]github.TeamMember, error) diff --git a/prow/cmd/peribolos/main_test.go b/prow/cmd/peribolos/main_test.go index d46105233b2b..39594d0dc6d0 100644 --- a/prow/cmd/peribolos/main_test.go +++ b/prow/cmd/peribolos/main_test.go @@ -69,6 +69,7 @@ func TestOptions(t *testing.T) { maximumDelta: 1, tokensPerHour: defaultTokens, tokenBurst: defaultBurst, + logLevel: "info", }, }, { @@ -81,6 +82,7 @@ func TestOptions(t *testing.T) { maximumDelta: 0, tokensPerHour: defaultTokens, tokenBurst: defaultBurst, + logLevel: "info", }, }, { @@ -93,6 +95,7 @@ func TestOptions(t *testing.T) { maximumDelta: defaultDelta, tokensPerHour: defaultTokens, tokenBurst: defaultBurst, + logLevel: "info", }, }, { @@ -121,6 +124,7 @@ func TestOptions(t *testing.T) { maximumDelta: defaultDelta, tokensPerHour: 0, tokenBurst: defaultBurst, + logLevel: "info", }, }, { @@ -133,6 +137,7 @@ func TestOptions(t *testing.T) { tokensPerHour: defaultTokens, tokenBurst: defaultBurst, dump: "frogger", + logLevel: "info", }, }, { @@ -145,11 +150,12 @@ func TestOptions(t *testing.T) { maximumDelta: defaultDelta, tokensPerHour: defaultTokens, tokenBurst: defaultBurst, + logLevel: "info", }, }, { name: "full", - args: []string{"--config-path=foo", "--github-token-path=bar", "--github-endpoint=weird://url", "--confirm=true", "--require-self=false", "--tokens=5", "--token-burst=2", "--dump=", "--fix-org", "--fix-org-members", "--fix-teams", "--fix-team-members"}, + args: []string{"--config-path=foo", "--github-token-path=bar", "--github-endpoint=weird://url", "--confirm=true", "--require-self=false", "--tokens=5", "--token-burst=2", "--dump=", "--fix-org", "--fix-org-members", "--fix-teams", "--fix-team-members", "--log-level=debug"}, expected: &options{ config: "foo", confirm: true, @@ -162,6 +168,7 @@ func TestOptions(t *testing.T) { fixOrgMembers: true, fixTeams: true, fixTeamMembers: true, + logLevel: "debug", }, }, } @@ -177,7 +184,7 @@ func TestOptions(t *testing.T) { case err != nil && tc.expected != nil: t.Errorf("%s: unexpected error: %v", tc.name, err) case tc.expected != nil && !reflect.DeepEqual(*tc.expected, actual): - t.Errorf("%s: actual %v != expected %v", tc.name, actual, *tc.expected) + t.Errorf("%s: got incorrect options: %v", tc.name, diff.ObjectReflectDiff(actual, *tc.expected)) } } } @@ -1552,7 +1559,7 @@ func TestConfigureOrgMeta(t *testing.T) { no := false str := "random-letters" fail := "fail" - read := org.Read + read := github.Read cases := []struct { name string @@ -1713,7 +1720,7 @@ func TestDumpOrgConfig(t *testing.T) { details := "wise and brilliant exemplary human specimens" yes := true no := false - perm := org.Write + perm := github.Write pub := org.Privacy("") secret := org.Secret closed := org.Closed @@ -1727,6 +1734,7 @@ func TestDumpOrgConfig(t *testing.T) { teams []github.Team teamMembers map[int][]string maintainers map[int][]string + repoPermissions map[int][]github.Repo expected org.Config err bool }{ @@ -1799,6 +1807,11 @@ func TestDumpOrgConfig(t *testing.T) { 6: {"giant", "jungle"}, 7: {"banana"}, }, + repoPermissions: map[int][]github.Repo{ + 5: {}, + 6: {{Name: "pull-repo", Permissions: github.RepoPermissions{Pull: true}}}, + 7: {{Name: "pull-repo", Permissions: github.RepoPermissions{Pull: true}}, {Name: "admin-repo", Permissions: github.RepoPermissions{Admin: true}}}, + }, expected: org.Config{ Metadata: org.Metadata{ Name: &hello, @@ -1821,6 +1834,7 @@ func TestDumpOrgConfig(t *testing.T) { Members: []string{"george", "james"}, Maintainers: []string{}, Children: map[string]org.Team{}, + Repos: map[string]github.RepoPermissionLevel{}, }, "enemies": { TeamMetadata: org.TeamMetadata{ @@ -1829,6 +1843,9 @@ func TestDumpOrgConfig(t *testing.T) { }, Members: []string{"george"}, Maintainers: []string{"giant", "jungle"}, + Repos: map[string]github.RepoPermissionLevel{ + "pull-repo": github.Read, + }, Children: map[string]org.Team{ "archenemies": { TeamMetadata: org.TeamMetadata{ @@ -1837,7 +1854,11 @@ func TestDumpOrgConfig(t *testing.T) { }, Members: []string{}, Maintainers: []string{"banana"}, - Children: map[string]org.Team{}, + Repos: map[string]github.RepoPermissionLevel{ + "pull-repo": github.Read, + "admin-repo": github.Admin, + }, + Children: map[string]org.Team{}, }, }, }, @@ -1919,6 +1940,7 @@ func TestDumpOrgConfig(t *testing.T) { Members: []string{"george", "james"}, Maintainers: []string{}, Children: map[string]org.Team{}, + Repos: map[string]github.RepoPermissionLevel{}, }, "enemies": { TeamMetadata: org.TeamMetadata{ @@ -1936,8 +1958,10 @@ func TestDumpOrgConfig(t *testing.T) { Members: []string{"patrick"}, Maintainers: []string{"starfish"}, Children: map[string]org.Team{}, + Repos: map[string]github.RepoPermissionLevel{}, }, }, + Repos: map[string]github.RepoPermissionLevel{}, }, }, Members: []string{"george", "jungle", "banana"}, @@ -1953,13 +1977,14 @@ func TestDumpOrgConfig(t *testing.T) { orgName = tc.orgOverride } fc := fakeDumpClient{ - name: orgName, - members: tc.members, - admins: tc.admins, - meta: tc.meta, - teams: tc.teams, - teamMembers: tc.teamMembers, - maintainers: tc.maintainers, + name: orgName, + members: tc.members, + admins: tc.admins, + meta: tc.meta, + teams: tc.teams, + teamMembers: tc.teamMembers, + maintainers: tc.maintainers, + repoPermissions: tc.repoPermissions, } actual, err := dumpOrgConfig(fc, orgName, tc.ignoreSecretTeams) switch { @@ -1984,13 +2009,14 @@ func TestDumpOrgConfig(t *testing.T) { } type fakeDumpClient struct { - name string - members []string - admins []string - meta github.Organization - teams []github.Team - teamMembers map[int][]string - maintainers map[int][]string + name string + members []string + admins []string + meta github.Organization + teams []github.Team + teamMembers map[int][]string + maintainers map[int][]string + repoPermissions map[int][]github.Repo } func (c fakeDumpClient) GetOrg(name string) (*github.Organization, error) { @@ -2058,6 +2084,14 @@ func (c fakeDumpClient) ListTeamMembers(id int, role string) ([]github.TeamMembe return c.makeMembers(people) } +func (c fakeDumpClient) ListTeamRepos(id int) ([]github.Repo, error) { + if id < 0 { + return nil, errors.New("injected ListTeamRepos error") + } + + return c.repoPermissions[id], nil +} + func fixup(ret *org.Config) { if ret == nil { return @@ -2140,3 +2174,217 @@ func TestOrgInvitations(t *testing.T) { }) } } + +type fakeTeamRepoClient struct { + repos map[int][]github.Repo + failList, failUpdate, failRemove bool +} + +func (c *fakeTeamRepoClient) ListTeamRepos(id int) ([]github.Repo, error) { + if c.failList { + return nil, errors.New("injected failure to ListTeamRepos") + } + return c.repos[id], nil +} + +func (c *fakeTeamRepoClient) UpdateTeamRepo(id int, org, repo string, permission github.RepoPermissionLevel) error { + if c.failUpdate { + return errors.New("injected failure to UpdateTeamRepos") + } + + updated := false + for i, repository := range c.repos[id] { + if repository.Name == repo { + c.repos[id][i].Permissions = github.PermissionsFromLevel(permission) + updated = true + break + } + } + + if !updated { + c.repos[id] = append(c.repos[id], github.Repo{Name: repo, Permissions: github.PermissionsFromLevel(permission)}) + } + + return nil +} + +func (c *fakeTeamRepoClient) RemoveTeamRepo(id int, org, repo string) error { + if c.failRemove { + return errors.New("injected failure to RemoveTeamRepos") + } + + for i, repository := range c.repos[id] { + if repository.Name == repo { + c.repos[id] = append(c.repos[id][:i], c.repos[id][i+1:]...) + break + } + } + + return nil +} + +func TestConfigureTeamRepos(t *testing.T) { + var testCases = []struct { + name string + githubTeams map[string]github.Team + teamName string + team org.Team + existingRepos map[int][]github.Repo + failList bool + failUpdate bool + failRemove bool + expected map[int][]github.Repo + expectedErr bool + }{ + { + name: "githubTeams cache not containing team errors", + githubTeams: map[string]github.Team{}, + teamName: "team", + expectedErr: true, + }, + { + name: "listing repos failing errors", + githubTeams: map[string]github.Team{"team": {ID: 1}}, + teamName: "team", + failList: true, + expectedErr: true, + }, + { + name: "nothing to do", + githubTeams: map[string]github.Team{"team": {ID: 1}}, + teamName: "team", + team: org.Team{ + Repos: map[string]github.RepoPermissionLevel{ + "read": github.Read, + "write": github.Write, + "admin": github.Admin, + }, + }, + existingRepos: map[int][]github.Repo{1: { + {Name: "read", Permissions: github.RepoPermissions{Pull: true}}, + {Name: "write", Permissions: github.RepoPermissions{Push: true}}, + {Name: "admin", Permissions: github.RepoPermissions{Admin: true}}, + }}, + expected: map[int][]github.Repo{1: { + {Name: "read", Permissions: github.RepoPermissions{Pull: true}}, + {Name: "write", Permissions: github.RepoPermissions{Push: true}}, + {Name: "admin", Permissions: github.RepoPermissions{Admin: true}}, + }}, + }, + { + name: "new requirement in org config gets added", + githubTeams: map[string]github.Team{"team": {ID: 1}}, + teamName: "team", + team: org.Team{ + Repos: map[string]github.RepoPermissionLevel{ + "read": github.Read, + "write": github.Write, + "admin": github.Admin, + "other-admin": github.Admin, + }, + }, + existingRepos: map[int][]github.Repo{1: { + {Name: "read", Permissions: github.RepoPermissions{Pull: true}}, + {Name: "write", Permissions: github.RepoPermissions{Push: true}}, + {Name: "admin", Permissions: github.RepoPermissions{Admin: true}}, + }}, + expected: map[int][]github.Repo{1: { + {Name: "read", Permissions: github.RepoPermissions{Pull: true}}, + {Name: "write", Permissions: github.RepoPermissions{Push: true}}, + {Name: "admin", Permissions: github.RepoPermissions{Admin: true}}, + {Name: "other-admin", Permissions: github.RepoPermissions{Admin: true}}, + }}, + }, + { + name: "change in permission on existing gets updated", + githubTeams: map[string]github.Team{"team": {ID: 1}}, + teamName: "team", + team: org.Team{ + Repos: map[string]github.RepoPermissionLevel{ + "read": github.Read, + "write": github.Write, + "admin": github.Read, + }, + }, + existingRepos: map[int][]github.Repo{1: { + {Name: "read", Permissions: github.RepoPermissions{Pull: true}}, + {Name: "write", Permissions: github.RepoPermissions{Push: true}}, + {Name: "admin", Permissions: github.RepoPermissions{Admin: true}}, + }}, + expected: map[int][]github.Repo{1: { + {Name: "read", Permissions: github.RepoPermissions{Pull: true}}, + {Name: "write", Permissions: github.RepoPermissions{Push: true}}, + {Name: "admin", Permissions: github.RepoPermissions{Pull: true}}, + }}, + }, + { + name: "omitted requirement gets removed", + githubTeams: map[string]github.Team{"team": {ID: 1}}, + teamName: "team", + team: org.Team{ + Repos: map[string]github.RepoPermissionLevel{ + "write": github.Write, + "admin": github.Read, + }, + }, + existingRepos: map[int][]github.Repo{1: { + {Name: "read", Permissions: github.RepoPermissions{Pull: true}}, + {Name: "write", Permissions: github.RepoPermissions{Push: true}}, + {Name: "admin", Permissions: github.RepoPermissions{Admin: true}}, + }}, + expected: map[int][]github.Repo{1: { + {Name: "write", Permissions: github.RepoPermissions{Push: true}}, + {Name: "admin", Permissions: github.RepoPermissions{Pull: true}}, + }}, + }, + { + name: "failed update errors", + failUpdate: true, + githubTeams: map[string]github.Team{"team": {ID: 1}}, + teamName: "team", + team: org.Team{ + Repos: map[string]github.RepoPermissionLevel{ + "will-fail": github.Write, + }, + }, + existingRepos: map[int][]github.Repo{1: {}}, + expected: map[int][]github.Repo{1: {}}, + expectedErr: true, + }, + { + name: "failed delete errors", + failRemove: true, + githubTeams: map[string]github.Team{"team": {ID: 1}}, + teamName: "team", + team: org.Team{ + Repos: map[string]github.RepoPermissionLevel{}, + }, + existingRepos: map[int][]github.Repo{1: { + {Name: "needs-deletion", Permissions: github.RepoPermissions{Pull: true}}, + }}, + expected: map[int][]github.Repo{1: { + {Name: "needs-deletion", Permissions: github.RepoPermissions{Pull: true}}, + }}, + expectedErr: true, + }, + } + + for _, testCase := range testCases { + client := fakeTeamRepoClient{ + repos: testCase.existingRepos, + failList: testCase.failList, + failUpdate: testCase.failUpdate, + failRemove: testCase.failRemove, + } + err := configureTeamRepos(&client, testCase.githubTeams, testCase.teamName, "org", testCase.team) + if err == nil && testCase.expectedErr { + t.Errorf("%s: expected an error but got none", testCase.name) + } + if err != nil && !testCase.expectedErr { + t.Errorf("%s: expected no error but got one: %v", testCase.name, err) + } + if actual, expected := client.repos, testCase.expected; !reflect.DeepEqual(actual, expected) { + t.Errorf("%s: got incorrect team repos: %v", testCase.name, diff.ObjectReflectDiff(actual, expected)) + } + } +} diff --git a/prow/config/org/BUILD.bazel b/prow/config/org/BUILD.bazel index 3b0e88fe1484..96e15e501026 100644 --- a/prow/config/org/BUILD.bazel +++ b/prow/config/org/BUILD.bazel @@ -5,6 +5,7 @@ go_library( srcs = ["org.go"], importpath = "k8s.io/test-infra/prow/config/org", visibility = ["//visibility:public"], + deps = ["//prow/github:go_default_library"], ) filegroup( diff --git a/prow/config/org/org.go b/prow/config/org/org.go index 533dcf18a1f9..a2b306ec5a45 100644 --- a/prow/config/org/org.go +++ b/prow/config/org/org.go @@ -18,22 +18,24 @@ package org import ( "fmt" + + "k8s.io/test-infra/prow/github" ) // Metadata declares metadata about the GitHub org. // // See https://developer.github.com/v3/orgs/#edit-an-organization type Metadata struct { - BillingEmail *string `json:"billing_email,omitempty"` - Company *string `json:"company,omitempty"` - Email *string `json:"email,omitempty"` - Name *string `json:"name,omitempty"` - Description *string `json:"description,omitempty"` - Location *string `json:"location,omitempty"` - HasOrganizationProjects *bool `json:"has_organization_projects,omitempty"` - HasRepositoryProjects *bool `json:"has_repository_projects,omitempty"` - DefaultRepositoryPermission *RepoPermissionLevel `json:"default_repository_permission,omitempty"` - MembersCanCreateRepositories *bool `json:"members_can_create_repositories,omitempty"` + BillingEmail *string `json:"billing_email,omitempty"` + Company *string `json:"company,omitempty"` + Email *string `json:"email,omitempty"` + Name *string `json:"name,omitempty"` + Description *string `json:"description,omitempty"` + Location *string `json:"location,omitempty"` + HasOrganizationProjects *bool `json:"has_organization_projects,omitempty"` + HasRepositoryProjects *bool `json:"has_repository_projects,omitempty"` + DefaultRepositoryPermission *github.RepoPermissionLevel `json:"default_repository_permission,omitempty"` + MembersCanCreateRepositories *bool `json:"members_can_create_repositories,omitempty"` } // Config declares org metadata as well as its people and teams. @@ -60,44 +62,13 @@ type Team struct { Children map[string]Team `json:"teams,omitempty"` Previously []string `json:"previously,omitempty"` -} - -// RepoPermissionLevel is admin, write, read or none. -// -// See https://developer.github.com/v3/repos/collaborators/#review-a-users-permission-level -type RepoPermissionLevel string - -const ( - // Read allows pull but not push - Read RepoPermissionLevel = "read" - // Write allows Read plus push - Write RepoPermissionLevel = "write" - // Admin allows Write plus change others' rights. - Admin RepoPermissionLevel = "admin" - // None disallows everything - None RepoPermissionLevel = "none" -) -var repoPermissionLevels = map[RepoPermissionLevel]bool{ - Read: true, - Write: true, - Admin: true, - None: true, -} - -// MarshalText returns the byte representation of the permission -func (l RepoPermissionLevel) MarshalText() ([]byte, error) { - return []byte(l), nil -} - -// UnmarshalText validates the text is a valid string -func (l *RepoPermissionLevel) UnmarshalText(text []byte) error { - v := RepoPermissionLevel(text) - if _, ok := repoPermissionLevels[v]; !ok { - return fmt.Errorf("bad repo permission: %s not in %v", v, repoPermissionLevels) - } - *l = v - return nil + // This is injected to the Team structure by listing privilege + // levels on dump and if set by users will cause privileges to + // be added on sync. + // https://developer.github.com/v3/teams/#list-team-repos + // https://developer.github.com/v3/teams/#add-or-update-team-repository + Repos map[string]github.RepoPermissionLevel `json:"repos,omitempty"` } // Privacy is secret or closed. diff --git a/prow/config/org/org_test.go b/prow/config/org/org_test.go index db77b4f8b24f..068e1acbcbc0 100644 --- a/prow/config/org/org_test.go +++ b/prow/config/org/org_test.go @@ -21,53 +21,6 @@ import ( "testing" ) -func TestRepoPermissionLevel(t *testing.T) { - get := func(v RepoPermissionLevel) *RepoPermissionLevel { - return &v - } - cases := []struct { - input string - expected *RepoPermissionLevel - }{ - { - "admin", - get(Admin), - }, - { - "write", - get(Write), - }, - { - "read", - get(Read), - }, - { - "none", - get(None), - }, - { - "", - nil, - }, - { - "unknown", - nil, - }, - } - for _, tc := range cases { - var actual RepoPermissionLevel - err := json.Unmarshal([]byte("\""+tc.input+"\""), &actual) - switch { - case err == nil && tc.expected == nil: - t.Errorf("%s: failed to receive an error", tc.input) - case err != nil && tc.expected != nil: - t.Errorf("%s: unexpected error: %v", tc.input, err) - case err == nil && *tc.expected != actual: - t.Errorf("%s: actual %v != expected %v", tc.input, tc.expected, actual) - } - } -} - func TestPrivacy(t *testing.T) { get := func(v Privacy) *Privacy { return &v diff --git a/prow/github/BUILD.bazel b/prow/github/BUILD.bazel index b292da58ebfb..d44ccbc48e06 100644 --- a/prow/github/BUILD.bazel +++ b/prow/github/BUILD.bazel @@ -10,6 +10,7 @@ go_test( name = "go_default_test", srcs = [ "client_test.go", + "helpers_test.go", "hmac_test.go", "links_test.go", "types_test.go", diff --git a/prow/github/client.go b/prow/github/client.go index d8c274cbad45..a513e836310c 100644 --- a/prow/github/client.go +++ b/prow/github/client.go @@ -37,7 +37,6 @@ import ( githubql "github.com/shurcooL/githubv4" "github.com/sirupsen/logrus" "golang.org/x/oauth2" - "k8s.io/test-infra/ghproxy/ghcache" "k8s.io/test-infra/prow/errorutil" ) @@ -2186,6 +2185,78 @@ func (c *Client) ListTeamMembers(id int, role string) ([]TeamMember, error) { return teamMembers, nil } +// ListTeamRepos gets a list of team repos for the given team id +// +// https://developer.github.com/v3/teams/#list-team-repos +func (c *Client) ListTeamRepos(id int) ([]Repo, error) { + c.log("ListTeamRepos", id) + if c.fake { + return nil, nil + } + path := fmt.Sprintf("/teams/%d/repos", id) + var repos []Repo + err := c.readPaginatedResultsWithValues( + path, + url.Values{ + "per_page": []string{"100"}, + }, + // This accept header enables the nested teams preview. + // https://developer.github.com/changes/2017-08-30-preview-nested-teams/ + "application/vnd.github.hellcat-preview+json", + func() interface{} { + return &[]Repo{} + }, + func(obj interface{}) { + repos = append(repos, *(obj.(*[]Repo))...) + }, + ) + if err != nil { + return nil, err + } + return repos, nil +} + +// UpdateTeamRepo adds the repo to the team with the provided role. +// +// https://developer.github.com/v3/teams/#add-or-update-team-repository +func (c *Client) UpdateTeamRepo(id int, org, repo string, permission RepoPermissionLevel) error { + c.log("UpdateTeamRepo", id, org, repo, permission) + if c.fake || c.dry { + return nil + } + + data := struct { + Permission string `json:"permission"` + }{ + Permission: string(permission), + } + + _, err := c.request(&request{ + method: http.MethodPut, + path: fmt.Sprintf("/teams/%d/repos/%s/%s", id, org, repo), + requestBody: &data, + exitCodes: []int{204}, + }, nil) + return err +} + +// RemoveTeamRepo removes the team from the repo. +// +// https://developer.github.com/v3/teams/#add-or-update-team-repository +func (c *Client) RemoveTeamRepo(id int, org, repo string) error { + c.log("RemoveTeamRepo", id, org, repo) + if c.fake || c.dry { + return nil + } + + _, err := c.request(&request{ + method: http.MethodDelete, + path: fmt.Sprintf("/teams/%d/repos/%s/%s", id, org, repo), + exitCodes: []int{204}, + }, nil) + return err +} + // ListTeamInvitations gets a list of team members with pending invitations for the // given team id // diff --git a/prow/github/helpers.go b/prow/github/helpers.go index eb550f26dfb4..47dd7fdc4c12 100644 --- a/prow/github/helpers.go +++ b/prow/github/helpers.go @@ -51,3 +51,34 @@ func ImageTooBig(url string) (bool, error) { } return false, nil } + +// LevelFromPermissions adapts a repo permissions struct to the +// appropriate permission level used elsewhere +func LevelFromPermissions(permissions RepoPermissions) RepoPermissionLevel { + if permissions.Pull { + return Read + } else if permissions.Push { + return Write + } else if permissions.Admin { + return Admin + } else { + return None + } +} + +// PermissionsFromLevel adapts a repo permission level to the +// appropriate permissions struct used elsewhere +func PermissionsFromLevel(permission RepoPermissionLevel) RepoPermissions { + switch permission { + case None: + return RepoPermissions{} + case Read: + return RepoPermissions{Pull: true} + case Write: + return RepoPermissions{Push: true} + case Admin: + return RepoPermissions{Admin: true} + default: + return RepoPermissions{} + } +} diff --git a/prow/github/helpers_test.go b/prow/github/helpers_test.go new file mode 100644 index 000000000000..928101ed8c87 --- /dev/null +++ b/prow/github/helpers_test.go @@ -0,0 +1,133 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package github + +import ( + "encoding/json" + "testing" +) + +func TestRepoPermissionLevel(t *testing.T) { + get := func(v RepoPermissionLevel) *RepoPermissionLevel { + return &v + } + cases := []struct { + input string + expected *RepoPermissionLevel + }{ + { + "admin", + get(Admin), + }, + { + "write", + get(Write), + }, + { + "read", + get(Read), + }, + { + "none", + get(None), + }, + { + "", + nil, + }, + { + "unknown", + nil, + }, + } + for _, tc := range cases { + var actual RepoPermissionLevel + err := json.Unmarshal([]byte("\""+tc.input+"\""), &actual) + switch { + case err == nil && tc.expected == nil: + t.Errorf("%s: failed to receive an error", tc.input) + case err != nil && tc.expected != nil: + t.Errorf("%s: unexpected error: %v", tc.input, err) + case err == nil && *tc.expected != actual: + t.Errorf("%s: actual %v != expected %v", tc.input, tc.expected, actual) + } + } +} + +func TestLevelFromPermissions(t *testing.T) { + var testCases = []struct { + permissions RepoPermissions + level RepoPermissionLevel + }{ + { + permissions: RepoPermissions{}, + level: None, + }, + { + permissions: RepoPermissions{Pull: true}, + level: Read, + }, + { + permissions: RepoPermissions{Push: true}, + level: Write, + }, + { + permissions: RepoPermissions{Admin: true}, + level: Admin, + }, + } + + for _, testCase := range testCases { + if actual, expected := LevelFromPermissions(testCase.permissions), testCase.level; actual != expected { + t.Errorf("got incorrect level from permissions, expected %v but got %v", expected, actual) + } + } +} + +func TestPermissionsFromLevel(t *testing.T) { + var testCases = []struct { + level RepoPermissionLevel + permissions RepoPermissions + }{ + { + level: RepoPermissionLevel("foobar"), + permissions: RepoPermissions{}, + }, + { + level: None, + permissions: RepoPermissions{}, + }, + { + level: Read, + permissions: RepoPermissions{Pull: true}, + }, + { + level: Write, + permissions: RepoPermissions{Push: true}, + }, + { + level: Admin, + permissions: RepoPermissions{Admin: true}, + }, + } + + for _, testCase := range testCases { + if actual, expected := PermissionsFromLevel(testCase.level), testCase.permissions; actual != expected { + t.Errorf("got incorrect permissions from level, expected %v but got %v", expected, actual) + } + } +} diff --git a/prow/github/types.go b/prow/github/types.go index c782670101df..b61e53e1c977 100644 --- a/prow/github/types.go +++ b/prow/github/types.go @@ -286,6 +286,60 @@ type Repo struct { Fork bool `json:"fork"` DefaultBranch string `json:"default_branch"` Archived bool `json:"archived"` + + // Permissions reflect the permission level for the requester, so + // on a repository GET call this will be for the user whose token + // is being used, if listing a team's repos this will be for the + // team's privilege level in the repo + Permissions RepoPermissions `json:"permissions"` +} + +// RepoPermissions describes which permission level an entity has in a +// repo. At most one of the booleans here should be true. +type RepoPermissions struct { + // Pull is equivalent to "Read" permissions in the web UI + Pull bool `json:"pull"` + // Push is equivalent to "Edit" permissions in the web UI + Push bool `json:"push"` + Admin bool `json:"admin"` +} + +// RepoPermissionLevel is admin, write, read or none. +// +// See https://developer.github.com/v3/repos/collaborators/#review-a-users-permission-level +type RepoPermissionLevel string + +const ( + // Read allows pull but not push + Read RepoPermissionLevel = "read" + // Write allows Read plus push + Write RepoPermissionLevel = "write" + // Admin allows Write plus change others' rights. + Admin RepoPermissionLevel = "admin" + // None disallows everything + None RepoPermissionLevel = "none" +) + +var repoPermissionLevels = map[RepoPermissionLevel]bool{ + Read: true, + Write: true, + Admin: true, + None: true, +} + +// MarshalText returns the byte representation of the permission +func (l RepoPermissionLevel) MarshalText() ([]byte, error) { + return []byte(l), nil +} + +// UnmarshalText validates the text is a valid string +func (l *RepoPermissionLevel) UnmarshalText(text []byte) error { + v := RepoPermissionLevel(text) + if _, ok := repoPermissionLevels[v]; !ok { + return fmt.Errorf("bad repo permission: %s not in %v", v, repoPermissionLevels) + } + *l = v + return nil } // Branch contains general branch information.