Skip to content

Commit

Permalink
Support team owners for policies (#2953)
Browse files Browse the repository at this point in the history
  • Loading branch information
DazWorrall authored Jan 10, 2023
1 parent 87f9f9a commit 7746655
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 5 deletions.
3 changes: 3 additions & 0 deletions runatlantis.io/docs/access-credentials.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ GitHub App needs these permissions. These are automatically set when a GitHub ap

::: tip NOTE
Since v0.19.7, a new permission for `Administration` has been added. If you have already created a GitHub app, updating Atlantis to v0.19.7 will not automatically add this permission, so you will need to set it manually.

Since v0.22.3, a new permission for `Members` has been added, which is required for features that apply permissions to an organizations team members rather than individual users. Like the `Administration` permission above, updating Atlantis will not automatically add this permission, so if you wish to use features that rely on checking team membership you will need to add this manually.
:::

| Type | Access |
Expand All @@ -69,6 +71,7 @@ Since v0.19.7, a new permission for `Administration` has been added. If you have
| Metadata | Read-only (default) |
| Pull requests | Read and write |
| Webhooks | Read and write |
| Members | Read-only |

### GitLab
- Follow: [https://docs.gitlab.com/ce/user/profile/personal_access_tokens.html#create-a-personal-access-token](https://docs.gitlab.com/ce/user/profile/personal_access_tokens.html#create-a-personal-access-token)
Expand Down
3 changes: 2 additions & 1 deletion runatlantis.io/docs/server-side-repo-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,8 @@ If you set a workflow with the key `default`, it will override this.
### Owners
| Key | Type | Default | Required | Description |
|-------------|-------------------|---------|------------|---------------------------------------------------------|
| users | []string | none | yes | list of github users that can approve failing policies |
| users | []string | none | no | list of github users that can approve failing policies |
| teams | []string | none | no | list of github teams that can approve failing policies |

### PolicySet

Expand Down
1 change: 1 addition & 0 deletions server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
dbUpdater,
silenceNoProjects,
false,
e2eVCSClient,
)

unlockCommandRunner := events.NewUnlockCommandRunner(
Expand Down
1 change: 1 addition & 0 deletions server/controllers/github_app_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ func (g *GithubAppController) New(w http.ResponseWriter, r *http.Request) {
"repository_hooks": "write",
"statuses": "write",
"administration": "read",
"members": "read",
},
}

Expand Down
5 changes: 5 additions & 0 deletions server/core/config/raw/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func (p PolicySets) ToValid() valid.PolicySets {

type PolicyOwners struct {
Users []string `yaml:"users,omitempty" json:"users,omitempty"`
Teams []string `yaml:"teams,omitempty" json:"teams,omitempty"`
}

func (o PolicyOwners) ToValid() valid.PolicyOwners {
Expand All @@ -48,6 +49,10 @@ func (o PolicyOwners) ToValid() valid.PolicyOwners {
if len(o.Users) > 0 {
policyOwners.Users = o.Users
}

if len(o.Teams) > 0 {
policyOwners.Teams = o.Teams
}
return policyOwners
}

Expand Down
4 changes: 4 additions & 0 deletions server/core/config/raw/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,9 @@ func TestPolicySets_ToValid(t *testing.T) {
Users: []string{
"test",
},
Teams: []string{
"testteam",
},
},
PolicySets: []raw.PolicySet{
{
Expand All @@ -199,6 +202,7 @@ func TestPolicySets_ToValid(t *testing.T) {
Version: version,
Owners: valid.PolicyOwners{
Users: []string{"test"},
Teams: []string{"testteam"},
},
PolicySets: []valid.PolicySet{
{
Expand Down
15 changes: 14 additions & 1 deletion server/core/config/valid/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type PolicySets struct {

type PolicyOwners struct {
Users []string
Teams []string
}

type PolicySet struct {
Expand All @@ -35,12 +36,24 @@ func (p *PolicySets) HasPolicies() bool {
return len(p.PolicySets) > 0
}

func (p *PolicySets) IsOwner(username string) bool {
func (p *PolicySets) HasTeamOwners() bool {
return len(p.Owners.Teams) > 0
}

func (p *PolicySets) IsOwner(username string, userTeams []string) bool {
for _, uname := range p.Owners.Users {
if strings.EqualFold(uname, username) {
return true
}
}

for _, orgTeamName := range p.Owners.Teams {
for _, userTeamName := range userTeams {
if strings.EqualFold(orgTeamName, userTeamName) {
return true
}
}
}

return false
}
24 changes: 21 additions & 3 deletions server/events/approve_policies_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/runatlantis/atlantis/server/events/command"
"github.com/runatlantis/atlantis/server/events/models"
"github.com/runatlantis/atlantis/server/events/vcs"
)

func NewApprovePoliciesCommandRunner(
Expand All @@ -15,6 +16,7 @@ func NewApprovePoliciesCommandRunner(
dbUpdater *DBUpdater,
SilenceNoProjects bool,
silenceVCSStatusNoProjects bool,
vcsClient vcs.Client,
) *ApprovePoliciesCommandRunner {
return &ApprovePoliciesCommandRunner{
commitStatusUpdater: commitStatusUpdater,
Expand All @@ -24,6 +26,7 @@ func NewApprovePoliciesCommandRunner(
dbUpdater: dbUpdater,
SilenceNoProjects: SilenceNoProjects,
silenceVCSStatusNoProjects: silenceVCSStatusNoProjects,
vcsClient: vcsClient,
}
}

Expand All @@ -37,6 +40,7 @@ type ApprovePoliciesCommandRunner struct {
// are found
SilenceNoProjects bool
silenceVCSStatusNoProjects bool
vcsClient vcs.Client
}

func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) {
Expand Down Expand Up @@ -91,9 +95,23 @@ func (a *ApprovePoliciesCommandRunner) buildApprovePolicyCommandResults(ctx *com
// Check if vcs user is in the owner list of the PolicySets. All projects
// share the same Owners list at this time so no reason to iterate over each
// project.
if len(prjCmds) > 0 && !prjCmds[0].PolicySets.IsOwner(ctx.User.Username) {
result.Error = fmt.Errorf("contact policy owners to approve failing policies")
return
if len(prjCmds) > 0 {
teams := []string{}

// Only query the users team membership if any teams have been configured as owners.
if prjCmds[0].PolicySets.HasTeamOwners() {
userTeams, err := a.vcsClient.GetTeamNamesForUser(ctx.Pull.BaseRepo, ctx.User)
if err != nil {
ctx.Log.Err("unable to get team membership for user: %s", err)
return
}
teams = append(teams, userTeams...)
}

if !prjCmds[0].PolicySets.IsOwner(ctx.User.Username, teams) {
result.Error = fmt.Errorf("contact policy owners to approve failing policies")
return
}
}

var prjResults []command.ProjectResult
Expand Down
27 changes: 27 additions & 0 deletions server/events/approve_policies_command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,41 @@ func TestApproveCommandRunner_IsOwner(t *testing.T) {
cases := []struct {
Description string
OwnerUsers []string
OwnerTeams []string // Teams configured as owners
UserTeams []string // Teams the user is a member of
ExpComment string
}{
{
Description: "When user is not an owner, approval fails",
OwnerUsers: []string{},
OwnerTeams: []string{},
ExpComment: "**Approve Policies Error**\n```\ncontact policy owners to approve failing policies\n```\n",
},
{
Description: "When user is an owner, approval succeeds",
OwnerUsers: []string{fixtures.User.Username},
OwnerTeams: []string{},
ExpComment: "Approved Policies for 1 projects:\n\n1. dir: `` workspace: ``\n\n\n",
},
{
Description: "When user is an owner via team membership, approval succeeds",
OwnerUsers: []string{},
OwnerTeams: []string{"SomeTeam"},
UserTeams: []string{"SomeTeam"},
ExpComment: "Approved Policies for 1 projects:\n\n1. dir: `` workspace: ``\n\n\n",
},
{
Description: "When user belongs to a team not configured as a owner, approval fails",
OwnerUsers: []string{},
OwnerTeams: []string{"SomeTeam"},
UserTeams: []string{"SomeOtherTeam}"},
ExpComment: "**Approve Policies Error**\n```\ncontact policy owners to approve failing policies\n```\n",
},
{
Description: "When user is an owner but not a team member, approval succeeds",
OwnerUsers: []string{fixtures.User.Username},
OwnerTeams: []string{"SomeTeam"},
UserTeams: []string{"SomeOtherTeam"},
ExpComment: "Approved Policies for 1 projects:\n\n1. dir: `` workspace: ``\n\n\n",
},
}
Expand Down Expand Up @@ -57,10 +82,12 @@ func TestApproveCommandRunner_IsOwner(t *testing.T) {
PolicySets: valid.PolicySets{
Owners: valid.PolicyOwners{
Users: c.OwnerUsers,
Teams: c.OwnerTeams,
},
},
},
}, nil)
When(vcsClient.GetTeamNamesForUser(fixtures.GithubRepo, fixtures.User)).ThenReturn(c.UserTeams, nil)

approvePoliciesCommandRunner.Run(ctx, &events.CommentCommand{Name: command.ApprovePolicies})

Expand Down
1 change: 1 addition & 0 deletions server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock
dbUpdater,
testConfig.SilenceNoProjects,
false,
vcsClient,
)

unlockCommandRunner = events.NewUnlockCommandRunner(
Expand Down
1 change: 1 addition & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
dbUpdater,
userConfig.SilenceNoProjects,
userConfig.SilenceVCSStatusNoPlans,
vcsClient,
)

unlockCommandRunner := events.NewUnlockCommandRunner(
Expand Down

0 comments on commit 7746655

Please sign in to comment.